[$500] when adding amount details to a failed smartscan, default currency isn’t local

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.13-8
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @dylanexpensify
Slack conversation:

Action Performed:

1. Head to any chat
2. Create request money
3. Choose Scan with faulty picture (e.g. a book)
4. Wait till scan fails
5. Click on the Amount details required

Expected Result:

the amount to input would default to your local currency

Actual Result:

the amount to input defaults to USD$

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

ENAL3616.1.MP4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01afd06e1036c5deb0
• Upwork Job ID: 1736882015099211776
• Last Price Increase: 2023-12-25
• Automatic offers:
• wlegolas | Contributor | 28076000

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01afd06e1036c5deb0

[melvin-bot]

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane (External)

[wlegolas]

Proposal

Please re-state the problem that we are trying to solve in this issue.

when adding amount details to a failed smartscan, default currency isn’t local

What is the root cause of that problem?

The component CurrencySymbolButton receives in its property currencySymbol which currency symbol needs to show, this information comes from the EditRequestPage component. The EditRequestPage component uses the method ReportUtils.getTransactionDetails to retrieve the transaction details and one of the information is currency that is used to get the current symbol.

App/src/pages/EditRequestPage.js

Lines 78 to 85 in 19ca744

	const {
	amount: transactionAmount,
	currency: transactionCurrency,
	comment: transactionDescription,
	merchant: transactionMerchant,
	category: transactionCategory,
	tag: transactionTag,
	} = ReportUtils.getTransactionDetails(transaction);

To retrieve the currency, is used some methods from ReportUtils file: getTransactionDetails -> getCurrency, in the getCurrency is a logic to retrieve the currency from the transaction object. The value returned from the method getCurrency is extracted from the transaction property modifiedCurrency that has the default currency (USD$) when the scanning fails.

App/src/libs/TransactionUtils.ts

Lines 280 to 286 in 19ca744

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

What changes do you think we should make in order to solve the problem?

We should change the getCurrency to verify if the transaction has an error using the method hasMissingSmartscanFields, if so, we need to get the currency from the currency property in the transaction object.

For example:

function getCurrency(transaction: OnyxEntry<Transaction>): string {
    const currency = transaction?.modifiedCurrency ?? '';
    const hasFieldErrors = hasMissingSmartscanFields(transaction);

    if (hasFieldErrors || _.isEmpty(currency)) {
        return transaction?.currency ?? CONST.CURRENCY.USD;
    }

    return currency;
}

What alternative solutions did you explore? (Optional)

N/A

POC

poc-issue-33258.mov

[Tony-MK]

This just needs a simple backend fix because once the scan fails, it sets the transaction.modifiedCurrency from '' to USD by the server. Also, the transaction.currency remains unchanged.

Check whether the backend is setting the transaction.modifiedCurrency to USD.

@rushatgabhane, could you confirm this?

[Victor-Nyagudi]

Not exactly the same, but the currency is changed from the local one to USD here too, so I thought it good to mention the issue for reference sake.

• [HOLD for payment 2024-04-25] [LOW] [Splits] [$500] Split bill - Currency for split bill via Scan is USD instead of local currency #29709

[melvin-bot]

@rushatgabhane, @lschurr Whoops! This issue is 2 days overdue. Let's get this updated quick!

[lschurr]

Bump on this one @rushatgabhane

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@rushatgabhane, @lschurr Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[lschurr]

@rushatgabhane - any update here?

[lschurr]

Bumped in Slack - https://expensify.slack.com/archives/C01GTK53T8Q/p1703871901016549

[rushatgabhane]

@wlegolas 's proposal LGTM #33258 (comment)
🎀 👀 🎀

[melvin-bot]

Triggered auto assignment to @aldo-expensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[aldo-expensify]

This still stuck on me, I have done some progress in reproducing the original problem in dev. The Expensiwork/smartscan was a bit tricky to get to work.

I need to do more testing to decide where is the best place to fix this (backend/frontend).

[wlegolas]

Hi @aldo-expensify did you have time to execute your tests to check the better place to fix this issue?

If there is something that I can help, please let me know.

[aldo-expensify]

I found the code in the backend where we are setting the modifiedCurrency when smartscan fails and it is new code that was introduced for the new app. I don't think this behaviour existed for the Old app and it seems like a bug introduced by mistake. So I still think the right fix would be to stop setting the modifiedCurrency incorrectly instead of ignoring the bad currency in the frontend.

This issue is kind of low priority, so I have been getting pulled to other higher priority issues.

I think what we should do is:

• I finds the right fix in the backend, and we close your PR
• Pay you something (half?) because you still spent some time investigating, coding and testing your solution.

These don't have to necessarily happen in that order.

[wlegolas]

I found the code in the backend where we are setting the modifiedCurrency when smartscan fails and it is new code that was introduced for the new app. I don't think this behaviour existed for the Old app and it seems like a bug introduced by mistake. So I still think the right fix would be to stop setting the modifiedCurrency incorrectly instead of ignoring the bad currency in the frontend.

This issue is kind of low priority, so I have been getting pulled to other higher priority issues.

I think what we should do is:

• I finds the right fix in the backend, and we close your PR
• Pay you something (half?) because you still spent some time investigating, coding and testing your solution.

These don't have to necessarily happen in that order.

Hi @aldo-expensify thank you for sharing your results.

If the fix should be done by the backend, for me doesn't have a problem, I don't know how it's the process (just close the PR or pay something for my time), so I leave it open to you to understand the best way to close this issue.

[melvin-bot]

@wlegolas, @rushatgabhane, @lschurr, @aldo-expensify, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

[melvin-bot]

@wlegolas, @rushatgabhane, @lschurr, @aldo-expensify, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

[aldo-expensify]

Not resolved yet!

[melvin-bot]

@wlegolas, @rushatgabhane, @lschurr, @aldo-expensify, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

[melvin-bot]

@wlegolas, @rushatgabhane, @lschurr, @aldo-expensify, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

[aldo-expensify]

Reopening for handling some payment for @wlegolas for the work done in the PR: #33853 (even if it wasn't merged because we preferred a backend solution in the end)

[lschurr]

Payment summary:

We agreed to pay half of the original job price since the PR wasn't merged but the job was assigned and @wlegolas put work into it.

• Contributor: $250 @wlegolas (paid in Upwork)
• Contributor+: $250 @rushatgabhane (Please request in Newdot)

[rushatgabhane]

@lschurr me too please, i had reviewed the issue and PR

#33853 (comment)

[lschurr]

Ah, thanks @rushatgabhane. I'll add you to the payment summary

[lschurr]

All set.

[JmillsExpensify]

$250 approved for @rushatgabhane

[wlegolas]

Hi folks.

Thank you so much for giving me this payment ❤️