Skip to content

feat(PayrollList): disable Run Payroll on regular rows when transition payrolls are pending#1989

Open
larchai wants to merge 1 commit into
mainfrom
cl/transition-payroll-blocker
Open

feat(PayrollList): disable Run Payroll on regular rows when transition payrolls are pending#1989
larchai wants to merge 1 commit into
mainfrom
cl/transition-payroll-blocker

Conversation

@larchai
Copy link
Copy Markdown
Contributor

@larchai larchai commented Jun 2, 2026

Context

JIRA: EMBJPD-427 — FreshBooks asked for a systematic way to prevent regular payrolls from being run while a transition payroll is pending, instead of relying solely on the pay_schedule_transition partner notification + their own custom UI guardrail. The proper backend fix (returning pay_schedule_transition in payroll_blockers) isn't on a near-term roadmap and would still depend on partners consuming the blockers field. In the meantime, this PR closes the related gap in the SDK so it matches the guardrail already shipped in Gusto-hosted flows.

Current state vs. this PR

embedded-react-sdk (before) gws-flows embedded-react-sdk (after)
Run Payroll TransitionPayrollAlert surfaces pending transitions, but the per-row Run button in PayrollList is always enabled Warning banner + date-range select and submit button disabled when unprocessed transition pay periods exist Per-row Run Payroll button disabled on Regular rows when unprocessed transition pay periods exist within the next 90 days
Off-Cycle No blocker No blocker Unchanged — off-cycle is the path used to actually run a transition payroll

Per Jackson Cox's 4/15 testing on the JIRA ticket: if a user runs a regular payroll while a transition is pending, the regular run updates last_paid_date, drops the transition from Get Pay Periods, and a direct call to the transition payroll_uuid 500s. So today partners building on top of PayrollList had to add their own front-end guardrail — exactly the case FreshBooks didn't want.

What's in the PR

  1. New internal hooksrc/components/Payroll/useUnprocessedTransitionPayPeriods.ts. Wraps usePaySchedulesGetPayPeriods({ payrollTypes: Transition, endDate: now+90d }) + the !processed filter that previously lived inline in TransitionPayrollAlert. Not exported from the package barrel; consumed by both TransitionPayrollAlert and PayrollList.
  2. TransitionPayrollAlert.tsx — refactored to use the hook. Behavior-equivalent except for (3).
  3. Payroll.TransitionPayrollAlert.json — appended Regular payroll functionality is blocked until you either run or skip transition payrolls. to the alert description. The alert is now the explanation surface.
  4. PayrollList.tsx — calls the hook, threads hasUnprocessedTransitions into the presentation, emits RUN_PAYROLL_BLOCKED_BY_TRANSITION once when the disabled state activates. JSDoc comment documents the coupling.
  5. PayrollListPresentation.tsx — gates the per-row Run Payroll button on (payroll.offCycle === false) && hasUnprocessedTransitions. Off-cycle rows and the Run off-cycle CTA untouched. Disabled state communicated to screen readers via aria-label.
  6. New event constantrunPayrollEvents.RUN_PAYROLL_BLOCKED_BY_TRANSITION.
  7. docs/workflows-overview/run-payroll.md — coupling note added to the PayrollList section + new event row.
  8. Tests — three new presentation-level cases (disabled on Regular, enabled with no transition, off-cycle row stays enabled while Regular is disabled); two integration cases in PayrollList.test.tsx (event fires, button disabled when MSW returns a transition row); new assertion in TransitionPayrollAlertPresentation.test.tsx for the appended copy.

Explicitly out of scope

  • No changes to off-cycle behavior.
  • No backend dependency. No dependency on EMBJPD-427's backend payroll_blockers work — but the two are additive. If/when the backend starts returning a pay_schedule_transition blocker, the existing blockers prop on PayrollList would light up the existing Skip-gating logic; gating the Run button on it would be a small follow-up. Defense in depth, same as on Gusto.com.

