diff --git a/docs/design-approval-policy-ui-2026-05-07.md b/docs/design-approval-policy-ui-2026-05-07.md new file mode 100644 index 0000000..c337763 --- /dev/null +++ b/docs/design-approval-policy-ui-2026-05-07.md @@ -0,0 +1,912 @@ +# Approval-policy authoring UI — design + +**Task:** t-paliad-154 +**Issue:** m/paliad#13 +**Inventor:** hilbert (2026-05-07) +**Branch:** mai/hilbert/inventor-approval-policy +**Status:** READY FOR REVIEW + +--- + +## §0 — One-paragraph summary + +cronus shipped the t-138 4-eye backend on 2026-05-06: tables, service layer, +HTTP API, audit events, the `/inbox` shell. The whole thing has been **dormant +in production since** because `paliad.approval_policies` has zero rows, and no +UI exists to author policies. m hit this hard 2026-05-07 22:55 — created a +deadline expecting a request on `/approvals`, got nothing. This design fills +the gap with **two coordinated changes**: (a) a backend extension to support +**per-partner-unit defaults** layered with **project-tree inheritance**, both +resolved most-restrictive, with an explicit `'none'` sentinel for project-level +opt-out; (b) a single new admin page `/admin/approval-policies` with a +project-picker → 8-cell matrix and a partner-unit defaults section, plus +in-context hints on the deadline/appointment forms when 4-eye applies. v1 +ships seeded conservative defaults for every existing partner unit so the gate +starts working on next deploy without per-project authoring. + +--- + +## §1 — What's already built (verified live, 2026-05-07) + +cronus's t-138 implementation is complete and merged. Verified premises: + +- **Schema (migration 054, applied):** `paliad.approval_policies` with + `(id, project_id, entity_type, lifecycle_event, required_role, created_at, + updated_at, created_by)` + UNIQUE composite on `(project_id, entity_type, + lifecycle_event)`. RLS enforces SELECT via `can_see_project(project_id)`, + WRITE via `global_role='global_admin'`. Read-only check on the live DB + via the migration file at `internal/db/migrations/054_approvals.up.sql:75`. +- **Required-role enum (post-059):** `partner | of_counsel | associate | + senior_pa | pa`. The `'lead' → 'partner'` rename happened in migration 059 + (t-148, kepler) — verified at `internal/db/migrations/059_profession_vs_responsibility.up.sql:166-172`. + Mirrors `paliad.users.profession` (firm-wide career tier), not + `paliad.project_teams.responsibility` (project-level role) — the gate keys + on profession because that's how the strict ladder + `paliad.approval_role_level()` works. +- **HTTP API (admin-gated):** three handlers in + `internal/handlers/approvals.go` register at `internal/handlers/handlers.go:421-426`: + - `GET /api/projects/{id}/approval-policies` → list + - `PUT /api/projects/{id}/approval-policies/{entity_type}/{lifecycle}` → upsert + - `DELETE /api/projects/{id}/approval-policies/{entity_type}/{lifecycle}` → clear + + All three wrapped with `auth.RequireAdminFunc(users, ...)`. +- **`LookupPolicy`** (`internal/services/approval_service.go:69-83`) does + **not walk the project tree** today. It SELECTs the exact + `(project_id, entity_type, lifecycle_event)` tuple and returns the row or + nil. Tree inheritance is brand-new in this design. +- **Audit:** approval-request submission and decisions emit + `paliad.project_events` rows; **policy CRUD does not**. Verified at + `internal/services/approval_service.go:255` (request emits) — no + `insertProjectEvent` call inside `UpsertPolicy`/`DeletePolicy` at lines + 913-948. +- **Partner-unit substrate (t-139, migration 055, applied):** + - `paliad.partner_units (id, name, lead_user_id, office, ...)` — verified + at `internal/services/partner_unit_service.go:29`. + - `paliad.project_partner_units (project_id, partner_unit_id, + derive_grants_authority, derive_unit_roles)` — verified at + `internal/db/migrations/055_hierarchy_aggregation.up.sql:47`. +- **Admin index pattern:** `/admin` is a card-grid of single-purpose admin + sub-pages — team, partner-units, audit-log, email-templates, event-types, + broadcasts. Verified at `frontend/src/admin.tsx:60-91`. New approval-policy + card slots into the same grid. +- **Migration tracker:** last applied is **061** + (`paliad.user_card_layouts`). Next is **062** — this design's migration. + +--- + +## §2 — m's locked decisions (2026-05-07 23:00) + +12 questions surfaced via AskUserQuestion (per dogma, not as a markdown +list). Locked verbatim — quoted as-asked + answer: + +### Q1 — Surface placement + +> Where should approval policies be authored? The backend admin-gates the +> CRUD endpoints, so anywhere we surface authoring is admin-only by +> definition. + +**Locked: Admin page only.** New `/admin/approval-policies` card on the +admin index. Single page with two sections: (a) Partner-unit defaults, +(b) Project picker → 8-cell matrix. Per-project tab is **out**. Project +visibility into effective rules happens at form-time (Q12 below), not as a +permanent tab. + +### Q2 — Default-policy concept + +> With ~30 projects and 8 cells each, authoring is tedious. Should we add +> firm-wide defaults that individual projects override? + +**Locked: Per partner-unit defaults.** Schema gets a nullable +`partner_unit_id`, project_id becomes nullable, XOR check enforces a row +applies to one or the other. Reuses the t-139 partner-unit infra. No +firm-wide defaults — one less concept. + +### Q3 — Multi-unit conflict resolution + +> A project attached to multiple partner units with conflicting unit +> defaults — e.g. Munich Lit unit defaults to deadline:create=partner, +> Düsseldorf to deadline:create=associate. What does the gate require? + +**Locked: Most-restrictive wins.** Take MAX(`approval_role_level`) across +all unit defaults for the project. Conservative — 4-eye exists to prevent +quiet errors, the higher bar wins. + +### Q4 — Tree inheritance + +> Projects also live in a tree. Should an ancestor project's policy inherit +> DOWN the project tree to descendants when they have no own row, or only +> via partner-unit defaults? + +**Locked: Both — tree inheritance AND unit defaults.** Three sources +contribute to the candidate set: project-specific rows, ancestor rows, +unit defaults. + +### Q5 — Cross-source precedence + +> When tree-inheritance and unit-defaults both produce a candidate, which +> wins? + +**Locked: Most-restrictive across ALL sources.** Project-specific row +overrides outright (any value, including `'none'`). When no project row, +take MAX(level) across all ancestor rows + all unit defaults. Symmetric +with the multi-unit rule. + +### Q6 — Explicit suppression sentinel + +> A project-specific row always wins. To set 'this project explicitly +> bypasses 4-eye on deadline:create' overriding a partner-unit default of +> 'partner', we need a sentinel. + +**Locked: `'none'` value in `required_role` enum.** Add `'none'` to the +CHECK constraint. Cell renders as "Keine Genehmigung erforderlich". Project +row with `required_role='none'` returns nil from `LookupPolicy` — +suppresses defaults explicitly. Single column, single concept. + +### Q7 — Soft-disable vs delete + +> Per-policy enable/disable toggle vs delete-only. With audit-log emission +> already locked in (Q8), do we still need soft-disable? + +**Locked: Delete-only.** One row = one rule. "This rule used to apply" is +answered by the audit log. KISS. + +### Q8 — Audit emission + +> Should policy changes emit project_events? + +**Locked: Only on `/admin/audit-log`, not on per-project `/verlauf`.** +New event types `approval_policy_set` and `approval_policy_cleared` +emitted via the existing audit-log path (not via the project-events +union). Project verlauf stays focused on entity-level history. + +### Q9 — Empty-state on /inbox + +> When admin opens /inbox and pending list is empty AND no policies exist, +> show a one-tap nudge? + +**Locked: Yes — admin-only card.** Conditional on `me.global_role === +'global_admin' && pending.length === 0 && !any_policies_exist`. Card links +to `/admin/approval-policies`. Solves the discoverability gap m hit. + +### Q10 — Bulk-apply + +> Bulk action on the admin page so an admin can fan a Mandant's matrix +> down to its 12 sub-projects without 96 clicks? + +**Locked: Yes — "Auf Unterprojekte anwenden" button per project row.** +Click → confirm modal listing affected descendants → applies the source +project's full matrix to all descendants. Idempotent. + +### Q11 — Seed defaults on first deploy + +> Should v1 ship seeded defaults, or strictly opt-in? + +**Locked: Seed conservative defaults for every partner_unit.** Migration +inserts 8 rows per existing partner_unit: + +| entity | lifecycle | required_role | +| :--- | :--- | :--- | +| deadline | create | associate | +| deadline | update | associate | +| deadline | delete | associate | +| deadline | complete | none | +| appointment | create | associate | +| appointment | update | associate | +| appointment | delete | associate | +| appointment | complete | none | + +Rationale: marking-as-done is low-risk; the planning ops (create/edit/delete +the date itself) need 4-eye. `none` on `complete` is an explicit "no gate" +sentinel, not a missing row — so MAX-across-sources still works correctly. + +### Q12 — Mobile shape + +> 8-cell matrix is too wide for narrow viewports. + +**Locked: Two stacked sections — Fristen, Termine, each as 4-row list.** +On viewports ≥ 700px: 2-row × 4-col matrix. On viewports < 700px: vertical +section per entity_type with full-width dropdown rows. + +### Q13 — Form-time hint visibility + +> Should we surface 4-eye to users authoring deadlines, before they save? + +**Locked: Yes — hint on the deadline-form.** Above the Speichern button on +`/projects/{id}/deadlines/new` and `/projects/{id}/appointments/new`, +render: "4-Augen-Prüfung erforderlich: nach dem Speichern wird ein +Genehmigungsantrag (associate-Level) ausgelöst." Pulled from new +`GET /api/projects/{id}/approval-policies/effective` endpoint at form load. + +--- + +## §3 — Backend extensions + +### §3.1 — Migration 062 + +`internal/db/migrations/062_approval_policy_unit_defaults.up.sql`: + +```sql +-- t-paliad-154: approval-policy authoring UI substrate. +-- +-- Extends t-138's paliad.approval_policies with: +-- 1. partner_unit_id column for unit-default rows (XOR with project_id) +-- 2. 'none' sentinel value for required_role (explicit suppression) +-- 3. paliad.approval_policy_effective() resolver — tree + unit + most-restrictive +-- 4. Conservative seed defaults for every existing partner_unit + +-- 1. partner_unit_id column + nullable project_id + XOR check. +ALTER TABLE paliad.approval_policies + ALTER COLUMN project_id DROP NOT NULL, + ADD COLUMN partner_unit_id uuid + REFERENCES paliad.partner_units(id) ON DELETE CASCADE, + ADD CONSTRAINT approval_policies_scope_xor CHECK ( + (project_id IS NOT NULL AND partner_unit_id IS NULL) OR + (project_id IS NULL AND partner_unit_id IS NOT NULL) + ); + +-- Replace UNIQUE (project_id, ...) with two partial unique indexes since +-- project_id is now nullable. +ALTER TABLE paliad.approval_policies + DROP CONSTRAINT IF EXISTS approval_policies_project_id_entity_type_lifecycle_event_key; + +CREATE UNIQUE INDEX approval_policies_project_unique + ON paliad.approval_policies (project_id, entity_type, lifecycle_event) + WHERE project_id IS NOT NULL; + +CREATE UNIQUE INDEX approval_policies_unit_unique + ON paliad.approval_policies (partner_unit_id, entity_type, lifecycle_event) + WHERE partner_unit_id IS NOT NULL; + +CREATE INDEX approval_policies_unit_idx + ON paliad.approval_policies (partner_unit_id); + +-- 2. 'none' sentinel. +ALTER TABLE paliad.approval_policies + DROP CONSTRAINT IF EXISTS approval_policies_required_role_check; +ALTER TABLE paliad.approval_policies + ADD CONSTRAINT approval_policies_required_role_check + CHECK (required_role IN ( + 'partner', 'of_counsel', 'associate', 'senior_pa', 'pa', 'none' + )); + +-- approval_role_level('none') already returns 0 (the ELSE branch). No +-- function change needed. + +-- 3. Resolver function. +-- +-- Returns the effective policy for (project, entity_type, lifecycle): +-- 1. project-specific row → wins outright (any value including 'none') +-- 2. else MAX(approval_role_level) across: +-- - all ancestor project rows on the path +-- - all unit-default rows for partner units attached to project +-- 3. else NULL (no candidates) → no policy applies +-- +-- Returns at most one row. Caller can detect "no policy" via empty result. +CREATE OR REPLACE FUNCTION paliad.approval_policy_effective( + p_project_id uuid, + p_entity_type text, + p_lifecycle text +) RETURNS TABLE ( + required_role text, + source text, -- 'project' | 'ancestor' | 'unit_default' + source_id uuid -- project_id for project/ancestor, partner_unit_id for unit_default +) +LANGUAGE plpgsql STABLE AS $$ +BEGIN + -- Step 1: project-specific row. + RETURN QUERY + SELECT ap.required_role, 'project'::text, ap.project_id + FROM paliad.approval_policies ap + WHERE ap.project_id = p_project_id + AND ap.entity_type = p_entity_type + AND ap.lifecycle_event = p_lifecycle; + IF FOUND THEN + RETURN; + END IF; + + -- Step 2: MAX across ancestor + unit_default. + RETURN QUERY + WITH path AS ( + SELECT string_to_array(p.path, '.')::uuid[] AS ids + FROM paliad.projects p WHERE p.id = p_project_id + ), + ancestor_rows AS ( + SELECT ap.required_role, + 'ancestor'::text AS src, + ap.project_id AS sid, + paliad.approval_role_level(ap.required_role) AS lvl + FROM paliad.approval_policies ap, path + WHERE ap.project_id = ANY(path.ids) + AND ap.project_id <> p_project_id + AND ap.entity_type = p_entity_type + AND ap.lifecycle_event = p_lifecycle + ), + unit_rows AS ( + SELECT ap.required_role, + 'unit_default'::text AS src, + ap.partner_unit_id AS sid, + paliad.approval_role_level(ap.required_role) AS lvl + FROM paliad.approval_policies ap + JOIN paliad.project_partner_units ppu + ON ppu.partner_unit_id = ap.partner_unit_id + WHERE ppu.project_id = p_project_id + AND ap.entity_type = p_entity_type + AND ap.lifecycle_event = p_lifecycle + ) + SELECT a.required_role, a.src, a.sid + FROM (SELECT * FROM ancestor_rows + UNION ALL + SELECT * FROM unit_rows) a + ORDER BY a.lvl DESC, a.src ASC -- 'ancestor' < 'unit_default' alphabetically; ancestor wins ties for stable attribution + LIMIT 1; +END; +$$; + +COMMENT ON FUNCTION paliad.approval_policy_effective(uuid, text, text) IS + 'Effective approval policy resolver (t-paliad-154). ' + 'project-specific row wins outright; else MAX(level) across ancestors ' + 'and unit-defaults attached to project; else no policy.'; + +-- 4. Seed conservative defaults for every existing partner_unit. +INSERT INTO paliad.approval_policies ( + project_id, partner_unit_id, entity_type, lifecycle_event, required_role +) +SELECT NULL, pu.id, t.entity_type, t.lifecycle_event, t.required_role + FROM paliad.partner_units pu + CROSS JOIN ( + VALUES + ('deadline', 'create', 'associate'), + ('deadline', 'update', 'associate'), + ('deadline', 'delete', 'associate'), + ('deadline', 'complete', 'none'), + ('appointment', 'create', 'associate'), + ('appointment', 'update', 'associate'), + ('appointment', 'delete', 'associate'), + ('appointment', 'complete', 'none') + ) AS t(entity_type, lifecycle_event, required_role) +ON CONFLICT DO NOTHING; +``` + +`062_approval_policy_unit_defaults.down.sql` reverses each step +(deletes seeded rows, drops the function, drops indexes, drops the +column + constraint, restores the original UNIQUE + CHECK). + +### §3.2 — Service-layer changes + +`internal/services/approval_service.go` changes (additive — existing +callers keep working): + +- **Rewire `LookupPolicy`** to call the resolver. New body: + ```go + func (s *ApprovalService) LookupPolicy(ctx, tx, projectID, entityType, lifecycleEvent) (*models.ApprovalPolicy, error) { + var row struct { + RequiredRole string `db:"required_role"` + Source string `db:"source"` + SourceID uuid.UUID `db:"source_id"` + } + q := `SELECT required_role, source, source_id + FROM paliad.approval_policy_effective($1, $2, $3)` + err := txOrDB(tx, s.db).GetContext(ctx, &row, q, projectID, entityType, lifecycleEvent) + if errors.Is(err, sql.ErrNoRows) || row.RequiredRole == "none" { + return nil, nil // no policy applies + } + if err != nil { return nil, fmt.Errorf("lookup approval policy: %w", err) } + // Synthetic ApprovalPolicy — preserves the calling contract. + return &models.ApprovalPolicy{ + ProjectID: projectID, + EntityType: entityType, + LifecycleEvent: lifecycleEvent, + RequiredRole: row.RequiredRole, + }, nil + } + ``` + The submit/decide chain at lines 142-380 continues to work unchanged. + `'none'` returning nil means: project explicitly opted out, no request + is created on save. + +- **New `GetEffectivePoliciesMatrix(ctx, projectID)`** returns 8 rows + (one per `entity_type × lifecycle_event`), each with attribution. Used + by the admin page and the form-hint endpoint. + ```go + type EffectivePolicy struct { + EntityType string + LifecycleEvent string + RequiredRole *string // nil if no policy + Source *string // nil if no policy + SourceID *uuid.UUID + } + func (s *ApprovalService) GetEffectivePoliciesMatrix(ctx, projectID) ([]EffectivePolicy, error) + ``` + Implementation: 8 calls to the resolver in a single round-trip via + `unnest()` join, or a small batch loop — both fine for ≤8 cells. + +- **Extend `UpsertPolicy` signature** to accept `partnerUnitID *uuid.UUID` + alongside `projectID *uuid.UUID`. Existing callers pass projectID + nil. + New callers (unit-default endpoints) pass nil + unit ID. + ```go + func (s *ApprovalService) UpsertPolicy(ctx, callerID, + projectID, partnerUnitID *uuid.UUID, + entityType, lifecycle, requiredRole string) (*models.ApprovalPolicy, error) + ``` + Same for `DeletePolicy`. Validates exactly one of (projectID, partnerUnitID) + is set. + +- **New `ApplyMatrixToDescendants(ctx, callerID, sourceProjectID, + targetIDs []uuid.UUID)`**: copies all eight rows of `sourceProjectID`'s + effective matrix to each `targetIDs[i]` as project-specific rows. Inside + one transaction. Validates `targetIDs` are actual descendants via the + ltree path predicate. Returns the count of (project, cell) writes + performed. Skips cells where source is `'none'` and target already has + no row (idempotent). Emits one audit-log event per write. + +- **Audit emission** in `UpsertPolicy` + `DeletePolicy` + `ApplyMatrixToDescendants`: + call existing `AuditService.Record` (the same path `/admin/audit-log` + uses). New event type strings: `approval_policy_set`, `approval_policy_cleared`. + Metadata: scope (project|partner_unit), scope_id, entity_type, lifecycle, + old_required_role (for set), new_required_role (for set). The audit + service already handles JSON metadata; no schema change. + + **No project_events emission** (per Q8 lock-in). Project verlauf stays + focused on entity-level lifecycle. + +### §3.3 — HTTP handlers + +`internal/handlers/approvals.go` extensions: + +- **Existing routes stay** at `handlers.go:421-426` (gated by + `RequireAdminFunc`). + +- **New unit-default routes** (also `RequireAdminFunc`-gated, registered + in the same admin block at handlers.go:386-427): + - `GET /api/admin/partner-units/{unit_id}/approval-policies` — list + all rows for that unit. + - `PUT /api/admin/partner-units/{unit_id}/approval-policies/{entity_type}/{lifecycle}` — upsert. + - `DELETE /api/admin/partner-units/{unit_id}/approval-policies/{entity_type}/{lifecycle}` — clear. + - `GET /api/admin/approval-policies/seeded` — quick existence check + used by the `/inbox` admin nudge ("are any policies set firm-wide?"). + +- **New endpoint for matrix view** (admin page): + - `GET /api/admin/approval-policies/matrix?project_id=...` — returns + `[]EffectivePolicy` (8 rows with attribution). + +- **New endpoint for form hint** (gateOnboarded, NOT admin-only — every + user authoring a deadline needs to see this): + - `GET /api/projects/{id}/approval-policies/effective?entity_type=deadline&lifecycle=create` + — returns one `EffectivePolicy` row. + +- **New endpoint for bulk apply**: + - `POST /api/admin/approval-policies/apply-to-descendants` — body + `{source_project_id: uuid, target_project_ids: [uuid, ...]}`. Validates, + applies, returns counts. + +- **New endpoint for project tree** (admin page picker — already exists + in part): + - `GET /api/admin/projects/tree-flat` — flat array of all projects with + `id, name, parent_id, depth, path` for the picker. Reuses + `ProjectService.ListAllForAdmin` (already present at + `internal/services/project_service.go` — admin-scoped tree). + +- **New page handler**: + - `GET /admin/approval-policies` → `dist/admin-approval-policies.html` + (server-static shell, hydrated on load). + +--- + +## §4 — Frontend + +### §4.1 — Admin page `/admin/approval-policies` + +New files: +- `frontend/src/admin-approval-policies.tsx` — page shell. Sections: + 1. Header: "Genehmigungsrichtlinien" + tool-subtitle. + 2. **"Partner-Unit-Standards"** — accordion list of partner units + (fetched from `/api/partner-units`). Each row expandable into the + 8-cell matrix (Fristen × 4 lifecycle, Termine × 4 lifecycle), each + cell a ``-on-row-click affordance pattern. +- ltree path-walk in SQL → `internal/services/visibility.go` and the + existing `paliad.can_see_project()` are the reference pattern. +- Audit emission → `internal/services/audit_service.go` (already plumbed). +- Form-hint above Speichern → similar to the t-148 profession hint + on `frontend/src/projects-detail.tsx:130` (`team-profession-hint`). + +**NOT cronus** per memory directive (paliad). **NOT noether** (parked on +t-151 and t-144). **NOT godel** (just fired on t-149). **NOT hilbert +(me)** — I'm parked after this design; head decides if I take the +coder shift on the same worktree (mai/hilbert/inventor-approval-policy) +or hands it to a fresh coder. + +--- + +## §11 — Out of scope (deferred to follow-ups) + +- **Per-policy time-window** — "this rule applies only Mon–Fri 9–17, after + hours skip 4-eye". Some firms do this. Deferred: another column would + be cheap, but no signal yet that anyone wants it. +- **Per-user exemptions** — "Alice is on PTO, route around her". Same + shape as today's `decision_kind='admin_override'` escape hatch — already + available via global_admin. +- **Multi-step approvals** — "needs partner THEN of_counsel sign-off". + cronus's t-138 is single-step by design (Q3 of t-138 locked it). Not + in scope here. +- **Policy templates / copy-from-other-project** — beyond bulk-apply-to- + descendants. If needed, would slot into the admin page as a + "Vorlage anwenden" affordance. Not v1. +- **Per-event_type policies** — "deadline.create with event_type='Klage' + needs partner; everything else of_counsel". The existing schema is + per-(entity_type, lifecycle_event); event-type granularity would + require an extra column + index. No signal yet. + +--- + +**END OF DESIGN.** + +Inventor stays parked. Awaits m's go/no-go on the 12 locked decisions +before any coder shift. Hand-off via head once green.