From a729967e8a4a924d6cccd935b9b1d779eaf7e4c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 24 Sep 2022 15:37:35 +0200 Subject: [PATCH 1/2] Make Devise::Mailer inherit from ApplicationMailer This way we remove duplication in the `from:` proc, the helpers, and the methods we're about to write. --- app/mailers/devise_mailer.rb | 1 - config/initializers/devise.rb | 6 +++++- spec/mailers/devise_mailer_spec.rb | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/mailers/devise_mailer.rb b/app/mailers/devise_mailer.rb index 30d404a9f..a3f3ed271 100644 --- a/app/mailers/devise_mailer.rb +++ b/app/mailers/devise_mailer.rb @@ -1,5 +1,4 @@ class DeviseMailer < Devise::Mailer - helper :application, :settings, :mailer include Devise::Controllers::UrlHelpers default template_path: "devise/mailer" diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index cf6788ff9..aeb30c2f1 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -14,11 +14,15 @@ Devise.setup do |config| # Configure the e-mail address which will be shown in Devise::Mailer, # note that it will be overwritten if you use your own mailer class # with default "from" parameter. - config.mailer_sender = proc { "'#{Setting["mailer_from_name"]}' <#{Setting["mailer_from_address"]}>" } + # We're not setting it here because it's set by the ApplicationMailer class + # config.mailer_sender = "please-change-me-at-config-initializers-devise@example.com" # Configure the class responsible to send e-mails. config.mailer = "DeviseMailer" + # Configure the parent class responsible to send e-mails. + config.parent_mailer = "ApplicationMailer" + # ==> ORM configuration # Load and configure the ORM. Supports :active_record (default) and # :mongoid (bson_ext recommended) by default. Other ORMs may be diff --git a/spec/mailers/devise_mailer_spec.rb b/spec/mailers/devise_mailer_spec.rb index b5c0fb124..3a53daa08 100644 --- a/spec/mailers/devise_mailer_spec.rb +++ b/spec/mailers/devise_mailer_spec.rb @@ -18,7 +18,7 @@ describe DeviseMailer do email = DeviseMailer.confirmation_instructions(create(:user), "ABC") - expect(email).to deliver_from "'New organization' " + expect(email).to deliver_from "New organization " end end end From 981275733254c24a6b1581774987c46688d90e64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 6 Oct 2022 13:08:35 +0200 Subject: [PATCH 2/2] Set the asset host based on default_url_options We were duplicating the asset host and the URL host in all environments, but we can make it so the asset host uses the URL host unless we specifically set it. Note that, inside the `ApplicationMailer`, the `root_url` method already uses `default_url_options` to generate the URL. In the rare case of CONSUL installations who have changed the asset host, this change has no effect since they'll get a conflict in the environment files when upgrading and they'll choose to use their own asset host. --- app/mailers/application_mailer.rb | 5 +++ config/environments/development.rb | 1 - config/environments/production.rb | 1 - config/environments/staging.rb | 1 - config/environments/test.rb | 1 - spec/mailers/application_mailer_spec.rb | 41 +++++++++++++++++++++++++ 6 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 spec/mailers/application_mailer_spec.rb diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 7ccbcecd5..ed9e0b7b2 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -4,4 +4,9 @@ class ApplicationMailer < ActionMailer::Base helper :mailer default from: proc { "#{Setting["mailer_from_name"]} <#{Setting["mailer_from_address"]}>" } layout "mailer" + before_action :set_asset_host + + def set_asset_host + self.asset_host ||= root_url.delete_suffix("/") + end end diff --git a/config/environments/development.rb b/config/environments/development.rb index 3a2780781..42304e757 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -31,7 +31,6 @@ Rails.application.configure do # Don't care if the mailer can't send. config.action_mailer.raise_delivery_errors = false config.action_mailer.default_url_options = { host: "localhost", port: 3000 } - config.action_mailer.asset_host = "http://localhost:3000" # Deliver emails to a development mailbox at /letter_opener config.action_mailer.delivery_method = :letter_opener diff --git a/config/environments/production.rb b/config/environments/production.rb index 5c61e471a..2eb1a3cdd 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -70,7 +70,6 @@ Rails.application.configure do # Set this to true and configure the email server for immediate delivery to raise delivery errors. config.action_mailer.raise_delivery_errors = true config.action_mailer.default_url_options = { host: Rails.application.secrets.server_name } - config.action_mailer.asset_host = "https://#{Rails.application.secrets.server_name}" # Configure your SMTP service credentials in secrets.yml if Rails.application.secrets.smtp_settings diff --git a/config/environments/staging.rb b/config/environments/staging.rb index 7b3c2ae66..1d947e064 100644 --- a/config/environments/staging.rb +++ b/config/environments/staging.rb @@ -69,7 +69,6 @@ Rails.application.configure do # Set this to true and configure the email server for immediate delivery to raise delivery errors. config.action_mailer.raise_delivery_errors = true config.action_mailer.default_url_options = { host: Rails.application.secrets.server_name } - config.action_mailer.asset_host = "https://#{Rails.application.secrets.server_name}" # Configure your SMTP service credentials in secrets.yml if Rails.application.secrets.smtp_settings diff --git a/config/environments/test.rb b/config/environments/test.rb index fb5deebb8..e58456200 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -45,7 +45,6 @@ Rails.application.configure do # ActionMailer::Base.deliveries array. config.action_mailer.delivery_method = :test config.action_mailer.default_url_options = { host: "test" } - config.action_mailer.asset_host = "http://consul.test" # Print deprecation notices to the stderr. config.active_support.deprecation = :stderr diff --git a/spec/mailers/application_mailer_spec.rb b/spec/mailers/application_mailer_spec.rb new file mode 100644 index 000000000..6a189c235 --- /dev/null +++ b/spec/mailers/application_mailer_spec.rb @@ -0,0 +1,41 @@ +require "rails_helper" + +describe ApplicationMailer do + describe "#set_asset_host" do + let(:mailer) { ApplicationMailer.new } + + it "returns a host based on the default_url_options by default" do + allow(ActionMailer::Base).to receive(:default_url_options).and_return(host: "consul.dev") + + mailer.set_asset_host + + expect(mailer.asset_host).to eq "http://consul.dev" + end + + it "considers options like port and protocol" do + allow(ActionMailer::Base).to receive(:default_url_options).and_return( + host: "localhost", + protocol: "https", + port: 3000 + ) + + mailer.set_asset_host + + expect(mailer.asset_host).to eq "https://localhost:3000" + end + + it "returns the asset host when set manually" do + default_asset_host = ActionMailer::Base.asset_host + + begin + ActionMailer::Base.asset_host = "http://consulassets.dev" + + mailer.set_asset_host + + expect(mailer.asset_host).to eq "http://consulassets.dev" + ensure + ActionMailer::Base.asset_host = default_asset_host + end + end + end +end