Open carlosmiceli opened 5 days ago
Triggered auto assignment to @greg-schroeder (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.
Job added to Upwork: https://www.upwork.com/jobs/~021851663004588841867
Triggered auto assignment to Contributor-plus team member for initial proposal review - @jayeshmangwani (External
)
Current assignee @greg-schroeder is eligible for the Bug assigner, not assigning anyone new.
if user selected company size in the OD, we are still showing the company size selecting modal in the onboarding flow
improvement
change this: https://github.com/Expensify/App/blob/084b1047e8f090a63eb4eb99ab89d13f267bbe8b/src/libs/actions/Welcome/OnboardingFlow.ts#L58C4-L61C6 to this:
if (isVsb) {
Onyx.set(ONYXKEYS.ONBOARDING_PURPOSE_SELECTED, CONST.ONBOARDING_CHOICES.MANAGE_TEAM);
Welcome.setOnboardingCompanySize(onboardingCompanySize.MICRO);
return `/${ONBOARDING_ACCOUNTING.route}`;
}
Skip onboarding screen with company size if signupQualifier is VSB
This is an improvement
If it's vsb, we should return the onboarding accounting route and set the companySize
as 1-10
and we also need to create a new WS if the onboardingPolicyID doesn't exist
if (isVsb) {
Onyx.set(ONYXKEYS.ONBOARDING_PURPOSE_SELECTED, CONST.ONBOARDING_CHOICES.MANAGE_TEAM);
Onyx.set(ONYXKEYS.ONBOARDING_COMPANY_SIZE, CONST.ONBOARDING_COMPANY_SIZE.MICRO);
if (!onboardingPolicyID) {
const {adminsChatReportID, policyID} = Policy.createWorkspace(undefined, true, '', Policy.generatePolicyID(), CONST.ONBOARDING_CHOICES.MANAGE_TEAM);
Welcome.setOnboardingAdminsChatReportID(adminsChatReportID);
Welcome.setOnboardingPolicyID(policyID);
}
return `/${ROUTES.ONBOARDING_ACCOUNTING.route}`;
}
OPTIONAL: We can move the logic creating the WS to the accounting page if onboardingPolicyID
doesn't exist.
Edited by proposal-police: This proposal was edited at 2024-10-30 18:22:10 UTC.
Skip onboarding screen with company size if signupQualifier is VSB
Feature request
We need to update the code below here to:
if (isVsb) {
Onyx.set(ONYXKEYS.ONBOARDING_PURPOSE_SELECTED, CONST.ONBOARDING_CHOICES.MANAGE_TEAM);
Onyx.set(ONYXKEYS.ONBOARDING_COMPANY_SIZE, CONST.ONBOARDING_COMPANY_SIZE.MICRO);
if (!onboardingPolicyID) {
const {adminsChatReportID, policyID} = Policy.createWorkspace(undefined, true, '', Policy.generatePolicyID(), CONST.ONBOARDING_CHOICES.MANAGE_TEAM);
Welcome.setOnboardingAdminsChatReportID(adminsChatReportID);
Welcome.setOnboardingPolicyID(policyID);
}
return `/${ROUTES.ONBOARDING_ACCOUNTING.route}`;
}
This will set the company size and also create the workspace.
Note that this change is not enough, we should also hide the back button on the accounting page if we have already selected this as the user shouldn't be able to change the employee count if she is the VSB, so here:
// we can improve this condition during PR phase as well
const isVsbOnboarding = Array.isArray(onboardingValues) ? false : onboardingValues?.signupQualifier === CONST.ONBOARDING_SIGNUP_QUALIFIERS.VSB;
<HeaderWithBackButton
shouldShowBackButton={isVsbOnboarding}
Note that we cannot use Welcome.setOnboardingAdminsChatReportID
or Welcome.setOnboardingPolicyID
because it will cause a cyclic dependency error with our CI check, so we need to use Onyx
directly here:
if (isVsb) {
Onyx.set(ONYXKEYS.ONBOARDING_PURPOSE_SELECTED, CONST.ONBOARDING_CHOICES.MANAGE_TEAM);
Onyx.set(ONYXKEYS.ONBOARDING_COMPANY_SIZE, CONST.ONBOARDING_COMPANY_SIZE.MICRO);
if (!onboardingPolicyID) {
const {adminsChatReportID, policyID} = Policy.createWorkspace(undefined, true, '', Policy.generatePolicyID(), CONST.ONBOARDING_CHOICES.MANAGE_TEAM);
Onyx.set(ONYXKEYS.ONBOARDING_ADMINS_CHAT_REPORT_ID, adminsChatReportID ?? null);
Onyx.set(ONYXKEYS.ONBOARDING_POLICY_ID, policyID ?? null);
}
return `/${ROUTES.ONBOARDING_ACCOUNTING.route}`;
}
onboardingPolicyID
will be fetched using Onyx.connect
Skip onboarding screen with company size if signupQualifier is VSB
improvement
we need to change the code here: https://github.com/Expensify/App/blob/1adb6c661cfd71f0d7bac082c4a372793083c704/src/libs/actions/Welcome/OnboardingFlow.ts#L54-L61
if (isVsb) {
Onyx.set(ONYXKEYS.ONBOARDING_PURPOSE_SELECTED, CONST.ONBOARDING_CHOICES.MANAGE_TEAM);
Onyx.set(ONYXKEYS.ONBOARDING_COMPANY_SIZE, CONST.ONBOARDING_COMPANY_SIZE.MICRO);
if (!onboardingPolicyID) {
const {adminsChatReportID, policyID} = Policy.createWorkspace(undefined, true, '', Policy.generatePolicyID(), CONST.ONBOARDING_CHOICES.MANAGE_TEAM);
Welcome.setOnboardingAdminsChatReportID(adminsChatReportID);
Welcome.setOnboardingPolicyID(policyID);
}
return `/${ROUTES.ONBOARDING_ACCOUNTING.route}`;
}
and then remove the employee selection from here: https://github.com/Expensify/App/blob/47b8525756602b794e88d1e9fc5e3aef04689c1d/src/libs/Navigation/AppNavigator/Navigators/OnboardingModalNavigator.tsx#L55-L58
📣 @mzdev0! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>
Contributor details Your Expensify account email: muazsync@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~01e4f874aa627a77c4
✅ Contributor details stored successfully. Thank you for contributing to Expensify!
The current onboarding flow for users with signupQualifier
set to VSB
includes a screen to select company size, which we want to skip.
The onboarding flow does not bypass the company size selection screen for VSB
users, which creates an unnecessary step.
If isVsb
is true
, we should:
OnboardingPurposeSelected
to Manage Team
.OnboardingCompanySize
to Micro
.onboardingPolicyID
is not set.The updated code block from lines 58 to 61 should look like this:
if (isVsb) {
Onyx.set(ONYXKEYS.ONBOARDING_PURPOSE_SELECTED, CONST.ONBOARDING_CHOICES.MANAGE_TEAM);
Welcome.setOnboardingCompanySize(CONST.ONBOARDING_COMPANY_SIZE.MICRO);
if (!onboardingPolicyID) {
const {adminsChatReportID, policyID} = Policy.createWorkspace(
undefined,
true,
'',
Policy.generatePolicyID(),
CONST.ONBOARDING_CHOICES.MANAGE_TEAM
);
Welcome.setOnboardingAdminsChatReportID(adminsChatReportID);
Welcome.setOnboardingPolicyID(policyID);
}
return `/${ROUTES.ONBOARDING_ACCOUNTING.route}`;
}
@carlosmiceli 'Help Wanted' label was removed here—I assume this was a mistake and that we want this issue fixed by an external contributor?
@D-01576 How does your proposal differ from the ones above https://github.com/Expensify/App/issues/51750#issuecomment-2447954731 and https://github.com/Expensify/App/issues/51750#issuecomment-2447974007?
We cannot remove the page entirely, as we still need it for users who aren’t coming from OD
@mzdev0 We cannot remove the page entirely, as we still need it for users who aren’t coming from OD
@Shahidullah-Muffakir Thanks for the proposal! You’ve suggested to update the company size here, but you should have also included the workspace creation aspect as well.
Note that this change is not enough, we should also hide the back button on the accounting page if we have already selected this as the user shouldn't be able to change the employee count if she is the VSB
@twilight2294 I feel that this is a thing that can be done in the PR and would have been caught when testing, so I feel that @nkdengineer Proposal is sufficient here
We can go with @nkdengineer’s proposal here.
🎀 👀 🎀 C+ reviewed
Current assignee @carlosmiceli is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.
I don’t think so, in the rush of writing proposals and posting them first these important points are missed!
and also he missed the point to hide the back button which is important!
On Thu, Oct 31, 2024 at 2:54 PM Jayesh Mangwani @.***> wrote:
Note that this change is not enough, we should also hide the back button on the accounting page if we have already selected this as the user shouldn't be able to change the employee count if she is the VSB
@twilight2294 https://github.com/twilight2294 I feel that this is a thing that can be done in the PR and would have been caught when testing, so I feel that @nkdengineer https://github.com/nkdengineer Proposal is sufficient here
— Reply to this email directly, view it on GitHub https://github.com/Expensify/App/issues/51750#issuecomment-2449402279, or unsubscribe https://github.com/notifications/unsubscribe-auth/BLXFUO6MEOWGG3I2XRWO6O3Z6HZMNAVCNFSM6AAAAABQ4QQEDKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBZGQYDEMRXHE . You are receiving this because you were mentioned.Message ID: @.***>
Whatever! I don’t think this needs to be in the proposal
Currently, as expected by the OP and the current behavior, the back button still displays and it isn't the main problem of this issue.
📣 @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 📖
@mallenexpensify I'd like your input here, because while @twilight2294's proposal wasn't selected, he did help us realize that the Back button should be hidden (and I brought it up to the team in Slack thanks to his suggestion here). I'm thinking that there should be either some compensation for helping us pick that up, or once I create an issue for it (waiting for confirmation that I should), maybe we can give prioritize his proposal for that issue (assuming it works)?
Ok, scratch that Matt, we've decided not to hide the back button so there won't be any extra work/value there.
@nkdengineer @carlosmiceli - I want to confirm that we're actually auto selecting 1-10 in the How many employees do you have
step and going directly to the accounting step. In other words, we're still storing introSelected.companySize : "1-10"
right?
Yup! We're setting that value as well.
When signing up as a company between 1-9 employees, the user is redirected to New Expensify and is welcomed by the onboarding flow. However the first thing we ask them after they open new expensify, is how big a company they want to set up. This is redundant and makes for a bad first impression.
Let's select 1-10 employees by default for signups with the 'vsb' signnpQualifier NVP and start the onboarding flow in the accounting software section. Let's also make sure that whatever logic is triggered by selecting 1-10 also occurs (before displaying the onboarding modal) for these signups.
Select this:
Skip this screen (but keep the choice logic):
This should be the first screen they see in the modal:
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @jayeshmangwani