Simplify PushTarget by removing template method pattern

Each target now implements process directly with its own logic,
rather than using processable?/perform_push hooks. The pushable?
check is done once in Notification#push before iterating targets.

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

app/models/notification/push_target.rb

@@ -12,17 +12,6 @@ class Notification::PushTarget
   end
 
   def process
-    return unless should_push?
-
-    perform_push
+    raise NotImplementedError
   end
-
-  private
-    def should_push?
-      notification.pushable?
-    end
-
-    def perform_push
-      raise NotImplementedError
-    end
 end

app/models/notification/push_target/web.rb

@@ -1,13 +1,11 @@
 class Notification::PushTarget::Web < Notification::PushTarget
-  private
-    def should_push?
-      super && subscriptions.any?
-    end
-
-    def perform_push
+  def process
+    if subscriptions.any?
       Rails.configuration.x.web_push_pool.queue(notification.payload.to_h, subscriptions)
     end
+  end
 
+  private
     def subscriptions
       @subscriptions ||= notification.user.push_subscriptions
     end

app/models/notification/pushable.rb

@@ -29,11 +29,9 @@ module Notification::Pushable
   end
 
   def push
-    self.class.push_targets.each { |target| push_to(target) }
-  end
+    return unless pushable?
 
-  def pushable?
-    !creator.system? && user.active? && account.active?
+    self.class.push_targets.each { |target| push_to(target) }
   end
 
   def payload
@@ -41,6 +39,10 @@ module Notification::Pushable
   end
 
   private
+    def pushable?
+      !creator.system? && user.active? && account.active?
+    end
+
     def push_to(target)
       target.process(self)
     end

saas/app/models/notification/push_target/native.rb

@@ -1,13 +1,11 @@
 class Notification::PushTarget::Native < Notification::PushTarget
-  private
-    def should_push?
-      super && devices.any?
-    end
-
-    def perform_push
+  def process
+    if devices.any?
       native_notification.deliver_later_to(devices)
     end
+  end
 
+  private
     def devices
       @devices ||= notification.identity.devices
     end

saas/test/models/notification/push_target/native_test.rb

@@ -50,16 +50,6 @@ class Notification::PushTarget::NativeTest < ActiveSupport::TestCase
     end
   end
 
-  test "does not push when creator is system user" do
-    stub_push_services
-    @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone")
-    @notification.update!(creator: users(:system))
-
-    assert_no_native_push_delivery do
-      Notification::PushTarget::Native.new(@notification).process
-    end
-  end
-
   test "pushes to multiple devices" do
     stub_push_services
     @identity.devices.delete_all

test/models/notification/push_target/web_test.rb

@@ -37,20 +37,6 @@ class Notification::PushTarget::WebTest < ActiveSupport::TestCase
     Notification::PushTarget::Web.new(@notification).process
   end
 
-  test "does not push for cancelled accounts" do
-    @user.account.cancel(initiated_by: @user)
-    @web_push_pool.expects(:queue).never
-
-    Notification::PushTarget::Web.new(@notification).process
-  end
-
-  test "does not push when creator is system user" do
-    @notification.update!(creator: users(:system))
-    @web_push_pool.expects(:queue).never
-
-    Notification::PushTarget::Web.new(@notification).process
-  end
-
   test "payload includes card title for card events" do
     @web_push_pool.expects(:queue).once.with do |payload, _|
       payload[:title] == @notification.card.title

test/models/notification/pushable_test.rb

@@ -46,19 +46,43 @@ class Notification::PushableTest < ActiveSupport::TestCase
     Notification.push_targets = original_targets
   end
 
-  test "pushable? returns true for normal notifications" do
-    assert @notification.pushable?
+  test "push processes targets for normal notifications" do
+    target_class = mock("push_target_class")
+    target_class.expects(:process).with(@notification)
+
+    original_targets = Notification.push_targets
+    Notification.push_targets = [ target_class ]
+
+    @notification.push
+  ensure
+    Notification.push_targets = original_targets
   end
 
-  test "pushable? returns false when creator is system user" do
+  test "push skips targets when creator is system user" do
     @notification.update!(creator: users(:system))
 
-    assert_not @notification.pushable?
+    target_class = mock("push_target_class")
+    target_class.expects(:process).never
+
+    original_targets = Notification.push_targets
+    Notification.push_targets = [ target_class ]
+
+    @notification.push
+  ensure
+    Notification.push_targets = original_targets
   end
 
-  test "pushable? returns false for cancelled accounts" do
+  test "push skips targets for cancelled accounts" do
     @user.account.cancel(initiated_by: @user)
 
-    assert_not @notification.pushable?
+    target_class = mock("push_target_class")
+    target_class.expects(:process).never
+
+    original_targets = Notification.push_targets
+    Notification.push_targets = [ target_class ]
+
+    @notification.push
+  ensure
+    Notification.push_targets = original_targets
   end
 end