Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.53k stars 2.88k forks source link

[$250] Remove the `New Workspace` option from Global Create for members of existing group workspace. #51616

Open JmillsExpensify opened 1 week ago

JmillsExpensify commented 1 week ago

Proposal: Remove the New Workspace option from Global Create for members of existing group workspace.

Background: Thousands of existing OldDot members land in NewDot on a daily basis, whether to interact with services like Guides, Account Managers (AMs), or Concierge. Once in NewDot, Full Story sessions reveal that members commonly explore our new product and the features available.

Problem: When submit employees that already belong to a existing group workspace see the New Workspace option, they unintentionally create more workspaces, which causes confusion and prevents them from submitting expenses to their employer.

Solution: Remove the New Workspace option from Global Create for all members that already belong to a group workspace – streamlining their experience by reducing confusion and preventing accidental workspace creation. Additionally, we’ll universally promote a single way to create your second, third, fourth or an infinite number of workspaces via one centralized flow in Settings > Workspaces.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021851027135757622934
  • Upwork Job ID: 1851027135757622934
  • Last Price Increase: 2024-10-28
  • Automatic offers:
    • nkdengineer | Contributor | 104766386
Issue OwnerCurrent Issue Owner: @sobitneupane
melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

Current assignee @JmillsExpensify is eligible for the NewFeature assigner, not assigning anyone new.

Krishna2323 commented 1 week ago

@JmillsExpensify, is there any way to reproduce this with a new email?

NJ-2020 commented 1 week ago

Edited by proposal-police: This proposal was edited at 2024-11-05 23:01:24 UTC.

Proposal

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

Remove the New Workspace option from Global Create for members of existing group workspace

What is the root cause of that problem?

New feature We only hide the New workspace button from Global action if the user has any active enabled chat policies https://github.com/Expensify/App/blob/db592c9c9f56a6ce8208b898c1e5ba1eb864aae8/src/libs/actions/Policy/Policy.ts#L249-L255

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

Based on this https://github.com/Expensify/App/issues/51616#issuecomment-2458322645 , we can check if the policy type is team or corporate then we will simply return true

const chatEnabledPolicies = Object.values(policies ?? {}).filter(
    (policy) => policy?.type === CONST.POLICY.TYPE.TEAM || policy?.type === CONST.POLICY.TYPE.CORPORATE && (!includeOnlyAdminPolicies || policy.role === CONST.POLICY.ROLE.ADMIN),
);

Or we can also do this

const chatEnabledPolicies = Object.values(policies ?? {}).filter(
    (policy) => policy?.type === CONST.POLICY.TYPE.TEAM || policy?.type === CONST.POLICY.TYPE.CORPORATE,
);

What alternative solutions did you explore? (Optional)

We can check if the policy type is not personal then we will return true

const chatEnabledPolicies = Object.values(policies ?? {}).filter(
    (policy) => policy?.type !== CONST.POLICY.TYPE.PERSONAL,
);
JmillsExpensify commented 1 week ago

@sobitneupane Do you mind reviewing this proposal when you're able? This is causing quite a bit of confusion for existing members, I'd like to get ahead of it.

nkdengineer commented 1 week ago

@JmillsExpensify Is there any way to reproduce this bug for the contributor account?

sobitneupane commented 1 week ago

I could not reproduce the issue. @NJ-2020 Were you able to reproduce the issue? Can you please add reproduction steps?

NJ-2020 commented 1 week ago

@sobitneupane Sure here's the reproduction steps

  1. Create two accounts on OD, A and B
  2. A: Create new workspace with control plan
  3. A: Invite B as a employee member
  4. B: Click support > concierge
  5. B: Inside ND click + button

https://github.com/user-attachments/assets/7285b84a-da4e-4575-b3b3-119660e116c5

sobitneupane commented 1 week ago

Thanks, @NJ-2020.

If I'm not mistaken, there should also be a #announce room and a workspace chat room when the user is redirected to newDot. It seems like these are missing, which might be related.

Screenshot 2024-10-30 at 19 38 03
NJ-2020 commented 1 week ago

I am not sure here, but if we create new workspace with Control plan the BE returning isPolicyExpenseChatEnabled false

But if we create new workspace with Collect plan the BE returns isPolicyExpenseChatEnabled true Screenshot 2024-10-30 at 06 57 19

sobitneupane commented 1 week ago

@NJ-2020 I don't think that's the case when the user is invited from newDot.

Screenshot 2024-10-30 at 20 44 59
JmillsExpensify commented 1 week ago

