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

Jump to bottom

Avoid unnecessary expensive call to getTransactionDetails #45529

Merged
merged 4 commits into from
Jul 19, 2024
Merged

Avoid unnecessary expensive call to getTransactionDetails #45529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2d012da
Avoid unnecessary expensive call to getTransactionDetails
neil-marcellini Jul 16, 2024
f2a450d
Remove more inefficient calls to getTransactionDetails
neil-marcellini Jul 16, 2024
e99e114
Fix ghToken arg name in prompt
neil-marcellini Jul 17, 2024
59c7221
Update other ghToken arg prompt
neil-marcellini Jul 17, 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
4 changes: 2 additions & 2 deletions scripts/symbolicate-profile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ if (Object.keys(argsMap).length === 0 || argsMap.help !== undefined) {
Logger.log('Options:');
Logger.log(' --profile=<filename> The .cpuprofile file to symbolicate');
Logger.log(' --platform=<ios|android> The platform for which the source map was uploaded');
Logger.log(' --gh-token Token to use for requests send to the GitHub API. By default tries to pick up from the environment variable GITHUB_TOKEN');
Logger.log(' --ghToken Token to use for requests send to the GitHub API. By default tries to pick up from the environment variable GITHUB_TOKEN');
mountiny marked this conversation as resolved.
Show resolved Hide resolved
Logger.log(' --help Display this help message');
process.exit(0);
}
Expand All @@ -53,7 +53,7 @@ if (argsMap.platform === undefined) {

const githubToken = argsMap.ghToken ?? process.env.GITHUB_TOKEN;
if (githubToken === undefined) {
Logger.error('No GitHub token provided. Either set a GITHUB_TOKEN environment variable or pass it using --gh-token');
Logger.error('No GitHub token provided. Either set a GITHUB_TOKEN environment variable or pass it using --ghToken');
process.exit(1);
}

Expand Down
31 changes: 16 additions & 15 deletions src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2958,16 +2958,17 @@ function getTransactionReportName(reportAction: OnyxEntry<ReportAction | Optimis
return Localize.translateLocal('iou.fieldPending');
}

const transactionDetails = getTransactionDetails(transaction);
if (ReportActionsUtils.isSentMoneyReportAction(reportAction)) {
return getIOUReportActionDisplayMessage(reportAction as ReportAction, transaction);
}

const formattedAmount = CurrencyUtils.convertToDisplayString(transactionDetails?.amount ?? 0, transactionDetails?.currency) ?? '';
const comment = (!TransactionUtils.isMerchantMissing(transaction) ? transactionDetails?.merchant : transactionDetails?.comment) ?? '';
const report = getReportOrDraftReport(transaction?.reportID);
const amount = TransactionUtils.getAmount(transaction, !isEmptyObject(report) && isExpenseReport(report)) ?? 0;
const formattedAmount = CurrencyUtils.convertToDisplayString(amount, TransactionUtils.getCurrency(transaction)) ?? '';
const comment = (!TransactionUtils.isMerchantMissing(transaction) ? TransactionUtils.getMerchant(transaction) : TransactionUtils.getDescription(transaction)) ?? '';
if (ReportActionsUtils.isTrackExpenseAction(reportAction)) {
return Localize.translateLocal('iou.threadTrackReportName', {formattedAmount, comment});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

should we also move this block up?

Copy link
Contributor

Choose a reason for hiding this comment

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

can we? it relies on the formattedAmount and comment being defined 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Lets explore this later if its possible?

if (ReportActionsUtils.isSentMoneyReportAction(reportAction)) {
return getIOUReportActionDisplayMessage(reportAction as ReportAction, transaction);
}
return Localize.translateLocal('iou.threadExpenseReportName', {formattedAmount, comment});
}

Expand Down Expand Up @@ -3013,9 +3014,9 @@ function getReportPreviewMessage(
return Localize.translateLocal('iou.receiptMissingDetails');
}

const transactionDetails = getTransactionDetails(linkedTransaction);
const formattedAmount = CurrencyUtils.convertToDisplayString(transactionDetails?.amount ?? 0, transactionDetails?.currency);
return Localize.translateLocal('iou.didSplitAmount', {formattedAmount, comment: transactionDetails?.comment ?? ''});
const amount = TransactionUtils.getAmount(linkedTransaction, !isEmptyObject(report) && isExpenseReport(report)) ?? 0;
const formattedAmount = CurrencyUtils.convertToDisplayString(amount, TransactionUtils.getCurrency(linkedTransaction)) ?? '';
return Localize.translateLocal('iou.didSplitAmount', {formattedAmount, comment: TransactionUtils.getDescription(linkedTransaction) ?? ''});
}
}

Expand All @@ -3035,9 +3036,9 @@ function getReportPreviewMessage(
return Localize.translateLocal('iou.receiptMissingDetails');
}

const transactionDetails = getTransactionDetails(linkedTransaction);
const formattedAmount = CurrencyUtils.convertToDisplayString(transactionDetails?.amount ?? 0, transactionDetails?.currency);
return Localize.translateLocal('iou.trackedAmount', {formattedAmount, comment: transactionDetails?.comment ?? ''});
const amount = TransactionUtils.getAmount(linkedTransaction, !isEmptyObject(report) && isExpenseReport(report)) ?? 0;
const formattedAmount = CurrencyUtils.convertToDisplayString(amount, TransactionUtils.getCurrency(linkedTransaction)) ?? '';
return Localize.translateLocal('iou.trackedAmount', {formattedAmount, comment: TransactionUtils.getDescription(linkedTransaction) ?? ''});
}
}

Expand Down Expand Up @@ -6403,8 +6404,8 @@ function getIOUReportActionDisplayMessage(reportAction: OnyxEntry<ReportAction>,
return Localize.translateLocal(translationKey, {amount: formattedAmount, payer: ''});
}

const transactionDetails = getTransactionDetails(!isEmptyObject(transaction) ? transaction : undefined);
const formattedAmount = CurrencyUtils.convertToDisplayString(transactionDetails?.amount ?? 0, transactionDetails?.currency);
const amount = TransactionUtils.getAmount(transaction, !isEmptyObject(iouReport) && isExpenseReport(iouReport)) ?? 0;
const formattedAmount = CurrencyUtils.convertToDisplayString(amount, TransactionUtils.getCurrency(transaction)) ?? '';
const isRequestSettled = isSettled(originalMessage?.IOUReportID);
const isApproved = isReportApproved(iouReport);
if (isRequestSettled) {
Expand All @@ -6426,7 +6427,7 @@ function getIOUReportActionDisplayMessage(reportAction: OnyxEntry<ReportAction>,
}
return Localize.translateLocal(translationKey, {
formattedAmount,
comment: transactionDetails?.comment ?? '',
comment: TransactionUtils.getDescription(transaction) ?? '',
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB, should this be called TransactionUtils.getComment?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess description works for now since we are calling it like this in App

});
}

Expand Down
Loading