Skip to content
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

Existing mailer previews are no longer available after updating to rspec-rails 3.2.1 #1333

Open
cupakromer opened this issue Mar 5, 2015 · 5 comments
Assignees

Comments

@cupakromer
Copy link
Member

Issue

Existing mailer previews are no longer accessible after updating to rspec-rails 3.2.1.

This was reported in #1313 (comment)

Root Cause

This issue affects projects which previously created mailer previews, either manually, or with the Minitest hook into the default test/mailers/previews location. Since that was the default path, it is not likely those projects added an explicit path to the config. After updating, rspec-rails now sets it's path spec/mailers/previews if no explicit path has been configured. So the old previews are no longer in the path.

Workaround

Set an explicit preview path in the config:

config.action_mailer.preview_path = 'test/mailers/previews'
@cupakromer cupakromer self-assigned this Mar 5, 2015
@fables-tales
Copy link
Member

@cupakromer would a simple solution to this to check test/mailers/previews when rails g rspec:install happens and copy them into spec/mailers/previews when that happens?

@stevecondylios
Copy link

stevecondylios commented Feb 3, 2021

Thank you @penelopezone that's a very simple fix. Worked for me (rails 6.0.3).

I wonder if a prominent console message after rails g rspec:install could help, just as a reminder this needs to be done, as it likely tripped up others too: https://stackoverflow.com/q/39204047

@JonRowe
Copy link
Member

JonRowe commented Feb 3, 2021

If you'd like to take a go at detecting this situation and issuing a warning that would be welcome @stevecondylios

@AFlowOfCode
Copy link

Why is it even necessary to automatically set the preview path to spec/mailers/previews under any circumstance?

If the path is explicitly set that means the previews are set up somewhere else, so rspec-rails takes no action. Yet if people are fine with using the default test/mailers/previews location, there's zero reason they would have explicitly set this, even though it is fulfilling the same conditions that indicate to rspec-rails the preview path should not be touched.

It makes more sense to log in the console during install that if so desired, the preview path can be manually switched to spec/mailers/previews. This is simpler than trying to detect or copy things, which may not even be desired behavior 100% of the time.

Doing it automatically is just asking for a random amount of users to suddenly have broken previews, and it may take a while to notice and figure out it was rspec-rails' fault as well.

@JonRowe
Copy link
Member

JonRowe commented Apr 9, 2021

Why is it even necessary to automatically set the preview path to spec/mailers/previews under any circumstance?

It's not "necessary" however it would be strange to have a test folder that only contained mailers/previews and nothing else, so the decision was taken for the majority of our developers who like to have everything in spec.

Yet if people are fine with using the default test/mailers/previews location, there's zero reason they would have explicitly set this, even though it is fulfilling the same conditions that indicate to rspec-rails the preview path should not be touched.

This is the problem, we don't know they are fine with it, as nobody has added detection code for its use, they could equally be using spec/mailers/previews quite happily.

Doing it automatically is just asking for a random amount of users to suddenly have broken previews, and it may take a while to notice and figure out it was rspec-rails' fault as well.

This only affected people during the install of rspec-rails, not many people switch half way through an apps lifetime, this original issue was from an upgrade from a version that didn't support this at all.

If you'd like to add a PR that skips this config if the folder is in use, I'd happily review it but our time as volunteers is limited and this is low priority for us so it's never been looked at.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants