[HOLD for payment 2024-03-26] [$500] Members - After removing the error of adding a member to WS, the red dot remains in the RHN

[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!

---

Version Number: 1.4.28-0
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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal team
Slack conversation:

Action Performed:

1. Open https://staging.new.expensify.com/
2. Log in under your HT account
3. Create a new WS
4. Navigate to the WS settings
5. Click the Members tab
6. Invite a user with a VOIP or landline
7. If required, delete the add Members error message
8. Return to the WS settings menu

Expected Result:

After removing the error of adding a member to WS, the red dot should disappear

Actual Result:

After removing the error of adding a member to WS, the red dot remains in the RHN

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

Bug6348407_1705733236808.Recording__1121.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015688669ede8e962b
• Upwork Job ID: 1748730789626449920
• Last Price Increase: 2024-02-28

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[kbecciv]

We think that this bug might be related to #wave6-collect-submitters
CC @greg-schroeder

[bernhardoj]

Proposal

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

When adding a member that will gives an error, the member is hidden from the list.

What is the root cause of that problem?

The member will not be included in the list if the personal detail is missing.

App/src/pages/workspace/WorkspaceMembersPage.js

Lines 318 to 321 in 52adf77

	if (!details) {
	Log.hmmm(`[WorkspaceMembersPage] no personal details found for policy member with accountID: ${accountID}`);
	return;
	}

When we add a new member, we will add a new optimistic personal detail.

App/src/libs/PersonalDetailsUtils.ts

Lines 95 to 138 in 52adf77

	function getNewPersonalDetailsOnyxData(logins: string[], accountIDs: number[]): Required<Pick<OnyxData, 'optimisticData' | 'finallyData'>> {
	const personalDetailsNew: PersonalDetailsList = {};
	const personalDetailsCleanup: PersonalDetailsList = {};
	logins.forEach((login, index) => {
	const accountID = accountIDs[index];
	if (allPersonalDetails && Object.keys(allPersonalDetails?.[accountID] ?? {}).length === 0) {
	personalDetailsNew[accountID] = {
	login,
	accountID,
	avatar: UserUtils.getDefaultAvatarURL(accountID),
	displayName: LocalePhoneNumber.formatPhoneNumber(login),
	};
	/**
	* Cleanup the optimistic user to ensure it does not permanently persist.
	* This is done to prevent duplicate entries (upon success) since the BE will return other personal details with the correct account IDs.
	*/
	personalDetailsCleanup[accountID] = null;
	}
	});
	const optimisticData: OnyxUpdate[] = [
	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: ONYXKEYS.PERSONAL_DETAILS_LIST,
	value: personalDetailsNew,
	},
	];
	const finallyData: OnyxUpdate[] = [
	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: ONYXKEYS.PERSONAL_DETAILS_LIST,
	value: personalDetailsCleanup,
	},
	];
	return {
	optimisticData,
	finallyData,
	};
	}

And we add a finallyData that will be merged whether it fails or success to clear the optimistic personal detail. When fail, we lost the optimistic personal detail, thus the member is hidden.

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

Don't use. finally data to clear the optimistic personal detail, but use success data

then we will update the usage of getNewPersonalDetailsOnyxData in both addMembersToWorkspace and inviteToRoom

[Tony-MK]

Proposal

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

Members - After removing the error of adding a member to WS, the red dot remains in the RHN

What is the root cause of that problem?

The purpose of finallyData is to clean up the newPersonalDetailsOnyxData set in the ${ONYXKEYS.COLLECTION.PERSONAL_DETAILS_LIST} collection because it might cause double personalDetails to be stored since the backend will either set nothing or new realistic data required for each new member.

App/src/libs/PersonalDetailsUtils.ts

Lines 110 to 114 in 52adf77

	/**
	* Cleanup the optimistic user to ensure it does not permanently persist.
	* This is done to prevent duplicate entries (upon success) since the BE will return other personal details with the correct account IDs.
	*/
	personalDetailsCleanup[accountID] = null;

