[$250] Room - Members number does not update in real time when user rejoin the room

[lanitochka17]

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.80-1
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
Email or phone of affected tester (no customers): gocemate+a178@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

1. Create room> Invite a member
2. Leave the room
3. Open the room again> Join
4. Go to header to open details page
5. Take a look at members section

Expected Result:

Members number should update since there is a new member in the room

Actual Result:

Members number does not update in real time when user rejoin the room. User must open Members page to update the info

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

Bug6505057_1717768640574.Recording__3142.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017450de92e982e995
• Upwork Job ID: 1799947003320904058
• Last Price Increase: 2024-06-09
• Automatic offers:
• dominictb | Contributor | 102720197

Issue Owner

Current Issue Owner: @allroundexperts

[melvin-bot]

Triggered auto assignment to @VictoriaExpensify (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]

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

[jainilparikh]

Proposal

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

Member count is not immediately updated when the User re-joins the chat. It is updated after the user clicks on 'Members' button.

What is the root cause of that problem?

When we open the side-pane (Which contains Members, Settings, Private notes etc) the state of a member does not get updated from the server.

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

Update the number of member's by calling the server using the OPEN_ROOM_MEMBERS_PAGE action in ReportDetailsPage.tsx (https://github.com/Expensify/App/blob/main/src/pages/ReportDetailsPage.tsx) class by adding the following code:

    const getRoomMembers = useCallback(() => {
        if (!report) {
            return;
        }
        Report.openRoomMembersPage(report.reportID);
    }, [report]);

    useEffect(() => {
        getRoomMembers();
        // eslint-disable-next-line react-hooks/exhaustive-deps
    }, []);

What alternative solutions did you explore? (Optional)

Since, RoomMemberPage is using the same function, create this function in a common file so both classes can use it.

Verified that the above solution updates the member count appropriately.

Video:

screen-recording-2024-06-08-at-35138-pm_BWW7Ctyh.mp4

[Tony-MK]

Proposal

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

Room - Members number does not update in real time when user rejoin the room

What is the root cause of that problem?

The joinRoom function is not consistent with the inviteToRoom function.

Therefore, the inviteToRoom function adds the new participant to the report.participants array.

Unlike when joining a room, when the getVisibleChatMemberAccountIDs function is called, it has one less participant in the report.participants array

App/src/pages/ReportDetailsPage.tsx

Lines 92 to 100 in f3bb5a5

	const participants = useMemo(() => {
	if (isGroupChat) {
	return ReportUtils.getParticipantAccountIDs(report.reportID ?? '');
	}
	if (isSystemChat) {
	return ReportUtils.getParticipantAccountIDs(report.reportID ?? '').filter((accountID) => accountID !== session?.accountID);
	}
	return ReportUtils.getVisibleChatMemberAccountIDs(report.reportID ?? '');
	}, [report, session, isGroupChat, isSystemChat]);

The report.participants array won't be updated in real-time because the front end has not been synced with the backend.

Therefore, there will also be one less active member of the room.

App/src/pages/ReportDetailsPage.tsx

Lines 103 to 107 in f3bb5a5

	const activeChatMembers = participants.flatMap((accountID) => {
	const pendingMember = report?.pendingChatMembers?.findLast((member) => member.accountID === accountID.toString());
	return !pendingMember || pendingMember.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ? accountID : [];
	});

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

Let's make the joinRoom function consistent with the inviteToRoom function by optimistically adding the new participant before updating its notification preference.

Similar to the inviteToRoom function, this change helps with the expected offline behavior.

We can do this by replicating and modifying the code below to be suited for the joinRoom function.

App/src/libs/actions/Report.ts

Lines 2718 to 2729 in f3bb5a5

	const optimisticData: OnyxUpdate[] = [
	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
	value: {
	participants: participantsAfterInvitation,
	pendingChatMembers,
	},
	},
	...newPersonalDetailsOnyxData.optimisticData,
	];

[VictoriaExpensify]

Keeping this as a low priority and adding it to the #vip_vsb project

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~017450de92e982e995

[melvin-bot]

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

[dominictb]

Proposal

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

Members number does not update in real time when user rejoin the room. User must open Members page to update the info

What is the root cause of that problem?

