refactor(EditCompensation): consume useJobForm + useCompensationForm hooks#1736
Draft
serikjensen wants to merge 1 commit intorefactor/job-compensation-hooks-splitfrom
Draft
Conversation
…hooks
Migrates the Compensation flow's edit screen onto the public hook contract:
- Replace `BaseComponent` + `EditCompensationPresentation` with a single
`BaseBoundaries`-wrapped Root that instantiates `useJobForm` and
`useCompensationForm` and threads them through `composeSubmitHandler` to
preserve the existing job-then-compensation submit chain (POST job →
PUT stub comp on create, PUT job → PUT comp on update). Events
(`EMPLOYEE_JOB_CREATED`/`UPDATED`, `EMPLOYEE_COMPENSATION_UPDATED`) and
the `onSaved(Compensation)` contract are unchanged.
- Delete the duplicate internal `compensationSchema.ts` and the
presentation/test pair: form behavior is now covered by the hooks'
unit tests (`useJobForm.test.tsx`, `useCompensationForm.test.tsx`,
`compose.test.tsx`) and the integration tests in
`Compensation.test.tsx` continue to assert the screen-level behavior
unchanged.
- Promote `title` (job) and `flsaStatus`/`rate`/`paymentUnit` (comp) via
`optionalFieldsToRequire: { update: [...] }` so the screen always
renders these fields as required (preserves the pre-migration UX —
the hooks default these to required only on `create`).
- Render fields inline with `formHookResult` props because the
screen interleaves job and compensation fields. Keep the FLSA
change warning as screen-local presentation state (uses `useWatch`
on the comp form); the hook still exposes `data.canTriggerCarveOut`
for partners who want to drive their own confirmation prompt.
Surfaced two hook-design fixes on PR #1735 during this migration:
the `flsaStatus` placeholder rendering and the EXEMPT rate-threshold
ordering. Those land first; this branch sits on top.
Co-authored-by: Cursor <cursoragent@cursor.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
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.
Migrates the Compensation flow's edit screen onto the public hook contract introduced by #1735.
Opening as a draft stacked on
refactor/job-compensation-hooks-splitbecause the migration validates the hook contract end-to-end against the existing integration tests. Will rebase ontomainand unmark draft once #1735 lands.Summary
BaseComponent+EditCompensationPresentationwith a singleBaseBoundaries-wrapped Root that instantiatesuseJobFormanduseCompensationFormand threads them throughcomposeSubmitHandler. The job-then-compensation submit chain is preserved (POST job → PUT stub comp on create, PUT job → PUT comp on update). Events (EMPLOYEE_JOB_CREATED/UPDATED,EMPLOYEE_COMPENSATION_UPDATED) and theonSaved(Compensation)contract are unchanged.compensationSchema.tsand theEditCompensationPresentation/test pair: form behavior is now covered by the hooks' unit tests (useJobForm.test.tsx,useCompensationForm.test.tsx,compose.test.tsx). The integration suite (Compensation.test.tsx) is unmodified and continues to assert screen-level behavior.title(job) andflsaStatus/rate/paymentUnit(comp) viaoptionalFieldsToRequire: { update: [...] }so the screen always renders these fields as required. The hooks default these to required only on'create'; the screen-level UX always required them.formHookResult={...}props because the screen interleaves job and compensation fields. Keep the FLSA change warning as screen-local presentation state viauseWatchon the comp form'shookFormInternals.formMethods.control. The hook still exposesdata.canTriggerCarveOutfor partners who want to drive their own confirmation prompt.Hook-design fixes that landed on #1735 because of this migration
The migration revealed two contract mismatches that #1735 needed before this PR could pass the existing integration tests without modification:
flsaStatusvalidator made.optional()so the field can render its empty placeholder when nothing is provided. Schema-levelrequiredFieldsConfigstill enforces it on submit in create mode. New fallback chain inherits from the employee's primary job's current compensation when adding a secondary job.RATE_EXEMPT_THRESHOLDsurfaces beforeRATE_MINIMUMforEXEMPT+ low rate (matches pre-migration UX).Test plan
npm run test -- --run src/components/Employee/Compensation— 85/85 ✅ (integration suite untouched)npm run lint:check src/components/Employee/Compensation— 0 errorsEmployee.Compensationflow end-to-end in Storybook / sdk-app (no-jobs onboarding, single-Exempt edit, multi-job FLSA carve-out)mainand unmark draft after feat!: split useCompensationForm into useJobForm + useCompensationForm hooks #1735 mergesMade with Cursor