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

Add Rails/ActionControllerFlashBeforeRender cop #759

Merged
merged 1 commit into from Sep 1, 2022

Conversation

americodls
Copy link
Contributor

This PR suggests a new Cop for detecting flash assignment before render a page (the correct approach would be to assign a message to flash.now (check on rails guides) .

This is a common mistake in Ruby on Rails, and it is easy to fall into this trap the flash will be rendered on the page. But the flash message still persisted and will show up on the following redirect.

This is an example of that issue: rubygems/rubygems.org#3149

Solved here: rubygems/rubygems.org#3198

This is a very simple version but helped me to find more 6 issues on that project.
I hope I can receive some suggestions on how to improve this solution.


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.
  • If this is a new cop, consider making a corresponding update to the Rails Style Guide.

config/default.yml Outdated Show resolved Hide resolved
config/default.yml Outdated Show resolved Hide resolved
config/default.yml Outdated Show resolved Hide resolved
lib/rubocop/cop/rails_cops.rb Outdated Show resolved Hide resolved
@americodls americodls force-pushed the flash-before-render branch 2 times, most recently from bd1d63d to da3f7ef Compare August 29, 2022 07:58
@americodls americodls requested a review from koic August 29, 2022 07:59
@simi
Copy link

simi commented Aug 29, 2022

Is there any way to scope it only to Rails controllers 🤔?

@americodls americodls force-pushed the flash-before-render branch 2 times, most recently from e625a35 to b697f90 Compare August 29, 2022 23:40
@americodls
Copy link
Contributor Author

americodls commented Aug 29, 2022

I added some changes to make the cop more specific and tidied up the tests.
Now the cop will check only instance methods and blocks passed to macros in a rails controller class.

@americodls americodls requested a review from koic August 31, 2022 00:00
@koic koic merged commit bf55de4 into rubocop:master Sep 1, 2022
@koic
Copy link
Member

koic commented Sep 1, 2022

Thanks!

@americodls americodls deleted the flash-before-render branch September 10, 2022 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants