[$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
• [x] iOS: Native
• [ ] iOS: mWeb Safari
• [ ] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/38435837/12b42847-8b4e-4bef-bf42-19c9f8bbfd08

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[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[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[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.

https://github.com/Expensify/App/blob/19ca74491ec7dc60bf8e3103058add651964533c/src/pages/EditRequestPage.js#L78-L85

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.

https://github.com/Expensify/App/blob/19ca74491ec7dc60bf8e3103058add651964533c/src/libs/TransactionUtils.ts#L280-L286

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

https://github.com/Expensify/App/assets/698363/56fe4bf1-7c01-4819-b661-77c2432ac134

[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.

• https://github.com/Expensify/App/issues/29709

[melvin-bot[bot]]

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

[lschurr]

Bump on this one @rushatgabhane

[melvin-bot[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[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 https://github.com/Expensify/App/issues/33258#issuecomment-1861932910 🎀 👀 🎀

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@rushatgabhane @lschurr @aldo-expensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot[bot]]

@rushatgabhane @lschurr @aldo-expensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot[bot]]

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

[melvin-bot[bot]]

📣 @wlegolas 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[aldo-expensify]

hmm, @Tony-MK that is a good point, but I think that change would affect also the Old website leaving the currency empty, and that may break things there.

[Tony-MK]

Sorry, I didn't know that. Good job @wlegolas. You deserve it. Thanks @aldo-expensify 😄

[wlegolas]

Hi everyone, I created the PR with the changes https://github.com/Expensify/App/pull/33853

If you have any questions or suggestions, please let me know.

[melvin-bot[bot]]

This issue has not been updated in over 15 days. @wlegolas, @rushatgabhane, @lschurr, @aldo-expensify eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[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[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[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!