Add integration test for notification delivery and simplify test helpers

- Add comprehensive integration test covering card assignment, comments,
  mentions, email bundling, and edge cases (system user, inactive user)
- Inline push notification test helpers into the only test that uses them
- Remove separate PushNotificationTestHelper module

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

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

@@ -189,4 +189,20 @@ class Notification::PushTarget::NativeTest < ActiveSupport::TestCase
     assert_equal @notification.creator.name, native.data[:creator_name]
   end
 
+  private
+    def assert_native_push_delivery(count: 1, &block)
+      assert_enqueued_jobs count, only: ApplicationPushNotificationJob do
+        perform_enqueued_jobs only: Notification::PushJob, &block
+      end
+    end
+
+    def assert_no_native_push_delivery(&block)
+      assert_enqueued_jobs 0, only: ApplicationPushNotificationJob do
+        perform_enqueued_jobs only: Notification::PushJob, &block
+      end
+    end
+
+    def stub_push_services
+      ActionPushNative.stubs(:service_for).returns(stub(push: true))
+    end
 end

saas/test/models/push_config_test.rb

@@ -1,21 +0,0 @@
-require "test_helper"
-
-class PushConfigTest < ActiveSupport::TestCase
-  test "loads push config from the saas engine" do
-    skip unless Fizzy.saas?
-
-    config = ActionPushNative.config
-
-    apple_team_id = config.dig(:apple, :team_id)
-    apple_topic = config.dig(:apple, :topic)
-    google_project_id = config.dig(:google, :project_id)
-
-    skip "Update test once APNS team_id is configured" if apple_team_id == "YOUR_TEAM_ID"
-    skip "Update test once APNS topic is configured" if apple_topic == "com.yourcompany.fizzy"
-    skip "Update test once FCM project_id is configured" if google_project_id == "your-firebase-project"
-
-    assert apple_team_id.present?
-    assert apple_topic.present?
-    assert google_project_id.present?
-  end
-end

saas/test/test_helpers/push_notification_test_helper.rb

@@ -1,33 +0,0 @@
-module PushNotificationTestHelper
-  # Assert native push notification is queued for delivery
-  def assert_native_push_delivery(count: 1, &block)
-    assert_enqueued_jobs count, only: ApplicationPushNotificationJob, &block
-  end
-
-  # Assert no native push notifications are queued
-  def assert_no_native_push_delivery(&block)
-    assert_native_push_delivery(count: 0, &block)
-  end
-
-  # Expect push notification to be delivered (using mocha)
-  def expect_native_push_delivery(count: 1)
-    ApplicationPushNotification.any_instance.expects(:deliver_later_to).times(count)
-    yield if block_given?
-  end
-
-  # Expect no push notification delivery
-  def expect_no_native_push_delivery(&block)
-    expect_native_push_delivery(count: 0, &block)
-  end
-
-  # Stub the push service to avoid actual API calls
-  def stub_push_services
-    ActionPushNative.stubs(:service_for).returns(stub(push: true))
-  end
-
-  # Stub push service to simulate token error (device should be deleted)
-  def stub_push_token_error
-    push_stub = stub.tap { |s| s.stubs(:push).raises(ActionPushNative::TokenError) }
-    ActionPushNative.stubs(:service_for).returns(push_stub)
-  end
-end

test/integration/notifications_test.rb

