[HOLD for payment 2024-06-13] [$250] Workspace - Missing options from settings if a WS is created on another device with same acc

[izarutskaya]

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.62.17
Reproducible in staging?: Y
Reproducible in production?: Y
Found when executing PR : https://github.com/Expensify/Expensify/issues/281020
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Device 1: Log in with a new Gmail account
2. Device 2: Log in with the same account
3. Device 1: Create a workspace
4. Device 2: Navigate to the workspace settings
5. Device 2: Click on members

Expected Result:

All settings should be available for the second device.

Actual Result:

Missing options from settings if a WS is created on another device with same account. The missing options are: "Categories" and "More features" from the settings tab, "Description", "Share" and "Delete" from the "Profile tab, "Invite members" from the "Members" tab. The settings are restored after relogging.

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

Bug6456581_1713710000435.bandicam_2024-04-21_16-13-20-916.mp4

Recording.2385.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01133e97d014430d38
• Upwork Job ID: 1784518575691853824
• Last Price Increase: 2024-05-05
• Automatic offers:
• eh2077 | Reviewer | 0

Issue Owner

Current Issue Owner: @

Issue Owner

Current Issue Owner: @laurenreidexpensify

[melvin-bot]

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

[izarutskaya]

We think this issue might be related to the #collect project.

[dragnoir]

Proposal

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

Missing options from settings if a WS is created on another device with same acc

What is the root cause of that problem?

The shouldShowProtectedItems should be true to display the missing menu items.

App/src/pages/workspace/WorkspaceInitialPage.tsx

Line 122 in 98d85e0

const shouldShowProtectedItems = PolicyUtils.isPolicyAdmin(policy);

On other devices, when policyDraft does not exist. The logic will use policyProp. The policy used here to determine if the user is ADMIN is missing the role. it's undefined.

App/src/libs/PolicyUtils.ts

Line 132 in 98d85e0

const isPolicyAdmin = (policy: OnyxEntry<Policy> | EmptyObject): boolean => policy?.role === CONST.POLICY.ROLE.ADMIN;

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

When policyDraft is not available and we are using policyProp,; we can add a backup check to isPolicyAdmin

We can use employeeList and check if the current user details has role admin
or from policy.employeeList

[laurenreidexpensify]

@trjExpensify - what you reckon - this wave-collect / a priority?

[trjExpensify]

Yeah, we should fix that. Very strange. It's treating the user on the second device like a mere member, not an admin. The majority of the workspace settings aren't visible to a member, only profile (read-only) and members list (without the ability to add anyone new).

[melvin-bot]

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

[melvin-bot]

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

[eh2077]

reviewing proposals

[eh2077]

@dragnoir 's proposal looks good to me. I agreed with their RCA and the solution. Implementation details can be discussed in PR.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[stitesExpensify]

Can you explain why we have 2 employeeList objects? It looks like they have the same information...?

[dragnoir]

Can you explain why we have 2 employeeList objects? It looks like they have the same information...?

I don't remember how I got the screenshot, I was debugging the data. But below a clean screenshot from console.log(policy)

just one employeeList object

[stitesExpensify]

Okay. Should we always look at the policy employee list for isAdmin? My concern here is that looking for something in 2 places seems like an anti-pattern. We should know where data is, and always look there. Ideally we should not have any duplicated data either

[dragnoir]

@stitesExpensify the PolicyDraft is introduced here #29256 to solve the issue

There is a delay of 1-2 seconds when a new workspace is created in the Android app.

with this proposal #16935 (comment)

For this issue, we can always use one source of information wish is Policy

[melvin-bot]

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

[laurenreidexpensify]

🎀👀🎀 C+ reviewed

[laurenreidexpensify]

@eh2077 can you do me a favour and repaste this message - #40686 (comment) - so we can trigger the automation to get another internal eng assigned to watch while @stitesExpensify is out thanks

[eh2077]

Sure

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[dragnoir]

PR ready #42008

[eh2077]

Still working on the PR

[rlinoz]

PR merged today, waiting for next deploy.

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

• Fix: ws options available from all devices for same user #42008

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

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

• @dragnoir requires payment (Needs manual offer from BZ)
• @eh2077 requires payment automatic offer (Reviewer)

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

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

[laurenreidexpensify]

Payment Summary -

• contributor @dragnoir paid in upwork $250
• C+ @eh2077 paid in upwork $250

@eh2077 please confirm the the checklist steps above thanks

[laurenreidexpensify]

@eh2077 please confirm the the checklist steps above s so we can close this out thank

[eh2077]

Checklist

• [@eh2077] The PR that introduced the bug has been identified. Link to the PR: I don't think there's a PR which is responsible for it. This is a missing edge case we choose to polish
• [@eh2077] 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
• [@eh2077] 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
• [@eh2077] Determine if we should create a regression test for this bug. No

[eh2077]

@laurenreidexpensify Checklist completed, thanks for the bump