Revert "Fix pending notifications not being received after going online"#80508
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8915f4570d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return onyxDataUpdatePromise | ||
| .then(() => { | ||
| // Trigger notifications only on successful responses. | ||
| if (Platform.OS === 'web' && !isMobile() && response.jsonCode === 200 && response.onyxData?.length) { | ||
| triggerNotifications(response.onyxData); | ||
| } | ||
| // Handle the request's success/failure data (client-side data) | ||
| if (response.jsonCode === 200 && request.successData) { | ||
| return updateHandler(request.successData); |
There was a problem hiding this comment.
Restore notifications for HTTPS Onyx updates
On desktop web, notifications were previously triggered when applyHTTPSOnyxUpdates applied response.onyxData (the path used for API catch-up after reconnect/offline). Removing that call means report actions delivered via HTTPS no longer call showReportActionNotification, and notifications now only happen for Pusher updates in User.ts. The regression is visible when Pusher is disconnected or the user comes back online and the server returns missed report actions in onyxData: the UI updates, but no desktop notification fires.
Useful? React with 👍 / 👎.
|
Straight revert, looks clean, except for the Typescript checker. Do we just merge anyway, to keep it from getting messy? |
|
@arosiclair looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
Straight revert and typescript checks are failing on main |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/arosiclair in version: 9.3.9-0 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.9-2 🚀
|
Reverts #76932
$ https://expensify.slack.com/archives/C05LX9D6E07/p1769373897780789