New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set action_mailer.default_url_options
values in development
and test
#51191
Set action_mailer.default_url_options
values in development
and test
#51191
Conversation
…test`. Prior to this commit, new Rails applications would raise `ActionView::Template::Error` if a mailer included a url built with a `*_path` helper. Since we already know [new apps will be served on `localhost:3000`][new apps], we set this as the value in `development`. In an effort to remain consistent with existing patters, we set the `host` to `www.example.com` in `test. [new apps]: https://github.com/rails/rails/blob/47300002db11d67d7b35103f5c429dad7dacdacd/README.md?plain=1#L81
fa72229
to
9412008
Compare
@@ -43,6 +43,8 @@ Rails.application.configure do | |||
config.action_mailer.raise_delivery_errors = false | |||
|
|||
config.action_mailer.perform_caching = false | |||
|
|||
config.action_mailer.define_url_options = { host: "localhost", port: 3000 } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though we can set this to anything, I felt it was important to keep it consistent with what Rails is served on, in an effort to make previews link to the application.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds like a helpful change—thanks! Quick question: The bin/rails server
command allows a --port XXXX
flag (and with bin/dev
you set it via an env var, e.g. PORT=XXXX bin/dev
). Any reason not to use that value, with a default of 3000
? I ask because I often use that flag when I have multiple rails apps running.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a great observation. I think we'd somehow need access to the value set in Rails::Commands::Server::ServerCommand
:
rails/railties/lib/rails/commands/server/server_command.rb
Lines 213 to 215 in bf8e0f5
def port | |
options[:port] || ENV.fetch("PORT", DEFAULT_PORT).to_i | |
end |
I attempted the following implementation, but if ENV["PORT"]
is not set, it won't match what's passed to the --port
flag.
config.action_mailer.default_url_options = { host: "localhost", port: ENV.fetch("PORT", 3000).to_i }
However, this seemed to work for the following cases:
- When
ENV["PORT"]
is supplied - When
--port
is supplied. - When neither
ENV["PORT"]
nor--port
is supplied (defaulting to3000
)
config.action_mailer.default_url_options = { host: "localhost", port: Rails::Server::Options.new.parse!(ARGV)[:Port] }
railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt
Outdated
Show resolved
Hide resolved
…onments/development.rb.tt Co-authored-by: Rafael Mendonça França <rafael@franca.dev>
…test` (rails#51191) * Set `action_mailer.default_url_options` values in `development` and `test`. Prior to this commit, new Rails applications would raise `ActionView::Template::Error` if a mailer included a url built with a `*_path` helper. Since we already know [new apps will be served on `localhost:3000`][new apps], we set this as the value in `development`. In an effort to remain consistent with existing patters, we set the `host` to `www.example.com` in `test. [new apps]: https://github.com/rails/rails/blob/47300002db11d67d7b35103f5c429dad7dacdacd/README.md?plain=1#L81 * Update railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt Co-authored-by: Rafael Mendonça França <rafael@franca.dev> --------- Co-authored-by: Rafael Mendonça França <rafael@franca.dev>
…test` (rails#51191) * Set `action_mailer.default_url_options` values in `development` and `test`. Prior to this commit, new Rails applications would raise `ActionView::Template::Error` if a mailer included a url built with a `*_path` helper. Since we already know [new apps will be served on `localhost:3000`][new apps], we set this as the value in `development`. In an effort to remain consistent with existing patters, we set the `host` to `www.example.com` in `test. [new apps]: https://github.com/rails/rails/blob/47300002db11d67d7b35103f5c429dad7dacdacd/README.md?plain=1#L81 * Update railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt Co-authored-by: Rafael Mendonça França <rafael@franca.dev> --------- Co-authored-by: Rafael Mendonça França <rafael@franca.dev>
Motivation / Background
Prior to this commit, new Rails applications would raise
ActionView::Template::Error
if a mailer included a url built with a*_path
helper.Since we already know new apps will be served on
localhost:3000
, we set this as the value indevelopment
.In an effort to remain consistent with existing patters, we set the
host
towww.example.com
intest
.Additional information
If we feel this is not the right solution, how do we feel about at least adding these values, but commented out?
Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]