Fix report appearing unread after returning online when messages sent offline#83756
Fix report appearing unread after returning online when messages sent offline#83756
Conversation
… appear unread When messages are sent offline and then the report is read (also offline), ReadNewestAction captures lastReadTime at that moment. On reconnect, the server assigns later timestamps to the offline-sent messages, making lastVisibleActionCreated > lastReadTime, which incorrectly shows the report as unread. This fix refreshes lastReadTime at send time in SequentialQueue, but only when the same report had offline outgoing messages earlier in the flush cycle, avoiding auto-reading messages from other users. Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
|
Fixed the failing The test failures (jobs 1, 5, 6, 7, 8) are unrelated to this PR. They all fail with |
Use lazy require for NetworkConnection in SequentialQueue to break the circular dependency chain: SequentialQueue → NetworkConnection → DateUtils → Localize → memoize → … → SequentialQueue that caused TypeError: (0, _memoize.default) is not a function in tests. Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
|
Fixed the failing Jest tests. The root cause was a circular dependency introduced by the top-level This caused Fix: Replaced the top-level import with a lazy |
|
The original test failure (circular dependency causing The remaining failures in the latest run are unrelated flaky tests:
Neither test is related to the |
…instability
The dynamic require('@libs/NetworkConnection') inside process() was
triggering heavy module-level side effects (NetInfo subscriptions, Onyx
connections) when loaded mid-test, causing random test failures across
different suites each run.
Instead, read networkTimeSkew from the existing ONYXKEYS.NETWORK Onyx
subscription and inline the simple DB-time formatting, avoiding the
circular dependency without dynamic require.
Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
|
Fixed the failing checks. The Root cause: The Fix: Replaced the dynamic
This avoids the circular dependency without loading the heavy NetworkConnection module at runtime. |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
Explanation of Change
When messages are sent while offline and then the user reads the report (also offline),
readNewestActioncaptureslastReadTimeat that moment. On reconnect, theSequentialQueuereplays requests FIFO: theAddCommentrequests are processed first, and the server assigns them new server-side timestamps that are later than the stalelastReadTime. WhenReadNewestActionis then processed with the stale timestamp,lastReadTime < lastVisibleActionCreated, causing the report to incorrectly appear as unread in the LHN.This fix adds bounded reconciliation in
SequentialQueue: during queue flush, it tracks which report IDs had offline outgoing message commands (AddComment, AddAttachment, AddTextAndAttachment) successfully processed. When an offlineReadNewestActionis about to be sent for one of those reports, itslastReadTimeis refreshed to the current time so it covers the server-assigned message timestamps. Reports without offline messages are unaffected, preventing auto-reading messages from other users.Fixed Issues
$ #79837
PROPOSAL: #79837 (comment)
Tests
Offline tests
QA Steps
Regression check:
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
N/A - logic-only change in SequentialQueue, no UI changes
Android: mWeb Chrome
N/A - logic-only change in SequentialQueue, no UI changes
iOS: Native
N/A - logic-only change in SequentialQueue, no UI changes
iOS: mWeb Safari
N/A - logic-only change in SequentialQueue, no UI changes
MacOS: Chrome / Safari
N/A - logic-only change in SequentialQueue, no UI changes