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
2.99k stars 2.5k forks source link

[$250] Send invoice - Workspace switcher is reset to "Expensify" when sending invoice to the same WS #41575

Open lanitochka17 opened 2 weeks ago

lanitochka17 commented 2 weeks 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: 1.4.70-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: N/A Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Open workspace switcher
  3. Select a workspace
  4. Go to FAB > Send invoice
  5. Enter amount and select participant
  6. On confirmation page, select the same workspace in Step 3
  7. Create the invoice

Expected Result:

Workspace switcher should remain unchanged when sending the invoice to the same workspace

Actual Result:

Workspace switcher is reset to "Expensify" when sending the invoice to the same workspace

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/d7e185b6-05cd-42a5-b1fa-31096cfc551e

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01038e8ff26d237540
  • Upwork Job ID: 1787748015120437248
  • Last Price Increase: 2024-05-14
  • Automatic offers:
    • bernhardoj | Contributor | 0
Issue OwnerCurrent Issue Owner: @laurenreidexpensify
melvin-bot[bot] commented 2 weeks ago

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

lanitochka17 commented 2 weeks ago

@laurenreidexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

bernhardoj commented 2 weeks ago

Proposal

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

Workspace switcher is switched to Expensify when creating a new send invoice.

What is the root cause of that problem?

When we complete the send invoice, it will call dismissModal. https://github.com/Expensify/App/blob/da2cd90b594b92be06de29942e8d4f47a3f69c25/src/libs/actions/IOU.ts#L3379

dismissModal will call dismissModalWithReport if there is a reportID from dismissModal and decide whether to switch to the "Expensify" (all workspace). https://github.com/Expensify/App/blob/da2cd90b594b92be06de29942e8d4f47a3f69c25/src/libs/Navigation/dismissModalWithReport.ts#L51

In doesReportBelongToWorkspace, it will compare the report policyID with the workspace policyID, but if policyID isn't available, it will compare the participants. https://github.com/Expensify/App/blob/da2cd90b594b92be06de29942e8d4f47a3f69c25/src/libs/ReportUtils.ts#L1048

However, when we complete the send invoice, we are creating a new report and the report data is not available yet (not merged to the onyx yet). So, doesReportBelongToWorkspace will always return false.

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

Replace Navigation.dismissModal with Navigation.dismissModalWithReport. https://github.com/Expensify/App/blob/da2cd90b594b92be06de29942e8d4f47a3f69c25/src/libs/actions/IOU.ts#L3379

Then, we pass the invoice room report instead of the ID. We need to return the report object from getSendInvoiceInformation

if it's happening on other places too, then we can apply the same fix

dragnoir commented 1 week ago

Proposal

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

Workspace switcher is reset to "Expensify" when sending invoice to the same WS

What is the root cause of that problem?

The sendInvoice function calls for the Navigation.dismissModal after the API call.

https://github.com/Expensify/App/blob/2e3950103b347911c7e1902b316283604c3a683a/src/libs/actions/IOU.ts#L3410-L3412

Within dismissModal, the function originalDismissModalWithReport will keep the correct workspace on the workspace switchef if shouldOpenAllWorkspace is false

https://github.com/Expensify/App/blob/2e3950103b347911c7e1902b316283604c3a683a/src/libs/Navigation/dismissModalWithReport.ts#L53-L57

The issue here is that shouldOpenAllWorkspace is turning TRUE, due to doesReportBelongToWorkspace not turning the currect value.

https://github.com/Expensify/App/blob/2e3950103b347911c7e1902b316283604c3a683a/src/libs/Navigation/dismissModalWithReport.ts#L51

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

We need to fix doesReportBelongToWorkspace

https://github.com/Expensify/App/blob/2e3950103b347911c7e1902b316283604c3a683a/src/libs/ReportUtils.ts#L1065-L1070

We can add another check, if policyID exist, then it's from a policy and we need to keep the workspace switcher.

if (policyID  &&  report?.policyID  !==  CONST.POLICY.ID_FAKE) {
  return  true;
}

return (
isConciergeChatReport(report) ||
(report?.policyID  ===  CONST.POLICY.ID_FAKE  ||  !report?.policyID  ?  hasParticipantInArray(report, policyMemberAccountIDs) :  report?.policyID  ===  policyID)
);

if policyID exist, then we check if report?.policyID !== CONST.POLICY.ID_FAKE, if so, the function turns true

as per the comment on the function

https://github.com/Expensify/App/blob/2e3950103b347911c7e1902b316283604c3a683a/src/libs/ReportUtils.ts#L1062

This is created to check if it's not a DM. My suggestion tested without regression

POC:

https://github.com/Expensify/App/assets/12425932/19a65f86-dcb6-44f9-a884-b900a38688bf

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

laurenreidexpensify commented 5 days ago

@allroundexperts bump for review pls

melvin-bot[bot] commented 4 days ago

๐Ÿ“ฃ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? ๐Ÿ’ธ

allroundexperts commented 4 days ago

Checking today

allroundexperts commented 4 days ago

Thanks for the proposals everyone. I don't see any issue with @bernhardoj's proposal. @dragnoir's proposal seems a little risky to me. Since @bernhardoj proposed first, let's go with them.

๐ŸŽ€ ๐Ÿ‘€ ๐ŸŽ€ C+ reviewed

melvin-bot[bot] commented 4 days ago

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

dragnoir commented 4 days ago

@allroundexperts I tested the selected [proposa](https://github.com/Expensify/App/issues/41575#issuecomment-2093129242)l and it's not working.

@bernhardoj can you pls share a test branch? I may tested with some wrong details!

allroundexperts commented 4 days ago

@dragnoir Did you do the following?

Then, we pass the invoice room report instead of the ID. We need to return the report object from getSendInvoiceInformation

dragnoir commented 4 days ago

@dragnoir Did you do the following?

Yes, I did, does it work for you?

bernhardoj commented 3 days ago

You can test it here

dragnoir commented 3 days ago

@allroundexperts the updates from @bernhardoj works well. Sorry about that and thank you @bernhardoj

melvin-bot[bot] commented 3 days ago

๐Ÿ“ฃ @bernhardoj ๐ŸŽ‰ An offer has been automatically sent to your Upwork account for the Contributor role ๐ŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review ๐Ÿง‘โ€๐Ÿ’ป Keep in mind: Code of Conduct | Contributing ๐Ÿ“–

bernhardoj commented 3 days ago

PR is ready

cc: @allroundexperts