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

[CP Staging] Fix: Expense - App crashes when changing date, description, merchant of the request #36919

Merged
merged 2 commits into from
Feb 20, 2024
Merged

[CP Staging] Fix: Expense - App crashes when changing date, description, merchant of the request #36919

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4228d15
used currency as a fallback value
Pujan92 Feb 20, 2024
56c7548
used transactionDetails for currency
Pujan92 Feb 20, 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
3 changes: 2 additions & 1 deletion src/libs/actions/IOU.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1140,7 +1140,8 @@ function getUpdateMoneyRequestParams(
? IOUUtils.updateIOUOwnerAndTotal(iouReport, updatedReportAction.actorAccountID ?? -1, diff, TransactionUtils.getCurrency(transaction), false, true)
: {};
}
updatedMoneyRequestReport.cachedTotal = CurrencyUtils.convertToDisplayString(updatedMoneyRequestReport.total, updatedTransaction?.modifiedCurrency);
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
updatedMoneyRequestReport.cachedTotal = CurrencyUtils.convertToDisplayString(updatedMoneyRequestReport.total, updatedTransaction?.modifiedCurrency || updatedTransaction?.currency);
Copy link
Contributor

@cead22 cead22 Feb 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Do we really need the fallback? It looks like updatedTransaction.modifiedCurrency gets set to the same value as updatedTransaction.currency in getUpdatedTransaction, so wouldn't it be the same if this was

updatedMoneyRequestReport.cachedTotal = CurrencyUtils.convertToDisplayString(updatedMoneyRequestReport.total, updatedTransaction?.currency);

This comment was marked as off-topic.

Sign in to view

Copy link
Contributor Author

@Pujan92 Pujan92 Feb 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In getUpdatedTransaction, modifiedCurrency gets set from the transactionChanges whereas the original transaction currency remains the old value only.

We can use transactionDetails.currency also as it is doing the same thing.

App/src/libs/actions/IOU.ts

Lines 1051 to 1052 in 12fc2cb

let updatedTransaction = transaction ? TransactionUtils.getUpdatedTransaction(transaction, transactionChanges, isFromExpenseReport) : null;
const transactionDetails = ReportUtils.getTransactionDetails(updatedTransaction);

App/src/libs/TransactionUtils.ts

Lines 291 to 297 in 299576f

function getCurrency(transaction: OnyxEntry<Transaction>): string {
const currency = transaction?.modifiedCurrency ?? '';
if (currency) {
return currency;
}
return transaction?.currency ?? CONST.CURRENCY.USD;
}


optimisticData.push({
onyxMethod: Onyx.METHOD.MERGE,
Expand Down
Loading