[HOLD for payment 2024-11-11] [$250] Track expense - Edited amount is not reflected correctly in amount editor on confirmation page

[IuliiaHerets]

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

---

Version Number: 9.0.50-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): applausetester+kh081006@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to self DM.
3. Track a $10 expense.
4. Go to transaction thread.
5. Change the amount to $20 (important).
6. Go back to main chat and click Submit it to someone from the whisper.
7. Select a user.
8. Click Amount on the confirmation page.
9. Change the amount to $30 and save it.
10. Click Amount.

Expected Result:

The amount will remain the same, which is $30.

Actual Result:

The amount is $20, while it shows $30 on the confirmation page.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6637715_1729172605869.20241017_213854.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021848821552702222159
• Upwork Job ID: 1848821552702222159
• Last Price Increase: 2024-10-22
• Automatic offers:
• FitseTLT | Contributor | 104618078

Issue Owner

Current Issue Owner: @puneetlath

[melvin-bot]

Triggered auto assignment to @puneetlath (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.

[IuliiaHerets]

@puneetlath FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[FitseTLT]

Edited by proposal-police: This proposal was edited at 2024-10-17 16:32:06 UTC.

Proposal

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

Track expense - Edited amount is not reflected correctly in amount editor on confirmation page

What is the root cause of that problem?

We are taking the modifiedAmount from the trackExpense when we create draft transaction on submit it to someone

App/src/libs/ReportUtils.ts

Lines 8037 to 8046 in 4a25c36

	IOU.createDraftTransaction({
	...transaction,
	actionableWhisperReportActionID: reportActionID,
	linkedTrackedExpenseReportAction,
	linkedTrackedExpenseReportID: reportID,
	created,
	modifiedCreated: undefined,
	amount,
	currency,
	comment,

So in IOUStepAmountPage we only update only amount (not modifiedAmount) of the transaction if the action is not editing

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

Line 170 in 4a25c36

IOU.setMoneyRequestAmount(transactionID, amountInSmallestCurrencyUnits, currency || CONST.CURRENCY.USD, shouldKeepUserInput);

So in confirmation page we will see this new amount we updated but when reopen the amount page it will show us modifiedAmount which has not been updated

App/src/libs/TransactionUtils/index.ts

Lines 337 to 340 in 4a25c36

	if (!isFromExpenseReport || isFromTrackedExpense) {
	const amount = transaction?.modifiedAmount ?? 0;
	if (amount) {
	return Math.abs(amount);

it prioritize modifiedAmount if over amount if it exists

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

We should mimic the setup we have for create expense (because submit it to someone is basically creating an expense from an existing some sort of expense template) so when we create a draft transaction increateDraftTransactionAndNavigateToParticipantSelector whether it is for submit, categorize or share we should reset modifiedAmount to undefined so that IOURequestStepAmount will have one source of truth amount same as what we have for creation of a new request.
We need to reset modifiedAmount on the draft transaction here

        currency,
        modifiedAmount: undefined,

What alternative solutions did you explore? (Optional)

[daledah]

Edited by proposal-police: This proposal was edited at 2024-10-17 15:50:45 UTC.

Proposal

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

The amount is $20, while it shows $30 on the confirmation page.

What is the root cause of that problem?

If the original amount is 100
When editing the track expense to 200, we will update modifiedAmount: -200, amount: -100

We're showing the amount on confirmation page

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

Line 640 in 47b6811

iouAmount={Math.abs(transaction?.amount ?? 0)}

But on amount page, we use modifiedAmount

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

Line 87 in 47b6811

const {amount: transactionAmount} = ReportUtils.getTransactionDetails(currentTransaction) ?? {amount: 0};

App/src/libs/TransactionUtils/index.ts

Lines 358 to 360 in 47b6811

	const amount = transaction?.modifiedAmount ?? 0;
	if (amount) {
	return Math.abs(amount);

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

We should update modifiedAmount if it's the draft transaction in

App/src/libs/actions/IOU.ts

Lines 404 to 406 in 47b6811

	function setMoneyRequestAmount(transactionID: string, amount: number, currency: string, shouldShowOriginalAmount = false) {
	Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {amount, currency, shouldShowOriginalAmount});
	}

function setMoneyRequestAmount(transactionID: string, amount: number, currency: string, shouldShowOriginalAmount = false) {
    Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {amount, currency, shouldShowOriginalAmount, modifiedAmount: amount});
}

What alternative solutions did you explore? (Optional)

NA

[FitseTLT]

Only added detail on the RCA

[melvin-bot]

@puneetlath Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

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

[melvin-bot]

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

[ntdiary]

Under review.

[ntdiary]

I personally lean towards @FitseTLT's proposal, we don't need modifiedXXX fields in the draft. Also, there's a similar issue with the modifiedCurrency field as well. :)

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Current assignee @puneetlath 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 📖

[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.56-9 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix - Track expense - Edited amount is not reflected correctly in amount editor on confirmation page #51579

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

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

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

[melvin-bot]

@ntdiary @puneetlath 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]

[puneetlath]

@FitseTLT has been paid.

@ntdiary waiting for the checklist for you.

[ntdiary]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

  Link to comment: https://github.com/Expensify/App/pull/38709/files#r1837453907

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

  Link to discussion: N/A

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again. N/A
  This is an edge case that was overlooked during the implementation, we already have a regression test specifically for modifiedCreated, I don't think it's necessary to create a new regression test. Please feel free to let me know if you have any other thoughts. :) cc @puneetlath

Regression Test Proposal Template

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

Precondition:

Test:

Do we agree 👍 or 👎

[puneetlath]

Payment summary:

• C - $250 - @FitseTLT - paid via Upwork
• C+ - $250 - @ntdiary - to be paid via NewDot

Thanks everyone!