When the user leaves the room, they will remain a member of the room (in reports.participants), but the hidden will be set to true in the back-end from the LeaveRoom endpoint so that member will be filtered out and not appear in the list of members.

When the user re-joins the room, the hidden of that members will be set to false so the member appears again, but the app's not doing this optimistically, so Members number looks like it doesn't change.

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

When joining room here, we should set the hidden of the participant joining to false

Since updateNotificationPreference is used elsewhere too, we can add isJoiningRoom and only set hidden to false for the participant if isJoiningRoom is true. And use isJoiningRoom = true from joinRoom operation.

We should not optimistically add the new participant because the participant is already there, and we need to retain the role of the participant (ie. If the user was Admin of the room, then leaves the room then rejoins again, they should remain the Admin of the room), adding the participant as new will override the role. We just need to update hidden to false.

We also should update hidden to true when leaveRoom (for case of leaving workspace room). Leaving room has the same problem when done in offline mode.

What alternative solutions did you explore? (Optional)

I noticed something odd when leaving the room as Admin. After the Admin user left the room, they do not appear in the Members list, but they can still remove other members from the room. We can consider to remove/disable the Remove button/functionality if the current user is hidden from the room, or maybe transfer the Admin role to another room member, when the current Admin leaves the room

Another issue needs to be fixed (raised in the PR: #43741 (comment)): If we invite a new email, the number of participant increase by 2 instead of 1, and stay there for a while until we reload the app

Root cause:

When we invite new email (which doesn't exist in the personal details), the report.participants will contain an optimistic record. When the BE returns data, it will add a record to report.participants with the same email, but with correct accountID. However, when we calculate the report members by ReportUtils.getParticipantsAccountIDsForDisplay function, we are using the whole report.participants record containing both the correct accountID and optimistic accountID of the new user. Hence, the member count is increased 2 instead of 1

Solution

• Once we receive response from InviteToRoom API call, we should clear the optimistic participant record, the same way as we clear optimistic personal details
• However, there will be a brief moment (since Onyx updates are async) that both the optimistic and correct participant records before the optimistic records are cleared. We'll overcome this by filtering the optimistic participant record in ReportUtils.getParticipantsAccountIDsForDisplay if the personalDetail.login matches

[allroundexperts]

Reviewing today!

[jainilparikh]

@allroundexperts any updates on this ?

[allroundexperts]

Will post an update tonight.

[allroundexperts]

Thanks for the proposals everyone. @jainilparikh your proposal won't work for the offline case. @Tony-MK The participant seem to exist already. It's just the hidden flag that we need to toggle off. As such, @dominictb's proposal looks good to me.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @dominictb 🎉 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 📖

[jainilparikh]

@allroundexperts , I believe handling offline case was an edge case for the solution that I had proposed and IMHO, this should have been covered as part of our PR discussion. I had already done the hard work on identifying the root-cause and proposing the right solution for it.

Most of @dominictb 's solution is based on the findings and solution (Toggling the hidden flag) that I had posted above with the slight change that it handles the offline case.

CC: @iwiznia / @VictoriaExpensify let me know your thoughts on the same.

[allroundexperts]

@allroundexperts , I believe handling offline case was an edge case for the solution that I had proposed and IMHO, this should have been covered as part of our PR discussion. I had already done the hard work on identifying the root-cause and proposing the right solution for it.

Most of @dominictb 's solution is based on the findings and solution (Toggling the hidden flag) that I had posted above with the slight change that it handles the offline case.

CC: @iwiznia / @VictoriaExpensify let me know your thoughts on the same.

@jainilparikh I don't think that the offline flow is an "edge case". Your solution seems to be different from what the selected proposal mentioned as well.

[dominictb]

PR should be up for review in a few hours.

[melvin-bot]

This issue has not been updated in over 15 days. @iwiznia, @allroundexperts, @VictoriaExpensify, @dominictb eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[dominictb]

This should be ready for payment, no?

[melvin-bot]

@iwiznia, @allroundexperts, @VictoriaExpensify, @dominictb, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

[allroundexperts]

@VictoriaExpensify This is still pending payment.

[VictoriaExpensify]

Payment Summary:
Contributor: @dominictb paid $250 via Upwork
Contributor+: @allroundexperts owed $250 via NewDot

Thanks for your work on this!

[JmillsExpensify]

$250 approved for @allroundexperts