[$500] IOU - No error message when changing amount of a paid request #33799

izarutskaya · 2023-12-30T18:56:33Z

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: v1.4.20-2
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
Expensify/Expensify Issue URL:
Issue reported by: Applause-Internal Team
Slack conversation: @

Action Performed:

[User A] Request money from User B.
[User A] Open IOU details page > Click Amount.
[User B] Pay the request.
[User A] Enter a new amount and save it.

Expected Result:

Error message "Unexpected error editing the money request, please try again later" will show up when editing the amount of a settled request.

Actual Result:

Error message does not show up.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6328897_1703905910295.bandicam_2023-12-30_03-16-45-208.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01064a73e07695ea66
Upwork Job ID: 1741171439973834752
Last Price Increase: 2023-12-30
Automatic offers:

fedirjh | Reviewer | 28077812
Pujan92 | Contributor | 28077813

melvin-bot · 2023-12-30T18:56:37Z

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

melvin-bot · 2023-12-30T18:56:38Z

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

melvin-bot · 2023-12-30T18:56:40Z

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-12-30T18:56:42Z

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

neonbhai · 2023-12-30T19:13:32Z

Proposal

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

IOU - No error message when changing amount of a paid request

What is the root cause of that problem?

This happens as we don’t check if the request is paid before calling IOU.updateMoneyRequestAmountAndCurrency here:  

App/src/pages/EditRequestPage.js

