feat(notifications): open the chat when a chat notification is tapped - #681
Conversation
Tapping a chat notification landed on the trade detail screen instead of
the conversation the notification was about.
Push notifications now carry a structured payload for chat actions:
- Action.chatMessage -> {"type":"peer_chat"} -> /chat_room/:orderId
- Action.sendDm -> {"type":"admin_dm"} -> /dispute_details/:disputeId
Non-chat actions keep the legacy plain orderId payload, so previously
scheduled notifications still resolve to the trade detail screen.
The in-app notification list follows the same dispatch: peer chat
messages open the P2P chat room and solver DMs open the dispute chat,
falling back to the trade detail when the session has no dispute id yet.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe notification service now creates structured payloads for peer chats and admin DMs. Notification taps route chats to chat rooms, DMs to dispute details when available, and otherwise to trade details. Standard notifications retain plain order IDs. ChangesNotification routing
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change routes chat notifications to the appropriate chat screens while preserving legacy trade-detail routing; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant NotificationItem
participant SessionProvider
participant Router
NotificationItem->>SessionProvider: Read dispute ID for solver DM
SessionProvider-->>NotificationItem: Return dispute ID or no ID
NotificationItem->>Router: Open chat room, dispute details, or trade details
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (4 skipped: 4 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Problem
Tapping a chat notification did not open the chat. Both chat kinds landed on the trade detail screen:
Action.chatMessage) built a plain-orderId payload, whichresolveNotificationRoutemaps to/trade_detail/:orderId.Action.sendDm) already had a JSON payload, but the in-app notification list ignored it and pushed/trade_detail/:orderIdtoo.Change
Chat notifications now carry a structured payload and route to the conversation itself:
chatMessage{"type":"peer_chat","orderId":…}/chat_room/:orderIdsendDm{"type":"admin_dm","orderId":…,"disputeId":…}/dispute_details/:disputeId(→DisputeChatScreen)orderId(unchanged)/trade_detail/:orderIdPayload construction was extracted into
buildNotificationPayload()so the payload and the route resolution are covered by the same tests.The in-app notification list (
NotificationItem) follows the same dispatch:chatMessageopens the P2P chat room,sendDmresolves the dispute id from the session and opens the solver chat, falling back to the trade detail when the session has no dispute id yet.Backward compatible: the legacy plain-orderId payload still resolves to the trade detail, so notifications posted before this change keep working. Both screens already render graceful error states when the session or peer is missing.
Test plan
flutter analyze— no new issues (the 2 remaining are pre-existingcontainsSemanticsdeprecations inautomation_contract_test.dart)flutter test— 1109 passingbuildNotificationPayload+resolveNotificationRoute(peer_chat,admin_dmwith/without dispute, legacy payload)NotificationItemnavigates to/chat_room,/dispute_details, and the no-dispute fallback (verified RED before the fix)Summary by CodeRabbit
New Features
Bug Fixes