Skip to content

Commit

Permalink
Fix ActionMailer asset host in production. (#1044)
Browse files Browse the repository at this point in the history
* Fix ActionMailer asset host in production (George)

This change was originally made in a suspenders-based app to fix a problem
we were having with images not rendering in emails. This commit replicates
the fix upstream for future suspenders users.

The change fixes image URLs in emails by adding a protocol to the
ActionMailer asset_host setting.

While the setting is called "host", based on the examples in the Rails
documentation, it could be more accurately described as a "URL-prefix" --
all of the examples include a protocol as well as a host.

Prior to this change, image URLs were generated in emails from production
environments with a protocol-relative URL (e.g. //example.com/image.png),
and were not rendering correctly in the various mail clients we tested.

* Fix regex that removes comment lines from Rails config files (Thiago)

That regex was causing the build to fail. The reason is that, when
generating a new Suspenders app, it removed the new asset host config
option introduced by George that contains a non-comment hashtag (#)
character.

Also, it was silently removing other lines of code that were being
confused as Ruby comments just because they had hashtag characters
somewhere (#) within them. Therefore, we are also fixing a bug here.

Unfortunately, the regex is a simplistic approach because it does not
factor in all possible Ruby comments. The only reliable way to remove
comments involves some AST parsing in order to reliably detect
comments and also preserve the general layout of the config files
without aggressively removing blank lines.

NOTE: The regex is buggy and may remove, for example, lines inside
heredoc strings starting with a # character, among others. Thankfully,
the config files we're dealing with here don't have such edge cases.

Co-authored-by: Thiago A. Silva <[email protected]>
  • Loading branch information
georgebrock and thiagoa authored Oct 29, 2021
1 parent 4e9ef77 commit 31ff7bc
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
6 changes: 4 additions & 2 deletions lib/suspenders/app_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,12 @@ def configure_action_mailer
action_mailer_host "test", %("www.example.com")
action_mailer_asset_host "test", %("http://www.example.com")
action_mailer_host "production", %{ENV.fetch("APPLICATION_HOST")}
# rubocop:disable Lint/InterpolationCheck
action_mailer_asset_host(
"production",
%{ENV.fetch("ASSET_HOST", ENV.fetch("APPLICATION_HOST"))}
%q{"https://#{ENV.fetch("ASSET_HOST", ENV.fetch("APPLICATION_HOST"))}"}
)
# rubocop:enable Lint/InterpolationCheck
end

def create_heroku_apps(flags)
Expand Down Expand Up @@ -206,7 +208,7 @@ def remove_config_comment_lines
path = File.join(destination_root, "config/#{config_file}")

accepted_content = File.readlines(path).reject { |line|
line =~ /^.*#.*$/ || line =~ /^$\n/
line =~ /^\s*#.*$/ || line =~ /^$\n/
}

File.open(path, "w") do |file|
Expand Down
6 changes: 4 additions & 2 deletions spec/features/new_project_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,10 @@

it "sets action mailer default host and asset host" do
config_key = "config.action_mailer.asset_host"
# rubocop:disable Lint/InterpolationCheck
config_value =
%q{ENV\.fetch\("ASSET_HOST", ENV\.fetch\("APPLICATION_HOST"\)\)}
%q{"https://#{ENV\.fetch\("ASSET_HOST", ENV\.fetch\("APPLICATION_HOST"\)\)}}
# rubocop:enable Lint/InterpolationCheck
expect(production_config).to match(/#{config_key} = #{config_value}/)
end

Expand Down Expand Up @@ -214,7 +216,7 @@
]

config_files.each do |file|
expect(file).not_to match(%r{.*#.*})
expect(file).not_to match(/^\s*#.*$/)
expect(file).not_to eq(file.strip)
expect(file).not_to match(%r{^$\n\n})
end
Expand Down

0 comments on commit 31ff7bc

Please sign in to comment.