Web - Participant to split with doesn't update dynamically when a user leave room

kbecciv 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:

Create a workspace and invite some users to the workspace
Create a restricted room with the workspace
Go to the room, click on the plus icon > split bill
Enter any amount to go to the next step
From another device, login with user B that is a member of the workspace
From user B, go to the room and leave room
From user A, notice split with option

Expected Result:

Split with participants should update dynamically when a user leaves the room

Actual Result:

Split with participants doesn't update dynamically when a user leaves the room

Workaround:

Unknown

Platforms:

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

[ ] Android / native
[ ] Android / Chrome
[ ] iOS / native
[ ] iOS / Safari
[x] MacOS / Chrome / Safari
[ ] MacOS / Desktop

Version Number: 1.3.45.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

Screencast from 26-07-2023 21_06_30.webm

https://github.com/Expensify/App/assets/93399543/1f713c6a-e7a0-4d9d-9f07-7aa6d20286f6

Expensify/Expensify Issue URL: Issue reported by: @dukenv0307 Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690380426381949

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)

[x] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
[x] This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
[ ] This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
[ ] This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
[ ] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

kbecciv commented 1 year ago

Proposal @dukenv0307 Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690380426381949

Proposal

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

Participant to split with doesn't update dynamically when a user leave room

What is the root cause of that problem?

Because we only update props.iou.participants when we click on next button in MoneyRequestAmountPage, props.iou.participants doesn't update if the participant of the current report is updated https://github.com/Expensify/App/blob/85739ea075b554a5a15c8750f2d92bed27005f2f/src/pages/iou/steps/MoneyRequestAmountPage.js#L414

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

We should add an extra useEffect to update iou.participants if the participants of the report is changed

useEffect(() => {
    if (props.report && props.report.reportID) {
        console.log(props.report.participantAccountIDs)
        const currentUserAccountID = props.currentUserPersonalDetails.accountID;
        const participants = ReportUtils.isPolicyExpenseChat(props.report)
            ? [{reportID: props.report.reportID, isPolicyExpenseChat: true, selected: true}]
            : _.chain(props.report.participantAccountIDs)
                    .filter((accountID) => currentUserAccountID !== accountID)
                    .map((accountID) => {
                        const currentParticipant = _.find(props.iou.participants, (option) => option.accountID === accountID);
                        const selected = currentParticipant ? currentParticipant.selected : false
                        return {accountID, selected}
                    })
                    .value();
        IOU.setMoneyRequestParticipants(participants);
    }
}, [props.report])

OPTIONAL: If we want to dismiss the modal, we should add a check in useEffect here to dismiss the modal if the participants of the report are empty

if (!_.isEmpty(props.report) && _.isEmpty(props.report.participantAccountIDs)) {
    Navigation.dismissModal();
    return;
}

https://github.com/Expensify/App/blob/60f48175c9e3342ef192d878218820e5ed5a76cd/src/pages/iou/steps/MoneyRequestConfirmPage.js#L75 and add an extra dependency props.report here https://github.com/Expensify/App/blob/85739ea075b554a5a15c8750f2d92bed27005f2f/src/pages/iou/steps/MoneyRequestConfirmPage.js#L99

What alternative solutions did you explore? (Optional)

I saw that we pusher data now is wrong when a new user joins the public room like the image. So I think we also should fix it in BE as well. Screenshot from 2023-07-10 17-23-13

stephanieelliott commented 1 year ago

I think this is expected behavior -- you don't have to be a member of a room or workspace to split a bill. A bill can be split with anyone, so likewise we shouldn't remove someone from a bill split just because they leave a workspace or room.

Expensify / App