Skip to content
42 changes: 29 additions & 13 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,8 @@ type GetReportNameParams = {
policies?: SearchPolicy[];
};

type ReportByPolicyMap = Record<string, Report[]>;

let currentUserEmail: string | undefined;
let currentUserPrivateDomain: string | undefined;
let currentUserAccountID: number | undefined;
Expand Down Expand Up @@ -779,6 +781,7 @@ Onyx.connect({
});

let allReports: OnyxCollection<Report>;
let reportsByPolicyID: ReportByPolicyMap;
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
Expand All @@ -794,12 +797,27 @@ Onyx.connect({
if (!value) {
return;
}
Object.values(value).forEach((report) => {

reportsByPolicyID = Object.values(value).reduce<ReportByPolicyMap>((acc, report) => {
if (!report) {
return;
return acc;
}

handleReportChanged(report);
});

// Get all reports, which are the ones that are:
// - Owned by the same user
// - Are either open or submitted
// - Belong to the same workspace
if (report.policyID && report.ownerAccountID === currentUserAccountID && (report.stateNum ?? 0) <= 1) {
if (!acc[report.policyID]) {
acc[report.policyID] = [];
}
acc[report.policyID].push(report);
}

return acc;
}, {});
},
});

Expand Down Expand Up @@ -6926,17 +6944,15 @@ function shouldDisplayViolationsRBRInLHN(report: OnyxEntry<Report>, transactionV
if (!isCurrentUserSubmitter(report.reportID)) {
return false;
}
if (!report.policyID || !reportsByPolicyID) {
return false;
}

// Get all potential reports, which are the ones that are:
// - Owned by the same user
// - Are either open or submitted
// - Belong to the same workspace
// And if any have a violation, then it should have a RBR
const reports = Object.values(allReports ?? {}) as Report[];
const potentialReports = reports.filter((r) => r?.ownerAccountID === currentUserAccountID && (r?.stateNum ?? 0) <= 1 && r?.policyID === report.policyID);
return potentialReports.some(
(potentialReport) => hasViolations(potentialReport.reportID, transactionViolations) || hasWarningTypeViolations(potentialReport.reportID, transactionViolations),
);
// If any report has a violation, then it should have a RBR
const potentialReports = reportsByPolicyID[report.policyID] ?? [];
return potentialReports.some((potentialReport) => {
return hasViolations(potentialReport.reportID, transactionViolations) || hasWarningTypeViolations(potentialReport.reportID, transactionViolations);
});
}

/**
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/SidebarUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,12 @@ describe('SidebarUtils', () => {
};

await Onyx.multiSet({
...MOCK_REPORTS,
...MOCK_TRANSACTION_VIOLATIONS,
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${MOCK_REPORT.reportID}` as const]: MOCK_REPORT_ACTIONS,
[ONYXKEYS.SESSION]: {
accountID: 12345,
},
...MOCK_REPORTS,
...MOCK_TRANSACTION_VIOLATIONS,
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${MOCK_REPORT.reportID}` as const]: MOCK_REPORT_ACTIONS,
Comment on lines +67 to +69

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually interesting why our tests broke after refactoring the code and we need to update tests after that 🤔

@TMisiukiewicz TMisiukiewicz Feb 12, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I had to spend some time to figure it out 🙈 The Onyx callback responsible for creating a map of reports by policy ID is using a value based on session, so when it was trying to access it, it was still undefined because it was declared in multiSet after reports

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh
Got it !
Thanks for the explanation !

[`${ONYXKEYS.COLLECTION.TRANSACTION}${MOCK_TRANSACTION.transactionID}` as const]: MOCK_TRANSACTION,
});

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/WorkspaceSettingsUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ describe('WorkspacesSettingsUtils', () => {
const transactions = mockData.transactions;

await Onyx.multiSet({
session,
...(reports as ReportCollectionDataSet),
...(reportActions as OnyxCollection<ReportActions>),
...(transactionViolations as OnyxCollection<TransactionViolations>),
...(transactions as OnyxCollection<Transaction>),
session,
});

await waitForBatchedUpdates();
Expand Down