Skip to content

Commit 045aaf4

Browse files
dadachiclaude
andauthored
Fix push notification delivery crashing on send (#71)
ItemTagNotifier's action_push_native config omitted the with_apple, with_google, and with_data options. Noticed's delivery method passes each to ActionPushNative, which does {}.merge(option) and raises TypeError (no implicit conversion of nil into Hash) when the option is nil. Provide the options (empty hashes for apple/google, the url payload for data), matching Noticed's canonical config. That crash masked a second bug: url referenced api_v1_shopkeeper_shop_item_tag_path, which does not exist because item_tags is declared shallow. Use the shallow helper api_v1_shopkeeper_item_tag_path. Existing tests only enqueued the notification and never performed the delivery job, so neither bug surfaced. Add a test that performs the delivery (excluding the real APNs/FCM send job) plus a test pinning the url to the shallow route. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent dcbc533 commit 045aaf4

2 files changed

Lines changed: 27 additions & 6 deletions

File tree

app/notifiers/item_tag_notifier.rb

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@ class ItemTagNotifier < ApplicationNotifier
44
config.format = -> {
55
{
66
title: title,
7-
body: body,
8-
data: {url: url}
7+
body: body
98
}
109
}
10+
config.with_data = -> { {url: url} }
11+
config.with_apple = -> { {} }
12+
config.with_google = -> { {} }
1113
end
1214

1315
notification_methods do
@@ -20,10 +22,7 @@ def body
2022
end
2123

2224
def url
23-
Rails.application.routes.url_helpers.api_v1_shopkeeper_shop_item_tag_path(
24-
shop_id: record.shop_id,
25-
id: record.id
26-
)
25+
Rails.application.routes.url_helpers.api_v1_shopkeeper_item_tag_path(record.id)
2726
end
2827
end
2928
end

test/notifiers/item_tag_notifier_test.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
require "test_helper"
22

33
class ItemTagNotifierTest < ActiveSupport::TestCase
4+
include ActiveJob::TestHelper
5+
46
setup do
57
@shopkeeper = shopkeepers(:one)
68
@shopkeeper.create_default_account
@@ -30,4 +32,24 @@ class ItemTagNotifierTest < ActiveSupport::TestCase
3032
assert_equal I18n.t("notifiers.item_tag.title", shop: @shop.name), notification.title
3133
assert_equal I18n.t("notifiers.item_tag.body", name: @item_tag.name), notification.body
3234
end
35+
36+
test "url resolves to the shallow item_tag path" do
37+
ItemTagNotifier.with(record: @item_tag).deliver(@shopkeeper)
38+
notification = @shopkeeper.notifications.last
39+
40+
expected = Rails.application.routes.url_helpers.api_v1_shopkeeper_item_tag_path(@item_tag.id)
41+
assert_equal expected, notification.url
42+
end
43+
44+
test "action_push_native delivery performs without raising" do
45+
ApplicationPushDevice.create!(owner: @shopkeeper, platform: "apple", token: "test-token")
46+
47+
# Performs the Noticed delivery job (where the payload is built) but not the
48+
# ApplicationPushNotificationJob, so no real APNs/FCM request is made.
49+
assert_nothing_raised do
50+
perform_enqueued_jobs(except: ApplicationPushNotificationJob) do
51+
ItemTagNotifier.with(record: @item_tag).deliver(@shopkeeper)
52+
end
53+
end
54+
end
3355
end

0 commit comments

Comments
 (0)