Styleguide: avoid ActiveRecord callbacks
What does this MR do and why?
- This has come up in a few code reviews but we have no styleguide around it that I am aware of.
- In general, callbacks should be avoided because:
- Callbacks are hard to reason about because invocation order is not obvious and they break code narrative.
- Callbacks are harder to locate and navigate because they rely on reflection to trigger rather than being ordinary method calls.
- Callbacks make it difficult to apply changes selectively to an object's state because changes always trigger the entire callback chain.
- Callbacks trap logic in the ActiveRecord class. This tight coupling encourages fat models that contain too much business logic, which could instead live in service objects that are more reusable, composable, and are easier to test.
- Illegal state transitions of an object can be better enforced through attribute validations.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Edited by Jessie Young