[PAID] [$250] Workspace - Non-admin is not instantly removed from #admins member list after being demoted

[vincdargento]

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: 9.0.69-1
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Yes
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): applausetester+232803@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace settings > Members.
3. Invite a member and promote the member to admin.
4. Go to #admins room.
5. Click on the chat header > Members.
6. Note that the new admin is in the member list.
7. Go to workspace settings > Members.
8. Demote the admin to member.
9. Go to #admins room.
10. Click on the chat header > Members.

Expected Result:

The non-admin should now be removed from #admins room and will not appear in member list in #admins room.

Actual Result:

The non-admin still appears in member list in #admins room after being demoted to member.
After resetting the app in Troubleshoot (or relogin), the non-admin is removed from member list in #admins.

Workaround:

Unknown

Platforms:

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

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

bug.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021866294583392794294
• Upwork Job ID: 1866294583392794294
• Last Price Increase: 2024-12-31
• Automatic offers:
• allgandalf | Reviewer | 105532926
• FitseTLT | Contributor | 105532927

Issue Owner

Current Issue Owner: @strepanier03

[melvin-bot]

Triggered auto assignment to @strepanier03 (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.

[FitseTLT]

Proposal

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

Workspace - Non-admin is not instantly removed from #admins member list after being demoted

What is the root cause of that problem?

We are neither adding a user to the admin room nor removing them when setting or unsetting a user's role to admin optimistically here

App/src/libs/actions/Policy/Member.ts

Lines 452 to 466 in 6482913

	function updateWorkspaceMembersRole(policyID: string, accountIDs: number[], newRole: ValueOf<typeof CONST.POLICY.ROLE>) {
	const previousEmployeeList = {...allPolicies?.[policyID]?.employeeList};
	const memberRoles: WorkspaceMembersRoleData[] = accountIDs.reduce((result: WorkspaceMembersRoleData[], accountID: number) => {
	if (!allPersonalDetails?.[accountID]?.login) {
	return result;
	}
	result.push({
	accountID,
	email: allPersonalDetails?.[accountID]?.login ?? '',
	role: newRole,
	});
	return result;
	}, []);

and although the BE return participants removing the key for the user onyx merge will not be able to remove the existing key for the member hence we need to clear cache to see the member removed from the members of the admin room

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

We will retrieve the admin room of the policy from allReports with the condition that a report is isAdminRoom and the policyID is the same as the current policy and then update the report.participants list accordingly by removing the user-account ids (passed to updateWorkspaceMembersRole function) who's roles are being removed from admin (or vice versa when it is being upgraded to admin role)
We will get the current role from the policy and then compare it with the role being set and determine whether it is being set as admin or being removed from admin role (when being set to admin we might not need to know the prev role) and then update report.participants object of the admin room accordingly : the structure something similar to

"user_accountId": {
                  "notificationPreference": "always"
            }

on the removing case we should specifically set the participants key for the member to null so that onyx merge will successfully remove it.

And we will revert on failureData

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

For tests: we currently have tests for Member.updateWorkspaceMembersRole here

App/tests/actions/PolicyMemberTest.ts

Lines 107 to 117 in 6482913

	Member.updateWorkspaceMembersRole(fakePolicy.id, [fakeUser2.accountID], CONST.POLICY.ROLE.ADMIN);
	await waitForBatchedUpdates();
	await new Promise<void>((resolve) => {
	const connection = Onyx.connect({
	key: `${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`,
	waitForCollectionCallback: false,
	callback: (policy) => {
	Onyx.disconnect(connection);
	const employee = policy?.employeeList?.[fakeUser2?.login ?? ''];
	expect(employee?.role).toBe(CONST.POLICY.ROLE.ADMIN);

but we are only asserting for the employeeList of the policy.
Now we can create a policy admin room report (setting explicitly the policyID, 'participantsandchatTypesome how similar to [here](https://github.com/FitseTLT/App/blob/64829131fe962681626b4b016140521d05f7fe52/tests/actions/PolicyMemberTest.ts#L36-L39)) and set in onyx and then we will add more assertions asserting that the admin room of the policy has the correctparticipants` list accordingly for both cases where admin role is set and unset

What alternative solutions did you explore? (Optional)

[saifelance]

Proposal

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

suggestions for the code that might help address the issue where a non-admin is not immediately removed from the #admins room member list after being demoted.

What is the root cause of that problem?

A lack of immediate UI synchronization after the user's role is updated in the backend.

• Backend Delay in Propagation: When a user is demoted, the backend may update the user's role, but the changes are not reflected immediately in the UI (specifically in the #admins room member list). This could be due to the frontend not receiving the updated data after the role change.

• UI State Not Refreshing: The member list UI does not automatically refresh or update when the user's role is changed. This could be because the frontend is not actively fetching or listening for the updated role list or because the UI is not reacting to role change events in real-time.

• Missing Real-Time Update Mechanism: If the app does not have a real-time synchronization mechanism (such as WebSockets or a polling mechanism) to notify the frontend about the role change, the frontend remains unaware of the updated role until a more substantial update (like a reset or relogin) occurs.

using useState and useEffect

import { useEffect, useState } from 'react';

// State to manage member list
const [adminMembers, setAdminMembers] = useState([]);

// This function would be called when a user's role is changed
const handleRoleChange = (userId, newRole) => {
 // Perform the role change logic (API call, etc.)
 api.demoteUser(userId, newRole)
   .then(() => {
     // After demotion, update the member list
     refreshAdminList(); // Re-fetch the list of admins
   })
   .catch((error) => console.error("Error demoting user:", error));
};

// Fetch updated admin list after role change
const refreshAdminList = async () => {
 const updatedMembers = await api.getAdminMembers(); // Replace with actual API call
 setAdminMembers(updatedMembers);
};

// Effect to initialize admin members on mount
useEffect(() => {
 refreshAdminList();
}, []);

return (
 <div>
   <h3>Admin Members</h3>
   <ul>
     {adminMembers.map(member => (
       <li key={member.id}>{member.name}</li>
     ))}
   </ul>
 </div>
);

Use of WebSockets or Event Listeners for Real-Time Updates

// Event listener for role change
socket.on('roleChanged', (userId, newRole) => {
  if (newRole !== 'admin') {
    // Remove the user from the admin list UI
    setAdminMembers(prevMembers => prevMembers.filter(member => member.id !== userId));
  } else {
    // Add the user to the admin list UI
    fetchUpdatedAdminList();
  }
});

Triggering UI Refresh on Role Change (In Vanilla JavaScript or jQuery)

// Function to handle role change
function demoteUser(userId) {
  // Assuming a function that performs the role demotion (API call)
  api.demoteUser(userId)
    .then(() => {
      // Refresh the member list after demotion
      refreshAdminList();
    })
    .catch((error) => {
      console.error("Failed to demote user:", error);
    });
}

// Function to refresh the admin member list
function refreshAdminList() {
  api.getAdminMembers().then((updatedMembers) => {
    const adminListContainer = document.getElementById('adminList');
    adminListContainer.innerHTML = ''; // Clear current list

    updatedMembers.forEach(member => {
      const memberItem = document.createElement('li');
      memberItem.textContent = member.name;
      adminListContainer.appendChild(memberItem);
    });
  });
}

// Trigger UI refresh after role change
document.getElementById('demoteBtn').addEventListener('click', () => {
  const userId = getUserIdToDemote(); // Fetch the user ID to demote
  demoteUser(userId);
});

Handle Immediate Role Removal from UI (For Example with API Polling)

// Poll for updates every 5 seconds (or longer)
setInterval(async () => {
  const members = await api.getAdminMembers();
  updateAdminListUI(members);
}, 5000);

// Function to update the UI
function updateAdminListUI(members) {
  const listElement = document.getElementById('adminList');
  listElement.innerHTML = ''; // Clear current list
  
  members.forEach(member => {
    const li = document.createElement('li');
    li.textContent = member.name;
    listElement.appendChild(li);
  });
}

Backend Example (Node.js with Socket.IO):

socket.on('demoteUser', async (userId) => {
  try {
    // Perform the role demotion in your backend
    await demoteUserRole(userId);

    // Emit an event to notify all connected clients of the role change
    io.emit('roleChanged', userId, 'member'); // Replace with the actual role
  } catch (error) {
    console.error("Error demoting user:", error);
  }
});

[melvin-bot]

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

[strepanier03]

Repro:

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[allgandalf]

Will review tomorrow

[melvin-bot]

@strepanier03 @allgandalf this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[allgandalf]

okay @FitseTLT your solutions seems plausible, but before moving forward i would love to test it a bit, can you please share a test branch?

[FitseTLT]

Here it is @allgandalf

[allgandalf]

Thanks, i will test and get back

[melvin-bot]

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

[melvin-bot]

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

[FitseTLT]

@allgandalf Bump on this one and @mjasikowski Can you help turn the label back to weekly? Thx

[mjasikowski]

@FitseTLT done

[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 9.1.5-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix - Workspace - Non-admin is not instantly removed from #admins member list after being demoted #54792

If no regressions arise, payment will be issued on 2025-03-05. 🎊

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

• @FitseTLT requires payment automatic offer (Contributor)
• @allgandalf requires payment automatic offer (Reviewer)

[melvin-bot]

@allgandalf @strepanier03 @allgandalf The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[strepanier03]

Payment Summary

• $250 - @FitseTLT via Upwork automatic offer (Contributor)
• $250 - @allgandalf via Upwork automatic offer (Reviewer)

[strepanier03]

I've paid out and closed the contract for @FitseTLT.

@allgandalf I'll check for a reg test later today and pay out if I can.

[strepanier03]

Little bump @allgandalf - I'll check again tomorrow.

[allgandalf]

I will complete the checklist tomorrow

[allgandalf]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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: N/A we never optimistically added / removed in admins room

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: N/A

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

Precondition:

• N/A

Test:

1. Go to workspace settings > Members.
2. Invite a member and promote the member to admin.
3. Go to #admins room.
4. Click on the chat header > Members.
5. Note that the new admin is in the member list.
6. Go to workspace settings > Members.
7. Demote the admin to member.
8. Go to #admins room.
9. Click on the chat header > Members.

verify that : The non-admin should now be removed from #admins room and will not appear in member list in #admins room.

Do we agree 👍 or 👎

[melvin-bot]

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

[allgandalf]

only payment remains

[strepanier03]

Reg test created.

[strepanier03]

Finished paying and closed the contract. Closing GH now.