[HOLD for payment 2024-06-06] [$250] Send invoice - Workspace switcher is reset to "Expensify" when sending invoice to the same WS

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] 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?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6469997_1714739554185.20240503_202652.mp4

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 Owner

Current Issue Owner: @laurenreidexpensify

[melvin-bot]

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]

@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]

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.

App/src/libs/actions/IOU.ts

Line 3379 in da2cd90

Navigation.dismissModal(invoiceRoomReportID);

dismissModal will call dismissModalWithReport if there is a reportID from dismissModal and decide whether to switch to the "Expensify" (all workspace).

App/src/libs/Navigation/dismissModalWithReport.ts

Line 51 in da2cd90

const shouldOpenAllWorkspace = isEmptyObject(targetReport) ? true : !doesReportBelongToWorkspace(targetReport, policyMemberAccountIDs, policyID);

In doesReportBelongToWorkspace, it will compare the report policyID with the workspace policyID, but if policyID isn't available, it will compare the participants.

App/src/libs/ReportUtils.ts

Line 1048 in da2cd90

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

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.

App/src/libs/actions/IOU.ts

Line 3379 in da2cd90

Navigation.dismissModal(invoiceRoomReportID);

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]

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.

App/src/libs/actions/IOU.ts

Lines 3410 to 3412 in 2e39501

	API.write(WRITE_COMMANDS.SEND_INVOICE, parameters, onyxData);
	Navigation.dismissModal(invoiceRoomReportID);

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

App/src/libs/Navigation/dismissModalWithReport.ts

Lines 53 to 57 in 2e39501

	if (shouldOpenAllWorkspace) {
	switchPolicyID(navigationRef, {route: ROUTES.HOME});
	} else {
	switchPolicyID(navigationRef, {policyID, route: ROUTES.HOME});
	}

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

App/src/libs/Navigation/dismissModalWithReport.ts

Line 51 in 2e39501

const shouldOpenAllWorkspace = isEmptyObject(targetReport) ? true : !doesReportBelongToWorkspace(targetReport, policyMemberAccountIDs, policyID);

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

We need to fix doesReportBelongToWorkspace

App/src/libs/ReportUtils.ts

Lines 1065 to 1070 in 2e39501

	function doesReportBelongToWorkspace(report: OnyxEntry<Report>, policyMemberAccountIDs: number[], policyID?: string) {
	return (
	isConciergeChatReport(report) ||
	(report?.policyID === CONST.POLICY.ID_FAKE || !report?.policyID ? hasParticipantInArray(report, policyMemberAccountIDs) : report?.policyID === policyID)
	);
	}

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

App/src/libs/ReportUtils.ts

Line 1062 in 2e39501

* Checks if the supplied report belongs to workspace based on the provided params. If the report's policyID is _FAKE_ or has no value, it means this report is a DM.

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

POC:

20240506_231821.mp4

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[laurenreidexpensify]

@allroundexperts bump for review pls

[melvin-bot]

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

[allroundexperts]

Checking today

[allroundexperts]

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]

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

[dragnoir]

@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]

@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]

@dragnoir Did you do the following?

Yes, I did, does it work for you?

[dragnoir]

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

[melvin-bot]

📣 @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]

PR is ready

cc: @allroundexperts

[melvin-bot]

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

[melvin-bot]

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

• Fix workspace switcher is reset to "Expensify" when sending invoice on the same WS #42197

If no regressions arise, payment will be issued on 2024-06-06. 🎊

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

• @allroundexperts requires payment through NewDot Manual Requests
• @bernhardoj requires payment automatic offer (Contributor)

[melvin-bot]

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:

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

[laurenreidexpensify]

Payment Summary

• c+ $250 - @allroundexperts requires payment through NewDot Manual Requests
• contributor $250 @bernhardoj requires payment in upwrok (site currently being unstable will try again later)

[laurenreidexpensify]

@allroundexperts pls complete checklist thanks

[laurenreidexpensify]

Payment Summary

c+ $250 - @allroundexperts requires payment through NewDot Manual Requests
contributor $250 @bernhardoj paid in upwork

[allroundexperts]

Checklist

1. I was not able to pinpoint any particular PR which caused this. I think this was a bug that existed since we added the workspace switcher.
2. N/A
3. N/A
4. A regression test would be good to have.

Regression test

1. Login to an account having many workspaces.
2. Open workspace switcher and Select a workspace
3. Go to FAB > Send invoice > Enter amount and select participant
4. On confirmation page, select the same workspace in Step 2 > Create the invoice

Verify that the workspace switcher still has the same workspace selected.

Do we 👍 or 👎 ?

[JmillsExpensify]

$250 approved for @allroundexperts