fix: Mark as unread doesn’t work with IOU report#81499
fix: Mark as unread doesn’t work with IOU report#81499Valforte merged 28 commits intoExpensify:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51ad7c9b4c
ℹ️ 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".
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product
|
@paulnjs PR has conflict. Also, could you please go through all the AI suggestions, react to each one with 👍 or 👎, and provide a brief explanation for your decision? Thanks! |
|
Sorry, this one is missing from my K2. Reviewing now |
|
@paulnjs Could you resolve conflict? |
|
@truph01 Resolved |
| key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, | ||
| value: { | ||
| lastReadTime, | ||
| ...(lastActorAccountID !== undefined && {lastActorAccountID}), |
There was a problem hiding this comment.
@paulnjs This logic was intended to address the following concern mentioned in your proposal:
This function only updates
lastReadTimeand does not updatelastActorAccountID, which may result inlastActorAccountIDbeing missed when called with an IOU report.
However, after testing, I was unable to find any scenario in which lastActorAccountID becomes undefined.
Could you share the case where it occurs?
There was a problem hiding this comment.
@truph01 This is the case that I debugged
Screen.Recording.2026-03-13.at.14.55.30.mov
|
@paulnjs I left a few comments, please help check. |
|
@paulnjs We have conflict again |
|
@truph01 Resolved |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-03-31.at.15.13.37.movAndroid: mWeb ChromeScreen.Recording.2026-03-31.at.15.11.05.moviOS: HybridAppScreen.Recording.2026-03-31.at.15.14.37.moviOS: mWeb SafariScreen.Recording.2026-03-31.at.15.11.41.movMacOS: Chrome / SafariScreen.Recording.2026-03-31.at.15.10.08.mov |
|
🚧 @Valforte has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
FYI this PR is making TS fail on main |
|
@MelvinBot Create PR to fix this #81499 (comment) |
|
🤖 Cannot proceed: |
|
PR fixing it here #86756 |
|
I'm not sure why everything passed when I approved it. What was your experience when merging this, @Valforte? |
|
🚀 Deployed to staging by https://github.com/Valforte in version: 9.3.51-0 🚀
Bundle Size Analysis (Sentry): |
|
🤖 Help Site Review — No Changes Required I reviewed all files under Findings:
Conclusion: Since this is a code-level bug fix to existing behavior (not a new feature or changed UI), no updates to the help site documentation are required. |
|
@Valforte @luacmartins https://expensify.slack.com/archives/C05LX9D6E07/p1774997693250829?thread_ts=1774991107.356179&cid=C05LX9D6E07 Since the deploy, a bunch of workspace chats appeared unread in my LHN and when I clicked into them they scroll me back to random IOU previews in the chat. Not entirely sure if it's related to this PR or not. 2026-03-31_23-53-06.mp4 |
|
@paulnjs did you look at this? |
|
@trjExpensify Sorry, I didn't have time check it ytd. Will check it today and get back to you asap |
|
|
||
| // If the user was mentioned and the comment got deleted the lastMentionedTime will be more recent than the lastVisibleActionCreated | ||
| return lastReadTime < (lastVisibleActionCreated ?? '') || lastReadTime < lastMentionedTime; | ||
| return lastReadTime < (lastVisibleActionCreated ?? '') || lastReadTime < lastMentionedTime || lastReadTime < (latestReportActionFromOtherUsers?.created ?? ''); |
There was a problem hiding this comment.
We should not look into report actions to determine if the report is unread. We should keep using only lastVisibleActionCreated as we don't want to mark the report unread if last action is non-actionable (e.g. system message)
|
We are reverting the PR #86980 |
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.3.51-10 🚀
|
|
Created #87041 to fix the original issue |
Explanation of Change
Fixed Issues
$#80486
PROPOSAL:#80486 (comment)
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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
android.mov
Android: mWeb Chrome
android_chorme.mov
iOS: Native
ios.mov
iOS: mWeb Safari
ios_sfr.mov
MacOS: Chrome / Safari
chorme.mov