feat(gp): example app demo for admin onboarding flow (PBYR-4044) - #1171
feat(gp): example app demo for admin onboarding flow (PBYR-4044)#1171hamzaremote wants to merge 5 commits into
Conversation
|
Deploy preview for remote-flows ready!
Deployed with vercel-action |
|
Deploy preview for adp-cost-calculator ready!
Deployed with vercel-action |
4ca62c2 to
3cff59d
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
📊 Coverage Report✅ Coverage increased! 🎉
Detailed BreakdownLines Coverage
Statements Coverage
Functions Coverage
Branches Coverage
✅ Coverage check passed |
| const currentStep = adminBag.stepState.currentStep.name; | ||
| const allSteps = Object.entries(STEP_LABELS); | ||
|
|
||
| if (adminBag.isLoading && !adminBag.countryCode) { |
There was a problem hiding this comment.
isn't this a bit counterintuitive?
if countryCode is defined and isLoading is triggered by any step the isLoading, it won't show
There was a problem hiding this comment.
Fixed in 5d0fb9c + dafa5b9. Changed the gate to adminBag.isLoading && adminBag.fields.length === 0 — it now blocks on any step whose schema hasn't loaded yet (not just before a country is picked), so it won't silently skip the loading state on step transitions later in the flow (e.g. while the contract_details/administrative_details schema is fetching, or while legal entities are being resolved).
| const renderStep = () => { | ||
| switch (currentStep) { | ||
| case 'select_country': { | ||
| const isFormReady = |
There was a problem hiding this comment.
shouldn't be isLoading and instead of not rendering the button put a disabled state?
There was a problem hiding this comment.
Fixed in dafa5b9. The button is no longer conditionally unmounted — it's always rendered with disabled={!isFormReady} (SubmitButton also composes that with its own isSubmitting/isLoading disabling), so it stays visible and just goes disabled/enabled as readiness changes.
| <> | ||
| <div className='steps-navigation'> | ||
| <ul> | ||
| {allSteps.map(([key, label], index) => ( |
There was a problem hiding this comment.
if for some reason in the future, the steps are dynamic like in the Onboarding & ContractorOnboarding, this won't work and we'll need to refactor it
Basically what we do is set the steps on the hook you read them through the adminBag and do the same iterations
There was a problem hiding this comment.
Good callout — leaving this as static STEP_LABELS/STEP_DESCRIPTIONS for now since the admin flow's steps aren't dynamic today (select_country is the only conditionally-visible one, and this demo never sets skipCountry). Agreed on the approach if/when that changes: read the steps through adminBag the same way Onboarding/ContractorOnboarding do, rather than hardcoding them in the demo. Not doing that refactor in this PR since it's speculative for the current flow.
| } | ||
|
|
||
| function GPAdminOnboardingInner() { | ||
| const { data: legalEntities, isLoading } = useGPLegalEntities(COMPANY_ID); |
There was a problem hiding this comment.
I would solve this in the hooks internally.
If there are no legalEntities, this property can be exposed in the adminBag and they can do whatever they want with it
And the legalEntityId pass to the hook can be set internally, instead of expecting people setting them themselves.
if there are no legal entities nothing should be set
Let me know if I am missing anything
There was a problem hiding this comment.
Implemented in dafa5b9 — moved this into usePayrollAdminOnboarding itself: legalEntityId is now optional, and when not provided the hook fetches the company's GP-enabled legal entities via useGPLegalEntities internally and defaults to the first one. adminBag.legalEntities exposes the (possibly empty) array so the consumer can render its own "no legal entity" state — the demo now does exactly that instead of a separate wrapper component that fetched legal entities up front. If legalEntityId still ends up unset when submitting select_country, onSubmit throws rather than silently sending an empty ID.
| @@ -0,0 +1,564 @@ | |||
| import { useState } from 'react'; | |||
There was a problem hiding this comment.
Can you extract this file to a separated PR
I want to review it standalone to avoid having mixed comments from the other files?
There was a problem hiding this comment.
Done — split out to #1201, which now carries PayrollEmployeeOnboarding.tsx plus its supporting server-side auth plumbing (jwt_auth.js employee-assertion minting, proxy.js routing, the employee-token route) and the two follow-up fix commits that were already reviewed here. This PR is now scoped to just the admin demo.
0124a97 to
609a706
Compare
Per review on #1171: instead of requiring callers to fetch legal entities themselves and pass legalEntityId in, usePayrollAdminOnboarding now fetches them internally via useGPLegalEntities and defaults legalEntityId to the first GP-enabled one. legalEntityId is now optional and still overridable. adminBag exposes the resulting legalEntities array so consumers can render their own "no GP-enabled legal entity" state; onSubmit throws if it's still missing when creating an employment. Also updates the admin demo per review: - loading gate now checks for empty fields rather than an unset countryCode, so it doesn't miss loading states past the first step - the "Create Employment & Continue" button is disabled while not ready instead of being unmounted - drops the separate useGPLegalEntities call/wrapper now that adminBag exposes legalEntities directly
Per review on #1171: gabrielseco asked to review the employee demo standalone rather than mixed in with the admin demo comments. Moved to #1201 (feat/gp-employee-onboarding) — this PR now only carries the admin onboarding demo and its supporting SDK changes. Removes example/src/PayrollEmployeeOnboarding.tsx, its App.tsx entry, and the employee-only backend plumbing (employee-assertion JWT minting in jwt_auth.js, employee-token proxy routing, the employee-token route). Keeps the authType 'none' addition in RemoteFlows.tsx and the production guard on the company-manager token endpoint in jwt_auth.js, since both are still needed by/apply to the admin demo.
hamzaremote
changed the title
📦 Bundle Size Report
Size Limits
Largest Files (Top 5)
View All Files (402 total)
✅ Bundle size check passed |
…44) (#1201) * feat(gp): example app demo for employee self-onboarding flow (PBYR-4044) Adds the Global Payroll employee self-onboarding demo page and its server-side auth plumbing, split out of the combined admin+employee demo PR (#1171) for standalone review: - example/api/jwt_auth.js: server-side employee-assertion JWT minting - example/api/proxy.js: path-based token routing — /v1/employee/* → employee assertion (via x-rf-employment-id header), everything else unchanged - example/api/routes.js: wire the employee-token route - example/src/RemoteFlows.tsx: add authType 'none' for proxy-minted tokens - example/src/PayrollEmployeeOnboarding.tsx: the demo itself - example/src/App.tsx: register the demo The employee demo derives the bank substep from the flow bag's selfOnboardingSubsteps rather than calling a hook directly, so it needs no extra SDK export. * fix(gp): correct completion detection in employee demo - derive the last step from SDK-reachable steps (tax steps only navigable once employeeBag.isComplete) rather than the sidebar list, so Submit/completion fires on the real final step (e.g. bank_account on a pre-enrollment USA run). - federal_taxes "Skip" now finishes (setDone) when it's the last reachable step (USA without jurisdiction), instead of a no-op next(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(gp): derive employee-demo completion reactively, not from stale closure Step onSuccess handlers latched `done` from that render's `isLastStep`, so if a later step became reachable after the submit (e.g. enrollment flips isComplete and adds tax steps), the demo showed "Complete" instead of advancing. Now each submit records the step name and completion is derived per-render from `lastSubmittedStep === lastStepKey`, so a step that stops being last no longer ends the flow. Explicit "Finish" on an unavailable tax step still latches done. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(gp): correct misleading company-manager auth claims in employee demo Per Cursor Bugbot on #1201: the outer context's comments and error message claimed the dev proxy mints a company-manager token for /v1/employments/* and /v1/companies/*, but proxy.js's getTokenType never has a rule for those paths — it always falls through to the refresh-token ('user-token') flow, same as most other demos behind this proxy. authType on the FE RemoteFlows component doesn't change this either, since the proxy middleware always overwrites the outgoing Authorization header itself. Actually wiring company-manager auth for those paths would change behavior for other demos sharing this proxy (ContractorOnboarding, Termination, ContractAmendment, Onboarding) that aren't part of this PR, so this only corrects the comments/error text to describe what actually happens today. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the Global Payroll demo pages and the server-side auth plumbing: - example/api/jwt_auth.js: server-side employee-assertion JWT minting - example/api/proxy.js: path-based token routing — /v1/employee/* → employee assertion (via x-rf-employment-id header), everything else unchanged - example/api/routes.js: wire the employee-token route - example/src/RemoteFlows.tsx: add authType 'none' for proxy-minted tokens - example/src/PayrollAdminOnboarding.tsx / PayrollEmployeeOnboarding.tsx: demos - example/src/App.tsx: register both demos The employee demo derives the bank substep from the flow bag's selfOnboardingSubsteps rather than calling a hook directly, so it needs no extra SDK export. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PayrollAdminOnboardingFlow still passed the null stubs from the original PBYR-4043 scaffolding commit into the render prop, even though the real step components were added later in PBYR-4044. The admin flow rendered nothing for every step.
Per review on #1171: instead of requiring callers to fetch legal entities themselves and pass legalEntityId in, usePayrollAdminOnboarding now fetches them internally via useGPLegalEntities and defaults legalEntityId to the first GP-enabled one. legalEntityId is now optional and still overridable. adminBag exposes the resulting legalEntities array so consumers can render their own "no GP-enabled legal entity" state; onSubmit throws if it's still missing when creating an employment. Also updates the admin demo per review: - loading gate now checks for empty fields rather than an unset countryCode, so it doesn't miss loading states past the first step - the "Create Employment & Continue" button is disabled while not ready instead of being unmounted - drops the separate useGPLegalEntities call/wrapper now that adminBag exposes legalEntities directly
Per review on #1171: gabrielseco asked to review the employee demo standalone rather than mixed in with the admin demo comments. Moved to onboarding demo and its supporting SDK changes. Removes example/src/PayrollEmployeeOnboarding.tsx, its App.tsx entry, and the employee-only backend plumbing (employee-assertion JWT minting in jwt_auth.js, employee-token proxy routing, the employee-token route). Keeps the authType 'none' addition in RemoteFlows.tsx and the production guard on the company-manager token endpoint in jwt_auth.js, since both are still needed by/apply to the admin demo.
815ec66 to
31017b5
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 31017b5. Configure here.
…tyId Per Cursor Bugbot: when legalEntityId was provided explicitly, the fetch was skipped as a minor optimization, so adminBag.legalEntities was always [] regardless of the company's actual legal entities. Since the documented contract is "empty legalEntities means the company has none," any caller that passed an explicit legalEntityId and checked that contract would hit the empty-state path incorrectly. Now always fetches so legalEntities stays accurate either way.
useGPLegalEntities failures left legalEntities as [], the same shape as "company has no GP-enabled legal entity" — so callers following that documented contract couldn't tell an API/auth failure from a real empty result. Expose adminBag.isErrorLegalEntities so consumers (and the demo) can render an error state instead of the empty-state message. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
a9635dd to
46a9f85
Compare
3 participants
What
The final piece of the Global Payroll onboarding work (see split plan, PR 5): the example-app demos for the admin and employee flows, plus the server-side auth plumbing they need. Both SDK flows (#1142, #1168) are already merged to
main.Contents
example/api/jwt_auth.js— server-side minting of an employee-assertion JWT-bearer token (getEmployeeToken), plus a production guard on the token endpoints.example/api/proxy.js— path-based token routing:/v1/employee/*→ employee assertion (employment resolved from thex-rf-employment-idheader, minted server-side so the FE never holds it); everything else unchanged.example/api/routes.js— wire the/api/fetch-employee-token/:employmentIdroute.example/src/RemoteFlows.tsx— addauthType: 'none'so the inner (employee) context skips the FE auth callback while the proxy mints tokens.example/src/PayrollAdminOnboarding.tsx/PayrollEmployeeOnboarding.tsx— the two demos.example/src/App.tsx— register both demos.Design notes (the open questions from the arch doc)
RemoteFlows-provider pattern (employee demo): the outer context uses a company-manager token to readcountry/jurisdictionfrom the employment (the employee assertion token can't fetch/v1/employments/:id); the inner context uses the employee-scoped token minted by the proxy.x-rf-employment-id; the proxy swaps it for a JWT-bearer employee token on/v1/employee/*.Notable
selfOnboardingSubsteps(not a direct hook call), so it needs no extra SDK export — consistent with dropping theuseGPOnboardingStepspublic export in feat(gp): PayrollAdminOnboarding flow — mutations, schemas, and step components (PBYR-4044) #1142.Verification
type-check(tsc -b) ✅ · oxfmt ✅ · oxlint ✅ (0 warnings on the demos)🤖 Generated with Claude Code
Note
Medium Risk
Optional
legalEntityIdis a public API change for integrators; employment creation now depends on legal-entity fetch and new error states must be handled correctly.Overview
Adds a GP Admin Onboarding example demo that walks through country/basic info, contract, administrative details, and invitation, including handling for legal-entity load failures and missing GP-enabled entities.
SDK:
PayrollAdminOnboardingFlownow wires in the real step components instead of placeholder null components.legalEntityIdis optional — the hook always loads GP-enabled legal entities for the company, defaults to the first match when omitted, and surfaceslegalEntitiesandisErrorLegalEntitiesonadminBagso hosts can distinguish fetch errors from “no GP legal entity.” Employment creation throws if no legal entity is available when creating a new employment.Reviewed by Cursor Bugbot for commit 46a9f85. Bugbot is set up for automated code reviews on this repo. Configure here.