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

Cop for detecting consecutive after_commit hooks for the same model #52

Closed
thisismydesign opened this issue May 10, 2019 · 3 comments · Fixed by #287
Closed

Cop for detecting consecutive after_commit hooks for the same model #52

thisismydesign opened this issue May 10, 2019 · 3 comments · Fixed by #287

Comments

@thisismydesign
Copy link

thisismydesign commented May 10, 2019

Problem

There can only be one after_commit hook defined for a model. This, however, is pretty confusing in the light of the on: option and its abbreviations (after_create_commit, after_update_commit and after_destroy_commit). For example, the following examples all result in a single after_commit hook being defined:

after_create_commit :log_create
after_update_commit :log_update
after_commit :callback, :on => :create
after_commit :callback, :on => :update, :if => :condition

See also:

Interestingly enough, the following works

after_create_commit -> { foo }
after_update_commit -> { foo }

See also:

So the cop could recommend to only use this format.

Describe the solution you'd like

I'd like rubocop to be able to detect this scenario and only allow for one after_commit and its abbreviations per model.

@Kani999
Copy link

Kani999 commented Apr 30, 2020

Still same problem. Costs me a lot of debugging.

Weird is that
after_create_commit :add_ac_role - Does not work
after_update_commit :add_ac_role - Does work

@thisismydesign
Copy link
Author

Both "work" it's just that they overwrite each other.

@fatkodima
Copy link
Contributor

Hey, I have implemented a cop for this 😄

@koic koic closed this as completed in #287 Jul 27, 2020
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 a pull request may close this issue.

3 participants