diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 56a3177b..8b252381 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -5,6 +5,9 @@ on: paths: - 'directus-cms/extensions/directus-extension-programmierbar-bundle/**' - 'nuxt-app/**' + # The extension bundle imports from shared-code/, so a change there can break its build or + # tests. Without this line those changes skip the gate entirely. + - 'shared-code/**' - '.github/workflows/run_tests.yml' jobs: @@ -17,13 +20,18 @@ jobs: - name: Checkout repository uses: actions/checkout@v4 - # Deliberately still 22 while nuxt-app moves to 24: this tree has its own lockfile, its own - # Jest setup, and is frozen pending the Directus licence clarification. Moving its runtime - # would be an untested change to a tree nobody is allowed to upgrade. - - name: Use Node.js 22.x + # Reads the bundle's own .nvmrc rather than naming a version here, so CI, the Docker image and + # local development have one file to change instead of three. + # + # Deliberately still 22 while nuxt-app is on 24 — but *not* because of the Directus licence + # block, which an earlier version of this comment claimed. Every @directus/* package this + # bundle uses is MIT; only the `directus` server package is licence-blocked, and it lives in + # directus-cms/package.json. The real reason is Dockerfile.directus: Node 24 brings npm 11, + # which gates the install scripts this tree's native dependencies need. + - name: Use Node.js uses: actions/setup-node@v4 with: - node-version: 22 + node-version-file: directus-cms/extensions/directus-extension-programmierbar-bundle/.nvmrc cache: 'npm' cache-dependency-path: 'directus-cms/extensions/directus-extension-programmierbar-bundle/package-lock.json' @@ -31,12 +39,24 @@ jobs: - name: Install dependencies run: npm ci + # Keep `prettier:check`, not `prettier` — the latter writes, so it would pass by mutating. + # Without this step formatting is voluntary, which is how 64 files drifted here unnoticed. + - name: Check formatting + run: npm run prettier:check + + # Keep `lint`, not `eslint` — the latter runs with --fix, so it mutates instead of failing. - name: Run ESLint run: npm run lint - name: Run tests run: npm test + # Nothing above confirms the bundle still compiles: `directus-extension build` has its own + # Rollup/esbuild pipeline that neither ESLint nor Jest exercises, so a change that breaks the + # actual artefact Directus loads could otherwise merge green. + - name: Build + run: npm run build + nuxt-app-test: runs-on: ubuntu-latest defaults: diff --git a/AGENTS.md b/AGENTS.md index 215d91c6..c21b1d97 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,9 +66,15 @@ npm run migrate:db # Database migrations ```bash # In directus-cms/extensions/directus-extension-programmierbar-bundle/ -npm test # Run Jest tests +npm run prettier:check # Formatting — fails, never rewrites +npm run lint # ESLint over .ts and the one .vue file +npm test # Jest +npm run build # directus-extension build ``` +That is the full CI gate, in order. There is deliberately no typecheck step yet — `tsc` cannot +currently run on that tree at all. See [the tooling plan](docs/directus-extension-tooling-plan.md). + ## Code Principles ### Consolidation & DRY @@ -114,16 +120,19 @@ Additional hints can be found in: ### Formatting is enforced, not requested -CI runs `npm run prettier:check` in `nuxt-app`, so unformatted code fails the build. Nobody should be -expected to remember the formatter — turn on **format on save** and it never comes up: +CI runs `npm run prettier:check` in **both** `nuxt-app` and the Directus extension bundle, so +unformatted code fails the build. Nobody should be expected to remember the formatter — turn on +**format on save** and it never comes up: - **WebStorm**: Settings → Languages & Frameworks → JavaScript → Prettier → *On save* - **VS Code**: the Prettier extension, plus `"editor.formatOnSave": true` -Both read `nuxt-app/.prettierrc` on their own, and `nuxt-app/.editorconfig` covers indentation and line -endings before that is set up. Keep those two in step — they overlap, and an editor that indents to a -different width than Prettier produces a diff on every save. If a PR fails the check, `npm run prettier` -fixes it — never hand-edit to satisfy it. +Editors pick up whichever `.prettierrc` is nearest the file, and each gated tree ships one alongside an +`.editorconfig` that covers indentation and line endings before the formatter is set up. Keep each pair +in step — they overlap, and an editor that indents to a different width than Prettier produces a diff on +every save. The two configs are identical apart from `nuxt-app`'s Tailwind class-sorting plugin, so the +same editor setup works in both. If a PR fails the check, `npm run prettier` fixes it — never hand-edit +to satisfy it. Note that `npm run lint` (ESLint) needs `nuxt prepare` to have run first, since `eslint.config.mjs` extends the generated `.nuxt/eslint.config.mjs`. `npm ci` does this via `postinstall`. diff --git a/Dockerfile.directus b/Dockerfile.directus index 45d413e2..6d1268c9 100644 --- a/Dockerfile.directus +++ b/Dockerfile.directus @@ -1,4 +1,10 @@ # Choose a base image +# +# Keep this in step with `directus-cms/extensions/directus-extension-programmierbar-bundle/.nvmrc`, +# which CI reads — the image and CI must build the extension on the same Node, or CI stops being +# evidence about production. Moving to 24 is deliberately deferred: Node 24 ships npm 11, which +# gates dependency install scripts, and this tree needs them to run (sharp, sqlite3, isolated-vm, +# esbuild all build native code on install). See docs/directus-extension-tooling-plan.md. FROM node:22 # Set working directory @@ -9,7 +15,10 @@ COPY directus-cms/package.json . COPY directus-cms/package-lock.json . # Install dependencies -RUN npm install +# +# `npm ci`, not `npm install`: the latter is free to resolve versions the lockfile does not record, +# so the image could ship dependencies CI never tested. `npm ci` fails instead, which is the point. +RUN npm ci # Copy the shared code that lives outside of directus dir COPY shared-code ../shared-code @@ -20,8 +29,8 @@ COPY directus-cms . # Set working directory to interface extension WORKDIR /usr/src/app/directus-cms/extensions/directus-extension-programmierbar-bundle -# Install publishable interface extension dependencies -RUN npm install +# Install extension bundle dependencies (its own lockfile, copied in by `COPY directus-cms .` above) +RUN npm ci # Build the extension RUN npm run build diff --git a/_ADRs/0003-three-tier-extension-testing.md b/_ADRs/0003-three-tier-extension-testing.md new file mode 100644 index 00000000..748806fd --- /dev/null +++ b/_ADRs/0003-three-tier-extension-testing.md @@ -0,0 +1,140 @@ +# ADR 0003: Three-tier testing for the Directus extension bundle + +- **Status:** Accepted +- **Date:** 2026-08-05 +- **Scope:** `directus-cms/extensions/directus-extension-programmierbar-bundle` +- **Relationship to other ADRs:** **complements** [ADR 0001](0001-jest-runs-in-cjs-mode.md) — its + `util/`-extraction guidance is Tier 1 here and still stands. Supersedes nothing. Gives + [ADR 0002](0002-batch-updates-use-updateone.md)'s unenforced convention a place to be enforced. + +## Context + +At the time of writing, 9 of the bundle's 26 entries have any test, and the ones that do follow a +single pattern: extract business logic into a `util/` module with no framework imports, unit-test +that. `post-to-discord` is the model — it tests `buildNewsEmbed` for URL construction, brand colour +and slug fallback, and touches neither the hook registration nor the HTTP call. + +That pattern is good and cheap, and it has a structural blind spot: **it cannot see the hook's +contract with Directus.** Every bug this bundle is known to have shipped lives in exactly that blind +spot: + +- **ADR 0002's bug.** `ItemsService.updateMany` fires a *single* action carrying `metadata.keys[]`. + Downstream hooks read `metadata.key || metadata.keys[0]`, so a cascade of five items reindexed only + the first. The other four were written to the database correctly and silently never reached Algolia. + Nothing threw. +- **The buzzsprout-class crash** referenced in ADR 0001: an un-awaited promise in a hook handler + taking down the CMS. The `no-floating-promises` lint rule now catches that specific shape, but not + the general class. +- The two lifecycle traps in [the conventions doc](../.claude/rules/directus-conventions.md): whether + a hook fires when an item is *created* already in the triggering state, and whether a guard prevents + it firing twice. + +None of these are reachable from a pure function. So the gap is not "17 entries are untested" — it is +that **nothing tests the hook-to-Directus contract, including for the 9 entries that are tested.** + +A real-Directus E2E suite is the obvious answer and the wrong first answer: it is slow, it flakes, and +writing one scenario per hook would put a multi-minute, network-shaped suite on the PR gate. + +## Decision + +Three tiers, with explicit ownership of what each is for. + +### Tier 1 — unit tests on extracted pure functions + +Unchanged from ADR 0001. Logic moves into `util/`, gets tested without mocks. Cheapest tests in the +tree; keep writing them, and keep extracting to make them possible. + +**Owns:** decisions, payload shapes, formatting, parsing, validation. + +### Tier 2 — hook contract tests + +Run the **real hook module** against a **fake Directus context**. Import the hook's default export, +pass a hand-built `{ services: { ItemsService }, getSchema, env, logger }`, capture the registered +`action`/`filter` callbacks and invoke them directly. No Directus process, no network, milliseconds. + +**Owns:** that the hook registers for the right events; that the create path and the update path both +behave; that guards prevent double execution; that the **ADR 0002 contract** holds — assert per-item +`updateOne` calls rather than one `updateMany`; that failures reach the notification path. + +This is not a new invention. Five test files already do it — `cascade-publish`, `create-news`, +`fetch-open-graph`, `newsletter-double-opt-in`, `schedule-publication`. The decision is to make it the +**default for every entry**, and to consolidate the per-file fake contexts into one shared helper as +the sixth is written. + +### Tier 3 — E2E against a real Directus, deliberately thin + +The **built bundle** loaded into a **real Directus 11.17.4**. Roughly five scenarios, not +twenty-six. + +**Owns only what Tier 2 fakes:** that the bundle *loads at all* (23 hooks + 2 endpoints register); +that a real `ItemsService` write fires a real action; that the `publishable` interface renders in the +admin UI. It is also the only thing that can validate `@directus/extensions-sdk` 18 against the +frozen host, which is why it must exist before that upgrade. + +**Not on the PR gate.** Same reasoning as `nuxt-app`'s `smoke_tests.yml`: a harness with a real server +and a real database will flake, and a flaky *required* check trains people to ignore red. Schedule it, +run it on demand, and promote it only once it has proven quiet. + +## External services in Tier 3: redirect, do not mock + +In Tier 3 the hook runs inside a separate Directus process, so module mocking is unavailable — there +is no `vi.mock` across a process boundary. The only lever is **configuration**: point every outbound +base URL at one local stub server. + +Audited 2026-08-05, all 11 outbound integrations: + +| Already redirectable via env | Hardcoded — needs a code change | +| --- | --- | +| Buzzsprout (`BUZZSPROUT_API_URL`) | Gemini — `GEMINI_API_BASE` module const | +| Browserless (`BROWSERLESS_API_URL`) | Bluesky — `BSKY_SERVICE = 'https://bsky.social'` | +| Deepgram (`DEEPGRAM_API_URL`) | Slack — `new WebClient(token)` at module load | +| Discord (`DISCORD_WEBHOOK_URL`) | Algolia — host derived from `ALGOLIA_APP_ID` | +| Vercel (`VERCEL_DEPLOY_WEBHOOK_URL`) | Wallet — `walletobjects.`/`oauth2.googleapis.com` | +| Mastodon (`MASTODON_INSTANCE_URL`) | | + +Six already work. The remaining five must become configurable — which +[AGENTS.md](../AGENTS.md) already requires under "No Hidden Behavior" (*"No hardcoded defaults buried +in business logic … If it affects behavior, it must be visible and configurable"*), so that refactor +is justified independently of testing. + +Two rules for the harness: + +1. **Fail closed.** The stub returns 500 for any unrecognised path and fails the run, so a missed + redirect is loud. +2. **No secrets in the environment at all** — not fake-but-plausible values. A missed redirect must + die on a connection error, never reach the production Slack workspace. + +**Explicitly excluded:** hitting real third-party APIs, including sandboxes. That imports their uptime +into CI. Contract drift at Buzzsprout is a scheduled canary against staging, not a PR gate. + +## Consequences + +**Positive** + +- The bug classes that have actually cost us are testable, in the fast suite, without infrastructure. +- ADR 0002's convention stops being unenforceable — a future `updateMany` fails a test. +- Tier 3 stays small enough to be maintainable, because Tier 2 covers the per-hook logic. +- The endpoint refactor Tier 3 forces is something the conventions already demanded. + +**Negative / costs** + +- Tier 2 tests are coupled to the *shape* of the Directus hook context. If a Directus upgrade changes + that shape, the fakes drift from reality and can pass while production breaks. **This is the main + risk, and it is what Tier 3 is insurance against** — the two tiers must not both be faked. +- One shared fake-context helper becomes a load-bearing test utility. Under-building it means 18 + divergent copies; over-building it means a mini-framework. Consolidate at the sixth use, not the + first. +- Tier 3 adds ~2 minutes of CI and a stub server to maintain. +- Tier 3 only proves behaviour against 11.17.4. Correct while the server is frozen; revisit when the + licence question resolves. + +## Ordering + +Tiers 2 and 3 both come **before** the dependency upgrades, which is a change from the first draft of +[the tooling plan](../docs/directus-extension-tooling-plan.md). The reason is measured, not +stylistic: of the 16 files the planned bumps touch, **3 have a test covering them**, and +`algolia-index` — largest module in the bundle, zero tests — is hit by four separate bumps +(`sanitize-html`, `algoliasearch`, `@directus/sdk`, `meow`). + +The one exception is the `sanitize-html` critical, which ships as a carve-out with unit tests for its +consumer in the same PR. Holding a security patch behind a multi-PR test phase is the wrong trade. diff --git a/directus-cms/extensions/directus-extension-programmierbar-bundle/.nvmrc b/directus-cms/extensions/directus-extension-programmierbar-bundle/.nvmrc new file mode 100644 index 00000000..2bd5a0a9 --- /dev/null +++ b/directus-cms/extensions/directus-extension-programmierbar-bundle/.nvmrc @@ -0,0 +1 @@ +22 diff --git a/directus-cms/extensions/directus-extension-programmierbar-bundle/.prettierignore b/directus-cms/extensions/directus-extension-programmierbar-bundle/.prettierignore new file mode 100644 index 00000000..98ba0b5c --- /dev/null +++ b/directus-cms/extensions/directus-extension-programmierbar-bundle/.prettierignore @@ -0,0 +1,8 @@ +# Local editor/agent tool state. `prettier . --write` walks the whole tree, and without this it +# rewrites contributors' untracked `.claude/settings.local.json` — a file the formatter has no +# business touching and that nobody reviews. +# +# `node_modules` and `dist` are not listed here on purpose: Prettier reads `.gitignore` in addition +# to this file, and the bundle's `.gitignore` already covers both. Duplicating them would mean two +# places to keep in step. +.claude diff --git a/directus-cms/extensions/directus-extension-programmierbar-bundle/.prettierrc b/directus-cms/extensions/directus-extension-programmierbar-bundle/.prettierrc index 0cf2d5fb..65b7c8e0 100755 --- a/directus-cms/extensions/directus-extension-programmierbar-bundle/.prettierrc +++ b/directus-cms/extensions/directus-extension-programmierbar-bundle/.prettierrc @@ -9,7 +9,6 @@ "jsxSingleQuote": false, "trailingComma": "es5", "bracketSpacing": true, - "jsxBracketSameLine": false, "arrowParens": "always", "endOfLine": "lf" } diff --git a/directus-cms/extensions/directus-extension-programmierbar-bundle/TESTING.md b/directus-cms/extensions/directus-extension-programmierbar-bundle/TESTING.md index cd6f48ce..f64ae723 100644 --- a/directus-cms/extensions/directus-extension-programmierbar-bundle/TESTING.md +++ b/directus-cms/extensions/directus-extension-programmierbar-bundle/TESTING.md @@ -1,64 +1,76 @@ -# Automated Testing Setup for directus-extension-programmierbar-bundle +# Testing `directus-extension-programmierbar-bundle` -This document provides an overview of the automated testing setup for the `directus-extension-programmierbar-bundle` project. +## Running the suite -## Testing Framework - -The project uses Jest as the testing framework, with the following configuration: - -- **TypeScript Support**: Using ts-jest for TypeScript support -- **Test Files**: Located in `__tests__` directories with `.test.ts` extension -- **Configuration**: Jest configuration in `jest.config.ts` - -## Test Structure - -The tests are organized as follows: - -- Each extension has its own `__tests__` directory -- Test files are named after the function or component they test -- Test utilities are stored in a `utils` directory within the `__tests__` directory - -## Running Tests +```bash +npm test # run once +npm run test:watch +``` -To run the tests, use the following command: +The full CI gate, in the order `.github/workflows/run_tests.yml` runs it: ```bash -npm test +npm run prettier:check # formatting — fails, never rewrites +npm run lint # ESLint over .ts and the one .vue file +npm test # Jest +npm run build # directus-extension build ``` -## Current Test Coverage +There is deliberately no typecheck step yet. `tsc` cannot currently run on this tree at all — see +[the tooling plan](../../../docs/directus-extension-tooling-plan.md), Phase 1. + +## Framework -The following components have automated tests: +Jest with `ts-jest`. Tests live in `__tests__/` directories beside the code they cover and are named +`*.test.ts`; `jest.config.ts` matches `**/__tests__/**/*.test.ts` and nothing else. -- `getPayloadWithSlug` function in the `set-slug` hook +**The suite runs as CommonJS, not ESM** — despite `jest.config.ts` asking for ESM. This is not a +detail you can ignore when writing tests, and +[ADR 0001](../../../_ADRs/0001-jest-runs-in-cjs-mode.md) explains why it is that way and what it +costs. In short: `jest.unstable_mockModule` and top-level `await` do not work, and an ESM-only +dependency anywhere in a tested import chain has to be stubbed before it can be imported. -## Adding More Tests +## How to write a test here -To add tests for other components: +**Prefer extracting pure functions.** The established pattern is to move business logic into a +`util/` module with no framework imports and unit-test that directly — no mocks, no module-format +problems. Five extensions already do this (`set-slug`, `member-matching`, `cascade-publish`, +`create-news`, `fetch-open-graph`), and it is the approach that will survive the move off Jest. -1. Create a `__tests__` directory in the component's directory -2. Create a test file with the `.test.ts` extension -3. Write tests using Jest's testing functions -4. Run the tests to verify they work +**When a hook's entry file must be tested directly**, use hoisted `jest.mock(...)` and stub the +ESM-only framework dependencies: + +```ts +// The real `defineHook` just returns its callback, so this stub exercises the real hook logic +// without loading the untranspiled package. +jest.mock('@directus/extensions-sdk', () => ({ + defineHook: (callback: unknown) => callback, +})) +``` -## Test Documentation +See `cascade-publish/__tests__/index.test.ts` for the full pattern. Also mock anything that reaches +the network — `postSlackMessage`, `email-service`, `axios` — so tests stay offline. -Each `__tests__` directory contains a README.md file that explains: +**Two things worth testing in every hook**, both from +[the Directus conventions](../../../.claude/rules/directus-conventions.md): -- The testing approach for that component -- The test cases covered -- How to run the tests -- How to add more tests -- The mocking strategy used +1. Does it behave correctly when an item is **created** already in the triggering state, not just + when one is updated into it? +2. Is there a guard that stops it firing repeatedly — a status field or equivalent? -## Dependencies +## Current coverage -The testing setup uses the following dependencies: +**205 assertions across 15 files, covering 9 of the bundle's 26 entries.** -- jest: The testing framework -- ts-jest: TypeScript support for Jest -- @types/jest: TypeScript type definitions for Jest -- @jest/globals: Global functions and types for Jest -- ts-node: For running TypeScript files directly +| Covered | Test files | +| ---------------------------------------------------------------------------------------------------- | ---------- | +| `shared` (`isPublishable`, `safeHook`, `settings`) | 3 | +| `fetch-open-graph` (incl. `openGraph`, `urlSafety`) | 3 | +| `cascade-publish` | 2 | +| `create-news` (incl. `newsTarget`) | 2 | +| `member-matching`, `newsletter-double-opt-in`, `post-to-discord`, `schedule-publication`, `set-slug` | 1 each | -These dependencies are listed in the `package.json` file. +The other 17 entries have no tests, including the four largest modules in the bundle +(`algolia-index` at 1333 LOC, `buzzsprout`, `asset-generation`, `social-media-publish`). Closing that +gap is Phase 5 of [the tooling plan](../../../docs/directus-extension-tooling-plan.md), which ranks +them by size and blast radius. diff --git a/directus-cms/extensions/directus-extension-programmierbar-bundle/eslint.config.js b/directus-cms/extensions/directus-extension-programmierbar-bundle/eslint.config.js index 226c6252..05ca958a 100644 --- a/directus-cms/extensions/directus-extension-programmierbar-bundle/eslint.config.js +++ b/directus-cms/extensions/directus-extension-programmierbar-bundle/eslint.config.js @@ -1,9 +1,11 @@ import eslint from '@eslint/js' +import prettierConfig from 'eslint-config-prettier' +import pluginVue from 'eslint-plugin-vue' import tseslint from 'typescript-eslint' export default tseslint.config( { - ignores: ['**/dist/**', '**/podcast-transcription/**', '**/assets/**', 'eslint.config.js', 'jest.config.ts'], + ignores: ['**/dist/**', '**/assets/**', 'eslint.config.js', 'jest.config.ts'], }, eslint.configs.recommended, { @@ -41,5 +43,24 @@ export default tseslint.config( '@typescript-eslint/no-misused-promises': ['error', { checksVoidReturn: false }], }, files: ['**/*.ts'], - } + }, + // The bundle's one Vue SFC (`publishable/presentation-publishable.vue`) was previously the only + // unlinted source file in the tree, because the block above matches `**/*.ts` only. + ...pluginVue.configs['flat/recommended'], + { + files: ['**/*.vue'], + // `vue-eslint-parser` handles the SFC itself and delegates `