Add before and around hooks by oriolgual · Pull Request #7 · factorialco/data_drip

oriolgual · 2026-02-06T08:12:07Z

Improves the hook system by adding before and around hooks for all status changes and update the naming so it's clear the existing ones run after the status change.

Copilot

Pull request overview

This PR expands DataDrip’s lifecycle hook system to support before_* and around_* hooks for status transitions (in addition to renaming the existing hooks to after_* to clarify timing), and refactors run/batch models to centralize hook execution via a new Hookable concern.

Changes:

Introduces DataDrip::Hookable and integrates it into BackfillRun and BackfillRunBatch to run before/around/after hooks around status transitions/actions.
Renames hook method conventions from on_* to after_*, and adds before_* / around_* variants in docs and test fixtures.
Updates specs and documentation to reflect new hook timing and ordering expectations.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
spec/test_app/app/services/hook_handler.rb	Updates test hook handler to implement `before/around/after` hook methods and record execution sequence.
spec/test_app/app/backfills/add_role_to_employee.rb	Updates test backfill to implement `before/around/after` hooks and record execution sequence.
spec/lib/data_drip/hook_handler_spec.rb	Updates expectations to `after_*` naming and adds ordering tests for before/around/after hooks.
spec/backfills/add_role_to_employee_spec.rb	Updates hook naming and asserts before/around/after ordering in sequences.
app/models/data_drip/backfill_run_batch.rb	Includes `Hookable`, wraps enum bang methods + enqueue/run! actions with hook execution, and refactors hook targeting.
app/models/data_drip/backfill_run.rb	Includes `Hookable`, wraps enum bang methods + enqueue action with hook execution, adds `with_run_hooks`, refactors hook targeting.
app/models/concerns/data_drip/hookable.rb	New concern implementing shared hook execution for status changes and action wrappers.
app/jobs/data_drip/dripper.rb	Wraps “running” execution in `with_run_hooks(:running)` so hooks can wrap the job action.
README.md	Updates hook documentation and examples to the new `before/around/after` naming and semantics.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

app/models/concerns/data_drip/hookable.rb

spec/lib/data_drip/hook_handler_spec.rb

oriolgual · 2026-02-13T09:25:02Z

@copilot fix conflicts and rubocop issues please

Copilot · 2026-02-13T09:25:10Z

@oriolgual I've opened a new pull request, #13, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-13T09:43:48Z

app/models/data_drip/backfill_run_batch.rb

+
    after_commit :enqueue, on: :create
-    after_commit :run_hooks
+    after_commit :run_status_change_hooks


Same concern as BackfillRun: after_commit :run_status_change_hooks will run for the initial pending status on create, and will execute before_*/around_* hooks even though the persistence operation already finished. Consider restricting this callback to after_* only, or removing it if all supported status transitions go through the overridden *_! methods / with_action_hooks.

Suggested change

after_commit :run_status_change_hooks

after_commit :run_status_change_hooks, on: :update

Copilot · 2026-02-13T09:43:49Z

README.md

@@ -128,63 +128,40 @@ Then create your hook handler class:
 ```ruby
 # app/services/hook_handler.rb
 class HookHandler
-  # Run hooks - triggered when a BackfillRun changes status
-  def self.on_run_pending(run)
-    # Called when a run is created
+  # Run hooks - triggered when a BackfillRun changes status or action starts
+  def self.before_run_enqueued(run)
+    # Called before the run is enqueued
  end

-  def self.on_run_enqueued(run)
+  def self.around_run_enqueued(run)
+    # Called around the enqueue action
+    yield
+  end
+
+  def self.after_run_enqueued(run)
    # Called when a run is enqueued for execution
    # Example: Track metrics
    Metrics.increment('backfill.enqueued', tags: { backfill: run.backfill_class_name })
  end

-  def self.on_run_running(run)
-    # Called when a run starts processing
-  end
-
-  def self.on_run_completed(run)
+  def self.after_run_completed(run)
    # Called when a run completes successfully
    # Example: Send notification
    SlackNotifier.notify("Backfill #{run.backfill_class_name} completed!")
  end

-  def self.on_run_failed(run)
+  def self.after_run_failed(run)
    # Called when a run fails
    # Example: Send error alert
    ErrorTracker.notify("Backfill failed: #{run.error_message}")
  end

-  def self.on_run_stopped(run)
-    # Called when a run is manually stopped
-  end
-
  # Batch hooks - triggered when a BackfillRunBatch changes status
-  def self.on_batch_pending(batch)
-    # Called when a batch is created
-  end
-
-  def self.on_batch_enqueued(batch)
-    # Called when a batch is enqueued
-  end
-
-  def self.on_batch_running(batch)
-    # Called when a batch starts processing
-  end
-
-  def self.on_batch_completed(batch)
+  def self.after_batch_completed(batch)
    # Called when a batch completes
    # Example: Update progress tracking
    ProgressTracker.update(batch.backfill_run_id, batch.id)
  end
-
-  def self.on_batch_failed(batch)
-    # Called when a batch fails
-  end
-
-  def self.on_batch_stopped(batch)
-    # Called when a batch is stopped
-  end
 end
 ```

