[HOLD for payment 2024-12-19] [$250] Split distance-Distance amount reverts to original amount when saving distance without editing

[IuliiaHerets]

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: 9.0.59-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Open DM.
3. Split a distance expense with the user.
4. Go to transaction thread.
5. Note that distance is split in half.
6. Click Distance field.
7. Click Save without modifying anything.
8. Note that Distance field reverts to the original distance, but the amount does not change.
9. Click on the receipt.
10. Go back to main chat.
11. Note that the distance in the receipt and expense preview remains split in half and does not revert to the original amount.

Expected Result:

In Step 7, if saving distance without changing the waypoints is meant to revert the split distance to the original distance, it should also reflect the changes in expense amount, receipt and expense preview.

Actual Result:

In Step 8, after saving distance without changing the waypoints, Distance field reverts to the original distance, but the amount does not change.
The receipt and expense preview still show the old amount and old distance.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6658639_1731058374015.20241108_172430.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021856609239459283873
• Upwork Job ID: 1856609239459283873
• Last Price Increase: 2024-11-13
• Automatic offers:
• FitseTLT | Contributor | 105164804

Issue Owner

Current Issue Owner: @Christinadobrzyn

[melvin-bot]

Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[FitseTLT]

Edited by proposal-police: This proposal was edited at 2024-11-08 13:31:14 UTC.

Proposal

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

Split distance-Distance amount reverts to original amount when saving distance without editing

What is the root cause of that problem?

We are only submitting waypoints to the backend when the old address and the new one are different

App/src/pages/iou/request/step/IOURequestStepDistance.tsx

Line 457 in 8a83e2b

