feat(phase15): notifications center — card widget, Sembast ... - #66
Merged
Conversation
…e, push service - notification_model: add ratingReceived, paymentReceived, invoiceRequest, orderTaken types; add detail field, toJson/fromJson, factory constructors - notifications_provider: SembastNotificationsStore (loadAll/save/delete); notificationsProviderWithDb; bridge stubs onTradeUpdated/onNewMessage - notification_card.dart: circular type icon, bold title/subtitle, detail section (blue border), green unread dot, overflow menu - notifications_screen: use NotificationCard widget; type-based tap navigation (rate, invoice, pay, trade, dispute) - push_notification_service.dart: platform-gated FCM stubs, NotificationListenerWidget routes tap payloads to GoRouter
… UX feedback - notifications_provider: fix _open() race with Completer; upsert save() via Finder instead of always add(); await all store ops with try/catch error logging; load persisted notifications on startup via loadInitialData(); add unreadNotificationCountProviderWithDb - notifications_screen: _handleTap shows SnackBar when required orderId/disputeId is null instead of silent no-op - push_notification_service: validate orderId/disputeId against alphanumeric+hyphen/underscore pattern before constructing routes
Contributor
|
Caution Review failedPull request was closed or merged during review WalkthroughThis PR adds a DB-backed notification system: extended NotificationModel (new types, detail field, JSON, factories), a Sembast-backed store and updated NotificationsNotifier (async persistence), a reusable NotificationCard widget, push-notification routing infrastructure, and screen wiring to use the new card. Changes
Sequence Diagram(s)sequenceDiagram
participant App as User/App
participant PushSvc as PushNotificationService
participant Payload as Push Payload
participant NotifListener as NotificationListenerWidget
participant Router as GoRouter
participant Provider as NotificationsNotifier
participant Store as SembastNotificationsStore
participant UI as NotificationCard
App->>PushSvc: initialize()
PushSvc-->>PushSvc: register platform listeners (stubbed)
Note over Payload: Push arrives (tap)
PushSvc->>NotifListener: deliver payload
NotifListener->>PushSvc: routeFromPayload(payload)
PushSvc-->>Router: return route path
NotifListener->>Router: context.push(route)
Note over App,Provider: User opens notifications
App->>Provider: loadInitialData()
Provider->>Store: loadAll()
Store-->>Provider: notifications[]
Provider->>UI: render NotificationCard(s)
UI->>Provider: onTap / onMarkRead / onDelete
Provider->>Store: save() / deleteRecord()
Store-->>Provider: persisted
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/features/notifications/screens/notifications_screen.dart (1)
22-36:⚠️ Potential issue | 🟠 MajorThe screen is still wired to the in-memory provider.
Every read/write path here uses
notificationsProvider, sonotificationsProviderWithDbnever hydrates or persists the actual notifications screen. After restart, the list will still reset even though the store exists.🛠️ Suggested fix
- final notifications = ref.watch(notificationsProvider); + final notifications = ref.watch(notificationsProviderWithDb); ... - ref.read(notificationsProvider.notifier).markAllAsRead(); + ref.read(notificationsProviderWithDb.notifier).markAllAsRead(); - ref.read(notificationsProvider.notifier).deleteAll(); + ref.read(notificationsProviderWithDb.notifier).deleteAll(); ... - ref.read(notificationsProvider.notifier).markAsRead(n.id), + ref.read(notificationsProviderWithDb.notifier).markAsRead(n.id), - ref.read(notificationsProvider.notifier).delete(n.id), + ref.read(notificationsProviderWithDb.notifier).delete(n.id),Also applies to: 91-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/notifications/screens/notifications_screen.dart` around lines 22 - 36, The UI is wired to the in-memory notificationsProvider instead of the persistent notificationsProviderWithDb; update all refs in this file (every ref.watch(...) and ref.read(...).notifier usage) to use notificationsProviderWithDb so the screen hydrates and persists state across restarts (e.g., change ref.watch(notificationsProvider) and ref.read(notificationsProvider.notifier).markAllAsRead()/deleteAll() to use notificationsProviderWithDb and its notifier methods); also verify the notifier exposes markAllAsRead and deleteAll used here and apply the same replacement for the other occurrences around lines 91-96.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/features/notifications/models/notification_model.dart`:
- Around line 94-155: The factory constructors
(NotificationModel.ratingReceived, paymentReceived, invoiceRequest, orderTaken,
backupReminder) currently generate IDs from
DateTime.now().microsecondsSinceEpoch which can collide; change the id
generation to a collision-resistant value (e.g., UUID v4 or secure random
string) and use that in each factory instead of the timestamp alone — for
example, use the uuid package (Uuid().v4()) or combine timestamp with a random
suffix so NotificationModel.id is unique across rapid bursts and won’t cause
save() upserts to overwrite other notifications.
In `@lib/features/notifications/providers/notifications_provider.dart`:
- Around line 187-191: The two bridge hook stubs onTradeUpdated and onNewMessage
are empty so Rust events never become NotificationModel entries; implement each
to build a NotificationModel (include type/tag like "trade_update" or
"new_message", orderId, status for trades, message text/summary for messages,
timestamps, and any ids) and pass it into the provider's add(...) method
(NotificationsProvider.add or the existing add function) so the notification is
stored/emitted; ensure you populate required fields used elsewhere
(icon/title/body/timestamp) and rely on add/notifyListeners to update the
center/bell UI.
- Around line 28-31: The web branch currently uses databaseFactoryMemory which
discards data on refresh; add the sembast_web dependency to pubspec.yaml and
switch the web code to use databaseFactoryWeb.openDatabase(_dbName) instead of
databaseFactoryMemory.openDatabase(_dbName). Also add the necessary import for
sembast_web (so the symbol databaseFactoryWeb is available) in
lib/features/notifications/providers/notifications_provider.dart and ensure any
platform-conditional logic still compiles for non-web targets.
In `@lib/features/notifications/services/push_notification_service.dart`:
- Around line 88-92: NotificationListenerWidget currently leaves initState()
empty so _handlePayload() never runs; subscribe to
FirebaseMessaging.onMessageOpenedApp in initState (e.g., assign a
StreamSubscription to a field like _openedAppSubscription) and in the listener
call _handlePayload(message.data) (or message.data ?? {}), and then cancel the
subscription in dispose() to avoid leaks; ensure you import FirebaseMessaging
and use the exact symbols NotificationListenerWidget, initState, _handlePayload,
onMessageOpenedApp and dispose when adding the subscription and cleanup.
- Around line 51-63: The routeFromPayload function currently uses string
literals that don't match the app's NotificationModel.type.name values and omits
the orderTaken case; update routeFromPayload to handle the serialized enum names
(use the same strings produced by NotificationType.*.name) and add a branch for
"orderTaken" that returns the correct route (reference function routeFromPayload
and the NotificationType.orderTaken enum/name and NotificationModel.type.name to
align values); ensure other cases use the enum-backed names so payloads from
NotificationModel.type.name map correctly.
In `@lib/features/notifications/widgets/notification_card.dart`:
- Around line 248-251: The branch that maps NotificationType.invoiceRequest,
NotificationType.orderUpdate, and NotificationType.orderTaken uses the invoice
glyph (Icons.description) causing orderTaken to show the wrong icon; update the
mapping for NotificationType.orderTaken in the tuple returned by the
match/branch so that orderTaken uses the add-circle icon (e.g.,
Icons.add_circle) with the same color (Colors.green) to match the rest of the PR
and keep iconography consistent for NotificationType.orderTaken.
- Around line 26-117: Replace the non-accessible GestureDetector wrapper with a
Material + InkWell (or wrap with FocusableActionDetector+Semantics) so the
notification card supports keyboard focus, ripple, and proper semantics; update
the widget that builds the tap surface (the top-level GestureDetector around the
Stack in notification_card.dart) to use Material + InkWell (forward existing
onTap, preserve padding/shape/borderRadius) and ensure it exposes focus/hover
states. Also fix the icon inconsistency by changing the mapping for the
orderTaken case in _TypeIconCircle (or wherever notification.type -> icon is
defined) to match the icon used in notifications_screen.dart (use the same
symbol, e.g., Icons.add_circle_outline) so both screens show the same icon for
orderTaken.
---
Outside diff comments:
In `@lib/features/notifications/screens/notifications_screen.dart`:
- Around line 22-36: The UI is wired to the in-memory notificationsProvider
instead of the persistent notificationsProviderWithDb; update all refs in this
file (every ref.watch(...) and ref.read(...).notifier usage) to use
notificationsProviderWithDb so the screen hydrates and persists state across
restarts (e.g., change ref.watch(notificationsProvider) and
ref.read(notificationsProvider.notifier).markAllAsRead()/deleteAll() to use
notificationsProviderWithDb and its notifier methods); also verify the notifier
exposes markAllAsRead and deleteAll used here and apply the same replacement for
the other occurrences around lines 91-96.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5618831e-ae77-45c3-8f53-b8c779fdb0cf
📒 Files selected for processing (6)
lib/features/notifications/models/notification_model.dartlib/features/notifications/providers/notifications_provider.dartlib/features/notifications/screens/notifications_screen.dartlib/features/notifications/services/push_notification_service.dartlib/features/notifications/widgets/notification_card.dartspecs/004-mostro-p2p-client/tasks.md
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 5 file(s) based on 7 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 5 file(s) based on 7 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
This was referenced Mar 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit