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 2025-01-13] [$250] Another invoice can't be sent after one is paid #53019

Closed
1 of 8 tasks
m-natarajan opened this issue Nov 23, 2024 · 30 comments
Closed
1 of 8 tasks
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff

Comments

@m-natarajan
Copy link

m-natarajan commented Nov 23, 2024

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


Version Number: 9.0.66-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
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: @neil-marcellini
Slack conversation (hyperlinked to channel name): Expensify billpay

Action Performed:

  1. Go to staging.new.expensify.com
  2. Create a workspace if needed
  3. Go to More features, scroll down and enable Invoices
  4. Click green plus, Send invoice
  5. Send an invoice to User B
  6. As User B Go to invoice chat and pay the invoice as business
  7. [User A] Go to invoice report
  8. [User A] Click Add bank account
  9. [User A] In settings Add bank account
  10. Click on the bank account for invoicing and choose to make it the default
  11. Click the green plus, send a $2 to user B
  12. Open the invoice report

Expected Result:

The invoice report can be opened

Actual Result:

The invoice report is not found, because it failed to actually be created even though there was no error message shown in product. The Network response shows 400 Unique Constraints Violation
Backlogs : https://www.expensify.com/_devportal/tools/logSearch/#query=request_id:(%228e6c7cac282da4d0-MIA%22)+AND+timestamp:[2024-11-22T21:51:56.285Z+TO+2024-11-22T23:51:56.285Z]&index=logs_expensify-031332

Workaround:

unknown

Platforms:

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

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence
Recording.787.mp4
MultipeInvoicesFail2024-11-22_17-52-27.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021861805315178904377
  • Upwork Job ID: 1861805315178904377
  • Last Price Increase: 2024-12-04
Issue OwnerCurrent Issue Owner: @zanyrenney
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 23, 2024
Copy link

melvin-bot bot commented Nov 23, 2024

