Skip to content

Prod release 20260801 - #512

Merged
corrin merged 46 commits into
productionfrom
main
Aug 1, 2026
Merged

Prod release 20260801#512
corrin merged 46 commits into
productionfrom
main

Conversation

@corrin

@corrin corrin commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Prod release 20260801

Promotes main (verified on testing/UAT) to production (ADR 0029).

Included work

Deploy notes

  • 31 new migrations, including data passes (blank→NULL) and ~159 new CHECK constraints. Several data migrations are documented irreversible — take the standard pre-deploy DB backup.
  • Pre-deploy gate: run the blank-string sweep on each prod DB immediately before deploying. 108 columns gain CHECK (col <> '') with no in-run cleanup; any lingering '' row aborts the migration run partway. Every count must be 0 (or be NULLed manually first).
  • Post-deploy: watch AppError for IntegrityError/not_blank entries — any writer still emitting "" now fails loudly by design.
  • Non-NZ instances (if any) should review CompanyDefaults.gst_rate after deploy; the default is NZ GST 15%.

Verification

  • All CI checks green (backend tests, mypy gate, frontend lint/type-check/unit, pre-commit).
  • Full testing pass on UAT tracking main prior to this PR.

corrin and others added 30 commits July 28, 2026 08:53
Combines the open dependabot PRs into one sweep, upgrading as far as each
resolver allows rather than stopping at the individually proposed versions.

Backend (~90 packages): protobuf 5.29.6 -> 7.35.1, xero-python 14 -> 15,
mypy 2.1 -> 2.3, redis 7.4 -> 8.0, matplotlib 3.11.1, litellm 1.93.0,
pillow 12.3.0, holidays 0.101 (pin widened to <0.102).

Frontend (~410 lock entries): @vueuse/core 13 -> 14, eslint-plugin-vue
10.10.0, vue-router 5.2.0, vue-tsc 3.3.8, vite 8.1.5, eslint 10.8.0,
typescript-eslint 8.65.0, axios 1.18.1, dompurify 3.4.12, rrweb-player
2.1.1, plus the fast-uri and shell-quote advisories.

Drop google-generativeai: the deprecated legacy Gemini SDK, imported
nowhere (the code uses google-genai), and the sole constraint holding
protobuf below 6.0.

djangorestframework-stubs 3.17 tightens __in lookup and serializer
context types, surfacing four real errors: sets that may contain None
passed to id__in, a Dict[object, frozenset] passed to job_id__in, and
indexed assignment on a now-Mapping serializer context. Fixed at the
source rather than baselined; the remaining five new entries are message
re-wordings of existing errors, so the synced baseline still shrinks
(4396 -> 4388).

typescript 7, astroid 4.1.2 and importlib-metadata 9 are left behind:
each is pinned out by an upstream package's own latest release
(@typescript-eslint <6.1.0, pylint <=4.1.dev0, litellm <9.0).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qo7UkADwmj73int333bEnV
Version constraints in this repo record the newest version that passed
testing, not a known incompatibility, so a release sweep must ignore them
and re-test at latest. The first pass respected the declared bounds and so
could only move within the previous sweep's ceiling. This pass widens them.

Backend, previously capped by our own carets: django-filter 25 -> 26,
pyngrok 7 -> 8, drf-spectacular 0.29 -> 0.30, anthropic 0.120.0,
ruff 0.16.0, google-api-python-client-stubs 1.39.0.

Frontend: pinia 3 -> 4, pdf-vue3 1 -> 2, @types/node 25 -> 26,
prettier 3.8.3 -> 3.9.6 (reformats 8 files).

drf-spectacular 0.30 emits oneOf [format, {maxLength: 0}] for
format-constrained fields with allow_blank=True, which is a more honest
description of the contract: Company.email is EmailField(null=True,
blank=True) and 45 rows hold "". The regenerated client therefore relaxes
those fields from z.string().email() to a union. The previous schema was
the inaccurate one -- it would have rejected data the backend legitimately
returns.

