update last read time for empty report task#43665
update last read time for empty report task#43665AndrewGable merged 5 commits intoExpensify:mainfrom
Conversation
|
I'll check the unit test run fail today |
|
@getusha, I’ve discovered another issue when changing to date comparison. The use case is: User A starts a chat with a new User B. In this case, the lastReadTime for User B is empty and it does not appear bold by Date compare. Therefore, I have switched to my alternative solution, which also covers this issue without causing regression. |
| }, [isInaccessibleWhisper]); | ||
|
|
||
| useEffect(() => { | ||
| if (!!report.lastReadTime || !ReportUtils.isTaskReport(report)) { |
There was a problem hiding this comment.
why did we add !ReportUtils.isTaskReport(report)? and are you sure we're fixing the root cause?
There was a problem hiding this comment.
!ReportUtils.isTaskReport(report) this condition to make sure only update readNewestAction for task report to avoid regression
There was a problem hiding this comment.
Actually, we can remove the condition !ReportUtils.isTaskReport(report). I added this condition to ensure the fix only addresses the scope of this issue and avoids regression.
There was a problem hiding this comment.
@suneox it might fix similar issues for different kinds of reports wdyt?
There was a problem hiding this comment.
Currently, I don't found another use case the same with this issue
There was a problem hiding this comment.
@getusha i think we can go with this change only fix for an use-case task report, to avoid regression
There was a problem hiding this comment.
Lets add a comment about the fix
There was a problem hiding this comment.
I have added comment explain reason below
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2024-06-23.at.1.10.14.in.the.afternoon.movAndroid: mWeb ChromeScreen.Recording.2024-06-23.at.12.30.31.in.the.afternoon.moviOS: NativeScreen.Recording.2024-06-23.at.12.55.15.in.the.afternoon.moviOS: mWeb SafariScreen.Recording.2024-06-23.at.12.32.42.in.the.afternoon.movMacOS: Chrome / SafariScreen.Recording.2024-06-23.at.12.24.20.in.the.afternoon.movMacOS: DesktopScreen.Recording.2024-06-23.at.12.53.49.in.the.afternoon.mov |
|
✋ 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 production by https://github.com/jasperhuangg in version: 9.0.3-7 🚀
|
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.0.5-13 🚀
|
Details
Fixed Issues
$ #42068
PROPOSAL: #42068 (comment)
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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
Screen.Recording.2024-06-13.at.21.29.57.mov
Android: mWeb Chrome
Screen.Recording.2024-06-13.at.21.27.46.mov
iOS: Native
Screen.Recording.2024-06-13.at.21.25.57.mov
iOS: mWeb Safari
Screen.Recording.2024-06-13.at.21.14.06.mov
MacOS: Chrome / Safari
Screen.Recording.2024-06-13.at.20.50.36.mov
MacOS: Desktop
Screen.Recording.2024-06-13.at.20.55.00.mov