Risks / open questions for the team

  1. Coupling between PayrollList and TransitionPayrollAlert is now load-bearing. The disabled state fires unconditionally when transitions exist; partners using PayrollList directly outside of PayrollLanding need to render an equivalent resolution surface or users see a disabled button without a path forward. Documented in JSDoc + the workflow doc. Open to feedback on whether to formalize this further (e.g. a wrapper context, or making TransitionPayrollAlert part of the public API and recommending it explicitly).
  2. title HTML attribute is dropped by react-aria-components. The design system's Button accepts title in its props type, but RAC's Button only forwards GlobalDOMAttributes which excludes title — so the existing title={t('runPayrollTitle')} calls in this file were effectively no-ops on the rendered DOM. Pivoted to aria-label for the disabled-state explanation (it IS forwarded by RAC and is the right a11y hook). A hovering sighted user won't see a native HTML tooltip on the disabled button itself — they get the explanation from the alert above. Worth a broader follow-up to either drop the unused title prop from ButtonProps or fix the forwarding upstream, but out of scope here.
  3. Tooltip copy tone. Currently A pending transition payroll must be run or skipped before running this payroll. (aria-label only). Mirrors the alert sentence tone. Easy to adjust if the team prefers softer copy.
  4. Observability event design. Fires once on initial detection of the disabled state, via a useEffect watching hasUnprocessedTransitions. Could instead emit on user attempt (e.g. click on the disabled button), but disabled buttons don't fire onClick, so detection-time is a more reliable signal for telemetry.

Test plan

  • npm run tsc clean
  • npm run i18n:generate regenerated src/types/i18next.d.ts
  • ESLint clean on touched files
  • npx vitest run src/components/Payroll — 576 tests pass, 1 expected fail (pre-existing, unrelated), 1 pre-existing flake on main (shows hamburger menu when pay period starts today)
  • Manual smoke test against the demo environment via sdk-app: verified baseline (no transition → button enabled), then exercised the flow on a demo company to confirm disabled state + alert copy + event emission. Off-cycle CTA unaffected.

…ns are pending

Mirrors the UI guardrail in Gusto-hosted flows: when the company has any
unprocessed transition pay periods within the next 90 days, the per-row
Run Payroll action on Regular rows is disabled. Off-cycle rows and the
Run off-cycle CTA stay enabled (off-cycle is the path actually used to
run a transition payroll).

Why: today the SDK shows a TransitionPayrollAlert but leaves the Run
Payroll button enabled. Running a regular payroll while a transition is
pending updates last_paid_date on the backend, drops the transition
from the Get Pay Periods response, and a direct call to the transition
payroll_uuid 500s — exactly the silent-failure scenario partners
(e.g. FreshBooks per EMBJPD-427) want guarded against. Gusto.com/flows
already enforce this client-side; the SDK now matches.

Changes
- New internal hook useUnprocessedTransitionPayPeriods, consumed by
  both TransitionPayrollAlert and PayrollList (not exported).
- PayrollList wires the hook + emits RUN_PAYROLL_BLOCKED_BY_TRANSITION
  when the disabled state activates.
- PayrollListPresentation gates the per-row Run button on Regular rows
  and surfaces the explanation via aria-label (the design system's
  Button forwards aria-* but not title).
- TransitionPayrollAlert copy extended with "Regular payroll
  functionality is blocked until you either run or skip transition
  payrolls." — the alert is now the explanation surface.
- docs/workflows-overview/run-payroll.md updated with the coupling note
  and the new event.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@larchai larchai requested a review from a team as a code owner June 2, 2026 01:27
Copy link
Copy Markdown
Member

@serikjensen serikjensen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few changes to pass to claude!

Comment on lines +121 to +126
useEffect(() => {
if (hasUnprocessedTransitions) {
onEvent(componentEvents.RUN_PAYROLL_BLOCKED_BY_TRANSITION)
}
}, [hasUnprocessedTransitions, onEvent])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to fire an event for this. Events are usually meant for the partner to action on after an action, ex. api response, navigation, etc.

We can revisit if there's partner need for component internals like this.

Longer term, there will be a hook for payroll list and that would just communicate this response as part of the hook response.

For now, i would recommend asking claude to remove the event along with the associated tests and documentation

const isRegular = !payroll.offCycle
const isDisabledByTransition = isRegular && hasUnprocessedTransitions

const ariaLabel = isDisabledByTransition ? t('runPayrollDisabledByTransitionTitle') : undefined
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have claude remove this aria label. We already set disabled which will get read by assistive tech

unprocessedPayPeriods,
hasUnprocessedTransitions: unprocessedPayPeriods.length > 0,
isLoading,
error,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of isLoading and error being returned, have Claude update the calls in this file to be suspense based queries. Embedded api has suspense variants of all api calls. The components where this is used are suspense based, so that way we can properly get the loading and error states configured. Otherwise with this configuration they'd need to be wired up separately.

@serikjensen
Copy link
Copy Markdown
Member

serikjensen commented Jun 2, 2026

also i'd have claude investigate the CI failure for endpoints that's breaking the build for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants