[$500] IOU - After editing a Distance request in offline mode, TBD does not appear in the Amount field #29899

lanitochka17 · 2023-10-18T18:45:01Z

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.3.86-1

Reproducible in staging?: Yes

Reproducible in production?: Yes

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:

Action Performed:

Prerequisites:
The account must have a distance request or create one

Open New Expensify app
Log in with your HT account
Navigate to a distance request details conversation
Disable the internet connection
Select the distance item
Add a new stop to the route
Save the change
Navigate to the report conversation

Expected Result:

After changing the route in offline mode the receipt route image should be pending, and the distance and amount should show as TBD

Actual Result:

The receipt route image is not in a pending state, and the distance and amount are not displayed as TBD when editing Distance expense request offline

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

Bug6242084_1697652502355.Recording__497.mp4

MacOS: Desktop

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01a882c54a16f7ff85
Upwork Job ID: 1714715676869181440
Last Price Increase: 2023-10-18
Automatic offers:

Tony-MK | Contributor | 27294889

melvin-bot · 2023-10-18T18:45:06Z

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

melvin-bot · 2023-10-18T18:45: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

melvin-bot · 2023-10-18T18:50:52Z

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

melvin-bot · 2023-10-18T18:50:58Z

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

FitseTLT · 2023-10-18T19:03:53Z

Proposal

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

After editing waypoints in offline mode, the amount and distance don't change to TBD

What is the root cause of that problem?

Our IOU.updateDistanceRequest doesn't handle the offline case on which it can't determine the updated amount and distance

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

In IOU.updateDistanceRequest we need to set the amount and distance value to TBD by checking if the user is offline.

What alternative solutions did you explore? (Optional)

We can also use OptimisticData to set the values to TBD until the user goes online.

Tony-MK · 2023-10-19T03:13:31Z

Proposal

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

After editing a Distance request in offline mode, TBD does not appear in the Amount field

What is the root cause of that problem?

The root cause of the problem is that when the user changes the waypoints when offline the transaction data remains the same until the new data, with the correct distance and amount, is fetched from the server changes it. The receipt data stays the same as well. Currently, there is only one handler for displaying common.tbd but it only does this when the transaction.amount and transaction.modifiedAmount are both set to zero. For example:

App/src/components/ReportActionItem/MoneyRequestView.js

Lines 102 to 104 in b38171d

    
           if (isDistanceRequest && !formattedTransactionAmount) { 
        
               formattedTransactionAmount = translate('common.tbd'); 
        
           }

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

Firstly, we need to handle the text to be displayed when the waypoints key is present in the transaction.pendingFields object. Therefore, we need to create new handlers near the lines where transaction.amount and transaction.merchant are used across the MoneyRequestView, MoneyRequestAction and ReportPreview components. This is to ensure consistency. Here is an example of the handlers on the MoneyRequestView component:

App/src/components/ReportActionItem/MoneyRequestView.js

Lines 102 to 104 in b38171d

    
           if (isDistanceRequest && !formattedTransactionAmount) { 
        
               formattedTransactionAmount = translate('common.tbd'); 
        
           }

const hasPendingWaypoints = lodashGet(transaction, 'pendingFields.waypoints');
if (isDistanceRequest && (!formattedTransactionAmount || hasPendingWaypoints)) {
     formattedTransactionAmount = translate('common.tbd');
}

Also for the merchant string.

App/src/components/ReportActionItem/MoneyRequestView.js

Line 210 in b38171d

title={transactionMerchant}

// For replaceMerchantForTBD, probably use regex to replace the distance in the merchant string with 
// common.tbd or use the Distance.Utils.getDistanceMerchant function to generate a merchant string
// with common.tbd
title={hasPendingWaypoints ? replaceMerchantForTBD() : transactionMerchant}

Lastly, to render the default receipt, we need to add some logic such as !Object.hasOwn(transaction?.pendingFields, 'waypoints') to allow the getThumbnailAndImageURIs function, in the ReceiptUtils.ts file, to output the ReceiptGeneric. This will happen when the waypoints key is in the transaction.pendingFields.

ArekChr · 2023-10-19T07:20:34Z

I chose the proposal from @Tony-MK, it provides a more detailed description of the problem and clearly defined solutions that cover a broader range of use cases.

🎀 👀 🎀 C+ reviewed

melvin-bot · 2023-10-19T07:20:40Z

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

paultsimura · 2023-10-19T08:08:08Z

Proposal

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

When editing a Distance request offline, the distance and amount fields are not set to "TBD"

What is the root cause of that problem?

We are not handling the loading state of the distance transaction properly.

We set the transaction.isLoading when modifying the distance request here:

App/src/libs/actions/IOU.js

Lines 757 to 766 in cd9cd57

    
           optimisticData.push({ 
        
               onyxMethod: Onyx.METHOD.MERGE, 
        
               key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, 
        
               value: { 
        
                   ...updatedTransaction, 
        
                   pendingFields, 
        
                   isLoading: _.has(transactionChanges, 'waypoints'), 
        
                   errorFields: null, 
        
               }, 
        
           });

And we handle this state in other places, like here:

App/src/components/DistanceRequest/index.js

Line 84 in 19c8e25

const isLoading = lodashGet(transaction, 'isLoading', false);

But we do not handle this state on the MoneyRequestView page.

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

On MoneyRequestView, add the following variable:

const isLoading = lodashGet(transaction, 'isLoading', false);

And use it accordingly here:

    if (isDistanceRequest && (!formattedTransactionAmount || isLoading)) {
        formattedTransactionAmount = translate('common.tbd');
    }

App/src/components/ReportActionItem/MoneyRequestView.js

Lines 102 to 104 in 641e652

    
           if (isDistanceRequest && !formattedTransactionAmount) { 
        
               formattedTransactionAmount = translate('common.tbd'); 
        
           }

And here:

title={isLoading ? translate('common.tbd') : transactionMerchant}

App/src/components/ReportActionItem/MoneyRequestView.js

Lines 206 to 217 in 641e652

    
           {isDistanceRequest ? ( 
        
               <OfflineWithFeedback pendingAction={lodashGet(transaction, 'pendingFields.waypoints') || lodashGet(transaction, 'pendingAction')}> 
        
                   <MenuItemWithTopDescription 
        
                       description={translate('common.distance')} 
        
                       title={transactionMerchant} 
        
                       interactive={canEdit} 
        
                       shouldShowRightIcon={canEdit} 
        
                       titleStyle={styles.flex1} 
        
                       onPress={() => Navigation.navigate(ROUTES.EDIT_REQUEST.getRoute(report.reportID, CONST.EDIT_REQUEST_FIELD.DISTANCE))} 
        
                   /> 
        
               </OfflineWithFeedback> 
        
           ) : (

As for me, this is a cleaner approach as it doesn't modify any data in the transactions, which looks more like a hack compared to using the transaction.isLoading state, and we don't need to explicitly check for the online/offline state.

Also, I don't think we should modify the map preview before #28965, as the receipt will be replaced with an actual map for loading distance requests.

Result:

Screen.Recording.2023-10-19.at.10.11.12.mov

What alternative solutions did you explore? (Optional)

paultsimura · 2023-10-19T08:09:21Z

@ArekChr let me kindly disagree with your decision as from my POV there is a cleaner solution. Just posted the proposal here: #29899 (comment)

arosiclair · 2023-10-19T12:55:09Z

@Tony-MK I think we should just show TBD when pendingFields.waypoints is set rather than clearing the old amount and modifiedAmount fields. Can you update your proposal to do so?

paultsimura · 2023-10-19T13:12:35Z

@arosiclair that's exactly what my proposal suggests, isn't it?

Tony-MK · 2023-10-19T20:52:37Z

@arosiclair, I updated my prospal according to your comment

arosiclair · 2023-10-20T13:32:42Z

@arosiclair that's exactly what my proposal suggests, isn't it?

I saw your proposal but it is not the same because isLoading isn't granular enough. We only want to show TBD when there are pending changes on the route and it was always the attention to use pendingFields.waypoints for this. Also your RCA isn't quite as good as it more or less just says "the root cause of the problem is that we aren't using my solution".

Your proposal is okay but for these reasons I'm going to stick to @Tony-MK's updated proposal.

melvin-bot · 2023-10-20T13:33:13Z

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

paultsimura · 2023-10-20T14:25:05Z

I saw your proposal but it is not the same because isLoading isn't granular enough. We only want to show TBD when there are pending changes on the route and it was always the attention to use pendingFields.waypoints for this. Also your RCA isn't quite as good as it more or less just says "the root cause of the problem is that we aren't using my solution".

Thank you for the explanation @arosiclair, I agree with your decision.

Just to clarify, the only place where we set transaction.isLoading is here:

App/src/libs/actions/IOU.js

Line 763 in 3b86da6

isLoading: _.has(transactionChanges, 'waypoints'),

This is exactly what you described as "it was always the attention to use pendingFields.waypoints for this". Other transaction types do not have the isLoading state, that's why I don't think it isn't granular enough - that's just a shorthand property for the suggested check.

As for the RCA – agree, it was made a bit in a hurry.

Tony-MK · 2023-10-24T05:26:09Z

Hey @ArekChr, can you kindly review the PR and tell me what you think. Thank you

JmillsExpensify · 2023-10-24T22:54:03Z

PR review in progress

melvin-bot · 2023-11-20T13:08:27Z

This issue has not been updated in over 15 days. @JmillsExpensify, @arosiclair, @Tony-MK, @ArekChr eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

arosiclair · 2023-11-20T15:06:22Z

Looks like automations failed here for some reason? Fixes we're deployed a week ago. cc @JmillsExpensify

JmillsExpensify · 2023-11-24T18:11:35Z

Ah, nice thanks for the heads up. Payment summary is as follows:

Issue reporter: N/A
Contributor: @Tony-MK $500
Contributor+: @ArekChr expert agency (no payment)

JmillsExpensify · 2023-11-24T18:14:43Z

Contributor has been paid out. @ArekChr Also do you mind confirming whether a regression test should be created before I close this issue?

ArekChr · 2023-11-27T09:25:56Z

Sure, we can add a regression test here.

Regression Test Proposal

Log in to the app with a high-traffic account.
Navigate to a distance request details conversation.
Disable the internet connection.
Select the distance item.
Add a new stop to the route and save.
Navigate to the report conversation.
Verify that after adding a stop in offline mode, the receipt route image should be pending, and the distance and amount should show as TBD.

Do we agree 👍 or 👎

JmillsExpensify · 2023-11-29T00:16:16Z

Regression test issue created. Closing!

lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 18, 2023

melvin-bot bot assigned JmillsExpensify Oct 18, 2023

lanitochka17 added the External Added to denote the issue can be worked on by a contributor label Oct 18, 2023

melvin-bot bot changed the title ~~IOU - After editing a Distance request in offline mode, TBD does not appear in the Amount field~~ [$500] IOU - After editing a Distance request in offline mode, TBD does not appear in the Amount field Oct 18, 2023

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

melvin-bot bot assigned ArekChr Oct 18, 2023

melvin-bot bot assigned arosiclair Oct 19, 2023

arosiclair assigned Tony-MK Oct 20, 2023

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 20, 2023

melvin-bot bot added the Overdue label Oct 23, 2023

Tony-MK mentioned this issue Oct 24, 2023

Fix/issue 29899 #30232

Merged

59 tasks

melvin-bot bot added Reviewing Has a PR in review and removed Overdue labels Oct 24, 2023

melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Oct 24, 2023

melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Nov 20, 2023

arosiclair removed the Reviewing Has a PR in review label Nov 20, 2023

arosiclair added Weekly KSv2 and removed Monthly KSv2 labels Nov 20, 2023

Tony-MK mentioned this issue Nov 22, 2023

[$500] Expense - Expense preview shows incorrect amount briefly when WS default currency is different #31638

Closed

6 tasks

JmillsExpensify closed this as completed Nov 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[$500] IOU - After editing a Distance request in offline mode, TBD does not appear in the Amount field #29899

[$500] IOU - After editing a Distance request in offline mode, TBD does not appear in the Amount field #29899

lanitochka17 commented Oct 18, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Oct 18, 2023

melvin-bot bot commented Oct 18, 2023

melvin-bot bot commented Oct 18, 2023

melvin-bot bot commented Oct 18, 2023

FitseTLT commented Oct 18, 2023 •

edited

Loading

Tony-MK commented Oct 19, 2023 •

edited

Loading

ArekChr commented Oct 19, 2023

melvin-bot bot commented Oct 19, 2023

paultsimura commented Oct 19, 2023 •

edited

Loading

paultsimura commented Oct 19, 2023

arosiclair commented Oct 19, 2023

paultsimura commented Oct 19, 2023

Tony-MK commented Oct 19, 2023

arosiclair commented Oct 20, 2023

melvin-bot bot commented Oct 20, 2023

paultsimura commented Oct 20, 2023

Tony-MK commented Oct 24, 2023

JmillsExpensify commented Oct 24, 2023

melvin-bot bot commented Nov 20, 2023

arosiclair commented Nov 20, 2023

JmillsExpensify commented Nov 24, 2023

JmillsExpensify commented Nov 24, 2023

ArekChr commented Nov 27, 2023

JmillsExpensify commented Nov 29, 2023

[$500] IOU - After editing a Distance request in offline mode, TBD does not appear in the Amount field #29899

[$500] IOU - After editing a Distance request in offline mode, TBD does not appear in the Amount field #29899

Comments

lanitochka17 commented Oct 18, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Oct 18, 2023

melvin-bot bot commented Oct 18, 2023

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Oct 18, 2023

melvin-bot bot commented Oct 18, 2023

FitseTLT commented Oct 18, 2023 • edited Loading

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)

Tony-MK commented Oct 19, 2023 • edited Loading

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?

ArekChr commented Oct 19, 2023

melvin-bot bot commented Oct 19, 2023

paultsimura commented Oct 19, 2023 • edited Loading

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?

Result:

What alternative solutions did you explore? (Optional)

paultsimura commented Oct 19, 2023

arosiclair commented Oct 19, 2023

paultsimura commented Oct 19, 2023

Tony-MK commented Oct 19, 2023

arosiclair commented Oct 20, 2023

melvin-bot bot commented Oct 20, 2023

paultsimura commented Oct 20, 2023

Tony-MK commented Oct 24, 2023

JmillsExpensify commented Oct 24, 2023

melvin-bot bot commented Nov 20, 2023

arosiclair commented Nov 20, 2023

JmillsExpensify commented Nov 24, 2023

JmillsExpensify commented Nov 24, 2023

ArekChr commented Nov 27, 2023

JmillsExpensify commented Nov 29, 2023

lanitochka17 commented Oct 18, 2023 •

edited by melvin-bot bot

Loading

FitseTLT commented Oct 18, 2023 •

edited

Loading

Tony-MK commented Oct 19, 2023 •

edited

Loading

paultsimura commented Oct 19, 2023 •

edited

Loading