fix(timesheets): stamp site on log rows, ending read-time drift
The timesheet log carried neither site nor project_code; attribution was synthesised at read time from employees.site, so moving a worker to another site silently migrated their entire payroll history with them. docs/data-model.md flags this as the schema's worst drift risk. Adds an optional site relation (maxSelect 1, cascadeDelete false so deleting a site cannot destroy payroll history) and stamps all 1367 rows from the employee's current site. 1367/1367 written, 0 orphans. Tightened to required in Task 16, once the import route writes it on every new row. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
DEls
parent
9117b73cd4
commit
a0d335b235
1 changed files with 276 additions and 0 deletions
276
scripts/migrations/2026-09-11-backfill-timesheet-sites.py
Normal file
276
scripts/migrations/2026-09-11-backfill-timesheet-sites.py
Normal file
|
|
@ -0,0 +1,276 @@
|
|||
#!/usr/bin/env python3
|
||||
"""
|
||||
Add timesheets.site and backfill it from employees.site.
|
||||
|
||||
PB_URL=... PB_ADMIN_EMAIL=... PB_ADMIN_PASSWORD=... \\
|
||||
python3 scripts/migrations/2026-09-11-backfill-timesheet-sites.py [--apply]
|
||||
|
||||
DRY RUN BY DEFAULT. Pass --apply to write.
|
||||
|
||||
WHY THIS RUNS NOW
|
||||
-----------------
|
||||
Today the timesheet log has neither site nor project_code; site is
|
||||
synthesised at read time from employees.site. That is the collection with
|
||||
the worst attribution drift in the system (docs/data-model.md section 2):
|
||||
move a worker between sites and their entire payroll history silently
|
||||
follows them to the new site.
|
||||
|
||||
Stamping site at import time fixes that permanently, and makes the future
|
||||
per-user site allowlist a single `site IN (...)` filter clause instead of
|
||||
a join through employees.
|
||||
|
||||
THE SAFETY PROPERTY
|
||||
-------------------
|
||||
site is resolved from exactly one source: the employee's own site
|
||||
relation. There is no second source to cross-check against, so the rule
|
||||
is simply that a row whose employee does not resolve to exactly one site
|
||||
is reported with the employee it points at and skipped - never guessed.
|
||||
Inventing a fallback site here would write a wrong payroll attribution
|
||||
that looks indistinguishable from a correct one afterwards.
|
||||
|
||||
The field is added optional. PocketBase rejects a required field that
|
||||
existing rows already violate, so tightening to required happens in
|
||||
Phase 3 once the import route writes it on every new row.
|
||||
|
||||
Idempotent: rows that already carry a site are left alone.
|
||||
"""
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
|
||||
APPLY = "--apply" in sys.argv
|
||||
COLLECTION = "timesheets"
|
||||
|
||||
|
||||
def admin_auth(base_url):
|
||||
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/record is absent" is a
|
||||
legitimate answer to a read. On a write it is not: a PATCH against a
|
||||
since-deleted record returning None would be counted as a successful
|
||||
write. Mutations raise.
|
||||
"""
|
||||
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 get_all(base_url, token, coll, fields):
|
||||
out, page = [], 1
|
||||
while True:
|
||||
data = api(
|
||||
base_url,
|
||||
token,
|
||||
"GET",
|
||||
f"/api/collections/{coll}/records"
|
||||
f"?perPage=500&page={page}&fields={fields}",
|
||||
)
|
||||
out.extend(data["items"])
|
||||
if page >= data["totalPages"]:
|
||||
return out
|
||||
page += 1
|
||||
|
||||
|
||||
def rel_id(value):
|
||||
"""
|
||||
Normalise a PocketBase relation field to a single id, or "" when it
|
||||
does not resolve to exactly one.
|
||||
|
||||
Single relations come back as a bare string, multi relations as a
|
||||
list. employees.site being a multi-relation with two entries is not
|
||||
an answer to "which site does this employee's labour belong to" - it
|
||||
is an ambiguity, and writing element [0] would silently pick one.
|
||||
Anything that is not exactly one id is "unresolved", which routes the
|
||||
row to the skip report rather than to a write.
|
||||
"""
|
||||
if isinstance(value, list):
|
||||
return value[0] if len(value) == 1 else ""
|
||||
return (value or "").strip()
|
||||
|
||||
|
||||
def main():
|
||||
base_url = os.environ.get("PB_URL", "http://127.0.0.1:8090").rstrip("/")
|
||||
token = admin_auth(base_url)
|
||||
# Say where we are writing. This script mutates production by default
|
||||
# target, and a run whose output names no host is a run you cannot
|
||||
# audit afterwards.
|
||||
print(f"target: {base_url} mode: {'APPLY' if APPLY else 'DRY RUN'}")
|
||||
|
||||
coll = api(base_url, token, "GET", f"/api/collections/{COLLECTION}")
|
||||
if coll is None:
|
||||
print(f"ABORT: collection {COLLECTION!r} not found on {base_url}")
|
||||
return 1
|
||||
fields = coll.get("fields", [])
|
||||
have = {f["name"] for f in fields}
|
||||
|
||||
sites_coll = api(base_url, token, "GET", "/api/collections/sites")
|
||||
if sites_coll is None:
|
||||
print(f"ABORT: collection 'sites' not found on {base_url}")
|
||||
return 1
|
||||
site_collection_id = sites_coll["id"]
|
||||
|
||||
# cascadeDelete MUST stay False. A cascade-delete relation here would
|
||||
# mean deleting a site deletes every timesheet row attributed to it -
|
||||
# i.e. destroys that site's payroll history. maxSelect 1 keeps the
|
||||
# field single-valued so the future `site IN (...)` filter is exact.
|
||||
site_field = {
|
||||
"name": "site",
|
||||
"type": "relation",
|
||||
"required": False,
|
||||
"collectionId": site_collection_id,
|
||||
"maxSelect": 1,
|
||||
"cascadeDelete": False,
|
||||
}
|
||||
|
||||
if "site" not in have:
|
||||
print(f"\n{COLLECTION}: would add optional `site` relation")
|
||||
print(f" -> sites collection id: {site_collection_id}")
|
||||
print(f" -> field def: {json.dumps(site_field, sort_keys=True)}")
|
||||
if APPLY:
|
||||
api(
|
||||
base_url,
|
||||
token,
|
||||
"PATCH",
|
||||
f"/api/collections/{COLLECTION}",
|
||||
{"fields": fields + [site_field]},
|
||||
)
|
||||
print(f"{COLLECTION}: added optional `site` relation")
|
||||
else:
|
||||
existing = next(f for f in fields if f["name"] == "site")
|
||||
print(f"\n{COLLECTION}: `site` field already present")
|
||||
print(f" -> existing def: {json.dumps(existing, sort_keys=True)}")
|
||||
|
||||
employees = get_all(base_url, token, "employees", "id,site")
|
||||
emp_site = {e["id"]: rel_id(e.get("site")) for e in employees}
|
||||
|
||||
rows = get_all(base_url, token, COLLECTION, "id,employee,site")
|
||||
|
||||
todo, orphan, done = [], [], 0
|
||||
for r in rows:
|
||||
if rel_id(r.get("site")):
|
||||
done += 1
|
||||
continue
|
||||
emp = rel_id(r.get("employee"))
|
||||
s = emp_site.get(emp, "")
|
||||
if s:
|
||||
todo.append((r["id"], s))
|
||||
continue
|
||||
|
||||
# Name the reason. "This row has no employee at all" is a very
|
||||
# different finding from "the employee exists but carries no
|
||||
# site", and a single undifferentiated orphan count hides which
|
||||
# one happened - and therefore who has to fix it.
|
||||
if not emp:
|
||||
reason = "no employee on row"
|
||||
elif emp not in emp_site:
|
||||
reason = "employee not found"
|
||||
else:
|
||||
reason = "employee has no site"
|
||||
orphan.append((r["id"], emp or "-", reason))
|
||||
|
||||
print(f"\nemployees={len(employees)} sites_collection={site_collection_id}")
|
||||
print(
|
||||
f"\n{COLLECTION} total={len(rows)} to_write={len(todo)} "
|
||||
f"no_employee_site={len(orphan)}"
|
||||
)
|
||||
print(f"{COLLECTION} already_had_site={done}")
|
||||
|
||||
for o in orphan[:20]:
|
||||
print(f" ORPHAN id={o[0]} employee={o[1]} ({o[2]})")
|
||||
if len(orphan) > 20:
|
||||
print(f" ... and {len(orphan) - 20} more")
|
||||
|
||||
if not APPLY:
|
||||
if orphan:
|
||||
print(
|
||||
f"\nNOTE: {len(orphan)} row(s) would be skipped. The plan "
|
||||
f"expects 0. Report these before applying - these rows "
|
||||
f"cannot be attributed without a human decision, and a "
|
||||
f"guessed site is an invented payroll fact."
|
||||
)
|
||||
print("\nDRY RUN. Re-run with --apply to write.")
|
||||
return 0
|
||||
|
||||
# Per-record write failures are tracked apart from the data-quality
|
||||
# orphans above. A row we chose not to touch and a row we tried to
|
||||
# touch and failed on are not the same fact; counting the latter as
|
||||
# the former makes a half-broken run read as a clean one.
|
||||
errors = []
|
||||
for i, (rec_id, site_id) in enumerate(todo, 1):
|
||||
try:
|
||||
api(
|
||||
base_url,
|
||||
token,
|
||||
"PATCH",
|
||||
f"/api/collections/{COLLECTION}/records/{rec_id}",
|
||||
{"site": site_id},
|
||||
)
|
||||
except Exception as e: # noqa: BLE001 - report, never swallow
|
||||
errors.append(f"{rec_id} ({e})")
|
||||
if i % 100 == 0:
|
||||
print(f" wrote {i}/{len(todo)}")
|
||||
|
||||
for n in errors[:20]:
|
||||
print(f" ERROR {n}")
|
||||
|
||||
remaining = [
|
||||
r
|
||||
for r in get_all(base_url, token, COLLECTION, "id,site")
|
||||
if not rel_id(r.get("site"))
|
||||
]
|
||||
print(
|
||||
f"\nwrote {len(todo) - len(errors)}/{len(todo)} "
|
||||
f"write errors: {len(errors)}"
|
||||
)
|
||||
print(
|
||||
f"{COLLECTION} still missing site: {len(remaining)} "
|
||||
f"(expected {len(orphan)} = the rows we deliberately skipped)"
|
||||
)
|
||||
|
||||
if errors:
|
||||
print(f"\nFAILED: {len(errors)} record(s) did not write. Re-run.")
|
||||
return 1
|
||||
if len(remaining) != len(orphan):
|
||||
print("\nFAILED: post-run count does not match the plan. Investigate.")
|
||||
return 1
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
try:
|
||||
sys.exit(main())
|
||||
except KeyError as e:
|
||||
print(f"ERROR: missing environment variable {e}", file=sys.stderr)
|
||||
sys.exit(2)
|
||||
Loading…
Reference in a new issue