Record the policy as ADR 0033 so the next sweep does not mistake a test
record for a compatibility bound.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qo7UkADwmj73int333bEnV
A text column had no single way to say "no value", and which way it used
varied by column. Company.email held 5 NULLs and 45 ""s, both meaning "no
email", so every reader had to test for both and several didn't.
PurchaseOrderLine.location said unset as NULL while Stock.location said it
as "" — the same fact spelled differently depending on the table. And
metal_type managed three spellings at once: NULL, "", and a
MetalType.UNSPECIFIED member that the frontend translated back to null
before display.

NULL is now the only unset, enforced by a CHECK (col <> '') constraint on
all 159 nullable text columns. The database is the only layer that can
hold the line: the admin, management commands, the Xero sync and raw SQL
all write straight past serializer validation, and each of them had put
empty strings in.

Either convention would have done — "" is what Django's docs recommend.
NULL wins on one measurable point: "" is not a valid email or URL, so a
blank-accepting EmailField forces drf-spectacular to emit
oneOf [format, ""], which openapi-zod-client collapses to
z.union([z.string(), z.string()]). That silently discarded every format
check in the generated client. This restores them: 32 such unions become
0, and .email()/.url() validation comes back.

Data: ~88,400 rows across 51 columns converted, plus 2,286 metal_type
rows. Two PurchaseOrderLine rows held 'steel', not a valid choice at all;
they become NULL rather than guessing which steel was meant. Migrations
are forward-only — reverse cannot tell a row that was "" from one that
was already NULL.

NullUnsetModelSerializer derives allow_blank=False from the model field so
the rule holds for every column without a per-field declaration, and keeps
holding when a column is added. Parameterising its 67 subclasses clears 52
long-standing type-arg violations; the baseline shrinks 4388 -> 4336.

BEHAVIOUR CHANGE: PATCHing "" to a CompanyDefaults URL previously returned
200 and stored NULL. It now returns 400. That canonicalization was
deliberate and tested; rejecting instead is the consistent rule applied to
the wire, and the settings form now sends null when a box is cleared.

Making rdti_type a real enum in the report serializers surfaced a live bug:
rdti-spend.vue compared against 'core' and 'supporting' when the values are
'core_rd' and 'supporting_rd', so both summary tiles were always empty and
every badge read "Supporting".

Two invariant tests keep this from decaying: one asserts every nullable
text column carries its CHECK constraint, the other that no serializer
field accepts "" for one. Both read live state, not source text.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qo7UkADwmj73int333bEnV
E2E found what the unit tests, mypy and the frontend suite could not: the
CHECK constraints from 9b7594a were firing on live write paths.

  ERROR apps.purchasing.views.purchasing_rest_views
    new row for relation "purchasing_purchaseorderline"
    violates check constraint "dimensions_not_blank"   (x17 -> HTTP 500)

Backend. Six write sites still produced "". FIELD_UPDATERS coerced with
`value or ""` on the purchase-order update path, which is the 500 above.
An audit over all 128 nullable text field names found five more that no
test touches: cost_line.desc, JobFile.mime_type, recording.sha256, and
Procedure.site_location in two places.

Dropping the `, ""` fallback from `.get()` was not enough on its own: it
handles a missing key, but the serializers here permit blank, so a key
present-and-empty still arrived as "" and was written through. The create
paths now coerce the value, not just the default.

Frontend. `allow_blank=False` makes DRF emit minLength: 1, so the
generated client rejects "" before the request is sent — no server log, no
console error, just a toast and a request that never leaves. Senders now
say null: the timesheet store and cost-line service (desc), contact-method
and phone-manager labels, PO line item_code, the AI provider model_name,
and procedure/form document_number and site_location.

Create and update contracts differ here and the type checker caught it:
FormCreateRequest defaults document_number to "" and forbids null, while
ProcedureUpdateRequest requires null and forbids "". Create paths keep "",
update paths send null. That asymmetry is a wart worth removing later.

Playwright: actionTimeout and navigationTimeout become 0, leaving the
120s test timeout as the only hard limit, as expectStepUnder already
documents. 120s exceeds any real page load, so reaching it is evidence of
breakage rather than slowness; a 30s cap made every such failure ambiguous
between the two. Slow-but-passing tests are still tagged perf-fail and the
run continues.

