feat(Dashboard): fix pending compensation edit — route date field by job type by serikjensen · Pull Request #1909 · Gusto/embedded-react-sdk

serikjensen · 2026-05-22T00:49:22Z

Summary

Primary new jobs (only future-dated comp, no current comp): replaces the Effective date field with a Hire date field. On submit, both hire_date on the job and effective_date on the compensation are set to the same chosen value, preventing the Gusto API from auto-creating a second compensation to fill the gap at the original hire date.
Secondary new jobs: floors the Effective date picker at max(tomorrow, job.hireDate) using local-time date parsing (normalizeToDate) to avoid a UTC off-by-one that made dates one day before the hire date selectable.
Add Another Job: applies the same max(tomorrow, primaryHireDate) floor to the Effective date picker for consistency.
Existing pending-comp-change edits (non-new jobs) are unchanged.

Test plan

Add a primary new job with a future hire date → edit comp → verify Hire date field shown (no Effective date field), save → confirm no duplicate compensation created
Add a secondary new job with a future hire date → Add Another Job flow → verify Effective date picker can't go earlier than the hire date
Edit a pending compensation on a secondary new job → verify Effective date picker is floored at the hire date (not one day before)
Edit a pending compensation on an existing job (non-new) → verify Effective date picker is floored at tomorrow, unchanged behavior
npm run test -- --run src/components/Employee/Compensation/management/EditPendingCompensation/EditPendingCompensation.test.tsx — 10 tests pass

Screen.Recording.2026-05-21.at.6.44.37.PM.mov

Made with Cursor

gusto-fresh-eyes · 2026-05-22T01:01:20Z

Fresh Eyes Review

Found 8 issues in this PR.

PR Description Issues

Minor | [fresh_eyes]: description-check: Undocumented Dashboard routing change: DashboardComponents.tsx now uses getPendingCompensationChanges to decide whether to route to EditPendingCompensation vs EditCompensation. This routing logic is the core integration point but is only implicitly described.
Major | [fresh_eyes]: description-check: Undocumented behavior change: useCompensationForm now filters the current compensationId out of futureCompensations, changing how maximumEffectiveDate is computed in update mode. This is a behavioral change with potential side effects on existing EditCompensation usage that the description doesn't mention.
Major | [fresh_eyes]: description-check: Undocumented refactoring: The FormBody from EditCompensation was extracted into a new shared ManagementCompensationFormBody component (~208 lines). This is a significant architectural change that isn't mentioned in the summary — reviewers should know this component is reused by both EditCompensation and EditPendingCompensation.

Additional Findings (outside diff)

🟠 Major | [fresh_eyes]: code-duplication | src/i18n/en/common.json:L95-L101
Duplicate JSON key compensationRateFormats added by this PR — the file already contains this key (with identical content) at line 81. JSON does not error on duplicate keys but only the last one takes effect, making the earlier instances dead code. The file now has 4 copies of this block. Remove the duplicate(s) and keep a single instance.

Info (2)

description-check: PR size is ~1200 lines across 13 files (including ~400 lines of tests). While tests are a large portion, the core changes span multiple components and a shared form body extraction — consider whether the refactoring could be split from the new feature.
🔵 Info | [fresh_eyes]: test-coverage | src/components/Employee/Compensation/shared/useCompensationForm/useCompensationForm.tsx:L293-L297
The new .filter(c => c.uuid !== compensationId) that excludes the comp being edited from the maximumEffectiveDate calculation has no dedicated unit test. The existing useCompensationForm test for maximumEffectiveDate uses a different compensationId that doesn't match the future comp, so the filter is not exercised in the unit test. Integration tests in EditPendingCompensation indirectly cover it, but the behavioral change (allowing the user to push the date past the current pending comp's date) isn't explicitly asserted.

Download findings.json — drag the file into Claude or use /add to propose fixes

^{Please 👍🏽 👎🏽 if you found this useful. Generated by Fresh Eyes Reviewer. Get help in #ai-code-reviews}

jeffredodd · 2026-05-22T00:59:18Z

@@ -5,6 +5,7 @@ import { useJobForm } from '../../shared/useJobForm'
 import { useCompensationForm } from '../../shared/useCompensationForm'


Do these files need tests?

jeffredodd · 2026-05-22T01:02:42Z

@@ -1,27 +1,17 @@
 import classNames from 'classnames'


Do these need tests?

- Derive pending compensation changes across all jobs using a new getPendingCompensationChanges helper; stacked future comps are diffed chronologically so each bullet remains meaningful. - Single job with a pending change: inline warning alert with bullet details (pay, title, FLSA status, minimum-wage adjustments) and a Cancel change button that fires DELETE /v1/compensations/{uuid}. - Multiple jobs with one pending change: inline alert includes the job title ("Compensation for {title} will change on {date}") so the affected job is unambiguous. - Multiple jobs with multiple pending changes: summary alert with a Review CTA that opens a modal listing every pending change by job. - Alert uses disableScrollIntoView; change details render as an UnorderedList; cancellation fires directly without a confirmation dialog. - Extend Alert primitive with an optional action prop for inline CTAs. - createSdkQueryClient shared factory aligns QueryClient defaults; mutations invalidate via their own cache keys so no manual invalidateQueries calls are needed. - payRateFormats in common.json updated to long-form ("per hour", "per year") matching design copy; weekly/monthly remain annualised. - New EMPLOYEE_COMPENSATION_CHANGE_CANCELLED event emitted on success. - Full unit and integration test coverage for all alert variants, cancellation success/failure paths, stacked future comps, and the review modal flow. Co-authored-by: Cursor <cursoragent@cursor.com>

Co-authored-by: Cursor <cursoragent@cursor.com>

… display Display weekly and monthly compensation rates as stored ($X per week, $X per month) rather than annualizing them. Introduces formatCompensationRate / useFormatCompensationRate alongside the existing formatPayRate so payroll contexts are unaffected. Also reverts payRateFormats i18n strings to original /hr /yr /paycheck compact format. Co-authored-by: Cursor <cursoragent@cursor.com>

…creating new When a job already has a pending (future-dated) compensation, clicking Edit now routes to ManagementEditPendingCompensation which sends a PUT to update the existing compensation rather than POSTing a new one. Extracts a shared ManagementCompensationFormBody and fixes useCompensationForm to exclude the compensation being edited from the maximumEffectiveDate constraint. Also fixes the alert container width in JobAndPayView to always be 100%, with padding applied only for multi-job layouts. Co-authored-by: Cursor <cursoragent@cursor.com>

Co-authored-by: Cursor <cursoragent@cursor.com>

For primary new jobs (hire date in the future, no current comp), show a Hire date field that keeps hire_date and comp effective_date in sync on submit, preventing the API from auto-creating a second compensation when the single pending comp moves off the hire date. For secondary new jobs, floor the effective date picker at max(tomorrow, job.hireDate) using local-time parsing to avoid UTC off-by-one issues. Apply the same hire date floor to the Add Another Job form for consistency. Co-authored-by: Cursor <cursoragent@cursor.com>

gusto-fresh-eyes · 2026-05-22T02:04:04Z

+  const primaryHireDateParsed = normalizeToDate(primaryHireDate)
+  const minEffectiveDate =
+    primaryHireDateParsed && primaryHireDateParsed > tomorrow ? primaryHireDateParsed : tomorrow


🟡 Minor | [fresh_eyes]: test-coverage

New minEffectiveDate logic using normalizeToDate(primaryHireDate) was added but AddAnotherJob has no test file at all. The date-flooring logic (max of tomorrow vs primary hire date) is identical to the pattern the PR is fixing for off-by-one issues and would benefit from a test.

^{Please 👍🏽 👎🏽 if you found this useful. Generated by Fresh Eyes Reviewer. Get help in #ai-code-reviews}

gusto-fresh-eyes · 2026-05-22T02:04:04Z

+    const loadingErrorHandling = composeErrorHandler([jobForm, compensationForm])
+    return <BaseLayout isLoading error={loadingErrorHandling.errors} />
+  }
+
+  // Secondary new job: effective date must be on or after the secondary job's
+  // hire date (and at least tomorrow) so we don't set a date before the job
+  // even starts.
+  const minEffectiveDate =
+    isNewJob && !isPrimaryJob && jobForm.data.currentJob?.hireDate


🟡 Minor | [fresh_eyes]: test-coverage

The secondary new job path (isNewJob=true, isPrimaryJob=false) that computes minEffectiveDate from the job's hire date is not covered by any test. Tests exist for isNewJob=false and for isNewJob+isPrimaryJob=true, but not this branch. Given the PR explicitly fixes a UTC off-by-one in date floor logic, this path deserves a test to prevent regressions.

^{Please 👍🏽 👎🏽 if you found this useful. Generated by Fresh Eyes Reviewer. Get help in #ai-code-reviews}

gusto-fresh-eyes · 2026-05-22T02:04:04Z

+  const minEffectiveDate =
+    isNewJob && !isPrimaryJob && jobForm.data.currentJob?.hireDate
+      ? new Date(
+          Math.max(
+            addDays(new Date(), 1).getTime(),
+            (normalizeToDate(jobForm.data.currentJob.hireDate) ?? addDays(new Date(), 1)).getTime(),
+          ),
+        )
+      : undefined


🟡 Minor | [fresh_eyes]: simplicity

Date-floor computation is hard to parse. The normalizeToDate(...) ?? addDays(new Date(), 1) fallback inside Math.max means both branches equal tomorrow when normalizeToDate returns null — making the Math.max a no-op in that path. Compare with the simpler pattern in AddAnotherJob.tsx (parsed && parsed > tomorrow ? parsed : tomorrow). Extract the tomorrow value once and use the same ternary pattern for clarity.

^{Please 👍🏽 👎🏽 if you found this useful. Generated by Fresh Eyes Reviewer. Get help in #ai-code-reviews}

gusto-fresh-eyes · 2026-05-22T02:04:04Z

+  const minEffectiveDate =
+    isNewJob && !isPrimaryJob && jobForm.data.currentJob?.hireDate
+      ? new Date(
+          Math.max(
+            addDays(new Date(), 1).getTime(),
+            (normalizeToDate(jobForm.data.currentJob.hireDate) ?? addDays(new Date(), 1)).getTime(),
+          ),
+        )
+      : undefined


🟡 Minor | [fresh_eyes]: code-duplication

Same minEffectiveDate = max(tomorrow, normalizeToDate(hireDate)) computation exists in AddAnotherJob.tsx (lines 77-82) with slightly different style (addDays vs tomorrow.setDate). Both encode the same business rule: 'effective date cannot precede the job hire date or tomorrow'. Consider extracting a shared helper like computeMinEffectiveDate(hireDate: string | undefined): Date to keep this rule in one place.

^{Please 👍🏽 👎🏽 if you found this useful. Generated by Fresh Eyes Reviewer. Get help in #ai-code-reviews}

serikjensen requested a review from a team as a code owner May 22, 2026 00:49

jeffredodd approved these changes May 22, 2026

View reviewed changes

serikjensen force-pushed the feat/dashboard-edit-pending-compensation branch from 0c952e3 to a277929 Compare May 22, 2026 01:12

serikjensen enabled auto-merge (squash) May 22, 2026 01:13

serikjensen and others added 6 commits May 21, 2026 19:50

chore: remove temporary screenshots taken during development

5a2cd7c

Co-authored-by: Cursor <cursoragent@cursor.com>

chore: regenerate i18n types

06cdd6b

Co-authored-by: Cursor <cursoragent@cursor.com>

serikjensen force-pushed the feat/dashboard-edit-pending-compensation branch from a277929 to d2fc1df Compare May 22, 2026 01:51

serikjensen merged commit 362d94f into main May 22, 2026
24 of 25 checks passed

serikjensen deleted the feat/dashboard-edit-pending-compensation branch May 22, 2026 01:58

gusto-fresh-eyes Bot reviewed May 22, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Dashboard): fix pending compensation edit — route date field by job type#1909

feat(Dashboard): fix pending compensation edit — route date field by job type#1909
serikjensen merged 6 commits into
mainfrom
feat/dashboard-edit-pending-compensation

serikjensen commented May 22, 2026 •

edited

Loading

Uh oh!

gusto-fresh-eyes Bot commented May 22, 2026 •

edited

Loading

Uh oh!

jeffredodd May 22, 2026

Uh oh!

jeffredodd May 22, 2026

Uh oh!

Uh oh!

gusto-fresh-eyes Bot May 22, 2026

Uh oh!

gusto-fresh-eyes Bot May 22, 2026

Uh oh!

gusto-fresh-eyes Bot May 22, 2026

Uh oh!

gusto-fresh-eyes Bot May 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -5,6 +5,7 @@ import { useJobForm } from '../../shared/useJobForm'
		import { useCompensationForm } from '../../shared/useCompensationForm'

Conversation

serikjensen commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

gusto-fresh-eyes Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fresh Eyes Review

PR Description Issues

Additional Findings (outside diff)

Uh oh!

jeffredodd May 22, 2026

Choose a reason for hiding this comment

Uh oh!

jeffredodd May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gusto-fresh-eyes Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

gusto-fresh-eyes Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

gusto-fresh-eyes Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

gusto-fresh-eyes Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

serikjensen commented May 22, 2026 •

edited

Loading

gusto-fresh-eyes Bot commented May 22, 2026 •

edited

Loading