docs(16.1-01): complete journal admin fixes plan

2026-10-09 21:05:31 +02:00
parent 9cac45c389
commit 0ec2e31ffa
2 changed files with 243 additions and 0 deletions
--- a/.planning/phases/16.1-journal-admin-urgent-fixes-relation-search-list-state-post-f/16.1-01-SUMMARY.md
+++ b/.planning/phases/16.1-journal-admin-urgent-fixes-relation-search-list-state-post-f/16.1-01-SUMMARY.md
@@ -0,0 +1,230 @@
+---
+phase: 16.1-journal-admin-urgent-fixes-relation-search-list-state-post-f
+plan: 01
+subsystem: admin
+tags: [gorm, postgres, cabana, vue, localStorage, goldmark, i18n]
+
+requires:
+  - phase: 16-grzybyfunkcjonalne-pl-on-reusable-blog-views
+    provides: Journal posts admin on cabana, the UAT deployment and its defect reports
+provides:
+  - Relation search through correlated EXISTS subqueries and to-many sort through string_agg, both built from GORM's parsed relationship
+  - Default content locale first in meta.enabledLocales, so ML form controls open on the stored host values
+  - sm-journal-plugin FormatHTML keeps raw HTML in content_html; the unsafe-HTML gate is unchanged
+  - Per-list query memory in the URL and browser storage (summer-admin.list:<controller>), restored on entry and cleared on logout
+affects: [16.1-02 release and redeploy, 16.1-03 unit tests, 16.2 blog editor]
+
+actuals:
+  tokens: 8422     # chars/4 over the realized diff of both repos, without the generated dist bundle
+  tasks: 4
+  commits: 3       # summercms.go, measured from the ledger; sm-journal-plugin adds 1 (528d2d8)
+plan_head_before: 3ca6a0d4b256a8e104154b36accaf3024ee0bdd4
+plan_head_after: 9cac45c3894815bc8fea91027f762a27635f67a2
+
+tech-stack:
+  added: []
+  patterns:
+    - "relationSubquery: relation SQL is built from gorm.Statement.Parse plus Schema.Relationships.Relations, aliased cabana_r<n>/cabana_p<n>, with no bound values"
+    - "DryRun SQL harness (openDryRunDB + sqlRecorder + newDryRunListService) tests list SQL without Docker"
+    - "List state: URL first, browser storage as the per-list memory, decided before the first list request"
+
+key-files:
+  created:
+    - modules/cabana/query_relation_test.go
+    - modules/cabana/locale_order_test.go
+    - admin/src/state/listState.ts
+  modified:
+    - modules/cabana/query.go
+    - modules/cabana/http.go
+    - modules/cabana/README.md
+    - docs/backend/lists-and-filters.md
+    - docs/backend/forms.md
+    - admin/src/views/ListView.vue
+    - admin/src/state/useAuth.ts
+    - admin/src/state/useSidebar.ts
+    - admin/tests/list/ListView.test.ts
+    - admin/tests/smoke/list.smoke.test.ts
+    - scripts/check-phase10.sh
+    - modules/boardwalk/dist/
+    - ../sm-journal-plugin/classes/format_html.go
+    - ../sm-journal-plugin/classes/format_html_test.go
+    - ../sm-journal-plugin/README.md
+
+key-decisions:
+  - "Every relation type is searched through EXISTS, belongsTo and hasOne included, so the count query never gains an outer JOIN; only to-one sort keeps the association join"
+  - "Relation sort validates through the same builder, so an unknown related column or a polymorphic relation answers 422 on sort as well as on search"
+  - "The selected related column is resolved through FieldSchema.LookUpField and emitted by its DBName"
+  - "Stored list state is the JSON of toListQuery (flat URL keys) and is read back through parseListQuery, so the stored format and the URL format are the same"
+
+patterns-established:
+  - "Pure DryRun tests for cabana SQL shape: reuse openDryRunDB and newDryRunListService from query_relation_test.go"
+
+requirements-completed: [D-01, D-02, D-03]
+
+coverage:
+  - id: D1
+    description: "List search over belongsToMany, hasMany and belongsTo columns uses EXISTS subqueries named from GORM's relationship and answers 200; to-many sort orders by a string_agg subquery with the primary-key tiebreak"
+    requirement: D-02
+    verification:
+      - kind: unit
+        ref: "modules/cabana/query_relation_test.go#TestListRelationSearchSmoke"
+        status: pass
+    human_judgment: false
+  - id: D2
+    description: "meta.enabledLocales lists the default locale first, then the writer's order"
+    requirement: D-03
+    verification:
+      - kind: unit
+        ref: "modules/cabana/locale_order_test.go#TestEnabledLocalesDefaultFirstSmoke"
+        status: pass
+    human_judgment: false
+  - id: D3
+    description: "FormatHTML keeps raw HTML (HTML paragraphs, a raw footer) and still refuses the unsafe cases"
+    requirement: D-03
+    verification:
+      - kind: unit
+        ref: "../sm-journal-plugin/classes/format_html_test.go#TestFormatHTMLKeepsRawHTML"
+        status: pass
+      - kind: unit
+        ref: "../sm-journal-plugin/classes/format_html_test.go#TestFormatHTMLRejectsUnsafeHTML"
+        status: pass
+    human_judgment: false
+  - id: D4
+    description: "List query restored from browser storage before the first list request; the URL wins and overwrites the stored query"
+    requirement: D-01
+    verification:
+      - kind: unit
+        ref: "admin/tests/list/ListView.test.ts#list state in the URL and browser storage (D-01)"
+        status: pass
+      - kind: other
+        ref: "bash scripts/check-admin-dist.sh"
+        status: pass
+    human_judgment: false
+  - id: D5
+    description: "On UAT the Journal posts list searches by category, remembers its filters after opening a post, and the post form shows the stored content and excerpt"
+    requirement: D-01
+    verification: []
+    human_judgment: true
+    rationale: "Needs the real UAT database and an admin login behind basic auth; plan 02 redeploys and checks it by hand"
+
+duration: 11min
+completed: 2026-10-09
+status: complete
+---
+
+# Phase 16.1 Plan 01: Journal admin fixes Summary
+
+**Fixed the four UAT defects in the Journal posts admin:**
+- **Relation search:** list search runs a correlated EXISTS subquery whose pivot, key and table names come from GORM's parsed relationship (`Schema.Relationships.Relations`), and to-many sort orders by a `string_agg` subquery.
+- **Form locale:** `meta.enabledLocales` lists the default content locale first.
+- **Raw HTML:** goldmark renders with `html.WithUnsafe()` behind the unchanged XSS gate.
+- **List state:** each list's query is kept in the URL and in browser storage.
+
+## Performance
+
+- **Duration:** 11 min
+- **Started:** 2026-10-09T18:53:15Z
+- **Completed:** 2026-10-09T19:04:24Z
+- **Tasks:** 4
+- **Files modified:** 17 in summercms.go (including the dist bundle) and 3 in sm-journal-plugin
+
+## Accomplishments
+
+- **Relation search (D-02).** Search over a belongsToMany column now answers 200. The count query reads `EXISTS (SELECT 1 FROM "<pivot>" AS "cabana_p0" JOIN "<related>" AS "cabana_r0" ON ... WHERE "cabana_p0".<owner fk> = "<main>"."id" AND LOWER(CAST(...)) LIKE ?)` and has no outer JOIN. hasMany, hasOne and belongsTo search use the same EXISTS form.
+  - A record that matches through several related rows is counted and listed once.
+  - Unqualified scope predicates (`id IN (...)`, `user_id = ?`) stay unambiguous.
+  - A soft-deleting related model adds `deleted_at IS NULL`, matching Winter's whereHas.
+  - Polymorphic relations and unknown related columns answer 422.
+- **Relation sort (D-02).** A hasMany or belongsToMany column sorts by `(SELECT string_agg(..., ', ' ORDER BY ...) FROM ... WHERE <correlation>)`, followed by the primary-key tiebreak. belongsTo and hasOne sorts keep the association join.
+- **Form locale (D-03).** `enabledContentLocales` moves `TranslationWriter.DefaultLocale` to index 0, so MLField and FormView open on the locale whose values are in the host columns (content, excerpt, title, slug).
+- **Raw HTML (D-03).** sm-journal-plugin `FormatHTML` passes raw HTML through, as Winter does. Admin saves, imports and Journal API writes no longer replace imported bodies with `<!-- raw HTML omitted -->`.
+  - `rejectUnsafe` still runs after the render.
+  - `cabana.RenderMarkdown` (the admin preview) is untouched.
+- **List state (D-01).** The list query is stored per list under `summer-admin.list:<controller id>`.
+  - A list entered without list params restores the stored query into the URL before its first list request. A URL with list params wins and overwrites the stored query.
+  - A 422 forgets the stored query, and a page past `last_page` is clamped to the last page.
+  - Logout clears every stored list query.
+
+## Task Commits
+
+summercms.go (master, not pushed):
+
+1. **Task 1: Tracer, relation search and sort through subqueries:** `06d75cb` (fix)
+2. **Task 2: Default content locale first in enabledLocales:** `bd8626e` (fix)
+4. **Task 4: List state in the URL and browser storage, rebuilt dist:** `9cac45c` (fix)
+
+sm-journal-plugin (main, not pushed; plan 02 pushes):
+
+3. **Task 3: Keep raw HTML in content_html behind the safety gate:** `528d2d8` (fix)
+
+Each task is one commit with the message the plan named; the plan specifies one commit per task. RED was shown before each implementation:
+- Tasks 1, 2 and 3: the new test failed on its assertion against the old code (the broken `"Tags"."rel_post_id"` join, `[en pl]`, and `<!-- raw HTML omitted -->`).
+- Task 4: the two new vitest cases were run against the HEAD `ListView.vue` and failed on their assertions.
+
+## Files Created/Modified
+
+- `modules/cabana/query.go`: `relationSubquery` / `relationSQL`, EXISTS search, to-many `string_agg` sort, and `applyListOrder` now returns an error.
+- `modules/cabana/query_relation_test.go`: relation fixtures, the DryRun db and recording logger, `newDryRunListService`, and `TestListRelationSearchSmoke`.
+- `modules/cabana/http.go`: `enabledContentLocales` puts the default locale first.
+- `modules/cabana/locale_order_test.go`: `TestEnabledLocalesDefaultFirstSmoke`.
+- `modules/cabana/README.md`, `docs/backend/forms.md`: default locale first.
+- `docs/backend/lists-and-filters.md`: relation search and to-many sort semantics.
+- `admin/src/state/listState.ts`: `LIST_STATE_PREFIX`, `readListState`, `writeListState` and `clearListStates`.
+- `admin/src/views/ListView.vue`: restore before the first request, the URL wins, 422 forgets the state, page clamp.
+- `admin/src/state/useAuth.ts`: logout clears list states.
+- `admin/src/state/useSidebar.ts`: header comment names `listState.ts` as the only other storage writer.
+- `admin/tests/list/ListView.test.ts`: the two D-01 smoke cases.
+- `admin/tests/smoke/list.smoke.test.ts`: `beforeEach` clears list states (deviation 1).
+- `scripts/check-phase10.sh`: storage allowlist now includes `listState.ts`.
+- `modules/boardwalk/dist/`: rebuilt SPA.
+- `../sm-journal-plugin/classes/format_html.go`: `html.WithUnsafe()`, variables renamed so nothing shadows the import.
+- `../sm-journal-plugin/classes/format_html_test.go`: `TestFormatHTMLKeepsRawHTML`.
+- `../sm-journal-plugin/README.md`: Posts bullet.
+
+## Decisions Made
+
+- All relation types are searched through EXISTS, not only to-many ones, so the count query never gains a JOIN (RESEARCH Pitfall 1).
+- Relation sort validates through the same builder, so a bad related column or a polymorphic relation answers 422 on sort as well as on search.
+- The related column is resolved with `FieldSchema.LookUpField` and emitted by its `DBName`.
+- The stored list state uses the flat URL form (`toListQuery`) and is read back through `parseListQuery`, so stale or tampered values are dropped exactly like a hand-edited URL.
+
+## Deviations from Plan
+
+### Auto-fixed Issues
+
+**1. [Rule 3 - Blocking] List smoke tests leaked list state between tests**
+- **Found during:** Task 4
+- **Issue:** `admin/tests/smoke/list.smoke.test.ts` has its own `beforeEach`, which does not clear browser storage. Once lists remember their query, a filter set by an earlier test was restored in "hides the filter bar without filters...". That test then saw `filter[created]` and `filter[maker]` in the URL and failed.
+- **Fix:** the file's `beforeEach` calls `clearListStates()`, so every test starts from a clean list. The file is not in the plan's `files_modified`; it was staged by explicit path in the Task 4 commit.
+- **Files modified:** `admin/tests/smoke/list.smoke.test.ts`
+- **Verification:** the full vitest suite passes (75 files, 1106 tests).
+- **Committed in:** `9cac45c`
+
+---
+
+**Total deviations:** 1 auto-fixed (1 blocking)
+**Impact on plan:** test isolation only; there is no behavior change and no scope creep.
+
+## Issues Encountered
+
+- `go test ./modules/cabana` without `-run` fails on the Docker-backed tests (no Docker socket permission), as the plan expected. Every failure is a Docker error, and none of those tests uses a writer whose default locale is not first. Recorded in `deferred-items.md`.
+- `scripts/check-phase10.sh --hygiene` and `--self-test` refuse 5 findings in files this plan does not touch (`admin/src/api/files.ts`, `FileCaptionModal.vue`, `MLField.vue`, `mlLocale.ts`). The storage rule changed here passes. Recorded in `deferred-items.md`.
+
+## User Setup Required
+
+None. No external service configuration is required.
+
+## Next Phase Readiness
+
+- Plan 02 can tag the summercms.go release (commits `06d75cb`, `bd8626e`, `9cac45c`), push sm-journal-plugin `528d2d8`, bump the host and redeploy.
+- After the deploy, re-save posts 53 and 64 to repair their `content_html`.
+- Before tagging, run the Postgres-backed cabana suite on a machine with Docker; this machine has none.
+- Plan 03 can reuse `openDryRunDB` and `newDryRunListService` for full coverage of the relation builder (hasOne, self-referencing, soft delete, polymorphic refusal, sort validation) and of `listState.ts`.
+
+---
+*Phase: 16.1-journal-admin-urgent-fixes-relation-search-list-state-post-f*
+*Completed: 2026-10-09*
+
+## Self-Check: PASSED
+
+All created files exist; commits 06d75cb, bd8626e, 9cac45c (summercms.go) and 528d2d8 (sm-journal-plugin) are present.
--- a/.planning/phases/16.1-journal-admin-urgent-fixes-relation-search-list-state-post-f/deferred-items.md
+++ b/.planning/phases/16.1-journal-admin-urgent-fixes-relation-search-list-state-post-f/deferred-items.md
@@ -0,0 +1,13 @@
+# Phase 16.1 deferred items
+
+Out-of-scope findings logged during execution. They were not fixed here.
+
+## From 16.1-01
+
+- `scripts/check-phase10.sh --hygiene` refuses 5 findings, all in files this plan does not touch. Because the clean scratch copy fails, `--self-test` also refuses ("self-test hygiene rejected the clean scratch copy"):
+  - `admin/src/api/files.ts:150`: `new XMLHttpRequest()` outside `admin/src/api/client.ts`.
+  - `admin/src/api/files.ts`: an unexpected file in `admin/src/api`.
+  - `admin/src/components/form/fields/FileCaptionModal.vue`, `admin/src/components/form/fields/MLField.vue` and `admin/src/components/form/mlLocale.ts`: not imported by any test.
+
+  The browser-storage rule that plan 01 changed passes: `listState.ts` is allowed, and `listState.ts` is imported by `admin/tests/list/ListView.test.ts`.
+- Each Docker-backed test in `modules/cabana` fails on this machine with "permission denied" on the Docker socket (217 failures in `go test ./modules/cabana`, each reporting a Docker error). This is the known environment limit; run that suite on a machine with Docker before the plan 02 tag.