Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use correct report field title for money request reports #34877

Closed
wants to merge 5 commits into from
Closed

feat: use correct report field title for money request reports #34877

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
51c3baf
feat: use correct report title everywhere
allroundexperts Jan 22, 2024
11a74ef
fix: type errors
allroundexperts Jan 22, 2024
b43d700
fix: type errors
allroundexperts Jan 22, 2024
ca84a64
feat: merge with main
allroundexperts Jan 23, 2024
0f6f93c
fix: create more helper functions for report fields
allroundexperts Jan 25, 2024
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
71 changes: 45 additions & 26 deletions src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import type {
PersonalDetailsList,
Policy,
PolicyReportField,
PolicyReportFields,
Report,
ReportAction,
ReportMetadata,
Expand Down Expand Up @@ -459,6 +460,19 @@ Onyx.connect({
callback: (value) => (allPolicies = value),
});

let allPolicyReportFields: OnyxCollection<PolicyReportFields>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I hope it's not one of these collections we're nuking and un-nuking all over again...

Copy link
Contributor Author

@allroundexperts allroundexperts Jan 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

What do you mean @cubuspl42? This is a new collection that we built specifically for the report fields feature.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is (or might be?) a pattern that sometimes we add an Onyx-synchronized in-memory collection just to nuke it later for performance reasons. But I don't have any sources handy right now. Likely we'll be fine 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm... The only reason why I used it like this was because this function is being used at a lot of places and getting this in all of those places might cause un-needed re-renders. Besides, I see that we're still doing this for report and policies.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it was a general comment, not a change request. This one doesn't need a resolution.

Onyx.connect({
key: ONYXKEYS.COLLECTION.POLICY_REPORT_FIELDS,
waitForCollectionCallback: true,
callback: (value) => (allPolicyReportFields = value),
});

let allBetas: OnyxEntry<Beta[]>;
Onyx.connect({
key: ONYXKEYS.BETAS,
callback: (value) => (allBetas = value),
});

let loginList: OnyxEntry<Login>;
Onyx.connect({
key: ONYXKEYS.LOGIN_LIST,
Expand Down Expand Up @@ -1898,10 +1912,41 @@ function getPolicyExpenseChatName(report: OnyxEntry<Report>, policy: OnyxEntry<P
return reportOwnerDisplayName;
}

/**
* Given a report field and a report, get the title of the field.
* This is specially useful when we have a report field of type formula.
*/
function getReportFieldTitle(report: OnyxEntry<Report>, reportField: PolicyReportField): string {
const value = report?.reportFields?.[reportField.fieldID] ?? reportField.defaultValue;

if (reportField.type !== 'formula') {
return value;
}

return value.replaceAll(CONST.REGEX.REPORT_FIELD_TITLE, (match, property) => {
if (report && property in report) {
return report[property as keyof Report]?.toString() ?? match;
}
return match;
});
}

/**
* Given a report field, check if the field is for the report title.
*/
function isReportFieldOfTypeTitle(reportField: PolicyReportField): boolean {
return reportField.type === 'formula' && reportField.fieldID === CONST.REPORT_FIELD_TITLE_FIELD_ID;
}

allroundexperts marked this conversation as resolved.
Show resolved Hide resolved
/**
* Get the title for an IOU or expense chat which will be showing the payer and the amount
*/
function getMoneyRequestReportName(report: OnyxEntry<Report>, policy: OnyxEntry<Policy> | undefined = undefined): string {
const reportFields = Object.entries(allPolicyReportFields ?? {}).find(([key]) => key.replace(ONYXKEYS.COLLECTION.POLICY_REPORT_FIELDS, '') === report?.policyID)?.[1];
const titleReportField = Object.values(reportFields ?? {})?.find((field) => field.type === 'formula');
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we extract one or two helper functions here? Long chain of invocations, giving it some name could help the readability

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

if (titleReportField && Permissions.canUseReportFields(allBetas ?? [])) {
return getReportFieldTitle(report, titleReportField);
}
const moneyRequestTotal = getMoneyRequestReimbursableTotal(report);
const formattedAmount = CurrencyUtils.convertToDisplayString(moneyRequestTotal, report?.currency, hasOnlyDistanceRequestTransactions(report?.reportID));
const payerOrApproverName = isExpenseReport(report) ? getPolicyName(report, false, policy) : getDisplayNameForParticipant(report?.managerID) ?? '';
Expand Down Expand Up @@ -4551,32 +4596,6 @@ function navigateToPrivateNotes(report: Report, session: Session) {
Navigation.navigate(ROUTES.PRIVATE_NOTES_LIST.getRoute(report.reportID));
}

/**
* Given a report field and a report, get the title of the field.
* This is specially useful when we have a report field of type formula.
*/
function getReportFieldTitle(report: OnyxEntry<Report>, reportField: PolicyReportField): string {
const value = report?.reportFields?.[reportField.fieldID] ?? reportField.defaultValue;

if (reportField.type !== 'formula') {
return value;
}

return value.replaceAll(CONST.REGEX.REPORT_FIELD_TITLE, (match, property) => {
if (report && property in report) {
return report[property as keyof Report]?.toString() ?? match;
}
return match;
});
}

/**
* Given a report field, check if the field is for the report title.
*/
function isReportFieldOfTypeTitle(reportField: PolicyReportField): boolean {
return reportField.type === 'formula' && reportField.fieldID === CONST.REPORT_FIELD_TITLE_FIELD_ID;
}

/**
* Checks if thread replies should be displayed
*/
Expand Down
Loading