@@ -196,29 +173,25 @@ DataDrip provides hooks for both backfill runs (entire backfill execution) and b

 #### Run Hooks

-These hooks receive a `BackfillRun` object as a parameter:
+These hooks receive a `BackfillRun` object as a parameter. For every status, you can define any or all of the following:

-| Hook               | Triggered When                    |
-| ------------------ | --------------------------------- |
-| `on_run_pending`   | Run is created                    |
-| `on_run_enqueued`  | Run is enqueued for execution     |
-| `on_run_running`   | Run starts processing batches     |
-| `on_run_completed` | All batches complete successfully |
-| `on_run_failed`    | Run encounters an error           |
-| `on_run_stopped`   | Run is manually stopped           |
+- `before_run_<status>` (runs first)
+- `around_run_<status>` (wraps the action and must `yield`)
+- `after_run_<status>` (runs last)
+
+Valid statuses: `pending`, `enqueued`, `running`, `completed`, `failed`, `stopped`.
+Hooks always wrap the action that performs the status transition. The `around_*` hook wraps the transition itself, and the `after_*` hook runs after the status update and after the `around_*` hook completes.

 #### Batch Hooks

-These hooks receive a `BackfillRunBatch` object as a parameter:
+These hooks receive a `BackfillRunBatch` object as a parameter. For every status, you can define any or all of the following:
+
+- `before_batch_<status>` (runs first)
+- `around_batch_<status>` (wraps the action and must `yield`)
+- `after_batch_<status>` (runs last)

-| Hook                 | Triggered When                   |
-| -------------------- | -------------------------------- |
-| `on_batch_pending`   | Batch is created                 |
-| `on_batch_enqueued`  | Batch is enqueued for processing |
-| `on_batch_running`   | Batch starts processing records  |
-| `on_batch_completed` | Batch completes successfully     |
-| `on_batch_failed`    | Batch encounters an error        |
-| `on_batch_stopped`   | Batch is stopped                 |
+Valid statuses: `pending`, `enqueued`, `running`, `completed`, `failed`, `stopped`.
+Hooks always wrap the action that performs the status transition. The `around_*` hook wraps the transition itself, and the `after_*` hook runs after the status update and after the `around_*` hook completes.


The documentation states that hooks “always wrap the action that performs the status transition”, but the implementation also runs hooks from an after_commit callback (where “before” is no longer before, and around_* may wrap a no-op). Please clarify in the README when hooks truly wrap an action (e.g., only when transitions use the provided *_!/enqueue/run! APIs) vs when they run post-commit for out-of-band status updates.

Copilot · 2026-02-13T09:43:49Z

app/models/data_drip/backfill_run.rb


    after_commit :enqueue
-    after_commit :run_hooks
+    after_commit :run_status_change_hooks


With the new Hookable approach, after_commit :run_status_change_hooks will invoke before_*/around_*/after_* hooks for the initial pending status on create (and for any direct status updates), but there’s no corresponding action being wrapped at that point. Consider whether this callback should be kept, and if so, whether it should only run after_* hooks to preserve consistent semantics.

Suggested change

after_commit :run_status_change_hooks

Avalonvdhorst

Nice!!

oriolgual force-pushed the improved-hooks branch from b332c96 to 1d7bc59 Compare February 6, 2026 08:12

oriolgual mentioned this pull request Feb 6, 2026

set current company scope before enqueue #6

Open

oriolgual requested a review from Copilot February 6, 2026 11:25

Copilot started reviewing on behalf of oriolgual February 6, 2026 11:25 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

app/models/concerns/data_drip/hookable.rb Outdated Show resolved Hide resolved

app/models/concerns/data_drip/hookable.rb Show resolved Hide resolved

spec/lib/data_drip/hook_handler_spec.rb Outdated Show resolved Hide resolved

Copilot AI mentioned this pull request Feb 13, 2026

Fix rubocop spacing violation in Hookable concern #13

Merged

Add before and around hooks

fd3bb45

oriolgual force-pushed the improved-hooks branch from e850813 to fd3bb45 Compare February 13, 2026 09:37

oriolgual requested a review from Copilot February 13, 2026 09:38

Copilot started reviewing on behalf of oriolgual February 13, 2026 09:39 View session

Copilot AI reviewed Feb 13, 2026

View reviewed changes

Avalonvdhorst approved these changes Feb 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add before and around hooks#7

Add before and around hooks#7
oriolgual wants to merge 1 commit intomainfrom
improved-hooks

oriolgual commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oriolgual commented Feb 13, 2026

Uh oh!

Copilot AI commented Feb 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Avalonvdhorst left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	after_commit :run_status_change_hooks
	after_commit :run_status_change_hooks, on: :update

Conversation

oriolgual commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oriolgual commented Feb 13, 2026

Uh oh!

Copilot AI commented Feb 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Avalonvdhorst left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants