[HOLD for payment 2024-01-17] [HOLD for payment 2024-01-11] [$500] Android- IOU-When user changes Stop address, the Request money amount is not updated #26946

izarutskaya · 2023-09-07T11:34:01Z

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

Action Performed:

Launch app
Tap profile icon
Tap Workspaces
Tap any Workspace
Tap Reimbursements
Tap Track distance
Enter Rate and Unit
Tap Save
Navigate back to LHN
Tap Fab
Tap request money
Tap distance
Tap Start and select an address
Tap Finish and select an address
Tap Add stop
Tap Finish and select an address
Tap next
Select a workspace
Note the distance details and request money amount is correct (15.39*2=₹30.78) i. e shown as ₹30.77
Tap back to distance money request page
Tap Stop
Replace the entered address with a new one
Tap next
Select a Workspace
Note the distance field and request money amount (9.68*2=₹19.36)

Expected Result:

When user changes Stop address, the Request money amount should be updated based on new distance.

Actual Result:

When user changes Stop address, the Request money amount is not updated. The distance amount(15.39*2=₹30.78) before changing address is shown even after changing the address.

For new distance when address changed (9.68*2=₹19.36) , but still old amount ₹30.77 is displayed.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Version Number: 1.3.65-0

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

Notes/Photos/Videos: Any additional supporting documentation

Bug6191115_km.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause-Internal Team / @ashimsharma10

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1693984012665899

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01f82f14ed7e096782
Upwork Job ID: 1700194911206653952
Last Price Increase: 2023-09-15
Automatic offers:

cubuspl42 | Reviewer | 28066262
DylanDylann | Contributor | 28066263

Issue Owner

Current Issue Owner: @CortneyOfstad

melvin-bot · 2023-09-07T11:34:06Z

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

melvin-bot · 2023-09-07T11:34:08Z

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

dukenv0307 · 2023-09-07T11:40:44Z

Proposal

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

distance - Amount isn't recalculated on changing waypoints

What is the root cause of that problem?

We only calculate when the amount is 0 and the request is distance. So when we edit the waypoint again, the amount is not recalculated

App/src/components/MoneyRequestConfirmationList.js

Line 173 in 1eafc7d

    
           const shouldCalculateDistanceAmount = props.isDistanceRequest && props.iouAmount === 0;

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

We should always calculate the amount when the distance is changed by changing the check here to

const shouldCalculateDistanceAmount = props.isDistanceRequest;

App/src/components/MoneyRequestConfirmationList.js

Line 173 in 1eafc7d

    
           const shouldCalculateDistanceAmount = props.isDistanceRequest && props.iouAmount === 0;

What alternative solutions did you explore? (Optional)

Whenever we edit the waypoint we will reset the amount to 0 to make it re-calculate in MoneyRequestConfirmationList

melvin-bot · 2023-09-08T17:10:31Z

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

melvin-bot · 2023-09-08T17:10:35Z

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

melvin-bot · 2023-09-08T17:10:41Z

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

ashimsharma10 · 2023-09-08T17:22:17Z

Have posted this before,
https://expensify.slack.com/archives/C049HHMV9SM/p1693984012665899

jeet-dhandha · 2023-09-08T17:45:36Z

Proposal

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

Amount is not recalculated when the distance is changed.

What is the root cause of that problem?

The shouldCalculateDistanceAmount flag is not set to true when the distance is changed. due to the props.iouAmount === 0 condition.

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

We can simply remove the shouldCalculateDistanceAmount flag and replace it with props.isDistanceRequest.
Keeping the shouldCalculateDistanceAmount flag doesn't make sense anymore since we are always calculating the amount when its a distance request.
This will solve two problems:
- The amount will be recalculated when the distance is changed.
- The amount will be recalculated when the rate is changed.

What alternative solutions did you explore? (Optional)

N/A

ahmedGaber93 · 2023-09-11T01:59:33Z

Proposal

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

distance - Amount isn't recalulated on changing waypoints

What is the root cause of that problem?

In the code below, the shouldCalculateDistanceAmount is true only when props.iouAmount === 0
this is good when we calculate the amount for the first time in const formattedAmount =
But when we re-calculate the amount in the useEffect we stop re-calculate by it, because shouldCalculateDistanceAmount is true only on the first time.

App/src/components/MoneyRequestConfirmationList.js

Lines 173 to 188 in b11bddc

    
           const shouldCalculateDistanceAmount = props.isDistanceRequest && props.iouAmount === 0; 
        
           const shouldCategoryEditable = !_.isEmpty(props.policyCategories) && !props.isDistanceRequest; 
        
           const formattedAmount = CurrencyUtils.convertToDisplayString( 
        
               shouldCalculateDistanceAmount ? DistanceRequestUtils.getDistanceRequestAmount(distance, unit, rate) : props.iouAmount, 
        
               props.isDistanceRequest ? currency : props.iouCurrencyCode, 
        
           ); 
        
           useEffect(() => { 
        
               if (!shouldCalculateDistanceAmount) { 
        
                   return; 
        
               } 
        
               const amount = DistanceRequestUtils.getDistanceRequestAmount(distance, unit, rate); 
        
               IOU.setMoneyRequestAmount(amount); 
        
           }, [shouldCalculateDistanceAmount, distance, rate, unit]);

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

keep props.iouAmount === 0 in shouldCalculateDistanceAmount condition, I think it intended because it is good to only calculate amount once here, then use props.iouAmount which calculated by the useEffect (will fix in step 2).

App/src/components/MoneyRequestConfirmationList.js

Lines 176 to 179 in b11bddc

    
           const formattedAmount = CurrencyUtils.convertToDisplayString( 
        
               shouldCalculateDistanceAmount ? DistanceRequestUtils.getDistanceRequestAmount(distance, unit, rate) : props.iouAmount, 
        
               props.isDistanceRequest ? currency : props.iouCurrencyCode, 
        
           );

replace the condition and deps in the useEffect to re-calculate the amount when any of its deps change, and the updated amount will pass to formattedAmount in props.iouAmount

useEffect(() => {
    // if (!shouldCalculateDistanceAmount) {
    if (!props.isDistanceRequest) {
         return;
    }

    const amount = DistanceRequestUtils.getDistanceRequestAmount(distance, unit, rate);
    IOU.setMoneyRequestAmount(amount);
}, [props.isDistanceRequest, distance, rate, unit]);

What alternative solutions did you explore? (Optional)

N/A

melvin-bot · 2023-09-11T07:21:21Z

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

melvin-bot · 2023-09-11T08:29:53Z

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

cubuspl42 · 2023-09-11T09:14:32Z

@dukenv0307 @jeet-dhandha @ahmedGaber93

Did you explore an alternative involving using useMemo (to calculate the amount) in tandem with useEffect (to call setMoneyRequestAmount), re-solving this problem cleanly? Is there some catch with that?

dukenv0307 · 2023-09-11T09:25:43Z

@cubuspl42 I think we should use a useEffect here because we only need to update amount if this is a distance request.

cubuspl42 · 2023-09-11T09:50:31Z

@ahmedGaber93

keep props.iouAmount === 0 in shouldCalculateDistanceAmount condition, I think it intended

This is a good point, I asked here

dukenv0307 · 2023-09-11T09:55:31Z

@cubuspl42 If we keep props.iouAmount === 0 in shouldCalculateDistanceAmount, the formattedAmount that is used to display in the UI will not be updated when the distance is changed.

App/src/components/MoneyRequestConfirmationList.js