if (isEqual(oldAddresses, addresses)) {

but in this case it is the same and we should revert the transaction back to it's back up value

App/src/pages/iou/request/step/IOURequestStepDistance.tsx

Lines 226 to 230 in 8a83e2b

	if (transactionWasSaved.current) {
	TransactionEdit.removeBackupTransaction(transaction?.transactionID ?? '-1');
	return;
	}
	TransactionEdit.restoreOriginalTransactionFromBackup(transaction?.transactionID ?? '-1', IOUUtils.shouldUseTransactionDraft(action));

but that only happens transactionWasSaved is false but we already set it to true here

App/src/pages/iou/request/step/IOURequestStepDistance.tsx

Line 448 in 8a83e2b

transactionWasSaved.current = true;

so now in money request view we will see the routes.route0.distance that was populated via useFetchRoute when we opened the distance page

App/src/pages/iou/request/step/IOURequestStepDistance.tsx

Lines 88 to 93 in 8a83e2b

	useFetchRoute(transactionBackup, backupWaypoints, action, CONST.TRANSACTION.STATE.BACKUP);
	const {shouldFetchRoute, validatedWaypoints} = useFetchRoute(
	transaction,
	waypoints,
	action,
	IOUUtils.shouldUseTransactionDraft(action) ? CONST.TRANSACTION.STATE.DRAFT : CONST.TRANSACTION.STATE.CURRENT,

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

For editing case, we only don't want to restore the transaction if the old and new address are different. So we should set transactionWasSaved to true above here

App/src/pages/iou/request/step/IOURequestStepDistance.tsx

Line 461 in 8a83e2b

IOU.updateMoneyRequestDistance({

change

App/src/pages/iou/request/step/IOURequestStepDistance.tsx

Lines 447 to 448 in 8a83e2b

	if (!isCreatingNewRequest) {
	transactionWasSaved.current = true;

if (!isCreatingNewRequest && !isEditing) {
            transactionWasSaved.current = true;

so the transaction will be restored from the backup on clicking submit without changing the waypoints

Side Note:
Though not directly linked to this issue I have observed that the distance we are setting optimstically is just the total distance and it only gets it's correct split value from the BE so we can fix that by setting the split distance in customUnit of the transaction same as we are already doing for the splitShare.amount

App/src/libs/actions/IOU.ts

Line 4228 in 8a83e2b

const splitAmount = splitShares?.[participant.accountID ?? -1]?.amount ?? IOUUtils.calculateAmount(participants.length, amount, currency, false);

What alternative solutions did you explore? (Optional)

We can also clear routes of the transaction here

App/src/pages/iou/request/step/IOURequestStepDistance.tsx

Line 227 in 8a83e2b

TransactionEdit.removeBackupTransaction(transaction?.transactionID ?? '-1');

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[melvin-bot]

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

[melvin-bot]

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

[nkdengineer]

Proposal

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

In Step 8, after saving distance without changing the waypoints, Distance field reverts to the original distance, but the amount does not change.
The receipt and expense preview still show the old amount and old distance.

What is the root cause of that problem?

When we create a split distance, the sub-transaction has quantity split from the total quantity

If we open the distance edit page, the quantity is updated to the current distance of waypoints. Then when we click on the save button without modifying anything, we do nothing here, but we still update transactionWasSaved.current to true.

Then the transaction is not reverted to the previous

App/src/pages/iou/request/step/IOURequestStepDistance.tsx

Lines 448 to 450 in c24d217

	if (!isCreatingNewRequest) {
	transactionWasSaved.current = true;
	}

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

When we edit the waypoint we should only update transactionWasSaved.current to true if we call updateMoneyRequestDistance. We can move these lines to here

And add transactionWasSaved.current = true here

We shouldn't update like the first proposal because it causes the transaction to be reverted here after we change the waypoint

App/src/pages/iou/request/step/IOURequestStepDistance.tsx

Line 230 in c24d217

TransactionEdit.restoreOriginalTransactionFromBackup(transaction?.transactionID ?? '-1', IOUUtils.shouldUseTransactionDraft(action));

What alternative solutions did you explore? (Optional)

NA

[Christinadobrzyn]

hey @allroundexperts this one is kinda weird (at least I think so) let me know if you need any questions answered from the team about expectations here. I think it's pretty clear but I had a hard time narrowing down the issue.

[melvin-bot]

@allroundexperts, @Christinadobrzyn Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

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

[FitseTLT]

@allroundexperts @nkdengineer 's proposal is exactly as mine. I think both of you misunderstood my proposal.
This part of his suggestion

When we edit the waypoint we should only update transactionWasSaved.current to true if we call updateMoneyRequestDistance. We can move these lines to here

is the same as what I said to exclude the case for isEditing

if (!isCreatingNewRequest && !isEditing) {
            transactionWasSaved.current = true;

And this part of his solution

And add transactionWasSaved.current = true here

I have already stated:
For editing case, we only don't want to restore the transaction if the old and new address are different. So we should set transactionWasSaved to true above here

App/src/pages/iou/request/step/IOURequestStepDistance.tsx

Line 461 in 8a83e2b

IOU.updateMoneyRequestDistance({

@nkdengineer has mislead you with his comments @allroundexperts Please re-take a look at my proposal 👍

cc @blimpich

[nkdengineer]

Just another note: I'm the first one to explain why this only happens for distance requests for split distances.

[FitseTLT]

Just another note: I'm the first one to explain why this only happens for distance requests for split distances.

The root cause of the issue has no relation with whether it is a split distance request or not. The result of the problem only gets visible for split distance request because we populate the distance with the splitted amount for split request and the fact that we are not reverting the transaction will be visible in this case because the distance will be set to the unsplitted/full amount. And the problem should be solved for all cases and that's why it is not important and I didn't mention.
@allroundexperts bump on #52241 (comment) You have selected the second proposal which is exactly the same as mine which is the first one please re-consider your review and let's proceed into creating the PR. Thx

[allroundexperts]

I reviewed this again. Specifically, I tried to test if the transaction would be reverted after we change the waypoint. Although it seemed like that would be the case initially, but upon inserting the proposal code in the app, I wasn't able to verify that.

@nkdengineer can you elaborate the following more?

We shouldn't update like the first proposal because it causes the transaction to be reverted here after we change the waypoint

[nkdengineer]

We shouldn't update like the first proposal because it causes the transaction to be reverted here after we change the waypoint

@allroundexperts this is my mistake, I missed the first step when reading the proposal.

The root cause of the issue has no relation with whether it is a split distance request or not

In general theory, if we do not change the address, nothing will happen even if we don't restore the transaction from the backup. So if we don't explain what happened to the split distance, it won't be clear why this bug happens.

[melvin-bot]

@blimpich, @allroundexperts, @Christinadobrzyn Whoops! This issue is 2 days overdue. Let's get this updated quick!

[FitseTLT]

@allroundexperts Please let's proceed with this. Thx

[melvin-bot]

@blimpich, @allroundexperts, @Christinadobrzyn 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[Christinadobrzyn]

Hi @allroundexperts can you provide an update on this? I'm catching up after some ooo and not sure where we are. TY!

[allroundexperts]

@nkdengineer Thanks for being honest here. Given that the transaction is not reverted, I'd agree with @FitseTLT that @nkdengineer's proposal is almost the same as @FitseTLT. As such, Let's go with @FitseTLT.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Current assignee @blimpich is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot]

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

[Christinadobrzyn]

@FitseTLT just checking on the PR. Thanks!

[Christinadobrzyn]

monitoring PR #53628

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.74-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix - Split distance-Distance amount reverts to original amount when saving distance without editing #53628

If no regressions arise, payment will be issued on 2024-12-19. 🎊

For reference, here are some details about the assignees on this issue:

• @allroundexperts requires payment through NewDot Manual Requests
• @FitseTLT requires payment automatic offer (Contributor)

[melvin-bot]

@allroundexperts @Christinadobrzyn @allroundexperts The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]