[$500] IOU - Total amount is not updated when change the different currency IOU in offline

[lanitochka17]

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.32-2
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: Applause - Internal Team
Slack conversation:

Issue found when executing PR #34011

Action Performed:

1. Go to staging.new.expensify.com.
2. Go offline
3. Click FAB > Request money > Manual.
4. Request money from a user that has no existing chat.
5. In 1:1 DM, click + > Request money.
6. Create another manual request in a different currency
7. Open the detail IOU page from second request
8. Change the currency into same as first IOU
9. Go to 1:1 conversation

Expected Result:

Total from two IOS request should be present, since both requests are in same currency

Actual Result:

Only amount of the first request is displayed in total

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

Bug6355235_1706222767182.Recording__1927.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0110932b60eddd10f4
• Upwork Job ID: 1750654949608632320
• Last Price Increase: 2024-01-25
• Automatic offers:
• cubuspl42 | Reviewer | 28131429
• dukenv0307 | Contributor | 28131430

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0110932b60eddd10f4

[melvin-bot]

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

[melvin-bot]

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

[dukenv0307]

Proposal

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

Only amount of the first request is displayed in total

What is the root cause of that problem?

In here, when updating money request, we only recalculate the total of the iouReport when the amount changes. This makes it not working for the case when user changes transaction currency from same as iouReport to different, or vice versa.

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

Update this block to account for the cases of changing currency as well.

There're 3 cases:

• Currency is changed from same as iouReport to different from iouReport: the transaction amount (retrieved from TransactionUtils.getAmount(updatedTransaction, ...)) should be subtracted from the iouReport total amount, because now the total amount won't include the total until the user goes online and the conversion rate is determined
• Currency is changed from different from iouReport to same as iouReport: the transaction amount should be added to the iouReport total amount, because previously the amount was not included in total and now it's included
• Currency is changed from different from iouReport to another currency different from iouReport: do nothing

When subtracted/added is mentioned above, it's understood that if the iouReport is stored as negative (in some cases), then subtracted/added will be added/subtracted.

When currency is mentioned above, it means modifiedCurrency field for updatedTransaction and transaction.modifiedCurrency || transaction.currency for transaction (or can just use TransactionUtils.getCurrency). It seems in here we're not using the correct currency field for updatedTransaction, we might want to fix that.

What alternative solutions did you explore? (Optional)

We can recalculate the iouReport total based on its money requests, when currency/amount is updated. That means we don't calculate the diff/changes and add to the iouReport total, but we'll calculate based on all the money requests from the ground up, this might help reduce chances of error.

Not related, but we can also reconsider the location of the Total will update when you're back online message, currently it seems to always show at the last money request in the iou report (even if that money request is not the one pending currency conversion), which is a bit odd to me.

[melvin-bot]

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

[jacobkim9881]

Proposal

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

When money requests are being made with different currencies offline, amount total of report thread is not calculated based on each currency.

What is the root cause of that problem?

The frontend app should wait for response with calculated amount total from server when currencies are different. If it is online then the server calculates amount total but if it is offline and then amount total is shown as un-calculated without any filter.

When offline the amount total is calculated but it is false calculation for different currencies to money request thread. Technically the calculation is not from the server but frontend.

The un-matched amount total go through not only report preview but also report header, LHN, report setting page, share code page and browser notification.

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

When it is offline and when the report thread with more than 2 reports has different currencies, total amount will be shown as 'TBD'. For this convertToDisplayString function will be used for the solution. The functions gives 'TBD' when amountInCents is 0 and when shouldFallbackToTbd is true.

If it has different currencies, as optimistic data total should be 0 for amountInCents should be 0 to show 'TBD' at convertToDisplayString. Because with server response, success data change total into calculated amount currently. To build this, total will be given 0 with the condition when deciding optimistic data.

To check offline, we should import default function from src/hooks/useNetwork.ts at ReportUtils.getReportName function and at src/components/ReportActionItem/ReportPreview.js.

On report preview page, getDisplayAmount is used to decide total. We use the same logic for the issue.

On such tags at LHN, money report header, ReportUtils.getReportName is used to decide total amount for showing. It has convertToDisplayString function and the same logic will be applied.

What alternative solutions did you explore? (Optional)

Fetch currency information from web and use in frontend.

[cubuspl42]

@jacobkim9881 So, effectively you're just suggesting to stop calculating any total sum while offline? This is not an acceptable behavior.

[cubuspl42]

I approve the proposal by @dukenv0307

C+ reviewed 🎀 👀 🎀

[melvin-bot]

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

[jacobkim9881]

Hi, @cubuspl42. Thanks for reviewing. I want to apologize for the short and unclear proposal. I would like to reply to your question shortly. For suggesting to stop calculating, it was only for limited money requests. Based on the proposal, only problematic report threads will be stopped to calculate.

[jacobkim9881]

With my proposal, the PR will solve un-calculated total at report's name on LHN too. I hope your reconsideration.

[melvin-bot]

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

Offer link
Upwork job

[melvin-bot]

📣 @dukenv0307 🎉 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 📖

[jacobkim9881]

I want to say my proposal doesn't stop to calculate the amount. My proposal makes un-calculated amount total shown as TBD. The amount total can't be calculated same from the server's calculation response. If the amount should be calculated then it should be cared an internal issue as a new feature.

[cubuspl42]

@jacobkim9881 Thanks for your proposal and the explanations. I understand what you're suggesting. Out of these two proposals, I decided that the other is a better way forward.

[jacobkim9881]

@cubuspl42 I see and respect your decision. I hope the issue will be solved well.

[bfitzexpensify]

How is this one going @dukenv0307?

[bfitzexpensify]

Bump @dukenv0307, can you please provide an update?

[dukenv0307]

I'm working on the PR, thanks for your bump.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[cubuspl42]

This is officially a disaster. I believe we underestimated the impact of the change.

[tylerkaraszewski]

I think all the deploy blockers related to this have been resolved now. Maybe we can close this out?

[dukenv0307]

@bfitzexpensify I think we're ready for payment now.

[cubuspl42]

I think we're ready for payment now.

For whatever is left of it, anyway 😅

[bfitzexpensify]

OK, there was a lot to try and follow here. I'm going to bring the price down to $125 - it should be halved again (3 regressions in total), but $62.50 feels too low for the work put in.

Payments complete.