@@ -0,0 +1,171 @@
+require "test_helper"
+
+class NotificationDeliveryTest < ActiveSupport::TestCase
+  setup do
+    @assigner = users(:david)
+    @assignee = users(:kevin)
+    @card = cards(:logo)
+
+    @card.assignments.destroy_all
+
+    stub_web_push_pool
+
+    @original_targets = Notification.push_targets.dup
+    Notification.push_targets = []
+    Notification.register_push_target(:web)
+    Notification.register_push_target(push_target_with_tracking)
+
+    # Give assignee a web push subscription
+    @assignee.push_subscriptions.create!(
+      endpoint: "https://fcm.googleapis.com/fcm/send/test123",
+      p256dh_key: "test_key",
+      auth_key: "test_auth"
+    )
+
+    Current.user = @assigner
+  end
+
+  teardown do
+    Notification.push_targets = @original_targets
+    @assignee.push_subscriptions.delete_all
+  end
+
+  test "card assignment creates notification and triggers push" do
+    assert_difference -> { Notification.count }, 1 do
+      perform_enqueued_jobs only: [ NotifyRecipientsJob, Notification::PushJob ] do
+        @card.toggle_assignment(@assignee)
+      end
+    end
+
+    notification = Notification.last
+    assert_equal @assignee, notification.user
+    assert_equal @assigner, notification.creator
+    assert_equal "card_assigned", notification.source.action
+
+    assert_push_delivered_for notification
+    assert_web_push_delivered
+  end
+
+  test "card assignment notification is bundled for email delivery when bundling enabled" do
+    @assignee.settings.update!(bundle_email_frequency: :every_few_hours)
+
+    assert_difference -> { Notification.count }, 1 do
+      perform_enqueued_jobs only: NotifyRecipientsJob do
+        @card.toggle_assignment(@assignee)
+      end
+    end
+
+    notification = @assignee.notifications.last
+    assert_not_nil notification, "Notification should be created for assignee"
+
+    bundle = @assignee.notification_bundles.pending.last
+    assert_not_nil bundle, "Bundle should be created when bundling is enabled"
+    assert_includes bundle.notifications, notification
+  end
+
+  test "comment creates notification for card watchers and triggers push" do
+    @card.watch_by(@assignee)
+
+    assert_difference -> { Notification.count }, 1 do
+      perform_enqueued_jobs only: [ NotifyRecipientsJob, Notification::PushJob ] do
+        @card.comments.create!(body: "Great work on this!", creator: @assigner)
+      end
+    end
+
+    notification = Notification.last
+    assert_equal @assignee, notification.user
+    assert_equal "comment_created", notification.source.action
+
+    assert_push_delivered
+    assert_web_push_delivered
+  end
+
+  test "mention creates notification and triggers push" do
+    mention_html = ActionText::Attachment.from_attachable(@assignee).to_html
+
+    perform_enqueued_jobs only: [ Mention::CreateJob, NotifyRecipientsJob, Notification::PushJob ] do
+      @card.comments.create!(
+        body: "#{mention_html} check this out",
+        creator: @assigner
+      )
+    end
+
+    mention_notification = @assignee.notifications.find_by(source_type: "Mention")
+    assert_not_nil mention_notification
+
+    assert_push_delivered_for mention_notification
+    assert_web_push_delivered
+  end
+
+  test "system user actions do not create notifications" do
+    Current.user = users(:system)
+
+    assert_no_difference -> { Notification.count } do
+      perform_enqueued_jobs only: [ NotifyRecipientsJob, Notification::PushJob ] do
+        @card.toggle_assignment(@assignee)
+      end
+    end
+
+    assert_no_push_delivered
+    assert_no_web_push_delivered
+  end
+
+  test "notifications for inactive users are created but do not trigger push" do
+    @assignee.deactivate
+
+    assert_difference -> { Notification.count }, 1 do
+      perform_enqueued_jobs only: [ NotifyRecipientsJob, Notification::PushJob ] do
+        @card.toggle_assignment(@assignee)
+      end
+    end
+
+    assert_no_push_delivered
+    assert_no_web_push_delivered
+  end
+
+  private
+    def stub_web_push_pool
+      @web_push_calls = []
+      web_push_pool = stub("web_push_pool")
+      web_push_pool.stubs(:queue).with do |payload, subs|
+        @web_push_calls << { payload: payload, subscriptions: subs }
+      end
+
+      Rails.configuration.x.stubs(:web_push_pool).returns(web_push_pool)
+    end
+
+    def push_target_with_tracking
+      @push_target_calls = []
+      fake_push_target = Class.new(Notification::PushTarget) do
+        class << self
+          attr_accessor :calls
+        end
+
+        def self.process(notification)
+          calls << notification
+        end
+      end
+
+      fake_push_target.tap { it.calls = @push_target_calls }
+    end
+
+    def assert_push_delivered
+      assert_not_empty @push_target_calls, "Expected push to be delivered"
+    end
+
+    def assert_push_delivered_for(notification)
+      assert_includes @push_target_calls, notification, "Expected push to be delivered for notification"
+    end
+
+    def assert_no_push_delivered
+      assert_empty @push_target_calls, "Expected no push to be delivered"
+    end
+
+    def assert_web_push_delivered
+      assert_not_empty @web_push_calls, "Expected web push to be delivered"
+    end
+
+    def assert_no_web_push_delivered
+      assert_empty @web_push_calls, "Expected no web push to be delivered"
+    end
+end

test/models/notification/push_target/web_test.rb

@@ -77,5 +77,4 @@ class Notification::PushTarget::WebTest < ActiveSupport::TestCase
 
     Notification::PushTarget::Web.new(notification).process
   end
-
 end

test/test_helper.rb

@@ -37,10 +37,6 @@ VCR.configure do |config|
   }
 end
 
-if Fizzy.saas?
-  require_relative "../saas/test/test_helpers/push_notification_test_helper"
-end
-
 module ActiveSupport
   class TestCase
     parallelize workers: :number_of_processors, work_stealing: ENV["WORK_STEALING"] != "false"
@@ -51,7 +47,6 @@ module ActiveSupport
     include ActiveJob::TestHelper
     include ActionTextTestHelper, CachingTestHelper, CardTestHelper, ChangeTestHelper, SessionTestHelper
     include Turbo::Broadcastable::TestHelper
-    include PushNotificationTestHelper if Fizzy.saas?
 
     setup do
       Current.account = accounts("37s")