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

[HOLD for payment 2023-08-03] DEV: The paid amount appears as 0 after sending a money request between users #22763

Closed
1 of 6 tasks
kbecciv opened this issue Jul 12, 2023 · 27 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff

Comments

@kbecciv
Copy link

kbecciv commented Jul 12, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Login as Account A and Account B on 2 devices
  2. Account A send money request to Account B
  3. Account B send money request back to Account A, with same amount
  4. Account B send other money request to Account A (maybe optional)
  5. Check the paid amount showing in the report and click to view details

Expected Result:

The correct paid amount should be displayed.

Actual Result:

The paid amount appears as 0 and there is error when click to view details

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.39-7 Dev
Reproducible in staging?: n/a
Reproducible in production?: n/a
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

Paid-amount-Issue.mp4

Expensify/Expensify Issue URL:
Issue reported by: @tranvantoan-qn
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1689132698599969

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01c15b83b5ee2d1b8e
  • Upwork Job ID: 1680113093348413440
  • Last Price Increase: 2023-07-15
@kbecciv kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 12, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 12, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jul 12, 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

@tranvantoan-qn
Copy link

Added screenshot and error log for more information:

Error 2
Error 1

console.txt

@tranvantoan-qn
Copy link

(It didn’t happen initially right after sending the Request Money, but then it started occurring and persisted, even after logging out and logging in again)

initially

@nkuoch nkuoch self-assigned this Jul 13, 2023
@nkuoch
Copy link
Contributor

nkuoch commented Jul 14, 2023

I saw this bug too, it's because the preview total relies on checking the iouReport, but no iouReport gets returned by the back end because no iouReportID gets returned for the chat once the iou is settled.
To me it happened also previous my PRs. (and I can also reproduce it in prod). I don't see how it ever worked, as auth never returns iouReportID for a chat once it's settled... Or did the getChats query returned some iouReports at some point and not anymore?

I made this fix to default on the reportAction amount if no iouReport gets returned ... but not sure how reliable it is.

@tranvantoan-qn
Copy link

@nkuoch
I checked out your branch and now I no longer see the both sender and the amount
image

@nkuoch
Copy link
Contributor

nkuoch commented Jul 14, 2023

When using my branch, can you tell me what you see if you click on the browser Application > IndexedDB > OnyxDB > keyvaluepairs > reportActions_{yourChatReportID}

An open the json with the reportPreview report action > message[0] > text.

image

@tranvantoan-qn
Copy link

I'm not sure which sub-element I should check, but the message is empty for some

I attached the database here (please convert it to .json, so It may help you to check it easier, the key is reportActions_285914402761623

reportActions_285914402761623.txt

Empty
Has Value

@nkuoch
Copy link
Contributor

nkuoch commented Jul 14, 2023

Thanks for testing. Can you pull my branch and try again?

@tranvantoan-qn
Copy link

tranvantoan-qn commented Jul 14, 2023

Hi,
It looks much better now as the paid amount is showing, but the sender name is still missing, or there is new logic added?

image

@nkuoch
Copy link
Contributor

nkuoch commented Jul 14, 2023

Thanks, can you pull my branch and try again?

@tranvantoan-qn
Copy link

image

It looks really good for me now, both paid and sender name are showing

Let me know if I can help you testing it further!

@tranvantoan-qn
Copy link

tranvantoan-qn commented Jul 14, 2023

Ah, sorry, there is one more thing:

It looks like the amount isn't showing correctly in the main report, it only gets updated to the correct value after clicking on the request to view details and then going back - as explained in the attached image

image

@adelekennedy
Copy link

following along here - I won't be moving this to external for now

@nkuoch nkuoch added the Internal Requires API changes or must be handled by Expensify staff label Jul 15, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 15, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jul 15, 2023

Triggered auto assignment to Contributor Plus for review of internal employee PR - @0xmiroslav (Internal)

@nkuoch
Copy link
Contributor

nkuoch commented Jul 15, 2023

@tranvantoan-qn Did you originally request 60 or 6000?

Do you mind sharing your steps from scratch (from creating the 2 accounts, make the request etc..). And possibly take a video of it.

@tranvantoan-qn
Copy link

tranvantoan-qn commented Jul 17, 2023

@nkuoch The original request was 6000. But the above issue only happened with the request I created before the fix.
It works fine if I create new requests

FYI: I tried to reproduce this issue today (17, July) and the app crashed when searched for Participants.

hasIOUWaitingOnCurrentUserBankAccount is not defined
    at eval (OptionsListUtils.js:664:1)
    at Function.each (each.js:14:1)
    at getOptions (OptionsListUtils.js:640:3)
    at Module.getNewChatOptions (OptionsListUtils.js:924:1)
    at MoneyRequestParticipantsSelector.getRequestOptions (MoneyRequestParticipantsSelector.js:85:14)

@melvin-bot melvin-bot bot added the Overdue label Jul 17, 2023
@adelekennedy
Copy link

not overdue conversation above

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jul 17, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 21, 2023

@nkuoch, @adelekennedy, @0xmiroslav Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@nkuoch
Copy link
Contributor

nkuoch commented Jul 24, 2023

Will be fixed with #20821

@melvin-bot melvin-bot bot removed the Overdue label Jul 24, 2023
@nkuoch nkuoch added Weekly KSv2 Reviewing Has a PR in review and removed Daily KSv2 labels Jul 24, 2023
@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jul 27, 2023
@melvin-bot melvin-bot bot changed the title DEV: The paid amount appears as 0 after sending a money request between users [HOLD for payment 2023-08-03] DEV: The paid amount appears as 0 after sending a money request between users Jul 27, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jul 27, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 27, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jul 27, 2023

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

If no regressions arise, payment will be issued on 2023-08-03. 🎊

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:

  • @0xmiroslav requires payment

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
Copy link

melvin-bot bot commented Jul 27, 2023

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:

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Aug 3, 2023
@adelekennedy
Copy link

checking in bugzero cause this one's a little weird

@adelekennedy
Copy link

adelekennedy commented Aug 6, 2023

@tranvantoan-qn I talked this over in Slack - I think in this case even though this was fixed elsewhere you deserve $250 reporting and $250 for help testing

Payment Summary
Payment Summary

@tranvantoan-qn - $250 for reporting bonus $ $250 for help testing. $500 total(paid via Upwork)

@tranvantoan-qn
Copy link

@adelekennedy
I truly appreciate your consideration!
I've accepted the offer. Thank you!

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. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff
Projects
None yet
Development

No branches or pull requests

5 participants