feat(db): schema 0002 master data tables #58

Merged
patrick merged 2 commits from feature/db-schema-master-data into main 2026-09-11 11:12:52 +00:00
Owner

PH1.1 — issue #9

Migration 0002 and matching typed SQLAlchemy mapped classes for every master-data table in docs/08-data-model.md: vehicle, ingest_record, site, presence_block, presence_window, catalogue (service_type, skill, service_type_skill, reason_code, site_suitability), job, person (with preferred_username), crew, crew_member, crew_skill, shift. No tooling table/joins, per docs/adr/0001-remove-tooling-catalogue.md.

Branch history note: started from origin/develop (at the time main had no backend/ yet — only develop had #4/#51 API-skeleton merged). While this was in flight, develop was merged into main and retired in favour of a single-trunk workflow (CLAUDE.md's Git section, commit 1a1dfc2/81e2f4c). Rebased this branch onto the new main before opening this PR, which now targets main per the updated workflow.

What's in here

  • Soft-delete columns on every genuine master-data table; job/shift/site_suitability get created_at/updated_at/created_by/updated_by only (no deleted_at) — their lifecycle is a status transition, matching the doc's "audit columns" phrasing for those three.
  • geography(POINT|POLYGON, 4326) via a small UserDefinedType (db/models/_types.py) instead of adding geoalchemy2 to pyproject.toml, which is outside this ticket's owned paths.
  • Native Postgres enums for every status/kind/source column, keyed by .value via db/models/_enum.py's native_enum() helper — sqlalchemy.Enum(SomePyEnum) defaults to the member .name, not .value, which silently disagrees with the hand-written lowercase labels in the Alembic migration otherwise (see the second commit below — this exact bug is why it needed fixing).
  • Every mapped class mixes in Auditable; composite-PK tables override audit_entity_id with a crc32 of their key (same approach as Setting).
  • All indexes/constraints from docs/08-data-model.md for these aggregates.

Also in here: a seam fix (db/audit.py)

Building this surfaced a real bug in polaris/db/audit.py (outside this ticket's owned paths, backend/src/polaris/db/models/** / backend/alembic/versions/** only): Auditable's before_flush hook and its default audit_entity_id never actually work for a server-generated identity PK — exactly the PK convention docs/08-data-model.md mandates for every table in this migration. It was latent before because Setting (0001) has a client-supplied string PK and bypasses both code paths, and task opts out of Auditable entirely.

Fixed directly here (first commit) rather than opening a separate PR and blocking on it, since it would otherwise ship PH1.1 broken; full writeup and rationale in the commit message and in seam-request issue #57 (opened for traceability, not as a blocker — already resolved). Existing tests/integration/db/test_audit_hook.py cases pass unchanged.

Tests

  • backend/tests/conftest.py and tests/integration/db/test_migrations.py switch the testcontainers Postgres image to postgis/postgis:17-3.4conftest.py's own header comment anticipated this as the first ticket to introduce geography columns.
  • test_migrations.py gains a dedicated 0002 upgrade-head / downgrade -1 / upgrade +1 round-trip test (the ticket's own acceptance criterion) alongside the existing full-chain upgrade/downgrade-base test.
  • New tests/integration/db/test_master_data_models.py exercises the whole PH1.1 aggregate end to end: FK wiring across all sixteen tables, geography WKT round-trip, enums stored lower-case, the job priority check constraint, the shift partial-unique-excluding-cancelled index, and soft-delete (row survives deleted_at being set).

Verified

  • mypy --strict clean (47 source files).
  • ruff check / ruff format --check clean.
  • alembic upgrade head / downgrade -1 / upgrade +1 all pass against a real Postgres+PostGIS (testcontainers).
  • Full backend test suite: 35 passed.

All four acceptance-criteria checkboxes ticked on #9.

Closes #9

🤖 Generated with Claude Code

https://claude.ai/code/session_01LoNrSy7Reyp7evkfcdHeLX

## PH1.1 — issue #9 Migration `0002` and matching typed SQLAlchemy mapped classes for every master-data table in `docs/08-data-model.md`: `vehicle`, `ingest_record`, `site`, `presence_block`, `presence_window`, catalogue (`service_type`, `skill`, `service_type_skill`, `reason_code`, `site_suitability`), `job`, `person` (with `preferred_username`), `crew`, `crew_member`, `crew_skill`, `shift`. No `tooling` table/joins, per `docs/adr/0001-remove-tooling-catalogue.md`. **Branch history note**: started from `origin/develop` (at the time `main` had no `backend/` yet — only `develop` had #4/#51 API-skeleton merged). While this was in flight, `develop` was merged into `main` and retired in favour of a single-trunk workflow (`CLAUDE.md`'s Git section, commit `1a1dfc2`/`81e2f4c`). Rebased this branch onto the new `main` before opening this PR, which now targets `main` per the updated workflow. ### What's in here - Soft-delete columns on every genuine master-data table; `job`/`shift`/`site_suitability` get `created_at`/`updated_at`/`created_by`/`updated_by` only (no `deleted_at`) — their lifecycle is a status transition, matching the doc's "audit columns" phrasing for those three. - `geography(POINT|POLYGON, 4326)` via a small `UserDefinedType` (`db/models/_types.py`) instead of adding `geoalchemy2` to `pyproject.toml`, which is outside this ticket's owned paths. - Native Postgres enums for every status/kind/source column, keyed by `.value` via `db/models/_enum.py`'s `native_enum()` helper — `sqlalchemy.Enum(SomePyEnum)` defaults to the member `.name`, not `.value`, which silently disagrees with the hand-written lowercase labels in the Alembic migration otherwise (see the second commit below — this exact bug is why it needed fixing). - Every mapped class mixes in `Auditable`; composite-PK tables override `audit_entity_id` with a crc32 of their key (same approach as `Setting`). - All indexes/constraints from `docs/08-data-model.md` for these aggregates. ### Also in here: a seam fix (`db/audit.py`) Building this surfaced a real bug in `polaris/db/audit.py` (outside this ticket's owned paths, `backend/src/polaris/db/models/**` / `backend/alembic/versions/**` only): `Auditable`'s `before_flush` hook and its default `audit_entity_id` never actually work for a server-generated identity PK — exactly the PK convention `docs/08-data-model.md` mandates for every table in this migration. It was latent before because `Setting` (0001) has a client-supplied string PK and bypasses both code paths, and `task` opts out of `Auditable` entirely. Fixed directly here (first commit) rather than opening a separate PR and blocking on it, since it would otherwise ship PH1.1 broken; full writeup and rationale in the commit message and in seam-request issue #57 (opened for traceability, not as a blocker — already resolved). Existing `tests/integration/db/test_audit_hook.py` cases pass unchanged. ### Tests - `backend/tests/conftest.py` and `tests/integration/db/test_migrations.py` switch the testcontainers Postgres image to `postgis/postgis:17-3.4` — `conftest.py`'s own header comment anticipated this as the first ticket to introduce geography columns. - `test_migrations.py` gains a dedicated `0002` upgrade-head / `downgrade -1` / `upgrade +1` round-trip test (the ticket's own acceptance criterion) alongside the existing full-chain upgrade/downgrade-base test. - New `tests/integration/db/test_master_data_models.py` exercises the whole PH1.1 aggregate end to end: FK wiring across all sixteen tables, geography WKT round-trip, enums stored lower-case, the job priority check constraint, the shift partial-unique-excluding-cancelled index, and soft-delete (row survives `deleted_at` being set). ### Verified - `mypy --strict` clean (47 source files). - `ruff check` / `ruff format --check` clean. - `alembic upgrade head` / `downgrade -1` / `upgrade +1` all pass against a real Postgres+PostGIS (testcontainers). - Full backend test suite: 35 passed. All four acceptance-criteria checkboxes ticked on #9. Closes #9 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LoNrSy7Reyp7evkfcdHeLX
Auditable.audit_entity_id read inspect(self).identity, and the hook fired on
before_flush. Both are wrong for the project's own PK convention
(docs/08-data-model.md: id bigint generated always as identity): before_flush
runs before the INSERT assigns the PK, and even after_flush doesn't register
inspect().identity until later in the flush's finalize step (confirmed
empirically against SQLAlchemy 2.0.52) even though the plain attribute is
already populated by then.

Latent until now because Setting (0001) has a client-supplied string PK and
overrides audit_entity_id to read it directly, bypassing both bugs, and task
opts out of Auditable entirely. Every PH1.1 master-data table mixes in
Auditable with a server-generated PK, which is exactly what exercises this.

- Hook moved to after_flush (still same transaction: new AuditLog rows just
  join the next autoflush/commit, so a rollback still rolls them back).
- audit_entity_id now reads the PK column's attribute directly instead of
  going through inspect().identity.

Existing tests/integration/db/test_audit_hook.py cases pass unchanged.
Outside PH1.1's owned paths (backend/alembic/versions/**,
backend/src/polaris/db/models/**) but blocks every future Auditable class
with an identity PK, so fixed here rather than widening the ticket further or
shipping it broken — seam-request issue #57 opened for traceability.

Refs #9

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LoNrSy7Reyp7evkfcdHeLX
Migration 0002 and matching typed SQLAlchemy mapped classes for every
master-data table in docs/08-data-model.md: vehicle, ingest_record, site,
presence_block, presence_window, catalogue (service_type, skill,
service_type_skill, reason_code, site_suitability), job, person (with
preferred_username per docs/07-architecture.md), crew, crew_member,
crew_skill, shift. No tooling table/join tables, per
docs/adr/0001-remove-tooling-catalogue.md.

- Soft-delete columns (created_at/updated_at/deleted_at/created_by/updated_by)
  on every genuine master-data table; job/shift/site_suitability get
  created_at/updated_at/created_by/updated_by only (no deleted_at) since their
  lifecycle is a status transition, matching the doc's 'audit columns' phrasing
  for those three.
- geography(POINT|POLYGON, 4326) columns via a small UserDefinedType
  (db/models/_types.py) rather than adding geoalchemy2 to pyproject.toml,
  which is outside this ticket's owned paths.
- Native Postgres enums for every status/kind/source column, keyed by
  .value via db/models/_enum.py's native_enum() helper (SQLAlchemy's
  Enum(SomePyEnum) defaults to the member .name, not .value, which would
  otherwise disagree with the hand-written lowercase labels in the Alembic
  migration).
- Every mapped class mixes in Auditable (hard rule #3); composite-PK
  tables override audit_entity_id with a crc32 of their key, same approach
  as Setting.
- All indexes/constraints from docs/08-data-model.md for these aggregates:
  site's GIST(polygon) and partial (vehicle_id) where valid_to is null,
  presence_window's (vehicle_id, date) and GIST(location), job's
  (status, due_date) and (vehicle_id, status) plus the 1..5 priority check,
  shift's partial unique (crew_id, date) where status != 'cancelled'.

backend/tests/conftest.py and tests/integration/db/test_migrations.py switch
the testcontainers Postgres image to postgis/postgis:17-3.4 (Postgres 17,
matching the compose stack) — conftest.py's own header comment anticipated
this as the first ticket to introduce geography columns. test_migrations.py
gains a dedicated 0002 upgrade-head/downgrade -1/upgrade +1 round-trip test
(the ticket's own acceptance criterion) alongside the existing full-chain
upgrade/downgrade-base test.

tests/integration/db/test_master_data_models.py exercises the whole PH1.1
aggregate end to end against real Postgres+PostGIS: FK wiring across all
sixteen tables, geography WKT round-trip, enums stored lower-case, the job
priority check constraint, and the shift partial-unique-excluding-cancelled
index.

mypy --strict and ruff clean. alembic upgrade head / downgrade -1 / upgrade
+1 all verified against testcontainers.

Closes #9

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LoNrSy7Reyp7evkfcdHeLX
patrick deleted branch feature/db-schema-master-data 2026-09-11 11:12:52 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
patrick/Polaris!58
No description provided.