Repository navigation
Recover Web/PWA push registrations after backend upgrades - #99
joan-code6 wants to merge 4 commits into
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbcec147ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await repairBrowserPushRegistration(token, controller.signal, isCurrent, repairState); | ||
| nextAttemptAt = Date.now() + 15 * 60_000; |
There was a problem hiding this comment.
Treat rejected registrations as failed attempts
When /notifications/subscription returns a normal response with { success: false }, repairBrowserPushRegistration resolves false, but this unconditional success path still postpones the next attempt for 15 minutes. The API contract explicitly permits that response and the settings flow treats it as a registration failure, so a transient server-side rejection here does not receive the documented one-minute retry and leaves push recovery delayed; only successful repairs should use the longer cadence.
Useful? React with 👍 / 👎.
|
Cloudflare preview: https://pr-99-lanis-ui-preview.bennetjoanwegener.workers.dev Commit: This preview uses the live LANIS API. Changes made after signing in affect your real account. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79629cfcec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| nextAttemptAt = Date.now() + 60_000; | ||
| try { | ||
| const repaired = await repairBrowserPushRegistration(token, controller.signal, isCurrent, repairState); | ||
| if (repaired) nextAttemptAt = Date.now() + 15 * 60_000; |
There was a problem hiding this comment.
Throttle successful no-op repair checks
When notifications are disabled from another device while this tab retains its browser subscription, or when server push configuration is unavailable, repairBrowserPushRegistration completes both API requests normally but returns false. Because only true advances this deadline to 15 minutes, every visible affected tab calls getConfig and getPreferences once per minute indefinitely. Distinguish transient registration failures from successful ineligible/no-op checks and apply the longer cadence to the latter.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0330c25ebe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export default function BrowserPushRuntime() { | ||
| const { token, isAuthenticated, user } = useAuth(); | ||
| const { pathname } = useLocation(); | ||
| const inactive = pathname.startsWith('/demo') || pathname === '/settings/notifications'; |
There was a problem hiding this comment.
Exclude all notification-settings URLs
When the settings page is opened with a trailing slash or suffix, such as /settings/notifications/, Settings still selects the notifications section because it uses the first path segment, but this exact comparison leaves the background repair enabled. The repair can then register or renew the shared browser subscription concurrently with the settings page's enable/disable flow, producing stale UI state or re-registering an endpoint after the user starts disabling it. Treat the notification-settings route and its trailing segments as inactive.
Useful? React with 👍 / 👎.
After the backend session-ownership migration removes legacy unbound endpoints, existing browsers stay subscribed locally but previously re-registered only from notification settings. Automatic notifications could silently stop while test pushes on a freshly registered device still worked.
Repair existing opted-in browser registrations on authenticated page visits and visible-page checks. Explicit results distinguish registered, inactive, retryable and cancelled work: registered/inactive checks wait 15 minutes; rejected registrations, unsuccessful API responses and network errors retry after one minute. Renew existing subscriptions when the VAPID key changes, and cancel obsolete work on account/session changes. Permission prompts, new-device opt-in, notification settings, demo routes and native Android keep their existing flows.
Validation: 15 browser/runtime regression tests, TypeScript checking and account-data lifecycle checks pass. Tests cover both retry cadences, lost records, renewal/retry, consent boundaries, cancellation and reconnect behavior. No frontend build or production deployment was run. Live push-provider delivery remains unverified.
Companion backend fix: joan-code6/lanis_api#70