Skip to content

Commit 7f29b17

Browse files
authored
Merge pull request Expensify#86727 from callstack-internal/fix/smartscan-iou-report-lookup
fix: pass iouReport instead of chatReport for smartscan field check
2 parents 892edd0 + bc5b697 commit 7f29b17

4 files changed

Lines changed: 150 additions & 14 deletions

File tree

src/libs/ReportUtils.ts

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5253,7 +5253,7 @@ function getLinkedTransaction(reportAction: OnyxEntry<ReportAction | OptimisticI
52535253
/**
52545254
* Get report action which is missing smartscan fields
52555255
*/
5256-
function getReportActionWithMissingSmartscanFields(report: OnyxEntry<Report>, iouReportID: string | undefined): ReportAction | undefined {
5256+
function getReportActionWithMissingSmartscanFields(iouReport: OnyxEntry<Report>, iouReportID: string | undefined): ReportAction | undefined {
52575257
const reportActions = Object.values(getAllReportActions(iouReportID));
52585258
return reportActions.find((action) => {
52595259
if (!isMoneyRequestAction(action)) {
@@ -5266,15 +5266,15 @@ function getReportActionWithMissingSmartscanFields(report: OnyxEntry<Report>, io
52665266
if (!wasActionTakenByCurrentUser(action)) {
52675267
return false;
52685268
}
5269-
return hasMissingSmartscanFieldsTransactionUtils(transaction, report);
5269+
return hasMissingSmartscanFieldsTransactionUtils(transaction, iouReport);
52705270
});
52715271
}
52725272

52735273
/**
52745274
* Check if iouReportID has required missing fields
52755275
*/
5276-
function shouldShowRBRForMissingSmartscanFields(report: OnyxEntry<Report>, iouReportID: string | undefined): boolean {
5277-
return !!getReportActionWithMissingSmartscanFields(report, iouReportID);
5276+
function shouldShowRBRForMissingSmartscanFields(iouReport: OnyxEntry<Report>, iouReportID: string | undefined): boolean {
5277+
return !!getReportActionWithMissingSmartscanFields(iouReport, iouReportID);
52785278
}
52795279

52805280
/**
@@ -9353,6 +9353,7 @@ function getAllReportActionsErrorsAndReportActionThatRequiresAttention(
93539353
report: OnyxEntry<Report>,
93549354
reportActions: OnyxEntry<ReportActions>,
93559355
isReportArchived = false,
9356+
reports?: OnyxCollection<Report>,
93569357
): ReportErrorsAndReportActionThatRequiresAttention {
93579358
const reportActionsArray = Object.values(reportActions ?? {}).filter((action) => !isDeletedAction(action));
93589359
const reportActionErrors: ErrorFields = {};
@@ -9368,9 +9369,9 @@ function getAllReportActionsErrorsAndReportActionThatRequiresAttention(
93689369
}
93699370
}
93709371

9371-
if (!isReportArchived && hasSmartscanError(reportActionsArray, report)) {
9372+
if (!isReportArchived && hasSmartscanError(reportActionsArray, report, reports)) {
93729373
reportActionErrors.smartscan = getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericSmartscanFailureMessage');
9373-
reportAction = getReportActionWithSmartscanError(reportActionsArray, report);
9374+
reportAction = getReportActionWithSmartscanError(reportActionsArray, report, reports);
93749375
}
93759376

93769377
if (!isReportArchived && isReportOwner(report) && report?.statusNum === CONST.REPORT.STATUS_NUM.OPEN) {
@@ -9390,9 +9391,9 @@ function getAllReportActionsErrorsAndReportActionThatRequiresAttention(
93909391
/**
93919392
* Get an object of error messages keyed by microtime by combining all error objects related to the report.
93929393
*/
9393-
function getAllReportErrors(report: OnyxEntry<Report>, reportActions: OnyxEntry<ReportActions>, isReportArchived = false): Errors {
9394+
function getAllReportErrors(report: OnyxEntry<Report>, reportActions: OnyxEntry<ReportActions>, isReportArchived = false, reports?: OnyxCollection<Report>): Errors {
93949395
const reportErrorFields = report?.errorFields ?? {};
9395-
const {errors: reportActionErrors} = getAllReportActionsErrorsAndReportActionThatRequiresAttention(report, reportActions, isReportArchived);
9396+
const {errors: reportActionErrors} = getAllReportActionsErrorsAndReportActionThatRequiresAttention(report, reportActions, isReportArchived, reports);
93969397

93979398
// All error objects related to the report. Each object in the sources contains error messages keyed by microtime
93989399
const errorSources = {
@@ -10749,15 +10750,16 @@ function canEditReportDescription(report: OnyxEntry<Report>, policy: OnyxEntry<P
1074910750
);
1075010751
}
1075110752

10752-
function getReportActionWithSmartscanError(reportActions: ReportAction[], report: OnyxEntry<Report>): ReportAction | undefined {
10753+
function getReportActionWithSmartscanError(reportActions: ReportAction[], report: OnyxEntry<Report>, reports?: OnyxCollection<Report>): ReportAction | undefined {
1075310754
return reportActions.find((action) => {
1075410755
const isReportPreview = isReportPreviewAction(action);
1075510756
const isSplitOrTrackAction = isSplitBillReportAction(action) || isTrackExpenseAction(action);
1075610757
if (!isSplitOrTrackAction && !isReportPreview) {
1075710758
return false;
1075810759
}
1075910760
const IOUReportID = getIOUReportIDFromReportActionPreview(action);
10760-
const isReportPreviewError = isReportPreview && shouldShowRBRForMissingSmartscanFields(report, IOUReportID) && !isSettled(IOUReportID);
10761+
const iouReport = IOUReportID ? reports?.[`${ONYXKEYS.COLLECTION.REPORT}${IOUReportID}`] : undefined;
10762+
const isReportPreviewError = isReportPreview && shouldShowRBRForMissingSmartscanFields(iouReport ?? report, IOUReportID) && !isSettled(IOUReportID);
1076110763
if (isReportPreviewError) {
1076210764
return true;
1076310765
}
@@ -10773,8 +10775,8 @@ function getReportActionWithSmartscanError(reportActions: ReportAction[], report
1077310775
/**
1077410776
* Checks if report action has error when smart scanning
1077510777
*/
10776-
function hasSmartscanError(reportActions: ReportAction[], report: OnyxEntry<Report>): boolean {
10777-
return !!getReportActionWithSmartscanError(reportActions, report);
10778+
function hasSmartscanError(reportActions: ReportAction[], report: OnyxEntry<Report>, reports?: OnyxCollection<Report>): boolean {
10779+
return !!getReportActionWithSmartscanError(reportActions, report, reports);
1077810780
}
1077910781

1078010782
function shouldAutoFocusOnKeyPress(event: KeyboardEvent): boolean {
@@ -12843,6 +12845,7 @@ function generateReportAttributes({
1284312845
reportActions,
1284412846
transactionViolations,
1284512847
isReportArchived = false,
12848+
reports,
1284612849
}: {
1284712850
report: OnyxEntry<Report>;
1284812851
chatReport: OnyxEntry<Report>;
@@ -12851,12 +12854,13 @@ function generateReportAttributes({
1285112854
isReportArchived: boolean;
1285212855
actionBadge?: ValueOf<typeof CONST.REPORT.ACTION_BADGE>;
1285312856
actionTargetReportActionID?: string;
12857+
reports?: OnyxCollection<Report>;
1285412858
}) {
1285512859
const reportActionsList = reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.reportID}`];
1285612860
const parentReportActionsList = reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.parentReportID}`];
1285712861
const hasViolationsToDisplayInLHN = !!getViolatingReportIDForRBRInLHN(report, transactionViolations);
1285812862
const hasAnyTypeOfViolations = hasViolationsToDisplayInLHN;
12859-
const reportErrors = getAllReportErrors(report, reportActionsList, isReportArchived);
12863+
const reportErrors = getAllReportErrors(report, reportActionsList, isReportArchived, reports);
1286012864
const hasErrors = Object.entries(reportErrors ?? {}).length > 0;
1286112865
const oneTransactionThreadReportID = getOneTransactionThreadReportID(report, chatReport, reportActionsList);
1286212866
const parentReportAction = report?.parentReportActionID ? parentReportActionsList?.[report.parentReportActionID] : undefined;

src/libs/SidebarUtils.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,7 @@ function getReasonAndReportActionThatHasRedBrickRoad(
635635
transactions: OnyxCollection<Transaction>,
636636
transactionViolations?: OnyxCollection<TransactionViolation[]>,
637637
isReportArchived = false,
638+
reports?: OnyxCollection<Report>,
638639
): ReasonAndReportActionThatHasRedBrickRoad | null {
639640
if (isReportArchived) {
640641
return null;
@@ -649,7 +650,7 @@ function getReasonAndReportActionThatHasRedBrickRoad(
649650
};
650651
}
651652

652-
const {reportAction} = getAllReportActionsErrorsAndReportActionThatRequiresAttention(report, reportActions, isReportArchived);
653+
const {reportAction} = getAllReportActionsErrorsAndReportActionThatRequiresAttention(report, reportActions, isReportArchived, reports);
653654
const errors = reportErrors;
654655
const hasErrors = Object.keys(errors).length !== 0;
655656

src/libs/actions/OnyxDerived/configs/reportAttributes.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ export default createOnyxDerivedValueConfig({
265265
reportActions,
266266
transactionViolations,
267267
isReportArchived,
268+
reports,
268269
});
269270

270271
const policy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`];
@@ -282,6 +283,7 @@ export default createOnyxDerivedValueConfig({
282283
transactions,
283284
transactionViolations,
284285
!!isReportArchived,
286+
reports,
285287
);
286288
// if report has errors or violations, show red dot
287289
if (reasonAndReportAction) {

tests/unit/ReportUtilsTest.ts

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ import {
9696
getPolicyIDsWithEmptyReportsForAccount,
9797
getPolicyName,
9898
getReasonAndReportActionThatRequiresAttention,
99+
getReportActionWithSmartscanError,
99100
getReportIDFromLink,
100101
getReportName as getReportNameDeprecated,
101102
getReportOrDraftReport,
@@ -112,6 +113,7 @@ import {
112113
hasActionWithErrorsForTransaction,
113114
hasEmptyReportsForPolicy,
114115
hasReceiptError,
116+
hasSmartscanError,
115117
hasVisibleReportFieldViolations,
116118
isAllowedToApproveExpenseReport,
117119
isArchivedNonExpenseReport,
@@ -15898,4 +15900,131 @@ describe('ReportUtils', () => {
1589815900
expect(action.reportActionID).toBeTruthy();
1589915901
});
1590015902
});
15903+
15904+
describe('getReportActionWithSmartscanError', () => {
15905+
const chatReportID = '100';
15906+
const expenseReportID = '200';
15907+
const transactionID = '300';
15908+
const iouReportActionID = '400';
15909+
15910+
const chatReport: Report = {
15911+
...LHNTestUtils.getFakeReport(),
15912+
reportID: chatReportID,
15913+
type: CONST.REPORT.TYPE.CHAT,
15914+
};
15915+
15916+
const expenseReport: Report = {
15917+
...LHNTestUtils.getFakeReport(),
15918+
reportID: expenseReportID,
15919+
type: CONST.REPORT.TYPE.EXPENSE,
15920+
parentReportID: chatReportID,
15921+
ownerAccountID: currentUserAccountID,
15922+
statusNum: CONST.REPORT.STATUS_NUM.OPEN,
15923+
};
15924+
15925+
// Transaction with $0 amount but valid merchant and created date.
15926+
// On an expense report this should NOT be a smartscan error (amount is
15927+
// irrelevant; only merchant matters). On a chat report the old code path
15928+
// would incorrectly flag getAmount(txn) === 0 as a missing field.
15929+
const transaction: Transaction = {
15930+
...createRandomTransaction(Number(transactionID)),
15931+
transactionID,
15932+
reportID: expenseReportID,
15933+
amount: 0,
15934+
merchant: 'Coffee Shop',
15935+
modifiedMerchant: '',
15936+
created: testDate,
15937+
};
15938+
15939+
// Money-request (IOU) action that lives inside the expense report's actions
15940+
const iouAction = {
15941+
...createRandomReportAction(Number(iouReportActionID)),
15942+
reportActionID: iouReportActionID,
15943+
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
15944+
actorAccountID: currentUserAccountID,
15945+
originalMessage: {
15946+
IOUTransactionID: transactionID,
15947+
IOUReportID: expenseReportID,
15948+
type: CONST.IOU.REPORT_ACTION_TYPE.CREATE,
15949+
amount: 0,
15950+
currency: CONST.CURRENCY.USD,
15951+
comment: '',
15952+
participantAccountIDs: [currentUserAccountID],
15953+
},
15954+
};
15955+
15956+
// REPORT_PREVIEW action that sits in the chat report and links to the expense report
15957+
const reportPreviewAction = buildOptimisticReportPreview(chatReport, expenseReport, '', transaction);
15958+
15959+
beforeAll(async () => {
15960+
await Onyx.set(ONYXKEYS.SESSION, {email: currentUserEmail, accountID: currentUserAccountID});
15961+
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, transaction);
15962+
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReportID}`, {[iouReportActionID]: iouAction});
15963+
return waitForBatchedUpdates();
15964+
});
15965+
15966+
it('should NOT flag $0 manual expense as smartscan error when expense report is resolved via reports collection', () => {
15967+
// With the reports collection the function can look up the actual
15968+
// expense report (type 'expense') via IOUReportID. For expense
15969+
// reports areRequiredFieldsEmpty checks isMerchantMissing (not
15970+
// amount), so a $0 expense with a valid merchant is fine.
15971+
const reportsCollection = {
15972+
[`${ONYXKEYS.COLLECTION.REPORT}${expenseReportID}`]: expenseReport,
15973+
};
15974+
15975+
const result = getReportActionWithSmartscanError([reportPreviewAction], chatReport, reportsCollection);
15976+
expect(result).toBeUndefined();
15977+
expect(hasSmartscanError([reportPreviewAction], chatReport, reportsCollection)).toBe(false);
15978+
});
15979+
15980+
it('should flag smartscan error when expense report has a missing merchant', async () => {
15981+
// Replace the transaction with one that has an empty merchant
15982+
const transactionMissingMerchant: Transaction = {
15983+
...transaction,
15984+
merchant: '',
15985+
modifiedMerchant: '',
15986+
};
15987+
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, transactionMissingMerchant);
15988+
await waitForBatchedUpdates();
15989+
15990+
const reportsCollection = {
15991+
[`${ONYXKEYS.COLLECTION.REPORT}${expenseReportID}`]: expenseReport,
15992+
};
15993+
15994+
expect(hasSmartscanError([reportPreviewAction], chatReport, reportsCollection)).toBe(true);
15995+
15996+
// Restore original transaction for subsequent tests
15997+
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, transaction);
15998+
await waitForBatchedUpdates();
15999+
});
16000+
16001+
it('should NOT flag settled (reimbursed) expense reports even with missing fields', async () => {
16002+
const settledExpenseReport: Report = {
16003+
...expenseReport,
16004+
statusNum: CONST.REPORT.STATUS_NUM.REIMBURSED,
16005+
};
16006+
16007+
// Even with missing merchant, a settled report should not show error
16008+
const transactionMissingMerchant: Transaction = {
16009+
...transaction,
16010+
merchant: '',
16011+
modifiedMerchant: '',
16012+
};
16013+
// isSettled reads from the global allReports Onyx store, so we must persist the settled status there
16014+
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, transactionMissingMerchant);
16015+
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${expenseReportID}`, settledExpenseReport);
16016+
await waitForBatchedUpdates();
16017+
16018+
const reportsCollection = {
16019+
[`${ONYXKEYS.COLLECTION.REPORT}${expenseReportID}`]: settledExpenseReport,
16020+
};
16021+
16022+
expect(hasSmartscanError([reportPreviewAction], chatReport, reportsCollection)).toBe(false);
16023+
16024+
// Restore
16025+
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, transaction);
16026+
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${expenseReportID}`, expenseReport);
16027+
await waitForBatchedUpdates();
16028+
});
16029+
});
1590116030
});

0 commit comments

Comments
 (0)