[$250] mWeb - Self DM - Self DM with track expense on mark as unread is not shown in bold in LHN
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: V9. 0.73-6 Reproducible in staging?: Yes Reproducible in production?: Yes If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Yes, reproducible on both If this was caught during regression testing, add the test name, ID and link from TestRail: N Email or phone of affected tester (no customers): N Issue reported by: Applause Internal Team
Action Performed:
- Go to https://staging.new.expensify.com/home
- Open self dm with no conversation
- Create a track expense
- Navigate to LHN
- Long press self dm and select mark as unread
- Note self dm not shown in bold
- Tap fab --new workspace
- Open workspace chat
- Create a track expense
- Navigate to LHN and mark as unread the chat
- Note workspace chat shown in bold.
Expected Result:
Self DM with track expense on mark as unread must be shown in bold in LHN.
Actual Result:
Self DM with track expense on mark as unread is not shown in bold in LHN.
Workaround:
Unknown
Platforms:
- [x] Android: Standalone
- [x] Android: HybridApp
- [x] Android: mWeb Chrome
- [ ] iOS: Standalone
- [ ] iOS: HybridApp
- [ ] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
https://github.com/user-attachments/assets/5819115b-ba49-4dc9-a36c-1a2ec9d09bd9
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~021866482645091110325
- Upwork Job ID: 1866482645091110325
- Last Price Increase: 2024-12-10
Issue Owner
Current Issue Owner: @paultsimura
Triggered auto assignment to @bfitzexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
Job added to Upwork: https://www.upwork.com/jobs/~021866482645091110325
Triggered auto assignment to Contributor-plus team member for initial proposal review - @paultsimura (External)
Proposal
Please re-state the problem that we are trying to solve in this issue.
Self DM - Self DM with track expense on mark as unread is not shown in bold in LHN
What is the root cause of that problem?
When executing the markCommentAsUnread function, the created time of the action track expense is used as the new lastReadTime instead of the report action which is last visible.
https://github.com/Expensify/App/blob/3982ed03ff644e7a723e8d4131535a2cceb9488d/src/libs/actions/Report.ts#L1359-L1368
https://github.com/Expensify/App/blob/3982ed03ff644e7a723e8d4131535a2cceb9488d/src/libs/actions/Report.ts#L1372-L1379
However, the isUnread function uses the report's lastVisibleActionCreated value which is unchanged, and compares itto lastReadTime which is greater than the lastVisibleActionCreated.
https://github.com/Expensify/App/blob/3982ed03ff644e7a723e8d4131535a2cceb9488d/src/libs/ReportUtils.ts#L6275-L6280
What changes do you think we should make in order to solve the problem?
Filter out actionable track expenses just like whisper actions.
https://github.com/Expensify/App/blob/3982ed03ff644e7a723e8d4131535a2cceb9488d/src/libs/actions/Report.ts#L1364
(!(ReportActionsUtils.isWhisperAction(current) || ReportActionsUtils.isActionableTrackExpense(current)) || current.actionName === CONST.REPORT.ACTIONS.TYPE.ACTIONABLE_MENTION_WHISPER)
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
We should test the markCommentAsUnread function by creating a variety of test reports that have actionable whisper actions.
What alternative solutions did you explore? (Optional)
Set the report's lastVisibleActionCreated optimistically to the newly created report action.
The proposal by @Tony-MK does the job. However, I wonder if it should be fixed on the BE.
While tracking an expense in self-DM, we correctly set the optimistic report.lastVisibleActionCreated to the actionableTrackExpenseWhisper.created:
https://github.com/Expensify/App/blob/cb88e73de5eb3ae467e3a48de1bd3c4d46a0023a/src/libs/actions/IOU.ts#L1474-L1481
However, when the TrackExpense API response arrives, it changes the report.lastVisibleActionCreated to the tracked expense report action's created time.
So our options are:
- Go with the suggested FE fix
- Fix the BE to set the
actionableTrackExpenseWhisper.createdas thereport.lastVisibleActionCreated
Side note: If we go with the FE fix, we'll have a side-effect that when marking Self-DM as unread, we'll not see the green line above the actionable tracked expense. I'm not sure it's bad though, because currently, we add the green line above the most recent such whisper. So even when there are other messages after it, we'll still add the green line above that whisper.
Current behavior with the green line:
https://github.com/user-attachments/assets/8667cd64-97d4-4b44-8c7b-0900ac0e441c
Behavior after the FE fix:
https://github.com/user-attachments/assets/edafa08a-487b-42e9-aa81-752c1623f699
🎀👀🎀 C+ reviewed
Triggered auto assignment to @dangrous, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
Hrm, I feel like yeah we should match the behavior of other chats - so green line above most recent message, and bold in LHN. So maybe the backend fix is the right one. I can look into this tomorrow; I actually just made a similar change for a different kind of actionable whisper, so I can see if I can do it for this one too.
Okay I have a potential BE fix for this up, but I realized I need one clarification @paultsimura - with the existing code, why does marking the chat as unread, even with additional messages after the whisper, add the green line above the whisper? Is that a BE thing that I can fix here too?
Updating the lastVisibleActionCreated makes the LHN bold on mark as unread, but the green line still appears above the whisper, even when there are more messages below.
Thanks!
Is that a BE thing that I can fix here too?
No, it's an FE thing – the logic for marking the chat unread is to find the last message sent by somebody else and mark it unread.
https://github.com/Expensify/App/blob/91699208ca24ffe378051507e7430f83c4203ebc/src/libs/actions/Report.ts#L1367-L1382
It so happens that, in the self-DM case, nobody else sends messages. Therefore, the last message sent by "not you" is that actionable whisper.
Okay cool, that feels like sort of weird behavior to me but if that's how we have it then we can stick to that for now.
I'll push the backend change up for review today and we can go from there.
Backend is merged, not yet deployed.
Okay, now it's deployed. Do we need any FE changes? (And can we retest please?) thanks!
Okay I just tested on staging and it seems to work.
I did notice another issue that is related but not super important probably? As @paultsimura called out here, mark as unread marks the last message that hasn't been sent by you. Which means you can't mark a self-DM as unread UNLESS it has a track expense whisper in it. I still find this weird, but if we decide to fix it it'll be out of scope of this issue.
I'm going to close this, if anyone disagrees - or if we need payment or anything - let me know!