Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/libs/ReportNameUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@

let allPersonalDetails: OnyxEntry<PersonalDetailsList>;

Onyx.connect({

Check warning on line 139 in src/libs/ReportNameUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (value) => {
allPersonalDetails = value;
Expand Down Expand Up @@ -725,12 +725,9 @@
if (!report || !report.reportID) {
return '';
}

const privateIsArchivedValue = privateIsArchived ?? allReportNameValuePairs?.[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report.reportID}`]?.private_isArchived;
const reportPolicy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`];

const isArchivedNonExpense = isArchivedNonExpenseReport(report, !!privateIsArchivedValue);

const reportPolicy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`];
const parentReport = reports?.[`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID}`];
const parentReportAction = isThread(report) ? reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.parentReportID}`]?.[report.parentReportActionID] : undefined;

Expand All @@ -741,6 +738,10 @@
return parentReportActionBasedName;
}

if (report?.reportName && report.type === CONST.REPORT.TYPE.EXPENSE) {
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB: in a follow-up pull request, can we please move this check all the way to the top of the function for better optimization? I don't think we need the "is archived non-expense" check, because this is already within the check that the report type is an expense report.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, that indeed we don't need to check isArchivedNonExpenseReport so I'll move this after if (report?.reportName && report.type === CONST.REPORT.TYPE.EXPENSE) check.

But if we do our check before this:

if (parentReportActionBasedName) {
        return parentReportActionBasedName;
}

unit tests will break as it's assumed that there are some special cases based on parent report's actions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Follow up: #82382

return isArchivedNonExpense ? generateArchivedReportName(report?.reportName) : report?.reportName;
}

if (isTaskReport(report)) {
return Parser.htmlToText(report?.reportName ?? '').trim();
}
Expand Down
Loading