Repository navigation
Refuse writes for employees who work at more than one location - #187
Conversation
Signed-off-by: tmfahey <taylor.fahey@gusto.com>
…efusal Signed-off-by: tmfahey <taylor.fahey@gusto.com>
…inputs on refusal Signed-off-by: tmfahey <taylor.fahey@gusto.com>
…to the review Signed-off-by: tmfahey <taylor.fahey@gusto.com>
jeff-gusto
left a comment
There was a problem hiding this comment.
Requesting changes for three issues in the payroll update multi-location path (see inline). The employee-command prechecks look good. I have a few smaller cleanup notes too and will send them separately.
|
A few smaller cleanup notes from the review. None of these block the PR.
|
Signed-off-by: tmfahey <taylor.fahey@gusto.com>
|
Thanks, all four are in 7efd738:
On counting future-dated addresses: the response has no end date or primary flag, so a planned move to a new location looks the same as a second location starting. Counting them would refuse ordinary relocations, so the check stays on addresses active today, and AGENTS.md calls out the gap. |
jeff-gusto
left a comment
There was a problem hiding this comment.
Thanks Taylor, 7efd738 covers everything. Checking before the write is the right fix, and agreed on dropping the update-work-address refusal since that PUT names its address. LGTM.
Summary
payroll updateortimesheet createcan't say which location they were worked at, and the API puts payroll hours on the employee's primary location without complaint.employee updatepicks which active address to overwrite, so with two of them it would be guessing.multi_location_not_supported(exit 8) and names them inerror.employee_uuids. Forpayroll update, clearing those employees' hours cells lets the rest of the file through, so bonus and tips still land.payroll updateonly looks up employees whose rows have hours, deduped and at most 4 at a time. A 429 is retried usingRetry-After. If a lookup fails, the employees it did find are still named; if it found no one, the write fails and says who couldn't be checked. A 404 is left for the update to report.--dry-runruns the same check when credentials are available.employee update-work-addressisn't refused. It changes only the address it names, so there's nothing to guess.--token-stdinnow keeps the piped token for the life of the process, sincepayroll updateresolves auth more than once. A test reset clears it between tests.payroll updatenow reports a stale version asversion_conflict(exit 8) withpayroll prepareas the fix, like the other versioned writes, instead of a genericapi_client_error.Linked issue
None
Test plan
bun run test:allpasses locally--agentand--humanoutput verified where touchedDCO
git commit -s) per the DCO