From d34f4a1cbae5f0f31ad3baee4a9b6f00a99064cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=A8rejean?= Date: Tue, 5 May 2026 09:02:08 +0200 Subject: [PATCH 01/14] chore: claude init. --- CLAUDE.md | 78 +++++++++++++++++++++++++++++++++++++++++++++++++ docs/.gitignore | 1 + 2 files changed, 79 insertions(+) create mode 100644 CLAUDE.md create mode 100644 docs/.gitignore diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..8f78655 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,78 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## What this is + +A SilverStripe CMS module (`wedevelopnl/silverstripe-menustructure`) that lets editors define multiple named, nested menus in the CMS and render them in templates by slug. PHP 8.1+, SilverStripe CMS `^5`, PSR-4 namespace `WeDevelop\Menustructure\` rooted at `src/`. + +There is no application around this module — the repo *is* the package. There is no `silverstripe/recipe-cms` or kitchen-sink installation here; the Docker image only installs Composer dependencies for tooling (php-cs-fixer). You cannot boot a running CMS from this repo alone. + +## Common commands + +All `make` targets shell into the `php` Docker service when run from the host (the `${docker}` prefix in the Makefile detects this); inside the container they run directly. + +| Command | What it does | +| --- | --- | +| `make build` | Build the Docker image and start detached | +| `make up` / `make down` | Start / stop the dev container | +| `make sh` | Open a shell inside the `php` container | +| `make test` | `php-cs-fixer fix --diff --dry-run` (style check only — there is no PHPUnit suite) | +| `make fix-cs` | Apply php-cs-fixer fixes | +| `make help` | List all targets | + +PHP-CS-Fixer rules (see `.php-cs-fixer.php`): `@PHP81Migration`, `@PSR12`, short array syntax, strict comparison, strict param, `array_push` rule, no unused imports. `declare_strict_types` is intentionally **off** (tracked TODO in the config — re-enabling is paired with adding PHPStan). + +## Architecture + +Three concerns matter here, and they're all small files — read `src/Model/Menu.php` and `src/Model/MenuItem.php` end-to-end before changing anything; the cross-cutting behaviour isn't obvious from the field lists. + +### Models and the rendering entry point + +- **`Menu`** (`Menustructure_Menu` table) — `Title` + `Slug`. Auto-fills `Slug` from `Title` via `URLSegmentFilter` on `onBeforeWrite` *only when empty*; once set, the slug is the stable public handle and is rendered read-only when the menu is "protected" (see below). +- **`MenuItem`** (`Menustructure_MenuItem` table) — self-referential via `ParentItem` (`has_one MenuItem`) and `Items` (`has_many MenuItem`), so menus are arbitrarily nested. Sorted by `Sort` and reordered through `Symbiote\GridFieldExtensions\GridFieldOrderableRows`. +- **`MenusAdmin`** is a thin `ModelAdmin` exposing only `Menu`. All permission checks on both models key off `CMS_ACCESS_WeDevelop\Menustructure\Admin\MenusAdmin` — there is no separate per-model permission code. + +`Menu` implements `TemplateGlobalProvider`, exposing two template helpers: + +- `$MenustructureMenu('slug')` → returns the `Menu` (renders via its default template — `templates/WeDevelop/Menustructure/Model/Menu.ss`). +- `$ViewableMenustructureMenu('slug', 'Path/To/Template')` → renders the matching menu with a custom template. + +Custom templates iterate `$Items` and check `$LinkType != "no-link"` before emitting `` (see the bundled `Menu.ss`). + +### LinkType state machine on `MenuItem` + +`LinkType` is a string enum with four values defined as private constants: `page`, `url`, `file`, `no-link`. The CMS field exposure of every other field is gated by display-logic on `LinkType` (using `UncleCheese\DisplayLogic\Forms\Wrapper`): + +- `page` → shows `LinkedPage` (`SiteTree` tree dropdown), optionally `QueryString` and `AnchorText`. +- `url` → shows `Url`. +- `file` → shows `File` (assets `has_one`, also in `$owns` so it's published with the item). +- `no-link` → renders as `` in the default template; `getLink()` returns `''`. + +`getLink()` is a `match` on `LinkType` that **also** appends `?QueryString` and `#AnchorText` for `page` links — but only when those features are enabled via config (`enable_query_string`, `enable_page_anchor`, both default `false`). The `updateLinkTypes` and `updateLink` extension hooks let downstream modules add new link types or rewrite generated links — preserve those when refactoring. + +### Cascading-write side effect (the non-obvious one) + +`MenuItem::onAfterWrite()` and `onBeforeDelete()` *propagate `LastEdited` upward* to the parent `MenuItem` and to the owning `Menu`. This is intentional: it lets downstream caching (HTTP cache, partial caches keyed on `Menu.LastEdited`) invalidate the whole menu when any descendant item changes. If you refactor write paths, do not break this propagation — there is no test catching it. + +`Menu::onBeforeDelete()` deletes all `Items` directly (rather than relying on `cascade_deletes`); the equivalent `cascade_deletes` config is not set. + +### Protected menus + +Set in YAML to prevent deletion of menus whose `Slug` matches: + +```yaml +WeDevelop\Menustructure\Model\Menu: + protected_menus: + - 'main-menu' + - 'footer' +``` + +`Menu::IsProtected()` flips `canDelete()` to `false` and forces the `Slug` field read-only in CMS edit. Note: `docs/configuration.md` still references the legacy `TheWebmen\Menustructure\Model\Menu` namespace and `templates/TheWebmen/...` path — the actual namespace is `WeDevelop\Menustructure` and the template lives at `templates/WeDevelop/Menustructure/Model/Menu.ss`. Treat the docs as out-of-date until updated. + +## Things to know before editing + +- **No automated tests exist.** The current branch (`feature/ss6-compatibility-and-test-integration`) is set up to introduce a test integration; do not assume `make test` runs PHPUnit. Verify behaviour manually in a host SilverStripe project or add the test harness as part of the change. +- **Branch alias** in `composer.json` maps `dev-main` → `4.x-dev`. The next major (matching SS6 compatibility) will likely be `5.x` — coordinate the alias bump with the release. +- **`composer.lock` is gitignored**, as is `vendor/`. The Dockerfile bakes `composer install` into the image build, but the entrypoint also runs it on container start — local code changes to `composer.json` require `make build` (rebuild) rather than just `make up`. +- **CHANGELOG**: releases are tagged on GitHub; do not maintain `CHANGELOG.md` manually (it points at the GitHub releases page). diff --git a/docs/.gitignore b/docs/.gitignore new file mode 100644 index 0000000..663426a --- /dev/null +++ b/docs/.gitignore @@ -0,0 +1 @@ +/superpowers/ From 404febdb432279dce3e068ba8471f716210e5efe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=A8rejean?= Date: Tue, 5 May 2026 09:18:23 +0200 Subject: [PATCH 02/14] chore: replace dev infra with FrankenPHP stack and add CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the legacy single-container php-cs-fixer-only Docker setup with the FrankenPHP + MySQL stack pattern used in silverstripe-media-field and silverstripe-grid. Adds GitHub Actions for code-style, static analysis, and a PHPUnit matrix; adds dependabot, .gitattributes export-ignore, and .dockerignore. Module composer.json is intentionally untouched — SS6 compat and the PHPUnit suite are followup tasks. The dev env and CI are pinned to PHP 8.2+ because FrankenPHP doesn't publish a PHP 8.1 image. --- .docker/Caddyfile | 4 ++ .docker/Dockerfile | 37 ++++++++++++ .docker/app/_config/.gitkeep | 0 .docker/app/composer.json | 56 ++++++++++++++++++ .docker/app/phpstan.neon.dist | 5 ++ .docker/app/phpunit.xml.dist | 18 ++++++ .docker/compose.yml | 56 ++++++++++++++++++ .docker/db-init/grant-test-privileges.sql | 4 ++ .docker/entrypoint.sh | 26 ++++++++ .docker/env.sh | 23 ++++++++ .dockerignore | 18 ++++++ .gitattributes | 9 +++ .github/dependabot.yml | 17 ++++++ .github/workflows/ci.yml | 67 +++++++++++++++++++++ .gitignore | 12 ++-- .php-cs-fixer.php | 5 +- CLAUDE.md | 31 ++++++---- Dockerfile | 22 ------- Makefile | 72 +++++++++++++++-------- dev/docker/docker-entrypoint.sh | 14 ----- docker-compose.yml | 11 ---- tests/.gitkeep | 0 22 files changed, 419 insertions(+), 88 deletions(-) create mode 100644 .docker/Caddyfile create mode 100644 .docker/Dockerfile create mode 100644 .docker/app/_config/.gitkeep create mode 100644 .docker/app/composer.json create mode 100644 .docker/app/phpstan.neon.dist create mode 100644 .docker/app/phpunit.xml.dist create mode 100644 .docker/compose.yml create mode 100644 .docker/db-init/grant-test-privileges.sql create mode 100755 .docker/entrypoint.sh create mode 100755 .docker/env.sh create mode 100644 .dockerignore create mode 100644 .gitattributes create mode 100644 .github/dependabot.yml create mode 100644 .github/workflows/ci.yml delete mode 100644 Dockerfile delete mode 100644 dev/docker/docker-entrypoint.sh delete mode 100644 docker-compose.yml create mode 100644 tests/.gitkeep diff --git a/.docker/Caddyfile b/.docker/Caddyfile new file mode 100644 index 0000000..d7d8730 --- /dev/null +++ b/.docker/Caddyfile @@ -0,0 +1,4 @@ +localhost { + root * /app/public + php_server +} diff --git a/.docker/Dockerfile b/.docker/Dockerfile new file mode 100644 index 0000000..2fac906 --- /dev/null +++ b/.docker/Dockerfile @@ -0,0 +1,37 @@ +ARG PHP_VERSION=8.2 +FROM dunglas/frankenphp:php${PHP_VERSION} + +RUN install-php-extensions \ + intl \ + gd \ + mysqli \ + pdo_mysql \ + zip \ + bcmath \ + exif \ + pcov + +RUN echo 'pcov.directory=/module/src' > /usr/local/etc/php/conf.d/99-pcov.ini + +COPY --from=composer:latest /usr/bin/composer /usr/bin/composer + +WORKDIR /app + +COPY app/composer.json /app/composer.json + +# SilverStripe requires Page and PageController classes in the project. +# Generated here (not stored in the repo) to avoid duplicate-class detection +# when the module is symlinked into vendor/. +RUN mkdir -p /app/src /app/_config \ + && printf ' /app/src/Page.php \ + && printf ' /app/src/PageController.php + +COPY app/_config/ /app/_config/ +COPY app/phpunit.xml.dist /app/phpunit.xml.dist +COPY app/phpstan.neon.dist /app/phpstan.neon.dist + +COPY Caddyfile /etc/caddy/Caddyfile +COPY entrypoint.sh /usr/local/bin/entrypoint.sh +RUN chmod +x /usr/local/bin/entrypoint.sh + +ENTRYPOINT ["entrypoint.sh"] diff --git a/.docker/app/_config/.gitkeep b/.docker/app/_config/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/.docker/app/composer.json b/.docker/app/composer.json new file mode 100644 index 0000000..88b9133 --- /dev/null +++ b/.docker/app/composer.json @@ -0,0 +1,56 @@ +{ + "minimum-stability": "dev", + "prefer-stable": true, + "require": { + "silverstripe/recipe-cms": "^5.0", + "wedevelopnl/silverstripe-menustructure": "*" + }, + "repositories": [ + { + "type": "path", + "url": "/module", + "options": { + "symlink": true + } + } + ], + "autoload": { + "classmap": [ + "src/" + ] + }, + "require-dev": { + "friendsofphp/php-cs-fixer": "^3.50", + "phpstan/extension-installer": "^1.4", + "phpstan/phpstan": "^2.1", + "phpunit/phpunit": "^9.6" + }, + "autoload-dev": { + "psr-4": { + "WeDevelop\\Menustructure\\Tests\\": "vendor/wedevelopnl/silverstripe-menustructure/tests/" + } + }, + "extra": { + "project-files-installed": [ + ".htaccess", + "app/.htaccess", + "app/_config/mimevalidator.yml", + "app/_config/mysite.yml", + "app/src/Page.php", + "app/src/PageController.php" + ], + "public-files-installed": [ + ".htaccess", + "index.php", + "web.config" + ] + }, + "config": { + "allow-plugins": { + "composer/installers": true, + "phpstan/extension-installer": true, + "silverstripe/recipe-plugin": true, + "silverstripe/vendor-plugin": true + } + } +} diff --git a/.docker/app/phpstan.neon.dist b/.docker/app/phpstan.neon.dist new file mode 100644 index 0000000..d6e1bc9 --- /dev/null +++ b/.docker/app/phpstan.neon.dist @@ -0,0 +1,5 @@ +parameters: + level: 9 + paths: + - /module/src + treatPhpDocTypesAsCertain: false diff --git a/.docker/app/phpunit.xml.dist b/.docker/app/phpunit.xml.dist new file mode 100644 index 0000000..8a778bd --- /dev/null +++ b/.docker/app/phpunit.xml.dist @@ -0,0 +1,18 @@ + + + + + vendor/wedevelopnl/silverstripe-menustructure/tests + + + + + vendor/wedevelopnl/silverstripe-menustructure/src + + + diff --git a/.docker/compose.yml b/.docker/compose.yml new file mode 100644 index 0000000..4af544d --- /dev/null +++ b/.docker/compose.yml @@ -0,0 +1,56 @@ +services: + app: + build: + context: . + dockerfile: Dockerfile + args: + PHP_VERSION: ${PHP_VERSION:-8.2} + ports: + - "${WEB_PORT}:443" + volumes: + - ../composer.json:/module/composer.json:ro + - ../.php-cs-fixer.php:/module/.php-cs-fixer.php:ro + - ../src:/module/src + - ../tests:/module/tests + - ../_config:/module/_config:ro + - ../templates:/module/templates:ro + - ../coverage:/app/coverage + - vendor:/app/vendor + healthcheck: + test: test -f /tmp/.app-ready + interval: 3s + start_period: 60s + retries: 20 + depends_on: + db: + condition: service_healthy + environment: + SS_DATABASE_SERVER: db + SS_DATABASE_NAME: silverstripe + SS_DATABASE_USERNAME: silverstripe + SS_DATABASE_PASSWORD: silverstripe + SS_DEFAULT_ADMIN_USERNAME: admin + SS_DEFAULT_ADMIN_PASSWORD: admin + SS_ENVIRONMENT_TYPE: dev + SS_PHPUNIT_FLUSH: 1 + + db: + image: mysql:8 + ports: + - "${DB_PORT}:3306" + volumes: + - db-data:/var/lib/mysql + - ./db-init:/docker-entrypoint-initdb.d:ro + environment: + MYSQL_DATABASE: silverstripe + MYSQL_USER: silverstripe + MYSQL_PASSWORD: silverstripe + MYSQL_ROOT_PASSWORD: root + healthcheck: + test: mysqladmin ping -h localhost + interval: 5s + retries: 10 + +volumes: + vendor: + db-data: diff --git a/.docker/db-init/grant-test-privileges.sql b/.docker/db-init/grant-test-privileges.sql new file mode 100644 index 0000000..d4adf9b --- /dev/null +++ b/.docker/db-init/grant-test-privileges.sql @@ -0,0 +1,4 @@ +-- Grant CREATE/DROP DATABASE privileges for SilverStripe's test framework +-- which creates temporary databases matching ss_tmpdb_* +GRANT ALL PRIVILEGES ON *.* TO 'silverstripe'@'%'; +FLUSH PRIVILEGES; diff --git a/.docker/entrypoint.sh b/.docker/entrypoint.sh new file mode 100755 index 0000000..677aa1d --- /dev/null +++ b/.docker/entrypoint.sh @@ -0,0 +1,26 @@ +#!/bin/sh +set -e + +composer install --no-interaction + +# FrankenPHP ships a default phpinfo() index.php. Replace it with the proper +# SilverStripe bootstrap after composer install makes the recipe available. +cp -f vendor/silverstripe/recipe-core/public/index.php /app/public/index.php + +# Ensure all vendor package resources are exposed. composer install skips the +# vendor-expose step when the named Docker volume already has packages from a +# previous run (nothing new to install → no post-install event fires). +composer vendor-expose + +# vendor-plugin uses realpath() to resolve library paths, which follows the +# symlink from vendor/wedevelopnl/silverstripe-menustructure → /module. +# Because /module is outside /app, getRelativePath() produces a broken path +# and the exposed resources are never created. Create them manually. +_res=/app/public/_resources/vendor/wedevelopnl/silverstripe-menustructure +mkdir -p "$_res" + +vendor/bin/sake dev/build flush=1 + +touch /tmp/.app-ready + +exec frankenphp run --config /etc/caddy/Caddyfile diff --git a/.docker/env.sh b/.docker/env.sh new file mode 100755 index 0000000..a266a9b --- /dev/null +++ b/.docker/env.sh @@ -0,0 +1,23 @@ +#!/bin/sh +set -e + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +WORKTREE_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" +DIR_NAME="$(basename "$WORKTREE_DIR")" + +HASH=$(printf '%s' "$DIR_NAME" | cksum | awk '{print $1}') +OFFSET=$((HASH % 1000)) + +WEB_PORT=$((8000 + OFFSET)) +DB_PORT=$((13000 + OFFSET)) + +cat > "$SCRIPT_DIR/.env" <- + $COMPOSE exec app vendor/bin/phpunit + --log-junit /app/coverage/phpunit-junit.xml + name: Run PHPUnit + - run: cp coverage/phpunit-junit.xml reports/phpunit-junit.xml + if: always() + - uses: dorny/test-reporter@v3 + if: always() + with: + name: PHPUnit Results (PHP ${{ matrix.php }}) + path: reports/phpunit-junit.xml + reporter: java-junit diff --git a/.gitignore b/.gitignore index 506e5de..329cc81 100755 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,10 @@ +vendor +composer.lock +.php-cs-fixer.cache +.phpunit.cache +.idea +.vscode +.docker/.env +coverage /**/*.js.map /**/*.css.map -/vendor/ -/composer.lock -/.idea/ -.php-cs-fixer.cache diff --git a/.php-cs-fixer.php b/.php-cs-fixer.php index 8603f16..ff9852d 100644 --- a/.php-cs-fixer.php +++ b/.php-cs-fixer.php @@ -1,7 +1,10 @@ in(__DIR__ . '/src') + ->in([ + __DIR__ . '/src', + __DIR__ . '/tests', + ]) ; $config = new PhpCsFixer\Config(); diff --git a/CLAUDE.md b/CLAUDE.md index 8f78655..8c609df 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,22 +6,25 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co A SilverStripe CMS module (`wedevelopnl/silverstripe-menustructure`) that lets editors define multiple named, nested menus in the CMS and render them in templates by slug. PHP 8.1+, SilverStripe CMS `^5`, PSR-4 namespace `WeDevelop\Menustructure\` rooted at `src/`. -There is no application around this module — the repo *is* the package. There is no `silverstripe/recipe-cms` or kitchen-sink installation here; the Docker image only installs Composer dependencies for tooling (php-cs-fixer). You cannot boot a running CMS from this repo alone. +The repo *is* the package — there is no surrounding application. The dev environment under `.docker/` builds a throwaway SilverStripe app (recipe-cms `^5`) that mounts the module at `/module` and pulls it in via a path repository, modelled on `silverstripe-grid/.docker/` and `silverstripe-media-field/.docker/`. SS6 compat is the next branch milestone — the dev infra is intentionally pinned to SS5 until that work begins. ## Common commands -All `make` targets shell into the `php` Docker service when run from the host (the `${docker}` prefix in the Makefile detects this); inside the container they run directly. - | Command | What it does | | --- | --- | -| `make build` | Build the Docker image and start detached | -| `make up` / `make down` | Start / stop the dev container | -| `make sh` | Open a shell inside the `php` container | -| `make test` | `php-cs-fixer fix --diff --dry-run` (style check only — there is no PHPUnit suite) | +| `make build` | Build the dev container without starting | +| `make up` | Start FrankenPHP + MySQL stack (auto-generates `.docker/.env`, builds if needed) | +| `make down` / `make destroy` | Stop services / stop and wipe volumes | +| `make sh` | Open a shell inside the `app` container | +| `make test` | Run PHPUnit (`.docker/app/phpunit.xml.dist`) | +| `make analyse` | Run PHPStan at level 9 | +| `make test-cs` | `php-cs-fixer fix --diff --dry-run` | | `make fix-cs` | Apply php-cs-fixer fixes | -| `make help` | List all targets | +| `make flush` / `make dev-build` | `sake flush` / `sake dev/build flush=1` | + +`make` targets pass through to `docker compose -f .docker/compose.yml exec app …`. `ensure-up` is a dependency on tool targets that brings the stack up if it's not already running. -PHP-CS-Fixer rules (see `.php-cs-fixer.php`): `@PHP81Migration`, `@PSR12`, short array syntax, strict comparison, strict param, `array_push` rule, no unused imports. `declare_strict_types` is intentionally **off** (tracked TODO in the config — re-enabling is paired with adding PHPStan). +PHP-CS-Fixer rules (see `.php-cs-fixer.php`): `@PHP81Migration`, `@PSR12`, short array syntax, strict comparison, strict param, `array_push` rule, no unused imports. `declare_strict_types` is intentionally **off** (tracked TODO in the config — re-enabling is paired with the PHPStan rollout). ## Architecture @@ -72,7 +75,13 @@ WeDevelop\Menustructure\Model\Menu: ## Things to know before editing -- **No automated tests exist.** The current branch (`feature/ss6-compatibility-and-test-integration`) is set up to introduce a test integration; do not assume `make test` runs PHPUnit. Verify behaviour manually in a host SilverStripe project or add the test harness as part of the change. +- **This branch (`feature/ss6-compatibility-and-test-integration`) introduces the dev infra only.** SS6 compatibility, PHPUnit test suite, and additional QA tooling (PHPStan rollout, type coverage, etc.) are followup tasks. `composer.json` is unchanged from `main` — still SS5/PHP 8.1. +- **Tests directory exists but is empty.** `make test` runs PHPUnit against `tests/` (mounted into the container). Adding the first test class is the followup task — until then PHPUnit will report "no tests executed". - **Branch alias** in `composer.json` maps `dev-main` → `4.x-dev`. The next major (matching SS6 compatibility) will likely be `5.x` — coordinate the alias bump with the release. -- **`composer.lock` is gitignored**, as is `vendor/`. The Dockerfile bakes `composer install` into the image build, but the entrypoint also runs it on container start — local code changes to `composer.json` require `make build` (rebuild) rather than just `make up`. +- **`composer.lock` is gitignored**, as is `vendor/`. The `entrypoint.sh` runs `composer install` on container start and re-runs `vendor-expose`; local code changes to `composer.json` need `make build` (or `make destroy && make up`) to rebuild the image cleanly. +- **Distribution**: `.gitattributes` marks `/.docker`, `/.github`, `/Makefile`, `/.php-cs-fixer.php`, `/docs`, `/tests`, etc. as `export-ignore` so they don't ship in Packagist tarballs. When adding new dev-only files at the root, add a matching `export-ignore` entry. - **CHANGELOG**: releases are tagged on GitHub; do not maintain `CHANGELOG.md` manually (it points at the GitHub releases page). + +## CI + +`.github/workflows/ci.yml` runs three jobs on `main` branch pushes/PRs: `code-style` (php-cs-fixer dry-run on the host), `static-analysis` (PHPStan via `make analyse`), and a `phpunit` matrix across PHP 8.2/8.3. PHP 8.1 isn't tested even though `composer.json` declares `php: ^8.1` — FrankenPHP doesn't ship a PHP 8.1 image. `.github/dependabot.yml` watches `composer`, the `.docker/` Dockerfile, and GitHub Actions versions. diff --git a/Dockerfile b/Dockerfile deleted file mode 100644 index 0de823c..0000000 --- a/Dockerfile +++ /dev/null @@ -1,22 +0,0 @@ -ARG ALPINE_VERSION=3.16 - -ARG NODE_VERSION=18.12.1 - -FROM node:$NODE_VERSION-alpine$ALPINE_VERSION AS node -FROM php:8.1-cli-alpine$ALPINE_VERSION AS php-cli - -RUN apk add php make perl --no-cache - -WORKDIR /app - -COPY --from=composer:2 /usr/bin/composer /usr/bin/composer -COPY composer.json ./ -ENV COMPOSER_ALLOW_SUPERUSER 1 -RUN composer install --prefer-dist --no-progress --no-suggest --no-interaction --no-plugins --ignore-platform-reqs - -COPY dev/docker/docker-entrypoint.sh /usr/local/bin/docker-entrypoint -RUN chmod +x /usr/local/bin/docker-entrypoint - -ENTRYPOINT ["docker-entrypoint"] - -CMD ["php"] diff --git a/Makefile b/Makefile index a448a8e..bb96fb5 100644 --- a/Makefile +++ b/Makefile @@ -1,34 +1,56 @@ -.PHONY: * +COMPOSE := docker compose -f .docker/compose.yml -.DEFAULT_GOAL := help -docker := $(shell if [ `pwd` != "/app" ]; then echo 'docker compose exec php'; fi;) +.PHONY: up down destroy build ensure-up test analyse test-cs fix-cs flush dev-build sh -HELP_FUNCTION = \ - %help; \ - while(<>) { push @{$$help{$$2 // 'options'}}, [$$1, $$3] if /^(.+)\s*:.*\#\#(?:@(\w+))?\s(.*)$$/ }; \ - print "usage: make [target]\n\n"; \ - for (keys %help) { \ - print "\033[33m $$_:\n"; \ - printf " \033[32m%-30s\033[0m %s\n", $$_->[0], $$_->[1] for @{$$help{$$_}} \ - } +## Generate .docker/.env with deterministic ports (auto-runs if missing) +.docker/.env: + .docker/env.sh -help: ##@develop Show this help. - @perl -e '$(HELP_FUNCTION)' $(MAKEFILE_LIST) +## Start services (build if needed) +up: .docker/.env + $(COMPOSE) up -d --build + @echo "\n Testbed running at https://localhost:$$(grep WEB_PORT .docker/.env | cut -d= -f2)\n" -build: ##@develop Build docker container and detach - docker compose up --build -d +## Stop services +down: + $(COMPOSE) down -up: ##@develop Docker compose up - docker compose up -d +## Stop services and remove volumes +destroy: + $(COMPOSE) down -v -down: ##@develop Docker compose down - docker compose down +## Build images without starting +build: + $(COMPOSE) build -test: ##@develop Run code style test - ${docker} ./vendor/bin/php-cs-fixer fix --diff --dry-run +## Ensure services are running and ready +ensure-up: .docker/.env + @$(COMPOSE) exec app true 2>/dev/null || $(COMPOSE) up -d --build --wait -fix-cs: ##@develop Fix code styling - ${docker} ./vendor/bin/php-cs-fixer fix +## Run PHPUnit +test: ensure-up + $(COMPOSE) exec app vendor/bin/phpunit -sh: ##@develop Open shell in container - ${docker} sh +## Run PHPStan static analysis +analyse: ensure-up + $(COMPOSE) exec app vendor/bin/phpstan analyse -c phpstan.neon.dist --memory-limit=512M + +## Run php-cs-fixer dry-run (was: `make test` in the old setup) +test-cs: ensure-up + $(COMPOSE) exec app sh -c 'cd /module && /app/vendor/bin/php-cs-fixer fix --diff --dry-run --config=.php-cs-fixer.php' + +## Auto-fix code style +fix-cs: ensure-up + $(COMPOSE) exec app sh -c 'cd /module && /app/vendor/bin/php-cs-fixer fix --config=.php-cs-fixer.php' + +## Clear SilverStripe cache +flush: ensure-up + $(COMPOSE) exec app vendor/bin/sake flush + +## Run dev/build to rebuild the database and manifest +dev-build: ensure-up + $(COMPOSE) exec app vendor/bin/sake dev/build flush=1 + +## Open shell in the app container +sh: ensure-up + $(COMPOSE) exec app sh diff --git a/dev/docker/docker-entrypoint.sh b/dev/docker/docker-entrypoint.sh deleted file mode 100644 index 764e179..0000000 --- a/dev/docker/docker-entrypoint.sh +++ /dev/null @@ -1,14 +0,0 @@ -#!/bin/sh -set -e - -# first arg is `-f` or `--some-option` -if [ "${1#-}" != "$1" ]; then - set -- php "$@" -fi - -if [ "$1" = 'php' ]; then - composer install --prefer-dist --no-progress --no-suggest --no-interaction --no-plugins --ignore-platform-reqs - echo "Container is ready!" -fi - -exec docker-php-entrypoint "$@" diff --git a/docker-compose.yml b/docker-compose.yml deleted file mode 100644 index a794b0d..0000000 --- a/docker-compose.yml +++ /dev/null @@ -1,11 +0,0 @@ -version: '3.6' -services: - php: - build: - context: . - dockerfile: Dockerfile - target: php-cli - working_dir: /app - volumes: - - .:/app/ - tty: true \ No newline at end of file diff --git a/tests/.gitkeep b/tests/.gitkeep new file mode 100644 index 0000000..e69de29 From 6e9e7bf986c08a65de03b6aae06ce3de29d3f89b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=A8rejean?= Date: Tue, 5 May 2026 10:02:15 +0200 Subject: [PATCH 03/14] test: add Menu and MenuItem PHPUnit suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Establishes the module's first tests on top of the FrankenPHP dev infra, covering both DataObjects' behavior — slug auto-fill, protected_menus config gating, permission delegation, getLink across all LinkType branches (with QueryString/AnchorText config flips), LinkingMode, getLevel, and LastEdited cascade on write/delete. Adds a `coverage` make target and declares the package as silverstripe-vendormodule so SilverStripe's class manifest scans the module's tests/ in test mode. --- Makefile | 9 +- composer.json | 1 + tests/Model/MenuItemTest.php | 286 ++++++++++++++++++++++++++++++++ tests/Model/MenuTest.php | 169 +++++++++++++++++++ tests/fixtures/MenuItemTest.yml | 65 ++++++++ tests/fixtures/MenuTest.yml | 21 +++ 6 files changed, 550 insertions(+), 1 deletion(-) create mode 100644 tests/Model/MenuItemTest.php create mode 100644 tests/Model/MenuTest.php create mode 100644 tests/fixtures/MenuItemTest.yml create mode 100644 tests/fixtures/MenuTest.yml diff --git a/Makefile b/Makefile index bb96fb5..715439e 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ COMPOSE := docker compose -f .docker/compose.yml -.PHONY: up down destroy build ensure-up test analyse test-cs fix-cs flush dev-build sh +.PHONY: up down destroy build ensure-up test coverage analyse test-cs fix-cs flush dev-build sh ## Generate .docker/.env with deterministic ports (auto-runs if missing) .docker/.env: @@ -31,6 +31,13 @@ ensure-up: .docker/.env test: ensure-up $(COMPOSE) exec app vendor/bin/phpunit +## Run PHPUnit with code coverage (text summary on stdout, HTML at coverage/html, clover XML for CI) +coverage: ensure-up + $(COMPOSE) exec app vendor/bin/phpunit \ + --coverage-text \ + --coverage-html /app/coverage/html \ + --coverage-clover /app/coverage/clover.xml + ## Run PHPStan static analysis analyse: ensure-up $(COMPOSE) exec app vendor/bin/phpstan analyse -c phpstan.neon.dist --memory-limit=512M diff --git a/composer.json b/composer.json index 0b17d6d..d7d5ca8 100644 --- a/composer.json +++ b/composer.json @@ -1,5 +1,6 @@ { "name": "wedevelopnl/silverstripe-menustructure", + "type": "silverstripe-vendormodule", "description": "Silverstripe module to create nested menus", "keywords": [ "silverstripe", diff --git a/tests/Model/MenuItemTest.php b/tests/Model/MenuItemTest.php new file mode 100644 index 0000000..fbd1828 --- /dev/null +++ b/tests/Model/MenuItemTest.php @@ -0,0 +1,286 @@ +objFromFixture(MenuItem::class, 'topLevel'); + + $this->assertSame('https://example.com', $item->getLink()); + } + + public function testGetLinkReturnsLinkedPageLinkForPageType(): void + { + $item = $this->objFromFixture(MenuItem::class, 'pageLink'); + $page = $this->objFromFixture(SiteTree::class, 'homepage'); + + $this->assertSame($page->Link(), $item->getLink()); + } + + public function testGetLinkReturnsEmptyStringForNoLinkType(): void + { + $item = $this->objFromFixture(MenuItem::class, 'emptyLink'); + + $this->assertSame('', $item->getLink()); + } + + public function testGetLinkReturnsFileLinkForFileType(): void + { + $item = $this->objFromFixture(MenuItem::class, 'fileLink'); + + // getLink() coerces null → '' via the trailing `?? ''`, so normalise the + // expected value the same way to keep the test independent of whether + // the asset has a published Link() in the test environment. + $this->assertSame((string)$item->File()->Link(), $item->getLink()); + } + + public function testGetLinkAppendsQueryStringWhenEnabledForPageType(): void + { + Config::modify()->set(MenuItem::class, 'enable_query_string', true); + $item = $this->objFromFixture(MenuItem::class, 'pageLink'); + $page = $this->objFromFixture(SiteTree::class, 'homepage'); + + $this->assertSame($page->Link() . '?utm=home', $item->getLink()); + } + + public function testGetLinkOmitsQueryStringWhenConfigDisabled(): void + { + Config::modify()->set(MenuItem::class, 'enable_query_string', false); + $item = $this->objFromFixture(MenuItem::class, 'pageLink'); + $page = $this->objFromFixture(SiteTree::class, 'homepage'); + + $this->assertSame($page->Link(), $item->getLink()); + } + + public function testGetLinkOmitsQueryStringWhenLinkTypeIsNotPage(): void + { + Config::modify()->set(MenuItem::class, 'enable_query_string', true); + + // Force a QueryString onto a URL-type item to prove the gate is on LinkType. + $item = $this->objFromFixture(MenuItem::class, 'topLevel'); + $item->QueryString = 'utm=foo'; + + $this->assertSame('https://example.com', $item->getLink()); + } + + public function testGetLinkAppendsAnchorWhenEnabledForPageType(): void + { + Config::modify()->set(MenuItem::class, 'enable_page_anchor', true); + $item = $this->objFromFixture(MenuItem::class, 'pageLink'); + $page = $this->objFromFixture(SiteTree::class, 'homepage'); + + $this->assertSame($page->Link() . '#top', $item->getLink()); + } + + public function testGetLinkOmitsAnchorWhenConfigDisabled(): void + { + Config::modify()->set(MenuItem::class, 'enable_page_anchor', false); + $item = $this->objFromFixture(MenuItem::class, 'pageLink'); + $page = $this->objFromFixture(SiteTree::class, 'homepage'); + + $this->assertSame($page->Link(), $item->getLink()); + } + + public function testGetLinkAppendsBothQueryStringAndAnchorWhenEnabled(): void + { + Config::modify()->set(MenuItem::class, 'enable_query_string', true); + Config::modify()->set(MenuItem::class, 'enable_page_anchor', true); + $item = $this->objFromFixture(MenuItem::class, 'pageLink'); + $page = $this->objFromFixture(SiteTree::class, 'homepage'); + + $this->assertSame($page->Link() . '?utm=home#top', $item->getLink()); + } + + public function testLinkingModeReturnsCurrentWhenCurrentControllerIDMatchesLinkedPage(): void + { + $item = $this->objFromFixture(MenuItem::class, 'pageLink'); + + $this->withCurrentController($item->LinkedPageID, function () use ($item) { + $this->assertSame('current', $item->LinkingMode()); + }); + } + + public function testLinkingModeReturnsLinkWhenCurrentControllerIDDoesNotMatchLinkedPage(): void + { + $item = $this->objFromFixture(MenuItem::class, 'pageLink'); + + $this->withCurrentController($item->LinkedPageID + 999, function () use ($item) { + $this->assertSame('link', $item->LinkingMode()); + }); + } + + public function testLinkingModeReturnsLinkForNonPageTypeWithoutTouchingCurrentController(): void + { + // The non-page branch must short-circuit before reading Controller::curr(). + // Asserting it works with no controller pushed pins down that contract — + // a refactor that removed the early return would surface here. + $item = $this->objFromFixture(MenuItem::class, 'topLevel'); + + $this->assertSame('link', $item->LinkingMode()); + } + + public function testGetLevelIsZeroForTopLevelItem(): void + { + $item = $this->objFromFixture(MenuItem::class, 'topLevel'); + + $this->assertSame(0, $item->getLevel()); + } + + public function testGetLevelCountsParentChainDepth(): void + { + $child = $this->objFromFixture(MenuItem::class, 'childLevel'); + $grandchild = $this->objFromFixture(MenuItem::class, 'grandchildLevel'); + + $this->assertSame(1, $child->getLevel()); + $this->assertSame(2, $grandchild->getLevel()); + } + + public function testWritingItemBumpsOwningMenuLastEdited(): void + { + $item = $this->objFromFixture(MenuItem::class, 'topLevel'); + $menu = $this->objFromFixture(Menu::class, 'default'); + + $this->stampLastEditedBackward(Menu::class, $menu->ID); + $beforeMenu = Menu::get()->byID($menu->ID); + $this->assertSame(self::PAST_DATETIME, $beforeMenu->LastEdited); + + $item->Title = 'Updated title'; + $item->write(); + + $afterMenu = Menu::get()->byID($menu->ID); + $this->assertNotSame(self::PAST_DATETIME, $afterMenu->LastEdited); + } + + public function testWritingItemBumpsParentItemLastEdited(): void + { + $child = $this->objFromFixture(MenuItem::class, 'childLevel'); + $parent = $this->objFromFixture(MenuItem::class, 'topLevel'); + + $this->stampLastEditedBackward(MenuItem::class, $parent->ID); + $beforeParent = MenuItem::get()->byID($parent->ID); + $this->assertSame(self::PAST_DATETIME, $beforeParent->LastEdited); + + $child->Title = 'Updated title'; + $child->write(); + + $afterParent = MenuItem::get()->byID($parent->ID); + $this->assertNotSame(self::PAST_DATETIME, $afterParent->LastEdited); + } + + public function testDeletingItemBumpsOwningMenuLastEdited(): void + { + $item = $this->objFromFixture(MenuItem::class, 'emptyLink'); + $menu = $this->objFromFixture(Menu::class, 'default'); + + $this->stampLastEditedBackward(Menu::class, $menu->ID); + $beforeMenu = Menu::get()->byID($menu->ID); + $this->assertSame(self::PAST_DATETIME, $beforeMenu->LastEdited); + + $item->delete(); + + $afterMenu = Menu::get()->byID($menu->ID); + $this->assertNotSame(self::PAST_DATETIME, $afterMenu->LastEdited); + } + + public function testDeletingItemBumpsParentItemLastEdited(): void + { + $child = $this->objFromFixture(MenuItem::class, 'grandchildLevel'); + $parent = $this->objFromFixture(MenuItem::class, 'childLevel'); + + $this->stampLastEditedBackward(MenuItem::class, $parent->ID); + $beforeParent = MenuItem::get()->byID($parent->ID); + $this->assertSame(self::PAST_DATETIME, $beforeParent->LastEdited); + + $child->delete(); + + $afterParent = MenuItem::get()->byID($parent->ID); + $this->assertNotSame(self::PAST_DATETIME, $afterParent->LastEdited); + } + + /** + * @dataProvider permissionMatrix + */ + public function testCanMethodHonoursAdminPermission(string $permission, string $method, bool $expected): void + { + $this->logInWithPermission($permission); + + $item = $this->objFromFixture(MenuItem::class, 'topLevel'); + + $this->assertSame($expected, $item->$method()); + } + + public static function permissionMatrix(): array + { + return [ + 'canCreate allowed with admin permission' => [self::ADMIN_PERMISSION, 'canCreate', true], + 'canCreate denied without admin permission' => ['SOME_OTHER_PERMISSION', 'canCreate', false], + 'canView allowed with admin permission' => [self::ADMIN_PERMISSION, 'canView', true], + 'canView falls through to parent without admin permission' => ['SOME_OTHER_PERMISSION', 'canView', false], + 'canEdit allowed with admin permission' => [self::ADMIN_PERMISSION, 'canEdit', true], + 'canEdit falls through to parent without admin permission' => ['SOME_OTHER_PERMISSION', 'canEdit', false], + 'canDelete allowed with admin permission' => [self::ADMIN_PERMISSION, 'canDelete', true], + 'canDelete falls through to parent without admin permission' => ['SOME_OTHER_PERMISSION', 'canDelete', false], + ]; + } + + /** + * Sets LastEdited to a known past timestamp via raw SQL so that we can + * detect cascade-write/delete propagation without relying on sleep(). + */ + private function stampLastEditedBackward(string $class, int $id): void + { + $tableName = Config::inst()->get($class, 'table_name'); + DB::prepared_query( + sprintf('UPDATE "%s" SET "LastEdited" = ? WHERE "ID" = ?', $tableName), + [self::PAST_DATETIME, $id] + ); + } + + /** + * Pushes a real Controller onto the stack with a real request and session + * so that `Controller::curr()->ID` resolves to the given $id while $body + * runs. This is the same primitive the framework uses at runtime — we're + * not mocking, we're configuring framework state for the test. + * + * pushCurrent() needs the controller's request to expose a session, so we + * attach one. popCurrent() runs in finally to keep the stack balanced even + * when the assertion throws. + */ + private function withCurrentController(int $id, callable $body): void + { + $request = new HTTPRequest('GET', '/'); + $request->setSession(new Session([])); + + $controller = Controller::create(); + $controller->setRequest($request); + $controller->ID = $id; + + $controller->pushCurrent(); + try { + $body(); + } finally { + $controller->popCurrent(); + } + } +} diff --git a/tests/Model/MenuTest.php b/tests/Model/MenuTest.php new file mode 100644 index 0000000..597f947 --- /dev/null +++ b/tests/Model/MenuTest.php @@ -0,0 +1,169 @@ +Title = 'Footer Links'; + $menu->write(); + + $this->assertSame('footer-links', $menu->Slug); + } + + public function testSlugIsNotOverwrittenWhenAlreadySet(): void + { + $menu = Menu::create(); + $menu->Title = 'Footer Links'; + $menu->Slug = 'custom-handle'; + $menu->write(); + + $this->assertSame('custom-handle', $menu->Slug); + } + + public function testSlugFiltersSpecialCharacters(): void + { + $menu = Menu::create(); + $menu->Title = 'Hôtel & Café (2026)!'; + $menu->write(); + + // URLSegmentFilter normalises to lowercase, ascii-folded, hyphenated. + $this->assertNotEmpty($menu->Slug); + $this->assertDoesNotMatchRegularExpression('/[ &()!]/', $menu->Slug); + $this->assertSame(strtolower($menu->Slug), $menu->Slug); + } + + /** + * @dataProvider isProtectedProvider + */ + public function testIsProtectedHonoursProtectedMenusConfig(array $protectedMenus, bool $expected): void + { + Config::modify()->set(Menu::class, 'protected_menus', $protectedMenus); + + $menu = $this->objFromFixture(Menu::class, 'protected'); + + $this->assertSame($expected, $menu->IsProtected()); + } + + public static function isProtectedProvider(): array + { + return [ + "slug appears in protected_menus → IsProtected" => [['main-menu'], true], + "slug absent from protected_menus → not IsProtected" => [['something-else'], false], + "empty protected_menus → not IsProtected" => [[], false], + ]; + } + + public function testCanDeleteIsBlockedWhenMenuIsProtected(): void + { + Config::modify()->set(Menu::class, 'protected_menus', ['main-menu']); + $this->logInWithPermission(self::ADMIN_PERMISSION); + + $menu = $this->objFromFixture(Menu::class, 'protected'); + + $this->assertFalse($menu->canDelete()); + } + + /** + * @dataProvider permissionMatrix + */ + public function testCanMethodHonoursAdminPermission(string $permission, string $method, bool $expected): void + { + // Keep canDelete out of the protected-menu gate; the protected case is + // covered by testCanDeleteIsBlockedWhenMenuIsProtected. + Config::modify()->set(Menu::class, 'protected_menus', []); + $this->logInWithPermission($permission); + + $menu = $this->objFromFixture(Menu::class, 'primary'); + + $this->assertSame($expected, $menu->$method()); + } + + public static function permissionMatrix(): array + { + return [ + 'canCreate allowed with admin permission' => [self::ADMIN_PERMISSION, 'canCreate', true], + 'canCreate denied without admin permission' => ['SOME_OTHER_PERMISSION', 'canCreate', false], + 'canView allowed with admin permission' => [self::ADMIN_PERMISSION, 'canView', true], + 'canView falls through to parent without admin permission' => ['SOME_OTHER_PERMISSION', 'canView', false], + 'canEdit allowed with admin permission' => [self::ADMIN_PERMISSION, 'canEdit', true], + 'canEdit falls through to parent without admin permission' => ['SOME_OTHER_PERMISSION', 'canEdit', false], + 'canDelete allowed with admin permission when not protected' => [self::ADMIN_PERMISSION, 'canDelete', true], + 'canDelete falls through to parent without admin permission when not protected' => ['SOME_OTHER_PERMISSION', 'canDelete', false], + ]; + } + + public function testMenustructureMenuReturnsMatchingMenuBySlug(): void + { + $menu = Menu::MenustructureMenu('primary'); + + $this->assertInstanceOf(Menu::class, $menu); + $this->assertSame('Primary navigation', $menu->Title); + } + + public function testMenustructureMenuReturnsNullForUnknownSlug(): void + { + $this->assertNull(Menu::MenustructureMenu('does-not-exist')); + } + + public function testViewableMenustructureMenuRendersWithGivenTemplate(): void + { + $rendered = Menu::ViewableMenustructureMenu('primary', 'WeDevelop/Menustructure/Model/Menu'); + + $this->assertNotNull($rendered); + $this->assertStringContainsString('