Lines 123 to 136 in 8cb48e4

    
           const saveAmountAndCurrency = useCallback( 
        
               ({amount, currency: newCurrency}) => { 
        
                   const newAmount = CurrencyUtils.convertToBackendAmount(Number.parseFloat(amount)); 
        
                   // If the value hasn't changed, don't request to save changes on the server and just close the modal 
        
                   if (newAmount === TransactionUtils.getAmount(transaction) && newCurrency === TransactionUtils.getCurrency(transaction)) { 
        
                       Navigation.dismissModal(); 
        
                       return; 
        
                   } 
        
                   IOU.updateMoneyRequestAmountAndCurrency(transaction.transactionID, report.reportID, newCurrency, newAmount); 
        
                   Navigation.dismissModal(); 
        
               }, 
        
               [transaction, report],

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

We should add a check to see if the request is paid using ReportUtils.canEditFieldOfMoneyRequest()  here:

App/src/pages/EditRequestPage.js

Lines 127 to 131 in 8cb48e4

    
           // If the value hasn't changed, don't request to save changes on the server and just close the modal 
        
           if (newAmount === TransactionUtils.getAmount(transaction) && newCurrency === TransactionUtils.getCurrency(transaction)) { 
        
               Navigation.dismissModal(); 
        
               return; 
        
           }

We could also use ReportUtils.isSettled() for the check.

Alternatively:

We can add the check in IOU.updateMoneyRequestAmountAndCurrency here.

Tony-MK · 2023-12-31T00:36:22Z

Proposal

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

What is the root cause of that problem?

The backend is returning the correct error message, however we don't expect it because we are not handling it in the MoneyRequestView.

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

Get the errors from the errorFields and set it as the errors prop for the OfflineWithFeedback. The error message should not persist. Therefore, we can delete from the onyx once the user navigates away from the page or onClose. However, if we need them for later, we can also shouldShowErrorMessages if the latest error message is past a certain duration.

App/src/components/ReportActionItem/MoneyRequestView.js

Lines 201 to 202 in 8cb48e4

    
           <OfflineWithFeedback pendingAction={getPendingFieldAction('pendingFields.amount')}> 
        
               <MenuItemWithTopDescription

errors={getLatestAmountCurrencyErrors()}

Pseudo-code

function getLatestAmountCurrencyErrors(){
 errors = lodashGet(transaction, 'errorFields.amount') || lodashGet(transaction, 'errorFields.currency')
 errors.sort() // If needed 
 return errors.length > 0 ? errors[0] : null
}

What alternative solutions did you explore? (Optional)

Create a parent OfflineWithFeedback component of MoneyRequestView. Then, use the transaction.error value for the errors prop of the component.
Listen for the transaction and disable the save button once it's paid. Also, displays an error.

MitchExpensify · 2023-12-31T02:18:33Z

The expected result is weird to me, why would we tell the user an "unexpected error" occurs if it's expected that you cannot edit a settled amount? Why don't we just tell the user they cannot edit a settled amount if that is indeed expected?

Victor-Nyagudi · 2024-01-01T09:42:05Z

Is it intentional a user can edit a paid request's amount?

Wouldn't that lead to conflicts over how much was paid by who later on?

fedirjh · 2024-01-02T07:29:48Z

The expected result is weird to me, why would we tell the user an "unexpected error" occurs if it's expected that you cannot edit a settled amount? Why don't we just tell the user they cannot edit a settled amount if that is indeed expected?

This seems like an edge case , following the video , these are the detailed steps:

User A send money request to user B
User B opens the money request
User A tries to edit the amount
In the same time the user A is trying to edit the amount , the user B pays the request
User A clicks update, but it fails to update the amount as it was already paid
It's expected to show an error message in this case

For other cases, when the amount is paid , you can't open the edit form.

fedirjh · 2024-01-02T08:07:19Z

@neonbhai and @Tony-MK This seems like a recent regression , here is an old screenshot about the expected behaviour :

Pujan92 · 2024-01-02T10:57:30Z

Proposal

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

No error message shown for updating the paid amount transaction

What is the root cause of that problem?

It seems the issue occurs after this PR where we have used a new method to update an amount. For the failure case we don't have any report action error entry which isn't showing any error.

App/src/libs/actions/IOU.js

Lines 990 to 1011 in 320ff54

    
           // Clear out loading states, pending fields, and add the error fields 
        
           failureData.push({ 
        
               onyxMethod: Onyx.METHOD.MERGE, 
        
               key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, 
        
               value: { 
        
                   pendingFields: clearedPendingFields, 
        
                   isLoading: false, 
        
                   errorFields, 
        
               }, 
        
           }); 
        
           // Reset the iouReport to it's original state 
        
           failureData.push({ 
        
               onyxMethod: Onyx.METHOD.MERGE, 
        
               key: `${ONYXKEYS.COLLECTION.REPORT}${iouReport.reportID}`, 
        
               value: iouReport, 
        
           }); 
        
           return { 
        
               params, 
        
               onyxData: {optimisticData, successData, failureData}, 
        
           };

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

Add an entry for the failureData in the onyx which sets error to that specific report action.

    failureData.push({
        onyxMethod: Onyx.METHOD.MERGE,
        key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThread.reportID}`,
        value: {
            [updatedReportAction.reportActionID]: {
                errors: ErrorUtils.getMicroSecondOnyxError('iou.error.genericEditFailureMessage'),
            },
        },
    })

Like we have for the older method

App/src/libs/actions/IOU.js

Lines 2195 to 2203 in 320ff54

    
           const failureData = [ 
        
               { 
        
                   onyxMethod: Onyx.METHOD.MERGE, 
        
                   key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThread.reportID}`, 
        
                   value: { 
        
                       [updatedReportAction.reportActionID]: { 
        
                           errors: ErrorUtils.getMicroSecondOnyxError('iou.error.genericEditFailureMessage'), 
        
                       }, 
        
                   },

Result

Screen.Recording.2024-01-02.at.16.28.17.mp4

fedirjh · 2024-01-02T14:56:53Z

@Pujan92 Thank you for the proposal, the root cause looks correct. I just retested with reverting #32533 and it's working as expected.

For the solution, it looks good to me, I believe we should add the failure data inside getUpdateMoneyRequestParams to fix the bug with other fields as well.

🎀 👀 🎀 C+ reviewed

melvin-bot · 2024-01-02T14:57:00Z

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

melvin-bot · 2024-01-02T20:36:13Z

📣 @fedirjh 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

melvin-bot · 2024-01-02T20:36:17Z

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

fedirjh · 2024-01-09T08:52:51Z

Update: PR was merged and deployed to production #33880 (comment). It seems automation was not triggered for this one.

Pujan92 · 2024-01-25T16:28:21Z

Bump @MitchExpensify :)

MitchExpensify · 2024-01-27T01:39:43Z

Payment summary::

C @Pujan92 (Upwork)
C+ @fedirjh $500 (Upwork)

Paid and contracts ended

izarutskaya added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 30, 2023

melvin-bot bot changed the title ~~IOU - No error message when changing amount of a paid request~~ [$500] IOU - No error message when changing amount of a paid request Dec 30, 2023

melvin-bot bot assigned MitchExpensify Dec 30, 2023

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

melvin-bot bot assigned fedirjh Dec 30, 2023

kavimuru mentioned this issue Dec 31, 2023

Deploy Checklist: New Expensify 2023-12-29 #33772

Closed

28 tasks

melvin-bot bot assigned nkuoch Jan 2, 2024

nkuoch assigned Pujan92 Jan 2, 2024

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 2, 2024

Pujan92 mentioned this issue Jan 3, 2024

Fix: IOU - No error message when changing amount of a paid request #33880

Merged

59 tasks

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

MitchExpensify closed this as completed Jan 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[$500] IOU - No error message when changing amount of a paid request #33799

[$500] IOU - No error message when changing amount of a paid request #33799

izarutskaya commented Dec 30, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Dec 30, 2023

melvin-bot bot commented Dec 30, 2023

melvin-bot bot commented Dec 30, 2023

melvin-bot bot commented Dec 30, 2023

neonbhai commented Dec 30, 2023 •

edited

Loading

Tony-MK commented Dec 31, 2023 •

edited

Loading

MitchExpensify commented Dec 31, 2023

Victor-Nyagudi commented Jan 1, 2024

fedirjh commented Jan 2, 2024

fedirjh commented Jan 2, 2024

Pujan92 commented Jan 2, 2024 •

edited

Loading

fedirjh commented Jan 2, 2024

melvin-bot bot commented Jan 2, 2024

melvin-bot bot commented Jan 2, 2024

melvin-bot bot commented Jan 2, 2024

fedirjh commented Jan 9, 2024

Pujan92 commented Jan 25, 2024

MitchExpensify commented Jan 27, 2024

[$500] IOU - No error message when changing amount of a paid request #33799

[$500] IOU - No error message when changing amount of a paid request #33799

Comments

izarutskaya commented Dec 30, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Dec 30, 2023

melvin-bot bot commented Dec 30, 2023

melvin-bot bot commented Dec 30, 2023

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Dec 30, 2023

neonbhai commented Dec 30, 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?

Alternatively:

Tony-MK commented Dec 31, 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)

MitchExpensify commented Dec 31, 2023

Victor-Nyagudi commented Jan 1, 2024

fedirjh commented Jan 2, 2024

fedirjh commented Jan 2, 2024

Pujan92 commented Jan 2, 2024 • 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?

fedirjh commented Jan 2, 2024

melvin-bot bot commented Jan 2, 2024

melvin-bot bot commented Jan 2, 2024

melvin-bot bot commented Jan 2, 2024

fedirjh commented Jan 9, 2024

Pujan92 commented Jan 25, 2024

MitchExpensify commented Jan 27, 2024

izarutskaya commented Dec 30, 2023 •

edited by melvin-bot bot

Loading

neonbhai commented Dec 30, 2023 •

edited

Loading

Tony-MK commented Dec 31, 2023 •

edited

Loading

Pujan92 commented Jan 2, 2024 •

edited

Loading