[HOLD for payment 2023-10-02] [$500] Web - Editing request money while the user paid the requested money will still notify the change

[kbecciv]

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:

1. Click '+' FAB from user A
2. Click on request money and request any amount from user B
3. When the request opens for user A go to editing page and click on the amount
4. Edit the amount from user A and leave it unsaved
5. Pay the request from USER B
6. Immediately after you paid the request save the change you made from User A in step 4

Expected Result:

No change should be reflected

Actual Result:

A notification that shows change will appear in the edit room

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.64.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

Recording.4292.mp4

sends.notification.to.paid.mp4

Expensify/Expensify Issue URL:
Issue reported by: @lidiyakelay
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1693558160950359

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b9b7308912bc30e7
• Upwork Job ID: 1702482062216015872
• Last Price Increase: 2023-09-15
• Automatic offers:
• namhihi237 | Contributor | 26743504
• lidiyakelay | Reporter | 26743507

[melvin-bot]

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

[melvin-bot]

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

[namhihi237]

Proposal

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

The request money report should not display a change message if the edit fails or gives an error

What is the root cause of that problem?

We have a step that is easier to reproduce.

1. Click the FAB button from user A
2. Click on request money and request any amount from user B
3. User A turns off the network
4. User B paid the request money
5. User A Edit amount and back online

From the above step, we can easily see that when editing the backend, an error will be returned because the request has been paid.

But when editing we have built message changes in optimisticData

App/src/libs/actions/IOU.js

Lines 999 to 1005 in 1eafc7d

	const optimisticData = [
	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThread.reportID}`,
	value: {
	[updatedReportAction.reportActionID]: updatedReportAction,
	},

And we have failureData here:

App/src/libs/actions/IOU.js

Lines 1037 to 1045 in 1eafc7d

	const failureData = [
	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThread.reportID}`,
	value: {
	[updatedReportAction.reportActionID]: updatedReportAction,
	},
	},
	{

It's easy to see that optimisticData and failureData for change messages are the same. That's why when the backend returns an error, nothing happens.

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

We should update failureData again to show the error in here

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

What alternative solutions did you explore? (Optional)

Or we can delete the message change if the error:

[alexpensify]

I've run out of time today and will test soon.

[alexpensify]

Still on my testing list

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[alexpensify]

@ntdiary - can you please review if the proposal will fix this issue? Thanks!

[ntdiary]

@alexpensify, just woke up, I will review it soon. 😄

[ntdiary]

@alexpensify, @namhihi237's proposal makes sense to me, and just a small tip, we already have the iou.error.genericEditFailureMessage prompt, which seems to not have been used yet. Using it here should work well. : )

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[alexpensify]

@ntdiary - all good, we are in different timezones. 😄 Thanks for the quick review here.

[melvin-bot]

❌ There was an error making the offer to @ntdiary for the Reviewer role. The BZ member will need to manually hire the contributor. cc @thienlnam

[roryabraham]

@namhihi237's proposal LGTM, and I agree iou.error.genericEditFailureMessage looks like the right message to use 👍🏼

[namhihi237]

@ntdiary @roryabraham While executing the step, I found an error in addition to the above one on current main and stg.
Steps:

1. Click the FAB button from user A
2. Click on request money and request any amount from user B
3. User A turns off the network
4. User B paid the request money
5. User A Edit amount and back online, Sometimes it is observed that messages are located in the wrong position

This error will not occur when there is any message before going offline.

1. Click the FAB button from user A
2. Click on request money and request any amount from user B
3. User A edit amount and save
4. User A turns off the network
5. User B paid the request money
6. User A Edit amount and back online

I think this error is not within the scope of this PR. What do you think? If agree, I will report a bug for this error and this will fix it in another issue.

Detail video.

Screen.Recording.2023-09-20.at.00.22.15.mov

[ntdiary]

User A Edit amount and back online, Sometimes it is observed that messages are located in the wrong position

@namhihi237, I couldn't reproduce it. 🤔

[namhihi237]

Yes, It happens sometimes, I'm not sure but guess it's due to the component rendering. I tried a few times but it was in the wrong position

[namhihi237]

I just tried to reproduce it on staging now

269210044-0a3f1eed-51ae-4922-8b79-a3e1e1147d60.mov

[ntdiary]

test.mp4

Ah, I can reproduce it, and even when I refresh the page, the message is still in the wrong position, and it keeps requesting to load more chats infinitely. I personally think we can create a separate issue to investigate it.
cc @roryabraham

The CREATED message is the first one in the List, but it actually renders below the MODIFIEDEXPENSE message. 🤔

[roryabraham]

I personally think we can create a separate issue to investigate it

Sure. @namhihi237 you should post it in #expensify-bugs so you can get the reporting bonus.

[melvin-bot]

🎯 ⚡️ Woah @ntdiary / @namhihi237, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @namhihi237 got assigned: 2023-09-19 00:56:48 Z
• when the PR got merged: 2023-09-20 13:29:13 UTC

On to the next one 🚀

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

• add failure data when edit request money #27758

If no regressions arise, payment will be issued on 2023-10-02. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @ntdiary requires payment
• @namhihi237 requires payment offer (Contributor)
• @lidiyakelay requires payment offer (Reporter)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

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:

• [@ntdiary] The PR that introduced the bug has been identified. Link to the PR:
• [@ntdiary] 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:
• [@ntdiary] 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:
• [@ntdiary] Determine if we should create a regression test for this bug.
• [@ntdiary] 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.
• [@alexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[alexpensify]

@ntdiary - please complete the checklist to prepare for the payment date. Thanks!

[ntdiary]

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:

• [@ntdiary] The PR that introduced the bug has been identified. Link to the PR: No need
• [@ntdiary] 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: No need
• [@ntdiary] 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
• [@ntdiary] Determine if we should create a regression test for this bug. No need
• [@ntdiary] 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. No need
• [@alexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@alexpensify, this is just polish for the new edit money request feature, so I don't think regression test is needed. : )

[alexpensify]

Here is the payment summary:

• External issue reporter @lidiyakelay $50
• Contributor that fixed the issue @namhihi237 $750
• Contributor+ that helped on the issue and/or PR @ntdiary $750

Upwork Job: https://www.upwork.com/jobs/~01b9b7308912bc30e7

*If applicable, the bonuses will be applied on the final payment

Extra Notes regarding payment: There is an urgency bonus here and I'll apply that in the payment process

[alexpensify]

Everyone has been paid and it looks like automation didn't add @ntdiary. I sent over an offer that needs your approval in Upwork.

[ntdiary]

@alexpensify, accepted the offer. 😄

[alexpensify]

Alright, everyone has been paid here. Great work!