The addMembersToWorkspace function creates the optimisticMembersState and failureMembersState in the ${ONYXKEYS.COLLECTION.POLICY_MEMBERS}${policyID} collection.

When a single member is non-existent, the onyxData response is an empty array, but the failureMembersState is set while the personalDetails is deleted.

App/src/libs/actions/Policy.ts

Lines 662 to 664 in 52adf77

	// Convert to object with each key containing the error. We don’t
	// need to remove the members since that is handled by onClose of OfflineWithFeedback.
	value: failureMembersState,

Hence, failureMemberState is not deleted because it is expected to be deleted by the OfflineFeedback's onClose function when the admin dismisses the error.

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

Amend the failureMembersState to make sure that errors are not created and the pendingAction, from the optimisticMembersState, is erased from the ${ONYXKEYS.COLLECTION.POLICY_MEMBERS}${policyID} collection once the response from the AddMembersToWorkspace endpoint arrives.

App/src/libs/actions/Policy.ts

Lines 617 to 619 in 52adf77

	failureMembersState[accountID] = {
	errors: ErrorUtils.getMicroSecondOnyxError('workspace.people.error.genericAdd'),
	};

Therefore, we should change the line above to be similar to the one below.

failureMembersState[accountID] = null;

Result

Result.mov

Note

I am theorizing that this is the correct solution since when testing the AddMembersToWorkspace endpoint by inviting a combination of existent and non-existent members in one request, the backend returned the same 404 response.

Hence, could I get confirmation of this behavior to rule out this solution?

[rushatgabhane]

Isn't the issue that the error message isn't shown? So you can't dismiss it.

[bernhardoj]

@rushatgabhane the member itself isn't shown, so the error message too.

[melvin-bot]

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

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[melvin-bot]

@rushatgabhane, @muttmuure Huh... This is 4 days overdue. Who can take care of this?

[muttmuure]

Trying to reproduce now

[melvin-bot]

@rushatgabhane @muttmuure 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!

[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]

@rushatgabhane, @muttmuure Eep! 4 days overdue now. Issues have feelings too...

[muttmuure]

Unable to reproduce this

[bernhardoj]

@muttmuure It's still reproducible. Notice the failed member is removed from the list but the red dot on the members menu is still showing.

Screen.Recording.2024-02-06.at.01.21.09.mov

[bernhardoj]

PR is ready

cc: @rushatgabhane

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

• Fix a failed-to-add workspace member is hidden #37895

If no regressions arise, payment will be issued on 2024-03-26. 🎊

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

• @rushatgabhane requires payment through NewDot Manual Requests
• @bernhardoj requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@rushatgabhane] The PR that introduced the bug has been identified. Link to the PR:
• [@rushatgabhane] 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:
• [@rushatgabhane] A discussion in #expensify-bugs 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:
• [@rushatgabhane] Determine if we should create a regression test for this bug.
• [@rushatgabhane] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@muttmuure] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[rushatgabhane]

1. The PR that introduced the bug has been identified. Link to the PR: refactor: SelectionList multiple selection #22622

2. 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: 717e22b#r140246790

3. A discussion in #expensify-bugs 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.

4. Determine if we should create a regression test for this bug. No

5. If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again - No

[rushatgabhane]

@muttmuure could you please attach payment summary, thank you 🙇

[muttmuure]

$500 for @rushatgabhane as C+
$500 for @bernhardoj as C

[JmillsExpensify]

$500 approved for @rushatgabhane

[bernhardoj]

@muttmuure hi, I think I haven't received the payment yet. As mentioned by melvin here, the previous offer has an error, but I haven't received a manual offer yet. Can you please recheck it, please?

[muttmuure]

Can you see this offer? https://www.upwork.com/nx/wm/offer/101695651

[bernhardoj]

Yes, I can see that one and accepted it.

[muttmuure]

All paid up!

[bernhardoj]

Thanks!