Expert IT Consulting for Business Excellence

Better yet, see us in person!

Kowdilya

Reviewed everything I could see, and I checked each fix against the domain rules rather than just its own test. **Short answer: most of them make sense — two are genuine bugs from my own iteration — but I would not commit this tree as it stands: the backend test module doesn't compile, and two files the new code hard-requires are untracked.** ⚠️ Note: the worktree changed *while* I reviewed (13 → 18 files, plus files created minutes ago), so this is a snapshot. ## Fixes that correct real bugs in my work | Fix | Why it's right | |---|---| | **`unavailableUnordered` guard restored** (`model.ts`, `TaskDetail.tsx`) | **The important one.** When I deleted the Hold action I also deleted the only guard for *"unavailable **and** not ordered"*. `Complete` would then PATCH `status: ORDER_PLACED` for a row the operator had explicitly said was **not** ordered — and `orderedRowsValid` couldn't catch it (it returns `true` when `!orderPlaced`). Since `ORDER_PLACED` reads as print-ready in the gate maths, that could open Printing for stones nobody ordered. Astra restored the predicate as a **validation, not a verb** (right for "the task can't be held as a whole"), disabled the button, named the reason in the bar's context line, and added defence-in-depth inside `submitProc`. | | **`ReaderZone.today()` instead of `current_date`** | My SQL used the *database server's* date for "overdue tasks" while the client computes due/overdue in the **reader's** zone — the two can disagree by a day. Now bound like the rest of the page, pinned by an IT that finds a zone whose date differs from the DB's. | | **Accept-dialog copy** | After the split-stage change my copy still said "a separate job at **print**" — wrong for wax/cast. Now `stageLabelShort(…)`, with a parametrised test asserting Wax/Casting and explicitly *not* "print". | | **IT fixture dates** | My hard-coded `2026-09-01` would drift past the 14-day threshold and silently change the test's meaning. Now `Instant.now().minus(1 day)`. | | **Factory harness DB guard** | Mine refused only the literal `workflow_db` — any other non-e2e database sailed through. Now an allow-list mirroring `global-teardown.ts`'s `_e2e(\Z\|_)`, refusing **before any write**, with adversarial unit cases (`workflow_db_e2evil`, trailing-newline names, non-strings, malformed metadata) and a CI step. The docstring now also admits the harness *changes users' permissions* — my wording claimed it only created orders. | | **Worklist desk gate** | I made the Command Centre's Procurement row a door for every admin but left `/work?scope=procurement` gated on `hasProcurement`, so the door led nowhere. `isAdmin \|\| hasProcurement` fixes it — and I verified it matches the server (`/procurement/stone-dashboard` guards with `requireOrderRead`, which admits the admin family). Findings-only holders keep the generic list. Pinned both ways. | | **Roster reconciliation** (IT `@BeforeEach` + e2e `syncWorkshopRoster`) | The shared Testcontainers DB and the reused e2e DB both leak routing state between classes/runs — my own factory harness grants capabilities to fixture users. Both harnesses now reconcile from **one** fixture, and `support.ts` loads the *same* JSON the backend loader reads (no second copy). Deactivating spec-created accounts instead of deleting them is the right call. | ## Blockers — this tree is not committable yet 1. **The backend test module doesn't compile.** `DesignRepositoryApiIntegrationTest:124` and `EnquiryDesignLineApiIntegrationTest` (same 4-line edit) call `patch("/admin/users/arno", "arif", …)` — and there is **no `patch` helper** in either class or in `IntegrationTestSupport`; it exists only as a private method in three *other* IT classes. `mvn verify` fails at `test-compile` (confirmed twice). The natural fix: hoist the shared HTTP helpers into `IntegrationTestSupport` (both classes already extend it, and that removes three duplicate copies). 2. **Two required files are untracked** → a clean checkout or CI fails: - `backend/src/test/resources/workshop-roster.json` — the new loader throws `Missing test resource` at static init, which kills **every** integration test (I hit exactly that in a run). - `frontend/e2e/global-setup.ts` — now referenced by `playwright.config.ts`. - Plus `scripts/test_factory_run.py` (the CI step runs it) and the roster scripts. 3. **Litter / naming:** `scripts/__pycache__/` is untracked (CI uses `python3 -B`; local runs don't). And a **tool** named `scripts/test_roster.py` sits beside its **test** named `scripts/test_test_roster.py` — a `unittest discover -s scripts` imports the tool as a test module. Renaming to `apply_workshop_roster.py` + `test_apply_workshop_roster.py` removes the trap. ## Nits (correct, but worth a line) - `await run(p().then(() => true))` works, but `runOk(...)` exists precisely to express "did this call succeed" — the endpoints do return bodies, so it's legitimate; it's just the older workaround. - The new in-function `submitProc` guards are unreachable in their test (the button is disabled first), so they're untested defence-in-depth. - `docs/stone-job-release.md` says cancellation keeps "its existing **holder** authorization" — the controller still also requires the **admin family**, so "admin-family holder" is the exact wording. ## Gate picture (mid-flight) Green: python guard test **4 ✓**, docs **2067 claims ✓**, `tsc` ✓, frontend **210 files / 1554 tests ✓** + build ✓. Red when I ran it: `mvn verify` (roster static-init, then the missing `patch` helper) — both in files that have changed since, so **re-run `mvn verify` and `./e2e.sh` once the edits stop** before committing. I deliberately didn't touch any of it — it's someone else's in-flight work and patching it mid-edit would collide. If you want, once Astra stops I'll fix the helper (hoist into `IntegrationTestSupport`), `git add` the required files, drop the `__pycache__`, and run the whole bar to give you a clean green.