Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$500] IOU - IOU report is duplicated in LHN after emptying Merchant field and saving it #26205

Closed
3 of 6 tasks
lanitochka17 opened this issue Aug 29, 2023 · 30 comments
Closed
3 of 6 tasks
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Monthly KSv2 Not a priority Reviewing Has a PR in review

Comments

@lanitochka17
Copy link

lanitochka17 commented Aug 29, 2023

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. Go to staging.new.expensify.com
  2. Request money from another use
  3. Click on IOU preview two times to enter detail page
  4. Click on the merchant
  5. Empty the merchant and click Save

Expected Result:

IOU report is not duplicated in LHN

Actual Result:

IOU report is now duplicated in LHN. The duplicate disappears after the page is refreshed

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.58-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

Notes/Photos/Videos: Any additional supporting documentation

20230829_124323.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01a14404ef51543e9e
  • Upwork Job ID: 1697289310359842816
  • Last Price Increase: 2023-09-07
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 29, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 29, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Aug 29, 2023

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

@StevenKKC
Copy link
Contributor

StevenKKC commented Aug 29, 2023

Proposal

Please state again the problem we are trying to solve in this issue.

IOU report is duplicated in LHN after emptying Merchant field and saving it.

What is the cause of this issue?

After emptying merchant field and saving it, BE returns error as below.
Screenshot 2023-08-29 151043

So the following failureData is merged.

App/src/libs/actions/IOU.js

Lines 1020 to 1038 in 766dd65

const failureData = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThread.reportID}`,
value: {
[updatedReportAction.reportActionID]: updatedReportAction,
},
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
value: transaction,
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${iouReport.report}`,
value: iouReport,
},
];

But iouReport.report is undefined (this should be a typo), so report_undefined will be merged into Onyx.
Because report_undefined's content is just same with iouReport, so this will be displayed in LHN.
Therefore, IOU report is duplicated.

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

Merchant field should not be empty and this is fixed by #26373.
We only need to change iouReport.report to iouReport.reportID in this line.

What alternative solutions have you investigated? (Optional)

None.

@garrettmknight
Copy link
Contributor

Might be related to #24608 (comment)

@luacmartins
Copy link
Contributor

I don't think this issue is related to #24608 (comment)

@garrettmknight
Copy link
Contributor

Cool, reproduced and I'll open it up!

@garrettmknight garrettmknight added the External Added to denote the issue can be worked on by a contributor label Aug 31, 2023
@melvin-bot melvin-bot bot changed the title IOU - IOU report is duplicated in LHN after emptying Merchant field and saving it [$500] IOU - IOU report is duplicated in LHN after emptying Merchant field and saving it Aug 31, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 31, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 31, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 31, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Aug 31, 2023

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

@Pujan92
Copy link
Contributor

Pujan92 commented Aug 31, 2023

I think will be fixed with this PR as not allowing empty merchant.

@StevenKKC
Copy link
Contributor

This #26373 disallow empty merchant.
But if EditMoneyRequest API fails, still appear duplicated IOU report.
I think we should fix this bug.

@melvin-bot melvin-bot bot added the Overdue label Sep 4, 2023
@garrettmknight
Copy link
Contributor

@jjcoffee What do you think?

@melvin-bot melvin-bot bot removed the Overdue label Sep 4, 2023
@jjcoffee
Copy link
Contributor

jjcoffee commented Sep 4, 2023

Hmm so looks like @Pujan92 is correct, that PR did fix the issue (it has been merged now and this particular issue is no longer reproducible), but @StevenKKC is also correct that there's a typo in the failureData, i.e. iouReport.report is used instead of iouReport.reportID, so if there is ever any error there will always be a duplicate report added as we'll be looking for an undefined report ID to merge into. I'm not sure how we'd normally handle this, should it be a new issue? @garrettmknight

@melvin-bot
Copy link

melvin-bot bot commented Sep 7, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Sep 7, 2023
@garrettmknight
Copy link
Contributor

@jjcoffee Let's fix that issue here! @StevenKKC can you propose the fix?

@melvin-bot melvin-bot bot removed the Overdue label Sep 7, 2023
@StevenKKC
Copy link
Contributor

Updated proposal.

@jjcoffee
Copy link
Contributor

jjcoffee commented Sep 8, 2023

@StevenKKC's proposed fix LGTM!

🎀👀🎀 C+ reviewed

@melvin-bot
Copy link

melvin-bot bot commented Sep 8, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Sep 11, 2023

@puneetlath, @garrettmknight, @jjcoffee Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 11, 2023
@garrettmknight
Copy link
Contributor

@StevenKKC looks like the automation didn't work, but I've assigned you. Please get the PR up when you get a chance.

@melvin-bot melvin-bot bot removed the Overdue label Sep 11, 2023
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Sep 11, 2023
@StevenKKC
Copy link
Contributor

@garrettmknight @jjcoffee PR #27194 is ready for review.

@jjcoffee
Copy link
Contributor

Reviewing tomorrow!

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@StevenKKC
Copy link
Contributor

@garrettmknight PR was deployed to production, but the title was not updated and I haven't received an offer. Could you please take a look at this?

@garrettmknight garrettmknight added Awaiting Payment Auto-added when associated PR is deployed to production and removed External Added to denote the issue can be worked on by a contributor labels Oct 11, 2023
@garrettmknight
Copy link
Contributor

@StevenKKC / @jjcoffee sorry for the wait on this one, slipped through the cracks with the 'Reviewing' label on it. Since this was such a small change, I'm going to forgo the urgency bonus.

Summarizing payments for this issue:

Upwork job: https://www.upwork.com/en-gb/ab/applicants/1697289310359842816/job-details

@garrettmknight
Copy link
Contributor

@StevenKKC Going to move this to monthly while your Upwork account gets sorted out. Ping me when it does and I'll complete payment.

@garrettmknight garrettmknight added Monthly KSv2 and removed Weekly KSv2 labels Oct 11, 2023
@StevenKKC
Copy link
Contributor

@garrettmknight I am afraid to let you know that My Upwork account is suspended so I couldn't get paid with my account. May I send proposal with my friend's account?

@melvin-bot melvin-bot bot added Daily KSv2 and removed Monthly KSv2 labels Oct 18, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 25, 2023

@puneetlath, @garrettmknight, @jjcoffee, @StevenKKC Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@garrettmknight
Copy link
Contributor

Hey @StevenKKC we're only able to pay through your account since it's verified by Upwork. Once you've got your Upwork account working again I'll pay.

@garrettmknight garrettmknight added Monthly KSv2 and removed Daily KSv2 labels Oct 25, 2023
Copy link

melvin-bot bot commented Dec 22, 2023

@puneetlath, @garrettmknight, @jjcoffee, @StevenKKC, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Monthly KSv2 Not a priority Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

8 participants