Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$250] The "new workspace" button is shown in Global Create to accounts already on workspaces #47567

Closed
1 of 6 tasks
m-natarajan opened this issue Aug 16, 2024 · 17 comments
Closed
1 of 6 tasks
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@m-natarajan
Copy link

m-natarajan commented Aug 16, 2024

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.21-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: @puneetlath
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1723669119268099

Action Performed:

  1. Invite user A to user B workspace
  2. Login as User B
  3. Click on global +

Expected Result:

Create new workspace should not appear

Actual Result:

New workspace option appears

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

image (8)

Snip - (2) New Expensify - Google Chrome

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~017c30fdb0f2028e60
  • Upwork Job ID: 1824549832586983861
  • Last Price Increase: 2024-08-16
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 16, 2024
Copy link

melvin-bot bot commented Aug 16, 2024

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

@daledah
Copy link
Contributor

daledah commented Aug 16, 2024

Proposal

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

We're showing the "new workspace" button in Global Create to accounts that are already on workspaces

What is the root cause of that problem?

Currently, we'll show the "new workspace" button in Global Create when the user has any active free policies (aka workspaces)

In function hasActiveChatEnabledPolicies we only check with admin role

const adminChatEnabledPolicies = Object.values(policies ?? {}).filter(
(policy) =>
policy &&
((policy.type === CONST.POLICY.TYPE.FREE && policy.role === CONST.POLICY.ROLE.ADMIN) ||
(!includeOnlyFreePolicies && policy.type !== CONST.POLICY.TYPE.PERSONAL && policy.role === CONST.POLICY.ROLE.ADMIN && policy.isPolicyExpenseChatEnabled)),
);

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

  1. We add new param shouldExcludeAdmin with default is false to function hasActiveChatEnabledPolicies
    const adminChatEnabledPolicies = Object.values(policies ?? {}).filter(
        (policy) =>
            policy &&
            ((policy.type === CONST.POLICY.TYPE.FREE && policy.role === CONST.POLICY.ROLE.ADMIN) ||
                (!includeOnlyFreePolicies &&
                    policy.type !== CONST.POLICY.TYPE.PERSONAL &&
+                  (!shouldExcludeAdmin ? policy.role === CONST.POLICY.ROLE.ADMIN : true) &&
                    policy.isPolicyExpenseChatEnabled)),
    );
  1. Pass param shouldExcludeAdmin = true in this condition

...(!isLoading && !Policy.hasActiveChatEnabledPolicies(allPolicies)

What alternative solutions did you explore? (Optional)

We'll write a new function to check the user was in a workspace that was not of type personal.

 Object.values(policies ?? {}).filter((policy) => {
            return policy?.type !== CONST.POLICY.TYPE.PERSONAL;
        }).length !== 0

@sakluger
Copy link
Contributor

Before I add the external label, I'm confirming the expected behavior in the original bug report thread in Slack.

@sakluger sakluger changed the title "new workspace" button in Global Create to accounts that are already on workspaces The "new workspace" button is shown in Global Create to accounts already on workspaces Aug 16, 2024
@sakluger sakluger added the External Added to denote the issue can be worked on by a contributor label Aug 16, 2024
@melvin-bot melvin-bot bot changed the title The "new workspace" button is shown in Global Create to accounts already on workspaces [$250] The "new workspace" button is shown in Global Create to accounts already on workspaces Aug 16, 2024
Copy link

melvin-bot bot commented Aug 16, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 16, 2024
Copy link

melvin-bot bot commented Aug 16, 2024

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

@thesahindia
Copy link
Member

@daledah's proposal looks good to me!

🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented Aug 17, 2024

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

@arosiclair
Copy link
Contributor

@daledah @thesahindia

We should also remove the includeOnlyFreePolcies param and policy.type === CONST.POLICY.TYPE.FREE since there are no free policies anymore. Then let's use a new includeOnlyAdminPolicies param that's false by default. So the new initial condition should just be

const chatEnabledPolicies = Object.values(policies).filter((policy) => policy?.isPolicyExpenseChatEnabled && (!includeOnlyAdminPolicies || policy.role === CONST.POLICY.ROLE.ADMIN));

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 19, 2024
Copy link

melvin-bot bot commented Aug 19, 2024

📣 @daledah You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

@daledah
Copy link
Contributor

daledah commented Aug 20, 2024

@thesahindia This PR is ready for review.

@sakluger
Copy link
Contributor

The automation didn't work, but this is on prod! Payment was due around Sept 2.

@sakluger sakluger removed Reviewing Has a PR in review Weekly KSv2 labels Sep 10, 2024
@sakluger sakluger added Daily KSv2 Awaiting Payment Auto-added when associated PR is deployed to production labels Sep 10, 2024
@sakluger
Copy link
Contributor

Summarizing payment on this issue:

Contributor: @daledah $250, sent offer via Upwork: https://www.upwork.com/nx/wm/offer/103905329
Contributor+: @thesahindia $250, please request on Newdot

@melvin-bot melvin-bot bot removed the Overdue label Sep 10, 2024
@sakluger
Copy link
Contributor

@thesahindia could you please let me know if you think we need regression test steps here?

Copy link

melvin-bot bot commented Sep 13, 2024

@sakluger @arosiclair @thesahindia @daledah this issue is now 4 weeks old, please consider:

  • Finding a contributor to fix the bug
  • Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
  • If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

@melvin-bot melvin-bot bot added the Overdue label Sep 13, 2024
@thesahindia
Copy link
Member

Sorry for the delay. I missed the notification on this.

Here are the test steps:-

  1. As user "A" add user "B" to your workspace.
  2. Login as user "B"
  3. Click on FAB icon
  4. Verify that "New workspace" option doesn't appear in the menu

@melvin-bot melvin-bot bot added Daily KSv2 and removed Overdue Daily KSv2 labels Sep 15, 2024
@sakluger
Copy link
Contributor

Thanks!

I think we're all good here!

@garrettmknight
Copy link
Contributor

$250 approved for @thesahindia

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
Status: Done
Development

No branches or pull requests

6 participants