From 06c9ece058ebfa835b72e7cd91f8218bd8a07765 Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Wed, 21 Jan 2026 19:31:19 +0100 Subject: [PATCH] Move payload method to Notification and make accessors public The notification now owns its payload via #payload method in Pushable, allowing direct access like notification.payload.title. Push classes simply use the notification's payload rather than building it themselves. Co-Authored-By: Claude Opus 4.5 --- app/models/notification/default_payload.rb | 24 +++---- app/models/notification/event_payload.rb | 70 +++++++++---------- app/models/notification/mention_payload.rb | 24 +++---- app/models/notification/push.rb | 11 --- app/models/notification/push/web.rb | 2 +- app/models/notification/pushable.rb | 9 +++ saas/app/models/notification/push/native.rb | 18 +++-- .../models/notification/push/native_test.rb | 21 ++---- 8 files changed, 87 insertions(+), 92 deletions(-) diff --git a/app/models/notification/default_payload.rb b/app/models/notification/default_payload.rb index acffb1365..fc2d5290a 100644 --- a/app/models/notification/default_payload.rb +++ b/app/models/notification/default_payload.rb @@ -11,19 +11,19 @@ class Notification::DefaultPayload { title: title, body: body, url: url } end + def title + "New notification" + end + + def body + "You have a new notification" + end + + def url + notifications_url + end + private - def title - "New notification" - end - - def body - "You have a new notification" - end - - def url - notifications_url - end - def card_url(card) Rails.application.routes.url_helpers.card_url(card, **url_options) end diff --git a/app/models/notification/event_payload.rb b/app/models/notification/event_payload.rb index 3cc02dc34..ebdb807b9 100644 --- a/app/models/notification/event_payload.rb +++ b/app/models/notification/event_payload.rb @@ -1,42 +1,42 @@ class Notification::EventPayload < Notification::DefaultPayload include ExcerptHelper + def title + case event.action + when "comment_created" + "RE: #{card_title}" + else + card_title + end + end + + def body + case event.action + when "comment_created" + format_excerpt(event.eventable.body, length: 200) + when "card_assigned" + "Assigned to you by #{event.creator.name}" + when "card_published" + "Added by #{event.creator.name}" + when "card_closed" + card.closure ? "Moved to Done by #{event.creator.name}" : "Closed by #{event.creator.name}" + when "card_reopened" + "Reopened by #{event.creator.name}" + else + event.creator.name + end + end + + def url + case event.action + when "comment_created" + card_url_with_comment_anchor(event.eventable) + else + card_url(card) + end + end + private - def title - case event.action - when "comment_created" - "RE: #{card_title}" - else - card_title - end - end - - def body - case event.action - when "comment_created" - format_excerpt(event.eventable.body, length: 200) - when "card_assigned" - "Assigned to you by #{event.creator.name}" - when "card_published" - "Added by #{event.creator.name}" - when "card_closed" - card.closure ? "Moved to Done by #{event.creator.name}" : "Closed by #{event.creator.name}" - when "card_reopened" - "Reopened by #{event.creator.name}" - else - event.creator.name - end - end - - def url - case event.action - when "comment_created" - card_url_with_comment_anchor(event.eventable) - else - card_url(card) - end - end - def event notification.source end diff --git a/app/models/notification/mention_payload.rb b/app/models/notification/mention_payload.rb index 4649f38f3..f07cbf20f 100644 --- a/app/models/notification/mention_payload.rb +++ b/app/models/notification/mention_payload.rb @@ -1,19 +1,19 @@ class Notification::MentionPayload < Notification::DefaultPayload include ExcerptHelper + def title + "#{mention.mentioner.first_name} mentioned you" + end + + def body + format_excerpt(mention.source.mentionable_content, length: 200) + end + + def url + card_url(card) + end + private - def title - "#{mention.mentioner.first_name} mentioned you" - end - - def body - format_excerpt(mention.source.mentionable_content, length: 200) - end - - def url - card_url(card) - end - def mention notification.source end diff --git a/app/models/notification/push.rb b/app/models/notification/push.rb index cf2adce44..5def39e5b 100644 --- a/app/models/notification/push.rb +++ b/app/models/notification/push.rb @@ -21,15 +21,4 @@ class Notification::Push def perform_push raise NotImplementedError end - - def build_payload - case notification.source_type - when "Event" - Notification::EventPayload.new(notification).to_h - when "Mention" - Notification::MentionPayload.new(notification).to_h - else - Notification::DefaultPayload.new(notification).to_h - end - end end diff --git a/app/models/notification/push/web.rb b/app/models/notification/push/web.rb index 33d34e3e7..664473101 100644 --- a/app/models/notification/push/web.rb +++ b/app/models/notification/push/web.rb @@ -9,7 +9,7 @@ class Notification::Push::Web < Notification::Push end def perform_push - Rails.configuration.x.web_push_pool.queue(build_payload, subscriptions) + Rails.configuration.x.web_push_pool.queue(notification.payload.to_h, subscriptions) end def subscriptions diff --git a/app/models/notification/pushable.rb b/app/models/notification/pushable.rb index a4f1a56c6..a780b11c5 100644 --- a/app/models/notification/pushable.rb +++ b/app/models/notification/pushable.rb @@ -33,4 +33,13 @@ module Notification::Pushable def pushable? !creator.system? && user.active? && account.active? end + + def payload + "Notification::#{payload_type}Payload".constantize.new(self) + end + + private + def payload_type + source_type.presence_in(%w[ Event Mention ]) || "Default" + end end diff --git a/saas/app/models/notification/push/native.rb b/saas/app/models/notification/push/native.rb index 0030cf4b5..2d0836b85 100644 --- a/saas/app/models/notification/push/native.rb +++ b/saas/app/models/notification/push/native.rb @@ -9,14 +9,18 @@ class Notification::Push::Native < Notification::Push end def perform_push - native_notification(build_payload).deliver_later_to(devices) + native_notification.deliver_later_to(devices) end def devices @devices ||= notification.identity.devices end - def native_notification(payload) + def payload + @payload ||= notification.payload + end + + def native_notification ApplicationPushNotification .with_apple( aps: { @@ -29,9 +33,9 @@ class Notification::Push::Native < Notification::Push android: { notification: nil } ) .with_data( - title: payload[:title], - body: payload[:body], - url: payload[:url], + title: payload.title, + body: payload.body, + url: payload.url, account_id: notification.account.external_account_id, avatar_url: creator_avatar_url, card_id: card&.id, @@ -40,8 +44,8 @@ class Notification::Push::Native < Notification::Push category: notification_category ) .new( - title: payload[:title], - body: payload[:body], + title: payload.title, + body: payload.body, badge: notification.user.notifications.unread.count, sound: "default", thread_id: card&.id, diff --git a/saas/test/models/notification/push/native_test.rb b/saas/test/models/notification/push/native_test.rb index 8ba0521a5..0cff2336a 100644 --- a/saas/test/models/notification/push/native_test.rb +++ b/saas/test/models/notification/push/native_test.rb @@ -104,8 +104,7 @@ class Notification::Push::NativeTest < ActiveSupport::TestCase @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") push = Notification::Push::Native.new(@notification) - payload = push.send(:build_payload) - native = push.send(:native_notification, payload) + native = push.send(:native_notification) assert_not_nil native.title assert_not_nil native.body @@ -116,8 +115,7 @@ class Notification::Push::NativeTest < ActiveSupport::TestCase @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") push = Notification::Push::Native.new(@notification) - payload = push.send(:build_payload) - native = push.send(:native_notification, payload) + native = push.send(:native_notification) assert_equal @notification.card.id, native.thread_id end @@ -127,8 +125,7 @@ class Notification::Push::NativeTest < ActiveSupport::TestCase notification.user.identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") push = Notification::Push::Native.new(notification) - payload = push.send(:build_payload) - native = push.send(:native_notification, payload) + native = push.send(:native_notification) assert native.high_priority end @@ -137,8 +134,7 @@ class Notification::Push::NativeTest < ActiveSupport::TestCase @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") push = Notification::Push::Native.new(@notification) - payload = push.send(:build_payload) - native = push.send(:native_notification, payload) + native = push.send(:native_notification) assert_not native.high_priority end @@ -147,8 +143,7 @@ class Notification::Push::NativeTest < ActiveSupport::TestCase @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") push = Notification::Push::Native.new(@notification) - payload = push.send(:build_payload) - native = push.send(:native_notification, payload) + native = push.send(:native_notification) assert_equal 1, native.apple_data.dig(:aps, :"mutable-content") assert_includes %w[active time-sensitive], native.apple_data.dig(:aps, :"interruption-level") @@ -159,8 +154,7 @@ class Notification::Push::NativeTest < ActiveSupport::TestCase @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") push = Notification::Push::Native.new(@notification) - payload = push.send(:build_payload) - native = push.send(:native_notification, payload) + native = push.send(:native_notification) assert_nil native.google_data.dig(:android, :notification) end @@ -169,8 +163,7 @@ class Notification::Push::NativeTest < ActiveSupport::TestCase @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") push = Notification::Push::Native.new(@notification) - payload = push.send(:build_payload) - native = push.send(:native_notification, payload) + native = push.send(:native_notification) assert_not_nil native.data[:url] assert_equal @notification.account.external_account_id, native.data[:account_id]