From 126ccb5e3a400168ad0e7b5d32b9ad9d0aca8600 Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Fri, 23 Jan 2026 21:44:43 +0100 Subject: [PATCH] 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 --- .../notification/push_target/native_test.rb | 16 ++ saas/test/models/push_config_test.rb | 21 --- .../push_notification_test_helper.rb | 33 ---- test/integration/notifications_test.rb | 171 ++++++++++++++++++ .../notification/push_target/web_test.rb | 1 - test/test_helper.rb | 5 - 6 files changed, 187 insertions(+), 60 deletions(-) delete mode 100644 saas/test/models/push_config_test.rb delete mode 100644 saas/test/test_helpers/push_notification_test_helper.rb create mode 100644 test/integration/notifications_test.rb diff --git a/saas/test/models/notification/push_target/native_test.rb b/saas/test/models/notification/push_target/native_test.rb index 5e038fa87..59182fb6c 100644 --- a/saas/test/models/notification/push_target/native_test.rb +++ b/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 diff --git a/saas/test/models/push_config_test.rb b/saas/test/models/push_config_test.rb deleted file mode 100644 index 554315818..000000000 --- a/saas/test/models/push_config_test.rb +++ /dev/null @@ -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 diff --git a/saas/test/test_helpers/push_notification_test_helper.rb b/saas/test/test_helpers/push_notification_test_helper.rb deleted file mode 100644 index 24355edf7..000000000 --- a/saas/test/test_helpers/push_notification_test_helper.rb +++ /dev/null @@ -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 diff --git a/test/integration/notifications_test.rb b/test/integration/notifications_test.rb new file mode 100644 index 000000000..b2c850144 --- /dev/null +++ b/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 diff --git a/test/models/notification/push_target/web_test.rb b/test/models/notification/push_target/web_test.rb index b492b48dd..da71d0db1 100644 --- a/test/models/notification/push_target/web_test.rb +++ b/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 diff --git a/test/test_helper.rb b/test/test_helper.rb index b8e96bf97..b1f813761 100644 --- a/test/test_helper.rb +++ b/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")