Conversation
This reverts commit 6e9de6a.
There was a problem hiding this comment.
Pull request overview
Updates the Web Push Notification::EventPayload body strings to provide more informative, action-specific messages (instead of the prior minimal fallback), aligning push notifications better with how mobile/web clients display notification bodies.
Changes:
- Expand
Notification::EventPayload#bodyto handle additional card actions (triage, sent back to triage, board/collection moves, title changes, postponed/auto-postponed). - Add private helpers (
column_name,new_location_name,new_title) to extract message details fromevent.particulars. - Add/extend Web push payload tests to cover the new messages and fallback behavior when particulars are missing/blank.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
app/models/notification/event_payload.rb |
Adds action-specific push notification body text and helper methods to pull details from event.particulars. |
test/models/notification/push_target/web_test.rb |
Adds coverage ensuring Web push payload body matches new action-specific messages and fallbacks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| end | ||
|
|
||
| def column_name | ||
| event.particulars.dig("particulars", "column") |
There was a problem hiding this comment.
I was going to ask whether this was correct because it seemed strange to have a particulars key inside the particulars hash, but I see this is unfortunately the case in some of the event tracking calls. In some cases, the calls to track_event include an extra particulars key. This is the case in:
def track_board_change_event(old_board_name)
track_event "board_changed", particulars: { old_board: old_board_name, new_board: board.name }
end
def track_title_change
if title_before_last_save.present?
track_event "title_changed", particulars: { old_title: title_before_last_save, new_title: title }
end
end
track_event "triaged", particulars: { column: column.name }And I see these are precisely the ones you're using here, which is unfortunate 😅 Ideally we'd fix the extra particulars key there, but that's out of scope for this change.
Perhaps, to remove a bit of the repetition here, I'd add an extra method like this:
def event_particulars
event.particulars.dig("particulars") || {}
endAnd then use it like
def column_name
event_particulars["column"]
end
rosa
left a comment
There was a problem hiding this comment.
Nice! I've double-checked that this shouldn't affect web push notifications in any way (just the body change, which is ok). Added a small suggestion.
This reopens #2626.