feat(gp): PayrollAdminOnboarding flow — mutations, schemas, and step components (PBYR-4044)#1142
Open
hamzaremote wants to merge 9 commits into
Open
feat(gp): PayrollAdminOnboarding flow — mutations, schemas, and step components (PBYR-4044)#1142hamzaremote wants to merge 9 commits into
hamzaremote wants to merge 9 commits into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
📦 Bundle Size Report
Size Limits
Largest Files (Top 5)
View All Files (359 total)
✅ Bundle size check passed |
Contributor
📊 Coverage Report
|
| Metric | Current | Previous | Change | Status |
|---|---|---|---|---|
| Lines | 85.78% | 88.46% | -2.68% | 🔴 |
| Statements | 85.23% | 87.87% | -2.64% | 🔴 |
| Functions | 82.90% | 84.84% | -1.94% | 🔴 |
| Branches | 77.85% | 79.86% | -2.01% | 🔴 |
Detailed Breakdown
Lines Coverage
- Covered: 3885 / 4529
- Coverage: 85.78%
- Change: -2.68% (0 lines)
Statements Coverage
- Covered: 3947 / 4631
- Coverage: 85.23%
- Change: -2.64% (0 statements)
Functions Coverage
- Covered: 1052 / 1269
- Coverage: 82.90%
- Change: -1.94% (0 functions)
Branches Coverage
- Covered: 2442 / 3137
- Coverage: 77.85%
- Change: -2.01% (0 branches)
✅ Coverage check passed
Contributor
|
Deploy preview for adp-cost-calculator ready!
Deployed with vercel-action |
Contributor
|
Deploy preview for remote-flows ready!
Deployed with vercel-action |
jordividaller
requested changes
Jul 6, 2026
jordividaller
left a comment
jordividaller
left a comment
Collaborator
There was a problem hiding this comment.
General concern: the flow re-implements mechanics that already exist in the lib
Overall this flow tends to reinvent logic that other flows already share, instead of reusing it. A few concrete points:
1. Error handling — reuse handleStepError.
There's a shared helper handleStepError already used by 9 components across ContractorOnboarding (7) and CreateCompany (2). It does the isMutationError check + normalizeFieldErrors + maps errors back onto the form fields in one call. This flow ignores it and re-inlines the same try/catch isMutationError block three times:
src/flows/PayrollAdminOnboarding/components/useStepSubmitHandler.tssrc/flows/PayrollAdminOnboarding/components/SelectCountryStep.tsxsrc/flows/PayrollAdminOnboarding/components/InvitationStep.tsx
Please route these through handleStepError like the other flows.
2. Field errors aren't normalized — likely a functional regression, not just style.
The other flows pass NormalizedFieldError[] (via handleStepError / normalizeFieldErrors), which is what attaches server-side validation errors to the correct form fields. Here we pass the raw error.fieldErrors (FieldError[]) with no normalization, so field-level API errors probably won't render under their fields the way they do elsewhere. On top of that, the public GPStepCallbacks type freezes fieldErrors: FieldError[] while the rest of the public API exposes NormalizedFieldError[] — that's an inconsistent public contract. Suggest normalizing and aligning the type on NormalizedFieldError[].
3. SelectCountryStep hand-rolls the country field instead of going through the schema/field pipeline (see the separate comment). ContractorOnboarding even reuses selectCountryStepSchema + countriesOptions for this.
What's legitimately new (not a concern)
useGPFormSchemais fair: unlike the other flows that co-locate static schemas injson-schemas/, GP needs a remote per-country/per-form schema (getV1CountriesCountryCodeForm), so a new hook is justified. Minor style nit only: the three paralleluseQuerycalls gated byenabled+ the manualcurrentSchemaswitch could collapse into a single hook parameterized by step.PayrollAdminForm/useStepSubmitHandlerare fine to exist — every flow has its own<XxxForm>, so that's the norm, not reinvention. The only issue is thatuseStepSubmitHandlerdoesn't callhandleStepError(see point 1).
Net: the new remote-schema piece is good; the concern is only about the parts that already existed as shared helpers.
Add GPStepCallbacks<TSuccess> to src/flows/types.ts — the shared callback prop type used by all GP step components. Replace the empty scaffold types in PayrollAdminOnboarding/types.ts with real component signatures. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GPAdminStepCallbacks is the public callback type consumers pass to each admin step component. useGPOnboardingSteps is the hook consumers need to read step state in an outer company-manager context. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two bugs flagged in review: 1. onSubmit guards returned undefined on failure (missing countryCode, missing employmentId, or no id in the create-employment response). useStepSubmitHandler treated undefined as success and called next(), advancing the wizard with no API write. Guards now throw so the catch block handles them correctly. 2. skipCountry only required initialCountryCode, so a pre-filled country with no initialEmploymentId would hide the select_country step (the only place createEmployment runs) and leave the flow with no employmentId for contract_details and beyond. skipCountry now requires both initialCountryCode and initialEmploymentId — meaning "skip" is only valid when resuming an existing employment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…type - Replace duplicated isMutationError try/catch blocks in useStepSubmitHandler, SelectCountryStep, and InvitationStep with handleStepError, matching the pattern used in ContractorOnboarding and CreateCompany - Align GPStepCallbacks.onError.fieldErrors to NormalizedFieldError[] (was FieldError[]) so the public contract is consistent with other flows - Remove hardcoded <select> HTML and Tailwind classes from SelectCountryStep; the component now renders PayrollAdminForm through the schema/field pipeline like ContractDetailsStep and AdministrativeDetailsStep — country selection is a consumer concern exposed via adminBag.setInternalCountryCode Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the three parallel useQuery calls (one per schema step) with a single useGPFormSchema call parameterized by the current step. This removes the manual currentSchema switch and the multi-schema isLoading union, reducing the hook to a single query that naturally tracks the active step. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Selecting a country via the raw <select> was the only mechanism that set internalCountryCode; removing it left the basic-information schema permanently disabled. Replace the ad-hoc <select> with a schema-driven flow: - Add a static gpSelectCountrySchema (country_code field, oneOf: []) - Add useGPCountrySelectSchema that fetches countries and injects them as options into the static schema via createHeadlessForm - On the select_country step, show the country-picker schema until a country is chosen (isSelectCountryPhase), then switch to the global_payroll_basic_information schema - Wrap setFieldValues as handleFieldValues: when the country-picker is active and country_code changes, automatically call setInternalCountryCode so the schema transition happens on field change rather than form submit - Key PayrollAdminForm on adminBag.countryCode in SelectCountryStep so the form fully remounts (resetting react-hook-form state) when the schema switches from country picker to basic information Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
react-hook-form's watch only fires on user changes, not on mount. If a consumer prefills country_code via initialValues.basic_information without passing the countryCode prop, handleFieldValues never runs on the untouched field and internalCountryCode stays undefined — trapping the flow in the country-picker phase. Seed internalCountryCode from initialValues.basic_information.country_code as a fallback so the basic-information form loads immediately when a country is already present in the prefilled values. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
remotecom
force-pushed
the
feat/gp-admin-onboarding-flow
branch
from
78adb54 to
8ece927
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
There are 4 total unresolved issues (including 1 from previous review).
❌ 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 8ece927. Configure here.
- skipCountry now derives from internalCountryCode (covers initialValues fallback), not just the explicit countryCode prop, avoiding a duplicate employment create on resume - sync internalCountryCode on every country_code change, not only during the country-picker phase, so edits made from the basic-information form aren't submitted under a stale country code - step forms restore stepState.values for the active step before falling back to initialValues, matching ContractorOnboarding/CreateCompany - InvitationStep now surfaces a missing employmentId through onError instead of silently no-op'ing Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes the
PayrollAdminOnboardingflow skeleton that was scaffolded in an earlier PR. The scaffold had step state and a render-prop container but no mutations, no schema loading, and no step components — this PR adds all of that.Part of the GP payroll split plan (
GP_PAYROLL_SPLIT_PLAN.md). Stacks on #1134.What's added:
api.ts— React Query hooks for the four admin API calls:useGPFormSchema(schema per step),useGPCreateEmployment,useGPUpdateContractDetails,useGPUpdateAdministrativeDetails,useGPInviteEmployeehooks.tsx— mutations wired intoonSubmit, one schema query per step gated byenabled: currentStep === X(lazy, never fetches ahead),refetchSteps()called after each successful submitcomponents/—SelectCountryStep,ContractDetailsStep,AdministrativeDetailsStep,InvitationStep,PayrollAdminForm,SubmitButton,BackButton,useStepSubmitHandlersrc/flows/types.ts—GPStepCallbacks<TSuccess>shared callback type (used here and by the employee flow in the next PR)src/index.tsx— exportsGPAdminStepCallbacks,useGPOnboardingStepsNote on
InvitationStep: this step doesn't go throughuseStepSubmitHandlerbecause there's no form — it's a single button that fires the invite mutation directly. It owns its mutation instance and callsadminBag.refetchSteps()/adminBag.next()itself.Test plan
npm run type-check— no errorsnpm run test— passesnpm run buildcompletes without OOM (heap raise from fix: datepicker dropdown navigation + JSF logging + build heap #1134 helps here)Note
Medium Risk
Touches employment creation and payroll contract/admin updates plus employee invite; logic bugs could block onboarding or submit wrong data, though patterns match existing flow APIs.
Overview
Completes the PayrollAdminOnboarding flow beyond the earlier scaffold by wiring JSON-schema forms, API mutations, and render-prop step components end to end.
usePayrollAdminOnboardingnow loads schemas per step (country picker until a country is chosen, then country-specific basic/contract/admin forms), validates via JSF, and submits: create employment onselect_country, PUT contract/admin details on later steps, withrefetchSteps()after each success. Country skip only when bothemploymentIdand country are known upfront (fixes bypassing employment creation).employment_idis passed into schema fetches for role-aware fields.New
api.tshooks cover form schemas, create/update employments, and invite. Step UI includes sharedPayrollAdminForm,useStepSubmitHandler(callbacks +adminBag.onSubmit+ advance),SubmitButton/BackButton, andInvitationStep(invite mutation only, no form).Adds shared
GPStepCallbacksand exportsGPAdminStepCallbacks/useGPOnboardingStepsfrom the package entry.Reviewed by Cursor Bugbot for commit 7177dc7. Bugbot is set up for automated code reviews on this repo. Configure here.