From 3e5233239ba74d90c2e2576c9c22dac85518736b Mon Sep 17 00:00:00 2001
From: Rosa Gutierrez <rosa@37signals.com>
Date: Fri, 20 Feb 2026 18:22:41 +0100
Subject: [PATCH] Fix push notification not firing on notification creation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Rails only applies the last callback when `after_create_commit` and
  `after_update_commit` reference the same method name [1]:

> However, if you use the `after_create_commit` and the
`after_update_commit` callback with the same method name, it will only
allow the last callback defined to take effect, as they both internally
alias to `after_commit` which overrides previously defined callbacks
with the same method name.

- Push notifications were never sent when a notification was first
  created — only when the source was updated
- Replaced the two callbacks with a single `after_save_commit`, which
  fires on both create and update, with the
  `source_id_previously_changed?` guard (true for both new records and
  source changes)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

[1] https://guides.rubyonrails.org/active_record_callbacks.html#aliases-for-after-commit
---
 app/models/concerns/push_notifiable.rb       |  3 +--
 test/models/concerns/push_notifiable_test.rb | 28 ++++++++++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)
 create mode 100644 test/models/concerns/push_notifiable_test.rb

diff --git a/app/models/concerns/push_notifiable.rb b/app/models/concerns/push_notifiable.rb
index e7bfdc56a..4bf9b575d 100644
--- a/app/models/concerns/push_notifiable.rb
+++ b/app/models/concerns/push_notifiable.rb
@@ -2,8 +2,7 @@ module PushNotifiable
   extend ActiveSupport::Concern
 
   included do
-    after_create_commit :push_notification_later
-    after_update_commit :push_notification_later, if: :source_id_previously_changed?
+    after_save_commit :push_notification_later, if: :source_id_previously_changed?
   end
 
   private
diff --git a/test/models/concerns/push_notifiable_test.rb b/test/models/concerns/push_notifiable_test.rb
new file mode 100644
index 000000000..f6d7ed26d
--- /dev/null
+++ b/test/models/concerns/push_notifiable_test.rb
@@ -0,0 +1,28 @@
+require "test_helper"
+
+class PushNotifiableTest < ActiveSupport::TestCase
+  test "enqueues push notification job when notification is created" do
+    assert_enqueued_with(job: PushNotificationJob) do
+      users(:david).notifications.create!(
+        source: events(:layout_published),
+        creator: users(:jason)
+      )
+    end
+  end
+
+  test "enqueues push notification job when notification source changes" do
+    notification = notifications(:logo_mentioned_david)
+
+    assert_enqueued_with(job: PushNotificationJob) do
+      notification.update!(source: events(:logo_published))
+    end
+  end
+
+  test "does not enqueue push notification job for other updates" do
+    notification = notifications(:logo_mentioned_david)
+
+    assert_no_enqueued_jobs only: PushNotificationJob do
+      notification.update!(unread_count: 5)
+    end
+  end
+end