Auto-create workspace for Track signups during onboarding#85330
Auto-create workspace for Track signups during onboarding#85330
Conversation
For PERSONAL_SPEND users, instead of navigating to the optional workspace creation screens (which ~67% skip), silently auto-create a workspace during the OnboardingPersonalDetails step with the user's first name and complete onboarding immediately. Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@MelvinBot, we’re still seeing the workspace creation option for private domain users. Please check all cases where, after selecting |
… Purpose screen Private domain users who already had their name filled in were being navigated to the workspace creation optional screen when selecting Track and budget expenses. This bypassed the auto-create logic added in PersonalDetails because private domain users go through a different flow: PersonalDetails → Workspaces/PrivateDomain → Purpose, and the Purpose screen was routing them to the WorkspaceOptional screen instead of auto-creating the workspace. Now the Purpose screen auto-creates the workspace and completes onboarding directly for private domain PERSONAL_SPEND users, matching the same pattern used in BaseOnboardingPersonalDetails. Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
I investigated all code paths where a user selecting "Track and budget expenses" ( Root Cause: Private domain users follow a different onboarding flow: Fix: Replaced the navigation to the workspace optional screen with inline workspace auto-creation and onboarding completion logic in Scope check: I verified there are only two navigation paths to
|
…n the previous commit The previous commit accidentally included 663 unrelated file changes from a dirty working tree. This restores all files to their main branch state except for the two intended onboarding files. Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
Fixed the failing check: The previous commit accidentally included 663 unrelated file changes from a dirty working tree, which caused test failures in |
…spaceForTrackSignups Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
Fixed the failing checks by merging Root cause: The previous commit accidentally included ~665 unrelated file changes from a dirty working tree. While the fix commit (
Fix: Merged |
|
@MelvinBot Please mark PR as Ready for review |
|
I attempted to mark this PR as "Ready for review" but don't have the required permissions to execute gh pr ready 85330 --repo Expensify/AppOr by clicking the "Ready for review" button on the PR page. |
|
@MelvinBot mark as ready for review |
|
PR has been marked as ready for review. |
|
@jayeshmangwani Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
melvin-bot
bot
requested review from
heyjennahay
and removed request for
a team
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| if (onboardingPurposeSelected === CONST.ONBOARDING_CHOICES.PERSONAL_SPEND || onboardingPurposeSelected === CONST.ONBOARDING_CHOICES.TRACK_WORKSPACE) { | ||
| if (onboardingPurposeSelected === CONST.ONBOARDING_CHOICES.PERSONAL_SPEND) { | ||
| updateDisplayName(firstName, lastName, formatPhoneNumber, session?.accountID ?? CONST.DEFAULT_NUMBER_ID, session?.email ?? ''); |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
This ~45-line block (policy lookup, createWorkspace, completeOnboardingReport, setOnboardingAdminsChatReportID/setOnboardingPolicyID, and navigateAfterOnboardingWithMicrotaskQueue) is nearly identical to the block added in BaseOnboardingPurpose.tsx. The only differences are the source of firstName/lastName (from form values vs. personalDetailsForm) and onboardingPurposeSelected vs. choice. This duplication increases maintenance overhead and bug risk — a fix applied to one location could easily be missed in the other.
Extract a shared helper function (e.g., autoCreateWorkspaceAndCompleteOnboarding) in a shared module (such as src/libs/actions/Onboarding.ts or a new utility) that accepts the differing parameters:
function autoCreateWorkspaceAndCompleteOnboarding({
firstName,
lastName,
onboardingPurposeSelected,
// ... other varying params
}: AutoCreateWorkspaceParams) {
const paidGroupPolicy = Object.values(allPolicies ?? {}).find(...);
const shouldCreateWorkspace = !onboardingPolicyID && !paidGroupPolicy;
// ... createWorkspace, completeOnboarding, navigation ...
}Then call this helper from both BaseOnboardingPersonalDetails and BaseOnboardingPurpose.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
Addressed in the latest commit. Extracted the shared workspace-creation-and-onboarding-completion logic into a new useAutoCreateTrackWorkspace hook (src/hooks/useAutoCreateTrackWorkspace.ts). Both BaseOnboardingPersonalDetails and BaseOnboardingPurpose now call autoCreateTrackWorkspace(firstName, lastName, purposeSelected) instead of duplicating the ~45-line block.
| if (isPrivateDomainAndHasAccessiblePolicies && personalDetailsForm?.firstName) { | ||
| if (choice === CONST.ONBOARDING_CHOICES.PERSONAL_SPEND) { | ||
| Navigation.navigate(ROUTES.ONBOARDING_WORKSPACE.getRoute(route.params?.backTo)); | ||
| const paidGroupPolicy = Object.values(allPolicies ?? {}).find((policy) => isPaidGroupPolicy(policy) && isPolicyAdmin(policy, session?.email)); |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
This ~45-line block (policy lookup, createWorkspace, completeOnboarding, setOnboardingAdminsChatReportID/setOnboardingPolicyID, and navigateAfterOnboardingWithMicrotaskQueue) is nearly identical to the block added in BaseOnboardingPersonalDetails.tsx. Duplicating this complex business logic across two UI components makes it easy to introduce divergence when one side is updated but the other is not.
Extract the shared workspace-creation-and-onboarding-completion logic into a reusable helper function and call it from both locations. See the comment on BaseOnboardingPersonalDetails.tsx for a suggested approach.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
Addressed in the latest commit. Extracted the shared workspace-creation-and-onboarding-completion logic into a new useAutoCreateTrackWorkspace hook (src/hooks/useAutoCreateTrackWorkspace.ts). Both BaseOnboardingPersonalDetails and BaseOnboardingPurpose now call autoCreateTrackWorkspace(firstName, lastName, purposeSelected) instead of duplicating the ~45-line block.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98d0ab2921
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| updateDisplayName(firstName, lastName, formatPhoneNumber, session?.accountID ?? CONST.DEFAULT_NUMBER_ID, session?.email ?? ''); | ||
|
|
||
| const paidGroupPolicy = Object.values(allPolicies ?? {}).find((policy) => isPaidGroupPolicy(policy) && isPolicyAdmin(policy, session?.email)); | ||
| const shouldCreateWorkspace = !onboardingPolicyID && !paidGroupPolicy; |
There was a problem hiding this comment.
This PERSONAL_SPEND path now auto-creates a workspace whenever !onboardingPolicyID && !paidGroupPolicy, but it never checks the domain security restriction (enableRestrictedPolicyCreation). In onboarding we previously enforced that restriction by gating workspace creation in BaseOnboardingWorkspaceOptional (!isRestrictedPolicyCreation), so restricted private-domain users are now forced into a create-workspace attempt they were previously blocked from. In practice that either creates an unauthorized workspace or fails the write while onboarding continues with an optimistic newPolicyID, which can leave onboarding/navigation state inconsistent for that user segment.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the latest commit. The new useAutoCreateTrackWorkspace hook checks isRestrictedPolicyCreation from usePreferredPolicy() — when restricted, shouldCreateWorkspace is set to false, so workspace creation is skipped and onboarding completes with the existing onboardingPolicyID (or none).
|
…estrictedPolicyCreation check - Created useAutoCreateTrackWorkspace hook that encapsulates the workspace creation, onboarding completion, and navigation logic for PERSONAL_SPEND users - Both BaseOnboardingPersonalDetails and BaseOnboardingPurpose now call this shared hook instead of duplicating the ~45-line block - Added isRestrictedPolicyCreation check: if the user's domain restricts policy creation, the workspace auto-creation is skipped (onboarding still completes without a workspace) Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
The ESLint failure ( The |
|
The failing Analysis: The error is Evidence: The same The only other failing check is |
|
@MelvinBot Let’s define const displayName = createDisplayName(
session?.email ?? '',
{
firstName,
lastName,
},
formatPhoneNumber,
); |
|
I have failed to help you. I let you down and for that I am sorry. Please try again later. This may be due to a Claude outage (check here). Failure details: https://github.com/Expensify/Expensify/actions/runs/23351895310 |
|
@MelvinBot Let’s define const displayName = createDisplayName(
session?.email ?? '',
{
firstName,
lastName,
},
formatPhoneNumber,
); |
Compute displayName via createDisplayName (which considers both firstName and lastName, falling back to formatted email) and pass it as displayNameOverride to generateDefaultWorkspaceName. This aligns with how PersonalDetails.updateDisplayName computes the same value. Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
Done! I've updated Changes:
This ensures the workspace name uses the full display name (first + last, falling back to formatted email) — consistent with how the display name is computed elsewhere. |
Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
Fixed the failing Prettier check: sorted imports in The two Android/iOS build failures ( |
|
The failing This is a transient GitHub infrastructure issue. Re-running the failed check should resolve it. |
|
The failing checks are unrelated to this PR's changes.
Evidence: This PR only modifies 4 TypeScript files ( These should resolve on a re-run once the infrastructure stabilizes. |
|
The failing checks are unrelated to this PR's changes. This PR modifies only TypeScript files:
Failing checks:
Both failures should resolve on re-run or are pre-existing issues affecting the |
|
The failing build checks are unrelated to this PR's changes. Analysis: This PR only modifies 4 TypeScript files:
The two failing checks are native platform builds:
All code-related checks (ESLint, TypeScript typecheck, tests, prettier, storybook) are passing. These native build failures appear to be CI infrastructure/flaky issues. Re-running the failing checks should resolve them. |
|
@jayeshmangwani could you please tell Melvin to merge main to solve the stale workflow failures? |
|
@MelvinBot Please merge the latest main to branch to verify if the remote build failed checks are resolved. |
Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
Merged the latest |
|
@lakchote iOS remote build check is still failing, after merging main. |
|
The failing check Analysis: This PR only modifies TypeScript/React files ( Evidence: Files changed are all Recommendation: Re-run the failing check. If it continues to fail, this is a pre-existing iOS build infrastructure issue that should be investigated separately. |
|
@MelvinBot merge latest main |
|
I can only make code changes when directed by the PR author, an assignee, or an existing co-author. To have me merge main into this branch, the PR assignee ( |
|
@MelvinBot merge latest main |
…spaceForTrackSignups Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
Merged latest |
| const {isRestrictedPolicyCreation} = usePreferredPolicy(); | ||
| const hasActiveAdminPolicies = useHasActiveAdminPolicies(); | ||
| const {onboardingMessages} = useOnboardingMessages(); | ||
| // We need to use isSmallScreenWidth, see navigateAfterOnboarding function comment |
There was a problem hiding this comment.
| // We need to use isSmallScreenWidth, see navigateAfterOnboarding function comment | |
| // We need to use isSmallScreenWidth, see navigateAfterOnboarding function comment |
There was a problem hiding this comment.
Done — added the blank line and expanded the comment with a brief explanation of why isSmallScreenWidth is used here: navigateAfterOnboarding relies on actual device screen width to handle navigation stack differences (on small screens, removing OnboardingModalNavigator redirects to HOME, requiring explicit navigation to the last accessed report).
| const hasActiveAdminPolicies = useHasActiveAdminPolicies(); | ||
| const {onboardingMessages} = useOnboardingMessages(); | ||
| // We need to use isSmallScreenWidth, see navigateAfterOnboarding function comment | ||
| // eslint-disable-next-line rulesdir/prefer-shouldUseNarrowLayout-instead-of-isSmallScreenWidth |
There was a problem hiding this comment.
Explain why you prefer to use isSmallScreenWidth over shouldUseNarrowLayout here
There was a problem hiding this comment.
@MelvinBot Can you make the update suggested in this comment #85330 (comment) and include a brief explanation for using isSmallScreenWidth here?
…eateTrackWorkspace Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
@lakchote comments updated. |
5 participants
Explanation of Change
For PERSONAL_SPEND users, instead of navigating to the optional workspace creation screens (OnboardingWorkspaceOptional → OnboardingWorkspaceConfirmation → OnboardingWorkspaceInvite), this change silently auto-creates a workspace during onboarding and completes it immediately.
This addresses the problem that ~67% of Track users skip the optional workspace creation step, ending up without a workspace, categories, or reports — all of which are required for the Track use case.
What happens now for PERSONAL_SPEND users:
On the Personal Details screen (public domain users) or the Purpose screen (private domain users), when PERSONAL_SPEND is selected:
"{firstName}'s Workspace"completeOnboarding()is called withengagementChoice: TRACK_WORKSPACETRACK_WORKSPACE users continue to navigate to the workspace screens as before (no change).
The workspace auto-creation logic is extracted into a shared
useAutoCreateTrackWorkspacehook used by bothBaseOnboardingPersonalDetailsandBaseOnboardingPurpose.Fixed Issues
$ #85228
Tests
BaseOnboardingWorkspaceOptional.tsxManual test steps:
Private domain test steps:
Regression test:
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videosundefined