Yes this flow is possible when a member doesn't have a workspace chats enabled workspace, but signed into NewDot. So generally you'd create a workspace on OldDot via user A, then invite user B. Sign in as user B to NewDot and you should see the option to create a New Workspace.

nkdengineer commented 1 week ago

Proposal

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

Remove the New Workspace option from Global Create for members of the existing group workspace

What is the root cause of that problem?

When we create a control WS in OldDot, isPolicyExpenseChatEnabled is false so there are no WS that match the condition here

https://github.com/Expensify/App/blob/780a236fb9d1bd55790eaf22b1e6ce0a3ba1bfe2/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.tsx#L457

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

If we want to not display the New Workspace option when the user already has a WS even isPolicyExpenseChatEnabled is false. To do that we only need to filter out the personal WS here

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

https://github.com/Expensify/App/blob/66195ad78fd1fab200ee82e2d530ec38e2947587/src/libs/actions/Policy/Policy.ts#L255-L258

What alternative solutions did you explore? (Optional)

sobitneupane commented 1 week ago

@nkdengineer Is there any reason for isPolicyExpenseChatEnabled to be false? What do you think about missing policy expense chats?

sobitneupane commented 1 week ago

I believe we should set isPolicyExpenseChatEnabled to true even when the policy is created and the user is invited from oldDot, as we want to display the policy expense chat. If there's no specific reason for it to be set to false, we should first check this on the backend.

🎀 👀 🎀 C+ reviewed

melvin-bot[bot] commented 1 week ago

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

nkdengineer commented 1 week ago

@nkdengineer Is there any reason for isPolicyExpenseChatEnabled to be false? What do you think about https://github.com/Expensify/App/issues/51616#issuecomment-2447236475

The policy expense chats are missing because isPolicyExpenseChatEnabled is false. About the reason why it's false, I think it's expected from OldDot. In the past when we create a WS in OldDot we need do some action to active it in NewDot here

 p = Policy.getCurrent();
p.policy.isPolicyExpenseChatEnabled = "true";
p.save();
JmillsExpensify commented 3 days ago

Agree with @nkdengineer. We're setting isPolicyExpenseChatEnabled to false by design, as the backend doesn't support the majority of OldDot workspaces. Further BE performance optimizations are needed, and in addition, we have to migrate customer data. So we shouldn't be setting isPolicyExpenseChatEnabled to true but instead respect whatever it's currently set to.

Julesssss commented 3 days ago

@sobitneupane any final thoughts on @nkdengineer's clarifications?

JmillsExpensify commented 2 days ago

Can we pick up the pace on this issue? This is one of the biggest sources of customer dissatisfaction, so we've got to get a PR merged so we can resolve it.

sobitneupane commented 2 days ago

We can proceed with @nkdengineer's proposal for now to resolve this ASAP, but the issue mentioned in https://github.com/Expensify/App/issues/51616#issuecomment-2447236475 will still persist.

cc: @Julesssss

nkdengineer commented 2 days ago

@sobitneupane For the announce room now, the WS should have at least 4 members to display the announce room.

sobitneupane commented 2 days ago

@nkdengineer If there are more than 4 members, is the #announce room displayed for the workspace created in the old dot? Is the workspace chat also displayed?

nkdengineer commented 2 days ago

I think the policy expense chat only displays when isPolicyExpenseChatEnabled is true.

JmillsExpensify commented 2 days ago

That's correct, for both the policy expense chat and the announce room.

Julesssss commented 2 days ago

Alright, so we all agree that when isPolicyExpenseChatEnabled is true for the OldDot workspaces the button won't be displayed.

To do that we only need to filter out the personal WS here

Though I'm not sure I agree with the solution yet. If the problem is that members of an OldDot submit workspace are being excluded from the filter, filtering out the personal workspaces wouldn't solve that case, right?

My understanding is that we'll need to set isPolicyExpenseChatEnabled correctly on the backend for the OldDot workspaces that are currently not marked true.

Also this should be independent of whether the chats are showing (as this simply depends on the amount of users in the workspace).

twilight2294 commented 2 days ago

Proposal

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

Remove the New Workspace option from Global Create for members of existing group workspace.

What is the root cause of that problem?

Feature request

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

