feat(schema): require site on every ingested collection
Tightened site from optional to required on fuel_logs, timesheet_entries, delivery_notes and purchase_orders now that every route writes it and every historical row has been backfilled. plant_hours_log was already required (matches docs/data-model.md's documented invariant, confirmed against production). timesheets (the document collection) was already required from Task 6 -- a no-op here, not a regression. fuel_sheets.site and plant_hours_sheets.site deliberately stay optional: those sheets can legitimately span multiple sites. Verified: creating a fuel_logs row with no site now gets a hard 400 (validation_required) from PocketBase itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
f184776148
commit
f007b5e8fb
2 changed files with 225 additions and 4 deletions
|
|
@ -2675,7 +2675,7 @@ trust it. An optional `site` is a row that can go missing from everyone.
|
|||
**Files:**
|
||||
- Create: `scripts/migrations/2026-09-14-require-site.py`
|
||||
|
||||
- [ ] **Step 1: Verify no row would violate the constraint**
|
||||
- [x] **Step 1: Verify no row would violate the constraint**
|
||||
|
||||
```bash
|
||||
PB_URL=https://pb.mega.infra.apphub.co.za PB_ADMIN_EMAIL=admin@mega.co.za PB_ADMIN_PASSWORD="$(grep '^POCKETBASE_ADMIN_PASSWORD=' .env.local | cut -d= -f2-)" python3 - <<'PY'
|
||||
|
|
@ -2703,7 +2703,7 @@ Expected: every line `0`, ending `safe to tighten`. **If any line is
|
|||
non-zero, go back and finish the relevant backfill — do not tighten and do
|
||||
not delete the offending rows.**
|
||||
|
||||
- [ ] **Step 2: Write the script**
|
||||
- [x] **Step 2: Write the script**
|
||||
|
||||
Following the Task 1 structure (same `admin_auth` / `api` helpers,
|
||||
`--apply` gated), for each collection below read its fields, find the one
|
||||
|
|
@ -2723,13 +2723,13 @@ sheets legitimately span sites — the fuel importer resolves a site per row
|
|||
from the `Current Site` column. Forcing one site onto the document would
|
||||
make the uploader pick a value that is not true.
|
||||
|
||||
- [ ] **Step 3: Dry run, apply, then prove it holds**
|
||||
- [x] **Step 3: Dry run, apply, then prove it holds**
|
||||
|
||||
After applying, attempt to create a `fuel_logs` row with no site via the
|
||||
PocketBase admin UI. Expected: PB refuses with a validation error naming
|
||||
the `site` field.
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
- [x] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add scripts/migrations/2026-09-14-require-site.py
|
||||
|
|
|
|||
221
scripts/migrations/2026-09-14-require-site.py
Normal file
221
scripts/migrations/2026-09-14-require-site.py
Normal file
|
|
@ -0,0 +1,221 @@
|
|||
#!/usr/bin/env python3
|
||||
"""
|
||||
Task 16 of the ingestion restructure: tighten `site` from optional to
|
||||
required on the collections where every row now has one written at
|
||||
create time (Tasks 11-13 made the routes write it; the backfills in
|
||||
Tasks 2-3-era migrations closed the historical gap).
|
||||
|
||||
PB_URL=https://pb.mega.infra.apphub.co.za \\
|
||||
PB_ADMIN_EMAIL=admin@mega.co.za \\
|
||||
PB_ADMIN_PASSWORD=... \\
|
||||
python3 scripts/migrations/2026-09-14-require-site.py [--apply]
|
||||
|
||||
DRY RUN BY DEFAULT.
|
||||
|
||||
Before running this, Step 1 of Task 16 (the plan's verification query)
|
||||
must show 0 rows with an empty `site` on every target collection. This
|
||||
script does not re-run that check - it assumes it has already been done
|
||||
and trusts the schema, not the data, from here.
|
||||
|
||||
Targets (site relation -> required=True, minSelect=1):
|
||||
fuel_logs - 147 rows were orphaned before the backfill; never again
|
||||
timesheet_entries - the future per-user allowlist filter reads this directly
|
||||
delivery_notes - already always set at upload
|
||||
purchase_orders - required at upload from Task 13
|
||||
timesheets - a wage sheet (the document, not timesheet_entries)
|
||||
belongs to one site
|
||||
|
||||
Assert-only, no write attempted:
|
||||
plant_hours_log - the plan claims this is already minSelect=1 in
|
||||
production. We read the live schema and fail loudly
|
||||
if that claim is wrong, rather than trusting it.
|
||||
|
||||
Explicitly NOT touched by this script (do not add them):
|
||||
fuel_sheets.site, plant_hours_sheets.site - these documents legitimately
|
||||
span multiple sites (the fuel importer resolves site per row from a
|
||||
spreadsheet column). Forcing one site onto the document would make the
|
||||
uploader assert something false.
|
||||
|
||||
Idempotent: a collection whose site field already has required=True and
|
||||
minSelect=1 is reported as a no-op, not re-patched.
|
||||
"""
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
|
||||
APPLY = "--apply" in sys.argv
|
||||
|
||||
TIGHTEN_TARGETS = [
|
||||
"fuel_logs",
|
||||
"timesheet_entries",
|
||||
"delivery_notes",
|
||||
"purchase_orders",
|
||||
"timesheets",
|
||||
]
|
||||
|
||||
ASSERT_ONLY_TARGETS = [
|
||||
"plant_hours_log",
|
||||
]
|
||||
|
||||
DO_NOT_TOUCH = ["fuel_sheets", "plant_hours_sheets"]
|
||||
|
||||
|
||||
def admin_auth(base_url: str) -> str:
|
||||
body = json.dumps(
|
||||
{
|
||||
"identity": os.environ["PB_ADMIN_EMAIL"],
|
||||
"password": os.environ["PB_ADMIN_PASSWORD"],
|
||||
}
|
||||
).encode()
|
||||
req = urllib.request.Request(
|
||||
f"{base_url}/api/collections/_superusers/auth-with-password",
|
||||
data=body,
|
||||
headers={"Content-Type": "application/json"},
|
||||
method="POST",
|
||||
)
|
||||
with urllib.request.urlopen(req) as resp:
|
||||
return json.loads(resp.read())["token"]
|
||||
|
||||
|
||||
def api(base_url, token, method, path, body=None):
|
||||
"""
|
||||
A 404 is only soft for GET - "this collection is absent" is a
|
||||
legitimate answer to a read. On a write it is not: a PATCH that
|
||||
404s returning None would be read downstream as a successful patch.
|
||||
"""
|
||||
data = json.dumps(body).encode() if body is not None else None
|
||||
req = urllib.request.Request(
|
||||
f"{base_url}{path}",
|
||||
data=data,
|
||||
method=method,
|
||||
headers={"Authorization": token, "Content-Type": "application/json"},
|
||||
)
|
||||
try:
|
||||
with urllib.request.urlopen(req) as resp:
|
||||
txt = resp.read().decode()
|
||||
return json.loads(txt) if txt else None
|
||||
except urllib.error.HTTPError as e:
|
||||
if e.code == 404 and method == "GET":
|
||||
return None
|
||||
raise RuntimeError(
|
||||
f"{method} {path}: {e.code} {e.read().decode() if e.fp else ''}"
|
||||
) from e
|
||||
|
||||
|
||||
def find_site_field(collection):
|
||||
return next((f for f in collection.get("fields", []) if f["name"] == "site"), None)
|
||||
|
||||
|
||||
def main() -> int:
|
||||
base_url = os.environ.get("PB_URL", "http://127.0.0.1:8090").rstrip("/")
|
||||
token = admin_auth(base_url)
|
||||
# Say where we are writing. The default target is production, and a
|
||||
# run whose output names no host is a run you cannot audit later.
|
||||
print(f"target: {base_url} mode: {'APPLY' if APPLY else 'DRY RUN'}")
|
||||
|
||||
print(f"\nnot touched (must stay optional): {', '.join(DO_NOT_TOUCH)}")
|
||||
|
||||
# --- Assert-only collections: plant_hours_log must already be tight.
|
||||
print("\n--- assert-only ---")
|
||||
for name in ASSERT_ONLY_TARGETS:
|
||||
coll = api(base_url, token, "GET", f"/api/collections/{name}")
|
||||
if coll is None:
|
||||
print(f"ERROR: collection {name!r} not found on {base_url}")
|
||||
return 1
|
||||
site = find_site_field(coll)
|
||||
if site is None:
|
||||
print(f"ERROR: {name} has no `site` field")
|
||||
return 1
|
||||
if site.get("required") is True and site.get("minSelect") == 1:
|
||||
print(
|
||||
f"{name}: OK, site already required=True minSelect=1 "
|
||||
f"(matches the plan's claim, not changing)"
|
||||
)
|
||||
else:
|
||||
print(
|
||||
f"DISCREPANCY: {name}.site is required={site.get('required')!r} "
|
||||
f"minSelect={site.get('minSelect')!r}, expected required=True "
|
||||
f"minSelect=1. The plan's documented invariant does not hold "
|
||||
f"in production. Not modifying this collection - flagging only."
|
||||
)
|
||||
return 1
|
||||
|
||||
# --- Tighten targets: patch if needed, no-op if already tight.
|
||||
print("\n--- tighten targets ---")
|
||||
to_patch = [] # (name, collection, updated_fields)
|
||||
for name in TIGHTEN_TARGETS:
|
||||
coll = api(base_url, token, "GET", f"/api/collections/{name}")
|
||||
if coll is None:
|
||||
print(f"ERROR: collection {name!r} not found on {base_url}")
|
||||
return 1
|
||||
site = find_site_field(coll)
|
||||
if site is None:
|
||||
print(f"ERROR: {name} has no `site` field")
|
||||
return 1
|
||||
|
||||
already_tight = site.get("required") is True and site.get("minSelect") == 1
|
||||
if already_tight:
|
||||
print(f"{name}: no-op, site already required=True minSelect=1")
|
||||
continue
|
||||
|
||||
print(
|
||||
f"{name}: would change site "
|
||||
f"required={site.get('required')!r}->True "
|
||||
f"minSelect={site.get('minSelect')!r}->1"
|
||||
)
|
||||
updated_fields = []
|
||||
for f in coll["fields"]:
|
||||
if f["name"] == "site":
|
||||
f = dict(f)
|
||||
f["required"] = True
|
||||
f["minSelect"] = 1
|
||||
updated_fields.append(f)
|
||||
to_patch.append((name, coll, updated_fields))
|
||||
|
||||
if not APPLY:
|
||||
if to_patch:
|
||||
print(
|
||||
f"\nDRY RUN. Would patch {len(to_patch)} collection(s): "
|
||||
f"{', '.join(n for n, _, _ in to_patch)}. Re-run with --apply to write."
|
||||
)
|
||||
else:
|
||||
print("\nDRY RUN. No collections need changes - all already tight.")
|
||||
return 0
|
||||
|
||||
for name, coll, updated_fields in to_patch:
|
||||
print(f"\nPATCH {name}: setting site.required=True, site.minSelect=1")
|
||||
patched = api(
|
||||
base_url, token, "PATCH", f"/api/collections/{name}",
|
||||
{"fields": updated_fields},
|
||||
)
|
||||
if patched is None:
|
||||
print(f" FAILED: PATCH {name} returned no collection")
|
||||
return 1
|
||||
|
||||
after = api(base_url, token, "GET", f"/api/collections/{name}")
|
||||
site = find_site_field(after) if after else None
|
||||
if site is None or site.get("required") is not True or site.get("minSelect") != 1:
|
||||
print(
|
||||
f" FAILED: post-patch read shows site required="
|
||||
f"{site.get('required') if site else None!r} "
|
||||
f"minSelect={site.get('minSelect') if site else None!r}"
|
||||
)
|
||||
return 1
|
||||
print(f" OK {name}.site is now required=True minSelect=1")
|
||||
|
||||
print(f"\napplied to {len(to_patch)} collection(s).")
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
try:
|
||||
sys.exit(main())
|
||||
except KeyError as e:
|
||||
print(
|
||||
f"ERROR: missing environment variable {e}. "
|
||||
f"Set PB_ADMIN_EMAIL and PB_ADMIN_PASSWORD.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(2)
|
||||
Loading…
Reference in a new issue