prod release with better leave handling - #519
Merged
Merged
Conversation
Prod (KAN-326): posting payroll crashed with the xero-python error "Invalid value for pay_run_status (Deleted)" every run once a week's leave stopped matching. Three defects, all fixed here: - The differ keyed leave on a uniform hours-per-day, so mixed-hours leave (e.g. 4.5/8/8/8) could never match and was deleted-and-recreated every run. Verified live: Xero only preserves span + total units (one lumped period per pay week), so the key is now (type, span, total) and the builder emits one request per contiguous run of days, summing same-day entries. - delete_same_week_draft_pay_run built PayRun(pay_run_status="Deleted"), which the SDK rejects client-side — and the NZ Payroll API has no pay-run delete/update endpoint at all (GET-only; UI-only deletion), so the self-heal could never work. Removed it and its helpers outright. - When a stale leave overlaps a desired request of the same type it is now updated in place — verified live that Xero permits leave updates during a draft pay run, unlike deletes. Only counterpart-less leave is deleted; if Xero blocks that, DraftPayRunBlocksLeaveChange tells the operator which draft pay run to remove in the Xero UI, with the staff member's name attached by the orchestrator. create_employee_leave now takes the verified payload shape (single pay-week period with total units — per-day units are silently discarded by Xero). Tests rebuilt around real xero_python models so client-side SDK validation is actually exercised; the old suite mocked out the very function that crashed. scripts/verify_kan326_leave_contracts.py captured the live API contracts and stays until the demo tenant is cleaned up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnuHL7w3UisSk6KQ81q3Qf
Its captured API contracts are recorded in PR #518 and on the Jira ticket; the test leave and draft pay run it created on the demo tenant have been removed (draft deleted in the Xero UI, leave via --cleanup, stale mirror row purged). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnuHL7w3UisSk6KQ81q3Qf
…ment KAN-230 (db85844) made CostLine.clean() require labour_subtype on time lines but missed the three timesheet backfill commands, all of which hand-roll CostLine creation: create_leave_entries and create_overtime_entries built lines without it (crashing on the first save), and reclassify_overtime_entries dropped it when splitting a line into an OT copy. Worse, create_leave_entries --dry-run reported success for input that would crash, because it returned before any CostLine was even constructed. - Both create commands now build their lines via a module-level builder that sets labour_subtype from the staff default and fails early with a named-staff CommandError when the default is missing. - create_leave_entries --dry-run now performs the real saves inside a transaction and rolls back, so it exercises every model rule, entry_seq assignment, and DB constraint — it can no longer report success for entries a live run would reject. (Validating before save is impossible by design: CostLine.clean() requires entry_seq, which only save() assigns.) - create_overtime_entries validates and builds every CSV row before its existing atomic write; reclassify_overtime_entries copies the source line's labour_subtype. - New tests cover the builders: subtype carried and line saves (the regression that shipped), and the named-staff failure when a staff row has no default subtype. Folded into KAN-326 per user decision - same payroll surface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnuHL7w3UisSk6KQ81q3Qf
Typing the manual-command fix surfaced that CostLine.save/ _save_with_summary_update/_with_sequence_update_fields were untyped, so every typed caller tripped no-untyped-call. Mirror the django-stubs Model.save signature (keyword-only since Django 5) through the override chain, and type create_overtime_entries' validated rows as a TypedDict. Baseline shrinks by 30 lines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnuHL7w3UisSk6KQ81q3Qf
…ilures - _leave_total_units no longer falls back to number_of_units_taken: that is the consumed quantity, not the paid amount the reconciliation key is built on. A period without number_of_units now raises. - The delete_employee_leave handler persists the failure with its business context (operation, employee, leave id) before converting to DraftPayRunBlocksLeaveChange, matching the update path and the converting-handler pattern; persist_app_error idempotency keeps it to one row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnuHL7w3UisSk6KQ81q3Qf
Copilot review: _draft_pay_run_summary listed all mirrored drafts, so a stale row from a previously-connected tenant could be named in the operator guidance even though it does not exist in the Xero UI being described. Single-tenant-per-instance makes this mostly theoretical, but current-tenant scoping is the semantically correct query and matches ensure_pay_run_for_week's precedent of scoping mirror reads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnuHL7w3UisSk6KQ81q3Qf
…atus KAN-326: leave reconciliation without the impossible pay-run delete
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
📝 Description
Short explanation of what you’ve built and why.
🔗 Related Jira Work Item
Jira: KAN-123
Use the uppercase Jira key (
KAN-123) in the PR title or body so GitHub forJira links the development event. Jira Automation owns the transition to Done
when the PR is merged.
🚀 Changes
useChatcomposable, splitChatHistoryandChatInputcomponents, added Pinia store).✅ Checklist
Vue.js (Composition API)
<script setup>& composables), no heavy logic in templatesuseChat, etc.)propsandemit, no direct parent/child ref duplicationQuality & Formatting
npx prettier --check .)npx eslint . --ext .js,.ts,.vue)Definition of Done
anyor loose passthrough types reviewedSee docs/jira-usage.md#required-checks.