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.56k stars 2.9k forks source link

Web - Split bill RHN doesn't close after member has been removed from a workspace #26262

Closed izarutskaya closed 1 year ago

izarutskaya commented 1 year 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!


Action Performed:

  1. Create a workspace on account A and invite account B into that workspace
  2. On account B go to the announce room and click on the plus icon > split bill > enter an amount and click next
  3. Go to account A and remove account B from the workspace
  4. Observe account B

Expected Result:

The split bill RHN should close as soon as account B has been removed from the workspace

Actual Result:

The RHN doesn't close on account B when account B has been removed from the workspace

Workaround:

Unknown

Platforms:

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

Version Number: v1.3.58-3

Reproducible in staging?: Y

Reproducible in production?: Y

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

https://github.com/Expensify/App/assets/115492554/24dc72e2-c3c3-47ca-9725-4d8a77b3d74d

https://github.com/Expensify/App/assets/115492554/a043341e-0503-4e0f-8cc1-728a10b3f1f0

Expensify/Expensify Issue URL:

Issue reported by: @Nathan-Mulugeta

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692706711050379

View all open jobs on GitHub

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Bug0 Triage Checklist (Main S/O)

spcheema commented 1 year ago

Proposed solution

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

Web - Split bill RHN doesn't close after member has been removed from a workspace

What is the root cause of that problem?**

On the report screen, we are showing a page not found view whenever a user does not have access to a policy but there is no business rule/condition in place to hide the Split bill RHN pages.

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

On NewRequestAmountPage & MoneyRequestConfirmPage page check if the policy access has been removed then close the RHN modal.

Here is the detailed flow:

const prevPolicyID = useRef(props.report.policyID);

    useEffect(() => {
        if((prevPolicyID.current !== undefined && prevPolicyID.current !== props.report.policyID))
        {
            Navigation.dismissModal(reportID);
        }
    }, [props.report])

Apply the same changes on NewRequestAmountPage page and yes it'll cover scenarios which means whenever user B is removed from a workspace

    useEffect(() => {
        if((prevPolicyID.current !== undefined && prevPolicyID.current !== report.policyID))
        {
            Navigation.dismissModal(reportID);
        }
    }, [report])

What alternative solutions did you explore? (Optional)

N/A

DylanDylann commented 1 year ago

Proposal

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

Split bill RHN doesn't close after member has been removed from a workspace

What is the root cause of that problem?

https://github.com/Expensify/App/blob/93bde2010f145bbb97c54c06922f0ea52453958c/src/pages/iou/steps/NewRequestAmountPage.js#L107-L112

In NewRequestAmountPage we will dismiss modal when the user don't have permission to comment like this report

Screenshot 2023-08-30 at 10 10 30

But with the announce room, when the user is removed from workspace the announce room will disappear totally and we don't have logic to dismiss modal in this case

In MoneyRequestConfirmPage, we also don't have logic to dismiss modal in above cases

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

  1. In NewRequestAmountPage https://github.com/Expensify/App/blob/93bde2010f145bbb97c54c06922f0ea52453958c/src/pages/iou/steps/NewRequestAmountPage.js#L107-L112 Update the condition to dismiss modal when the report is deleted like this

    useEffect(() => {
         // Never dismiss if it is new request without reportID in route
        if (!reportID) {
            return
        }
        // dismiss modal if we request money/split bill in the invalid report
        if ((report.reportID || report.isLoadingReportActions) && !ReportUtils.shouldDisableWriteActions(report)) {
            return;
        }
        Navigation.dismissModal(reportID);
    }, [report, reportID]);
  2. In MoneyRequestConfirmPage, add the same logic like we did in NewRequestAmountPage

    useEffect(() => {
        // Never dismiss if it is new request without reportID in route
        if (!reportID) {
            return
        }
        // dismiss modal if we request money/split bill in the invalid report
        if ((props.report.reportID || props.report.isLoadingReportActions) && !ReportUtils.shouldDisableWriteActions(props.report)) {
            return;
        }
    
        Navigation.dismissModal(reportID);
    }, [props.report, reportID]);

    Result

    My proposal works well for 4 cases

  3. Remove user when user is requesting money in amount step (at workspace room)

  4. Remove user when user are requesting money in confirm step (at workspace room)

  5. Remove user when user are splitting bill in amount step (at announce room)

  6. Remove user when user are splitting bill in confirm step (at announce room)

https://github.com/Expensify/App/assets/141406735/a14c4a85-e5b2-436e-972c-f7c158ee3eb1

stephanieelliott commented 1 year ago

This is the expected behavior, you can split a bill without a shared workspace. We would expect the RHP to persist on user B's account until they manually closed it.