[$1000] IOU - Total amount is changed if user is canceling the amount in USD (first request)

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Issue was found when executing PR #12488

Action Performed:

1. Launch the app
2. Log in with any account
3. Send a money request in USD while being online, e.g. $100.
4. Send a second money request in a different currency, e.g. 100 AED. The total should now be converted to USD.
5. Go offline, cancel the request in USD (first request online)

Expected Result:

Total amount is not changed if user is canceling the amount in USD (first request)

Actual Result:

Total amount is changed if user is canceling the amount in USD (first request)

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.2.28.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers): any

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Screen_Recording_20221115-220706_New.Expensify.1.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017f20fafc88928bb6
• Upwork Job ID: 1593711708909940736

[melvin-bot]

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

[melvin-bot]

Current assignee @davidcardoza is eligible for the External assigner, not assigning anyone new.

[davidcardoza]

I was able to reproduce, sending out to external.

Job posted - https://www.upwork.com/jobs/~0113dfaeac3d495f2e

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~017f20fafc88928bb6

[melvin-bot]

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

[melvin-bot]

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

[mmuflih55]

Proposal

the problem is caused by

if (currency !== iouReport.currency && isNetworkOffline) {

currency when we cancel is same the iouReport.currency, so it will not pass in this condition

Solution

adjust the logic by the type

+  if (type === 'cancel' ? (currency === iouReport.currency) : (currency !== iouReport.currency) && isNetworkOffline) {
-    if (currency !== iouReport.currency && isNetworkOffline) {
        return iouReport;
    }

Result

recored.mov

Note

I think it's not best proposal. maybe you can give more explanation about the logic, like why total amount should not changed only if currency is different, etc..

[s77rt]

I don't see what's wrong here. Looks logical to me.

1. You requested USD100 -- TOTAL: USD100
2. You requested AED100 -- TOTAL: USD127
3. You cancelled the first USD100 -- TOTAL: USD27

[mollfpr]

It’s a regression from #12488 and here's the linked issue #12433

[bernhardoj]

Yep, agree with @s77rt as the cancel request has the same currency (USD).

[mollfpr]

The change should be updated when going online.

[s77rt]

@mollfpr What would be the expected results then?

[mollfpr]

Same as in the description, the total amount should be updated when online.

Also please check the related issue #12433

[bernhardoj]

@mollfpr I believe that the total amount when canceling a money request with a different currency should be updated only when online because we need to consider the exchange rate that could change, which is being also mentioned in the 4th step of the related issue.

Canceling a money request with the same currency won't get affected by the exchange rate, and thus should not be prevented to be updated when offline, right?

[Emz27]

Proposal

• we can probably use the cachedTotal coming from the iouReport as opposed to the component cachedTotal

https://github.com/Expensify/App/blob/503aa3647480d3457c816f79c248bfa43fe3d8e7/src/libs/ReportUtils.js#L675L693

which looks like this cachedTotal: ($100)

I imagine this property is always the same as the backend values, it doesnt get updated if we do actions like request or cancel locally (not yet synced to backend) and this will be the first time it will be used in this repo because I searched it and I only seen it on create report.

using this approach would regress the requirement about updating the amount when same currency but I can also work on that

I'll send the video in a moment

[bernhardoj]

Btw, in case we still want to prevent it, here is my proposal

Proposal
From this PR #12488, it adds a check to prevent the updates while offline and if the currency is different. We could simply remove the currency check.

function updateIOUOwnerAndTotal(iouReport, actorEmail, amount, currency, type = CONST.IOU.REPORT_ACTION_TYPE.CREATE) {
-   if (currency !== iouReport.currency && isNetworkOffline) {
+   if (isNetworkOffline) {
        return iouReport;
    }

    const iouReportUpdate = {...iouReport};

    if (actorEmail === iouReport.ownerEmail) {
        iouReportUpdate.total += type === CONST.IOU.REPORT_ACTION_TYPE.CANCEL ? -amount : amount;
    } else {
        iouReportUpdate.total += type === CONST.IOU.REPORT_ACTION_TYPE.CANCEL ? amount : -amount;
    }

    if (iouReportUpdate.total < 0) {
        // The total sign has changed and hence we need to flip the manager and owner of the report.
        iouReportUpdate.ownerEmail = iouReport.managerEmail;
        iouReportUpdate.managerEmail = iouReport.ownerEmail;
        iouReportUpdate.total = -iouReportUpdate.total;
    }

    return iouReportUpdate;
}

[mollfpr]

Thanks guys, I’ll review the proposal tomorrow!

[Emz27]

Hi this is the followup video for #12779 (comment)

Screen.Recording.2022-11-20.at.12.15.41.AM.mov

as you can see in the video, the report.cachedTotal is the values last fetched from the onyx backend

and the component cachedTotal is coming from this

App/src/components/ReportActionItem/IOUPreview.js

Lines 126 to 130 in 503aa36

	const cachedTotal = props.iouReport.total && props.iouReport.currency
	? props.numberFormat(
	Math.abs(props.iouReport.total) / 100,
	{style: 'currency', currency: props.iouReport.currency},
	) : '';

just shaping the values for display

In theory, we can use the report.cachedTotal if we need to wait for true result from the backend because the backend is the one who do the conversions for currencies

[fedirjh]

Same as in the description, the total amount should be updated when online.
Also please check the related issue #12433

@mollfpr i think this is the logical behavior here , the usd is the default (main) currency for this account , so there is no exchange rate that will be applied when user get back online , the total amount can be updated when offline if the currency is the same as the main currency of the account .

[mollfpr]

Sorry, I just have enough time to understand the issue and scroll back to the related discussion. I agree with y'all that the current logic makes sense. In the meantime, I'll hold off reviewing the proposals until we have a clear explanation of the expected result.

@kbecciv Could you clarify if the IOU amount should not be updated before going online although the currency is the same as the IOU report currency?

[youssef-lr]

This is the expected behavior when the the request cancelled has the same currency as the IOU report, as per this issue: #12433. Thank you all for your proposals! Closing this one out.