docs(09): record approved review decisions and close UAT

This commit is contained in:
Jakub Zych
2026-10-01 23:13:02 +02:00
parent 2da8112dbb
commit ba1aaefa01
2 changed files with 15 additions and 16 deletions

View File

@@ -177,11 +177,11 @@ Verification ran in the main checkout (`workflow.use_worktrees` is `false`), not
**Files modified:** `modules/cabana/tx_context.go` (new), `modules/cabana/crud.go`, `modules/cabana/relation.go`, `modules/cabana/crud_lifecycle_test.go`, `modules/cabana/README.md`, `docs/backend/admin-controllers.md`; fonoteka.go `plugins/golem15/fonoteka/controllers/albums_admin_controller.go`, `plugins/golem15/fonoteka/controllers/collections_admin_controller.go`, `plugins/golem15/fonoteka/controllers/request_db.go` (new), `plugins/golem15/fonoteka/admin_albums_test.go`
**Applied fix:** cabana puts the write's transaction on the context inside every `lagoon.Transaction` callback and exports `cabana.TxFromContext(ctx)`. The hook and scope signatures are unchanged, so no plugin contract breaks. The Fonoteka album and collection hooks read through it via a small `requestDB` helper and fall back to the pool outside a transaction (the list). The new Fonoteka test limits the pool to one connection and creates and updates an album; it hangs (and fails after 20 s) without the fix.
## Decisions needed
## Decisions (resolved 2026-10-01, approved by the user)
- **WR-11, unique index on `lower(email)` (not applied).** The suggestion adds a migration to `modules/lagoon/backend_admin_migrations.go`. Winter's `backend_users` has plain unique `login` and `email`; a functional unique index could make the migration fail on a copied table that holds two emails differing only in case, which D-01 says must copy straight in. The login-time ambiguity check and the `admin:create` check already close the practical hole, and an index would only add race protection. Decide whether to add the index, and whether to dedupe copied rows first.
- **WR-03, single-record delete.** It is allowed whenever a form exists, not only when the list toolbar has `delete`, because the SPA's form screen always shows its own delete button (as in Winter). If delete should follow the toolbar too, the form schema needs a new field so the SPA can hide that button; that changes the admin API shape and OpenAPI.
- **WR-17, deny versus wildcard.** Winter's merge does not let an exact-code deny remove a wildcard grant. If SummerCMS should be stricter than Winter here, say so and `applyUserPermissions` plus its tests change.
- **WR-11, unique index on `lower(email)`: added** in `2da8112` (summercms.go). The new migration `202610010001_backend_users_email_ci_unique` creates `backend_users_email_lower_unique`. Rows copied from Winter are not changed automatically: when emails differ only in case, the migration refuses to run and names the clashing logins, so the operator changes or removes one and runs `migrate` again. Picking an admin account to drop is not something a migration should do. Tests: `TestBackendAdminEmailCaseInsensitiveUnique` and `TestBackendAdminEmailIndexRefusesCaseDuplicates`. Documented in `modules/lagoon/README.md` and `docs/backend/users-and-permissions.md`.
- **WR-03, single-record delete: kept as Winter.** It stays allowed whenever a form exists, because the form screen's delete button matches Winter, and the admin API shape stays unchanged.
- **WR-17, deny versus wildcard: kept as Winter.** An exact-code deny does not remove a wildcard grant, matching Winter's `getMergedPermissions`.
## Test results

View File

@@ -1,39 +1,38 @@
---
status: testing
status: complete
phase: 09-backend-admin-authentication-and-schema-pipeline
source: [09-VERIFICATION.md]
started: 2026-09-27T22:03:21Z
updated: 2026-09-27T22:03:21Z
updated: 2026-10-01T22:30:00Z
---
## Current Test
number: 1
name: Decide CR-01 (admin CRUD cannot save a writable number field; type mismatches return 500 instead of 422)
expected: |
Either fix it before closing Phase 9 (normalize json.Number before lagoon.Fill or teach convertValue about it, map conversion failures to a per-field 422, add a CRUD test writing a non-protected number field bound to an int column), or record an explicit deferral or override with a reason.
awaiting: user response
[testing complete]
## Tests
### 1. Decide CR-01 (admin CRUD cannot save a writable number field; type mismatches return 500 instead of 422)
expected: Fixed before closing Phase 9, or an explicit deferral/override with a reason is recorded.
result: [pending]
result: pass
note: Fixed in Phase 10.1 (c3efbc3, e60e697); `TestCRUDFillTypeIsValidation` passes. Recorded as already fixed in 09-REVIEW-FIX.md.
### 2. Triage the 19 open code-review warnings (priority: WR-01, WR-02, WR-14, WR-17 for AUTH-08 Winter permission semantics)
expected: Each warning set to fixed, deferred (with reason) or skipped in 09-REVIEW-DISPOSITION.md; none left open.
result: [pending]
result: pass
note: WR-01 to WR-19 are all fixed (09-REVIEW-FIX.md, ledger 2ecc85e). The three follow-up choices were approved by the user on 2026-10-01: WR-11 index added (2da8112), WR-03 and WR-17 kept as Winter.
### 3. Review the 24 judgment-tier prohibitions in 09-VERIFICATION.md
expected: Verdicts accepted or rejected; the two qualified ones (09-11 #1 navigation reveals albums target to a genres-only admin, WR-02; 09-12 #1 zero-test detection per invocation, not per package, WR-18) are resolved.
result: [pending]
result: pass
note: Verdicts accepted by the user on 2026-10-01. 09-11 #1 (WR-02) and 09-12 #1 (WR-18) are no longer violated after the fixes. 09-03 #1 (`type: partial` refused) is confirmed as superseded by Phase 10.1 D-09, which allows only a bare-name, sanitized html/template partial; the path-less Winter partial is still refused.
## Summary
total: 3
passed: 0
passed: 3
issues: 0
pending: 3
pending: 0
skipped: 0
blocked: 0