From 92cb749e16f805614a1df70081d80fa570cf3cb4 Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Fri, 20 Feb 2026 18:38:43 +0100 Subject: [PATCH] Fix tests for renamed fixtures and new stacked notifications Clear assignee's existing notifications in setup since Notifier now uses create_or_find_by instead of create, and reload the association to avoid caching after destroy_all. Update fixture references after rebase: use `logo_assignment_kevin` as base notification and `logo_mentioned_david` for mention tests. Update source to `logo_published` in tests that need generic card events. Co-Authored-By: Claude Opus 4.5 --- Gemfile.saas.lock | 4 +-- app/models/notification/pushable.rb | 3 +- saas/db/saas_schema.rb | 20 ++++++------- .../notification/push_target/native_test.rb | 12 +++++--- test/integration/notifications_test.rb | 3 +- test/models/concerns/push_notifiable_test.rb | 28 ------------------- .../notification/push_target/web_test.rb | 17 ++++------- test/models/notification/pushable_test.rb | 27 ++++++++++++------ 8 files changed, 47 insertions(+), 67 deletions(-) delete mode 100644 test/models/concerns/push_notifiable_test.rb diff --git a/Gemfile.saas.lock b/Gemfile.saas.lock index 5dfc9adc6..fc88e7aec 100644 --- a/Gemfile.saas.lock +++ b/Gemfile.saas.lock @@ -308,7 +308,6 @@ GEM addressable (>= 2.5.0) globalid (1.3.0) activesupport (>= 6.1) - gvltools (0.4.0) google-cloud-env (2.3.1) base64 (~> 0.2) faraday (>= 1.0, < 3.a) @@ -321,6 +320,7 @@ GEM multi_json (~> 1.11) os (>= 0.9, < 2.0) signet (>= 0.16, < 2.a) + gvltools (0.4.0) hashdiff (1.2.1) http-2 (1.1.1) httpx (1.7.1) @@ -700,8 +700,8 @@ PLATFORMS x86_64-linux-musl DEPENDENCIES - actionpack-xml_parser action_push_native + actionpack-xml_parser activeresource audits1984! autotuner diff --git a/app/models/notification/pushable.rb b/app/models/notification/pushable.rb index 4d5d14d0d..e02f3a8de 100644 --- a/app/models/notification/pushable.rb +++ b/app/models/notification/pushable.rb @@ -4,8 +4,7 @@ module Notification::Pushable included do class_attribute :push_targets, default: [] - after_create_commit :push_later - after_update_commit :push_later, if: :source_id_previously_changed? + after_save_commit :push_later, if: :source_id_previously_changed? end class_methods do diff --git a/saas/db/saas_schema.rb b/saas/db/saas_schema.rb index debc08b19..298cc033a 100644 --- a/saas/db/saas_schema.rb +++ b/saas/db/saas_schema.rb @@ -43,16 +43,6 @@ ActiveRecord::Schema[8.2].define(version: 2026_01_26_230838) do t.index ["stripe_subscription_id"], name: "index_account_subscriptions_on_stripe_subscription_id", unique: true end - create_table "audits1984_auditor_tokens", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| - t.uuid "auditor_id", null: false - t.datetime "created_at", null: false - t.datetime "expires_at", null: false - t.string "token_digest", null: false - t.datetime "updated_at", null: false - t.index ["auditor_id"], name: "index_audits1984_auditor_tokens_on_auditor_id", unique: true - t.index ["token_digest"], name: "index_audits1984_auditor_tokens_on_token_digest", unique: true - end - create_table "action_push_native_devices", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "created_at", null: false t.string "name" @@ -66,6 +56,16 @@ ActiveRecord::Schema[8.2].define(version: 2026_01_26_230838) do t.index ["session_id"], name: "index_action_push_native_devices_on_session_id" end + create_table "audits1984_auditor_tokens", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + t.uuid "auditor_id", null: false + t.datetime "created_at", null: false + t.datetime "expires_at", null: false + t.string "token_digest", null: false + t.datetime "updated_at", null: false + t.index ["auditor_id"], name: "index_audits1984_auditor_tokens_on_auditor_id", unique: true + t.index ["token_digest"], name: "index_audits1984_auditor_tokens_on_token_digest", unique: true + end + create_table "audits1984_audits", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.uuid "auditor_id", null: false t.datetime "created_at", null: false diff --git a/saas/test/models/notification/push_target/native_test.rb b/saas/test/models/notification/push_target/native_test.rb index 59182fb6c..0418d96c2 100644 --- a/saas/test/models/notification/push_target/native_test.rb +++ b/saas/test/models/notification/push_target/native_test.rb @@ -4,7 +4,7 @@ class Notification::PushTarget::NativeTest < ActiveSupport::TestCase setup do @user = users(:kevin) @identity = @user.identity - @notification = notifications(:logo_published_kevin) + @notification = notifications(:logo_assignment_kevin) # Ensure user has no web push subscriptions (we want to test native push independently) @user.push_subscriptions.delete_all @@ -23,12 +23,14 @@ class Notification::PushTarget::NativeTest < ActiveSupport::TestCase end test "payload category returns mention for mentions" do - notification = notifications(:logo_card_david_mention_by_jz) + notification = notifications(:logo_mentioned_david) assert_equal "mention", notification.payload.category end test "payload category returns card for other card events" do + @notification.update!(source: events(:logo_published)) + assert_equal "card", @notification.payload.category end @@ -92,7 +94,7 @@ class Notification::PushTarget::NativeTest < ActiveSupport::TestCase end test "native notification sets high_priority for mentions" do - notification = notifications(:logo_card_david_mention_by_jz) + notification = notifications(:logo_mentioned_david) notification.user.identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") push = Notification::PushTarget::Native.new(notification) @@ -112,6 +114,7 @@ class Notification::PushTarget::NativeTest < ActiveSupport::TestCase end test "native notification sets normal priority for other card events" do + @notification.update!(source: events(:logo_published)) @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") push = Notification::PushTarget::Native.new(@notification) @@ -141,7 +144,7 @@ class Notification::PushTarget::NativeTest < ActiveSupport::TestCase end test "native notification sets time-sensitive interruption level for mentions" do - notification = notifications(:logo_card_david_mention_by_jz) + notification = notifications(:logo_mentioned_david) notification.user.identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") push = Notification::PushTarget::Native.new(notification) @@ -161,6 +164,7 @@ class Notification::PushTarget::NativeTest < ActiveSupport::TestCase end test "native notification sets active interruption level for other card events" do + @notification.update!(source: events(:logo_published)) @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") push = Notification::PushTarget::Native.new(@notification) diff --git a/test/integration/notifications_test.rb b/test/integration/notifications_test.rb index b2c850144..f581fb5e9 100644 --- a/test/integration/notifications_test.rb +++ b/test/integration/notifications_test.rb @@ -7,6 +7,7 @@ class NotificationDeliveryTest < ActiveSupport::TestCase @card = cards(:logo) @card.assignments.destroy_all + @assignee.notifications.destroy_all stub_web_push_pool @@ -55,7 +56,7 @@ class NotificationDeliveryTest < ActiveSupport::TestCase end end - notification = @assignee.notifications.last + notification = @assignee.notifications.reload.last assert_not_nil notification, "Notification should be created for assignee" bundle = @assignee.notification_bundles.pending.last diff --git a/test/models/concerns/push_notifiable_test.rb b/test/models/concerns/push_notifiable_test.rb deleted file mode 100644 index f6d7ed26d..000000000 --- a/test/models/concerns/push_notifiable_test.rb +++ /dev/null @@ -1,28 +0,0 @@ -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 diff --git a/test/models/notification/push_target/web_test.rb b/test/models/notification/push_target/web_test.rb index da71d0db1..94628fb82 100644 --- a/test/models/notification/push_target/web_test.rb +++ b/test/models/notification/push_target/web_test.rb @@ -3,10 +3,7 @@ require "test_helper" class Notification::PushTarget::WebTest < ActiveSupport::TestCase setup do @user = users(:david) - @notification = @user.notifications.create!( - source: events(:logo_published), - creator: users(:jason) - ) + @notification = notifications(:logo_mentioned_david) @user.push_subscriptions.create!( endpoint: "https://fcm.googleapis.com/fcm/send/test123", @@ -38,6 +35,8 @@ class Notification::PushTarget::WebTest < ActiveSupport::TestCase end test "payload includes card title for card events" do + @notification.update!(source: events(:logo_published)) + @web_push_pool.expects(:queue).once.with do |payload, _| payload[:title] == @notification.card.title end @@ -57,24 +56,20 @@ class Notification::PushTarget::WebTest < ActiveSupport::TestCase end test "payload for assignment includes assigned message" do - event = events(:logo_assignment_david) - notification = @user.notifications.create!(source: event, creator: event.creator) + @notification.update!(source: events(:logo_assignment_david)) @web_push_pool.expects(:queue).once.with do |payload, _| payload[:body].include?("Assigned to you") end - Notification::PushTarget::Web.new(notification).process + Notification::PushTarget::Web.new(@notification).process end test "payload for mention includes mentioner name" do - mention = mentions(:logo_card_david_mention_by_jz) - notification = @user.notifications.create!(source: mention, creator: users(:jz)) - @web_push_pool.expects(:queue).once.with do |payload, _| payload[:title].include?("mentioned you") end - Notification::PushTarget::Web.new(notification).process + Notification::PushTarget::Web.new(@notification).process end end diff --git a/test/models/notification/pushable_test.rb b/test/models/notification/pushable_test.rb index 7c54b0036..9036453bd 100644 --- a/test/models/notification/pushable_test.rb +++ b/test/models/notification/pushable_test.rb @@ -3,10 +3,7 @@ require "test_helper" class Notification::PushableTest < ActiveSupport::TestCase setup do @user = users(:david) - @notification = @user.notifications.create!( - source: events(:logo_published), - creator: users(:jason) - ) + @notification = notifications(:logo_mentioned_david) end test "push_later enqueues Notification::PushJob" do @@ -28,12 +25,24 @@ class Notification::PushableTest < ActiveSupport::TestCase end test "push_later is called after notification is created" do - Notification.any_instance.expects(:push_later) + assert_enqueued_with(job: Notification::PushJob) do + @user.notifications.create!( + source: events(:layout_published), + creator: users(:jason) + ) + end + end - @user.notifications.create!( - source: events(:logo_published), - creator: users(:jason) - ) + test "push_later is called when notification source changes" do + assert_enqueued_with(job: Notification::PushJob) do + @notification.update!(source: events(:logo_published)) + end + end + + test "push_later is not called for other updates" do + assert_no_enqueued_jobs only: Notification::PushJob do + @notification.update!(unread_count: 5) + end end test "register_push_target accepts symbols" do