Rename push to process on PushTarget for clearer semantics
"Push to target" reads naturally - we push the notification to the target. "Target processes" also makes sense - the target receives and handles the notification in its own way. - Add class method PushTarget.process(notification) that instantiates and calls the instance method - Rename instance method from push to process - Add private push_to helper in Pushable for readable iteration Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -3,11 +3,15 @@ class Notification::PushTarget
|
||||
|
||||
delegate :card, to: :notification
|
||||
|
||||
def self.process(notification)
|
||||
new(notification).process
|
||||
end
|
||||
|
||||
def initialize(notification)
|
||||
@notification = notification
|
||||
end
|
||||
|
||||
def push
|
||||
def process
|
||||
return unless should_push?
|
||||
|
||||
perform_push
|
||||
|
||||
@@ -16,9 +16,10 @@ module Notification::Pushable
|
||||
|
||||
private
|
||||
def resolve_push_target(target)
|
||||
if target.is_a?(Notification::PushTarget) then target
|
||||
else
|
||||
if target.is_a?(Symbol)
|
||||
"Notification::PushTarget::#{target.to_s.classify}".constantize
|
||||
else
|
||||
target
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -28,9 +29,7 @@ module Notification::Pushable
|
||||
end
|
||||
|
||||
def push
|
||||
self.class.push_targets.each do |target|
|
||||
target.new(self).push
|
||||
end
|
||||
self.class.push_targets.each { |target| push_to(target) }
|
||||
end
|
||||
|
||||
def pushable?
|
||||
@@ -42,6 +41,10 @@ module Notification::Pushable
|
||||
end
|
||||
|
||||
private
|
||||
def push_to(target)
|
||||
target.process(self)
|
||||
end
|
||||
|
||||
def payload_type
|
||||
source_type.presence_in(%w[ Event Mention ]) || "Default"
|
||||
end
|
||||
|
||||
@@ -38,7 +38,7 @@ class Notification::PushTarget::NativeTest < ActiveSupport::TestCase
|
||||
@identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone")
|
||||
|
||||
assert_native_push_delivery(count: 1) do
|
||||
Notification::PushTarget::Native.new(@notification).push
|
||||
Notification::PushTarget::Native.new(@notification).process
|
||||
end
|
||||
end
|
||||
|
||||
@@ -46,7 +46,7 @@ class Notification::PushTarget::NativeTest < ActiveSupport::TestCase
|
||||
@identity.devices.delete_all
|
||||
|
||||
assert_no_native_push_delivery do
|
||||
Notification::PushTarget::Native.new(@notification).push
|
||||
Notification::PushTarget::Native.new(@notification).process
|
||||
end
|
||||
end
|
||||
|
||||
@@ -56,7 +56,7 @@ class Notification::PushTarget::NativeTest < ActiveSupport::TestCase
|
||||
@notification.update!(creator: users(:system))
|
||||
|
||||
assert_no_native_push_delivery do
|
||||
Notification::PushTarget::Native.new(@notification).push
|
||||
Notification::PushTarget::Native.new(@notification).process
|
||||
end
|
||||
end
|
||||
|
||||
@@ -67,7 +67,7 @@ class Notification::PushTarget::NativeTest < ActiveSupport::TestCase
|
||||
@identity.devices.create!(token: "token2", platform: "google", name: "Pixel")
|
||||
|
||||
assert_native_push_delivery(count: 2) do
|
||||
Notification::PushTarget::Native.new(@notification).push
|
||||
Notification::PushTarget::Native.new(@notification).process
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
@@ -27,28 +27,28 @@ class Notification::PushTarget::WebTest < ActiveSupport::TestCase
|
||||
subscriptions.count == 1
|
||||
end
|
||||
|
||||
Notification::PushTarget::Web.new(@notification).push
|
||||
Notification::PushTarget::Web.new(@notification).process
|
||||
end
|
||||
|
||||
test "does not push when user has no subscriptions" do
|
||||
@user.push_subscriptions.delete_all
|
||||
@web_push_pool.expects(:queue).never
|
||||
|
||||
Notification::PushTarget::Web.new(@notification).push
|
||||
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).push
|
||||
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).push
|
||||
Notification::PushTarget::Web.new(@notification).process
|
||||
end
|
||||
|
||||
test "payload includes card title for card events" do
|
||||
@@ -56,7 +56,7 @@ class Notification::PushTarget::WebTest < ActiveSupport::TestCase
|
||||
payload[:title] == @notification.card.title
|
||||
end
|
||||
|
||||
Notification::PushTarget::Web.new(@notification).push
|
||||
Notification::PushTarget::Web.new(@notification).process
|
||||
end
|
||||
|
||||
test "payload for comment includes RE prefix" do
|
||||
@@ -67,7 +67,7 @@ class Notification::PushTarget::WebTest < ActiveSupport::TestCase
|
||||
payload[:title].start_with?("RE:")
|
||||
end
|
||||
|
||||
Notification::PushTarget::Web.new(notification).push
|
||||
Notification::PushTarget::Web.new(notification).process
|
||||
end
|
||||
|
||||
test "payload for assignment includes assigned message" do
|
||||
@@ -78,7 +78,7 @@ class Notification::PushTarget::WebTest < ActiveSupport::TestCase
|
||||
payload[:body].include?("Assigned to you")
|
||||
end
|
||||
|
||||
Notification::PushTarget::Web.new(notification).push
|
||||
Notification::PushTarget::Web.new(notification).process
|
||||
end
|
||||
|
||||
test "payload for mention includes mentioner name" do
|
||||
@@ -89,7 +89,7 @@ class Notification::PushTarget::WebTest < ActiveSupport::TestCase
|
||||
payload[:title].include?("mentioned you")
|
||||
end
|
||||
|
||||
Notification::PushTarget::Web.new(notification).push
|
||||
Notification::PushTarget::Web.new(notification).process
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
@@ -15,12 +15,9 @@ class Notification::PushableTest < ActiveSupport::TestCase
|
||||
end
|
||||
end
|
||||
|
||||
test "push calls push on all registered targets" do
|
||||
test "push calls process on all registered targets" do
|
||||
target_class = mock("push_target_class")
|
||||
target_instance = mock("push_target_instance")
|
||||
|
||||
target_class.expects(:new).with(@notification).returns(target_instance)
|
||||
target_instance.expects(:push)
|
||||
target_class.expects(:process).with(@notification)
|
||||
|
||||
original_targets = Notification.push_targets
|
||||
Notification.push_targets = [ target_class ]
|
||||
|
||||
Reference in New Issue
Block a user