We should update the below check to valdiate against the shouldShowCreateWorkspace like we do in the workspace switcher page: https://github.com/Expensify/App/blob/3d0f309b5ebc4f5bfd1279949c6eddca663e42a4/src/pages/WorkspaceSwitcherPage/index.tsx#L149

    const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY);
    const usersWorkspaces = useMemo<WorkspaceListItem[]>(() => {
        if (!policies || isEmptyObject(policies)) {
            return [];
        }
    const shouldShowCreateWorkspace = usersWorkspaces.length === 0;

And then use the shouldShowCreateWorkspace check below to hide the button if the user already has existing group workspace
https://github.com/Expensify/App/blob/780a236fb9d1bd55790eaf22b1e6ce0a3ba1bfe2/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.tsx#L457

What alternative solutions did you explore? (Optional)

nkdengineer commented 2 days ago

@Julesssss

https://github.com/Expensify/App/blob/b9149456da0d57536587e8cfa30ec207cf96f6fe/src/libs/PolicyUtils.ts#L189-L194

https://github.com/Expensify/App/blob/66195ad78fd1fab200ee82e2d530ec38e2947587/src/libs/actions/Policy/Policy.ts#L271

https://github.com/Expensify/App/blob/780a236fb9d1bd55790eaf22b1e6ce0a3ba1bfe2/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.tsx#L457

JmillsExpensify commented 1 day ago

That's a good point @Julesssss. Why wouldn't we simply look for whether the user is a member of any Team or Corporate type policies?

NJ-2020 commented 1 day ago

Proposal Updated

nkdengineer commented 1 day ago

That's a good point @Julesssss. Why wouldn't we simply look for whether the user is a member of any Team or Corporate type policies?

That is what shouldShowPolicy doing

twilight2294 commented 1 day ago

@sobitneupane can you please review my proposal

Julesssss commented 1 day ago

I'm just noting here that the issue only seems to be reproducible if you open the concierge chat before signing into NewDot. While I continue to test out the proposals, does anyone have a theory for why that is? Update: Actually I cannot reproduce locally at all

Of course, we will still move forward, but it seems odd, and I'd like to fully understand the underlying issue.

Screenshot 2024-11-06 at 09 28 39 Screenshot 2024-11-06 at 09 28 56
nkdengineer commented 1 day ago

@Julesssss The issue will happen if the user is invited to a WS that is created in OldDot or created a WS in OldDot (which has isPolicyExpenseChatEnabled is false). I think in this case above, you selected Manage my team's expense that will create a new WS in NewDot with isPolicyExpenseChatEnabled as true then New Worksapce button doesn't appear because hasActiveChatEnabledPolicies function is filtering the WS with isPolicyExpenseChatEnabled as true.

https://github.com/Expensify/App/blob/66195ad78fd1fab200ee82e2d530ec38e2947587/src/libs/actions/Policy/Policy.ts#L254-L258

If you sign up in NewDot and don't select Manage my team's expense in the onboarding flow then another user invited you to a WS that is created in OldDot, you can also reproduce this bug.

Julesssss commented 1 day ago
  • The issue will happen if the user is invited to a WS that is created in OldDot or created a WS in OldDot
  • I think in this case above, you selected Manage my team's expense

Thanks. I have been creating both my users and workspaces in OldDot and I also have never selected 'Manage my team's expense' though, so there must be another step I am missing 😕

Are you using private or public domain users?

nkdengineer commented 1 day ago

I'm using outlook email.

nkdengineer commented 1 day ago

Can you share the full video then we can see what is happening.

Julesssss commented 1 day ago

If you sign up in NewDot and don't select Manage my team's expense in the onboarding flow then another user invited you to a WS that is created in OldDot, you can also reproduce this bug.

This also doesn't work for me. Would you mind listing out the full steps, because I am completely unable to reproduce currently (examples below)

I think in this case above, you selected Manage my team's expense that will create a new WS in NewDot with isPolicyExpenseChatEnabled as true

So isPolicyExpenseChatEnabled was true for me when debugging, but this condition is what caused the button to not display (an undefined action), so perhaps that explains our different results?

https://github.com/user-attachments/assets/d8e5163d-2dab-4d12-a6e8-3a3ac445f7c1

https://github.com/user-attachments/assets/dc6c9b17-7df1-4683-87c7-1b1e2ecfd21c

nkdengineer commented 1 day ago

@Julesssss You need to create a new WS like this to reproduce.

https://github.com/user-attachments/assets/f164aead-9b85-4033-806b-59308a759667

Julesssss commented 1 day ago

Thank you. Wow that is an annoying gotcha 😩😭

Okay great, I am totally on board with your proposal. Your solution was written way before than @NJ-2020 and @twilight2294's proposals.

melvin-bot[bot] commented 1 day ago

📣 @nkdengineer 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

JmillsExpensify commented 1 day ago

Yeah, wow thank you @nkdengineer for clearing this up! Apologies I didn't make this more clear from the beginning.