From ee8437a9fa357206d51745277dfa1de3aebfc7c8 Mon Sep 17 00:00:00 2001 From: turegjorup Date: Wed, 27 May 2026 17:44:45 +0200 Subject: [PATCH] chore: add Claude Code project automation (hooks, skills, subagents) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Project-shared Claude Code configuration to reduce repeat instruction overhead, catch the project's most common failure modes earlier, and codify the patterns documented in ADR 010 and ADR 003. - `.claude/settings.json` — hooks (php-cs-fixer / phpstan / prettier / markdownlint / env coverage on edit; lint:container on stop; API-spec drift + template-pair Stop hooks; PreToolUse guards blocking edits to generated and locked files and `$this->addSql()` in migrations). - `.claude/skills/` — guided workflows for the project's high-friction multi-file changes (add-api-resource, add-migration, add-slide-template, add-feed-type). User-invocable only. - `.claude/agents/` — focused reviewers (api-resource-reviewer, migration-portability-reviewer, slide-template-reviewer, rtk-cache-invalidation-reviewer, feed-type-reviewer). - `.mcp.json` + settings — auto-enables the context7 MCP (live Symfony / Doctrine / API Platform docs), the php-lsp plugin (Intelephense), and the playwright plugin (browser MCP). - `CLAUDE.md` — project conventions for future Claude Code sessions (Docker-only dev tooling, the /v2/ API is versioned and BC-frozen, Schema-tool-only migrations, multi-tenancy, prerequisites). - `scripts/claude-hook-check-{api-spec-drift,template-pairs}.sh` — Stop-hook helpers. - `.gitignore` — `anthropic/claude-code` block tracks `.claude/{settings.json,agents,commands,skills}` while keeping `settings.local.json` and other local state ignored. `.playwright-mcp/` added to the Playwright block. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/agents/api-resource-reviewer.md | 87 ++++++ .claude/agents/feed-type-reviewer.md | 121 ++++++++ .../agents/migration-portability-reviewer.md | 63 ++++ .../agents/rtk-cache-invalidation-reviewer.md | 66 ++++ .claude/agents/slide-template-reviewer.md | 88 ++++++ .claude/settings.json | 282 ++++++++++++++++++ .claude/skills/add-api-resource/SKILL.md | 105 +++++++ .claude/skills/add-feed-type/SKILL.md | 279 +++++++++++++++++ .claude/skills/add-migration/SKILL.md | 85 ++++++ .claude/skills/add-slide-template/SKILL.md | 161 ++++++++++ .gitignore | 9 +- .mcp.json | 8 + CHANGELOG.md | 3 + CLAUDE.md | 162 ++++++++++ scripts/claude-hook-check-api-spec-drift.sh | 35 +++ scripts/claude-hook-check-template-pairs.sh | 54 ++++ 16 files changed, 1607 insertions(+), 1 deletion(-) create mode 100644 .claude/agents/api-resource-reviewer.md create mode 100644 .claude/agents/feed-type-reviewer.md create mode 100644 .claude/agents/migration-portability-reviewer.md create mode 100644 .claude/agents/rtk-cache-invalidation-reviewer.md create mode 100644 .claude/agents/slide-template-reviewer.md create mode 100644 .claude/settings.json create mode 100644 .claude/skills/add-api-resource/SKILL.md create mode 100644 .claude/skills/add-feed-type/SKILL.md create mode 100644 .claude/skills/add-migration/SKILL.md create mode 100644 .claude/skills/add-slide-template/SKILL.md create mode 100644 .mcp.json create mode 100644 CLAUDE.md create mode 100755 scripts/claude-hook-check-api-spec-drift.sh create mode 100755 scripts/claude-hook-check-template-pairs.sh diff --git a/.claude/agents/api-resource-reviewer.md b/.claude/agents/api-resource-reviewer.md new file mode 100644 index 00000000..d39729c3 --- /dev/null +++ b/.claude/agents/api-resource-reviewer.md @@ -0,0 +1,87 @@ +--- +name: api-resource-reviewer +description: Use this agent to review a new or modified API Platform resource in display-api-service. Cross-checks entity, migration, DTO operations, Provider/Processor wiring, voter coverage, OpenAPI regeneration, and RTK Query enhancement against the project's conventions. Invoke after creating/changing anything under src/Dto/, src/State/, src/Security/Voter/, src/Entity/Tenant/, or migrations/. Returns a structured punch list of what's missing. +tools: Glob, Grep, LS, Read, NotebookRead, Bash +model: sonnet +--- + +You are reviewing an API Platform resource change in display-api-service (Symfony 6.4 + API Platform 3.x + Doctrine + MariaDB). Your job is to confirm the full resource bundle is consistent before the PR opens. + +The project splits responsibilities deliberately: + +- `src/Entity/Tenant/` — Doctrine persistence; extends `AbstractTenantScopedEntity`. +- `src/Dto/` — external API shape; carries `#[ApiResource]` and operation attributes. +- `src/State/` — Provider (read) + Processor (write) translating DTO ↔ Entity. +- `src/Security/Voter/` — per-resource permission checks. +- `migrations/` — Doctrine Schema tool API only (ADR 010). +- `public/api-spec-v2.{yaml,json}` — generated from API Platform attributes. +- `assets/shared/redux/{generated-api.ts,enhanced-api.ts}` — RTK Query client. + +## What to check + +For each resource touched in the diff, verify: + +0. **Backwards compatibility (highest priority)** + - The `/v2/` API is versioned — **no breaking changes allowed** (ADR 003). The `apispec` CI job runs `oasdiff` and fails the build on any BC-classified diff. + - For **modified** existing resources, flag any of these as **blockers** unless an exception is justified inline: + - Removed or renamed field on a DTO read response. + - Type change on an existing field (e.g. string → object). + - New required write field, or a previously optional field becoming required. + - Removed endpoint or removed HTTP method on an existing endpoint. + - Tighter response (e.g. nullable field becoming non-nullable in the output). + - For **new** resources, BC is irrelevant — just verify nothing in the diff *also* mutates an existing one's shape as a side effect. + - When in doubt, run: `git diff origin/$(git merge-base origin/HEAD HEAD)..HEAD -- public/api-spec-v2.yaml | head -100` to see what oasdiff will see. + +1. **Entity ↔ Migration parity** + - Every new/changed entity field has a corresponding column in a migration. + - Migration uses Schema tool API (`$schema->createTable`, `$table->addColumn(Types::*, ...)`). **Flag any `$this->addSql(...)` — ADR 010 violation.** + - Type references use `Doctrine\DBAL\Types\Types::*` constants, `UlidType::NAME`, or `RRuleType::RRULE` (not free-form strings). + - `down()` mirrors `up()` symmetrically. + - Tenant-scoped tables include `tenant_id` FK + index. + +2. **DTO** + - `#[ApiResource]` is on the DTO, **not** the entity. + - Operations are declared explicitly (`GetCollection`, `Get`, `Post`, `Put`, `Delete` as appropriate). + - Each operation has a `security:` expression — at minimum `is_granted("ROLE_USER")`, plus voter-style checks for object-level access. + - Each operation references its Provider/Processor via `provider:` / `processor:`. + +3. **State** + - Provider exists in `src/State/` and extends `AbstractProvider`. + - Processor exists in `src/State/` and extends `AbstractProcessor`. + - Both are wired into the DTO operation attributes. + - Tenant scoping is preserved (the `TenantExtension` filter handles default scoping; custom queries must respect it). + +4. **Voter** + - A voter class exists in `src/Security/Voter/` for the resource (or an existing generic voter clearly covers it). + - Flag if the resource has only role-level checks and no object-level voter — that's usually a tenant-leak risk. + +5. **OpenAPI regeneration** + - Run: `git diff --name-only HEAD -- public/api-spec-v2.yaml public/api-spec-v2.json` + - If `src/Dto/`, `src/State/`, or `config/api_platform/` changed but the spec files didn't, flag it. Resolution: `task generate:api-spec`. + +6. **RTK Query enhancement** + - If new operations or new tag types were added, `assets/shared/redux/enhanced-api.ts` should mention the new resource — check for `providesTags` / `invalidatesTags` references. + - Flag missing tag invalidation as a probable Admin cache-staleness bug. + +7. **Tests** + - Look for a matching `tests/Api/Test.php` or equivalent. If absent, flag it but don't fail on it. + +## How to investigate + +- Start with `git diff --name-only HEAD` and `git diff --stat` to see scope. +- Read the DTO, then the Provider/Processor, then the entity, then the migration, **in that order**. Mismatches between these are the most common failure mode. +- For voter coverage, `grep -r "supports.*" src/Security/Voter/`. +- For RTK enhancement, `grep "" assets/shared/redux/enhanced-api.ts`. + +## Output format + +Return a punch list — each finding has: + +- **Severity**: blocker / risk / nit +- **Location**: file:line if applicable +- **Finding**: one sentence +- **Fix**: one sentence with the concrete action + +Group findings by category (Entity/Migration, DTO, State, Voter, Spec, RTK, Tests). End with a short verdict: "Ready" / "Ready with nits" / "Not ready" + the top 1–3 blockers if not ready. + +Keep it concrete. Don't restate the convention — point at the specific file:line where it's violated. diff --git a/.claude/agents/feed-type-reviewer.md b/.claude/agents/feed-type-reviewer.md new file mode 100644 index 00000000..0377358c --- /dev/null +++ b/.claude/agents/feed-type-reviewer.md @@ -0,0 +1,121 @@ +--- +name: feed-type-reviewer +description: Use this agent after creating or modifying a class in src/Feed/ that implements App\Feed\FeedTypeInterface. Verifies the FeedType contract — all 7 interface methods present, SUPPORTED_FEED_TYPE constant aligns with FeedOutputModels, getData() rethrows (not swallows) for the error-cache contract, getConfigOptions() does swallow, per-tenant data stays in secrets rather than env vars, the class is registered in FeedServiceTest, env vars exist in both .env and .env.test, and a doc was added. +tools: Glob, Grep, LS, Read, NotebookRead, Bash +model: sonnet +--- + +You are reviewing a new or modified FeedType in display-api-service. + +A FeedType lives in `src/Feed/`, implements `App\Feed\FeedTypeInterface`, and gets auto-tagged into `FeedService` via `_instanceof` in `config/services.yaml`. The most common bugs are subtle — silent caching of empty results, mismatched output shapes, missing test registration. Your job is to catch those before the PR opens. + +## What to check + +### 1. Interface contract + +- All **7 methods** are implemented: + - `getData(Feed $feed): array` + - `getAdminFormOptions(FeedSource $feedSource): array` + - `getConfigOptions(Request $request, FeedSource $feedSource, string $name): ?array` + - `getRequiredSecrets(): array` + - `getRequiredConfiguration(): array` + - `getSupportedFeedOutputType(): string` + - `getSchema(): array` +- `SUPPORTED_FEED_TYPE` constant (convention, not required) uses a `FeedOutputModels::*` value, e.g. `final public const string SUPPORTED_FEED_TYPE = FeedOutputModels::POSTER_OUTPUT;`. Flag if it's a free-form string. +- `getSupportedFeedOutputType()` returns the constant, not a hard-coded string. + +### 2. `getData()` error handling — the most important check + +Failure modes in this method silently break feeds for hours. The rule is **rethrow, don't swallow**: `FeedService::getData()` wraps the call and caches `[]` for **30 seconds** on exception (`ERROR_CACHE_TTL_SECONDS`). Catching and returning `[]` yourself caches empty at the **full feed TTL** (often hours), so the feed stays broken long after the upstream recovers. + +- **Blocker**: any `catch` block in `getData()` that returns `[]` or `null` instead of rethrowing. Allowed pattern is log-then-rethrow. +- **Risk**: catching `\Throwable` without logging at all — masks problems entirely. +- **Risk**: no validation of `getRequiredSecrets`/`getRequiredConfiguration` keys before use — should throw `\RuntimeException` with class-prefixed message if missing. +- **Nit**: per-call caching wrapped around the whole `getData()` body — `FeedService` already caches per-feed, this duplicates state. + +### 3. `getConfigOptions()` error handling — inverse of `getData()` + +This *should* swallow exceptions and return `null` or `[]`. The admin form is interactive; an editor is often mid-typing credentials, so failures here must degrade gracefully. + +- **Bug**: `getConfigOptions()` propagates exceptions (the admin form will explode). +- **Bug**: no log statement in the catch — silent failures are hard to diagnose. + +### 4. Output shape matches the declared model + +Cross-reference the array returned by `getData()` against the docblock for the chosen model in `src/Feed/FeedOutputModels.php`: + +| Model | Required keys (abridged) | +|---|---| +| `RSS_OUTPUT` | `title`, `entries[].title`, `entries[].lastModified`, `entries[].content` | +| `CALENDAR_OUTPUT` | `id`, `title`, `startTime`, `endTime`, `resourceTitle`, `resourceId` (timestamps, not ISO strings) | +| `INSTAGRAM_OUTPUT` | `textMarkup`, `mediaUrl`, `videoUrl`, `username`, `createdTime` | +| `POSTER_OUTPUT` | Reference: `src/Feed/EventDatabaseApiV2FeedType` | +| `BRND_BOOKING_OUTPUT` | `activity`, `area`, `bookingBy`, `bookingcode`, `startTime`, `endTime` | + +Flag missing required keys; flag type mismatches (e.g. ISO string instead of Unix timestamp on `CALENDAR_OUTPUT`). + +### 5. Per-tenant vs system-wide configuration + +The rule: anything that differs per-tenant lives in `FeedSource.secrets`; only system-wide wiring (shared endpoints, global cache tuning) belongs in env vars. + +- **Blocker**: tokens, tenant ids, or account keys injected from env vars (means the install can only ever talk to one tenant of the upstream). +- **Acceptable**: shared base URLs, cache TTLs as env vars. + +### 6. Service registration + +- **Don't manually tag** the service with `app.feed.feed_type` — `_instanceof` in `config/services.yaml` handles it. Flag any manual tag as redundant. +- If env-bound scalars are injected, an explicit service definition under `App\Feed\MyFeedType:` in `config/services.yaml` is required for the `arguments:` map. Flag if scalars are read via `getenv()` or `$_ENV[...]` instead. + +### 7. Test registration + +Run `grep "MyFeedType" tests/Service/FeedServiceTest.php`. The class **must** appear inside `testGetFeedTypes()` as `$this->assertTrue(in_array(MyFeedType::class, $feedTypes));`. Missing this is the most common silent regression — the test continues to pass when the service tag breaks tomorrow, and only ops notices when feeds stop working. + +- **Blocker**: new class not added to `FeedServiceTest::testGetFeedTypes()`. + +### 8. Env var coverage + +- Every env var used by the FeedType (`%env(...)%` in `config/services.yaml`) must appear in `.env`. The `scripts/check-env-coverage.sh` PostToolUse hook enforces this — flag any miss. +- Same vars should also appear in `.env.test`, otherwise the test kernel fails to boot. The env-coverage script only checks `.env` — `.env.test` is your responsibility. + +### 9. Documentation + changelog + +- A doc under `docs/feed/.md` covering: required env vars, required secrets shape, required per-feed config keys, sample data shape. Mostly informational; missing = flag as nit unless the FeedType has secrets that aren't self-explanatory. +- CHANGELOG entry under unreleased referencing the PR. Nit if missing. + +### 10. Secret leakage + +`grep -nE "logger->(error|warning|info|debug)" src/Feed/.php` and inspect each call. Flag if `secrets`, `token`, `apikey`, `password`, or any value sourced from `$feedSource->getSecrets()` is interpolated into a log message body. Logging the **fact** of an auth failure is fine; logging the **value** is a bug. + +## How to investigate + +```shell +# Files in scope +git diff --name-only HEAD -- src/Feed/ tests/Feed/ tests/Service/FeedServiceTest.php config/services.yaml .env .env.test docs/feed/ CHANGELOG.md + +# Method completeness +grep -nE "public function (getData|getAdminFormOptions|getConfigOptions|getRequiredSecrets|getRequiredConfiguration|getSupportedFeedOutputType|getSchema)" src/Feed/.php + +# Error-handling smell (the big one) +sed -n '/function getData/,/^ }/p' src/Feed/.php | grep -nE 'catch|return *\[\]|throw' + +# Test registration +grep -n "::class" tests/Service/FeedServiceTest.php + +# Env var coverage +grep -E "MY_FEED_TYPE|%env" config/services.yaml +grep -E "MY_FEED_TYPE" .env .env.test +``` + +## Output format + +Group findings by severity: + +1. **Blockers** — feed will silently break in production (swallowed exceptions in `getData()`, missing `FeedServiceTest` registration, per-tenant data in env vars, secret leakage in logs). +2. **Bugs** — works locally but wrong in production (output shape mismatch, missing key validation, exception propagation from `getConfigOptions()`). +3. **Nits** — style/maintenance (missing `SUPPORTED_FEED_TYPE` constant, missing doc, missing CHANGELOG entry). + +Each finding: `file:line`, one-sentence finding, one-sentence fix. + +End with: "FeedType ready" / "FeedType has bugs — see findings" / "FeedType will silently break — fix blockers first". Name the FeedType class in the verdict. + +Be specific. Don't say "exception handling is wrong" — quote the line. diff --git a/.claude/agents/migration-portability-reviewer.md b/.claude/agents/migration-portability-reviewer.md new file mode 100644 index 00000000..d32cb983 --- /dev/null +++ b/.claude/agents/migration-portability-reviewer.md @@ -0,0 +1,63 @@ +--- +name: migration-portability-reviewer +description: Use this agent to review new/changed Doctrine migrations for the lower-yield MariaDB-vs-Postgres portability risks not already caught by the project's hooks, PHPStan rule, or Postgres CI gate. Invoke after editing anything under migrations/ or after changing schema-shaping entity attributes. Returns concrete portability findings (free-form type strings, JSON ambiguity, default-value coercion, FULLTEXT/spatial indexes, custom enums, down() asymmetry). +tools: Glob, Grep, LS, Read, NotebookRead, Bash +model: sonnet +--- + +You are reviewing Doctrine migration code in display-api-service for portability between MariaDB (production) and Postgres (the CI portability gate from ADR 010). + +## What's already covered elsewhere — do not duplicate + +- **`$this->addSql(...)` in migrations** — blocked three ways: PreToolUse hook rejects the edit, `App\PhpStan\NoAddSqlInMigrationRule` flags it, Postgres CI fails the build. Don't waste output flagging it; assume it's caught upstream. +- **Type-checking the migration runs on both engines** — Postgres CI job `Validate Schema (postgres:16, experimental)` in `.github/workflows/doctrine.yaml` does this. Your job is the *fragile but currently-green* class of risks, not "does it run." + +You are also **not** reviewing runtime SQL — `RelationsChecksumCalculator` and `MultiTenantRepositoryTrait` are knowingly MariaDB-specific and out of scope. + +## What's in scope (the gap between hooks and CI) + +1. **Type references** must be one of: + - `Doctrine\DBAL\Types\Types::*` constants. + - `Symfony\Bridge\Doctrine\Types\UlidType::NAME`. + - `App\DBAL\Types\RRuleType::RRULE`. + - Free-form strings (`'string'`, `'datetime_immutable'`, …) — flag. They pass CI today but break silently when a Doctrine type is renamed. + +2. **JSON columns** — `Types::JSON` emits `LONGTEXT` on MariaDB, `JSON` on Postgres. The DDL is portable, but **application code** that assumes one shape over the other isn't. Flag if the column is read back in a way that assumes MariaDB's string semantics (e.g. `JSON_CONTAINS` raw SQL in a listener). + +3. **Default values & timestamps** — `Types::DATETIME_IMMUTABLE` with a PHP-side default (constructor / lifecycle callback) is portable. `default CURRENT_TIMESTAMP` at the DDL level emits different SQL on the two engines — flag and propose moving the default to the entity. + +4. **Reserved-word identifiers** — `order`, `user`, `group`, `table`. `user` is already quoted by Doctrine and unavoidable; flag any *new* uses and propose a rename. + +5. **Indexes & constraints** + - FULLTEXT and SPATIAL indexes are MariaDB-only — flag if present. + - FK cycles work on MariaDB but are stricter on Postgres about ordering during drops. Flag any new FK that participates in a cycle with an existing one. + +6. **`down()` symmetry** — `down()` should reverse `up()` in reverse order. Asymmetric `down()` methods are a portability *and* a rollback risk — flag. + +7. **Custom enums** — Postgres has native enums; MariaDB simulates them. Prefer `Types::STRING` + application validation. Flag uses of `Types::SIMPLE_ARRAY` (legacy) or any custom DBAL enum type. + +## How to investigate + +```shell +# Files in scope +git diff --name-only HEAD -- migrations/ src/Entity/ + +# Free-form type strings (rule 1) +git diff HEAD -- migrations/ | grep -E "addColumn\([^,]+, *'[a-z_]+'" + +# FULLTEXT / SPATIAL indexes (rule 5) +git diff HEAD -- migrations/ | grep -iE 'fulltext|spatial' +``` + +Read the entity diff alongside the migration — most portability bugs show up as a default in the entity that should have moved out of the DDL, or a JSON column that the entity reads as a string. + +## Output format + +Return a punch list grouped by: + +1. **Portability risks** (currently works on both engines but fragile — e.g. relying on MariaDB-specific default coercion, JSON shape assumption). +2. **Style nits** (free-form type strings, missing `down()` symmetry). + +Each finding: `file:line`, one-sentence finding, one-sentence fix. + +End with a verdict: "Portable" / "Portable with nits" / "Risky — see findings". Keep it short. If there are no findings beyond what hooks/PHPStan/CI already catch, say so in one line. diff --git a/.claude/agents/rtk-cache-invalidation-reviewer.md b/.claude/agents/rtk-cache-invalidation-reviewer.md new file mode 100644 index 00000000..1c801c3d --- /dev/null +++ b/.claude/agents/rtk-cache-invalidation-reviewer.md @@ -0,0 +1,66 @@ +--- +name: rtk-cache-invalidation-reviewer +description: Use this agent after task generate:redux-toolkit-api (or any change to assets/shared/redux/generated-api.ts) to verify the matching enhancements in assets/shared/redux/enhanced-api.ts. Catches the most common Admin staleness bug: a new endpoint lands in the generated client but has no providesTags/invalidatesTags entry, so successful writes don't refresh the UI. +tools: Glob, Grep, LS, Read, NotebookRead, Bash +model: sonnet +--- + +You are reviewing the RTK Query layer in display-api-service for cache invalidation coverage. + +The split: + +- `assets/shared/redux/generated-api.ts` — **generated** by `task generate:redux-toolkit-api` from `public/api-spec-v2.json`. Never hand-edited (a PreToolUse hook blocks edits). +- `assets/shared/redux/enhanced-api.ts` — hand-maintained layer that calls `.enhanceEndpoints({ addTagTypes, endpoints: { ... } })` to attach `providesTags` (queries) and `invalidatesTags` (mutations) so the Admin cache invalidates correctly when data changes. +- `assets/shared/redux/openapi-config.js` — codegen config; rarely changes. + +The failure mode you are catching: a new endpoint lands in `generated-api.ts` (because someone added a DTO/operation and ran codegen) but `enhanced-api.ts` has no matching entry. The Admin then shows stale data after successful mutations until a manual reload. + +## What to check + +1. **List endpoints currently in the generated client** + - `grep -oE "(get|post|put|patch|delete)[A-Z][A-Za-z0-9]+:" assets/shared/redux/generated-api.ts | sort -u` (or read the file directly — it's typed and small enough). + - Each line is an endpoint name like `getV2Slides` or `postV2Slides`. + +2. **List endpoints currently enhanced** + - `grep -oE "[a-z][A-Za-z0-9]+: *\{" assets/shared/redux/enhanced-api.ts` (inside `endpoints: { ... }`). + - Also list the `addTagTypes` set at the top of `enhanced-api.ts`. + +3. **Compute the gap** + - Endpoints in (1) **not** in (2): these are unenhanced. For each, decide: + - **Read-side** (`getV2*`): should have `providesTags: [...]` so mutations can invalidate it. Missing = the resource never refreshes from cache. + - **Write-side** (`postV2*`, `putV2*`, `patchV2*`, `deleteV2*`): should have `invalidatesTags: [...]` listing the tag types its result affects. Missing = the Admin shows stale data after a successful mutation. + +4. **Tag-type vocabulary** + - Cross-reference each `providesTags` / `invalidatesTags` value against the `addTagTypes` array. Tag references that don't match a declared tag are silently no-ops. + +5. **ID-scoped invalidation** + - Mutations on a single resource should invalidate `{ type: 'Foo', id }` not just `'Foo'` when feasible, to keep list caches stable. Flag broad invalidations on single-resource mutations as a perf nit, not a correctness bug. + +## How to investigate + +```shell +git diff --name-only HEAD assets/shared/redux/ + +# What endpoints exist +grep -nE "(get|post|put|patch|delete)V2[A-Z][A-Za-z0-9]*:" assets/shared/redux/generated-api.ts | head -50 + +# What enhancements exist +grep -nE "^ +[a-z][A-Za-z0-9]+: *\{" assets/shared/redux/enhanced-api.ts + +# Tag types in scope +grep -nA1 "addTagTypes" assets/shared/redux/enhanced-api.ts +``` + +If the diff is small, read both files end-to-end. They are typed and well under 1000 lines combined. + +## Output format + +Three sections: + +1. **Missing invalidations** (correctness bugs — Admin UI will stay stale): mutation name, tags it should invalidate, one-line reason. +2. **Missing providesTags** (queries that no mutation can refresh): query name, tag it should provide. +3. **Style nits**: broad-tag invalidations where ID-scoped would be tighter; tag references not in `addTagTypes`. + +End with: "RTK cache complete" / "RTK cache has gaps — see findings". If complete, one line and done. + +Be specific. Don't say "the new endpoints aren't enhanced" — name them. diff --git a/.claude/agents/slide-template-reviewer.md b/.claude/agents/slide-template-reviewer.md new file mode 100644 index 00000000..39ec23f0 --- /dev/null +++ b/.claude/agents/slide-template-reviewer.md @@ -0,0 +1,88 @@ +--- +name: slide-template-reviewer +description: Use this agent after creating or modifying anything in assets/shared/templates/*.{jsx,json}. Verifies the slide template contract — id()/config()/renderSlide() exported, .jsx/.json paired with matching ULIDs, slideDone() signalled so the slide ever advances, and adminForm schema coherent with the keys the renderer reads from slide.content. +tools: Glob, Grep, LS, Read, NotebookRead, Bash +model: sonnet +--- + +You are reviewing a slide template in display-api-service (`assets/shared/templates/`). Each template is a two-file unit — `.jsx` and `.json` — that the Screen Client loads to render content on physical displays. Failures here are subtle: the admin form looks fine, the slide enters the playlist, then it never advances. + +## The contract + +Every template `.jsx` must: + +1. Import its sibling `.json` and re-export `{ id, config, renderSlide }` as the default export: + + ```jsx + import myTemplateConfig from "./my-template.json"; + + function id() { return myTemplateConfig.id; } + function config() { return myTemplateConfig; } + function renderSlide(slide, run, slideDone) { + return ; + } + + export default { id, config, renderSlide }; + ``` + +2. **Call `slideDone()`** at some point — either via `BaseSlideExecution` (which calls it after `slide.content.duration` seconds, see `assets/shared/slide-utils/base-slide-execution.js`) or directly from a `useEffect` / event handler. **A template that never calls slideDone() locks the playlist.** + +3. Read content from `slide.content` using keys that exist in the `.json`'s `adminForm` `name:` values. Mismatched keys = the admin form writes data the renderer never reads. + +Every template `.json` must: + +1. Have a stable **ULID** in `id:` (26 characters, Crockford base32) — never reuse another template's ULID, never change a published template's ULID. +2. Have a `title:` (Danish — see existing templates for tone), and an `adminForm:` array of input schemas. +3. Each `adminForm` entry needs a unique `key:` and a `name:` that matches what the renderer expects from `slide.content`. + +## What to check + +1. **Pair existence** — both `.jsx` and `.json` exist for any new/changed base name. If only one is present, that's a blocker. (The Stop hook `check-template-pairs.sh` also warns about this; you're verifying for new templates specifically.) + +2. **ULID validity** — `id` is 26 chars, alphanumeric (no `I/L/O/U`). Cross-check it's not duplicated against any other template: + + ```shell + grep -RoE '"id": *"[0-9A-HJ-KM-NP-TV-Z]{26}"' assets/shared/templates/ | sort | uniq -c | sort -rn | head + ``` + +3. **Re-export shape** — `.jsx` exports `default { id, config, renderSlide }`. Missing any of the three = the template won't register. + +4. **slideDone signalling** — the rendered component either uses `BaseSlideExecution` (preferred, lives in `assets/shared/slide-utils/base-slide-execution.js`) or calls `slideDone()` directly. Search for `slideDone` in the `.jsx`; if it's only in the prop signature and never called, that's a blocker. + +5. **adminForm / renderer key alignment** — for each `name:` in `adminForm`, confirm it's read by the renderer (`slide.content.` or destructured from `content`). For each key the renderer reads from `slide.content`, confirm there's a matching `adminForm` entry — otherwise the admin can't set it. Mismatches both ways are bugs. + +6. **`renderSlide` signature** — `(slide, run, slideDone)`. Other signatures are wrong. + +7. **`adminForm` input types** — must be one of the supported types (see `assets/admin/components/slide/content/` for the dispatch). Common: `header`, `header-h3`, `input`, `select`, `checkbox`, `rich-text-input`, `image`, `video`, `file`, `duration`, `contacts`, `feed`, `table`, `textarea`. Anything else won't render. + +8. **Feed integration** — if `adminForm` has an `input: "feed"` entry, the renderer needs to handle `slide.feed` and `slide.feedData`. Flag if the form requests a feed but the renderer doesn't consume it. + +## How to investigate + +```shell +# Files in scope +git diff --name-only HEAD -- assets/shared/templates/ + +# Re-export shape +grep -n "export default" + +# slideDone signalling +grep -n "slideDone" + +# ULID dupes +grep -RhoE '"id": *"[^"]+"' assets/shared/templates/*.json | sort | uniq -c | awk '$1>1' +``` + +Compare side-by-side: read the `.json` adminForm, then grep the `.jsx` for each `name:` to confirm consumption. + +## Output format + +Group by severity: + +1. **Blockers** — template won't register or will lock the playlist (no `slideDone`, missing re-export field, missing pair, duplicate ULID). +2. **Bugs** — works but reads/writes wrong data (adminForm/renderer key drift, wrong renderSlide signature). +3. **Nits** — minor (unused adminForm field, unsupported input type with a sensible fallback). + +Each finding: `file:line` (or `file` if not line-specific), one-sentence finding, one-sentence fix. + +End with: "Template ready" / "Template has bugs — see findings" / "Template will lock the playlist — fix blockers". Be specific about which template name the verdict applies to. diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..35fe91d9 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,282 @@ +{ + "$schema": "https://json.schemastore.org/claude-code-settings.json", + "enabledPlugins": { + "php-lsp@claude-plugins-official": true, + "playwright@claude-plugins-official": true + }, + "enabledMcpjsonServers": [ + "context7" + ], + "permissions": { + "allow": [ + "Bash(docker compose:*)", + "WebFetch(domain:api-platform.com)", + "Bash(grep:*)", + "Bash(./scripts/test *)", + "Bash(git add:*)", + "Bash(git fetch*)", + "Bash(git pull*)", + "Bash(git ls-remote*)", + "Bash(git remote -v*)", + "Bash(git remote show*)", + "Bash(git remote get-url*)", + "Bash(git clone*)", + "Bash(git ls-tree *)", + "Bash(gh --version*)", + "Bash(gh version*)", + "Bash(gh help*)", + "Bash(gh status*)", + "Bash(gh auth status*)", + "Bash(gh auth token*)", + "Bash(gh pr list*)", + "Bash(gh pr view*)", + "Bash(gh pr diff*)", + "Bash(gh pr checks*)", + "Bash(gh pr status*)", + "Bash(gh issue list*)", + "Bash(gh issue view*)", + "Bash(gh issue status*)", + "Bash(gh repo list*)", + "Bash(gh repo view*)", + "Bash(gh repo clone*)", + "Bash(gh repo deploy-key list*)", + "Bash(gh release list*)", + "Bash(gh release view*)", + "Bash(gh release download*)", + "Bash(gh run list*)", + "Bash(gh run view*)", + "Bash(gh run watch*)", + "Bash(gh run download*)", + "Bash(gh workflow list*)", + "Bash(gh workflow view*)", + "Bash(gh label list*)", + "Bash(gh secret list*)", + "Bash(gh variable list*)", + "Bash(gh variable get*)", + "Bash(gh gist list*)", + "Bash(gh gist view*)", + "Bash(gh ssh-key list*)", + "Bash(gh gpg-key list*)", + "Bash(gh project list*)", + "Bash(gh project view*)", + "Bash(gh project item-list*)", + "Bash(gh project field-list*)", + "Bash(gh codespace list*)", + "Bash(gh codespace view*)", + "Bash(gh codespace logs*)", + "Bash(gh codespace ports*)", + "Bash(gh extension list*)", + "Bash(gh extension search*)", + "Bash(gh search*)", + "Bash(gh browse*)", + "Bash(task coding-standards:assets:check)", + "Bash(task test:unit)", + "Bash(task coding-standards:assets:apply)", + "Bash(task coding-standards:php:apply)", + "Bash(task coding-standards:php:check)", + "Bash(task code-analysis *)", + "Bash(task test:api*)" + ], + "deny": [ + "Bash(git push --delete*)", + "Bash(gh auth login*)", + "Bash(gh auth logout*)", + "Bash(gh auth refresh*)", + "Bash(gh auth setup-git*)", + "Bash(gh pr close*)", + "Bash(gh pr reopen*)", + "Bash(gh pr merge*)", + "Bash(gh pr ready*)", + "Bash(gh pr review*)", + "Bash(gh pr lock*)", + "Bash(gh pr unlock*)", + "Bash(gh pr update-branch*)", + "Bash(gh issue create*)", + "Bash(gh issue edit*)", + "Bash(gh issue close*)", + "Bash(gh issue reopen*)", + "Bash(gh issue comment*)", + "Bash(gh issue delete*)", + "Bash(gh issue lock*)", + "Bash(gh issue unlock*)", + "Bash(gh issue pin*)", + "Bash(gh issue unpin*)", + "Bash(gh issue transfer*)", + "Bash(gh issue develop*)", + "Bash(gh repo create*)", + "Bash(gh repo delete*)", + "Bash(gh repo edit*)", + "Bash(gh repo rename*)", + "Bash(gh repo archive*)", + "Bash(gh repo unarchive*)", + "Bash(gh repo fork*)", + "Bash(gh repo sync*)", + "Bash(gh repo deploy-key add*)", + "Bash(gh repo deploy-key delete*)", + "Bash(gh release create*)", + "Bash(gh release delete*)", + "Bash(gh release edit*)", + "Bash(gh release upload*)", + "Bash(gh release delete-asset*)", + "Bash(gh secret set*)", + "Bash(gh secret delete*)", + "Bash(gh variable set*)", + "Bash(gh variable delete*)", + "Bash(gh workflow disable*)", + "Bash(gh workflow enable*)", + "Bash(gh workflow run*)", + "Bash(gh run cancel*)", + "Bash(gh run delete*)", + "Bash(gh run rerun*)", + "Bash(gh label create*)", + "Bash(gh label delete*)", + "Bash(gh label edit*)", + "Bash(gh label clone*)", + "Bash(gh gist create*)", + "Bash(gh gist delete*)", + "Bash(gh gist edit*)", + "Bash(gh gist rename*)", + "Bash(gh gist clone*)", + "Bash(gh ssh-key add*)", + "Bash(gh ssh-key delete*)", + "Bash(gh gpg-key add*)", + "Bash(gh gpg-key delete*)", + "Bash(gh project create*)", + "Bash(gh project delete*)", + "Bash(gh project edit*)", + "Bash(gh project copy*)", + "Bash(gh project close*)", + "Bash(gh project field-create*)", + "Bash(gh project field-delete*)", + "Bash(gh project item-add*)", + "Bash(gh project item-archive*)", + "Bash(gh project item-create*)", + "Bash(gh project item-delete*)", + "Bash(gh project item-edit*)", + "Bash(gh project mark-template*)", + "Bash(gh project unmark-template*)", + "Bash(gh project link*)", + "Bash(gh project unlink*)", + "Bash(gh api -X POST*)", + "Bash(gh api -X PUT*)", + "Bash(gh api -X PATCH*)", + "Bash(gh api -X DELETE*)", + "Bash(gh api -XPOST*)", + "Bash(gh api -XPUT*)", + "Bash(gh api -XPATCH*)", + "Bash(gh api -XDELETE*)", + "Bash(gh api --method POST*)", + "Bash(gh api --method PUT*)", + "Bash(gh api --method PATCH*)", + "Bash(gh api --method DELETE*)", + "Bash(gh api --method=POST*)", + "Bash(gh api --method=PUT*)", + "Bash(gh api --method=PATCH*)", + "Bash(gh api --method=DELETE*)" + ], + "ask": [ + "Bash(git commit*)", + "Bash(git push:*)", + "Bash(git push*)", + "Bash(git push-*)", + "Bash(gh pr create*)", + "Bash(gh pr edit*)" + ] + }, + "hooks": { + "SessionStart": [ + { + "matcher": "startup", + "hooks": [ + { + "type": "command", + "command": "docker compose up --detach --quiet-pull 2>/dev/null || true", + "timeout": 60, + "statusMessage": "Starting Docker services..." + } + ] + } + ], + "PreToolUse": [ + { + "matcher": "Edit|Write", + "hooks": [ + { + "type": "command", + "command": "case \"$CLAUDE_FILE_PATH\" in */composer.lock|*/package-lock.json|*/.env.local|*/.env.*.local|*/public/api-spec-v2.yaml|*/public/api-spec-v2.json|*/assets/shared/redux/generated-api.ts|*/vendor/*|*/node_modules/*|*/var/cache/*) echo 'BLOCKED: edit the source — this path is generated, locked, or operator-only' >&2; exit 1 ;; esac" + }, + { + "type": "command", + "command": "case \"$CLAUDE_FILE_PATH\" in */migrations/Version*.php) PAYLOAD=\"${CLAUDE_NEW_STRING:-}${CLAUDE_CONTENT:-}\"; if printf '%s' \"$PAYLOAD\" | grep -qE '\\$this->addSql\\b'; then echo 'BLOCKED: $this->addSql() is disallowed in migrations/ (ADR 010 — use Doctrine Schema tool API: $schema->createTable(...), $table->addColumn(Types::*, ...), etc.)' >&2; exit 1; fi ;; esac" + } + ] + } + ], + "PostToolUse": [ + { + "matcher": "Write|Edit", + "hooks": [ + { + "type": "command", + "command": "case \"$CLAUDE_FILE_PATH\" in *.php) REL_PATH=\"${CLAUDE_FILE_PATH#$CLAUDE_PROJECT_DIR/}\"; docker compose exec -T phpfpm vendor/bin/php-cs-fixer fix --quiet \"$REL_PATH\" 2>/dev/null || true ;; esac", + "timeout": 30 + }, + { + "type": "command", + "command": "case \"$CLAUDE_FILE_PATH\" in *.php) REL_PATH=\"${CLAUDE_FILE_PATH#$CLAUDE_PROJECT_DIR/}\"; docker compose exec -T phpfpm vendor/bin/phpstan analyse --no-progress --error-format=raw \"$REL_PATH\" 2>/dev/null || true ;; esac", + "timeout": 30 + }, + { + "type": "command", + "command": "case \"$CLAUDE_FILE_PATH\" in *.twig) REL_PATH=\"${CLAUDE_FILE_PATH#$CLAUDE_PROJECT_DIR/}\"; docker compose exec -T phpfpm vendor/bin/twig-cs-fixer lint --fix \"$REL_PATH\" 2>/dev/null || true ;; esac", + "timeout": 15 + }, + { + "type": "command", + "command": "case \"$CLAUDE_FILE_PATH\" in */composer.json) docker compose exec -T phpfpm composer normalize --quiet 2>/dev/null || true ;; esac", + "timeout": 30 + }, + { + "type": "command", + "command": "case \"$CLAUDE_FILE_PATH\" in *.js|*.jsx|*.css|*.scss|*.yaml|*.yml) REL_PATH=\"${CLAUDE_FILE_PATH#$CLAUDE_PROJECT_DIR/}\"; docker compose run --rm -T --volume \"$CLAUDE_PROJECT_DIR:/md\" prettier \"$REL_PATH\" --write 2>/dev/null || true ;; esac", + "timeout": 30 + }, + { + "type": "command", + "command": "case \"$CLAUDE_FILE_PATH\" in *.md) REL_PATH=\"${CLAUDE_FILE_PATH#$CLAUDE_PROJECT_DIR/}\"; docker compose run --rm -T markdownlint markdownlint --fix \"$REL_PATH\" 2>/dev/null || true ;; esac", + "timeout": 30 + }, + { + "type": "command", + "command": "case \"$CLAUDE_FILE_PATH\" in */.env|*/.env.*) sh \"$CLAUDE_PROJECT_DIR/scripts/check-env-coverage.sh\" 2>&1 || true ;; esac", + "timeout": 15 + } + ] + } + ], + "Stop": [ + { + "hooks": [ + { + "type": "command", + "command": "docker compose exec -T phpfpm bin/console lint:container 2>/dev/null || true", + "timeout": 30, + "statusMessage": "Validating Symfony DI container..." + }, + { + "type": "command", + "command": "sh \"$CLAUDE_PROJECT_DIR/scripts/claude-hook-check-api-spec-drift.sh\" || true", + "timeout": 10, + "statusMessage": "Checking API spec drift..." + }, + { + "type": "command", + "command": "sh \"$CLAUDE_PROJECT_DIR/scripts/claude-hook-check-template-pairs.sh\" || true", + "timeout": 10, + "statusMessage": "Checking slide template pairs..." + } + ] + } + ] + } +} diff --git a/.claude/skills/add-api-resource/SKILL.md b/.claude/skills/add-api-resource/SKILL.md new file mode 100644 index 00000000..41e5970e --- /dev/null +++ b/.claude/skills/add-api-resource/SKILL.md @@ -0,0 +1,105 @@ +--- +name: add-api-resource +description: Add a new API Platform resource to display-api-service end-to-end — entity, migration, DTO with #[ApiResource], state Provider+Processor, voter, OpenAPI regeneration, and RTK Query enhancement. Use when the user asks to expose a new resource at /v2/..., add a new API endpoint, or wire up a new entity for the Admin/Client to consume. +disable-model-invocation: true +--- + +# Add a new API Platform resource + +## Backwards compatibility — read first + +The `/v2/` API is a **published, versioned contract** (ADR 003). The `apispec` CI workflow runs `oasdiff` +against the base branch and **fails the build on any breaking change** (renamed/removed fields, removed +endpoints, changed types, newly required parameters, tightened responses). + +- **New resources, new endpoints, new optional fields, new optional query params** — all fine, surface as a + non-breaking diff comment on the PR. +- **Modifying an existing resource's shape** — must stay backwards-compatible. Add `title` alongside `name`, + don't rename. Keep both fields in sync via the Processor (write-side) and Provider (read-side). +- **Genuine BC break** — would require bumping to `/v3`. Flag it for human review before doing the work. + +This split is why the project routes through DTOs rather than putting `#[ApiResource]` on entities — DTOs are +the API contract, entities are persistence. + +## The architecture split + +This project does **not** put `#[ApiResource]` on entities. The split is: + +- `src/Entity/Tenant/*.php` — Doctrine persistence (extends `AbstractTenantScopedEntity`). +- `src/Dto/*.php` — the **external** API shape, carries `#[ApiResource]` and operation attributes. +- `src/State/*Provider.php`, `src/State/*Processor.php` — translate DTO ↔ Entity for reads/writes. +- `src/Security/Voter/*.php` — per-resource permission checks (tenant scope + role). + +Forgetting any of these (or forgetting to regenerate the OpenAPI spec) is what breaks CI. + +## Checklist (do them in this order) + +1. **Entity** — `src/Entity/Tenant/.php` + - Extend `App\Entity\Tenant\AbstractTenantScopedEntity` (FK to `Tenant` is inherited). + - Use `Symfony\Component\Uid\Ulid` for the primary key (see existing entities for `#[ORM\Id]` + `UlidType` columns). + - Add `relationsChecksum` and `relationsModified` traits if the resource has nested collections clients need to invalidate (see `App\Entity\Tenant\Slide` for the canonical example). + +2. **Migration** — `migrations/Version.php` + - Generate via the `add-migration` skill (or `bin/console doctrine:migrations:generate`). + - **Schema tool API only** — `$schema->createTable(...)`, `$table->addColumn(Types::*, ...)`, `$table->addForeignKeyConstraint(...)`. No `$this->addSql(...)`. PHPStan rule `NoAddSqlInMigrationRule` blocks it. ADR 010. + - Type references must be `Doctrine\DBAL\Types\Types::*` constants, or `UlidType::NAME` / `RRuleType::RRULE` for the two custom types. + - `down()` must mirror `up()` symmetrically. + +3. **DTO** — `src/Dto/.php` and (if mutable) `src/Dto/Input.php` + - Attach `#[ApiResource]` here, not on the entity. + - Declare operations explicitly: `new GetCollection()`, `new Get()`, `new Post()`, `new Put()`, `new Delete()` — each pointing at the matching Provider/Processor. + - Set `security:` on each operation so the voter gets a chance (e.g. `'is_granted("ROLE_USER")'`). + - See `src/Dto/Slide.php` for a complete example with input class, security expressions, and provider/processor wiring. + +4. **State classes** — `src/State/Provider.php` (extends `AbstractProvider`) and `src/State/Processor.php` (extends `AbstractProcessor`) + - Provider: hydrate DTO from Entity via the repository; respect tenant scope (handled by `TenantExtension` filter, but verify). + - Processor: validate `Input`, persist via repository, return DTO. Use `RelationsChecksumCalculator` only via the entity listener — don't call SQL directly here. + - Register them as services (autowiring handles this if they extend the abstract base classes). + +5. **Voter** — `src/Security/Voter/Voter.php` + - Even if it just delegates to `ROLE_ADMIN` / tenant membership, **create the voter file** so the resource isn't covered by a generic catch-all. The api-resource-reviewer subagent will flag missing voters. + +6. **Regenerate OpenAPI** — from the host: + + ```shell + task generate:api-spec + ``` + + Commits `public/api-spec-v2.yaml` + `public/api-spec-v2.json`. The CI `apispec` workflow fails if these drift. + +7. **RTK Query** — if the operation set changed (new endpoint, new method on existing one): + + ```shell + task generate:redux-toolkit-api + ``` + + Then edit `assets/shared/redux/enhanced-api.ts`: + - Add the new resource to `tagTypes` if it's a new type. + - Configure `providesTags` / `invalidatesTags` on the new endpoints so the Admin cache invalidates correctly. + +## Validation + +After all 7 steps: + +```shell +task code-analysis # PHPStan, will catch addSql in migration +task test:api # PHPUnit + test-setup re-migrates fresh DB +task coding-standards:php:check +``` + +Open the resource in the Admin (`task site-open:admin`) and confirm it reads + writes; the `/docs` page should list the new operations under the right tag. + +## Common mistakes (called out so I don't repeat them) + +- Forgetting to regenerate the API spec → `apispec` CI job fails with a diff. +- Forgetting to enhance `enhanced-api.ts` → Admin shows stale data after mutations. +- Putting `#[ApiResource]` on the entity → bypasses the DTO indirection and leaks persistence shape into the API. +- Missing tenant scope in a custom repository query → cross-tenant data leak (use `MultiTenantRepositoryTrait`, but be aware its raw SQL is MariaDB-specific). +- Migration uses `$this->addSql(...)` → PHPStan rule rejects it, Postgres CI gate also rejects it. See `add-migration` skill. + +## Related + +- ADR 010: `docs/adr/010-schema-tool-migrations.md` +- ADR 002 (API-first): `docs/adr/002-api-first.md` +- ADR 003 (API versioning): `docs/adr/003-api-versioning.md` +- Subagent `api-resource-reviewer` cross-checks the whole bundle before PR. diff --git a/.claude/skills/add-feed-type/SKILL.md b/.claude/skills/add-feed-type/SKILL.md new file mode 100644 index 00000000..aff6b22c --- /dev/null +++ b/.claude/skills/add-feed-type/SKILL.md @@ -0,0 +1,279 @@ +--- +name: add-feed-type +description: Add a new FeedType to display-api-service — a PHP class implementing App\Feed\FeedTypeInterface that pulls data from an external source, normalizes it into one of the shared output models (RSS/CALENDAR/INSTAGRAM/POSTER/BRND_BOOKING), and describes its own admin form and configuration. Use when the user asks to add a new feed source, integrate an external API into the slide-feed system, or extend the renderer with a new feed shape. +disable-model-invocation: true +--- + +# Add a new FeedType + +A FeedType is a PHP class in `src/Feed/` that: + +1. Knows how to talk to one external data source. +2. **Normalizes** that data into one of the shared **output models** (`RSS_OUTPUT`, `CALENDAR_OUTPUT`, `INSTAGRAM_OUTPUT`, `POSTER_OUTPUT`, `BRND_BOOKING_OUTPUT`) that slide templates render against. +3. Describes its own admin UI (`getAdminFormOptions`, `getConfigOptions`) and config/secret requirements (`getRequiredSecrets`, `getRequiredConfiguration`, `getSchema`). + +Decoupling templates from feed implementations via the output-model contract is the whole point — see README "Feeds" + ADR 005. + +## TL;DR checklist + +- [ ] Pick the output model that fits your source (or add a new one — heavy, requires template updates). +- [ ] Create `src/Feed/MyFeedType.php` implementing `App\Feed\FeedTypeInterface`. +- [ ] Implement the 7 interface methods (see below). +- [ ] If env-bound scalars are needed, declare in `config/services.yaml` and add to `.env` + `.env.test`. +- [ ] **Don't** manually tag the service — `_instanceof` auto-tags via the interface. +- [ ] Add a unit test under `tests/Feed/MyFeedTypeTest.php`. +- [ ] **Add the class to `FeedServiceTest::testGetFeedTypes()`** — regression guard. +- [ ] Add a doc under `docs/feed/my-feed-type.md` (env vars, secrets shape, sample data). +- [ ] CHANGELOG entry under unreleased. +- [ ] Smoke-test end-to-end via the admin. + +## 1. Decide what you're building + +| Question | Why | +|---|---| +| Which **output model**? | Determines which slide templates can render the feed. Must match an existing `FeedOutputModels::*` constant unless adding a new template too. | +| What **secrets** does the source need? | Per-tenant, stored encrypted in `FeedSource.secrets` (JSON column). Set `exposeValue => true` on non-sensitive ones so the UI can display them. | +| What **per-feed config** does an editor pick? | URL, item count, taxonomy filters — lands in `Feed.configuration` (JSON column). | +| Does the admin need **dynamic dropdowns** (e.g. "list my calendars")? | If yes, implement `getConfigOptions()` and use `multiselect-from-endpoint` in `getAdminFormOptions()`. | +| What **caching** is needed? | `FeedService` already caches `getData()` per-feed. Only add lower-level caching for expensive shared lookups (see `CalendarApiFeedType::$calendarApiCache`). | + +If the output model genuinely doesn't fit anything existing, add a constant to `src/Feed/FeedOutputModels.php` with a docblock example of the data shape — but that also requires frontend template work (templates filter `FeedSource` by `supportedFeedOutputType`, so a new model is invisible to existing templates). + +## 2. Skeleton + +```php + true` makes a key readable by the frontend. + +```php +return [ + 'token' => ['type' => 'string'], + 'locations' => [ + 'type' => 'string_array', + 'options' => $this->getLocationOptions(), + 'exposeValue' => true, + ], +]; +``` + +### 3.3 `getRequiredConfiguration(): array` +Keys that must exist in `Feed.configuration` before `getData()` runs. Documentation only — enforce in `getData()`. + +### 3.4 `getSchema(): array` +JSON Schema (draft-04) validated by `FeedSourceProcessor` on POST/PUT. Start permissive. + +```php +return [ + '$schema' => 'http://json-schema.org/draft-04/schema#', + 'type' => 'object', + 'properties' => ['token' => ['type' => 'string']], + 'required' => ['token'], +]; +``` +Gotcha: PUT with empty `secrets` payload skips validation (intentional — allows editing title without re-sending secrets). + +### 3.5 `getAdminFormOptions(FeedSource $feedSource): array` +Admin form descriptor. Common inputs: + +| `input` | Use for | +|---|---| +| `input` + `type: 'url' \| 'number' \| 'text'` | Plain text/number. See `RssFeedType`. | +| `multiselect-from-endpoint` | Dropdown populated from `getConfigOptions()`. See `NotifiedFeedType`. | +| `checkbox-options` | Static checkbox group. See `CalendarApiFeedType` modifiers. | + +Dynamic-dropdown pattern: + +```php +$endpoint = $this->feedService->getFeedSourceConfigUrl($feedSource, 'feeds'); +return [[ + 'key' => 'my-feeds-selector', + 'input' => 'multiselect-from-endpoint', + 'endpoint' => $endpoint, + 'name' => 'feeds', + 'label' => 'Vælg feed', + 'formGroupClasses' => 'col-md-6 mb-3', +]]; +``` + +`name` becomes the key under `Feed.configuration`. The second arg to `getFeedSourceConfigUrl()` is the `$name` your `getConfigOptions()` dispatches on. + +### 3.6 `getConfigOptions(Request $request, FeedSource $feedSource, string $name): ?array` +Called by `FeedSourceConfigGetController` for dynamic dropdowns. Return `[{id, title, value}, …]` or `null` for unknown `$name`. + +**Swallow exceptions here** — the admin form should degrade gracefully while an editor is still typing credentials. Log + return `null`/`[]`. + +```php +try { + if ('feeds' === $name) { + $data = $this->callRemoteApi($feedSource->getSecrets()['token'] ?? ''); + return array_map(fn (array $item) => [ + 'id' => Ulid::generate(), + 'title' => $item['name'] ?? '', + 'value' => $item['id'] ?? '', + ], $data); + } +} catch (\Throwable $t) { + $this->logger->error('{code}: {message}', ['code' => $t->getCode(), 'message' => $t->getMessage()]); +} +return null; +``` + +### 3.7 `getData(Feed $feed): array` — the hot path + +Rules (these are the most common bugs): + +1. **Validate required secrets/config first** — throw `\RuntimeException` with class-prefixed message if missing. +2. **Let exceptions bubble.** `FeedService::getData()` wraps the call and caches an empty result for **30 s** on failure (`ERROR_CACHE_TTL_SECONDS`). Catching-and-returning-`[]` yourself caches empty with the **full feed TTL** (often hours) — silent breakage. +3. **Log at the catch site, then rethrow:** + ```php + try { /* … */ } + catch (\Throwable $t) { + $this->logger->error('MyFeedType: Failed for feed {feedId}: {message}', [ + 'feedId' => $feed->getId(), + 'message' => $t->getMessage(), + 'exception' => $t, + ]); + throw $t; + } + ``` +4. **Don't add per-call caching around the whole `getData()`** — `FeedService` already caches per-feed and honours `configuration['cache_expire']` as a per-feed TTL override. Cache lower-level shared lookups separately if needed. + +## 4. Output data contract + +Your returned array must match `FeedOutputModels.php` docblocks for the chosen model. Summary: + +| Model | Constant | Shape (abridged) | +|---|---|---| +| RSS | `RSS_OUTPUT` | `{title, entries: [{title, lastModified, content, author?}]}` or `[{title, lastModified, content}]` (see `RssFeedType` vs `ColiboFeedType`). | +| Calendar | `CALENDAR_OUTPUT` | `[{id, title, startTime, endTime, resourceTitle, resourceId}]` — Unix timestamps. | +| Instagram | `INSTAGRAM_OUTPUT` | `[{textMarkup, mediaUrl, videoUrl, username, createdTime}]`. | +| Poster | `POSTER_OUTPUT` | See `EventDatabaseApiV2FeedType` for the concrete shape. | +| Brnd booking | `BRND_BOOKING_OUTPUT` | `[{activity, area, bookingBy, bookingcode, startTime, endTime, …}]`. | + +Missing source fields → fill with sensible defaults or drop the entry. Templates should never crash on `null`. + +## 5. Wire configuration + +### Env-var-bound scalars → `config/services.yaml` + +```yaml +App\Feed\MyFeedType: + arguments: + $endpoint: '%env(string:MY_FEED_TYPE_ENDPOINT)%' + $cacheExpireSeconds: '%env(int:MY_FEED_TYPE_CACHE_EXPIRE_SECONDS)%' +``` + +Add the vars to **both** `.env` (with safe defaults / empty strings) and `.env.test` — the env-coverage check (`scripts/check-env-coverage.sh`) and the test kernel both depend on it. + +### Per-tenant data → `FeedSource.secrets`, not env vars + +Tokens, tenant ids, account keys — anything per-tenant. Only system-wide wiring (shared endpoint URLs, global cache tuning) goes in env vars. + +### Dedicated cache pool (if needed) + +Declare in `config/packages/cache.yaml` and inject by name. **Don't** reuse `$feedsCache` for intermediate lookups — its keys are feed ids, you'll collide with `FeedService`. + +## 6. Reference implementations to copy from + +| Your source looks like… | Start from | +|---|---| +| Plain RSS/Atom URL | `src/Feed/RssFeedType.php` | +| REST API with bearer token + dynamic dropdowns | `src/Feed/NotifiedFeedType.php` | +| Multiple endpoints with heavy caching + transformations | `src/Feed/CalendarApiFeedType.php` | +| External system with its own SDK/client | `src/Feed/BrndFeedType.php` + `src/Feed/SourceType/Brnd/` | +| Hydra/REST poster-event source | `src/Feed/EventDatabaseApiV2FeedType.php` + `src/Feed/EventDatabaseApiV2Helper.php` | + +**Deprecated — do not use as templates:** `SparkleIOFeedType`, `EventDatabaseApiFeedType`, `KobaFeedType`. + +## 7. Testing + +Add `tests/Feed/MyFeedTypeTest.php`. Pure unit tests (no kernel) are fine for transformation logic — see `tests/Feed/CalendarApiFeedTypeTest.php`. + +**Update `tests/Service/FeedServiceTest.php`** — add `$this->assertTrue(in_array(MyFeedType::class, $feedTypes));` to `testGetFeedTypes()`. This catches the regression where the service tag stops picking up your class. + +`testGetDataErrorIsNotCachedWithNormalTtl` in the same file documents the error-caching contract — read it before tweaking exception handling. + +Run: +```shell +task test:api -- tests/Feed tests/Service/FeedServiceTest.php +task code-analysis +``` + +## 8. Smoke-test end-to-end + +1. Boot the stack, log into admin. +2. Create a **FeedSource** — pick `App\Feed\MyFeedType`, fill secrets. Validation errors come from `getSchema()`. +3. Create a **Feed** on that source — the form is `getAdminFormOptions()`. Dynamic dropdowns confirm `getConfigOptions()`. +4. `GET /v2/feeds/{id}/data` (operation `_api_Feed_get_data` in `config/api_platform/feed.yaml`) — should return your normalized payload. +5. Break upstream (revoke token / dead URL) → re-request → expect `[]` with 30s cache, exception in logs. +6. Attach Feed to a slide with a matching-model template → confirm render. + +## 9. Before opening the PR + +- [ ] `task test:api` + `task coding-standards:php:check` clean. +- [ ] `task code-analysis` clean (PHPStan). +- [ ] `CHANGELOG.md` entry under unreleased. +- [ ] `docs/feed/my-feed-type.md` covering: required env vars, required secrets, required per-feed config keys, sample data shape (so frontend can mock). +- [ ] Class listed in `FeedServiceTest::testGetFeedTypes()`. +- [ ] No secret values logged — grep your own `$this->logger->error` calls. + +## 10. Common pitfalls + +- **Catching exceptions in `getData()` and returning `[]`.** Caches empty at feed-level TTL (often hours). Rethrow. Let `FeedService` handle the short error-cache. +- **Forgetting to add the class to `FeedServiceTest::testGetFeedTypes()`.** No CI failure today; quiet regression when the service tag breaks tomorrow. +- **Missing env var in `.env.test`.** Test kernel can't boot. `scripts/check-env-coverage.sh` (PostToolUse hook on `.env*`) catches the production-side miss; the test-env miss only surfaces when tests run. +- **Using `$feedsCache` for intermediate lookups.** Key collision with `FeedService`. Make your own cache pool. +- **Mutating `$feed->getConfiguration()` in `getData()` expecting persistence.** It's read-only. Documented exception: `EventDatabaseApiV2FeedType` unpublishes the owning slide via Doctrine when upstream returns 404. +- **Adding a new output model without updating frontend templates.** Templates filter `FeedSource` by `supportedFeedOutputType` — the new model is invisible until at least one template consumes it. +- **Logging secrets.** Tempting while debugging; strip before merging. + +## Related + +- README "Feeds" — architectural overview. +- ADR 005 — technology stack (FeedType pattern context). +- `src/Feed/FeedTypeInterface.php` — authoritative method signatures. +- `src/Feed/FeedOutputModels.php` — output model constants + shape docblocks. +- Subagent `feed-type-reviewer` — contract checks against the cookbook rules. diff --git a/.claude/skills/add-migration/SKILL.md b/.claude/skills/add-migration/SKILL.md new file mode 100644 index 00000000..4a0bfa09 --- /dev/null +++ b/.claude/skills/add-migration/SKILL.md @@ -0,0 +1,85 @@ +--- +name: add-migration +description: Create a Doctrine migration in display-api-service using the Schema tool API only (no raw SQL DDL). Use when the user asks to add a migration, change the database schema, add/remove a column, or follow up an entity change. Enforces ADR 010 and the PHPStan NoAddSqlInMigrationRule. +disable-model-invocation: true +--- + +# Add a Doctrine migration (Schema tool API only) + +ADR 010 forbids `$this->addSql(...)` in `migrations/`. Two gates enforce it: + +- `App\PhpStan\NoAddSqlInMigrationRule` — PHPStan rejects any `addSql` call under `migrations/`. +- `.github/workflows/doctrine.yaml` — `Validate Schema (postgres:16, experimental)` runs `doctrine:migrations:migrate` on Postgres 16; platform-specific DDL fails the build. + +A PreToolUse hook also blocks `Edit`/`Write` to `migrations/Version*.php` that contains `$this->addSql`. + +## Generate the skeleton + +```shell +task compose -- exec phpfpm bin/console doctrine:migrations:generate +``` + +This drops `migrations/Version.php` with empty `up()` / `down()` bodies and an injected `$this->isMariaDbServer()` check. **Delete that platform check** — Schema tool calls are portable; the check is only useful when emitting native SQL. + +## Authoring rules + +1. **Use the Schema tool**, not `addSql`: + + ```php + use Doctrine\DBAL\Schema\Schema; + use Doctrine\DBAL\Types\Types; + use Symfony\Bridge\Doctrine\Types\UlidType; + use App\DBAL\Types\RRuleType; + + public function up(Schema $schema): void + { + $table = $schema->createTable('foo'); + $table->addColumn('id', UlidType::NAME); + $table->addColumn('tenant_id', UlidType::NAME); + $table->addColumn('title', Types::STRING, ['length' => 255]); + $table->addColumn('config', Types::JSON, ['notnull' => false]); + $table->addColumn('created_at', Types::DATETIME_IMMUTABLE); + $table->setPrimaryKey(['id']); + $table->addIndex(['tenant_id'], 'IDX_foo_tenant'); + $table->addForeignKeyConstraint('tenant', ['tenant_id'], ['id'], [], 'FK_foo_tenant'); + } + + public function down(Schema $schema): void + { + $schema->dropTable('foo'); + } + ``` + +2. **Type references** must be one of: + - `Doctrine\DBAL\Types\Types::*` constants (`STRING`, `TEXT`, `INTEGER`, `BIGINT`, `BOOLEAN`, `DATETIME_IMMUTABLE`, `JSON`, `BLOB`, …). + - `Symfony\Bridge\Doctrine\Types\UlidType::NAME` for ULID primary keys. + - `App\DBAL\Types\RRuleType::RRULE` for the RRule column on `Schedule`. + - **Not** free-form strings like `'string'` or `'datetime_immutable'` — these break when a Doctrine type is renamed. + +3. **`down()` must mirror `up()` symmetrically.** Drop tables/columns/indexes in reverse order. Test-setup (`composer run test-setup`) drops the test DB every run, but production migrations may be rolled back. + +4. **Tenant-scoped tables** need a `tenant_id` column + FK + index. See `Slide`, `Playlist`, etc. + +5. **ULID primary keys** are the project default. Use `UlidType::NAME` not `Types::GUID` or `Types::STRING`. + +## When you genuinely need MariaDB-specific DDL + +The Schema tool can't express every MariaDB feature (e.g. specific collation tweaks, FULLTEXT indexes with options). Before reaching for `addSql`: + +1. Try Doctrine `$table->addOption('charset', 'utf8mb4')` or similar — most common needs are covered. +2. If not, **write an ADR amendment** documenting why this migration widens the MariaDB-only surface. Don't silently bypass the rule. + +## Validation + +```shell +task code-analysis # PHPStan — catches addSql +task test:api # test-setup re-migrates fresh DB +``` + +The full Postgres portability gate runs only in CI (`Validate Schema (postgres:16, experimental)` in `.github/workflows/doctrine.yaml`), but locally `task code-analysis` is enough to catch the disallowed-`addSql` failure mode. + +## Related + +- ADR 010: `docs/adr/010-schema-tool-migrations.md` +- PHPStan rule: `tools/phpstan/NoAddSqlInMigrationRule.php` +- The `add-api-resource` skill calls this one when a new entity needs a migration. diff --git a/.claude/skills/add-slide-template/SKILL.md b/.claude/skills/add-slide-template/SKILL.md new file mode 100644 index 00000000..4eee77f0 --- /dev/null +++ b/.claude/skills/add-slide-template/SKILL.md @@ -0,0 +1,161 @@ +--- +name: add-slide-template +description: Add a new slide template to display-api-service — generate a ULID, create the .jsx + .json pair under assets/shared/templates/, wire the id()/config()/renderSlide() contract with slideDone() signalling, and decide between shipped-with-project vs custom-templates/ placement. Use when the user asks to add a new slide type, a new template, or extend the renderer with a new content layout. +disable-model-invocation: true +--- + +# Add a slide template + +A slide template is the visual layout shown on a Screen. Each template is a two-file unit that the Screen Client loads and the Admin renders an editor form for. Both files must ship together. + +## Where it lives + +Two locations, same shape: + +- `assets/shared/templates/.{jsx,json}` — shipped with the project. PRs that add templates here are contributions to upstream. +- `assets/shared/custom-templates/.{jsx,json}` — installation-specific templates. This folder is **gitignored** (see `.gitignore`); populate it via a fork, symlink, or per-deployment repo. + +If the template is general-purpose, put it in `templates/` and consider a contribution PR (see README "Contributing template"). If it's specific to your tenant's needs, put it in `custom-templates/`. + +## Files to create + +### `.json` — config + admin form schema + +```json +{ + "title": "Display name (Danish — match existing templates' tone)", + "id": "", + "options": {}, + "adminForm": [ + { "key": "-form-1", "input": "header", "text": "Skabelon: ", "name": "header1", "formGroupClasses": "h4 mb-3" }, + { "key": "<name>-form-2", "input": "textarea", "name": "title", "label": "Overskrift", "formGroupClasses": "col-md-6" }, + { "key": "<name>-form-3", "input": "duration", "name": "duration", "min": "1", "type": "number", "label": "Varighed (i sekunder)", "required": true, "formGroupClasses": "col-md-6 mb-3" } + ] +} +``` + +**ULID generation** — 26 chars, Crockford base32 (excludes `I/L/O/U`). Never reuse another template's ULID; never change a published template's ULID. Quick generation: + +```shell +docker compose exec phpfpm php -r 'echo (new Symfony\Component\Uid\Ulid()) . "\n";' +``` + +Or any online ULID generator — just paste the result into `id`. + +**`adminForm` input types** (defined by the Admin renderer in `assets/admin/components/slide/content/`): + +| Input | Purpose | +|---|---| +| `header`, `header-h3` | Section headings (`text:` is the heading) | +| `input` | Plain HTML5 input; combine with `type: "text" \| "number" \| "email"` | +| `textarea` | Multi-line text | +| `rich-text-input` | HTML editor (tiptap) | +| `select` | Dropdown; needs `options: [{key, title, value}]` | +| `checkbox` | Boolean toggle | +| `image` / `video` / `file` | Media picker (set `multipleImages: true` for image arrays) | +| `duration` | Slide duration field | +| `contacts` | Contact entries | +| `feed` | Bind a feed to the slide (see "Feed integration" below) | +| `table` | Editable table | + +Every `adminForm` entry needs a unique `key:` (scoped to the template is fine) and a `name:`. The `name:` is the field on `slide.content` the renderer reads. + +### `<name>.jsx` — the renderer + contract + +```jsx +import { useEffect } from "react"; +import BaseSlideExecution from "../slide-utils/base-slide-execution.js"; +import "../slide-utils/global-styles.css"; +import myTemplateConfig from "./<name>.json"; + +function id() { + return myTemplateConfig.id; +} + +function config() { + return myTemplateConfig; +} + +function renderSlide(slide, run, slideDone) { + return ( + <MyTemplate + slide={slide} + run={run} + slideDone={slideDone} + content={slide.content} + executionId={slide.executionId} + /> + ); +} + +function MyTemplate({ slide, run, slideDone, content, executionId }) { + // BaseSlideExecution calls slideDone() after `content.duration` seconds. + // For anything more complex (video end, user interaction), invoke + // slideDone() yourself. + useEffect(() => { + if (!run) return; + const exec = new BaseSlideExecution(slide, slideDone); + exec.start(); + return () => exec.stop(); + }, [run, slide, slideDone]); + + const { title } = content; + return <div className="my-template">{title}</div>; +} + +export default { id, config, renderSlide }; +``` + +**Critical: `slideDone()` must be called.** A template that never signals done locks the playlist on whichever screen it loads on. `BaseSlideExecution` is the standard way for fixed-duration slides; for video-driven or interactive slides, call `slideDone()` from the relevant event handler. + +### Optional: `<name>/<name>.scss` + +Component-scoped styles go in a sibling subfolder (see `image-text/image-text.scss` for the canonical example). Import it from the `.jsx`. + +## Feed integration (optional) + +If the template displays external data (RSS, calendar, events): + +1. Add `{"input": "feed", "name": "feed", ...}` to `adminForm`. The Admin will show a feed-picker; the result lands at `slide.feed` and `slide.feedData`. +2. In the renderer, consume `slide.feedData` — its shape is the **feed output model** the chosen `FeedSource` produces (see `src/Feed/OutputModel/`). +3. The Client refreshes `slide.feedData` according to `CLIENT_PULL_STRATEGY_INTERVAL` (default 10 min) — the renderer doesn't need to fetch. + +Templates are decoupled from feed implementations via the output-model contract. A new feed source that produces the same output model can power any existing template — no template changes required. See README "Feeds" for the architecture. + +## Build & test + +After saving the two files: + +```shell +task assets:build +``` + +This compiles the templates into the asset bundle. The Client picks them up on next refresh. + +To preview interactively (dev mode), visit `http://<base-url>/template` — it renders every template against the fixtures in `fixtures/`. + +For automated checks: + +```shell +task test:unit # Vitest on any *.test.jsx alongside the template +task test:frontend-local # Playwright if you added a *.spec.js +``` + +## Validation + +The `slide-template-reviewer` subagent checks the contract — invoke it after creating/changing template files. It catches the common bugs (missing `slideDone()` call, ULID duplicate, adminForm/renderer key drift, wrong `renderSlide` signature). + +## Common mistakes + +- **Forgetting `slideDone()`** — most common bug. Slide enters playlist, never advances. `BaseSlideExecution` solves the fixed-duration case. +- **Reusing a ULID** — silently overwrites the other template's registration. Always generate a fresh one. +- **adminForm `name:` doesn't match what the renderer reads** — Admin writes to `slide.content.foo`, renderer reads `slide.content.bar`. Form changes appear to do nothing. +- **Shipping only `.jsx` or only `.json`** — half-broken template. The Stop hook `claude-hook-check-template-pairs.sh` warns; the slide-template-reviewer flags as a blocker. +- **Putting custom templates in `templates/` and committing them** — they're tenant-specific; use `custom-templates/` (gitignored) or fork. + +## Related + +- README "Custom Templates" — full reference for `adminForm` input types and the contribution path. +- `assets/shared/custom-templates-example/` — a working example to copy from. +- `assets/shared/slide-utils/base-slide-execution.js` — the slideDone helper. +- Subagent `slide-template-reviewer` — contract checks. diff --git a/.gitignore b/.gitignore index 40168e85..16b605cf 100644 --- a/.gitignore +++ b/.gitignore @@ -59,10 +59,17 @@ phpstan.neon /playwright-report/ /blob-report/ /playwright/.cache/ +/.playwright-mcp/ #< Playwright ###> vincentlanglet/twig-cs-fixer ### /.twig-cs-fixer.cache ###< vincentlanglet/twig-cs-fixer ### -.claude/ +###> anthropic/claude-code ### +.claude/* +!.claude/settings.json +!.claude/agents/ +!.claude/commands/ +!.claude/skills/ +###< anthropic/claude-code ### diff --git a/.mcp.json b/.mcp.json new file mode 100644 index 00000000..c5280c5f --- /dev/null +++ b/.mcp.json @@ -0,0 +1,8 @@ +{ + "mcpServers": { + "context7": { + "type": "http", + "url": "https://mcp.context7.com/mcp" + } + } +} diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d774c83..01b5cd53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +- Added project-shared Claude Code configuration (hooks, skills, subagents, MCP servers, plugins) under `.claude/` + for consistent AI-assisted workflows on this codebase. Requires `npm install -g intelephense` once per developer + to activate the PHP language server (the `playwright` and `context7` plugins need no prerequisites). - Rewrote the consolidated end-of-2.8 migration to Doctrine's Schema tool API; added a `NoAddSqlInMigrationRule` PHPStan rule to enforce the convention on future migrations. - Added a Postgres `Validate Schema` job to the Doctrine workflow as a regression gate against diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..cd924958 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,162 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Dev tooling — always through `task` + +This is a Dockerised dev environment. The host has no PHP/Composer/Node — all PHP and JS commands run inside the +`phpfpm` / `node` containers via `task` (which wraps `docker compose exec`). Never invoke `php`, `composer`, +`phpunit`, `phpstan`, `npm`, etc. directly on the host. + +Common commands (run from repo root): + +- `task` — list all tasks +- `task site-install` / `task site-install-with-fixtures` — reset + bootstrap the local stack +- `task composer -- <cmd>` — composer in the phpfpm container +- `task compose -- exec phpfpm bin/console <cmd>` — Symfony console +- `task test:api -- --filter SomeTest` — PHPUnit; `--` forwards to phpunit; `composer run test-setup` (drop+create + test DB and re-migrate) runs first +- `task test:unit` — Vitest (JS unit/component) +- `task test:frontend-built` / `task test:frontend-local` — Playwright e2e +- `task coding-standards:check` / `:apply` — php-cs-fixer + prettier + markdownlint + env coverage +- `task code-analysis` — PHPStan (level 6, with project-local rules) +- `task rector:check` / `:apply` — Rector +- `task db:migrate` — Doctrine migrations +- `task generate:api-spec` then `task generate:redux-toolkit-api` — regenerate `public/api-spec-v2.{yaml,json}` and + the RTK Query client (`assets/shared/redux/generated-api.ts`). Must be done whenever the API surface changes. +- `task app:update` — `bin/console app:update` (migrate + reinstall templates), the production update path + +PostToolUse hooks (see `.claude/settings.json`) automatically run php-cs-fixer, phpstan, twig-cs-fixer, +prettier, markdownlint, and env-coverage on files you edit — you don't need to invoke these yourself for +single-file changes. A Stop hook also warns if you change `src/Dto/` or `src/State/` without regenerating the +OpenAPI spec, and a PreToolUse hook rejects `$this->addSql(...)` in migrations (ADR 010). + +### Claude Code prerequisites (one-time, host-side) + +Two things teammates install once on their host so the project's auto-enabled plugin and MCP server work: + +- **Intelephense** — required by the `php-lsp@claude-plugins-official` plugin enabled in `.claude/settings.json`. + Install: `npm install -g intelephense`. Provides go-to-definition, find-references, and inline diagnostics + across the Entity ↔ DTO ↔ Provider/Processor chain. Static analyser only — does not need host PHP. +- **context7 MCP** — auto-enabled via `.mcp.json` + `.claude/settings.json` `enabledMcpjsonServers`. No + install step; Claude Code fetches it on first use. Used for live Symfony/API Platform/Doctrine docs lookup. + +## The `/v2/` API is versioned — no breaking changes allowed + +The HTTP API at `/v2/` is a **published, versioned contract** (ADR 003). Breaking changes — renamed/removed +fields, removed endpoints, changed types, newly required parameters, tightened responses — are gated by +CI: `.github/workflows/apispec.yaml` runs `oasdiff` against the base branch and **fails the build** on any +BC-classified diff (`fail-on: ERR`). Non-breaking additions (new endpoints, new optional fields, new optional +parameters) are fine and surface as a PR comment. + +If you need to break a field's shape, add the new field *alongside* the old one and keep both in sync (see +the example in ADR 003 — adding `title` without removing `name`). A genuine BC break would require bumping to +`/v3` and is out of scope for routine work — flag it for human review, do not just push through the gate. + +## High-level architecture + +This is a **monorepo** (see ADR 008) containing: + +- A Symfony 6.4 + **API Platform 3.x** backend (`src/`, `config/`, `migrations/`) — produces the JSON-LD/Hydra API at + `/v2/...` (see "no breaking changes" above) and OpenAPI docs at `/docs`. +- Two React/Vite frontends — `assets/admin/` (content editor, served at `/admin`) and `assets/client/` (the screen + client, served at `/client`). Shared code in `assets/shared/`. The admin/client talk to the backend through an + RTK Query slice generated from the OpenAPI spec. +- Slide templates in `assets/shared/templates/` (rendered both by `/template` preview and by the client). + +### Content model (the "big picture") + +`Screen → Layout (with ScreenLayoutRegions) → PlaylistScreenRegion → Playlist → PlaylistSlide → Slide → Template/Theme/Media/Feed`. +Plus `ScreenGroup` (collections of screens) and `Campaign` overrides via `ScreenCampaign` / `ScreenGroupCampaign`. +See the partial class diagram in `README.md` and the entity classes in `src/Entity/Tenant/`. + +### Multi-tenancy + +- Every content entity extends `App\Entity\Tenant\AbstractTenantScopedEntity` (FK to `Tenant`). +- Tenant scoping is enforced via `App\Security\TenantScopedAuthenticator` + Doctrine extensions in + `src/Filter/TenantExtension.php` and voters in `src/Security/Voter/`. New tenant-scoped repository queries should + use `MultiTenantRepositoryTrait` (which still emits MariaDB-specific SQL — see "Schema portability" below). +- A user has roles per tenant via `UserRoleTenant`; the JWT carries the active tenant. + +### API Platform layer + +API endpoints are declared via attributes on **DTO classes in `src/Dto/`** (the external API shape), not directly on +entities. Each DTO has a matching **State Provider** (`src/State/*Provider.php`, reads) and **Processor** +(`src/State/*Processor.php`, writes) that translate between DTO ↔ Entity. When adding a resource: + +1. Add/update entity in `src/Entity/Tenant/`. +2. Add migration in `migrations/` using the Schema tool API (see below). +3. Add/update DTO in `src/Dto/` with the `#[ApiResource]` attribute and operations. +4. Add/update provider + processor in `src/State/`. +5. `task generate:api-spec && task generate:redux-toolkit-api`, then enhance `assets/shared/redux/enhanced-api.ts` + with tag invalidation for the new endpoints. + +### Authentication + +Two flows, both via Lexik JWT: + +- `/v2/authentication/token` — username/password or OIDC for users (admin/editor). OIDC has two providers: + `internal` (authoritative — grants tenant access from claims like `Example1Admin`) and `external` (only + authenticates; tenant access granted later via activation code). See `App\Security\AzureOidcAuthenticator`, + `ExternalUserAuthenticator`. +- `/v2/authentication/screen` — separate flow for screen clients (`ScreenAuthenticator`, `ScreenUser`). + +API is stateless except for these auth routes. + +### `relationsModified` checksum tree + +Entities expose a `relationsModified` map of SHA1 checksums of their related collections so clients can detect +whether they need to re-fetch nested data. Recalculated in `App\EventListener\RelationsModifiedAtListener` +(postFlush) by `App\DBAL\RelationsChecksumCalculator` using raw MariaDB SQL that walks the tree bottom-up. This +listener is intentionally MariaDB-specific and is one of the known-non-portable parts of the runtime (see ADR 010). + +### Feeds + +External data sources (RSS, calendar APIs, Microsoft Graph, custom HTTP, …) implement +`App\Feed\FeedTypeInterface` and normalise their output into a small set of well-known **output models** +(`src/Feed/OutputModel/`). Slide templates render against output models, decoupling them from feed implementations. +A `FeedSource` is the configured instance (URL + secrets); a `Feed` ties a `Slide` to a `FeedSource` with per-slide +configuration. Manage via the `app:feed:*` console commands. + +## Schema portability & migrations (ADR 010) + +- Default DB is **MariaDB 11.4**. CI also exercises MariaDB 10.11 (`.github/workflows/{phpunit,doctrine}.yaml` + matrix). To run locally against 10.11, set `MARIADB_IMAGE=mariadb:10.11` and `MARIADB_VERSION=10.11.13-MariaDB` + (the latter goes into Doctrine's `serverVersion` and must match — see `.env`). +- **Postgres is a portability regression gate, not a deployment target.** The `Validate Schema (postgres:16, + experimental)` job in `doctrine.yaml` runs migrations against Postgres 16; a green run does not mean the app + works on Postgres. +- **No `addSql` in `migrations/`** — enforced by `App\PhpStan\NoAddSqlInMigrationRule` (in `tools/phpstan/`, + wired in `phpstan.dist.neon`). All DDL must go through Doctrine's Schema tool API + (`$schema->createTable(...)`, `$table->addColumn(...)`, …) and use `Doctrine\DBAL\Types\Types::*` constants + (plus `UlidType::NAME`, `RRuleType::RRULE`) rather than free-form strings. +- Entity listeners and `MultiTenantRepositoryTrait` still emit native MariaDB SQL — that's known tech debt, out of + scope of ADR 010, but means runtime queries are not yet Postgres-safe. + +## Files to leave alone + +- `composer.lock`, `package-lock.json` — generated; let composer/npm update them. +- `public/api-spec-v2.yaml`, `public/api-spec-v2.json` — regenerated by `task generate:api-spec`. +- `assets/shared/redux/generated-api.ts` — regenerated by `task generate:redux-toolkit-api`. Customise via + `assets/shared/redux/enhanced-api.ts`. +- `.env.local`, `.env.*.local` — operator/local secrets, not committed. +- `phpstan-baseline.neon` — regenerate with `task phpstan:generate-baseline` rather than hand-editing. + +A PreToolUse hook blocks edits to these. + +## Coding standards & quality gates + +PRs must pass the workflows in `.github/workflows/` (php-cs-fixer, phpstan, rector, phpunit on the MariaDB matrix, +playwright, vitest, env-coverage, twig, markdownlint, prettier, composer-normalize, apispec drift, doctrine +schema-portability). Run `task run-checks-and-tests` locally to mirror the bulk of these. + +Every Symfony env var referenced under `config/` must be documented in `.env` — `scripts/check-env-coverage.sh` +(also run as a PostToolUse hook) enforces this. + +## Docs + +- `docs/adr/` — architectural decisions (read these before deviating from established patterns). +- `docs/configuration/` — OIDC, calendar feed, and other integration setup. +- `docs/test-guide/test-guide.md` — manual test checklist. +- `README.md` — extensive reference for config env vars, feed types, templates, and screen-status semantics. +- `UPGRADE.md` — version-to-version upgrade notes. diff --git a/scripts/claude-hook-check-api-spec-drift.sh b/scripts/claude-hook-check-api-spec-drift.sh new file mode 100755 index 00000000..bfbb6ff4 --- /dev/null +++ b/scripts/claude-hook-check-api-spec-drift.sh @@ -0,0 +1,35 @@ +#!/bin/sh +# Stop-hook helper: warn when API surface changed without regenerating the OpenAPI spec. +# +# Triggers when working-tree or staged changes include files under +# src/Dto/, src/State/*.php, or config/api_platform/ but no matching change +# to public/api-spec-v2.{yaml,json}. Writes a warning to stderr and exits 0 +# (advisory — never blocks). + +set -u + +cd "${CLAUDE_PROJECT_DIR:-.}" || exit 0 + +CHANGED="$(git status --porcelain 2>/dev/null | awk '{print $NF}')" + +echo "$CHANGED" | grep -qE '^(src/Dto/|src/State/.+[.]php$|config/api_platform/)' || exit 0 + +if echo "$CHANGED" | grep -qE '^public/api-spec-v2[.](yaml|json)$'; then + exit 0 +fi + +cat >&2 <<'MSG' +WARN: API surface changed (src/Dto, src/State, or config/api_platform/) without + regenerating public/api-spec-v2.{yaml,json}. Run: + + task generate:api-spec + + If endpoints were added/removed, also run: + + task generate:redux-toolkit-api + + and enhance assets/shared/redux/enhanced-api.ts with the new tag + invalidations. +MSG + +exit 0 diff --git a/scripts/claude-hook-check-template-pairs.sh b/scripts/claude-hook-check-template-pairs.sh new file mode 100755 index 00000000..afd937fa --- /dev/null +++ b/scripts/claude-hook-check-template-pairs.sh @@ -0,0 +1,54 @@ +#!/bin/sh +# Stop-hook helper: warn when a slide template's .jsx and .json drift apart. +# +# A slide template is two files in assets/shared/templates/<name>.{jsx,json}. +# Pushing only one half half-renders the admin form. This script checks the +# working tree for any base name where exactly one of the pair is present, +# writes a warning to stderr, and exits 0 (advisory — never blocks). + +set -u + +cd "${CLAUDE_PROJECT_DIR:-.}" || exit 0 + +DIR="assets/shared/templates" +[ -d "$DIR" ] || exit 0 + +ORPHAN_JSX="" +ORPHAN_JSON="" + +# .jsx without .json +for f in "$DIR"/*.jsx; do + [ -e "$f" ] || continue + base=$(basename "$f" .jsx) + if [ ! -f "$DIR/$base.json" ]; then + ORPHAN_JSX="$ORPHAN_JSX $base" + fi +done + +# .json without .jsx +for f in "$DIR"/*.json; do + [ -e "$f" ] || continue + base=$(basename "$f" .json) + if [ ! -f "$DIR/$base.jsx" ]; then + ORPHAN_JSON="$ORPHAN_JSON $base" + fi +done + +if [ -z "$ORPHAN_JSX" ] && [ -z "$ORPHAN_JSON" ]; then + exit 0 +fi + +{ + echo "WARN: slide template pair(s) out of sync in $DIR/" + if [ -n "$ORPHAN_JSX" ]; then + echo " Missing .json for:" + for name in $ORPHAN_JSX; do echo " - $name"; done + fi + if [ -n "$ORPHAN_JSON" ]; then + echo " Missing .jsx for:" + for name in $ORPHAN_JSON; do echo " - $name"; done + fi + echo " A slide template must ship both halves — see add-slide-template skill." +} >&2 + +exit 0