Lines 176 to 178 in 1eafc7d

    
           const formattedAmount = CurrencyUtils.convertToDisplayString( 
        
               shouldCalculateDistanceAmount ? DistanceRequestUtils.getDistanceRequestAmount(distance, unit, rate) : props.iouAmount, 
        
               props.isDistanceRequest ? currency : props.iouCurrencyCode,

ahmedGaber93 · 2023-09-11T14:43:30Z

If we keep props.iouAmount === 0 in shouldCalculateDistanceAmount, the formattedAmount that is used to display in the UI will not be updated when the distance is changed.

No, it will work fine after update useEffect to

useEffect(() => {
    // if (!shouldCalculateDistanceAmount) {
    if (!props.isDistanceRequest) {
         return;
    }

    const amount = DistanceRequestUtils.getDistanceRequestAmount(distance, unit, rate);
    IOU.setMoneyRequestAmount(amount);
}, [props.isDistanceRequest, distance, rate, unit]);

The useEffect will call IOU.setMoneyRequestAmount(amount) which change the props.iouAmount that will update the formattedAmount without recalculating it because shouldCalculateDistanceAmount now is false and will use props.iouAmount that updated by use effect

App/src/components/MoneyRequestConfirmationList.js

Line 177 in 1eafc7d

    
           shouldCalculateDistanceAmount ? DistanceRequestUtils.getDistanceRequestAmount(distance, unit, rate) : props.iouAmount,

This is a good point, I asked here

I remember I read before the amount can be editable manually by user in MoneyRequestConfirmationList.js, but we temporarily disable this feature.
So I think shouldCalculateDistanceAmount with props.iouAmount === 0 maybe added for that, to prevent reset the amount by recalculating it after manually edit it and use props.iouAmount.

Let us wait the answer here.

cubuspl42 · 2023-09-12T11:58:04Z

Hey everyone, we've got a response from the original author of the PR. It seems that @ahmedGaber93 had good intuition here.

I marked with 👎 proposals which seem not to match the expectations here.

ahmedGaber93 · 2023-09-13T18:32:34Z

@cubuspl42 what do you think of using Onyx store to save this value? are this option available?

// IOU.js > add new function to set the value.
function setIsDistanceAmountOverridden(isDistanceAmountOverridden) {
    Onyx.merge(ONYXKEYS.IOU, {isDistanceAmountOverridden});
}

// IOU.js > edit this function and add isDistanceAmountOverridden rest value.
function resetMoneyRequestInfo(id = '') {
    const created = currentDate || moment().format('YYYY-MM-DD');
    Onyx.merge(ONYXKEYS.IOU, {
        ...
        isDistanceAmountOverridden: false,
    });
}


// in NewRequestAmountPage.js > navigateToNextPage
if(isDistanceRequestTab) {
    IOU.setIsDistanceAmountOverridden(true);
}

// in MoneyRequestConfirmPage.js
<MoneyRequestConfirmationList
    ...
    isDistanceAmountOverridden={props.iou.isDistanceAmountOverridden}


// in MoneyRequestConfirmationList.js > shouldCalculateDistanceAmount
const shouldCalculateDistanceAmount = props.isDistanceRequest && !props.isDistanceAmountOverridden;

result

Screen.Recording.2023-09-13.at.8.29.47.PM.mov

melvin-bot · 2024-01-10T06:00:12Z

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

melvin-bot · 2024-01-10T06:00:15Z

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

Fix/26946: Reset amount when edit waypoint #33959

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

@cubuspl42 paid $250 as C+ (-50% due to regression) in Upwork
@DylanDylann paid $250 as Contributor (-50% due to regression) in Upwork

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

@cubuspl42 requires payment automatic offer (Reviewer)
@DylanDylann requires payment automatic offer (Contributor)

melvin-bot · 2024-01-10T06:00:16Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@cubuspl42] The PR that introduced the bug has been identified. Link to the PR:
[@cubuspl42] 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:
[@cubuspl42] A discussion in #expensify-bugs 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:
[@cubuspl42] Determine if we should create a regression test for this bug.
[@cubuspl42] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@CortneyOfstad] – https://github.com/Expensify/Expensify/issues/363208

CortneyOfstad · 2024-01-15T15:42:08Z

Payment is not set until the 17th 👍

CortneyOfstad · 2024-01-17T14:21:51Z

Payments have been completed!

Payment Summary

