Uh oh!
There was an error while loading. Please reload this page.
Chore/eid wallet UI fixes - #917
Conversation
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds push notification foreground listener support to the eid-wallet application, includes minor formatting adjustments in the control panel, removes automatic scan restart after authentication, and enhances the onboarding verification flow with alternative options and contact information. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App Layout
participant NS as NotificationService
participant PN as Push Notifications
participant Panel as Local Panel
App->>+NS: onMount - listenForForegroundNotifications()
NS->>+PN: Subscribe to notifications (onNotificationReceived)
PN-->>-NS: Setup listener
Note over NS: Listen for push source<br/>with non-empty title
PN->>+NS: Foreground notification received
NS->>NS: Filter & normalize data
NS->>+Panel: addNotification()
Panel-->>-NS: Stored
NS-->>-PN: Processed
Note over App: Component unmounts
App->>+NS: onDestroy
NS->>+PN: Cleanup listener
PN-->>-NS: Unregistered
NS-->>-App: Complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infrastructure/eid-wallet/src/lib/services/NotificationService.ts`:
- Around line 411-430: The foreground listener in
listenForForegroundNotifications currently always calls addNotification which
can create duplicates because checkAndShowNotifications/sendLocalNotification
also persists pushes; before calling addNotification inside
listenForForegroundNotifications, compute a deterministic dedupe key (preferably
from a stable field in notification.extra like an id or otherwise from
title+body+JSON.stringify(data)) and query the notifications store (or use the
existing addNotification API if it supports a lookup) to see if a notification
with that same key already exists; only call addNotification when no matching
entry is found. Reference: listenForForegroundNotifications, addNotification,
and sendLocalNotification (checkAndShowNotifications flow) to implement the
dedupe check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eef499dc-880f-45d4-925e-bcf8ab798e8e
📒 Files selected for processing (7)
infrastructure/control-panel/src/routes/api/references/+server.tsinfrastructure/control-panel/src/routes/visualizer/+page.server.tsinfrastructure/eid-wallet/src/lib/services/NotificationService.tsinfrastructure/eid-wallet/src/routes/(app)/+layout.svelteinfrastructure/eid-wallet/src/routes/(app)/notifications/+page.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/scanLogic.tsinfrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
💤 Files with no reviewable changes (2)
- infrastructure/control-panel/src/routes/visualizer/+page.server.ts
- infrastructure/eid-wallet/src/routes/(app)/scan-qr/scanLogic.ts
Uh oh!
There was an error while loading. Please reload this page.
Description of change
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
New Features
Bug Fixes
Style