fix(SDK-1231): gate payroll prepare so it cannot wipe a completed calculation - #2656
Open
serikjensen wants to merge 6 commits into
Open
fix(SDK-1231): gate payroll prepare so it cannot wipe a completed calculation#2656serikjensen wants to merge 6 commits into
serikjensen wants to merge 6 commits into
Conversation
…culation PayrollConfiguration's prepare query was only gated on the transient `isCalculating` flag. When a calculation completed, `isPolling` cleared in the same tick that fired RUN_PAYROLL_CALCULATED, so for the render(s) before unmount `isCalculating` went false and the prepare query re-enabled. A cancelled-mid-calculation prepare has no cached success, so it refetched, hit the server, and reset `calculatedAt` to null. PayrollOverview then mounted, read the nulled payroll, and threw "payroll is not calculated". The multi-tab case reproduced it every time via a genuinely in-flight prepare. Fix: gate prepare on a durable signal, not just `isCalculating`. - Add `disablePrepare` to usePayrollConfigurationData; `enabled` now also requires `!disablePrepare`. - PayrollConfiguration sets it from a `hasStartedCalculationRef` latch (same tab) OR the server payroll status being calculating/calculated (direct loads + multi-tab). Prepare is only valid for a fresh, uncalculated draft. Remove the SDK-1018 band-aids that compensated for this root cause: - PayrollOverview `refetchOnMount: 'always'`, `gcTime: 0`, and the `isFetching` throw guard. - PayrollConfiguration post-calculate `invalidateQueries` (and now-unused useQueryClient / API_QUERY_NAMESPACE imports). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
serikjensen
force-pushed
the
payroll-not-calculated
branch
from
August 27, 2026 22:35
cbbfabd to
5e1ecd8
Compare
…lc mount The calculatedAt-based gate was too broad: returning to configuration via Edit reopens an already-calculated payroll, which legitimately needs prepare to re-fire. Gating on `isCalculatedStatus` left the employee table empty. Narrow the gate to the two cases that are actually the bug: prepare re-firing on the SAME mount that kicked off a calculation (the hasStartedCalculationRef latch), and the server being actively mid-calculate (isCalculatingStatus, covers multi-tab). A fresh mount on a calculated payroll now prepares as before. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…repare gate Fold every "don't prepare" reason into a single `disablePrepare` the caller owns, and drop the hook's separate `isCalculating` param so the query no longer reads `!isCalculating && !disablePrepare`. The hook now exposes one unambiguous gate used by both the prepare query and the excluded-details query, and its cancel effect keys off the same flag. Behavior is unchanged: disablePrepare = isPolling || isCalculatingPayroll || hasStartedCalculationRef || server-actively-calculating. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ating The per-mount latch keyed only on a local Calculate click, so a second tab that never clicked Calculate had nothing holding prepare off. When the first tab's calculation finished, the second tab's prepare re-fired and wiped `calculatedAt`. Replace `hasStartedCalculationRef` with `hasSeenCalculatingRef`, which trips the instant this mount observes a calculating payroll from any source (local click or another tab's calculation seen via the polled status). It stays true through the calculating -> calculated transition, so the wiping re-fire is blocked in both the single-tab handoff and the multi-tab case. A fresh mount that never sees "calculating" (Edit on an already-calculated payroll) leaves it false, so prepare still reopens the payroll. The guard releases on calculation failure/timeout so prepare can retry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A second tab suppresses prepare (it saw the payroll calculating), so it has no prepared rows. When the calculation finished, isPolling cleared and the full-page calculating loader dropped, exposing an empty table for a few seconds until navigation to overview completed. Drive the loader off a derived isCalculatingActive that includes hasSeenCalculatingRef, so the loader stays up continuously from calc start through the handoff on both tabs. A fresh Edit mount never sees calculating, so it still renders the table immediately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
serikjensen
marked this pull request as ready for review
August 28, 2026 00:34
Co-Authored-By: Claude Opus 4.8 <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
1 participant
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.
Problem
Navigating from Payroll Configuration to Payroll Overview sometimes threw "payroll is not calculated".
Root cause
The payroll prepare call reopens/resets a payroll, which clears
calculatedAton the server. Prepare was disabled on the react query call based on the payroll calculation status. Once payroll was calculated, prepare was switched back on. This created a race condition where if you didn't navigate quick enough, the prepare call would execute and clear the payroll calculated at value.Fix
We update to ensure the prepare call is disabled once calculate has been set on configuration. We also ensure if you come into a payroll mid calculation on configuration that it properly keeps prepare disallowed.
Cleanup
Removes the SDK-1018 fixes that were not actually getting at the root cause
refetchOnMount: 'always',gcTime: 0, and theisFetchingthrow guard.invalidateQueries(+ now-unuseduseQueryClient/API_QUERY_NAMESPACE).Testing
Forcing the behavior
We were able to force the behavior by delaying navigation from the configuration screen. This caused prepare to fire and clear the payroll.
Screen.Recording.2026-08-27.at.4.14.25.PM.mov
Fix
Screen.Recording.2026-08-27.at.4.21.02.PM.mov
🤖 Generated with Claude Code
Posted by Claude on behalf of Steve