[HOLD #22699] Workspace custom room members list is not updated dynamically

[kbecciv]

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. Open the app
2. Click on plus
3. Click on new room
4. Write any name, select any workspace and select any visibility
5. Click on Create room
6. Open settings
7. Open Workspaces
8. Open workspace used in step 4
9. Click on members
10. Invite any member
11. Close the menu after invite and click on room name
12. Click on Members and observe that member invited in step 10 is not displayed in room
13. Reload the page and observe that now member is displayed

Expected Result:

App should dynamically update room member list if new member is added

Actual Result:

App does not dynamically update room member list and needs reload to display updated list

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.27.4

Reproducible in staging?: yes

Reproducible in production?: yes

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

does.not.update.room.member.list.dynamically.mp4

Expensify/Expensify Issue URL:

Issue reported by: @dhanashree-sawant

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01502d61faff3465e2
• Upwork Job ID: 1669451236729647104
• Last Price Increase: 2023-07-07

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• 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

[namhihi237]

Proposal

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

App should dynamically update room member list if new member is added

What is the root cause of that problem?

The problem here is that after inviting new members to workspaces, the app has not updated optimisticData member for the rooms of workspaces addMembersToWorkspace so we have to reopen the room to have new data.

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

We will find the rooms of workspaces that have chatTypes of “#policyRoom" and “#policyAnnounce" and will update participantAccountIDs for them with optimisticData:

First we need to create get policyRoom and policyAnnounce function in here:

function getAllPolicyRoomsAndAnnounce(policyID) {
 return  _.filter(allReports, (report) => report && report.policyID === policyID && [CONST.REPORT.CHAT_TYPE.POLICY_ROOM, CONST.REPORT.CHAT_TYPE.POLICY_ANNOUNCE].includes(report.chatType));
}

Then we need to add a function to build onyxOptimisticData for the room:

function addMembersToRoom(rooms, invitedEmailsToAccountIDs) {
	const roomMembersChats = {
		onyxOptimisticData: [],
		onyxFailureData: [],
	};
	  
	_.each(rooms, (room) => {
		roomMembersChats.onyxOptimisticData.push({
			onyxMethod: Onyx.METHOD.MERGE,
			key: `${ONYXKEYS.COLLECTION.REPORT}${room.reportID}`,
			value: {
		          participantAccountIDs: _.union(room.participantAccountIDs, _.values(invitedEmailsToAccountIDs))
			}
		})
		roomMembersChats.onyxFailureData.push({
			onyxMethod: Onyx.METHOD.MERGE,
			key: `${ONYXKEYS.COLLECTION.REPORT}${room.reportID}`,
			value: {
			 participants: room.participantAccountIDs
			}
		})
	});

	return roomMembersChats;
}

Update onyxOptimisticData in addMembersToWorkspace

const policyReport = ReportUtils.getAllPolicyRoomsAndAnnounce(policyID);
const membersToRoom = addMembersToRoom(policyReport, invitedEmailsToAccountIDs);

const optimisticData = [
      // rest code
	...membersChats.onyxOptimisticData,
	...membersToRoom.onyxOptimisticData 
];

const failureData = [
       // rest code
        ...membersChats.onyxFailureData,
        ...membersToRoom.onyxFailureData
    ];

Result:

Screen.Recording.2023-06-13.at.01.56.45.mov

What alternative solutions did you explore? (Optional)

N/A

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01502d61faff3465e2

[melvin-bot]

Current assignee @mallenexpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[mallenexpensify]

Was able to reproduce. @aimane-chnaif, please review the proposal above. Also let me know if you don't think this should be External.

[dukenv0307]

Proposal

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

App does not dynamically update room member list and needs reload to display updated list

What is the root cause of that problem?

The problem is here

App/src/pages/ReportParticipantsPage.js

Line 57 in bd0e843

const {participantAccountIDs} = report;

where we're getting the participantAccountIDs from the report. That list of participants is the same as the workspace members. So when the workspace members changed, the list of participants will become outdated and will only be updated a few seconds later.

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

Since the participants of the chat room is always the same as the workspace members, we shouldn't consider them as 2 data sources. The same problem of outdated policyName of the report happens earlier and we decided that if the user has access to the policy in the report, we should get information directly from that policy rather than depending on outdated data of the report (see this). It's basically the same issue here, just that now it happens for the workspace members rather than workspace name.

So we should do the same as what we've done there for the policyName of the report:

1. Create a method getReportParticipantAccountIds (similar to the getPolicyName here )
2. If the report is a workspace chat room, return the list of accountIds of the workspace members.
3. If not, return the participantAccountIDs of the report

This will make sure the room participants and workspace members are always synced since there's only 1 source of truth.

What alternative solutions did you explore? (Optional)

We can also optimistically update the participantAccountIDs when we add members to the workspace, but we also need to optimistically update it when we remove members from the workspace as well.

But there're some reasons I don't prefer this approach:

• Previously we just update the workspace data but now we're updating all the chat rooms as well, this can cause performance problems if there're many chat rooms from that workspace
• We don't have a single source of truth for data, which means now any update to the workspace members (add, remove, edit, ...) now need to involve the chat rooms, which add complexity (handling all actions to the workspace and all optimistic/success/failure data)

[melvin-bot]

@mallenexpensify, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

[mallenexpensify]

@aimane-chnaif , please review the above proposals. Thx

[abekkala]

@mallenexpensify @aimane-chnaif curious if the fix for this issue would also resolve #21059 that was just reported?

[mallenexpensify]

Unsure @abekkala but, since we have two proposals here already, it might be good to put that one on hold.
@aimane-chnaif please review the proposals above, thx

[aimane-chnaif]

@dukenv0307 can you please share snippet of getReportParticipantAccountIds you're going to implement?

[melvin-bot]

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

[dukenv0307]

@aimane-chnaif I just checked again on the latest main and see that now the member list in the public workspace room is not updated even though we reloaded the app. The member belongs to this room only when the member has access to this public room. I don't know if this is a new feature or not. @mallenexpensify Could you help to confirm what is expected here
See my video for more detail

Screen-Recording-2023-06-23-at-18.50.15.mp4

1. The member list in the workspace room only update when member access this room
2. The member list should be dynamic update when new member is added to the workspace

[aimane-chnaif]

I'd like to confirm the expected behavior based on room visibility:

1. Public

• All invitees to the workspace are already member, either joined room or not. (Or should they join automatically when invite to workspace or when create new room in invited workspace?)
• Non-invitees can become member only when they join the room

2. Restricted

• All invitees to the workspace are already member, either joined room or not. (Or should they join automatically when invite to workspace or when create new room in invited workspace?)
• Non-invitees don't have access to this room, cannot join

3. Private
   I am not sure about this logic since we don't have the feature to invite room yet. AFAIK, no one can access to this room except owner for now.

@mallenexpensify please correct logic if anything wrong.

[mallenexpensify]

Good question @aimane-chnaif , did you copy/paste the above from somewhere? I'm asking internally about expected behavior, it seems like something we should have documented somewhere.

[aimane-chnaif]

No copy/paste, just my thought

[mallenexpensify]

I'm checking internally here

I think room functionality is still in beta, or nor launched. I tried to test creating rooms on workspaces with multiple accounts and nothing seemed to work, the rooms don't show other people in the workspace.

I think we might want to put this on hold or I/we need to dig more

[melvin-bot]

@mallenexpensify, @aimane-chnaif Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[dukenv0307]

I'm checking internally here

I think room functionality is still in beta, or nor launched. I tried to test creating rooms on workspaces with multiple accounts and nothing seemed to work, the rooms don't show other people in the workspace.

@mallenexpensify Any update here? I can't check the internal discussion

[melvin-bot]

@mallenexpensify @aimane-chnaif this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[melvin-bot]

@mallenexpensify, @aimane-chnaif Eep! 4 days overdue now. Issues have feelings too...

[mallenexpensify]

Sorry for the delay @dukenv0307 , it's been 👻 in the Slack post and also in StackOverflow. I followed up again and shared with the room to get more 👀
https://expensify.slack.com/archives/C01GTK53T8Q/p1688599842980999?thread_ts=1687019472.887419&cid=C01GTK53T8Q

[melvin-bot]

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

[mallenexpensify]

I got the answer we needed

Public: Anyone with the room URL can access the room
Restricted: Can be discovered and commented in by users that are members of the Workspace the room is tied to.
Private: Can only be discovered and commented in by users that are explicitly invited to the room.

But... we're not planning to treat 'dynamically updating' issues as bugs, per this discussion,

Checking on next steps here

[aimane-chnaif]

I don't think this is "dynamically updating" issue as action performed by current user in current device.
All dynamic issues 1-8 here are from multiple devices test. (do action on device B and then device A is not synced).
But issue 9 (this GH) is from single device.
Also this one affects bad user experience as they will likely to see members list after creating new room.

[aimane-chnaif]

Public: Anyone with the room URL can access the room
Restricted: Can be discovered and commented in by users that are members of the Workspace the room is tied to.
Private: Can only be discovered and commented in by users that are explicitly invited to the room.

This answer is not clear enough to proceed this GH.
My real question was #20860 (comment)

[dukenv0307]

Agree with this point

But issue 9 (this GH) is from single device.
Also this one affects bad user experience as they will likely to see members list after creating new room.

[melvin-bot]

Current assignee @mallenexpensify is eligible for the NewFeature assigner, not assigning anyone new.

[melvin-bot]

Triggered auto assignment to Design team member for new feature review - @shawnborton (NewFeature)

[mallenexpensify]

Removing you @shawnborton since this is on hold.
Checking here about the single device/platform
https://expensify.slack.com/archives/C01GTK53T8Q/p1689111199121799?thread_ts=1688646398.596839&cid=C01GTK53T8Q

[mallenexpensify]

Closing based on this

https://expensify.slack.com/archives/C01GTK53T8Q/p1689112054991499?thread_ts=1688646398.596839&cid=C01GTK53T8Q

On that one, I think we can close it actually. The workspace rooms will be discoverable by everyone, but the workspace members wont be members of all the rooms when added to the workspace, they can join them but admin adding them to the workspace wont add them to all the rooms I think thats the latest on what we want to do so you can probably close that isse