[Due for payment 2025-03-13] [$250] LHN - Mark as unread doesn't work with IOU report

[vincdargento]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: v9.0.88-6
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): N/A
Issue reported by: Applause Internal Team

Action Performed:

1. Open ND app
2. Log in with any account
3. Long press on an IOU report in LHN
4. Select Mark as unread

Expected Result:

IOU report should be marked as unread, bold (as in PR #23277)

Actual Result:

Nothing happens

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

bug.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021882151333454468302
• Upwork Job ID: 1882151333454468302
• Last Price Increase: 2025-01-22

Issue Owner

Current Issue Owner: @sakluger

[melvin-bot]

Triggered auto assignment to @sakluger (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.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021882151333454468302

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @akinwale (External)

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Mark as unread doesn't work on a one-expense report.

What is the root cause of that problem?

When we mark as unread a one-expense report, we mark the transaction thread report as unread instead instead of the expense report.

App/src/pages/home/report/ContextMenu/ContextMenuActions.tsx

Lines 283 to 284 in 523edfe

	const originalReportID = getOriginalReportID(reportID, reportAction);
	markCommentAsUnread(originalReportID, reportAction?.created);

getOriginalReportID returns the transaction thread report. getOriginalReportID was added in #34640 to get the thread ancestor reportID correctly.

What changes do you think we should make in order to solve the problem?

We don't need to use getOriginalReportID because reportID passed to the param is already the correct thread ancestor reportID thanks to #43518.

markCommentAsUnread(reportID, reportAction?.created);

This works fine. However, it's limited to marking the expense report action as unread only. If the one-expense report has an update system message and we press mark as unread, the new line message will show above the message "a".

It's because when we mark as unread, it will take the expense report lastVisibleActionCreated (which is message "a"), subtract it by 1, and set it as the new lastReadTime.

App/src/libs/actions/Report.ts

Lines 1430 to 1431 in 523edfe

	const actionCreationTime =
	reportActionCreated || (latestReportActionFromOtherUsers?.created ?? allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.lastVisibleActionCreated ?? DateUtils.getDBTime(0));

To fix this, we should take the transaction thread report lastVisbileActionCreated instead if it's bigger, just like here.

App/src/pages/home/report/ReportActionsList.tsx

Lines 383 to 386 in 523edfe

	const lastVisibleActionCreated =
	(transactionThreadReport?.lastVisibleActionCreated ?? '') > (report.lastVisibleActionCreated ?? '')
	? transactionThreadReport?.lastVisibleActionCreated
	: report.lastVisibleActionCreated;

const report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`];
const transactionThreadReportID = ReportActionsUtils.getOneTransactionThreadReportID(reportID, reportActions ?? []);
const transactionThreadReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionThreadReportID}`];
const lastVisibleActionCreated = (transactionThreadReport?.lastVisibleActionCreated ?? '') > (report?.lastVisibleActionCreated ?? '')
    ? transactionThreadReport?.lastVisibleActionCreated
    : report?.lastVisibleActionCreated;
// If no action created date is provided, use the last action's from other user
const actionCreationTime =
    reportActionCreated || (latestReportActionFromOtherUsers?.created ?? lastVisibleActionCreated ?? DateUtils.getDBTime(0));

But we still have another problem. Even though the new line message now shows on the last system message, the LHN is not bold. That's because isUnread currently only compares the expense report lastReadTime with the expense report lastVisibleActionCreated. In this case, it's always bigger. (remember, the lastReadTime is set to the transaction thread lastVisibleActionCreated - 1 which is bigger than the message "a" created time)

App/src/libs/ReportUtils.ts

Lines 6563 to 6568 in 523edfe

	const lastVisibleActionCreated = report.lastVisibleActionCreated ?? '';
	const lastReadTime = report.lastReadTime ?? '';
	const lastMentionedTime = report.lastMentionedTime ?? '';
	// If the user was mentioned and the comment got deleted the lastMentionedTime will be more recent than the lastVisibleActionCreated
	return lastReadTime < lastVisibleActionCreated || lastReadTime < lastMentionedTime;

To fix this, we need to apply the same solution above.

const lastVisibleActionCreated =
    (transactionThreadReport?.lastVisibleActionCreated ?? '') > (report.lastVisibleActionCreated ?? '')
        ? transactionThreadReport?.lastVisibleActionCreated
        : report.lastVisibleActionCreated;
...

return lastReadTime < (lastVisibleActionCreated ?? '') || lastReadTime < lastMentionedTime;

I think we can create a util function to get the lastVisibleActionCreated. transactionThreadReport will be a new param for isUnread function. We will pass it down from LHNOptionsList > OptionRowLHNData > SidebarUtils.getOptionData > isUnread.
To get the transactionThreadReport in LHNOptionsList, we can use getOneTransactionThreadReportID.

App/src/components/LHNOptionsList/LHNOptionsList.tsx

Lines 117 to 122 in 523edfe

	const renderItem = useCallback(
	({item: reportID}: RenderItemProps): ReactElement => {
	const itemFullReport = reports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`];
	const itemParentReport = reports?.[`${ONYXKEYS.COLLECTION.REPORT}${itemFullReport?.parentReportID ?? '-1'}`];
	const itemReportActions = reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`];
	const itemParentReportActions = reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${itemFullReport?.parentReportID}`];

// pass this down to OptionRowLHNData
const itemTransactionThreadReport = reports?.[`${ONYXKEYS.COLLECTION.REPORT}${ReportActionsUtils.getOneTransactionThreadReportID(reportID, itemReportActions, isOffline)}`];

This will now show a bold LHN because lastReadTime is the transaction thread lastVisibleActionCreated - 1 which is smaller than the transaction thread lastVisibleActionCreated itself.

However, there is still 1 problem left. If a new system message is added, the LHN will be bold even though it's not unread. That's because the expense report lastReadTime is smaller than the transaction thread lastVisibleActionCreated. For example, let's say the expense report has a message A and the lastReadTime is 1. When I edit the money request, a new system message is added to the transaction thread which updates the transaction thread lastReadTime and lastVisibleActionCreated to 2. Since 1 < 2, the LHN is bold. To fix this, we need to update the expense report lastReadTime too when updating the money request.

App/src/libs/actions/IOU.ts

Line 3209 in 523edfe

function getUpdateMoneyRequestParams(

if (isOneTransactionThread(transactionThreadReportID, iouReport?.reportID, undefined)) {
    optimisticData.push(
        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: `${ONYXKEYS.COLLECTION.REPORT}${iouReport?.reportID}`,
            value: {
                lastReadTime: updatedReportAction.created,
            },
        },
    );
}

And also update the effect here to include the transaction thread lastVisibleActionCreated.

App/src/pages/home/report/ReportActionsList.tsx

Lines 420 to 435 in 523edfe

	if (isUnread(report)) {
	// On desktop, when the notification center is displayed, isVisible will return false.
	// Currently, there's no programmatic way to dismiss the notification center panel.
	// To handle this, we use the 'referrer' parameter to check if the current navigation is triggered from a notification.
	const isFromNotification = route?.params?.referrer === CONST.REFERRER.NOTIFICATION;
	if ((isVisible || isFromNotification) && scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD) {
	readNewestAction(report.reportID);
	if (isFromNotification) {
	Navigation.setParams({referrer: undefined});
	}
	} else {
	readActionSkipped.current = true;
	}
	}
	// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
	}, [report.lastVisibleActionCreated, report.reportID, isVisible]);

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We can add a new test to UnreadIndicatorsTest.ts to verify the new message line shows on the correct message and LHN is bolded correctly.

[akinwale]

@bernhardoj's proposal is thorough and we can go with the solution here.

🎀👀🎀 C+ reviewed.

[melvin-bot]

Triggered auto assignment to @dangrous, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[dangrous]

Cool! A lot of moving pieces but that all makes sense. Only quick thought - we should make sure that updating the lastReadTime on any updated request doesn't cause any weirdness elsewhere. I don't expect it will, but it seems like the only part of this that could cause new bugs.

Thanks!

[bernhardoj]

This turned out to be more complex. I'll need more time to open the PR

[bernhardoj]

Okay, the problem that I'm facing is actually an existing issue: #54679, but was closed because I can't repro it on web anymore, but turns out it's easily reproducible on Android/mWeb. I have requested to reopen it so we can fix that first.

[sakluger]

Great find @bernhardoj! I reopened that other issue and added a hold to this issue for that one.

[bernhardoj]

@sakluger thanks!

[dangrous]

I think this is good to move forward and take off hold - looks like the fix for that linked issue is on prod?

[bernhardoj]

Oh, you're right. I'll prepare the PR.

[bernhardoj]

PR is ready

cc: @akinwale

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.1.9-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix can't mark one-expense report as unread #56893

If no regressions arise, payment will be issued on 2025-03-13. 🎊

For reference, here are some details about the assignees on this issue:

• @akinwale requires payment through NewDot Manual Requests
• @bernhardoj requires payment through NewDot Manual Requests

[melvin-bot]

@akinwale @sakluger @akinwale The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]