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] Improve/fix logic for creating a workspace during onboarding flow #53326

Open
carlosmiceli opened this issue Nov 29, 2024 · 5 comments
Open
Assignees
Labels
Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Improvement Item broken or needs improvement. Overdue

Comments

@carlosmiceli
Copy link
Contributor

carlosmiceli commented Nov 29, 2024

We want new users that are taken to the Employee Count screen to have a workspace created automatically (this is currently in place):

image

However, for signups that join via expensify.com and select either vsb or smb as signup qualifier, we want to skip actually creating a workspace at this step because that's going to be created automatically via the API during the account creation. Otherwise we'll be creating one twice. These are the corresponding vsb and smb signup options on expensify.com:

Screenshot 2024-11-29 at 4 03 39 PM

To clarify, the current onboarding flow logic for creating a workspace should still occur for any new user that's taken to the onboarding modal and does NOT have either vsb or smb as a signupQualifier. Those would be:

  • a sign-up on new.expensify.com on web/mWeb
  • a sign-up on the New Expensify mobile app
  • a sign-up on the native Expensify mobile app

Please include videos of all the possible signup cases and confirming that only one workspace is created for each. Also confirm that this bug where we created duplicate workspaces doesn't reoccur (if it's still happening). Finally, we think there may be an extreme edge case where:

  • User downloaded old version of the app
  • Signs up via web
  • Gets deep linked into the app
  • App is on old version
  • Somehow gets two workspaces.

Please include in your proposal a way to prevent this if it could occur.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021862573059516200142
  • Upwork Job ID: 1862573059516200142
  • Last Price Increase: 2024-11-29
Issue OwnerCurrent Issue Owner: @thesahindia
@carlosmiceli carlosmiceli added External Added to denote the issue can be worked on by a contributor Daily KSv2 Improvement Item broken or needs improvement. labels Nov 29, 2024
@carlosmiceli carlosmiceli self-assigned this Nov 29, 2024
@melvin-bot melvin-bot bot changed the title Improve/fix logic for creating a workspace during onboarding flow [$250] Improve/fix logic for creating a workspace during onboarding flow Nov 29, 2024
Copy link

melvin-bot bot commented Nov 29, 2024

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

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

melvin-bot bot commented Nov 29, 2024

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

@nkdengineer
Copy link
Contributor

Proposal

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

Improve/fix logic for creating a workspace during onboarding flow

What is the root cause of that problem?

This is an improvement

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

Instead of creating the policy in the employee step or auto-create a new workspace for vsb onboarding, we can only create the new workspace when the user completes the final step.

  1. We can remove the create a new workspace logic here and here

  2. In here, if the onboardingPurposeSelected is newDotManageTeam, create a new workspace via createWorkspace and then get adminsChatReportID, policyID and pass it to completeOnboarding function

let onboardingAdminsChatReportID; let onboardingPolicyID;
if (onboardingPurposeSelected === CONST.ONBOARDING_CHOICES.MANAGE_TEAM) {
    const {adminsChatReportID, policyID} = Policy.createWorkspace(undefined, true, '', Policy.generatePolicyID(), CONST.ONBOARDING_CHOICES.MANAGE_TEAM);
    onboardingAdminsChatReportID =  adminsChatReportID;
    onboardingPolicyID = policyID;
};
Report.completeOnboarding(
    onboardingPurposeSelected,
    CONST.ONBOARDING_MESSAGES[onboardingPurposeSelected],
    undefined,
    undefined,
    onboardingAdminsChatReportID ?? undefined,
    onboardingPolicyID,
    undefined,
    onboardingCompanySize,
    userReportedIntegration,
);

That can make sure the workspace is only created when we complete the onboarding flow with onboardingPurposeSelected as newDotManageTeam

What alternative solutions did you explore? (Optional)

In completeOnboarding function, we can build the optimistic data for new workspace via buildPolicyData function and then add these onyx data in completeOnboarding and from the backend, we will use the onboardingPolicyID to create a new workspace.

