Skip to content

Commit df2b12b

Browse files
committed
Fix regression issue with preview_path=
The `preview_path` feature of `ActionMailer` was added in Rails 4.1. Versions before that release are not able to handle that configuration. The regression issue occurred in part to the spec suite overlooking changes to the environment and how that affects the shelling out. Additionally, the previous changes noted that the configuration for a particular module always responds to a setting; even when it should not. However, while this logic was noted for `show_previews`, a few lines further down, that same logic was overlooked for `preview_path`. It appears the only available methods for detecting `ActionMailer`, and it's features, is tasting the top-level `config` in addition to checking the Rails version string.
1 parent 88b3823 commit df2b12b

File tree

1 file changed

+12
-9
lines changed

1 file changed

+12
-9
lines changed

lib/rspec-rails.rb

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,12 @@ class Railtie < ::Rails::Railtie
2929
private
3030

3131
def setup_preview_path(app)
32-
# If the action mailer railtie isn't loaded the config will not respond
3332
return unless supports_action_mailer_previews?(app.config)
3433
options = app.config.action_mailer
3534
config_default_preview_path(options) if config_preview_path?(options)
3635
end
3736

3837
def config_preview_path?(options)
39-
# This string version check avoids loading the ActionMailer class, as
40-
# would happen using `defined?`. This is necessary because the
41-
# ActionMailer class only loads it's settings once, at load time. If we
42-
# load the class now any settings declared in a config block in an
43-
# initializer will be ignored.
44-
#
4538
# We cannot use `respond_to?(:show_previews)` here as it will always
4639
# return `true`.
4740
if ::Rails::VERSION::STRING < '4.2'
@@ -59,8 +52,18 @@ def config_default_preview_path(options)
5952
end
6053

6154
def supports_action_mailer_previews?(config)
62-
config.respond_to?(:action_mailer) &&
63-
config.action_mailer.respond_to?(:preview_path)
55+
# These checks avoid loading `ActionMailer`. Using `defined?` has the
56+
# side-effect of the class getting loaded if it is available. This is
57+
# problematic because loading `ActionMailer::Base` will cause it to
58+
# read the config settings; this is the only time the config is read.
59+
# If the config is loaded now, any settings declared in a config block
60+
# in an initializer will be ignored.
61+
#
62+
# If the action mailer railtie has not been loaded then `config` will
63+
# not respond to the method. However, we cannot use
64+
# `config.action_mailer.respond_to?(:preview_path)` here as it will
65+
# always return `true`.
66+
config.respond_to?(:action_mailer) && ::Rails::VERSION::STRING > '4.1'
6467
end
6568
end
6669
end

0 commit comments

Comments
 (0)