test(trip): publish-toggle Playwright specs + plan/spec docs; bump user pin
TP1/TP1b/TP2–TP6 cover the owner gate, coverless drafts, cache-correct hide/restore, the active-trip confirm, backend authz (401/403/400), and the home fallback. The suite pins site.owner_username to the authenticated test user (restore on teardown) and runs serially — it mutates global config and clears the shared cache, so it collides with parallel readers. Bumps the user/ pin to the finished trip-publish-toggle content (064f0f0) and marks the plan Complete. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mpdu3Dt1iVoozHwAMyjrbn
This commit is contained in:
Claude Opus 4.8
@@ -0,0 +1,266 @@
|
||||
---
|
||||
title: Trip Publish/Unpublish Toggle - Plan
|
||||
type: feat
|
||||
date: 2026-07-08
|
||||
origin: docs/working/specs/2026-07-08-trip-publish-toggle-design.md
|
||||
artifact_contract: ce-unified-plan/v1
|
||||
artifact_readiness: implementation-ready
|
||||
product_contract_source: legacy-requirements
|
||||
execution: code
|
||||
---
|
||||
|
||||
# Trip Publish/Unpublish Toggle - Plan
|
||||
|
||||
**Status:** ✅ Complete (2026-07-08)
|
||||
|
||||
## Goal Capsule
|
||||
|
||||
- **Objective:** Let the logged-in site owner publish/unpublish any trip from the `/trips` listing, with correct page-tree cache invalidation so the change is reflected everywhere on the next load. Anonymous/non-owner visitors see no change.
|
||||
- **Authority hierarchy:** The design doc (`docs/working/specs/2026-07-08-trip-publish-toggle-design.md`) is authoritative for behavior; this plan is authoritative for sequencing and file-level implementation. Repo conventions (CLAUDE.md) and the cited existing patterns override any incidental detail here.
|
||||
- **Stop conditions:** Surface a blocker if implementation reveals that Grav 2.0's `$page->save()` does not persist `published` from a mutated header (the pattern KTD1 depends on), or that `$pages->find()` refuses to resolve unpublished trips from the listing context — either contradicts the design doc's cited behavior.
|
||||
- **Execution profile:** Standard feature — one owner-gated API write, one shared partial, two template edits, one new bundled JS file, CSS, and seven Playwright specs (TP1, TP1b, TP2–TP6). Test-after is fine except U7, which is written against the finished surfaces.
|
||||
- **Tail ownership:** Rebuild theme assets (`make build-assets`) after U6; run the trip Playwright suite after U7.
|
||||
|
||||
---
|
||||
|
||||
## Product Contract
|
||||
|
||||
### Summary
|
||||
|
||||
Add an owner-only publish/unpublish switch to each card on the `/trips` listing. The switch POSTs to a new `entry-actions` route that mutates the trip's `trip.md` frontmatter (`published: true|false`), then clears and invalidates Grav's page-tree cache. The trip detail page carries no publish UI — an unpublished trip's detail page 404s for everyone including the owner, so management is listing-only. When the active trip is unpublished, the home page falls back to its between-trips / pre-departure state.
|
||||
|
||||
### Problem Frame
|
||||
|
||||
Publishing a trip today means editing `trip.md` frontmatter by hand (or via Admin) and manually clearing cache. The owner wants a reversible in-UI toggle. The listing is the only viable surface: it already shows drafts to the owner and is fully reversible, whereas the detail page is unreachable while a trip is unpublished. The change is security-sensitive (an owner-only write) and cache-sensitive (publish state feeds `.published()` collections, routability, nav, and the home render, all keyed through the page-tree index — the exact class of bug fixed in `deleteEntry`).
|
||||
|
||||
### Requirements
|
||||
|
||||
**Owner gate & authorization**
|
||||
- R1. The publish write control renders only for the owner: `grav.user.authenticated and grav.user.username == grav.config.site.owner_username`. This is broader than `owner_can_edit` in `trip.html.twig` (which also requires the active trip) — publishing must work on any trip.
|
||||
- R2. The backend enforces the owner check independently of the UI (defense in depth): anonymous → 401, authenticated non-owner → 403, with frontmatter unchanged on disk.
|
||||
- R3. The endpoint enforces the same `api.pages.write` scope cap as the stock media/page-write endpoints (owner already holds it).
|
||||
|
||||
**Publish write**
|
||||
- R4. `POST /api/v1/trip/{slug}/publish` with body `{ "published": true|false }` sets the trip's published state and persists it to `trip.md` frontmatter.
|
||||
- R5. A missing or non-boolean `published` value is rejected with 400 (no silent coercion).
|
||||
- R6. The slug is validated as a safe single segment; the target must resolve through the page tree to a direct child of `/trips`, else 404.
|
||||
- R7. `find()` resolves unpublished trips too, so the owner can republish a draft from the listing.
|
||||
- R8. On success the endpoint clears the cache (`deleteAll()` + `Cache::invalidateCache()`) and returns 204, and writes an audit-log line.
|
||||
|
||||
**Listing surface**
|
||||
- R9. The owner sees unpublished trips in the `/trips` listing (with a `Draft` badge); anonymous/non-owner listings are unchanged (published only).
|
||||
- R10. Each owner-visible card carries a toggle switch overlaid on the cover image, top-right, that does not sit inside the card's navigating `<a>`. The switch is legible over arbitrary cover photos and carries a ≥44px touch target clear of the anchor hit area.
|
||||
- R11. The switch is accessible: `role="switch"`, `aria-checked`, and a per-instance accessible name identifying the trip.
|
||||
|
||||
**Interaction & feedback**
|
||||
- R12. Unpublishing the active trip prompts a `window.confirm` warning that the home page loses it; cancelling reverts the switch.
|
||||
- R13. A toggle in flight is disabled (`aria-busy`, dimmed, wait cursor), ignoring further toggles until success or failure revert.
|
||||
- R14. On success the UI updates optimistically in place (switch position/label, `Draft` badge, `data-published`) with no full reload; the card stays visible to the owner.
|
||||
- R15. On failure the switch reverts and an error surfaces via a shared page-level `aria-live` toast (401/403 → "sign in again"; other → "Couldn't update — try again.").
|
||||
|
||||
**Home fallback**
|
||||
- R16. When the resolved active trip is unpublished, `home.html.twig`'s active-trip branch does not render; home falls through to its between-trips / pre-departure state. `site.active_trip` is not modified.
|
||||
|
||||
### Scope Boundaries
|
||||
|
||||
**Out of scope (v1)**
|
||||
- Bulk publish/unpublish.
|
||||
- Scheduling / publish dates.
|
||||
- Cascading child (dailies/stories) publish state — unpublishing a trip does not change its children.
|
||||
- Reordering trips by publish state (order stays date desc).
|
||||
- Any publish/unpublish write control or `Draft` indicator on the trip detail page (`trip.html.twig`) — management is listing-only by design.
|
||||
|
||||
**Non-goal clarification**
|
||||
- This toggle governs only whether a trip appears in the `/trips` listing; it is not a content-privacy control. A story reachable by a direct link stays reachable while its parent trip is unpublished, which is acceptable.
|
||||
|
||||
---
|
||||
|
||||
## Planning Contract
|
||||
|
||||
### Key Technical Decisions
|
||||
|
||||
- KTD1. **Persist published state by mutating the page header before save, not `$page->published()`.** In Grav 2.0 `$page->published($v)` sets only the in-memory property (`Page.php:1714`), while `save()` serializes from the header object (`Page.php:1256`) and the flag is read one-way from the header at init (`Page.php:541`). Mirror `cache-on-save`'s header-mutation pattern: `$header = $page->header(); $header->published = $published; $page->save();`. Without this the on-disk `trip.md` is unchanged and the toggle silently no-ops.
|
||||
- KTD2. **Reject non-boolean `published` explicitly; never `(bool)`-cast.** `array_key_exists('published', $body) && is_bool($body['published'])` or 400. A cast coerces `"false"`, `0`, `""`, or a missing key into a valid boolean and never rejects, contradicting R5.
|
||||
- KTD3. **Clear the cache with `deleteAll()` + `Cache::invalidateCache()`.** `deleteAll()` alone drops cache stores but does not rebuild the page-tree index (keyed on folderHash under `cache.check.method: folder`), so the listing/nav/home render stale. This is the same fix as `deleteEntry` — see `docs/solutions/integration-issues/grav-deleteall-doesnt-invalidate-page-tree-index.md`.
|
||||
- KTD4. **Resolve the trip through `$pages->find()` + a parent-route assertion, never raw path concatenation.** New guard `EntryScopeGuard::resolveTripChild($grav, $slug)` mirrors `resolveActiveDailyChild`: call `enablePages()` (guarded by `method_exists` — the API context lazily disables the tree), `find('/trips/' . $slug)`, then assert the resolved page's parent route is exactly `/trips`. Reuses `isSafeSegment` for traversal safety.
|
||||
- KTD5. **The JS request must send `Content-Type: application/json`.** The API's `JsonBodyParserMiddleware` (`JsonBodyParserMiddleware.php:16`) only parses the body when that header is present; without it the body decodes to `[]`, the strict `is_bool` guard sees no key, and every toggle 400s. Model the send on `post-form.js`'s `apiSend` (JSON body + both headers + `credentials: 'include'`), **not** `feed-actions.js` (a body-less DELETE with no `Content-Type`).
|
||||
- KTD6. **The card toggle is an overlay sibling of the cover, not a child of the card `<a>`.** A toggle inside the anchor would navigate on click. Restructure the card so the cover sits in a positioned wrapper and the toggle overlays it as a sibling. Reuse the existing `.journal-draft-badge` styling (Field Notes paper/teal) so the pill stays legible over any cover.
|
||||
- KTD7. **Gate `home.html.twig`'s active-trip branch on `trip.published` as well as `config.site.travelling`.** `trip` is already resolved at `home.html.twig:10`; adding `and trip.published` to the branch condition is the whole home fallback — no need to touch `site.active_trip`.
|
||||
- KTD8. **New JS file needs an esbuild build entry.** `js/src/trip-publish.js` does not build automatically — add an esbuild invocation to the theme's `package.json` `build` script (same `--bundle --minify --format=iife` shape as the `feed-actions.js` entry) so `make build-assets` emits `js/trip-publish.js`.
|
||||
|
||||
### High-Level Technical Design
|
||||
|
||||
Request flow for one toggle:
|
||||
|
||||
```mermaid
|
||||
sequenceDiagram
|
||||
participant U as Owner (listing card switch)
|
||||
participant JS as trip-publish.js
|
||||
participant API as entry-actions route
|
||||
participant Ctl as setTripPublished
|
||||
participant G as EntryScopeGuard
|
||||
participant FS as trip.md + cache
|
||||
|
||||
U->>JS: change (with active-trip confirm if applicable)
|
||||
JS->>JS: disable switch, aria-busy
|
||||
JS->>API: POST /api/v1/trip/{slug}/publish {published}
|
||||
API->>Ctl: dispatch
|
||||
Ctl->>Ctl: getUser (401 anon) + requirePermission(api.pages.write)
|
||||
Ctl->>G: isOwnerUser (else 403)
|
||||
Ctl->>Ctl: isSafeSegment(slug) (else 400)
|
||||
Ctl->>G: resolveTripChild(slug) (else 404)
|
||||
Ctl->>Ctl: validate published is_bool (else 400)
|
||||
Ctl->>FS: header.published = v; save(); deleteAll(); invalidateCache()
|
||||
Ctl-->>JS: 204
|
||||
JS->>U: optimistic UI (switch, Draft badge, data-published)
|
||||
```
|
||||
|
||||
### Assumptions
|
||||
|
||||
- The Playwright harness runs as the owner because the local test setup treats `testrunner` as `owner_username` — the same setup the existing owner-only delete-flow specs rely on. The new specs inherit it rather than introducing a new override mechanism. Verify by mirroring `tests/ui/post/delete-flow.spec.js` (which already exercises the owner API gate).
|
||||
- `css/style.css` is hand-authored (the theme has no active SCSS pipeline for it), so toggle/badge styling is added there directly, next to the existing `.journal-draft-badge` (line 283) and `.trip-card*` (line 1220+) rules.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Units
|
||||
|
||||
### U1. Guard: resolve a trip as a direct child of `/trips`
|
||||
|
||||
- **Goal:** Add `EntryScopeGuard::resolveTripChild($grav, $slug): ?PageInterface`, the trip-scoped analogue of `resolveActiveDailyChild`, so the controller resolves the target safely (R6, R7, KTD4).
|
||||
- **Requirements:** R6, R7.
|
||||
- **Dependencies:** none.
|
||||
- **Files:** `user/plugins/cache-on-save/classes/EntryScopeGuard.php`.
|
||||
- **Approach:** New static method: reject via `isSafeSegment($slug)` → null; get `$pages = $grav['pages']`; if `method_exists($pages, 'enablePages')` call it; `$page = $pages->find('/trips/' . $slug)`; return null unless `$page !== null` and `$page->parent()?->route() === '/trips'`. No raw path concatenation beyond the `find()` argument, matching the sibling method's style. Do not filter on published state — `find()` returning drafts is required for republish (R7).
|
||||
- **Patterns to follow:** `EntryScopeGuard::resolveActiveDailyChild` in the same file (lines 104–129).
|
||||
- **Test scenarios:** Covered end-to-end by U7 (TP2/TP3 exercise resolve-and-republish; TP5 exercises the reject paths). No standalone PHP unit-test harness exists in this repo.
|
||||
- **Verification:** Method exists and returns a `PageInterface` for a real trip slug, `null` for an unsafe segment, a non-existent slug, and a page whose parent is not `/trips`.
|
||||
|
||||
### U2. API route + `setTripPublished` controller
|
||||
|
||||
- **Goal:** Register `POST /api/v1/trip/{slug}/publish` and implement the owner-gated write that persists published state and invalidates cache (R2–R8).
|
||||
- **Requirements:** R2, R3, R4, R5, R6, R7, R8.
|
||||
- **Dependencies:** U1.
|
||||
- **Files:** `user/plugins/entry-actions/entry-actions.php`, `user/plugins/entry-actions/classes/EntryActionsApiController.php`.
|
||||
- **Approach:** In `onApiRegisterRoutes`, add `$routes->post('/trip/{slug}/publish', [EntryActions\EntryActionsApiController::class, 'setTripPublished'])`. In the controller, mirror `deleteEntry` step-for-step: `getUser` (401), `requirePermission($request, 'api.pages.write')`, `isOwnerUser` (else `ForbiddenException`), `isSafeSegment` (else 400), `resolveTripChild` (else `NotFoundException`). Read body via `getRequestBody`; enforce KTD2 (`array_key_exists` + `is_bool`, else 400); assign the raw boolean. Persist per KTD1 (mutate `$page->header()->published`, then `$page->save()`). Clear cache per KTD3. Log `owner "%s" set trip "%s" published=%s`. Return `ApiResponse::noContent()`.
|
||||
- **Patterns to follow:** `EntryActionsApiController::deleteEntry` (guard chain, cache calls, audit log) and `reorderPhotos` (JSON body read) in the same file; `cache-on-save` header-mutation for the save.
|
||||
- **Test scenarios:** Covered by U7 — TP2 (publish→off persists + hides for anon), TP3 (republish), TP5 (401 anon, 403 non-owner, frontmatter unchanged), plus the 400 non-boolean path asserted via a direct API call in TP5.
|
||||
- **Verification:** `curl` (or the Playwright request context) as owner with `{"published":false}` returns 204 and `trip.md` on disk gains `published: false`; anon → 401; non-owner → 403; missing/`"false"`/`0` body → 400.
|
||||
|
||||
### U3. Shared toggle partial + styling
|
||||
|
||||
- **Goal:** Create `partials/trip-publish-toggle.html.twig` (the sliding switch + `Draft` badge) and its CSS, so both the markup and its legible-over-cover styling exist as one reusable unit (R10, R11, KTD6).
|
||||
- **Requirements:** R10, R11.
|
||||
- **Dependencies:** none (consumed by U4).
|
||||
- **Files:** `user/themes/intotheeast/templates/partials/trip-publish-toggle.html.twig`, `user/themes/intotheeast/css/style.css`.
|
||||
- **Approach:** Partial params: `trip` (Page), `is_active` (bool). Render a styled checkbox switch (`role="switch"`, `aria-checked` bound to `trip.published`, `aria-label="Published — {{ trip.title }}"`) plus a `Draft` badge when `not trip.published`. Emit `data-trip-slug`, `data-trip-route`, `data-published`, `data-active` for the JS. Wrap the control class `.trip-publish-toggle`. CSS: a solid pill/chip background reusing `.journal-draft-badge` colors so it stays legible on any cover; absolute positioning is applied by the card container in U4 (which must exist even for a coverless draft — see U4), but the switch's own visual (track/knob, ≥44px hit area, dimmed `[aria-busy]` + wait-cursor pending state per R13, and a legible keyboard focus ring that reads over a busy cover photo) lives here.
|
||||
- **Visible failure toast (not sr-only):** the design's page-level toast (R15) is meant for the sighted owner, but `feed-actions.js`'s live region is `sr-only` (visually hidden) and the trip card — unlike the delete flow — has no inline message slot, so a straight reuse would leave a sighted owner seeing only a silent switch revert. Add CSS here for a **visible** page-level toast as a **new, separate DOM element and CSS class** (e.g. `#trip-publish-live` / a `.trip-publish-toast` class, with `role="status"`, `aria-live="polite"`, positioned so it does not depend on the cramped card overlay) that U6 populates. This element is distinct from `feed-actions.js`'s `#feed-actions-live` / `.sr-only` region: reuse the *copy* but do **not** modify the shared `.sr-only` utility (still used by `feed-actions.js` on `trip.html.twig`/`home.html.twig`) or make its live region visible. Toast behavior: auto-dismiss after ~5s, include a manual close control, and replace (not queue) the message if a new failure arrives before the previous one dismisses.
|
||||
- **Patterns to follow:** existing `.journal-draft-badge` (style.css:283) and the `journal-draft-badge` span in `partials/entry-journal.html.twig:7`.
|
||||
- **Test scenarios:** Rendered presence/absence is asserted by U7 TP1 (owner sees `.trip-publish-toggle`, anon does not); `Draft` badge presence by TP2. Accessible name/`role` are asserted structurally in TP1.
|
||||
- **Verification:** Partial renders a switch with the correct `data-*` and `aria-*` for a published and an unpublished trip; the pill is legible over a cover image in the browser.
|
||||
|
||||
### U4. `/trips` listing — owner-aware collection, card restructure, JS load
|
||||
|
||||
- **Goal:** Make the listing owner-aware (drafts for owner), restructure each card so the toggle overlays the cover as a non-anchor sibling, render the toggle for the owner, and load the JS gated on owner (R9, R10, R12–R15 wiring).
|
||||
- **Requirements:** R1, R9, R10.
|
||||
- **Dependencies:** U3, U6 (built `js/trip-publish.js`).
|
||||
- **Files:** `user/themes/intotheeast/templates/trips.html.twig`.
|
||||
- **Approach:** Compute `is_owner` (R1) at the top. Change the collection to `{% set trips = (is_owner ? page.children : page.children.published())|sort(...) %}`. Restructure the card: keep the navigating `<a class="trip-card">` for cover + title + meta, but wrap the cover in a positioned container so `{% if is_owner %}{% include 'partials/trip-publish-toggle.html.twig' with { trip: trip, is_active: is_active } only %}{% endif %}` sits as an overlay sibling outside the click-navigation path. **The positioned container must exist even when the cover macro emits nothing** — see the coverless-draft note below. Gate the asset: `{% if is_owner %}{% do assets.addJs('theme://js/trip-publish.js', {group: 'bottom'}) %}{% endif %}` (mirrors the `feed-actions.js` gate in `home.html.twig:27`). Compute `is_active` robustly — `site.active_trip` may be a full route (`/trips/x`) or a bare slug — by normalizing both sides before comparing, e.g. `{% set active = config.site.active_trip|trim('/') %}` then `{% set is_active = (active == trip.route|trim('/')) or (active == ('trips/' ~ trip.slug)) %}`. Comparing only against `trip.route`/`trip.url` (full-route form) would silently drop the R12 active-trip confirm if the config ever stores a bare slug (both forms are already supported in `helpers.js` and `cache-on-save`).
|
||||
- **Coverless-draft state (blocks the primary use case):** the shared cover macro emits its wrapper + `<img>` only when a cover exists (an author-set `cover_image` or a published journal image), and **nothing at all** for a freshly-created draft trip with neither — which is exactly the most common publish-toggle target. The positioned container the toggle overlays must therefore be provided by the card itself (a min-height header strip or the card element), not by the cover wrapper, so the toggle has an anchor whether or not `cover.render` emits an image. Enumerate this state in U3's markup and assert it in U7 (a no-cover fixture trip still shows a working toggle).
|
||||
- **Patterns to follow:** the owner-aware feed collection + gated `addJs` in `home.html.twig:23-27`; the existing card markup in `trips.html.twig:16-34`.
|
||||
- **Test scenarios:** Covered by U7 — TP1 (owner sees toggle + draft trip; anon does not, and the draft trip is absent for anon), TP2/TP3 (draft badge on listing after toggle).
|
||||
- **Verification:** Owner load of `/trips` shows `.trip-publish-toggle` on each card and includes unpublished fixture trips; anon load shows neither; clicking a card cover still navigates (toggle click does not).
|
||||
|
||||
### U5. Home fallback when the active trip is unpublished
|
||||
|
||||
- **Goal:** Gate the home active-trip branch on the active trip being published so an unpublished active trip falls through to the between-trips / pre-departure state (R16, KTD7).
|
||||
- **Requirements:** R16.
|
||||
- **Dependencies:** none.
|
||||
- **Files:** `user/themes/intotheeast/templates/home.html.twig`.
|
||||
- **Approach:** Change the branch condition at `home.html.twig:12` from `{% if config.site.travelling %}` to `{% if config.site.travelling and trip.published %}`. `trip` is already resolved at line 10. No change to `site.active_trip`.
|
||||
- **Patterns to follow:** existing branch structure in `home.html.twig`.
|
||||
- **Test scenarios:** Covered by U7 TP6 (active fixture trip unpublished → home renders between-trips/pre-departure, not the draft active-trip view).
|
||||
- **Verification:** With `travelling: true` and the active trip unpublished, `/` renders the fallback branch; republishing restores the active-trip view.
|
||||
|
||||
### U6. `trip-publish.js` + esbuild build wiring
|
||||
|
||||
- **Goal:** Implement the toggle behavior (confirm, pending, POST, optimistic success, failure revert + toast) and wire it into the theme build so `make build-assets` emits `js/trip-publish.js` (R12–R15, KTD5, KTD8).
|
||||
- **Requirements:** R12, R13, R14, R15.
|
||||
- **Dependencies:** U2 (endpoint), U3 (markup contract).
|
||||
- **Files:** `user/themes/intotheeast/js/src/trip-publish.js`, `user/themes/intotheeast/package.json`.
|
||||
- **Approach:** Bind each `.trip-publish-toggle`. On change: if turning **off** and `data-active` is true → `window.confirm('This is your active trip — unpublishing it also removes it from the home page. Unpublish anyway?')`; on cancel revert and stop (R12). Set pending: disable the switch, `aria-busy`, dim + wait cursor, ignore further toggles (R13). Send `POST /api/v1/trip/<slug>/publish` with `headers: { 'Content-Type': 'application/json', Accept: 'application/json' }`, `body: JSON.stringify({ published })`, `credentials: 'include'` — modeled on `post-form.js` `apiSend` (KTD5). Success: flip `data-published`, toggle the `Draft` badge, update switch position/label/`aria-checked` in place; re-enable (R14). Failure: revert switch to prior state, re-enable, surface an error via the **visible** shared page-level toast defined in U3 (`role="status"`, `aria-live="polite"` — reuse the `feed-actions.js` copy but not its `sr-only` region, so a sighted owner actually sees it: 401/403 → "sign in again"; other → "Couldn't update — try again.") (R15). Wire the build: add an esbuild entry for `js/src/trip-publish.js` to `package.json` `build`, same flags as the `feed-actions.js` entry.
|
||||
- **Patterns to follow:** `post-form.js` `apiSend` (js/src/post-form.js:890) for the request; `feed-actions.js` for the live-region + error copy + double-tap lock; the `feed-actions.js` esbuild entry in `package.json` `build`.
|
||||
- **Test scenarios:** Covered by U7 — TP2/TP3 (optimistic flip + persistence), TP4 (active-trip confirm dismiss leaves published). Failure/toast copy is exercised where practical in TP5.
|
||||
- **Verification:** `make build-assets` produces `js/trip-publish.js`; in the browser, toggling a card updates it in place without reload; unpublishing the active trip prompts a confirm.
|
||||
|
||||
### U7. Playwright specs (TP1, TP1b, TP2–TP6)
|
||||
|
||||
- **Goal:** Cover the owner gate, cache-correct hide/restore, active-trip confirm, authz, and home fallback (R1–R16 as observable behavior).
|
||||
- **Requirements:** R1–R16.
|
||||
- **Dependencies:** U1–U6.
|
||||
- **Files:** `user/themes/intotheeast/...` (none); `tests/ui/trip/trip-publish.spec.js`.
|
||||
- **Approach:** Run as the owner (same setup as the delete-flow specs). **New scaffolding this unit must build (not mirrored from the entry helpers):** the existing `createPhotoEntry`/`cleanupEntry`/`findEntry` helpers create/clean *entry* folders inside the active trip's `dailies` (`TRACKER_DIR`) — none create a *trip*. This unit needs a small on-disk trip-fixture helper that writes `pages/01.trips/<fixture>/` with a `trip.md` (a `date` for the listing sort, `published` set per test), plus `01.dailies/` and `04.stories/` `routable:false` container `.md` files, and cleans it up. TP6 additionally repoints `site.active_trip` to the fixture with `travelling: true` — the cited home-suite specs only patch `travelling`, never override `active_trip`, so this override is also new (restore `site.yaml` on teardown). Use the DEL4 fixture-then-reload assertion shape from `delete-flow.spec.js`.
|
||||
- **TP1 — gate:** owner load of `/trips` shows `.trip-publish-toggle`; anon (cleared storageState) does not, and an unpublished fixture trip is absent for anon.
|
||||
- **TP1b — coverless draft:** a fixture trip with no `cover_image` and no published entry image still renders a working `.trip-publish-toggle` for the owner (guards the coverless-container state from U4).
|
||||
- **TP2 — unpublish hides it (caching):** owner toggles a published fixture off → reload `/trips` as anon → trip absent; owner reload → `Draft` badge present on the listing (the detail page 404s for the owner too). Mirrors DEL4's page-tree-index assertion.
|
||||
- **TP3 — republish restores it:** owner on `/trips` toggles a Draft fixture back on → anon reload sees it; assert on the listing, not the detail page.
|
||||
- **TP4 — active-trip confirm:** unpublishing the active trip prompts a confirm; dismissing leaves it published.
|
||||
- **TP5 — authz:** `POST /api/v1/trip/<slug>/publish` as anon → 401; as an authenticated non-owner → 403; frontmatter unchanged on disk. Include a non-boolean-body → 400 assertion. **The 403 leg needs a second, authenticated non-owner identity** — the harness authenticates only one account (`auth.setup.js` → one `storageState`), so this leg requires either a second account + storageState (e.g. a non-owner login) or an in-test override of `owner_username` to a value the logged-in test user does not match, then a restore on teardown. This is not provided by the delete-flow setup; pick one approach and wire it explicitly.
|
||||
- **TP6 — active trip unpublished → home falls back:** with the fixture set as `site.active_trip` and `travelling: true`, unpublish it → reload `/` → home renders between-trips/pre-departure, not the draft active-trip view (needs the `active_trip` override on the fixture; mirror the home-suite setup).
|
||||
- **Patterns to follow:** `tests/ui/post/delete-flow.spec.js` (owner fixture + reload + on-disk assertion), `tests/ui/trip/trips-list.spec.js` (listing selectors), `tests/ui/post/anon-view.spec.js` (anon storageState + draft-visibility).
|
||||
- **Test scenarios:** the six specs above are the scenarios.
|
||||
- **Verification:** `npm run test:ui -- tests/ui/trip/trip-publish.spec.js` (from `tests/`) passes all seven (TP1, TP1b, TP2–TP6), with fixture folders cleaned up afterward.
|
||||
|
||||
---
|
||||
|
||||
## Verification Contract
|
||||
|
||||
| Gate | Command | Applies to |
|
||||
|---|---|---|
|
||||
| Rebuild theme assets | `make build-assets` | U6 (emits `js/trip-publish.js`) |
|
||||
| Trip publish specs | `npm run test:ui -- tests/ui/trip/trip-publish.spec.js` (run from `tests/`) | U7 |
|
||||
| Full trip suite (no regressions) | `npm run test:ui -- tests/ui/trip` | U4, U5, U7 |
|
||||
| Backend contract (manual/spec) | owner POST → 204 + on-disk `published:` change; anon → 401; non-owner → 403; non-boolean → 400 | U2 |
|
||||
|
||||
Run the dev stack for tests via the worktree's own container (`docker compose -p itte-<feature> up`) per the worktree dev-server convention. Do **not** flip any dev/prod mode flags to work around caching — the cache-clear is handled in-code (KTD3).
|
||||
|
||||
---
|
||||
|
||||
## Definition of Done
|
||||
|
||||
**Global**
|
||||
- All seven Playwright specs (TP1, TP1b, TP2–TP6) pass; the broader `tests/ui/trip` suite shows no regressions.
|
||||
- `make build-assets` emits `js/trip-publish.js`; `js/trip-publish.js` and `js/feed-actions.js` are both current (no hand-edits to built files).
|
||||
- Anonymous and non-owner behavior is unchanged: no toggle rendered, listing shows published trips only, backend rejects with 401/403.
|
||||
- No abandoned/experimental code left in the diff; the plan status line is updated to `✅ Complete (YYYY-MM-DD)`.
|
||||
|
||||
**Per unit**
|
||||
- U1: `resolveTripChild` returns the trip page for a real slug and `null` for unsafe/nonexistent/wrong-parent inputs.
|
||||
- U2: endpoint persists `published` to `trip.md`, invalidates cache, returns 204/400/401/403/404 correctly.
|
||||
- U3: partial renders the accessible switch + `Draft` badge with correct `data-*`, legible over a cover.
|
||||
- U4: owner listing includes drafts + toggles; anon listing unchanged; card cover still navigates.
|
||||
- U5: unpublished active trip → home fallback; published → active-trip view.
|
||||
- U6: JS confirm/pending/optimistic/revert behaviors work in the browser; build entry wired.
|
||||
- U7: specs implemented, fixtures cleaned up.
|
||||
|
||||
---
|
||||
|
||||
## Risks & Dependencies
|
||||
|
||||
- **Grav 2.0 save semantics (KTD1).** If a mutated-header `save()` does not persist `published`, the toggle no-ops silently. Mitigation: TP2 asserts the on-disk frontmatter change, not just UI; the `cache-on-save` plugin already relies on this pattern.
|
||||
- **Cache staleness (KTD3).** Omitting `invalidateCache()` reproduces the `deleteEntry` bug (stale listing/nav/home). Mitigation: TP2/TP3 assert visibility after a full reload as a fresh (anon) client.
|
||||
- **Build step required (KTD8).** Editing `js/src/trip-publish.js` without adding the esbuild entry (or without running `make build-assets`) ships nothing. Mitigation: DoD requires the built file to be current; U6 owns the `package.json` edit.
|
||||
- **Test-harness owner identity.** The specs assume `testrunner` acts as owner (as the delete-flow specs do). If that assumption is wrong, the owner-gated specs fail fast at the gate; resolve by matching the existing owner-only spec setup rather than inventing a new override.
|
||||
- **Upstream dependency:** none external; this is self-contained within `user/` (theme + two custom plugins) and the `tests/` harness.
|
||||
|
||||
---
|
||||
|
||||
## Open Questions
|
||||
|
||||
Both are non-blocking (defense-in-depth / UX-copy) and do not hold up implementation, but resolve them before or during U2/U6.
|
||||
|
||||
- **CSRF boundary is implicit.** The endpoint is a session-cookie-authenticated write with `credentials: 'include'`. Its only cross-origin protection is incidental: KTD5's required `Content-Type: application/json` plus the strict `is_bool` guard force a CORS-preflighted request an attacker cannot forge — *unless* the `api` plugin emits permissive CORS headers. Verify the `api` plugin sends no `Access-Control-Allow-Origin`/`-Credentials` that would defeat the preflight, and state the preflight as the intended CSRF boundary in U2 (or add an explicit token check if it does).
|
||||
- **Draft is not a privacy control (owner mental model).** Unpublishing hides the trip from the `/trips` listing but leaves every child URL (stories, dailies, media) publicly served (documented non-goal). An owner clicking a `Draft` switch may reasonably expect the content to go private. Decide whether the unpublish `confirm()` copy (R12) or toggle help text should say child content stays reachable by direct link, so `Draft` is not mistaken for a retract-content action.
|
||||
|
||||
### From 2026-07-08 doc review
|
||||
|
||||
- **Owner test-identity for the Playwright suite is unspecified and contradicts committed config (adversarial, P1 — blocking for U7).** The Assumptions block asserts the harness treats `testrunner` as `owner_username`, but committed `user/config/site.yaml` sets `owner_username: mischa`, and `EntryScopeGuard::isOwnerUser` is a strict username match with no super-admin bypass. So every owner-gated spec (TP1, TP1b, TP2, TP3, TP4, and TP5's owner leg) depends on untracked local state (a dirty `site.yaml` or a `.env` `GRAV_TEST_USER` override) that the new specs cannot reproducibly "inherit" — and TP5's non-owner override is described in the *inverted* direction (it only makes sense if `testrunner` were owner by default). **Resolve before writing U7:** confirm the worktree container's actual `GRAV_TEST_USER` / `owner_username` binding, then replace the "inherit testrunner-as-owner" assumption with an explicit tracked suite-setup step that pins `site.owner_username` to the authenticated test user (restore on teardown) and derives TP5's 403 leg from a value that user does not match. Do not rely on the committed `owner_username: mischa` or an untracked local `site.yaml`.
|
||||
@@ -5,11 +5,14 @@
|
||||
|
||||
## Goal
|
||||
|
||||
Let the logged-in **owner** publish/unpublish any trip directly from the UI, on
|
||||
two surfaces: the **Past Trips listing** (`/trips`) and each **trip detail page**
|
||||
(`/trips/<slug>`). Anonymous/non-owner visitors see no change. Toggling must
|
||||
correctly invalidate Grav's page-tree cache so the change is reflected
|
||||
everywhere on the next load.
|
||||
Let the logged-in **owner** publish/unpublish any trip directly from the UI. The
|
||||
**write control lives on one surface — the Past Trips listing** (`/trips`), which
|
||||
already shows drafts and toggles both directions reversibly. The **trip detail
|
||||
page** (`/trips/<slug>`) carries **no publish UI**: an unpublished trip's detail
|
||||
page 404s (for everyone, owner included), so a control there could only strand the
|
||||
owner and a `Draft` indicator there would be unreachable — see Surface 2.
|
||||
Anonymous/non-owner visitors see no change. Toggling must correctly invalidate
|
||||
Grav's page-tree cache so the change is reflected everywhere on the next load.
|
||||
|
||||
## Owner gate
|
||||
|
||||
@@ -21,9 +24,9 @@ A single rule, mirroring the post feed's owner logic:
|
||||
|
||||
This is **broader** than `owner_can_edit` in `trip.html.twig` (which also
|
||||
requires the page to be the active trip). Publishing must work on *any* trip, so
|
||||
it gets its own `is_owner` flag. `is_owner` is computed in both `trips.html.twig`
|
||||
and `trip.html.twig`. The backend enforces the same owner check independently
|
||||
(defense in depth) — the UI gate is not the security boundary.
|
||||
it gets its own `is_owner` flag, computed in `trips.html.twig` (the listing — the
|
||||
only surface with the write control). The backend enforces the same owner check
|
||||
independently (defense in depth) — the UI gate is not the security boundary.
|
||||
|
||||
## Backend — extend the `entry-actions` plugin
|
||||
|
||||
@@ -45,15 +48,27 @@ In `EntryActionsApiController`, mirroring `deleteEntry`:
|
||||
3. `EntryScopeGuard::isOwnerUser($this->grav, $user)` — else `ForbiddenException`.
|
||||
4. Validate `slug` via `EntryScopeGuard::isSafeSegment` — else 400.
|
||||
5. Resolve the page via a **new** guard `EntryScopeGuard::resolveTripChild($grav, $slug)`:
|
||||
`$pages->find('/trips/' ~ slug)`, assert the resolved page's parent route is
|
||||
call `$pages->enablePages()` first (guarded by `method_exists` — the API request
|
||||
context lazily disables the page tree, exactly as `resolveActiveDailyChild` does),
|
||||
then `$pages->find('/trips/' ~ slug)`, assert the resolved page's parent route is
|
||||
exactly `/trips` (no raw path concatenation — same style as
|
||||
`resolveActiveDailyChild`). Return `null` → `NotFoundException`.
|
||||
6. Read desired state: `$published = (bool) ($body['published'] ?? …)`; reject a
|
||||
missing/non-bool value with 400.
|
||||
7. Set published + persist frontmatter. Use Grav's page API (verify exact call
|
||||
against `add-page-by-form` / `cache-on-save` savers before coding — likely
|
||||
`$page->published($published); $page->save();`). The write must land in
|
||||
`trip.md` frontmatter as `published: true|false`.
|
||||
`resolveActiveDailyChild`). Return `null` → `NotFoundException`. (`find()` returns
|
||||
unpublished trips too — verified against `Pages.php:966`/`1986` — so the owner can
|
||||
republish a draft from the listing.)
|
||||
6. Read desired state: reject a missing or non-boolean value with 400 —
|
||||
`if (!array_key_exists('published', $body) || !is_bool($body['published'])) → 400`
|
||||
— then assign the raw boolean (`$published = $body['published']`). Do **not**
|
||||
`(bool)`-cast the value: a cast silently coerces anything (`"false"`, `0`, `""`,
|
||||
a missing key) into a valid boolean and never rejects, contradicting the 400.
|
||||
7. Set published + persist frontmatter by mutating the page **header** before
|
||||
saving: `$header = $page->header(); $header->published = $published; $page->save();`
|
||||
— mirroring `cache-on-save`'s `setOverwriteMode()` header-mutation pattern. Do
|
||||
**not** rely on `$page->published($published)` alone: in Grav 2.0 that only sets
|
||||
the in-memory property (`Page.php:1714`), while `save()` serializes from the
|
||||
header object (`Page.php:1256`) and the flag is read one-way *from* the header at
|
||||
init (`Page.php:541`) — so the on-disk `trip.md` would be unchanged and the
|
||||
toggle would silently no-op. The write must land in `trip.md` frontmatter as
|
||||
`published: true|false`.
|
||||
8. **Caching:** `$this->grav['cache']->deleteAll(); Cache::invalidateCache();` —
|
||||
publish state feeds `.published()` collections and routability, both keyed
|
||||
through the page-tree index; without `invalidateCache()` the listing/nav/home
|
||||
@@ -72,6 +87,11 @@ Params: `trip` (the trip Page), `is_active` (bool, whether this trip is
|
||||
`data-published`, and `data-active` for the JS to read. Rendered only when
|
||||
`is_owner`.
|
||||
|
||||
The switch carries `role="switch"` + `aria-checked` and a per-instance accessible
|
||||
name — `aria-label="Published — {{ trip.title }}"` — so a screen-reader user on
|
||||
the listing (where every card's switch is otherwise identical) can tell which trip
|
||||
a toggle controls before triggering a destructive unpublish.
|
||||
|
||||
### Surface 1 — `/trips` listing (`trips.html.twig`)
|
||||
|
||||
- Make the collection owner-aware:
|
||||
@@ -85,39 +105,80 @@ Params: `trip` (the trip Page), `is_active` (bool, whether this trip is
|
||||
the card so the cover image is in a positioned wrapper and the toggle sits as
|
||||
an overlay sibling. Toggle placement: **absolutely positioned over the cover
|
||||
image, top-right corner.** `Draft` badge on unpublished cards.
|
||||
- **Legibility over arbitrary covers:** give the overlay toggle a solid pill/chip
|
||||
background reusing the `Draft`-badge styling (Field Notes paper/teal) so it stays
|
||||
legible on any cover photo, and a ≥44px touch target kept clear of the card `<a>`
|
||||
hit area.
|
||||
|
||||
### Surface 2 — trip detail page (`trip.html.twig`)
|
||||
|
||||
- Compute `is_owner` (separate from `owner_can_edit`).
|
||||
- Render the shared toggle in the header area, **top-right corner near the trip
|
||||
header**, with the `Draft` badge when unpublished.
|
||||
**No publish UI in v1 — management is listing-only.** The detail page gets neither
|
||||
a write toggle nor a `Draft` indicator, for a concrete reason: an unpublished trip
|
||||
is not routable, and Grav's frontend serves a 404 for unpublished pages to
|
||||
*everyone including the owner* (`Page::routable()` = `routable && published`, with
|
||||
no published routable child to redirect to since `dailies`/`stories` are
|
||||
`routable:false` — verified in `Pages::dispatch` / `Page.php:1772`). So a trip's
|
||||
detail page only ever renders while it is **published** — which means a `Draft`
|
||||
indicator there would be unreachable, and a write toggle could only *unpublish*,
|
||||
immediately stranding the owner on a page that 404s on the next load with no in-UI
|
||||
path back. All publish/unpublish therefore happens on the `/trips` listing
|
||||
(Surface 1), which shows drafts and is fully reversible. `trip.html.twig` needs no
|
||||
`is_owner` computation for this feature.
|
||||
|
||||
### JS — `js/src/trip-publish.js` → built to `js/trip-publish.js`
|
||||
|
||||
Loaded in the `bottom` group **only when `is_owner`** (like `feed-actions.js`).
|
||||
Loaded on `/trips` in the `bottom` group **only when `is_owner`** (gated like
|
||||
`feed-actions.js` — but note the request shape below differs from it).
|
||||
|
||||
- Binds each `.trip-publish-toggle` control.
|
||||
- Binds each `.trip-publish-toggle` control (listing cards only).
|
||||
- On change:
|
||||
- If turning **off** (unpublish) AND `data-active` is true → `window.confirm(
|
||||
'This is your active trip — unpublish it anyway?')`; if cancelled, revert the
|
||||
switch and stop.
|
||||
- `POST /api/v1/trip/<slug>/publish` with `{ published }`,
|
||||
`credentials: 'include'`.
|
||||
'This is your active trip — unpublishing it also removes it from the home page.
|
||||
Unpublish anyway?')`; if cancelled, revert the switch and stop. (Home falls back
|
||||
to its pre-departure state when the active trip is unpublished — see Edge cases.)
|
||||
- **Pending:** disable the switch and set `aria-busy` for the duration of the
|
||||
request, ignoring further toggles — guards against a double-tap, or a toggle
|
||||
during the active-trip `confirm()`, firing a second contradictory POST and
|
||||
racing the revert paths. Show it dimmed with a wait cursor while pending;
|
||||
re-enable on success or after the failure revert.
|
||||
- `POST /api/v1/trip/<slug>/publish` sending **`headers: { 'Content-Type':
|
||||
'application/json', Accept: 'application/json' }` and `body: JSON.stringify({
|
||||
published })`**, `credentials: 'include'`. Model this on `post-form.js`'s
|
||||
`apiSend`, **not** `feed-actions.js` (which is a body-less DELETE with no
|
||||
`Content-Type`). The `Content-Type: application/json` is load-bearing: the API's
|
||||
`JsonBodyParserMiddleware` only parses the body when that header is present
|
||||
(`JsonBodyParserMiddleware.php:16`); without it the body decodes to `[]`, the
|
||||
strict `is_bool` guard (backend step 6) sees no `published` key, and **every
|
||||
toggle 400s**.
|
||||
- **Success:** optimistic UI — flip `data-published`, toggle the `Draft` badge,
|
||||
update the switch position/label. No full reload needed (server state is
|
||||
persisted + cache invalidated for other surfaces).
|
||||
- **Failure:** revert the switch to its prior state and show an inline,
|
||||
`aria-live` error (reuse the copy style from `feed-actions.js`:
|
||||
401/403 → "sign in again"; other → "Couldn't update — try again.").
|
||||
update the switch position/label in place on the card. No full reload needed
|
||||
(server state is persisted + cache invalidated for other surfaces). The card
|
||||
stays visible to the owner either way (the owner-aware collection includes
|
||||
drafts).
|
||||
- **Failure:** revert the switch to its prior state and surface an error via one
|
||||
shared page-level `aria-live` toast region (the listing's corner overlay has no
|
||||
room for an inline message). Reuse the copy style from `feed-actions.js`:
|
||||
401/403 → "sign in again"; other → "Couldn't update — try again."
|
||||
|
||||
## Edge cases
|
||||
|
||||
- **Active trip unpublish** → JS `confirm()` (above). Allowed on confirm.
|
||||
- **Active trip unpublish** → JS `confirm()` (above), allowed on confirm. **Home
|
||||
then treats it as no active trip:** gate `home.html.twig`'s active-trip branch on
|
||||
the resolved active trip being **published** as well as `config.site.travelling`
|
||||
(`{% if config.site.travelling and trip.published %}` — `trip` is already resolved
|
||||
at `home.html.twig:10`). When the active trip is unpublished, home falls through to
|
||||
its between-trips / pre-departure state instead of rendering a draft trip. No need
|
||||
to touch `site.active_trip`.
|
||||
- **Anon / non-owner** → no toggle rendered; listing shows `.published()` only;
|
||||
backend rejects with 401/403.
|
||||
- **Unpublished trip visibility** → drops from the public `/trips` listing; its
|
||||
detail page 404s for anon (Grav default for unpublished/unroutable). Owner
|
||||
still sees it in the listing (Draft badge) and can re-publish.
|
||||
detail page 404s for **everyone including the owner** (Grav default for
|
||||
unpublished/unroutable — there is no owner-preview bypass). The owner still sees
|
||||
the trip in the `/trips` listing (Draft badge) and re-publishes from there.
|
||||
(Scope note: this toggle governs only whether the trip appears in the `/trips`
|
||||
listing — it is not a content-privacy control. Child dailies are aggregated inline
|
||||
by the trip page and are not individually linked; a story reachable by a direct
|
||||
link stays reachable, which is acceptable.)
|
||||
- **Child dailies/stories cascade** → out of scope for v1; unpublishing a trip
|
||||
does not change its children's published state.
|
||||
|
||||
@@ -130,13 +191,20 @@ the post specs. Use a throwaway fixture trip folder (create/cleanup on disk).
|
||||
`.trip-publish-toggle`; an anon (cleared storageState) load does not, and an
|
||||
unpublished fixture trip is absent for anon.
|
||||
2. **TP2 — unpublish hides it (caching).** Owner toggles a published fixture trip
|
||||
off → **reload** `/trips` as anon → the trip is absent; owner reload → Draft
|
||||
badge present. This is the page-tree-index assertion (mirrors DEL4).
|
||||
3. **TP3 — republish restores it.** Toggle back on → anon reload sees it again.
|
||||
off → **reload** `/trips` as anon → the trip is absent; owner reload of the
|
||||
`/trips` listing → Draft badge present (asserted on the listing, since the detail
|
||||
page 404s for the owner too). This is the page-tree-index assertion (mirrors DEL4).
|
||||
3. **TP3 — republish restores it (from the listing).** As owner on `/trips`, toggle
|
||||
a Draft fixture trip back on → anon reload sees it again. Republish is asserted on
|
||||
the listing surface, not the detail page (which 404s while unpublished).
|
||||
4. **TP4 — active-trip confirm.** Unpublishing the active trip prompts a confirm;
|
||||
dismissing leaves it published.
|
||||
5. **TP5 — authz.** `POST /api/v1/trip/<slug>/publish` as anon → 401; as a
|
||||
non-owner authenticated user → 403; frontmatter unchanged on disk.
|
||||
6. **TP6 — active trip unpublished → home falls back.** With the fixture trip set as
|
||||
`site.active_trip` and `travelling: true`, unpublish it → reload `/` → home renders
|
||||
its between-trips / pre-departure state, not the draft trip's active-trip view.
|
||||
(Needs the `active_trip` override on the fixture; mirrors the home-suite setup.)
|
||||
|
||||
## Out of scope
|
||||
|
||||
@@ -144,3 +212,6 @@ the post specs. Use a throwaway fixture trip folder (create/cleanup on disk).
|
||||
- Scheduling / publish dates.
|
||||
- Cascading child publish state.
|
||||
- Reordering trips by publish state (order stays by date desc).
|
||||
- A publish/unpublish write control on the trip detail page. Management is
|
||||
listing-only by design (an unpublished trip's detail page 404s, so a detail-page
|
||||
toggle could only strand the owner — see Surface 2).
|
||||
|
||||
Reference in New Issue
Block a user