1 files changed, 139 insertions, 0 deletions
diff --git a/doc/development/backend/ruby_style_guide.md b/doc/development/backend/ruby_style_guide.md
index 714c8571d20..d6e2f7a48d2 100644
--- a/doc/development/backend/ruby_style_guide.md
+++ b/doc/development/backend/ruby_style_guide.md
@@ -161,6 +161,145 @@ def method
 end
 ```
 
+## Avoid ActiveRecord callbacks
+
+[ActiveRecord callbacks](https://guides.rubyonrails.org/active_record_callbacks.html) allow
+you to "trigger logic before or after an alteration of an object's state."
+
+Use callbacks when no superior alternative exists, but employ them only if you
+thoroughly understand the reasons for doing so.
+
+When adding new lifecycle events for ActiveRecord objects, it is preferable to
+add the logic to a service class instead of a callback.
+
+## Why callbacks shoud be avoided
+
+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.
+- Heavy use of callbacks affects factory creation speed. With some classes
+  having hundreds of callbacks, creating an instance of that object for
+  an automated test can be a very slow operation, resulting in slow specs.
+
+Some of these examples are discussed in this [video from thoughtbot](https://www.youtube.com/watch?v=GLBMfB8N1G8).
+
+The GitLab codebase relies heavily on callbacks and it is hard to refactor them
+once added due to invisible dependencies. As a result, this guideline does not
+call for removing all existing callbacks.
+
+### When to use callbacks
+
+Callbacks can be used in special cases. Some examples of cases where adding a
+callback makes sense:
+
+- A dependency uses callbacks and we would like to override the callback
+  behavior.
+- Incrementing cache counts.
+- Data normalization that only relates to data on the current model.
+
+### Example of moving from a callback to a service
+
+There is a project with the following basic data model:
+
+```ruby
+class Project
+  has_one :repository
+end
+
+class Repository
+  belongs_to :project
+end
+```
+
+Say we want to create a repository after a project is created and use the
+project name as the repository name. A developer familiar with Rails might
+immediately think: sounds like a job for an ActiveRecord callback! And add this
+code:
+
+```ruby
+class Project
+  has_one :repository
+
+  after_initialize :create_random_name
+  after_create :create_repository
+
+  def create_random_name
+    SecureRandom.alphanumeric
+  end
+
+  def create_repository
+    Repository.create!(project: self)
+  end
+end
+
+class Repository
+  after_initialize :set_name
+
+  def set_name
+    name = project.name
+  end
+end
+
+class ProjectsController
+  def create
+    Project.create! # also creates a repository and names it
+  end
+end
+```
+
+While this seems pretty harmless for a baby Rails app, adding this type of logic
+via callbacks has many downsides once your Rails app becomes large and complex
+(all of which are listed in this documentation). Instead, we can add this
+logic to a service class:
+
+```ruby
+class Project
+  has_one :repository
+end
+
+class Repository
+  belongs_to :project
+end
+
+class ProjectCreator
+  def self.execute
+    ApplicationRecord.transaction do
+      name = SecureRandom.alphanumeric
+      project = Project.create!(name: name)
+      Repository.create!(project: project, name: name)
+    end
+  end
+end
+
+class ProjectsController
+  def create
+    ProjectCreator.execute
+  end
+end
+```
+
+With an application this simple, it can be hard to see the benefits of the second
+approach. But we already some benefits:
+
+- Can test `Repository` creation logic separate from `Project` creation logic. Code
+  no longer violates law of demeter (`Repository` class doesn't need to know
+  `project.name`).
+- Clarity of invocation order.
+- Open to change: if we decide there are some scenarios where we do not want a
+  repository created for a project, we can create a new service class rather
+  than needing to refactor to `Project` and `Repository` classes.
+- Each instance of a `Project` factory does not create a second (`Repository`) object.
+
 ## Styles we have no opinion on
 
 If a RuboCop rule is proposed and we choose not to add it, we should document that decision in this guide so it is more discoverable and link the relevant discussion as a reference.