[$1000] [MEDIUM] Scan - No red dot for transaction thread in LHN when scanning fails

[lanitochka17]

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: 1.4.28-0
Reproducible in staging?: Y
Reproducible in production?: N
Issue reported by: Applause - Internal Team

Issue found when executing PR #31448

Action Performed:

1. Go to workspace chat
2. Create a scan request with receipt that will fail the scanning
3. Wait for the scanning to fail
4. Click on the preview to open report page
5. Click on the preview to open details page

Expected Result:

• The transaction thread should have the RBR on the LHN for the submitter.
• The transaction thread should not have the RBR on the LHN for the admin.
• The expense report should not have the RBR on the LHN for either user

Actual Result:

Both expense report and details page do not show red dot in LHN

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6347876_1705685194195.20240120_001908.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e2581b13bb117eee
• Upwork Job ID: 1757711391539257344
• Last Price Increase: 2024-02-16

[melvin-bot]

Triggered auto assignment to @laurenreidexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @marcochavezf (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[lanitochka17]

We think that this bug might be related to #wave6.
CC @greg-schroeder

[mountiny]

We should fix this but I dont think its that major to be a deploy blocker now cc @cead22

[cead22]

Agreed, let's HOLD this one too, since I think it may be fixed by a fix for a bug we just found in the main violations PR -- context here https://expensify.slack.com/archives/C01GTK53T8Q/p1705693401154309?thread_ts=1705692222.233999&cid=C01GTK53T8Q

[laurenreidexpensify]

on hold for bug mentioned above, not overdue

[marcochavezf]

Hi @cead22, this is the PR we were holding for this issue, correct?

[melvin-bot]

Current assignee @allroundexperts is eligible for the Internal assigner, not assigning anyone new.

[melvin-bot]

Upwork job price has been updated to $1000

[laurenreidexpensify]

increased bounty

[bernhardoj]

Proposal

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

Based on the new expected result, I see only the last one that doesn't match, that is the red dot shown in the expense report.

What is the root cause of that problem?

We have a logic here to get the report errors that include expense/IOU reports which will show the red dot error if the user is the owner.

App/src/libs/OptionsListUtils.ts

Lines 487 to 490 in 20af9e5

	} else if ((ReportUtils.isIOUReport(report) || ReportUtils.isExpenseReport(report)) && report?.ownerAccountID === currentUserAccountID) {
	if (ReportUtils.hasMissingSmartscanFields(report?.reportID ?? '') && !ReportUtils.isSettled(report?.reportID)) {
	reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage');
	}

This was first added in #27494.

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

If we want to change it, then we can simply remove the condition or just the expense report if we want to keep the IOU one.

(IOU report only) Related to the red dot, I see that the red dot is shown in the report preview, DM chat LHN, and the request preview even though they are not the owner. I think the red dot shouldn't be shown if they aren't the owner because they can't edit it. We need to check the owner, but I didn't include the details here as it could be expected and probably out of scope

[Tony-MK]

Proposal

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

Scan - No red dot for transaction thread in LHN when scanning fails

What is the root cause of that problem?

In trying to reproduce this bug, the following expectations from #34827 (comment) were fulfilled:

• The transaction thread should have the RBR on the LHN for the submitter.
• The transaction thread should not have the RBR on the LHN for the admin.

However the third expectation, "The expense report should not have the RBR on the LHN for either user", is not fulfilled.

App/src/libs/OptionsListUtils.ts

Lines 481 to 493 in 91ea979

	if (parentReportAction?.actorAccountID === currentUserAccountID && ReportActionUtils.isTransactionThread(parentReportAction)) {
	const transactionID = parentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU ? parentReportAction?.originalMessage?.IOUTransactionID : null;
	const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`];
	if (TransactionUtils.hasMissingSmartscanFields(transaction ?? null) && !ReportUtils.isSettled(transaction?.reportID)) {
	reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage');
	}
	} else if ((ReportUtils.isIOUReport(report) || ReportUtils.isExpenseReport(report)) && report?.ownerAccountID === currentUserAccountID) {
	if (ReportUtils.hasMissingSmartscanFields(report?.reportID ?? '') && !ReportUtils.isSettled(report?.reportID)) {
	reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage');
	}
	} else if (ReportUtils.hasSmartscanError(Object.values(reportActions ?? {}))) {
	reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage');
	}

The third expectation is caused by the third condition in the getAllReportErrors function since the function does not consider report?.ownerAccountID === currentUserAccountID.

Therefore, the third condition will pass because the ReportUtils.hasSmartscanError returned true for the reportActions.

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

In the third condition, check if the report is an expense report, to prevent reportActionErrors.smartscan to be being set to expense reports that didn't meet the report?.ownerAccountID === currentUserAccountID conditions.

Hence, let's use the ReportUtils.isExpenseReport function as such:

else if (!ReportUtils.isExpenseReport(report) && ReportUtils.hasSmartscanError(Object.values(reportActions ?? {})))

App/src/libs/OptionsListUtils.ts

Lines 491 to 493 in 91ea979

	} else if (ReportUtils.hasSmartscanError(Object.values(reportActions ?? {}))) {
	reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage');
	}

[laurenreidexpensify]

@allroundexperts couple of proposals to review ^^

[allroundexperts]

Still reviewing these. Need a little more time. Will provide an update shortly.

[allroundexperts]

Can we confirm if even for IOU reports, we don't want to show the RBR in LHN if there is a smart scan error? @marcochavezf @cead22

[allroundexperts]

@bernhardoj Removing the second condition won't work in this case since the third condition would simply kick in, causing the RBR to show up.

@Tony-MK Removing only the last condition would result in showing of an RBR for the expense report if the user is admin which we don't want. As such, I don't think that your proposal would work.

[allroundexperts]

Still looking for a more accurate proposal.

[bernhardoj]

The third condition won't kick in because it will return false if it's not a report preview action. The condition is used to show the RBR on the chat report if there is a split bill or report preview error.

App/src/libs/ReportUtils.ts

Lines 4767 to 4770 in 0797c0a

	return reportActions.some((action) => {
	if (!ReportActionsUtils.isSplitBillAction(action) && !ReportActionsUtils.isReportPreviewAction(action)) {
	return false;
	}

[allroundexperts]

Ah, I did not notice this condition. Given this, your proposal should be good.

We just need to confirm if we need to show the RBR for IOU report or not.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[cead22]

If by IOU report you mean reports holding p2p request between users and outside of a workspace, then those won't get violations. Only money requests on workspaces get violations. Let me know if that answers your question

[Tony-MK]

@allroundexperts, I think you might be confusing the two proposals.

My proposal did not suggest removing any conditions.

Kindly, could you re-review my proposal?

Thanks

[Tony-MK]

Hi @cead22, just a quick question.

In a p2p request, is it expected to show the RBR of IOUs for only the requestor to be notified? 🤔

Thank you.

[cead22]

• In a p2p request that is outside of a workspace, there should be no RBR for violations showing for any user
• In a money request made in the context of a workspace, the RBR should show for everyone on the report preview, and theh money request preview. And the RBR on the LHN should only be shown to the person making the request

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[laurenreidexpensify]

I am closing based on the above comment that this is no longer reproducible

[eVoloshchak]

Hey folks, this is still reproducible in #37044
Should we handle it there or re-open this issue?

[allroundexperts]

We should re-open this. cc @laurenreidexpensify