@cubuspl42 paid $250 as C+ (-50% due to regression) in Upwork
@DylanDylann paid $250 as Contributor (-50% due to regression) in Upwork

CortneyOfstad · 2024-01-17T14:22:15Z

@cubuspl42 any update on the checklist above? Thanks!

ashimsharma10 · 2024-01-17T14:55:05Z

@CortneyOfstad Waiting for payment.
I'm the issue reporter.
Thanks.

CortneyOfstad · 2024-01-17T17:28:31Z

Thanks @ashimsharma10 — sorry about that!

I had to create a new job post since the old one expired. I've sent you an offer here — https://www.upwork.com/jobs/~01d9cdd3e5a961ef64.

Please let me know once you accept and I can get that paid ASAP. Thanks!

cubuspl42 · 2024-01-18T09:34:43Z

The PR that introduced the bug has been identified. Link to the PR:
- Modify MoneyRequestConfirmPage for Distance Request #25707
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/25707/files#r1322653311
A discussion in #expensify-bugs 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:
- No need for additional discussion
Determine if we should create a regression test for this bug.
- Up to the QA team
If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
- Create new distance request
- Fill start point and end point
- Go to confirmation page and observe the amount field
- Click back to the distance page and edit waypoint
- Go to confirmation page and verify that the amount is updated

CortneyOfstad · 2024-01-18T14:09:27Z

Thanks @cubuspl42!

@ashimsharma10 any update on the proposal so I can get the payment processed? Thanks!

ashimsharma10 · 2024-01-19T14:43:08Z

@CortneyOfstad I'm not able to apply.
my upwork is this : https://www.upwork.com/freelancers/~018a92cf13e1e88eed

CortneyOfstad · 2024-01-22T14:11:16Z

@ashimsharma10 that Upwork profile is where I sent the proposal. I just sent it again 👍

CortneyOfstad · 2024-01-24T14:07:51Z

@ashimsharma10 sorry for the delay here — finalized the payment this morning so you're all set!

Regression test has been created here, so this is all set to be closed!

izarutskaya added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 7, 2023

melvin-bot bot assigned CortneyOfstad Sep 7, 2023

CortneyOfstad added the External Added to denote the issue can be worked on by a contributor label Sep 8, 2023

melvin-bot bot changed the title ~~Android- IOU-When user changes Stop address, the Request money amount is not updated~~ [$500] Android- IOU-When user changes Stop address, the Request money amount is not updated Sep 8, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 8, 2023

melvin-bot bot assigned cubuspl42 Sep 8, 2023

melvin-bot bot added the Overdue label Sep 11, 2023

melvin-bot bot removed the Overdue label Sep 11, 2023

cubuspl42 mentioned this issue Sep 11, 2023

Modify MoneyRequestConfirmPage for Distance Request #25707

Merged

57 tasks

dukenv0307 mentioned this issue Sep 12, 2023

[$500] [Distance] - Incorrect Amount after updating distance rate #27051

Closed

6 tasks

This comment was marked as outdated.

Sign in to view

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Jan 8, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Jan 10, 2024

melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Jan 11, 2024

melvin-bot bot removed the Overdue label Jan 15, 2024

melvin-bot bot added the Overdue label Jan 22, 2024

Expensify deleted a comment from ashimsharma10 Jan 24, 2024

CortneyOfstad closed this as completed Jan 24, 2024

melvin-bot bot removed Overdue labels Jan 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-01-17] [HOLD for payment 2024-01-11] [$500] Android- IOU-When user changes Stop address, the Request money amount is not updated #26946

[HOLD for payment 2024-01-17] [HOLD for payment 2024-01-11] [$500] Android- IOU-When user changes Stop address, the Request money amount is not updated #26946

izarutskaya commented Sep 7, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Sep 7, 2023

melvin-bot bot commented Sep 7, 2023

dukenv0307 commented Sep 7, 2023

melvin-bot bot commented Sep 8, 2023

melvin-bot bot commented Sep 8, 2023