@Shahidullah-Muffakir
Copy link
Contributor

Shahidullah-Muffakir commented Nov 29, 2024

Proposal

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

If user completed the onboarding flow using OD, two workspaces are created.

What is the root cause of that problem?

  1. if signupQualifier is VSB, we are not showing How many employees do you have? or OnboardingEmployees page, hence this useEffect is added to automatically create a workspace for it.
    // If the signupQualifier is VSB, the company size step is skip.
    // So we need to create the new workspace in the accounting step
    useEffect(() => {
    const filteredPolicies = Object.values(allPolicies ?? {}).filter(PolicyUtils.isPaidGroupPolicy);
    if (!isVsb || filteredPolicies.length > 0 || isLoadingOnyxValue(allPoliciesResult)) {
    return;
    }
    const {adminsChatReportID, policyID} = Policy.createWorkspace(undefined, true, '', Policy.generatePolicyID(), CONST.ONBOARDING_CHOICES.MANAGE_TEAM);
    Welcome.setOnboardingAdminsChatReportID(adminsChatReportID);
    Welcome.setOnboardingPolicyID(policyID);
    }, [isVsb, allPolicies, allPoliciesResult]);
  2. if signupQualifier is SMB, it is not considered anywhere in code to skip the workspace creation, workspace is created in any case here.
    if (!onboardingPolicyID) {
    const {adminsChatReportID, policyID} = Policy.createWorkspace(undefined, true, '', Policy.generatePolicyID(), CONST.ONBOARDING_CHOICES.MANAGE_TEAM);
    Welcome.setOnboardingAdminsChatReportID(adminsChatReportID);
    Welcome.setOnboardingPolicyID(policyID);
    }

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

  1. remove this useEffect, because it is used to create a workspace if signupQualifier is VSB

    // If the signupQualifier is VSB, the company size step is skip.
    // So we need to create the new workspace in the accounting step
    useEffect(() => {
    const filteredPolicies = Object.values(allPolicies ?? {}).filter(PolicyUtils.isPaidGroupPolicy);
    if (!isVsb || filteredPolicies.length > 0 || isLoadingOnyxValue(allPoliciesResult)) {
    return;
    }
    const {adminsChatReportID, policyID} = Policy.createWorkspace(undefined, true, '', Policy.generatePolicyID(), CONST.ONBOARDING_CHOICES.MANAGE_TEAM);
    Welcome.setOnboardingAdminsChatReportID(adminsChatReportID);
    Welcome.setOnboardingPolicyID(policyID);
    }, [isVsb, allPolicies, allPoliciesResult]);

  2. add one more condition here in BaseOnboardingEmployees, if the signupQualifier is SMB, then don't create the workspace.
    const isSmb = onboardingValues && 'signupQualifier' in onboardingValues && onboardingValues.signupQualifier === CONST.ONBOARDING_SIGNUP_QUALIFIERS.SMB;

    if (!onboardingPolicyID) {
    const {adminsChatReportID, policyID} = Policy.createWorkspace(undefined, true, '', Policy.generatePolicyID(), CONST.ONBOARDING_CHOICES.MANAGE_TEAM);
    Welcome.setOnboardingAdminsChatReportID(adminsChatReportID);
    Welcome.setOnboardingPolicyID(policyID);
    }

as :
if (!onboardingPolicyID && !isSmb) {

  1. and to cover the edge case, we can add the following conditions in above line:
const filteredPolicies = Object.values(allPolicies ?? {}).filter(PolicyUtils.isPaidGroupPolicy);

as:

                  if (!onboardingPolicyID && !isSmb && filteredPolicies.length <=0 ) {

@danielrvidal
Copy link
Contributor

@thesahindia any thoughts on which proposal to go with?

@melvin-bot melvin-bot bot added the Overdue label Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Improvement Item broken or needs improvement. Overdue
Projects
None yet
Development

No branches or pull requests

5 participants