[$500] Manage members - Page is blinking after removing user from Workspace

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Launch the app
2. Go to Setting > Workspace > Manage members
3. Invite few members(if you haven't)
4. Delete the member

Expected Result:

Page is not blinking after removing user from Workspace

Actual Result:

Page is blinking after removing user from Workspace

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Desktop App
• Mobile Web

Version Number: 1.1.57.12

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers): any

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

ios.8689.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause

Slack conversation:

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @TomatoToaster (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[melvin-bot]

Triggered auto assignment to @adelekennedy (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[adelekennedy]

internal
external

[melvin-bot]

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

[melvin-bot]

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

[thesahindia]

Proposal

When we remove a member removeMembers function gets called and inside it we update the policy after removing the users using Onyx.set

App/src/libs/actions/Policy.js

Lines 296 to 302 in 572f6cb

	// Make a shallow copy to preserve original data and remove the members
	const policy = _.clone(allPolicies[key]);
	policy.employeeList = _.without(policy.employeeList, ...members);
	// Optimistically remove the members from the policy
	Onyx.set(key, policy);

We are doing the same thing again here. It's not needed and removing it will solve this.

App/src/libs/actions/Policy.js

Lines 519 to 527 in 572f6cb

	function subscribeToPolicyEvents() {
	_.each(allPolicies, (policy, key) => {
	const pusherChannelName = `public-policyEditor-${policy.id}${CONFIG.PUSHER.SUFFIX}`;
	Pusher.subscribe(pusherChannelName, 'policyEmployeeRemoved', ({removedEmails, policyExpenseChatIDs, defaultRoomChatIDs}) => {
	const policyWithoutEmployee = _.clone(policy);
	policyWithoutEmployee.employeeList = _.without(policy.employeeList, ...removedEmails);
	// Remove the members from the policy
	Onyx.set(key, policyWithoutEmployee);

I propose the following changes -

function subscribeToPolicyEvents() {
-    _.each(allPolicies, (policy, key) => {
+    _.each(allPolicies, (policy) => {
        const pusherChannelName = `public-policyEditor-${policy.id}${CONFIG.PUSHER.SUFFIX}`;
        Pusher.subscribe(pusherChannelName, 'policyEmployeeRemoved', ({removedEmails, policyExpenseChatIDs, defaultRoomChatIDs}) => {
-            const policyWithoutEmployee = _.clone(policy);
-            policyWithoutEmployee.employeeList = _.without(policy.employeeList, ...removedEmails);
-
-            // Remove the members from the policy
-            Onyx.set(key, policyWithoutEmployee);

            // Refetch the policy expense chats to update their state
            if (!_.isEmpty(policyExpenseChatIDs)) {
                Report.fetchChatReportsByIDs(policyExpenseChatIDs);
            }

[mananjadhav]

@thesahindia I went through your proposal. Based on what I see, policyEmployeeRemoved is an event subscribed via Pusher, so I am guessing it is to sync between devices. If we remove this, it might not sync.

[deetergp]

I agree with @mananjadhav — we're going to need that value in Onyx & for Pusher.

[deetergp]

Switching this back to Weekly since we're still open to more proposals.

[deetergp]

I missed adding the weekly label

[deetergp]

Also, bumping @adelekennedy to re-add the job in Upwork so @thesahindia can start work on it.

[adelekennedy]

reopened! @thesahindia do you mind applying here?

[thesahindia]

reopened! @thesahindia do you mind applying here?

Applied ✅

[mananjadhav]

@thesahindia Any updates for me on this one?

[thesahindia]

@mananjadhav, I have pushed the code now.

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[adelekennedy]

Still going through tests - not overdue

[mvtglobally]

Issue not reproducible during KI retests. (Second week)

[deetergp]

@adelekennedy It looks like this one is good to go and we should pay @thesahindia for their effort 👍

[adelekennedy]

@deetergp ty ty, I'll pay this out in 7 days!

[mananjadhav]

This hit production but the title wasn't updated

[mvtglobally]

Issue not reproducible during KI retests. (Third week)

[mananjadhav]

@adelekennedy Quick bump. The regression period is complete. PR was deployed 8 days back. For some reason title wasn't updated.

[mvtglobally]

Issue not reproducible during KI retests. (Fourth week) Are we good to close this?

[mananjadhav]

@mvtglobally Payments are pending for this one. We can close this out once that is through.

@adelekennedy quick reminder

[adelekennedy]

Hmm, do we know why the title didn't change? Happy to pay out but I'm concerned that something is broken here @deetergp

[adelekennedy]

No reporting bonus, C+: @mananjadhav, Contributor: @thesahindia

[thesahindia]

Bump for upwork.
cc: @adelekennedy

[mananjadhav]

bump @adelekennedy @deetergp for Upwork

[adelekennedy]

Strange - I'm not getting any notifications from this GH, I just sent the payment through with a bonus for waiting

[mananjadhav]

Thanks @adelekennedy for the quick TAT and bonus 🤑