[C+ Checklist Needs Completion] [$500] Workspace - Unable to select members after adding them to the workspace

[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.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,Navigate to Settings > Profile > Contact Method > New Contact Method > Add Email
2, change the new email as the default contact
3, Copy the main email and remove it
4, Go to Settings > Workspace > Members > click on Invite> then paste the main email or search for the email used to create the account
5, Add the email, click on the checkbox, and observe that it remains unselected

Expected Result:

After changing the default email and removing the main contact email (used to create the Expensify account), adding the main contact address to the workspace as a member should either be selected or not allowed

Actual Result:

After changing the default email and removing the main contact email, attempting to select the main contact address as a member in the workspace by clicking on the checkbox is not work

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

Bug6347877_1705685205802.Contact.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01285e5476dd516a43
• Upwork Job ID: 1748410004851982336
• Last Price Increase: 2024-01-26
• Automatic offers:
• tienifr | Contributor | 0

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[lanitochka17]

We think that this bug might be related to #wave 8.
CC @zanyrenney

[tienifr]

Proposal

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

After changing the default email and removing the main contact email, attempting to select the main contact address as a member in the workspace by clicking on the checkbox is not work

What is the root cause of that problem?

In here, we still use the policy.owner, we should use policy.ownerAccountID instead.

When the user changes the default contact method, the policy.owner is currently not changed to the new default contact method. So when selecting member in WorkspaceMembersPage, it will be disabled because the login of the email, that the account is created from, is still the policy.owner.

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

Comparing logins is not a good way to check if 2 users are the same, we should compare by accountID instead, because 1 user can have multiple contact methods, but only have 1 single accountID.

We should compare accountID and policy.ownerAccountID here instead.

accountID === props.policy.ownerAccountID ||

We can also consider replacing the usage of policy.owner throughout that page (or elsewhere in the app) by policy.ownerAccountID. Some extra places and what we should do with them have been identified here. By the way, when creating the workspace, we need to set the ownerAccountID optimistically (currently we don't).

We've already moved to use accountID similarly throughout the app so we should do it here as well.

What alternative solutions did you explore? (Optional)

We can also fix by updating the policy.owner optimistically to the primary user's email when setting another contact method as default, and make sure it's also updated similarly in the back-end.

[tienifr]

Proposal updated

[greg-schroeder]

Awaiting proposal review @0xmiroslav

[melvin-bot]

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

[mallenexpensify]

@sobitneupane reassigning, please take over as C+. If you don't have bandwidth, unassign yourself. 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? 💸

[greg-schroeder]

@sobitneupane said he's working partially this week, let's try to get this one reviewed ASAP once you're back though!

[DylanDylann]

I can take over this issue as C+ contributor if @sobitneupane didn't start on it

[sobitneupane]

I am back and will review the proposal by EOD tomorrow.

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

• fix: Unable to select members after adding them to the workspace #36713

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

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

• @sobitneupane requires payment through NewDot Manual Requests
• @tienifr requires payment automatic offer (Contributor)

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

• [@sobitneupane] The PR that introduced the bug has been identified. Link to the PR:
• [@sobitneupane] 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:
• [@sobitneupane] 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:
• [@sobitneupane] Determine if we should create a regression test for this bug.
• [@sobitneupane] 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.
• [@greg-schroeder] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[tienifr]

@youssef-lr I think the PR fix on react-native-onyx here would be out of the original scope. Can I get the bonus for that fix? The same situation is in here cc @sobitneupane @greg-schroeder

[youssef-lr]

@tienifr I think $500 is still a fair price given that the original change was straightforward.

[greg-schroeder]

Processing

[greg-schroeder]

@sobitneupane $500 (manual request)
@tienifr $500 (paid)

Next up is the checklist!

[greg-schroeder]

@sobitneupane quick bump on the checklist so we can close this out!

[melvin-bot]

@youssef-lr, @sobitneupane, @greg-schroeder, @tienifr Whoops! This issue is 2 days overdue. Let's get this updated quick!

[youssef-lr]

Waiting on @sobitneupane

[sobitneupane]

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:

• [@sobitneupane] The PR that introduced the bug has been identified. Link to the PR:

I guess it was missed while refactoring to use accountID.

• [@sobitneupane] 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

• [@sobitneupane] 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 It is an edge case which could be easily missed. Regression Tests should be enough.

• [@sobitneupane] Determine if we should create a regression test for this bug.

The issue was reported by internal team so I am not sure if regression test is required.

• [@sobitneupane] 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.

#34828 (comment)

[sobitneupane]

Regression Test Proposal

1. Navigate to Settings > Profile > Contact Method > New Contact Method > Add Email
2. Change the new email as the default contact
3. Copy the main email and remove it
4. Go to Settings > Workspace > Members > click on Invite> then paste the main email or search for the email used to create the account
5. Add the email, click on the checkbox, and verify that it's selected

Do we agree 👍 or 👎

[sobitneupane]

Requested payment in newDot.

Payment Summary: #34828 (comment)

[JmillsExpensify]

$500 approved for @sobitneupane based on summary.

[greg-schroeder]

Filed test, closing