Triggered auto assignment to @zanyrenney (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

Copy link

melvin-bot bot commented Nov 27, 2024

@zanyrenney Whoops! This issue is 2 days overdue. Let's get this updated quick!

@zanyrenney
Copy link
Contributor

triaged the bug.

@melvin-bot melvin-bot bot removed the Overdue label Nov 27, 2024
@zanyrenney zanyrenney added the External Added to denote the issue can be worked on by a contributor label Nov 27, 2024
@melvin-bot melvin-bot bot changed the title Another invoice can't be sent after one is paid [$250] Another invoice can't be sent after one is paid Nov 27, 2024
Copy link

melvin-bot bot commented Nov 27, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 27, 2024
Copy link

melvin-bot bot commented Nov 27, 2024

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

@allgandalf
Copy link
Contributor

Waiting for proposals

@zanyrenney zanyrenney moved this to Bugs and Follow Up Issues in [#whatsnext] #expense Dec 2, 2024
@neil-marcellini neil-marcellini self-assigned this Dec 2, 2024
Copy link

melvin-bot bot commented Dec 4, 2024

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

@allgandalf
Copy link
Contributor

Not sure if this is purely FE, @neil-marcellini can you provide any more data from the logs, can you provide the data received in the logs to see if we send everything correctly ?

@neil-marcellini
Copy link
Contributor

Yeah fair, this is probably internal, at least to identify the root cause.

@neil-marcellini neil-marcellini added Internal Requires API changes or must be handled by Expensify staff Weekly KSv2 Engineering and removed External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Daily KSv2 labels Dec 5, 2024
Copy link

melvin-bot bot commented Dec 7, 2024

@neil-marcellini @zanyrenney @allgandalf this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@allgandalf
Copy link
Contributor

@neil-marcellini any updates here ? can you share some logs from devportal ?

@neil-marcellini
Copy link
Contributor

It looks like the problem is that we try to create the invoice room report again vs re-using the one that already exists. The invoiceRoomReportID is 1915196797357303 and that's where we get the unique constraints violation. So now it's time to figure out why that happens.

Logs
8e6c7cac282da4d0-MIA 2024-11-22 22:51:56 284 neil@expensifail.com staging-www1.sjc info Bedrock\Client - Starting a request ~~ command: 'SendInvoice' clusterName: 'auth' headers: '[authToken: '' senderPolicyID: '65D0DCB20D56CBCD' receiverEmail: 'neil+chat@expensifail.com' receiverInvoiceRoomID: '0' amount: '200' currency: 'USD' comment: 't2' merchant: '(none)' date: '2024-11-22' category: '' invoiceRoomReportID: '1915196797357303' createdChatReportActionID: '7119337413948476846' invoiceReportID: '8474422852713015' createdIOUReportActionID: '5945907154987080997' reportActionID: '1873530802046189827' reportPreviewReportActionID: '680976267448613529' transactionID: '2148214281977961263' transactionThreadReportID: '4106816954743422' createdReportActionIDForThread: '6699656931864550903' idempotent: '' companyName: '' companyWebsite: '' ip: '104.28.157.233' logParam: 'neil@expensifail.com' urlToNewDot: 'https://staging.new.expensify.com/' shouldReadUsingThreads: '' shouldHandleOnyxUpdates: '1' isNewDotRequest: '1' clientUpdateID: '3078029124' maxNumberOfUpdates: '500' requestID: '8e6c7cac282da4d0-MIA' lastIP: '104.28.157.233' writeConsistency: 'ASYNC' priority: '500' timeout: '110000']'
8e6c7cac282da4d0-MIA 2024-11-22 22:51:56 284 neil@expensifail.com staging-www1.sjc info Bedrock\Client - APC fetch host configs ~~ 0: 'db1.sjc' 1: 'db2.sjc' 2: 'db1.lax' 3: 'db2.lax' 4: 'db1.rno' 5: 'db2.rno'
8e6c7cac282da4d0-MIA 2024-11-22 22:51:56 284 neil@expensifail.com staging-www1.sjc info Bedrock\Client - Possible hosts ~~ nonBlacklistedHosts: '[0: 'db1.sjc' 1: 'db2.sjc' 2: 'db1.rno' 3: 'db2.rno' 4: 'db2.lax' 5: 'db1.lax']'
8e6c7cac282da4d0-MIA 2024-11-22 22:51:56 284 neil@expensifail.com staging-www1.sjc info Bedrock\Client - Reusing socket ~~ host: 'db1.sjc' cluster: 'auth' pid: '2450273'
8e6c7cac282da4d0-MIA 2024-11-22 22:51:56 284 neil@expensifail.com db1.sjc info [concurrent] Beginning transaction
8e6c7cac282da4d0-MIA 2024-11-22 22:51:56 284 neil@expensifail.com db1.sjc info Checking authToken validity for accountID=2777229 / partnerID=83 / partnerUserID=expensify.cash-e000bd93-c3d2-0063-da4e-2e6914511ce7 / isValidated=true / expiresAt=1732322754363771 / isPasswordless=true
8e6c7cac282da4d0-MIA 2024-11-22 22:51:56 285 neil@expensifail.com db1.sjc info CreateReport::process reportID 1915196797357303
8e6c7cac282da4d0-MIA 2024-11-22 22:51:56 285 neil@expensifail.com db1.sjc info {SQLITE} Code: 1555, Message: abort at 28 in [INSERT INTO reports ( reportID, created, state, accountID, reportName, currency ) VALUES ( 1915196797357303, '2024-11-22 22:51:56', 0, 0, 'Chat Report', 'USD' );]: UNIQUE constraint failed: reports.reportID
8e6c7cac282da4d0-MIA 2024-11-22 22:51:56 285 neil@expensifail.com db1.sjc info 'read/write transaction', query failed with error #19 (UNIQUE constraint failed: reports.reportID): INSERT INTO reports ( reportID, created, state, accountID, reportName, currency ) VALUES ( 1915196797357303, '2024-11-22 22:51:56', 0, 0, 'Chat Report', 'USD' );
8e6c7cac282da4d0-MIA 2024-11-22 22:51:56 285 neil@expensifail.com db1.sjc alrt ENSURE_BUGBOT Encountered random reportID collision! It happened!
8e6c7cac282da4d0-MIA 2024-11-22 22:51:56 285 neil@expensifail.com db1.sjc warn Unique Constraints Violation, command: SendInvoice
8e6c7cac282da4d0-MIA 2024-11-22 22:51:56 285 neil@expensifail.com db1.sjc info command 'SendInvoice' timing info (ms): prePeek:0 (count: 0), peek:0 (count:1), process:0 (count:1), postProcess:0 (count:0), total:1, unaccounted:0, blockingCommitThreadTime:0, exclusiveTransactionLockTime:0. Commit: worker:0, sync:0. Queue: worker:0, sync:0, blocking:0, pageLock:0, escalation:0. Blocking: prePeek:0, peek:0, process:0, postProcess:0, commit:0. Upstream: peek:0, process:0, total:0, unaccounted:0.
8e6c7cac282da4d0-MIA 2024-11-22 22:51:56 285 neil@expensifail.com staging-www1.sjc info Bedrock\Client - Request finished ~~ host: 'db1.sjc' command: 'SendInvoice' jsonCode: '400 Unique Constraints Violation' duration: '2' net: '0.869' wait: '0.331' proc: '0.8' commitCount: ''
8e6c7cac282da4d0-MIA 2024-11-22 22:51:56 285 neil@expensifail.com staging-www1.sjc info Auth - AuthRequest #2 ~~ command: 'SendInvoice' jsonCode: '400' size: '183' duration: '2ms' host: 'db1.sjc' transactionID: '2148214281977961263'
8e6c7cac282da4d0-MIA 2024-11-22 22:51:56 285 neil@expensifail.com staging-www1.sjc info Auth - Call SendInvoice return 400
8e6c7cac282da4d0-MIA 2024-11-22 22:51:56 286 neil@expensifail.com staging-www1.sjc hmmm Throw ExpException - 20d0858735b59a0bea5fdc18a980d9d0 ~~ message: '400 Unique Constraints Violation' exceptionMessage: 'Auth SendInvoice returned an error' exceptionFile: '/git/releases/expensify.com/535064a/lib/Auth.php' exceptionLine: '133' exceptionCode: '400'

After some digging around I've found that we fail to get the receiverInvoiceRoomID here when sending the invoice. The reason is that we try to get it with receiver type set to individual, but it has been changed to the policy type when the receiver chooses to pay the invoice with a business bank account.

I also noticed that in App that the invoiceReceiver value still has the accountID set within it, but the type is updated to policy. I think maybe the Onyx update doesn't clear the accountID properly, though maybe that's not a big deal.

In Auth we should not be trying the get the invoice room with the receiver type sent to individual when it's been changed to policy, so is App passing the wrong parameters? I see that it's passing the receiverEmail, and in App it looks like it should probably pass the receiverInvoiceRoomID as it would do here if the invoiceChatReport.reportID is set. Evidently it's not.

2024-12-13T16:56:16.463627+00:00 expensidev2004 php-fpm: HOhGcU /api.php we@dont.know !ecash9.0.76-0! ?api? [info] Processing 'SendInvoice' for 'expensify.com' from '10.2.2.1'  ~~ command: 'SendInvoice' createdIOUReportActionID: '7853016652172516058' createdReportActionIDForThread: '5342542468315827578' reportActionID: '5442056646700209901' senderWorkspaceID: 'B1E844D99AF93E5C' accountID: '30' amount: '200' currency: 'USD' comment: '' merchant: '(none)' category: '' date: '2024-12-13' invoiceRoomReportID: '6356206350481224' createdChatReportActionID: '1887148289277430468' invoiceReportID: '263505571434242' reportPreviewReportActionID: '7400900744724627612' transactionID: '6793522903945149606' transactionThreadReportID: '2577528467645857' receiverEmail: 'b@53019.com' pusherSocketID: '810396.164154' authToken: '<REDACTED>' referer: 'ecash' platform: 'web' api_setCookie: 'false' email: 'a@53019.com' isFromDevEnv: 'true' appversion: '9.0.76-0' clientUpdateID: '1956' partnerName: 'expensify.com' browserGUID: '675c673071051' HTTP_CF_BOT_SCORE: '' HTTP_CF_JA3_HASH: '' HTTP_CF_JA4: ''

If the receiverInvoiceRoomID is set and the receiverEmail is not set, then Auth will hit this block and properly set the invoiceManagerID. Then we won't try and fail to get receiverInvoiceRoomID, nor will we try to create one in these blocks, and so there will be no error and the invoice will be properly created.

So it looks like the next step is to figure out why the invoiceChatReport.reportID is not set in App. I don't like that App has so much control over whether this succeeds in Auth, but I'm not sure there is a way to fix that given the design.

In this flow the user sends the second invoice to the individual (userB) via global create, and it's created as an invoice sent on the chat between userA's workspace and userB's, because userB chose to pay the invoice as a business the first time. That's actually kind of an odd UX, because once userB pays an invoice from you as a business you can't send them an invoice individually. I don't think we should force that. Maybe it's ok if userB can choose to pay it as an individual later, but I'm not sure due to this bug.

Given all that, I will fix it in App for now, then start a discussion about whether we want to change the UX.

@neil-marcellini
Copy link
Contributor

The App PR with the fix is ready to go, I just need to write the manual test and record a video.

@allgandalf
Copy link
Contributor

@neil-marcellini kudos with the RCA investigation, I was 😮 reading traces and logs and your explanation !! (Wish i had access to auth repo to look more 😬 )

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Dec 17, 2024
@neil-marcellini
Copy link
Contributor

PR is ready for review!

@allgandalf
Copy link
Contributor

🔴 Hopeful to get the PR merged this week

@allgandalf
Copy link
Contributor

♻️ PR was merged!!

@allgandalf
Copy link
Contributor

🔴 PR yet to hit staging

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jan 6, 2025
@melvin-bot melvin-bot bot changed the title [$250] Another invoice can't be sent after one is paid [HOLD for payment 2025-01-13] [$250] Another invoice can't be sent after one is paid Jan 6, 2025
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jan 6, 2025
Copy link

melvin-bot bot commented Jan 6, 2025

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

Copy link

melvin-bot bot commented Jan 6, 2025

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.80-6 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 2025-01-13. 🎊

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

  • @allgandalf requires payment (Needs manual offer from BZ)

Copy link

melvin-bot bot commented Jan 6, 2025

@allgandalf @zanyrenney @allgandalf The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@garrettmknight garrettmknight moved this from Bugs and Follow Up Issues to Hold for Payment in [#whatsnext] #expense Jan 7, 2025
@allgandalf
Copy link
Contributor

allgandalf commented Jan 9, 2025

BugZero Checklist:

  • [Contributor] Classify the bug:
Bug classification

Source of bug:

  • 1a. Result of the original design (eg. a case wasn't considered)
  • 1b. Mistake during implementation
  • 1c. Backend bug
  • 1z. Other: There is no source this case was never considered!

Where bug was reported:

  • 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
  • 2b. Reported on staging (eg. found during regression or PR testing)
  • 2d. Reported on a PR
  • 2z. Other:

Who reported the bug:

  • 3a. Expensify user
  • 3b. Expensify employee
  • 3c. Contributor
  • 3d. QA
  • 3z. Other:
  • [Contributor] 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: N/A (This was never considered, it has been this way since the beginning)

  • [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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:

  • [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

  • [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

    Link to issue:

Regression Test Proposal

Precondition:

Test:

  1. Create a workspace > Go to More features, scroll down and enable Invoices
  2. Click green plus, Send invoice
  3. Send an invoice to User B
  4. As User B Go to invoice chat and pay the invoice as business
  5. [User A] Go to invoice report
  6. [User A] Click Add bank account
  7. [User A] In settings Add bank account
  8. Click on the bank account for invoicing and choose to make it the default
  9. Click the green plus, send any amount to user B
  10. Open the invoice report and verify it loads successfully

Do we agree 👍 or 👎

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jan 13, 2025
Copy link

melvin-bot bot commented Jan 13, 2025

Payment Summary

Upwork Job

BugZero Checklist (@zanyrenney)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1861805315178904377/hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@zanyrenney
Copy link
Contributor

@zanyrenney
Copy link
Contributor

@allgandalf invited you to the job!

@zanyrenney
Copy link
Contributor

@allgandalf
Copy link
Contributor

Accepted thanks!!

@zanyrenney
Copy link
Contributor

can you double-check @allgandalf - it doesn't look like you have in upwork. Thanks! normally here we see "view hires" here if the offer has been accepted

Image

@zanyrenney
Copy link
Contributor

payment summary

paid $250 to @allgandalf via upwork.

@github-project-automation github-project-automation bot moved this from Hold for Payment to Done in [#whatsnext] #expense Jan 16, 2025
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 Engineering Internal Requires API changes or must be handled by Expensify staff
Projects
Status: Done
Development

No branches or pull requests

4 participants