From 3621df8f0fc01b83e393430109fe14d337e518b6 Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Thu, 22 Jan 2026 17:19:21 +0100 Subject: [PATCH] Consolidate push jobs into single Notification::PushJob Replace separate WebPushJob and NativePushJob with a single PushJob that calls notification.push, which iterates over registered targets. Each target handles its own delivery - Web pushes synchronously via the pool, Native enqueues device-level jobs via deliver_later_to. Co-Authored-By: Claude Opus 4.5 --- app/jobs/notification/push_job.rb | 5 +++++ app/jobs/notification/web_push_job.rb | 5 ----- app/models/notification/push_target/web.rb | 4 ---- app/models/notification/pushable.rb | 6 +++++- saas/app/jobs/notification/native_push_job.rb | 5 ----- .../models/notification/push_target/native.rb | 4 ---- .../notification/push_target/native_test.rb | 5 ----- .../notification/push_target/web_test.rb | 5 ----- test/models/notification/pushable_test.rb | 19 ++++++++++++++----- 9 files changed, 24 insertions(+), 34 deletions(-) create mode 100644 app/jobs/notification/push_job.rb delete mode 100644 app/jobs/notification/web_push_job.rb delete mode 100644 saas/app/jobs/notification/native_push_job.rb diff --git a/app/jobs/notification/push_job.rb b/app/jobs/notification/push_job.rb new file mode 100644 index 000000000..233762b37 --- /dev/null +++ b/app/jobs/notification/push_job.rb @@ -0,0 +1,5 @@ +class Notification::PushJob < ApplicationJob + def perform(notification) + notification.push + end +end diff --git a/app/jobs/notification/web_push_job.rb b/app/jobs/notification/web_push_job.rb deleted file mode 100644 index 3fb83a513..000000000 --- a/app/jobs/notification/web_push_job.rb +++ /dev/null @@ -1,5 +0,0 @@ -class Notification::WebPushJob < ApplicationJob - def perform(notification) - Notification::PushTarget::Web.new(notification).push - end -end diff --git a/app/models/notification/push_target/web.rb b/app/models/notification/push_target/web.rb index fe9b72587..68c971d8f 100644 --- a/app/models/notification/push_target/web.rb +++ b/app/models/notification/push_target/web.rb @@ -1,8 +1,4 @@ class Notification::PushTarget::Web < Notification::PushTarget - def self.push_later(notification) - Notification::WebPushJob.perform_later(notification) - end - private def should_push? super && subscriptions.any? diff --git a/app/models/notification/pushable.rb b/app/models/notification/pushable.rb index 8b90a36ba..c8519951a 100644 --- a/app/models/notification/pushable.rb +++ b/app/models/notification/pushable.rb @@ -24,8 +24,12 @@ module Notification::Pushable end def push_later + Notification::PushJob.perform_later(self) + end + + def push self.class.push_targets.each do |target| - target.push_later(self) + target.new(self).push end end diff --git a/saas/app/jobs/notification/native_push_job.rb b/saas/app/jobs/notification/native_push_job.rb deleted file mode 100644 index e06fc18d2..000000000 --- a/saas/app/jobs/notification/native_push_job.rb +++ /dev/null @@ -1,5 +0,0 @@ -class Notification::NativePushJob < ApplicationJob - def perform(notification) - Notification::PushTarget::Native.new(notification).push - end -end diff --git a/saas/app/models/notification/push_target/native.rb b/saas/app/models/notification/push_target/native.rb index 4a4e77be3..28727ff0c 100644 --- a/saas/app/models/notification/push_target/native.rb +++ b/saas/app/models/notification/push_target/native.rb @@ -1,8 +1,4 @@ class Notification::PushTarget::Native < Notification::PushTarget - def self.push_later(notification) - Notification::NativePushJob.perform_later(notification) - end - private def should_push? super && devices.any? diff --git a/saas/test/models/notification/push_target/native_test.rb b/saas/test/models/notification/push_target/native_test.rb index 162b7a32c..2a098ebac 100644 --- a/saas/test/models/notification/push_target/native_test.rb +++ b/saas/test/models/notification/push_target/native_test.rb @@ -199,9 +199,4 @@ class Notification::PushTarget::NativeTest < ActiveSupport::TestCase assert_equal @notification.creator.name, native.data[:creator_name] end - test "push_later enqueues Notification::NativePushJob" do - assert_enqueued_with(job: Notification::NativePushJob, args: [ @notification ]) do - Notification::PushTarget::Native.push_later(@notification) - end - end end diff --git a/test/models/notification/push_target/web_test.rb b/test/models/notification/push_target/web_test.rb index abf95f386..627f15357 100644 --- a/test/models/notification/push_target/web_test.rb +++ b/test/models/notification/push_target/web_test.rb @@ -92,9 +92,4 @@ class Notification::PushTarget::WebTest < ActiveSupport::TestCase Notification::PushTarget::Web.new(notification).push end - test "push_later enqueues Notification::WebPushJob" do - assert_enqueued_with(job: Notification::WebPushJob, args: [ @notification ]) do - Notification::PushTarget::Web.push_later(@notification) - end - end end diff --git a/test/models/notification/pushable_test.rb b/test/models/notification/pushable_test.rb index 898061b51..bdc3f2034 100644 --- a/test/models/notification/pushable_test.rb +++ b/test/models/notification/pushable_test.rb @@ -9,14 +9,23 @@ class Notification::PushableTest < ActiveSupport::TestCase ) end - test "push_later calls push_later on all registered targets" do - target = mock("push_target") - target.expects(:push_later).with(@notification) + test "push_later enqueues Notification::PushJob" do + assert_enqueued_with(job: Notification::PushJob, args: [ @notification ]) do + @notification.push_later + end + end + + test "push calls push on all registered targets" do + target_class = mock("push_target_class") + target_instance = mock("push_target_instance") + + target_class.expects(:new).with(@notification).returns(target_instance) + target_instance.expects(:push) original_targets = Notification.push_targets - Notification.push_targets = [ target ] + Notification.push_targets = [ target_class ] - @notification.push_later + @notification.push ensure Notification.push_targets = original_targets end