melvin-bot bot commented Sep 8, 2023

ashimsharma10 commented Sep 8, 2023

jeet-dhandha commented Sep 8, 2023

ahmedGaber93 commented Sep 11, 2023

melvin-bot bot commented Sep 11, 2023

melvin-bot bot commented Sep 11, 2023

cubuspl42 commented Sep 11, 2023

dukenv0307 commented Sep 11, 2023

cubuspl42 commented Sep 11, 2023

dukenv0307 commented Sep 11, 2023

ahmedGaber93 commented Sep 11, 2023

This comment was marked as outdated.

cubuspl42 commented Sep 12, 2023 •

edited

Loading

ahmedGaber93 commented Sep 13, 2023 •

edited

Loading

melvin-bot bot commented Jan 10, 2024

melvin-bot bot commented Jan 10, 2024 •

edited by CortneyOfstad

Loading

melvin-bot bot commented Jan 10, 2024 •

edited by CortneyOfstad

Loading

CortneyOfstad commented Jan 15, 2024

CortneyOfstad commented Jan 17, 2024

CortneyOfstad commented Jan 17, 2024

ashimsharma10 commented Jan 17, 2024

CortneyOfstad commented Jan 17, 2024

cubuspl42 commented Jan 18, 2024

CortneyOfstad commented Jan 18, 2024

ashimsharma10 commented Jan 19, 2024

CortneyOfstad commented Jan 22, 2024

CortneyOfstad commented Jan 24, 2024

[HOLD for payment 2024-01-17] [HOLD for payment 2024-01-11] [$500] Android- IOU-When user changes Stop address, the Request money amount is not updated #26946

[HOLD for payment 2024-01-17] [HOLD for payment 2024-01-11] [$500] Android- IOU-When user changes Stop address, the Request money amount is not updated #26946

Comments

izarutskaya commented Sep 7, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

melvin-bot bot commented Sep 7, 2023

melvin-bot bot commented Sep 7, 2023

Bug0 Triage Checklist (Main S/O)

dukenv0307 commented Sep 7, 2023

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Sep 8, 2023

melvin-bot bot commented Sep 8, 2023

melvin-bot bot commented Sep 8, 2023

ashimsharma10 commented Sep 8, 2023

jeet-dhandha commented Sep 8, 2023

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

ahmedGaber93 commented Sep 11, 2023

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Sep 11, 2023

melvin-bot bot commented Sep 11, 2023

cubuspl42 commented Sep 11, 2023

dukenv0307 commented Sep 11, 2023

cubuspl42 commented Sep 11, 2023

dukenv0307 commented Sep 11, 2023

ahmedGaber93 commented Sep 11, 2023

This comment was marked as outdated.

cubuspl42 commented Sep 12, 2023 • edited Loading

ahmedGaber93 commented Sep 13, 2023 • edited Loading

melvin-bot bot commented Jan 10, 2024

melvin-bot bot commented Jan 10, 2024 • edited by CortneyOfstad Loading

melvin-bot bot commented Jan 10, 2024 • edited by CortneyOfstad Loading

CortneyOfstad commented Jan 15, 2024

CortneyOfstad commented Jan 17, 2024

Payment Summary

CortneyOfstad commented Jan 17, 2024

ashimsharma10 commented Jan 17, 2024

CortneyOfstad commented Jan 17, 2024

cubuspl42 commented Jan 18, 2024

CortneyOfstad commented Jan 18, 2024

ashimsharma10 commented Jan 19, 2024

CortneyOfstad commented Jan 22, 2024

CortneyOfstad commented Jan 24, 2024

izarutskaya commented Sep 7, 2023 •

edited by melvin-bot bot

Loading

cubuspl42 commented Sep 12, 2023 •

edited

Loading

ahmedGaber93 commented Sep 13, 2023 •

edited

Loading

melvin-bot bot commented Jan 10, 2024 •

edited by CortneyOfstad

Loading

melvin-bot bot commented Jan 10, 2024 •

edited by CortneyOfstad

Loading