feat(4/5): add SelectEmployees container for holiday pay policies (SDK-567)#1616
feat(4/5): add SelectEmployees container for holiday pay policies (SDK-567)#1616jeffredodd merged 5 commits intomainfrom
Conversation
fcbf391 to
63e1327
Compare
55b3ce9 to
28d57da
Compare
63e1327 to
1cc43ea
Compare
28d57da to
de815ad
Compare
1cc43ea to
533317d
Compare
de815ad to
d89a746
Compare
533317d to
73686e3
Compare
d89a746 to
270f050
Compare
73686e3 to
c6b5911
Compare
270f050 to
ba3120e
Compare
| const filteredEmployees = useMemo(() => { | ||
| if (!searchValue) return employees | ||
| const lower = searchValue.toLowerCase() | ||
| return employees.filter(e => | ||
| `${e.firstName ?? ''} ${e.lastName ?? ''}`.toLowerCase().includes(lower), | ||
| ) | ||
| }, [employees, searchValue]) |
There was a problem hiding this comment.
Seeing some of these repeated across the PRs, there may be opportunities for shared utils?
There was a problem hiding this comment.
Good call — extracted a useSelectEmployeesData hook that consolidates the employee list fetching, mapping, filtering, pagination, selection state, and search handlers that were duplicated across both containers. Both SelectEmployeesHoliday and SelectEmployeesTimeOff now call into it.
serikjensen
left a comment
There was a problem hiding this comment.
Seems like there's some duplication here that we may be able to consolidate with a shared hook or utils. i trust you'll be able to resolve that
| const handleContinue = useCallback(async () => { | ||
| if (mode === 'wizard') { | ||
| onEvent(componentEvents.TIME_OFF_HOLIDAY_ADD_EMPLOYEES_DONE, { | ||
| employeeUuids: [...selectedUuids], | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| await baseSubmitHandler({}, async () => { | ||
| await addEmployees({ | ||
| request: { | ||
| companyUuid: companyId, | ||
| requestBody: { | ||
| version: policyData.holidayPayPolicy?.version ?? '', | ||
| employees: [...selectedUuids].map(uuid => ({ uuid })), | ||
| }, | ||
| }, | ||
| }) | ||
| onEvent(componentEvents.TIME_OFF_HOLIDAY_ADD_EMPLOYEES_DONE) | ||
| }) | ||
| }, [mode, baseSubmitHandler, addEmployees, companyId, policyData, selectedUuids, onEvent]) |
There was a problem hiding this comment.
actually this feels extremely similar to the last one i looked at 🤔
There was a problem hiding this comment.
Yep, same shape entirely. Refactored both containers to use useSelectEmployeesData — the shared hook now owns all the common logic and each container is left with only its policy-specific mutation and continue handler.
| }, | ||
| }, | ||
| }) | ||
| onEvent(componentEvents.TIME_OFF_HOLIDAY_ADD_EMPLOYEES_DONE) |
There was a problem hiding this comment.
Same feedback here where we might consider passing the result of the add employees call
There was a problem hiding this comment.
Done — now captures the result and passes result.holidayPayPolicy to onEvent, matching the existing response.timeOffPolicy pattern in SelectEmployeesTimeOff.
4afa962 to
c0c078a
Compare
…arch - Add min, max, maxLength passthrough props to Input and TextInput - Add search input with inline clear button to EmployeeTable - Add isFetching loading state and pagination support to EmployeeTable Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes the absolutely-positioned custom clear button and its SCSS.
The browser's built-in search cancel button is now used, which works
naturally with component adapter overrides since type is a standard
HTML attribute forwarded by any TextInput implementation.
Bridges native clear (fires onChange('')) to onSearchClear so
pagination resets in consumers remain intact.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fetches active employees and the holiday pay policy (for the required version field), manages selection/search/pagination state, and calls useHolidayPayPoliciesAddEmployeesMutation on Continue in standalone mode. In wizard mode, emits TIME_OFF_HOLIDAY_ADD_EMPLOYEES_DONE with selected UUIDs. No starting balance column (holiday policies don't support it) and no reassignment warning (employees can be in both holiday and time-off policies simultaneously). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…yees containers - Extract useSelectEmployeesData hook consolidating employee list fetching, mapping, filtering, pagination, selection, and search state shared between SelectEmployeesHoliday and SelectEmployeesTimeOff - Pass result.holidayPayPolicy to onEvent in SelectEmployeesHoliday standalone mode, matching the existing pattern in SelectEmployeesTimeOff - Wrap SelectEmployeesTimeOff handleContinue in useCallback Addresses code duplication and result-passing feedback from PR #1616 review. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ba3120e to
a0bd1ad
Compare
There was a problem hiding this comment.
Pull request overview
Adds a holiday-pay-policy “Select Employees” container and refactors shared employee-list selection/search/pagination logic into a reusable hook, along with tests and endpoint inventory/docs updates for the new holiday add-employees endpoint.
Changes:
- Introduces
SelectEmployeesHolidaycontainer with standalone vs wizard mode behavior. - Extracts shared employees list + selection + search + pagination logic into
useSelectEmployeesDataand updatesSelectEmployeesTimeOffto use it. - Adds
SelectEmployeesHolidayunit tests and updates endpoint reference/inventory to include the holiday add endpoint.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/UNSTABLE_TimeOff/TimeOffManagement/SelectEmployees/useSelectEmployeesData.ts | New shared hook for paginated employees fetch, selection state, and search filtering. |
| src/components/UNSTABLE_TimeOff/TimeOffManagement/SelectEmployees/SelectEmployeesTimeOff.tsx | Refactors Time Off container to use the shared hook and simplifies search/selection wiring. |
| src/components/UNSTABLE_TimeOff/TimeOffManagement/SelectEmployees/SelectEmployeesHoliday.tsx | New Holiday container that fetches holiday policy version and adds employees (or emits wizard event). |
| src/components/UNSTABLE_TimeOff/TimeOffManagement/SelectEmployees/SelectEmployeesHoliday.test.tsx | New tests covering holiday selection UI, standalone mutation behavior, and wizard event behavior. |
| docs/reference/endpoint-reference.md | Adds the holiday add-employees PUT endpoint to the UNSTABLE_TimeOff endpoint tables. |
| docs/reference/endpoint-inventory.json | Adds the holiday add-employees PUT endpoint to the endpoint inventory entries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const result = await addEmployees({ | ||
| request: { | ||
| companyUuid: companyId, | ||
| requestBody: { | ||
| version: policyData.holidayPayPolicy?.version ?? '', |
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| [employeesData.httpMeta.response.headers, isFetching, currentPage, itemsPerPage], |
Summary
SelectEmployeesHolidaycontainer component for holiday pay policiesuseEmployeesListSuspensewith paginationuseHolidayPayPoliciesGetSuspensestandalonemode (callsuseHolidayPayPoliciesAddEmployeesMutationdirectly) andwizardmode (emitsTIME_OFF_HOLIDAY_ADD_EMPLOYEES_DONEevent)Test plan
npm run test -- --run src/components/UNSTABLE_TimeOff/TimeOffManagement/SelectEmployees/SelectEmployeesHolidayPart of stacked PR series for SDK-567:
feat/text-input-employee-table-search→ mainfeat/select-employees-presentation→ PR 1feat/select-employees-time-off→ PR 2feat/select-employees-holiday→ PR 3feat/select-employees-flow-integration→ PR 4🤖 Generated with Claude Code