Verified against a full E2E run: the PO line item, staff wage, and three
timesheet tests all pass, with zero constraint violations across 107.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qo7UkADwmj73int333bEnV
Five E2E runs died on a 401 (#6, #22, #29, #34, #11), each on a different
test. The history shows it predates this branch; it is here because it
blocks this PR's gate.

The auth fixture navigates to an authenticated page and drives login
itself, so the app is mounted while unauthenticated and every already-
mounted component fires its load at once. From logs/access.log, one such
burst: kanban columns, /api/process/categories/, /api/accounts/staff/all/
and a session-replay chunk POST, ~10 requests, all 401.

The browser logs "Failed to load resource: ... 401" for each, and the
fixture fails a test on any unexpected console error (rule 30). The
allowance recorded one only for GET /api/accounts/me/, so most of the
burst was unconsumable by construction — whichever test was running when
it landed failed.

recordResponse now gates on a new isExpectedPreAuthResponse, which accepts
any 401 except the token endpoints: a 401 there means the credentials were
refused, which must keep failing. isUnauthenticatedSessionCheckResponse is
left alone because isLoginCompletionResponse uses it to decide when login
finished.

Both existing bounds are untouched, and they are what keep this from
becoming a blanket 401 mute: nothing is allowed outside an open login
window, and each console error still needs its own observed 401 within 5s.

401 is the right status and stays — it means no valid credentials, which
is true. Nothing in the app changes: the allowance exists only in the E2E
fixture's assertion, so production behaviour is byte-for-byte identical.
The tempting alternative, stopping the stores toasting on 401, is left
undone deliberately: nothing currently redirects on token expiry, so
suppressing those errors would trade noise for silent failure.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qo7UkADwmj73int333bEnV
The submit handler deleted model_name when the box was empty, so PATCH
never carried the field and an existing model name could not be cleared.
Send null instead — the column's unset is NULL and the schema marks the
field nullable on both create and update — matching the `|| null`
convention the rest of this change uses.

Also correct two comments that still described webhook_key's unset as
"" after it became NULL-only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0144Y9ZmM9QhbwAfGphHazp3
chore: upgrade dependencies and enforce NULL-only text values
…ess-reloads-the-whole-board-on-routine-job-changes-e-tags-can-miss-aggregate-changes

# Conflicts:
#	apps/job/serializers/job_serializer.py
#	apps/purchasing/serializers.py
#	frontend/schema.yml
#	frontend/src/api/generated/api.ts
The person-clear branch of handlePersonSelected queued and flushed an
autosave unconditionally, unlike the select branch which dedupes on the
same value. A server sync can null person_id before PersonSelector emits
its own clear, so the clear fired a PATCH that changed nothing and bumped
the resource ETag for no reason.

Guard the queue/flush on the previous value being non-null, keeping the
display resets and the init/hydrate/sync guards unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Syd6w1ZCbXzFiSex7hbcHn
…the-whole-board-on-routine-job-changes-e-tags-can-miss-aggregate-changes

fix: reconcile Kanban freshness incrementally and enforce ETags
…-branches

docs: separate integration from production releases
Type the hot test helpers that drove most no-untyped-call errors in the
test baseline. Model-building factories whose callers pass a known set of
overrides get Unpack[TypedDict] (the _stock, _row, _client, _job, XeroApp
factories); helpers that forward arbitrary model kwargs or pop a non-field
key (the phone-pop company builders, _make_job(**extra)) take object as a
no-regret fallback that a TypedDict can replace later. Also annotate the
production convert_html_to_reportlab (str -> str), which 24 test calls
depended on.

The baseline sync also records the production fixes from b2cb5ed that had
not yet been baseline-synced, so the diff reorders more than this commit's
test edits alone.
.qwen/settings.json holds developer-specific permission grants written by
the Qwen Code CLI during a session; like .codex/ and .anthropic/ it is
local tooling state, not project config.
Second pass over the hot test helpers: the XeroApp/quota, XeroPayItem,
Company, Job, and service factories. Most take Unpack[TypedDict] keyed on
the override fields callers actually pass — including the company builders
that pop a non-field phone key before create(), which a TypedDict describes
fine once phone is a declared key. _consume keeps Iterator[object] because
sync_xero_data is untyped and yields unschema'd dict events; _job casts its
create() result, which django-stubs types as Any under Unpack kwargs.
Both classes inherited BaseTestCase and reassigned self.client = APIClient(),
so mypy still saw Django's Client and flagged force_authenticate and
response.data. Inheriting BaseAPITestCase types self.client as DRF's
APIClient, which removes the redundant assignment and the attr-defined
errors. Also tighten a few Optional payloads in the schema tests with
assert-narrowing and a list[str] annotation.
- Type sync_xero_data as Iterator[XeroSyncEvent] (reusing the existing
  TypedDict) and _consume likewise, so e.get("severity") checks against a
  real contract instead of object.
- Widen the nullable override keys to match the models: xero_contact_id in
  the company builders and access_token/refresh_token in _active_app.
- Keep the cast in the _job data-version helper but document why: the custom
  JobQuerySet.create() takes a staff= kwarg that is not a model field, so
  django-stubs can neither type its return nor accept the kwarg; annotating
  the manager trades the cast for a misc error and an internal no-any-return.
corrin and others added 16 commits July 31, 2026 14:30
mypy-baseline stores entries in mypy's output order, which shifts whenever
code changes perturb module processing order, so every sync rewrote the whole
file and buried the real add/remove in reorder churn. Adopt the tool's
--sort-baseline flag in the documented sync command (check_mypy.sh, CLAUDE.md)
and re-sort the existing baseline once. The re-sort is a pure reorder (no
entries added or removed); the filter gate is order-independent and still
passes.
…anup

KAN-316 test: remove low-hanging test mypy debt
Nothing in DocketWorks ever writes Staff.last_login. Authentication is
JWT-only: SIMPLE_JWT leaves UPDATE_LAST_LOGIN at its False default, and
CustomTokenObtainPairSerializer authenticates via authenticate(), which
never emits the user_logged_in signal that Django's update_last_login
receiver listens for. django.contrib.admin is not installed either, so
there is no session-login path. The inherited AbstractBaseUser column
has been NULL by construction.

Admin > Staff nonetheless rendered a permanently blank "Last Login"
column, and StaffFormModal round-tripped the value back to the server
with no input bound to it. Worse, StaffCreateSerializer listed only
wage_rate as read-only, so POST /api/accounts/staff/ accepted a
client-supplied last_login -- an API client could forge a login time.

Remove last_login from STAFF_API_FIELDS and from the admin staff table
(fixing the empty-state colspan, which was already off by one), and drop
the dead last_login/date_joined round-trip from StaffFormModal. The DB
column stays; it belongs to AbstractBaseUser.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018CvGn5ojk7NtQzr5WmzAEi
`getattr(account, "description", None)` and
`getattr(account, "enable_payments_to_account", False)` both imply the
attribute might be absent. Neither ever is: both are declared in
`Account.openapi_types` and initialised in the xero-python constructor,
so the defaults are unreachable. Verified against all 166 XeroAccount
rows — both keys present in every raw payload, and
`enable_payments_to_account` is never null (148 False / 18 True).

Pure dead-code removal, no behaviour change. The sibling fields in the
same dict already use direct access (`account.type or None`), so this
also removes an internal inconsistency.

`or None` is retained on `description` — 57 of 166 accounts come back
from Xero with `""`, and that clause is what turns them into NULL. It is
deliberately not added to `enable_payments`: coercing a null to False
would mask a genuinely surprising value, where the NOT NULL constraint
fails early instead.

Raised by CodeRabbit on PR #505.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016TZsTtMB6opqWJFwzrUteK
…production-data

KAN-321 Fix NULL-only text failures in UAT provisioning
DocketWorks needs a sales-tax rate to quote amounts that Xero has not yet
invoiced (the Finish Job remaining balance). The only rate in the codebase was
a cosmetic constant in the read-only Xero provider, which meant no authoritative
source and two places to change.

gst_rate is a per-client tunable on CompanyDefaults (0.1500 = 15% NZ GST) and
the read-only provider now reads it, so the system has one rate. The dynamic
settings UI picks the field up from COMPANY_DEFAULTS_FIELD_SECTIONS, so it is
editable without a hand-written input.

KAN-323

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ACLSprAHJSeQErMxQqLKDH
…cklist

The counter question — what does this customer owe, including GST — had no
authoritative answer. JobActualTab computed "to be invoiced" in the browser from
a quote total and an invoice list, which is a business calculation in the
presentation layer (ADR 0020) and silently ignored what Xero still expects to be
paid.

The summary service answers it server-side: job value on its own pricing basis,
valid invoiced, outstanding Xero balances, the positive remaining amount, GST on
it, and Total to pay. Over-invoicing is reported as a separate diagnostic rather
than netted off, so a T&M job whose actuals came in under its invoices shows the
excess instead of hiding it behind a smaller balance.

The job-value basis, price cap and valid-invoice statuses are extracted from
invoice_calculation rather than reimplemented, so the amount a user can invoice
and the amount they are shown cannot drift apart.

Only the basis cost set is loaded, with its cost lines prefetched: the n+1 guard
caught both a query-per-cost-line and, once fixed, an eager load of the cost set
the summary never reads. A query-count test pins it.

The completion checklist is deliberately advisory — tests pin that confirming
items changes neither job status nor invoice eligibility. Each changed item
writes one job-history event, including withdrawals, which is the change most
worth finding later.

Also drops .__func__ from the JobEvent description registry: staticmethod objects
have been directly callable since 3.10, and removing it clears 23 baselined mypy
errors instead of adding a 24th.

KAN-323

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ACLSprAHJSeQErMxQqLKDH
Cost Analysis was a read-only comparison table almost nobody opened, while
Actual mixed cost entry with the invoice controls. Neither answered the question
staff actually have at the counter with the customer standing there: what do
they pay, including GST.

Finish Job is that one place. It leads with the server-owned customer balance —
subtotal, GST, unpaid invoices, Total to pay — then the invoice card, modal,
history, Xero links and deletion moved intact from Actual, then the completion
checklist, then the cost comparison retained in compact form underneath.

Actual keeps materials, adjustments, its summary and detailed cost entry. Its
Invoiced and To Be Invoiced chips are gone, along with the toBeInvoiced
computation behind them: invoice figures now have exactly one authoritative
home, and no quote-minus-invoices arithmetic happens in the browser.

Behaviour changes worth naming:
- a fully covered job offers no invoice action at all, instead of an
  always-visible button that would raise a $0 invoice;
- invoice wording follows the stage of work — advance/deposit/progress before
  completion, remaining balance after — without gating invoicing by status;
- the balance is re-read from the server after create and delete rather than
  patched locally, because it is only correct against the current invoice set.

KAN-222 ships here too: labour budget, hours used, and remaining-or-overrun, in
the component that would otherwise have to be reopened next week to add them.

The checklist rollback uses v-model rather than a one-way :checked binding —
with the latter, Vue cannot repaint a value that never changed in state, so a
failed save left a tick showing that the server had rejected.

The invoice E2E now drives Finish Job and additionally asserts the acceptance
criterion the old spec could not see: after invoicing in full, the job shows
nothing remaining and stops asking for another invoice.

KAN-323, KAN-222

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ACLSprAHJSeQErMxQqLKDH
A fixed three-column grid put the labour budget, used and remaining figures into
roughly 100px each on a phone, which is where staff read them while standing at
the job. They stack below the sm breakpoint now.

KAN-323

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ACLSprAHJSeQErMxQqLKDH
The completion checklist shipped as a separate JobCompletionChecklist table with
a OneToOne back to Job. Being 1:1, that was columns with a join in front of them,
and it cost more than the join: a get_completion_checklist() that returned an
unsaved instance for jobs with no row yet, updated_at/updated_by columns that
recorded who last touched the checklist rather than who ticked what, and a
service that hand-wrote a JobEvent per item.

Job already does all of that. _FIELD_HANDLERS turns any tracked field change into
a job-history event, which is where "Marked as paid" comes from. So the items are
Job booleans now, the table and its service are gone, and the per-item audit is
the machinery that was already there.

The items also changed to the questions the front desk actually works through:
foreman signed off, timesheets collected, materials checked, customer called,
released. Same five on every job — urgency differs between T&M and quoted work,
but that is a note under the list, not a branch in the schema, so the
conditional rendering goes too.

collected is renamed to released rather than replaced. It meant the same thing
and was never writable — no serializer, endpoint, admin or UI ever set it — so
every row holds the default and the rename carries nothing stale forward. This
also drops customer_approval_confirmed: a customer is not handed work they have
just rejected, so the tick is implied by release for a collection, and for a
delivery it cannot be answered at the moment it would be ticked.

Migration 0010 is replaced rather than added to, since nothing has been pushed.

KAN-323

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ACLSprAHJSeQErMxQqLKDH
…alance

Review found this PR re-implementing totals logic the app already had, and
presenting invented figures as fact. Both are fixed by deletion.

pricing_methodology was being branched on four times to answer one question —
what is this job worth. calculate_invoice_amount dispatched on it and then the
handlers asked again via two functions this PR had added; get_job_total_value
had its own copy reading the summary["rev"] float mirror; and
recalculate_job_invoicing_state had a fourth, which is how fully_invoiced could
ignore a price cap the invoice itself respects. There is now one
get_job_invoicing_basis and all of them call it, so a job cannot be reported at
one value and invoiced at another. A test pins the agreement, and another pins
the price cap the old reporting path dropped.

Three more things existed already and were being duplicated:
get_job_for_invoice_calculation (this PR had written a second loader),
Invoice.amount_due (already serialized), and useJobFinancials.ts — a third copy
of the same sum in the browser, which counted VOIDED invoices and ignored the
price cap, so the Rejected-Job warning disagreed with what the job would
actually be invoiced for. It is deleted and that warning now reads the server's
figure.

The basis field went too: it was pricing_methodology with the values relabelled,
returned by the API and never read.

On the frontend, loadAll's catch cleared `loading` while leaving a fabricated
zero summary on screen, so a failed request rendered "Total to pay $0.00 —
Nothing left to pay. This job is fully invoiced and settled." There is no
placeholder now; a failed load says so and shows no figures. Tested.

Job.DoesNotExist was re-raised as ValueError inside an except clause, which a
sibling clause cannot catch, so every job view answered 500 for a missing job.
http_status_for_exception already maps it to 404, so the clauses are deleted
across the file and a test covers it.

Structurally: the invoice card moves to JobInvoiceCard.vue and the checklist
items are declared once as Job.COMPLETION_CHECKLIST_FIELDS, which both
serializers derive from. JobFinishTab is now recognisably a rename of
JobCostAnalysisTab rather than a rewrite of it.

KAN-323

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ACLSprAHJSeQErMxQqLKDH
…icated

The completion-checklist tests used force_login, which only ever worked by
accident: JWTAuthentication is the sole authenticator, and it short-circuits
to None when ENABLE_JWT_AUTH is off, so the session force_login creates is
never consulted. Locally .env turns JWT auth on and the authenticator's
middleware fallback honours the session; CI copies .env.precommit, which
turns it off, so every request 401'd. force_authenticate replaces the
authenticator list outright and is what the rest of the suite uses.

Also from review:

- Scope the checklist save to the ticked fields. A bare Job.save() rewrites
  every column held on the instance, so a concurrent write to fully_invoiced
  or status was reverted to whatever this request read first.
- Hold gst_rate to a fraction. The field was unbounded, and the API advertised
  -10 < rate < 10; a CHECK constraint carries it because the admin, management
  commands and the Xero sync all bypass serializer validation.
- Report a failed balance reload after invoicing instead of leaving the
  pre-invoice figure on screen looking current.
- Drop the casts around invoice creation. The mode arguments were typed str
  and only fit XeroInvoiceCreateRequest because a cast hid it, and the error
  payload was typed with a property the code below it did not read.
- Give the invoice list stable automation ids so its tests stop selecting by
  CSS class and list position.
The open and delete buttons on each invoice row carry only an icon, so a
screen reader announced them as unlabelled buttons. The label includes the
invoice number because these render once per row — five buttons all reading
"Delete" would not say which invoice they act on.
…e-with-gst-inclusive-customer-balance

KAN-323: Finish Job workspace with GST-inclusive customer balance
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e5a7c343-3e21-4579-bbf4-b2f13b3ef130

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@corrin
corrin requested a review from Copilot August 1, 2026 09:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@corrin
corrin merged commit d55cca0 into production Aug 1, 2026
12 checks passed
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