Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.33k stars 2.76k forks source link

[Awaiting Payment 2024-09-19][$250] [HIGH] Invoicing V1 - Archive B2B invoice room when the receiver workspace is deleted #47170

Open lanitochka17 opened 1 month ago

lanitochka17 commented 1 month ago

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.18-7 Reproducible in staging?: Y Reproducible in production?: N If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

Expected Result:

Send invoice option should be hidden for User A because User B is no longer an admin of his workspace

Actual Result:

User A can still send invoice to User B after User B deletes all the workspaces, which results in error.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/909d1f4c-e62b-487d-ab1d-50421fc3471b

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01903a726126738169
  • Upwork Job ID: 1829110835274275153
  • Last Price Increase: 2024-08-29
Issue OwnerCurrent Issue Owner: @
melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @rlinoz (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

github-actions[bot] commented 1 month ago

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.
rlinoz commented 1 month ago

@lanitochka17 do you have the prod behavior by any chance?

rlinoz commented 1 month ago

Hmmm did we just add the pay as business to invoices?

rlinoz commented 1 month ago

We added it here https://github.com/Expensify/App/pull/44970, I think we should do something on the backend, let me check if we are already planning something.

rlinoz commented 1 month ago

There is a section for it in the doc, but I don't see an issue for it. Anyway I am removing the Web-E deploy blocker label then.

rlinoz commented 1 month ago

Asked in Slack https://expensify.slack.com/archives/CSL3XBCCR/p1723216392276109

melvin-bot[bot] commented 1 month ago

@cristipaval Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot[bot] commented 1 month ago

@cristipaval Huh... This is 4 days overdue. Who can take care of this?

cristipaval commented 4 weeks ago

Work in progress

cristipaval commented 3 weeks ago

Same, WIP.

cristipaval commented 3 weeks ago

^Auth PR up for review

cristipaval commented 3 weeks ago

PR still in review

cristipaval commented 2 weeks ago

Just resolved the conflicts in the Auth PR.

cristipaval commented 2 weeks ago

@VickyStash do you think you could work on this issue and do the App changes needed when the b2b room is archived?

cristipaval commented 2 weeks ago

Also, @jjcoffee would you like to be assigned as the C+ for it?

rezkiy37 commented 2 weeks ago

Hi, I am Michael (Mykhailo) from Callstack, an expert agency and I can work on this issue.

jjcoffee commented 2 weeks ago

@cristipaval Sure, I'd be happy to!

jjcoffee commented 2 weeks ago

Just to note that I found a similar issue whilst testing another invoice rooms PR. If using the FAB to send an invoice personally to User B (so not using the inline + to send to the workspace directly), the invoice will get added to User B's archived workspace invoice room.

  1. [User A] Send an invoice to User B (who has no workspaces)
  2. [User B] Pay invoice as business
  3. [User B] Delete created workspace
  4. [User A] Send another invoice to User B
  5. Note there is an API request error: A record already exists with this ID, the request shows the same invoiceRoomReportID for the room created in (2).
rezkiy37 commented 2 weeks ago

Just to note that I found a similar issue whilst testing another invoice rooms PR. If using the FAB to send an invoice personally to User B (so not using the inline + to send to the workspace directly), the invoice will get added to User B's archived workspace invoice room.

  1. [User A] Send an invoice to User B (who has no workspaces)
  2. [User B] Pay invoice as business
  3. [User B] Delete created workspace
  4. [User A] Send another invoice to User B
  5. Note there is an API request error: A record already exists with this ID, the request shows the same invoiceRoomReportID for the room created in (2).

@jjcoffee This bug is not replicable, because User B cannot pay as a business in the current main or staging. https://github.com/Expensify/App/pull/47862 introduces this feature. cc @VickyStash

Details https://github.com/user-attachments/assets/a10fbc94-cfbc-4f08-b4c2-e288897f6f1d
rezkiy37 commented 2 weeks ago

I tested the bug and it is not replicable if the users are online and have a stable internet connection. The room was archived immediately once I deleted the workspace.

Details Archived No policy

I could reproduce the "bug" when User A is offline. It is expected because User A does not receive the latest updates until User A goes online.

Details https://github.com/user-attachments/assets/e9cb6b8d-c135-4163-9f16-6540b0a52746

However, I tried to test the same flow with expense submission and I have the same error message when User A is offline.

Details https://github.com/user-attachments/assets/098e470f-8813-42cf-b76b-016c5d0d755b

So I would say that this is not a bug and this behavior is correct. WDYT @cristipaval?

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @isabelastisser (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.

rezkiy37 commented 2 weeks ago

Just to note that I found a similar issue whilst testing another invoice rooms PR. If using the FAB to send an invoice personally to User B (so not using the inline + to send to the workspace directly), the invoice will get added to User B's archived workspace invoice room.

  1. [User A] Send an invoice to User B (who has no workspaces)
  2. [User B] Pay invoice as business
  3. [User B] Delete created workspace
  4. [User A] Send another invoice to User B
  5. Note there is an API request error: A record already exists with this ID, the request shows the same invoiceRoomReportID for the room created in (2).

@jjcoffee This bug is not replicable, because User B cannot pay as a business in the current main or staging. #47862 introduces this feature. cc @VickyStash

Details

The bug is replicable in staging, but only if user B already has a workspace to pay as a business. This logic is presented in staging. The app attaches the archived room ID for further invoices.

Details Screenshot 2024-08-29 at 12 53 53 Screenshot 2024-08-29 at 12 50 24
cristipaval commented 2 weeks ago

@rezkiy37, sorry for the lack of context. I fixed the original bug in the backend.

cristipaval commented 2 weeks ago

So you need to:

  1. Send an invoice to a user who is an admin of their primary workspace
  2. Pay as business (so that the invoice room is upgraded to b2b)
  3. Now the receiver deletes the workspace and the invoice room is archived
  4. The sender now opens the invoice room (which is archived).
  5. The App crashes
rezkiy37 commented 2 weeks ago

So you need to:

  1. Send an invoice to a user who is an admin of their primary workspace
  2. Pay as business (so that the invoice room is upgraded to b2b)
  3. Now the receiver deletes the workspace and the invoice room is archived
  4. The sender now opens the invoice room (which is archived).
  5. The App crashes

@cristipaval The staging does not crash for me, but I can send invoices to the archived invoice room. I will fix this because the app attaches the incorrect reportID.

Details

https://github.com/user-attachments/assets/3bc7d1d1-351b-498a-9196-bd4bed85727a https://github.com/user-attachments/assets/45831def-a31b-4630-974c-d9d2edec82e2

jjcoffee commented 2 weeks ago

Should we also fix the missing reportArchiveReasons.invoiceReceiverPolicyDeleted translation as part of this? It causes a crash on dev and an ugly string that displays in the archived room.

image

rezkiy37 commented 2 weeks ago

@jjcoffee Yes, I am fixing it in the scope of the upcoming PR.

rezkiy37 commented 2 weeks ago

Actively working on the issue.

rezkiy37 commented 1 week ago

Discussing with @cristipaval in Slack.

rezkiy37 commented 1 week ago

Hey! I will be OOO from 04.09 (afternoon) till 09.09. I will continue to work on this one once I am back.

isabelastisser commented 1 week ago

Thanks for the update!

rezkiy37 commented 1 week ago

Hey! I've opened the PR (https://github.com/Expensify/App/pull/48275) for review because the app's side is ready. One small backend improvement left. Discussion in Slack.

jjcoffee commented 4 days ago

@isabelastisser Looks like @cristipaval is on parental leave at the moment, is it possible to get another engineer assigned to review the PR?

isabelastisser commented 4 days ago

@rlinoz, any chance you can review the PR since Cristi is on leave? TIA!

rlinoz commented 4 days ago

Sure, on it.

rlinoz commented 1 day ago

This has been deployed to prod https://github.com/Expensify/App/pull/48275#issuecomment-2347262503