diff --git a/CLAUDE.md b/CLAUDE.md index f7d394d..f07b852 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,10 +65,14 @@ Four interface-backed collaborators, each with a default implementation: | Interface | Default | Responsibility | |---|---|---| | `Contracts\Registrar_Interface` | `Registrar` | holds registered `Sub_Plugin` objects | -| `Contracts\Notices_Interface` | `Notices` | notice queue + activation-error rewrite | -| `Conflict\Resolver_Interface` | `Conflict\Resolver` | standalone detection, deactivation, redirect | +| `Notices\Contracts\Queue_Interface` | `Notices\Queue` | notice queue + activation-error rewrite | +| `Conflict\Contracts\Resolver_Interface` | `Conflict\Resolver` | standalone detection, deactivation, redirect | | `Contracts\Activation_Interface` | `Activation` | run-once activation-callback tracking | +An interface belonging to a folder-scoped concern lives in that folder's `Contracts\`, not beside its +implementation and not in the top-level `src/Contracts/`. `src/Contracts/` is for the interfaces whose +implementations sit at the root — `Registrar`, `Plugin_State`, `Activation`. + All four come through one generic helper — `Loader::resolve( string $interface, string $default_class ): object` — which returns the container binding when `$container->has()`, otherwise `new $default_class()`, memoized either way. Collaborators reach each other through the accessors @@ -86,7 +90,7 @@ config predicates there is what lets collaborators stay thin and lets them be te ### What exists today -`Loader` and the four collaborators above are not built yet. Currently: +`Loader`, `Conflict\Resolver` and `Activation` are not built yet. Currently: | Path | What | |---|---| @@ -94,7 +98,9 @@ config predicates there is what lets collaborators stay thin and lets them be te | `src/Sub_Plugin.php` | Value object; validates config and answers everything config alone decides. | | `src/Conflict_Policy.php` | The three policy constants, `default()`, `is_valid()`. | | `src/Plugin_State.php` | The only file that touches WordPress plugin functions. | -| `src/Contracts/`, `src/Exceptions/` | `Plugin_State_Interface`, `Config_Exception`. | +| `src/Registrar.php` | Holds registered `Sub_Plugin` objects. | +| `src/Notices/` | `Queue` (what a notice says, who may consume it), `Store` (keeps it), `Renderer` (draws it), `Contracts\Queue_Interface`. | +| `src/Contracts/`, `src/Exceptions/` | `Plugin_State_Interface`, `Registrar_Interface`, `Config_Exception`. | ### Boot lifecycle @@ -106,7 +112,7 @@ Loader::boot(); // idempotent plugins_loaded @1 → Conflict\Resolver::resolve_all() plugins_loaded @2 → Loader::load_all() -admin_notices → Loader::render_notices() [is_admin() only] +all_admin_notices → Loader::render_notices() [is_admin() only] wp_admin_notice_markup → Loader::filter_activation_error_markup() [is_admin() only] ``` @@ -126,8 +132,18 @@ owner deliberately turned on. ### Keys - Filters: `{$hook_prefix}/plugin_absorber/should_load`, `{$hook_prefix}/plugin_absorber/conflict_policy` -- Option: `{$hook_prefix}_plugin_absorber_activations` -- Transient: `{$hook_prefix}_plugin_absorber_notices` +- Options: `{$option_prefix}_plugin_absorber_activations`, `{$option_prefix}_plugin_absorber_notices` + +Both are built in `Config` — `get_hook_name()` and `get_option_name()` — so nothing else assembles +the segment between the host's prefix and the key's own name. The two differ in one respect: +`{$option_prefix}` is the hook prefix lowercased with hyphens folded to underscores, because the +prefix validator admits `A-Z` and `-` and a hook-naming value should not reach a storage key +verbatim. Hook names keep the host's casing exactly as it passed it. + +The notice queue is an option, not a transient: with a persistent object cache a transient never +reaches the database, so a `wp_cache_flush()` would destroy a merge notice that is raised exactly +once and never re-queued. It is read and written through `get_site_option()`/`update_site_option()`, +so it is a network option on multisite — matching `deactivate_plugins()`, which is network-wide. ## Conventions @@ -197,6 +213,13 @@ treatment. Any older sketch showing `Config::reset()` or `Loader::reset()` means - **The guard constant and the standalone basename are two separate keys.** No constant does double duty as both a load guard and a path resolver. +- **`get_hook_name()` and `get_option_name()` do not share a normalisation.** Folding case into the + hook prefix itself would silently rename the host's filters; leaving the raw prefix in an option + name puts `A-Z` and `-` into a storage key. Only the option side normalises, and collapsing the two + code paths breaks whichever end it is collapsed toward. +- **Notice messages are rendered through `wp_kses_post()`, not escaped.** They come from the host's + own config or filter, never from user input, so a knowledge-base link survives. Tightening this to + `esc_html()` after 1.0 would break every host that shipped one. - **A configured string is never called; every other callable form is.** A string function name is indistinguishable from a string value, so honouring it would make the result depend on what else the site loaded — `date`, `flush` and `key` are all real functions and plausible values. Closures, @@ -265,6 +288,13 @@ namespace, a `Config::set_version()` that was removed, and an `ob_start()` appro `wp_admin_notice_markup` filter. `docs/superpowers/plans/2026-07-31-plugin-absorber.md` holds the task-by-task breakdown. +Once a task's PR merges to `main`, delete that task's section from the plan in the next branch that +touches the file; git history keeps it. A shipped task's plan describes code that already exists in +`src/`, so all it can still do is make an agent read past it to reach what is unbuilt — and since +the plan is edited on every branch of a stacked series, an oversized one is a standing +merge-conflict surface. Never renumber what survives: the numbers map 1:1 to branch names. The spec +is the durable document; the plan is scaffolding and should shrink toward empty as the series lands. + Human-facing docs are `README.md` plus `docs/installing.md`, `docs/configuration.md`, -`docs/conflict-handling.md`, and `docs/filters.md`. Keep them short and keep rationale here or in -code comments — do not grow the README back. +`docs/conflict-handling.md`, `docs/filters.md`, and `docs/notices.md`. Keep them short and keep +rationale here or in code comments — do not grow the README back. diff --git a/README.md b/README.md index 16d7479..18d0bc0 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ plugins shipping different versions of this library will collide otherwise. See ```php use Nexcess\PluginAbsorber\Config; -Config::set_hook_prefix( 'give' ); // required — keys hooks, transients, options +Config::set_hook_prefix( 'give' ); // required — keys the hooks and options Config::set_container( give()->container ); // optional — lets you rebind collaborators ``` @@ -41,6 +41,7 @@ Each sub-plugin is then described by a config array: - [Configuration](docs/configuration.md) — the hook prefix, the container, every sub-plugin key. - [Conflict handling](docs/conflict-handling.md) — the policies, the load guard, and its limits. - [Filters](docs/filters.md) — the runtime overrides for policies and notice text. +- [Notices](docs/notices.md) — where the queue lives, who may see it, and how to render it yourself. ## License diff --git a/docs/configuration.md b/docs/configuration.md index 5af82e6..2e26b41 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -5,12 +5,14 @@ ```php use Nexcess\PluginAbsorber\Config; -Config::set_hook_prefix( 'give' ); // required — keys hooks, transients, options +Config::set_hook_prefix( 'give' ); // required — keys hooks and options Config::set_container( give()->container ); // optional — lets you rebind collaborators ``` The hook prefix accepts letters, numbers, hyphens, and underscores. Anything else throws -`Config_Exception`, as does reading the prefix before it is set. +`Config_Exception`, as does reading the prefix before it is set. Hook names repeat it verbatim; +option names lowercase it and turn hyphens into underscores, so `Give-Core` hooks +`Give-Core/plugin_absorber/should_load` and stores `give_core_plugin_absorber_notices`. The container is optional. Without one, the library instantiates its own collaborators; with one, a host can rebind them. diff --git a/docs/notices.md b/docs/notices.md new file mode 100644 index 0000000..b91d438 --- /dev/null +++ b/docs/notices.md @@ -0,0 +1,36 @@ +# Notices + +The three notices this library raises — the standalone was deactivated, the standalone is still +active, a dependency check failed — are queued in a single option named +`{prefix}_plugin_absorber_notices`, where `{prefix}` is the value passed to +`Config::set_hook_prefix()`. On multisite it is a **network** option, so the queue is shared across +every site on the network. + +An option and not a transient, on purpose. With a persistent object cache a transient never reaches +the database, so a `wp_cache_flush()` from a deploy script or a "purge cache" button would destroy +the queue. The deactivation notice is raised exactly once and never re-queued, so losing it means +the site owner is never told their plugin was turned off. + +## Who sees them + +`Notices\Queue::render()` prints the queue and then clears it, and it is gated on the +`activate_plugins` capability. Since rendering consumes the queue, a user who cannot act on a notice +must not be shown one — a subscriber loading their profile page would otherwise silently swallow the +only warning an administrator was ever going to get. + +On multisite `activate_plugins` maps through `manage_network_plugins`, so it is a network +administrator, not the site administrator who installed the plugin, who sees these. + +## Rendering them yourself + +`Notices\Queue::option_name()` is public, so you can render the queue yourself without replacing +anything. The value is an `array` keyed `slug:type` — `give-recurring:merge`, for +example — and the messages may contain markup; the default rendering passes them through +`wp_kses_post()`, so a link, emphasis or a list survives while scripts and event handlers are +stripped. Paragraphs come from `wpautop()`, so send the message unwrapped and let a blank line +break it — a `

` of your own is left as it is rather than nested inside another. + +The queue is three classes: `Notices\Queue` decides what a notice says and who may consume it, +`Notices\Store` keeps it, `Notices\Renderer` draws it. Both collaborators are constructor arguments, +so `new Queue( null, $renderer )` keeps the queue and replaces only the markup, and +`new Queue( $store )` does the reverse. Replacing either one leaves the other alone. diff --git a/docs/superpowers/plans/2026-07-31-plugin-absorber.md b/docs/superpowers/plans/2026-07-31-plugin-absorber.md index 715540c..88feafc 100644 --- a/docs/superpowers/plans/2026-07-31-plugin-absorber.md +++ b/docs/superpowers/plans/2026-07-31-plugin-absorber.md @@ -17,7 +17,7 @@ Every task's requirements implicitly include this section. - **PHP floor:** `>=7.4`. **WordPress floor:** 6.4 (the `wp_admin_notice_markup` filter). Stated in the README only — WordPress is not a Composer dependency, so it is not enforceable in `require`. - **Class naming:** `Snake_Case` (`Sub_Plugin`, `Conflict_Policy`, `Config_Exception`). Methods fully spelled out and readable. Config keys descriptive and WordPress-centric. - **Filter names:** `"{$hook_prefix}/plugin_absorber/should_load"` and `"{$hook_prefix}/plugin_absorber/conflict_policy"`. -- **Storage keys:** option `"{$hook_prefix}_plugin_absorber_activations"`, transient `"{$hook_prefix}_plugin_absorber_notices"`. +- **Storage keys:** option `"{$option_prefix}_plugin_absorber_activations"`, option `"{$option_prefix}_plugin_absorber_notices"`. Both are assembled by `Config::get_option_name( string $name )` and nowhere else, alongside `Config::get_hook_name()` for filters. **Amended 2026-08-11:** `{$option_prefix}` is the hook prefix lowercased with hyphens folded to underscores, so `Give-Core` yields the option `give_core_plugin_absorber_notices` while still yielding the filter `Give-Core/plugin_absorber/should_load` — the prefix validator admits `A-Z` and `-`, and a hook-naming value should not reach a storage key verbatim. The two normalisations stay separate: folding case into the hook side would silently rename the host's own filters. **Amended 2026-08-03 (PR 10 review):** the notice queue was specified as a *transient* and is now an option. `set_transient()` returns before touching the database whenever an external object cache is present, so on any Redis or Memcached site the queue would live only in the cache — where a routine `wp_cache_flush()` from a deploy script or a "purge cache" button destroys it. The merge notice is raised exactly once and never re-queued, so losing it means a site owner is never told their plugin was deactivated. On multisite both this and the activation option are network options, because the resolver deactivates network-wide. - **Production dependencies:** `stellarwp/container-contract` only. `lucatume/di52` is dev-only. No other StellarWP library. - **PR size cap:** ≤10 files per PR, tests and test infrastructure excluded. No logic-bearing PR exceeds 4 source files. - **PR body format** — exactly four parts, nothing else. No boilerplate headings, no restating the diff, no checklists: @@ -33,6 +33,8 @@ Every task's requirements implicitly include this section. - **Branching:** stacked. Each branch cuts from the previous branch, and merges to `main` in order. Never open PR N+1 before PR N's branch exists. - **Commits:** no co-author trailers, ever. - **Every source file** carries a file-level docblock with `@package Nexcess\PluginAbsorber` and every method a docblock with `@since 1.0.0`. This binds `src/` only. Test classes and test support classes keep the file-level docblock, but their methods do not need `@since` — the test code in this plan's own tasks is written that way deliberately (ruled 2026-07-31). +- **Container tests use the test-support adapter, never di52 directly** (verified 2026-07-31 against `vendor/`). `lucatume\DI52\Container` implements `ArrayAccess` and **PSR's** `Psr\Container\ContainerInterface` — not `StellarWP\ContainerContract\ContainerInterface`; `stellarwp/container-contract` ships an adapter example at `examples/di52/Container.php` precisely because DI52 must be wrapped. Passing `new Container()` to `Config::set_container()` is a `TypeError`. Tests use `Nexcess\PluginAbsorber\Tests\Support\Test_Container`, which wraps a di52 container and implements the contract's four methods (`bind`, `get`, `has`, `singleton`); **every `use lucatume\DI52\Container;` in the task blocks below means `Test_Container`.** `Config::set_container()`'s signature is unchanged — the StellarWP contract stays the public API, per the production-dependency constraint above. +- **`Config` carries no version handling** (ruled 2026-08-11, PR 4 review). `set_version()`/`get_version()` and the `$version` property were removed: nothing in the library reads a host version, and the one scenario that would want it — telling a bundled copy apart from a standalone at a specific release — is the host's problem. This closes spec known-issue F by deletion rather than by use. - **No test-only seams in `src/`** (ruled 2026-08-11, PR 4 review). Production classes do not carry a `reset()` for the suite's benefit — that is API the library then supports forever. Tests clear static state by reflection instead, through a helper under `tests/_support/`. `Config` is served by `Nexcess\PluginAbsorber\Tests\Support\Config_State::reset()`; **every `Config::reset()` in the task blocks below means `Config_State::reset()`.** `Registrar` is served by `Tests\Support\Registrar_State::reset( Registrar $registrar )` and `Loader` by `Tests\Support\Loader_State::reset()`, which empties the memoized registrar through `Registrar_State` before discarding the memo; both are spelled out in full in the task blocks below. ## File Structure @@ -40,20 +42,27 @@ Every task's requirements implicitly include this section. ``` plugin-absorber/ ├── src/ -│ ├── Config.php # static config facade: hook prefix, version, container +│ ├── Config.php # static config facade: hook prefix, container, name building │ ├── Loader.php # static facade: resolve/register/boot/load loop │ ├── Sub_Plugin.php # value object + every per-sub-plugin predicate │ ├── Conflict_Policy.php # three policy string constants +│ ├── Plugin_State.php # the only file touching WordPress plugin functions │ ├── Registrar.php # default slug => Sub_Plugin map │ ├── Activation.php # default run-once activation tracking -│ ├── Notices.php # default notice queue + activation-error rewrite │ ├── Conflict/ -│ │ ├── Resolver.php # default standalone detection/deactivation/redirect -│ │ └── Resolver_Interface.php -│ ├── Contracts/ -│ │ ├── Registrar_Interface.php -│ │ ├── Notices_Interface.php -│ │ └── Activation_Interface.php +│ │ ├── Contracts/ +│ │ │ └── Resolver_Interface.php +│ │ └── Resolver.php # default standalone detection/deactivation/redirect +│ ├── Contracts/ # interfaces whose implementations sit at the src/ root +│ │ ├── Activation_Interface.php +│ │ ├── Plugin_State_Interface.php +│ │ └── Registrar_Interface.php +│ ├── Notices/ +│ │ ├── Contracts/ +│ │ │ └── Queue_Interface.php +│ │ ├── Queue.php # default queue: wording + capability gate +│ │ ├── Renderer.php # markup and severity +│ │ └── Store.php # the option the queue lives in │ └── Exceptions/ │ └── Config_Exception.php ├── tests/ @@ -73,1461 +82,9 @@ One responsibility per file. `Sub_Plugin` holds every predicate so the collabora --- -## Task 1: Repo bootstrap - -**PR 1** · branch `01-repo-bootstrap` from `main` · 6 source files - -**Files:** -- Create: `composer.json`, `LICENSE`, `.gitignore`, `.gitattributes`, `.editorconfig`, `README.md`, `cspell.json` - -**Interfaces:** -- Consumes: nothing. -- Produces: the `Nexcess\PluginAbsorber\` PSR-4 autoload root that every later task relies on, and the `composer test:analysis` script Task 5 wires into CI. - -- [ ] **Step 1: Rename the working directory to match the remote** - -```bash -cd /Users/owl/www/wp-plugins -mv sub-plugin-loader plugin-absorber -cd plugin-absorber -git remote -v # expect: origin https://github.com/stellarwp/plugin-absorber.git -``` - -- [ ] **Step 2: Cut the branch** - -```bash -git checkout -b 01-repo-bootstrap -``` - -- [ ] **Step 3: Write `composer.json`** - -```json -{ - "name": "stellarwp/plugin-absorber", - "description": "Safely load bundled WordPress plugins inside a host plugin, togglable or always-on, without fatal errors.", - "type": "library", - "license": "GPL-2.0-or-later", - "minimum-stability": "stable", - "authors": [ - { - "name": "StellarWP", - "email": "eric_defore@vendor.stellarwp.com" - } - ], - "require": { - "php": ">=7.4", - "stellarwp/container-contract": "^1.0" - }, - "require-dev": { - "codeception/module-asserts": "^1.0", - "codeception/util-universalframework": "^1.0", - "lucatume/di52": "^3.0", - "lucatume/wp-browser": "^3.6.5", - "php-stubs/wordpress-stubs": "^6.4", - "phpunit/phpunit": "^9.5", - "szepeviktor/phpstan-wordpress": "^1.3" - }, - "autoload": { - "psr-4": { - "Nexcess\\PluginAbsorber\\": "src/" - } - }, - "autoload-dev": { - "psr-4": { - "Nexcess\\PluginAbsorber\\Tests\\": "tests/", - "Nexcess\\PluginAbsorber\\Tests\\Support\\": "tests/_support" - } - }, - "scripts": { - "test:analysis": [ - "phpstan analyse -c phpstan.neon.dist --memory-limit=512M" - ], - "test:unit": [ - "slic run unit" - ] - }, - "config": { - "optimize-autoloader": true, - "preferred-install": "dist", - "platform": { - "php": "7.4" - }, - "allow-plugins": { - "phpstan/extension-installer": true - } - } -} -``` - -- [ ] **Step 4: Write `.gitignore`** - -```gitignore -/vendor/ -/composer.lock -/tests/_output/* -!/tests/_output/.gitkeep -/phpstan-cache/ -/codeception.yml -/.env.testing.local -.DS_Store -.idea/ -.vscode/ -``` - -- [ ] **Step 5: Write `.gitattributes`** - -Keeps development files out of consumer installs. - -```gitattributes -/.github export-ignore -/docs export-ignore -/tests export-ignore -/.editorconfig export-ignore -/.env.testing export-ignore -/.env.testing.slic export-ignore -/.gitattributes export-ignore -/.gitignore export-ignore -/codeception.dist.yml export-ignore -/codeception.slic.yml export-ignore -/cspell.json export-ignore -/engineering-plan.md export-ignore -/phpstan.neon.dist export-ignore -``` - -- [ ] **Step 6: Write `.editorconfig`** - -```editorconfig -root = true - -[*] -charset = utf-8 -end_of_line = lf -insert_final_newline = true -trim_trailing_whitespace = true -indent_style = tab - -[*.{yml,yaml,json,md}] -indent_style = space -indent_size = 4 - -[*.md] -trim_trailing_whitespace = false -``` - -- [ ] **Step 7: Write `cspell.json`** - -The spec review flagged that domain vocabulary trips the editor spell-checker. Fix the dictionary, not the prose. - -```json -{ - "version": "0.2", - "language": "en", - "words": [ - "absorber", - "Codeception", - "codeception", - "fatals", - "Kadence", - "learndash", - "multisite", - "Nexcess", - "nexcess", - "Packagist", - "phpstan", - "referer", - "slic", - "stellarwp", - "StellarWP", - "Strauss", - "unhookable", - "uopz", - "Uopz", - "wpunit" - ], - "ignorePaths": [ - "vendor/**", - "tests/_output/**", - "composer.lock" - ] -} -``` - -- [ ] **Step 8: Add the GPL-2.0-or-later `LICENSE`** - -```bash -curl -sL https://www.gnu.org/licenses/old-licenses/gpl-2.0.txt -o LICENSE -head -3 LICENSE # expect: GNU GENERAL PUBLIC LICENSE / Version 2, June 1991 -``` - -- [ ] **Step 9: Write the README skeleton** - -Every later PR appends its own section here, so the README is never out of sync with what has shipped. Keep it dense — no prose introduction, no FAQ. - -```markdown -# Plugin Absorber - -Safely load bundled WordPress plugins inside a host plugin — togglable or always-on — without -re-declaration fatal errors. - -## Install - -```bash -composer require stellarwp/plugin-absorber -``` - -**Use [Strauss](https://github.com/stellarwp/global-docs/blob/main/docs/strauss-setup.md).** -Two or more plugins shipping different versions of this library will collide otherwise. - -> **Do not let `extra.strauss.constant_prefix` rewrite a sub-plugin's `plugin_loaded_constant`.** -> Those are real, shared runtime constants — the whole safety mechanism depends on the bundled -> copy and the standalone defining the *same* name. Add them to `exclude_from_copy`. - -Requires PHP 7.4+ and WordPress 6.4+. - -## Usage - -_Added as each piece lands._ -``` - -- [ ] **Step 10: Verify Composer accepts the manifest** - -Run: `composer validate --no-check-lock` -Expected: `./composer.json is valid` - -- [ ] **Step 11: Commit** - -```bash -git add composer.json LICENSE .gitignore .gitattributes .editorconfig cspell.json README.md -git commit -m "Add repo skeleton: composer manifest, license, dotfiles, README stub" -``` - -- [ ] **Step 12: Push and open the PR** - -```bash -git push -u origin 01-repo-bootstrap -gh pr create --base main --title "Repo bootstrap" --body 'What: composer manifest, GPL-2.0 license, dotfiles, cspell dictionary, README skeleton. - -Usage: - - composer require stellarwp/plugin-absorber - -Why this way: `stellarwp/plugin-absorber` over `sub-plugin-loader` — Packagist returns 21 hits for -`plugin-loader` and the top five are WordPress mu-plugin autoloaders, so the term is taken and -misleading; `plugin-absorber` returns zero. Dropping `-loader` also kills the `PluginAbsorber\Loader` -stutter. `stellarwp/container-contract` is the only production dependency, matching uplink. - -Verify: `composer validate --no-check-lock`. No tests — nothing here has behavior. The test harness -lands in the next PR and CI goes green in the one after.' -``` - ---- - -## Task 2: Codeception harness - -**PR 2** · branch `02-codeception-harness` from `01-repo-bootstrap` · 0 source files - -Configuration modelled on `stellarwp/harbor`, the closest sibling library. The `singlesite`/`multisite` envs matter — Task 12 needs multisite to test network-aware deactivation. - -**Files:** -- Create: `codeception.dist.yml`, `codeception.slic.yml`, `tests/unit.suite.yml`, `tests/_bootstrap.php`, `tests/_support/UnitTester.php`, `tests/_support/Helper/Unit.php`, `.env.testing`, `.env.testing.slic`, `tests/_output/.gitkeep` - -**Interfaces:** -- Consumes: the `Nexcess\PluginAbsorber\Tests\` autoload-dev root from Task 1. -- Produces: a `unit` suite runnable as `slic run unit`, with `--env singlesite` and `--env multisite`. Every later test class extends `\Codeception\TestCase\WPTestCase` and lives in `tests/unit/`. - -- [ ] **Step 1: Cut the branch** - -```bash -git checkout 01-repo-bootstrap && git checkout -b 02-codeception-harness -``` - -- [ ] **Step 2: Write `codeception.dist.yml`** - -```yaml -actor: Tester -bootstrap: _bootstrap.php -paths: - tests: tests - output: tests/_output - data: tests/_data - support: tests/_support - envs: tests/_envs -actor_suffix: Tester -settings: - colors: true - memory_limit: 1024M - error_level: E_ALL -params: - - .env.testing -extensions: - enabled: - - Codeception\Extension\RunFailed -``` - -- [ ] **Step 3: Write `codeception.slic.yml`** - -slic overlays this file to swap in the containerised database host. - -```yaml -params: - - .env.testing.slic -``` - -- [ ] **Step 4: Write `tests/unit.suite.yml`** - -`multisite: false` is the default; the `multisite` env flips it. Note this suite loads WordPress — it is a "unit" suite by name only, matching the `admin-notices` convention. - -```yaml -# Codeception Test Suite Configuration -# Loads a real WordPress via WPLoader. uopz stubs the functions WP cannot hook. -actor: UnitTester -bootstrap: _bootstrap.php -modules: - enabled: - - WPLoader - - Asserts - - "Nexcess\\PluginAbsorber\\Tests\\Support\\Helper\\Unit" - config: - WPLoader: - wpRootFolder: "%WP_ROOT_FOLDER%" - dbName: "%WP_TEST_DB_NAME%" - dbHost: "%WP_TEST_DB_HOST%" - dbUser: "%WP_TEST_DB_USER%" - dbPassword: "%WP_TEST_DB_PASSWORD%" - tablePrefix: test_ - domain: "%WP_DOMAIN%" - adminEmail: admin@plugin-absorber.test - title: "Plugin Absorber Tests" - theme: twentytwentythree - multisite: false - -env: - singlesite: - multisite: - modules: - config: - WPLoader: - multisite: true -``` - -- [ ] **Step 5: Write `tests/_bootstrap.php`** - -```php -assertTrue( function_exists( 'add_action' ) ); - $this->assertTrue( defined( 'ABSPATH' ) ); - } - - public function test_uopz_is_available(): void { - $this->assertTrue( extension_loaded( 'uopz' ), 'uopz is required to stub WordPress functions.' ); - $this->assertTrue( function_exists( 'uopz_set_return' ) ); - } - - public function test_uopz_can_stub_a_function(): void { - $this->setFunctionReturn( 'wp_get_referer', 'https://example.test/wp-admin/plugins.php' ); - - $this->assertSame( 'https://example.test/wp-admin/plugins.php', wp_get_referer() ); - } -} -``` - -There is deliberately no test that `exit` can be neutralised. See Step 4. - -- [ ] **Step 3: Run it to verify it fails** - -Run: `slic run unit` -Expected: FAIL — `wp_get_referer()` returns the real value, so `test_uopz_can_stub_a_function` fails on the `assertSame`, until `use UopzFunctions` is in place. - -- [ ] **Step 4: Add `TestException` and the tests README** - -There is no local uopz trait to write. `use lucatume\WPBrowser\Traits\UopzFunctions;` in the smoke test is the entire change on the stubbing side: it ships with wp-browser, undoes every override via its own `@after resetUopzAlterations()`, and takes an explicit `$execute` flag instead of guessing whether a value is callable. - -`UopzFunctions::preventExit()` exists, but this library does not use it. Neutralising `exit` lets a test keep running past the point where production would have stopped, so a test that should fail can report as passing and CI will not say otherwise. Tasks 8 and 13 instead stub the call immediately before `exit` and throw from it, which stops execution at a point the test controls. - -```php -> ${GITHUB_WORKSPACE}/slic/containers/slic/php.ini - - - name: Set up slic env vars - run: | - echo "SLIC_BIN=${GITHUB_WORKSPACE}/slic/slic" >> $GITHUB_ENV - echo "SLIC_WP_DIR=${GITHUB_WORKSPACE}/slic/_wordpress" >> $GITHUB_ENV - echo "SLIC_WORDPRESS_DOCKERFILE=Dockerfile.base" >> $GITHUB_ENV - - - name: Set run context for slic - run: echo "SLIC=1" >> $GITHUB_ENV && echo "CI=1" >> $GITHUB_ENV - - - name: Start ssh-agent - run: | - eval `ssh-agent -s` - echo "SSH_AUTH_SOCK=${SSH_AUTH_SOCK}" >> $GITHUB_ENV - - - name: Set up slic for CI - run: | - cd ${GITHUB_WORKSPACE}/.. - ${SLIC_BIN} here - ${SLIC_BIN} interactive off - ${SLIC_BIN} build-prompt off - ${SLIC_BIN} build-subdir off - ${SLIC_BIN} xdebug off - ${SLIC_BIN} debug on - ${SLIC_BIN} php-version set ${{ matrix.php }} --skip-rebuild - - - name: Set up the library - run: | - ${SLIC_BIN} use plugin-absorber - ${SLIC_BIN} composer set-version 2 - ${SLIC_BIN} composer validate - ${SLIC_BIN} composer install - - # The slic image ships a fixed WordPress that varies by PHP version, and - # WP_VERSION in .env.testing.slic does not change it. Without this step a - # leg named "WP latest" silently tests whatever core the image happened to - # bake in. WPLoader installs from this codebase, so pinning here is what - # actually puts the suite on the version the leg claims. - - name: Pin the WordPress version - run: ${SLIC_BIN} site-cli core update --version=${{ matrix.wp }} --force - - - name: Build codeception - run: ${SLIC_BIN} cc build - - - name: Run unit tests (singlesite) - run: ${SLIC_BIN} run unit --env singlesite --ext DotReporter - - # Run even when singlesite failed: one run should report both envs rather - # than making you fix one and rediscover the other. - - name: Run unit tests (multisite) - if: ${{ !cancelled() }} - run: ${SLIC_BIN} run unit --env multisite --ext DotReporter - - - name: Upload test output - if: failure() - uses: actions/upload-artifact@v7 - with: - name: "test-output-php${{ matrix.php }}-wp${{ matrix.wp }}" - path: tests/_output - if-no-files-found: ignore - retention-days: 7 -``` - -- [ ] **Step 7: Commit** - -```bash -git add tests/unit/SmokeTest.php tests/_support/TestException.php tests/README.md .github/workflows/tests-php.yml -git commit -m "Add harness smoke test, exit policy, and PHP tests workflow" -``` - -- [ ] **Step 8: Push and confirm CI is actually green** - -```bash -git push -u origin 03-ci-tests -gh pr create --base 02-codeception-harness --title "First green CI" --body 'What: a smoke test that proves the harness, the `TestException` and README that set the stubbing rules, and the PHP tests workflow. - -Usage: - - class SomeTest extends WPTestCase { - use UopzFunctions; // from wp-browser, not a local trait. - - public function test_something(): void { - $this->setFunctionReturn( "is_plugin_active", true ); - } - } - -Why this way: the smoke test asserts WordPress is loaded, uopz is present, and a function can -actually be stubbed — the assumptions every later test rests on. Proving them here means a later -failure is a real bug rather than a harness problem. - -No local `WithUopz`: `lucatume\WPBrowser\Traits\UopzFunctions` ships with wp-browser, is maintained -by its author, undoes overrides through its own `@after`, and exists as far back as the `^3.6.5` -floor this library pins. One less copy to drift across repos. - -`exit` is never mocked. Neutralising it lets a test keep running past the point production would -have stopped, so a test that should fail can report as passing. Redirect branches are tested by -stubbing the call immediately before `exit` and throwing `TestException` from it — worked example -in tests/README.md. - -Verify: `slic run unit` — 3 tests. CI runs both envs across PHP 7.4 through 8.5 against WordPress -latest and nightly — four legs, with the nightly ones non-blocking. Static analysis is not -wired yet; it lands after the first src/ file, because PHPStan errors on an empty directory.' - -gh run watch -``` -Expected: all matrix legs green. **Do not proceed until they are** — every later task assumes this harness works. - ---- - -## Task 4: `Config` - -**PR 4** · branch `04-config` from `03-ci-tests` · 3 source files - -**Files:** -- Create: `src/Config.php`, `src/Exceptions/Config_Exception.php`, `tests/unit/ConfigTest.php` -- Modify: `README.md` - -**Interfaces:** -- Consumes: nothing. -- Produces: - - `Config_Exception extends \RuntimeException` - - `Config::set_hook_prefix( string ): void` — throws `Config_Exception` on characters outside `[a-zA-Z0-9_-]` - - `Config::get_hook_prefix(): string` — throws `Config_Exception` when unset - - `Config::set_container( ContainerInterface ): void` / `get_container(): ?ContainerInterface` / `has_container(): bool` - - Every later task calls `Config::set_hook_prefix()` in `setUp()` and `Config_State::reset()` in `tearDown()`. - -> **Deviation from the engineering plan, deliberate:** the plan's sketch throws bare -> `RuntimeException`. This throws `Config_Exception`, which extends `RuntimeException`, so the -> documented contract still holds while callers get one catchable type across the whole library. - -> **Second deviation, deliberate (added 2026-08-03):** `set_hook_prefix()` also rejects the empty -> string. The character-class check alone would accept `''` — it contains no invalid character — -> and the failure would resurface at `get_hook_prefix()` as the misleading "You must call -> `Config::set_hook_prefix()`" long after the real mistake. - -> **Third deviation, from the PR 4 review (2026-08-11) — two removals. The code blocks below still -> show both; they are wrong and were left for the record.** -> -> 1. **`set_version()` / `get_version()` are gone**, along with the `$version` property, its two -> tests, and the README line. Nothing in the library reads a host version, and the one scenario -> that would want it — telling a bundled copy apart from a standalone at a specific release, à la -> ProPanel v3.0 — is the host's problem, not this library's. This closes spec known-issue F by -> deletion rather than by use. -> 2. **`Config::reset()` is gone.** It existed only so the suite could clear static state, and a -> public method is a promise to everyone, not just to tests. The suite now uses -> `Tests\Support\Config_State::reset()`, which walks `Config`'s declared static defaults by -> reflection — so state added to `Config` later is cleared with no change to the helper. See the -> Global Constraint on test-only seams. -> -> The `ConfigTest` block below is also superseded on two points the review raised: the prefix tests -> are driven by `public static` `Generator` data providers (valid and invalid, the empty string -> among the invalid), and the `RuntimeException` test now catches as `RuntimeException` and asserts -> `instanceof Config_Exception` — proving both halves of the contract instead of passing merely -> because one extends the other. - -- [ ] **Step 1: Cut the branch** - -```bash -git checkout 03-ci-tests && git checkout -b 04-config -``` - -- [ ] **Step 2: Write the failing test** - -```php -assertSame( 'give', Config::get_hook_prefix() ); - } - - public function test_it_accepts_letters_numbers_hyphens_and_underscores(): void { - Config::set_hook_prefix( 'give-recurring_2' ); - - $this->assertSame( 'give-recurring_2', Config::get_hook_prefix() ); - } - - /** - * @dataProvider invalid_hook_prefixes - * - * @param string $prefix Prefix under test. - */ - public function test_it_rejects_invalid_hook_prefixes( string $prefix ): void { - $this->expectException( Config_Exception::class ); - - Config::set_hook_prefix( $prefix ); - } - - /** - * @return array - */ - public function invalid_hook_prefixes(): array { - return [ - 'slash' => [ 'give/recurring' ], - 'space' => [ 'give recurring' ], - 'dot' => [ 'give.recurring' ], - 'backslash' => [ 'give\\recurring' ], - ]; - } - - public function test_it_throws_when_the_hook_prefix_was_never_set(): void { - $this->expectException( Config_Exception::class ); - - Config::get_hook_prefix(); - } - - public function test_it_stores_and_returns_the_version(): void { - Config::set_version( '3.0.0' ); - - $this->assertSame( '3.0.0', Config::get_version() ); - } - - public function test_the_version_defaults_to_an_empty_string(): void { - $this->assertSame( '', Config::get_version() ); - } - - public function test_it_reports_no_container_by_default(): void { - $this->assertFalse( Config::has_container() ); - $this->assertNull( Config::get_container() ); - } - - public function test_it_stores_and_returns_a_container(): void { - $container = new Container(); - - Config::set_container( $container ); - - $this->assertTrue( Config::has_container() ); - $this->assertSame( $container, Config::get_container() ); - } - - public function test_reset_clears_every_value(): void { - Config::set_hook_prefix( 'give' ); - Config::set_version( '3.0.0' ); - Config::set_container( new Container() ); - - Config::reset(); - - $this->assertSame( '', Config::get_version() ); - $this->assertFalse( Config::has_container() ); - $this->assertNull( Config::get_container() ); - - $this->expectException( Config_Exception::class ); - Config::get_hook_prefix(); - } -} -``` - -> **CORRECTION (2026-07-31, verified against vendor/):** `lucatume\DI52\Container` does **not** -> implement `StellarWP\ContainerContract\ContainerInterface`. It implements `ArrayAccess` and -> **PSR's** `Psr\Container\ContainerInterface`. `stellarwp/container-contract` ships an adapter -> example at `examples/di52/Container.php` precisely because DI52 must be wrapped. -> `new Container()` therefore cannot be passed to `Config::set_container()` — it is a `TypeError`. -> -> Tests must use the test-support adapter `Nexcess\PluginAbsorber\Tests\Support\Test_Container` -> (wraps a DI52 container, implements the StellarWP contract's four methods: `bind`, `get`, -> `has`, `singleton`). This affects **Task 4 and Task 10** — both of their test blocks below still -> show the incorrect `use lucatume\DI52\Container;`. `Config::set_container()`'s signature is -> unchanged: the StellarWP contract stays the public API, per the Global Constraint that -> `stellarwp/container-contract` is the only production dependency. - -- [ ] **Step 3: Run it to verify it fails** - -Run: `slic run unit` -Expected: FAIL — `Class "Nexcess\PluginAbsorber\Config" not found`. - -- [ ] **Step 4: Write `src/Exceptions/Config_Exception.php`** - -```php -container ); // optional — see Rebinding below -``` - -The hook prefix accepts letters, numbers, hyphens, and underscores. Anything else throws -`Config_Exception`, as does reading it before it is set. -``` - -- [ ] **Step 8: Commit** - -```bash -git add src/Config.php src/Exceptions/Config_Exception.php tests/unit/ConfigTest.php README.md -git commit -m "Add Config facade and Config_Exception" -``` - -- [ ] **Step 9: Push and open the PR** - -```bash -git push -u origin 04-config -gh pr create --base 03-ci-tests --title "Config facade" --body 'What: `Config` static facade — hook prefix, version, optional container — plus `Config_Exception`. - -Usage: - - Config::set_hook_prefix( "give" ); - Config::set_container( give()->container ); // optional - -Why this way: the plan sketched bare `RuntimeException`; this throws `Config_Exception extends -RuntimeException`, so the documented contract still holds and callers get one catchable type across -the library. `set_hook_prefix()` validates eagerly rather than at use, because a bad prefix -otherwise surfaces as a silently-never-firing filter much later. - -Verify: `slic run unit` — the validation regex over valid and invalid prefixes, the unset-prefix -throw, the `RuntimeException` catchability contract, and container storage. Version handling is not -covered because it no longer exists; see the third deviation above.' -``` - ---- - -## Task 5: Static analysis in CI - -**PR 5** · branch `05-ci-static-analysis` from `04-config` · 2 source files - -Lands here rather than in Task 1 because PHPStan errors on an empty `src/`. - -**Files:** -- Create: `phpstan.neon.dist`, `.github/workflows/static-analysis.yml` - -**Interfaces:** -- Consumes: `src/Config.php` from Task 4 — the first file to analyse. -- Produces: a green `composer test:analysis`. Every later task must keep it green. - -- [ ] **Step 1: Cut the branch** - -```bash -git checkout 04-config && git checkout -b 05-ci-static-analysis -``` - -- [ ] **Step 2: Write `phpstan.neon.dist`** - -Level 5 per the engineering plan. `szepeviktor/phpstan-wordpress` supplies WordPress function signatures. - -```neon -includes: - - vendor/szepeviktor/phpstan-wordpress/extension.neon - -parameters: - phpVersion: 70400 - level: 5 - tmpDir: phpstan-cache - treatPhpDocTypesAsCertain: false - reportUnmatchedIgnoredErrors: false - - paths: - - src - - scanDirectories: - - vendor/stellarwp/container-contract/src -``` - -- [ ] **Step 3: Run it and confirm it is clean** - -Run: `composer test:analysis` -Expected: `[OK] No errors`. - -If `Config::$container` reports an unknown `ContainerInterface`, confirm -`stellarwp/container-contract` installed and that `scanDirectories` points at its real `src` path. - -- [ ] **Step 4: Write the static analysis workflow** - -Adapted from `stellarwp/harbor`'s `static-analysis.yml`. - -```yaml -# cspell:ignore shivammathur ramsey reqs -name: PHPStan - -on: - pull_request: - push: - branches: - - main - -jobs: - phpstan: - name: phpstan - runs-on: ubuntu-latest - - steps: - - name: Checkout - uses: actions/checkout@v6 - - - name: Configure PHP environment - uses: shivammathur/setup-php@v2 - with: - php-version: "8.0" - extensions: mbstring, intl - coverage: none - - - uses: ramsey/composer-install@v3 - with: - composer-options: "--ignore-platform-reqs --optimize-autoloader" - dependency-versions: highest - - - name: Restore PHPStan cache - uses: actions/cache/restore@v4 - with: - path: phpstan-cache - key: v1-phpstan-${{ runner.os }}-${{ github.ref_name }}-${{ github.run_id }} - restore-keys: | - v1-phpstan-${{ runner.os }}-${{ github.ref_name }}- - v1-phpstan-${{ runner.os }}- - v1-phpstan- - - - name: Run PHPStan static analysis - run: composer test:analysis - - - name: Save PHPStan cache - uses: actions/cache/save@v4 - if: ${{ !cancelled() }} - with: - path: phpstan-cache - key: v1-phpstan-${{ runner.os }}-${{ github.ref_name }}-${{ github.run_id }} -``` - -- [ ] **Step 5: Commit** - -```bash -git add phpstan.neon.dist .github/workflows/static-analysis.yml -git commit -m "Add PHPStan level 5 and the static analysis workflow" -``` - -- [ ] **Step 6: Push, open the PR, confirm green** - -```bash -git push -u origin 05-ci-static-analysis -gh pr create --base 04-config --title "Static analysis" --body 'What: PHPStan level 5 with `szepeviktor/phpstan-wordpress`, plus its CI workflow. - -Usage: - - composer test:analysis - -Why this way: level 5 per the engineering plan rather than harbor is level max — this library has -almost no generics or array shapes to model, so max would mostly generate baseline noise. It lands -now rather than in the bootstrap PR because PHPStan errors on an empty `src/`, so it needed a real -file to analyse. - -Verify: `composer test:analysis` is clean. From this PR on, both workflows gate every merge.' - -gh run watch -``` - ---- - -## Task 6: `Conflict_Policy` - -**PR 6** · branch `06-conflict-policy` from `05-ci-static-analysis` · 2 source files - -Split from `Sub_Plugin` so that PR 7 is purely predicate logic. - -**Files:** -- Create: `src/Conflict_Policy.php`, `tests/unit/ConflictPolicyTest.php` -- Modify: `README.md` - -**Interfaces:** -- Consumes: nothing. -- Produces: `Conflict_Policy::DEACTIVATE` (`'deactivate'`), `Conflict_Policy::DEFER` (`'defer'`), `Conflict_Policy::NOTICE_ONLY` (`'notice_only'`). Tasks 7 and 12 both depend on these exact string values. - -- [ ] **Step 1: Cut the branch** - -```bash -git checkout 05-ci-static-analysis && git checkout -b 06-conflict-policy -``` - -- [ ] **Step 2: Write the failing test** - -The values are asserted literally because they are a public contract — a host may store one in an option, and changing a value later would silently break it. - -> **Deviation, deliberate (added 2026-08-03, from PR 6 review):** the class also ships -> `is_valid( string ): bool`, over a private `all(): string[]`. Without it nothing rejects an unknown policy: -> `Sub_Plugin::get_conflict_policy()` returns whatever the config or the filter hands back, and -> `Conflict\Resolver::resolve()` switches on it with `default:` falling into `deactivate()`. A typo -> like `'defered'`, or a stale filter return, would therefore deactivate a plugin the site owner -> deliberately turned on — the most surprising and least recoverable of the three outcomes, reached -> by accident. Task 12 must call `is_valid()` and treat an unknown policy as its own case rather -> than relying on the fallthrough. The reflection test pins the constant set so a fourth policy -> cannot be added without that switch being revisited, and the valid-policy provider reads the -> constants too, so a policy declared but never taught to `is_valid()` fails as well. -> -> `all()` is private: nothing in this plan reads the set, only `is_valid()` does. Widen it if a -> host ever needs to enumerate the policies. - -```php -assertSame( 'deactivate', Conflict_Policy::DEACTIVATE ); - $this->assertSame( 'defer', Conflict_Policy::DEFER ); - $this->assertSame( 'notice_only', Conflict_Policy::NOTICE_ONLY ); - } - - public function test_the_policies_are_distinct(): void { - $policies = [ - Conflict_Policy::DEACTIVATE, - Conflict_Policy::DEFER, - Conflict_Policy::NOTICE_ONLY, - ]; - - $this->assertCount( 3, array_unique( $policies ) ); - } -} -``` - -- [ ] **Step 3: Run it to verify it fails** - -Run: `slic run unit` -Expected: FAIL — `Class "Nexcess\PluginAbsorber\Conflict_Policy" not found`. - -- [ ] **Step 4: Write `src/Conflict_Policy.php`** - -```php - Conflict_Policy::DEACTIVATE, // or DEFER, or NOTICE_ONLY - -Why this way: string constants rather than an enum because the PHP floor is 7.4, and rather than -bare strings because a host may persist one in an option. The test asserts the literal values for -that reason — they are a public contract, not an implementation detail. - -Verify: `slic run unit` — 2 tests. Split out from Sub_Plugin so that PR reviews as pure predicate -logic.' -``` +Tasks 1–6 (repo bootstrap, Codeception harness, first green CI, `Config`, static analysis in CI, +`Conflict_Policy`) shipped in PRs #1–#6 and their sections have been removed. Git history has them if +you need to read one back. --- @@ -2076,7 +633,7 @@ class Sub_Plugin { } return (string) apply_filters( - Config::get_hook_prefix() . '/plugin_absorber/conflict_policy', + Config::get_hook_name( 'conflict_policy' ), $policy, $this ); @@ -2963,8 +1520,8 @@ Config::set_container( $container ); | Interface | Default | Responsibility | |---|---|---| | `Contracts\Registrar_Interface` | `Registrar` | Holds the registered sub-plugins. | -| `Contracts\Notices_Interface` | `Notices` | Notice queue and the activation-error rewrite. | -| `Conflict\Resolver_Interface` | `Conflict\Resolver` | Standalone detection, deactivation, redirect. | +| `Notices\Contracts\Queue_Interface` | `Notices\Queue` | Notice queue and the activation-error rewrite. | +| `Conflict\Contracts\Resolver_Interface` | `Conflict\Resolver` | Standalone detection, deactivation, redirect. | | `Contracts\Activation_Interface` | `Activation` | Run-once activation tracking. | The container is **not** used to wire hooks — those stay plain static callbacks, so the container @@ -3012,19 +1569,95 @@ container.' Lands before the load path and the resolver because both call into it. **Files:** -- Create: `src/Contracts/Notices_Interface.php`, `src/Notices.php`, `tests/unit/NoticesTest.php` +- Create: `src/Notices/Contracts/Queue_Interface.php`, `src/Notices/Queue.php`, `src/Notices/Store.php`, `src/Notices/Renderer.php`, `tests/unit/Notices/QueueTest.php`, `tests/unit/Notices/StoreTest.php`, `tests/unit/Notices/RendererTest.php` - Modify: `src/Loader.php` (add the `notices()` accessor), `README.md` **Interfaces:** -- Consumes: `Config::get_hook_prefix()` (Task 4), `Sub_Plugin` message getters (Task 7), the `WithSubPlugins` trait (Task 7) for its fixtures, `Loader::resolve()` (Task 9). +- Consumes: `Config::get_option_name()` (Task 4), `Sub_Plugin` message getters (Task 7), the `WithSubPlugins` trait (Task 7) for its fixtures, `Loader::resolve()` (Task 9). - Produces: - - `Notices_Interface` with `queue_merge_notice( Sub_Plugin ): void`, `queue_conflict_notice( Sub_Plugin ): void`, `queue_dependency_notice( Sub_Plugin ): void`, `render(): void` - - `Loader::notices(): Notices_Interface` + - `Notices\Contracts\Queue_Interface` with `queue_merge_notice( Sub_Plugin ): void`, `queue_conflict_notice( Sub_Plugin ): void`, `queue_dependency_notice( Sub_Plugin ): void`, `render(): void` + - `Loader::notices(): Notices\Contracts\Queue_Interface` Task 11 calls `queue_dependency_notice()`; Task 12 calls `queue_merge_notice()` and `queue_conflict_notice()`; Task 14 extends this interface. **Design notes:** -- Transient `"{$hook_prefix}_plugin_absorber_notices"`, no expiry, so the queue survives the resolver's `wp_safe_redirect()` and renders on the next admin load. +- Option `"{$option_prefix}_plugin_absorber_notices"`, built by `Config::get_option_name( 'notices' )`, so the queue survives the resolver's `wp_safe_redirect()` and renders on the next admin load. + +> **Deviations, deliberate (added 2026-08-03, from the PR 10 review):** +> +> 1. **An option, not a transient.** Verified in core: `set_transient()` short-circuits to +> `wp_cache_set()` and never writes the database when an external object cache is present. On a +> Redis or Memcached site the queue would exist only in the cache, and `wp_cache_flush()` — run +> by deploy scripts and every "purge cache" button — destroys it. The merge notice is raised +> once and never re-queued, so losing it means the site owner is never told. This queue is not +> a cache. See the amended Global Constraint. +> 2. **Network options on multisite.** The resolver passes `$network_wide` to +> `deactivate_plugins()`, which removes the plugin from every site in the network. A per-site +> option would have parked the explanation in whichever site's options table happened to serve +> the request that triggered it — invisible to the superadmin, on one of fifty sites. +> 3. **`render()` checks `activate_plugins` first.** Rendering *consumes* the queue, so without a +> gate any logged-in user loading `profile.php` would silently swallow the one warning an +> administrator was going to get. On multisite this correctly resolves to superadmins, since +> `activate_plugins` maps through `manage_network_plugins` there. +> 4. **`all_admin_notices`, not `admin_notices`** (see Task 11). The three notice hooks are +> mutually exclusive branches in `admin-header.php`, and `admin_notices` does not fire in the +> network admin — exactly where a network-wide deactivation would be noticed. +> 5. **`get_conflict_notice_message( $default )` replaces the planned private `message_or_default()` +> helper**, using the parameter added to `Sub_Plugin` in PR 7. Identical semantics, one less +> duplicated method. +> 6. **`get_queue()` drops non-string entries** rather than printing them, and writes the cleaned +> array back, so a corrupted queue heals on the next write. This moved to `Notices\Store::all()` +> in deviation 7. +> 7. **`Notices` is split into three, renamed, and grouped into `src/Notices/`** (added +> 2026-08-11). `Notices\Store` owns the option, `Notices\Renderer` owns the markup and the +> severity map, and `Notices\Queue` keeps the interface, the message defaults and the capability +> gate. One class had four reasons to change — storage, wording, severity, markup — and Task 14 +> adds a fifth by hanging the activation-error rewrite off the same class. Splitting now means a +> host that wants only different markup replaces `Notices\Renderer` instead of reimplementing +> the queue. +> +> `Notices` was a plural bag noun naming the subject rather than the job, which read worst of +> the three once the split landed. The folder carries the subject and the class carries the job, +> following `Conflict\Resolver`. The rename is free now and a breaking change after 1.0. +> +> The interface follows the folder: `Notices\Contracts\Queue_Interface` in +> `src/Notices/Contracts/Queue_Interface.php`. A folder-scoped concern owns its own contract, so +> the top-level `src/Contracts/` is left holding only the interfaces whose implementations sit at +> the `src/` root — `Registrar_Interface`, `Plugin_State_Interface`, `Activation_Interface`. +> Task 12 does the same for `Conflict\Contracts\Resolver_Interface`. +> +> Both collaborators are constructor arguments defaulting to the standard implementations, so +> `new Queue()` — what `Loader::resolve()` builds when the container holds no binding — behaves +> exactly as `new Notices()` did, and the interface's method set is unchanged. Tasks 11, 12 and +> 14 need no rework beyond the new type name and import. +> +> The capability check stays in `Notices\Queue::render()` rather than moving into the renderer, +> because it guards clearing the queue as much as drawing it: split apart, a user who may not +> see the queue could still consume it. +> +> Not done, and deliberately: the interface still mixes queueing with rendering, so a host +> replacing one inherits the other, and a fourth notice type is still a breaking interface +> change. Both need a spec amendment and rework in Tasks 11, 12 and 14, and fixing them here +> would put this PR over the four-source-file cap. The same cap is why `Notices\Store` and +> `Notices\Renderer` are concrete rather than interface-backed. +> +> 8. **`Notices\Store` builds its key with `Config::get_option_name( 'notices' )`** (added +> 2026-08-11), not by concatenating `Config::get_hook_prefix()`. The prefix validator admits +> `A-Z` and `-`, so `Give-Core` would otherwise put a capitalised, hyphenated segment straight +> into a storage key. `get_option_name()` lowercases and folds hyphens to underscores; +> `get_hook_name()` deliberately does not, because normalising there would silently rename the +> host's own filters. Nothing outside `Config` assembles either kind of name. +> 9. **`Notices\Renderer` prints through `wp_kses_post()`, not `esc_html()`** (added 2026-08-11). +> A message reaches the renderer only from the host's own `conflict_notice_message` / +> `dependency_notice_message` config or from its filter — never from user input — and the +> commonest thing a host wants in a merge notice is a link to its own knowledge-base article. +> `wp_kses_post()` still strips scripts and event handlers, so the XSS surface is unchanged. +> Task 14's activation-error rewrite takes the same message from the same source onto the same +> screen, so the two must not diverge. +> +> The Step 4/5 code listings below still show the pre-split, pre-rename shape, exactly as they +> still show the transient that deviation 1 replaced and the hand-built option key that +> deviation 8 replaced. The deviations are the record of what shipped. - Queue entries are keyed `"{$slug}:{$type}"`, not by slug alone. A sub-plugin can legitimately earn a merge notice at `plugins_loaded` @1 and a dependency notice at @2 in the same request; keying by slug alone would silently drop one. - Default messages live **here**, not in `Sub_Plugin`. `get_conflict_notice_message()` returns `''` when unconfigured (Task 7 asserts this), and each notice type supplies its own fallback sentence — so auto-deactivating a plugin can never leave the user with no explanation. @@ -3150,14 +1783,21 @@ class NoticesTest extends WPTestCase { $this->assertStringContainsString( 'Bundled now.', $output ); } - public function test_render_escapes_the_message(): void { + public function test_render_strips_unsafe_markup_but_keeps_a_link(): void { $notices = new Notices(); - $notices->queue_merge_notice( $this->make_sub_plugin( [ 'conflict_notice_message' => '' ] ) ); + $notices->queue_merge_notice( + $this->make_sub_plugin( + [ 'conflict_notice_message' => 'Read more' ] + ) + ); $output = $this->render_to_string( $notices ); $this->assertStringNotContainsString( '' ] ); + public function test_it_strips_unsafe_markup_from_the_replacement_but_keeps_a_link(): void { + $this->register( + [ 'conflict_notice_message' => 'Read more' ] + ); $result = ( new Notices() )->filter_activation_error_markup( $this->wordpress_markup ); $this->assertStringNotContainsString( '' )` is the empty + // string. Either that or a whitespace-only message would print an empty notice box, + // which reads as a bug. + $message = trim( wp_kses_post( $message ) ); + + if ( $message === '' ) { + continue; + } + + printf( + '

%s
', + esc_attr( $this->notice_class( (string) $key ) ), + // Already filtered: escaping it again here would undo the whole point and print a + // link as literal angle brackets. Trimmed because `wpautop()` leaves a trailing + // newline, which would otherwise sit inside the div on every notice. + trim( wpautop( $message ) ) + ); + } + } + + /** + * The `notice-*` class for a queue entry, taken from the type half of its `slug:type` key. + * + * @since 1.0.0 + * + * @param string $key Queue key. + * + * @return string + */ + private function notice_class( string $key ): string { + $parts = explode( ':', $key ); + $type = (string) end( $parts ); + + // An entry written by an older version, or by a host reading and rewriting the option, is + // shown rather than dropped: a warning is the safe severity for something unrecognised. + return self::CLASSES[ $type ] ?? 'notice-warning'; + } +} diff --git a/src/Notices/Store.php b/src/Notices/Store.php new file mode 100644 index 0000000..23297db --- /dev/null +++ b/src/Notices/Store.php @@ -0,0 +1,99 @@ + + */ + public function all(): array { + // Outside multisite `get_site_option()` is `get_option()`, so this reads back whatever + // put() wrote on either install type. + $queue = get_site_option( self::option_name(), [] ); + + if ( ! is_array( $queue ) ) { + return []; + } + + // Anything that is not a string message is dropped rather than printed. put() writes the + // filtered array back, so a corrupted entry heals itself. + return array_filter( $queue, 'is_string' ); + } + + /** + * Store one message, replacing any it already holds under the same key. + * + * @since 1.0.0 + * + * @param string $key Queue key, `slug:type`. + * @param string $message Resolved message. + * + * @throws Config_Exception When no hook prefix has been set. + * + * @return void + */ + public function put( string $key, string $message ): void { + $queue = $this->all(); + + $queue[ $key ] = $message; + + // One call covers both install types. Outside multisite `update_site_option()` ends in + // `update_option( $option, $value, false )`, or `add_option( $option, $value, '', false )` + // the first time — either way autoload is off, which is exactly what this queue wants: it + // is empty on almost every request and only ever read in the admin. + update_site_option( self::option_name(), $queue ); + } + + /** + * @since 1.0.0 + * + * @throws Config_Exception When no hook prefix has been set. + * + * @return void + */ + public function clear(): void { + // Outside multisite `delete_site_option()` is `delete_option()`. + delete_site_option( self::option_name() ); + } +} diff --git a/tests/unit/ConfigTest.php b/tests/unit/ConfigTest.php index 8a41a82..169526d 100644 --- a/tests/unit/ConfigTest.php +++ b/tests/unit/ConfigTest.php @@ -105,6 +105,51 @@ public function test_a_hook_name_needs_a_prefix(): void { Config::get_hook_name( 'conflict_policy' ); } + /** + * The assertion that keeps hook names and option names apart. Only the storage key is folded, + * so a host that passed `Give-Core` can still hook the filter name it was given. + */ + public function test_a_hook_name_keeps_the_prefix_verbatim(): void { + Config::set_hook_prefix( 'Give-Core' ); + + $this->assertSame( + 'Give-Core/plugin_absorber/should_load', + Config::get_hook_name( 'should_load' ) + ); + } + + /** + * @dataProvider option_name_prefixes + * + * @param string $prefix Prefix under test. + * @param string $expected Option name it must produce. + */ + public function test_it_builds_an_option_name_from_a_normalised_prefix( + string $prefix, + string $expected + ): void { + Config::set_hook_prefix( $prefix ); + + $this->assertSame( $expected, Config::get_option_name( 'notices' ) ); + } + + /** + * @return Generator + */ + public static function option_name_prefixes(): Generator { + yield 'nothing to fold' => [ 'give', 'give_plugin_absorber_notices' ]; + yield 'underscore is kept' => [ 'give_recurring', 'give_recurring_plugin_absorber_notices' ]; + yield 'mixed case' => [ 'GiveRecurring', 'giverecurring_plugin_absorber_notices' ]; + yield 'hyphen' => [ 'give-recurring', 'give_recurring_plugin_absorber_notices' ]; + yield 'mixed case and hyphen' => [ 'Give-Core', 'give_core_plugin_absorber_notices' ]; + } + + public function test_an_option_name_needs_a_prefix(): void { + $this->expectException( Config_Exception::class ); + + Config::get_option_name( 'notices' ); + } + public function test_it_reports_no_container_by_default(): void { $this->assertFalse( Config::has_container() ); $this->assertNull( Config::get_container() ); diff --git a/tests/unit/Notices/QueueTest.php b/tests/unit/Notices/QueueTest.php new file mode 100644 index 0000000..b8dd041 --- /dev/null +++ b/tests/unit/Notices/QueueTest.php @@ -0,0 +1,691 @@ +clear_queue(); + + // render() consumes the queue, so it is gated on a capability. Most tests care about the + // queue rather than the gate, so they run as someone who has it. + $user_id = $this->create_user( 'administrator' ); + + // On multisite activate_plugins is a network capability, so an administrator of a site is + // not enough — see test_a_site_administrator_on_multisite_cannot_consume_the_queue(). + if ( is_multisite() ) { + grant_super_admin( $user_id ); + } + + wp_set_current_user( $user_id ); + } + + public function tearDown(): void { + $this->clear_queue(); + delete_site_option( self::OPTION_FOR_OTHER_PREFIX ); + Config_State::reset(); + parent::tearDown(); + } + + public function test_the_default_notices_satisfy_the_contract(): void { + $this->assertInstanceOf( Queue_Interface::class, new Queue() ); + } + + /** + * @dataProvider queued_notices + * + * @param string $method Method on Queue that queues the notice. + * @param array $overrides Sub-plugin config overrides. + * @param string $key Queue key the notice must land under. + * @param string $expected Expected message, whole or partial. + * @param bool $exact Whether $expected is the whole message. + */ + public function test_it_queues_a_notice( + string $method, + array $overrides, + string $key, + string $expected, + bool $exact + ): void { + $notices = new Queue(); + $notices->{$method}( $this->make_sub_plugin( $overrides ) ); + + $queue = $this->queue(); + + $this->assertArrayHasKey( $key, $queue ); + + if ( $exact ) { + $this->assertSame( $expected, $queue[ $key ] ); + + return; + } + + // The fallbacks are not pinned word for word — they are allowed to be reworded, as long + // as they still name the sub-plugin and are not empty. + $this->assertStringContainsString( $expected, $queue[ $key ] ); + $this->assertNotSame( $expected, $queue[ $key ] ); + } + + /** + * Both the configured message and the fallback for each of the three notice types. The + * fallbacks are covered here rather than in their own methods because the assertion is the + * same one: the right message lands under the right `slug:type` key. + * + * @return Generator,2:string,3:string,4:bool}> + */ + public static function queued_notices(): Generator { + yield 'merge, configured' => [ + 'queue_merge_notice', + [ 'conflict_notice_message' => static fn() => 'Bundled now.' ], + 'give-recurring:merge', + 'Bundled now.', + true, + ]; + + yield 'merge, fallback' => [ + 'queue_merge_notice', + [], + 'give-recurring:merge', + 'give-recurring', + false, + ]; + + yield 'conflict, configured' => [ + 'queue_conflict_notice', + [ 'conflict_notice_message' => static fn() => 'Bundled now.' ], + 'give-recurring:conflict', + 'Bundled now.', + true, + ]; + + yield 'conflict, fallback' => [ + 'queue_conflict_notice', + [], + 'give-recurring:conflict', + 'give-recurring', + false, + ]; + + yield 'dependency, configured' => [ + 'queue_dependency_notice', + [ 'dependency_notice_message' => static fn() => 'Needs Give.' ], + 'give-recurring:dependency', + 'Needs Give.', + true, + ]; + + yield 'dependency, fallback' => [ + 'queue_dependency_notice', + [], + 'give-recurring:dependency', + 'give-recurring could not be loaded because its requirements are not met.', + true, + ]; + } + + /** + * The two conflict-flavoured notices say opposite things — one reports a deactivation that + * already happened, the other asks the user to do it. Sharing a default would be wrong. + */ + public function test_the_merge_and_conflict_defaults_differ(): void { + $notices = new Queue(); + $notices->queue_merge_notice( $this->make_sub_plugin() ); + $notices->queue_conflict_notice( $this->make_sub_plugin() ); + + $queue = $this->queue(); + + $this->assertNotSame( $queue['give-recurring:merge'], $queue['give-recurring:conflict'] ); + } + + public function test_a_configured_message_is_used_for_both_conflict_types(): void { + $notices = new Queue(); + $notices->queue_merge_notice( + $this->make_sub_plugin( [ 'conflict_notice_message' => static fn() => 'Ours.' ] ) + ); + $notices->queue_conflict_notice( + $this->make_sub_plugin( [ 'conflict_notice_message' => static fn() => 'Ours.' ] ) + ); + + $queue = $this->queue(); + + $this->assertSame( 'Ours.', $queue['give-recurring:merge'] ); + $this->assertSame( 'Ours.', $queue['give-recurring:conflict'] ); + } + + public function test_queueing_the_same_slug_and_type_twice_does_not_duplicate(): void { + $notices = new Queue(); + $notices->queue_merge_notice( $this->make_sub_plugin() ); + $notices->queue_merge_notice( $this->make_sub_plugin() ); + + $this->assertCount( 1, $this->queue() ); + } + + public function test_one_slug_can_hold_notices_of_different_types(): void { + $notices = new Queue(); + $notices->queue_merge_notice( $this->make_sub_plugin() ); + $notices->queue_dependency_notice( $this->make_sub_plugin() ); + + $this->assertCount( 2, $this->queue() ); + } + + public function test_different_slugs_do_not_collide(): void { + $notices = new Queue(); + $notices->queue_merge_notice( $this->make_sub_plugin() ); + $notices->queue_merge_notice( $this->make_sub_plugin( [ 'slug' => 'give-fee-recovery' ] ) ); + + $queue = $this->queue(); + + $this->assertCount( 2, $queue ); + $this->assertArrayHasKey( 'give-recurring:merge', $queue ); + $this->assertArrayHasKey( 'give-fee-recovery:merge', $queue ); + } + + public function test_render_outputs_dismissible_markup(): void { + $notices = new Queue(); + $notices->queue_merge_notice( + $this->make_sub_plugin( [ 'conflict_notice_message' => static fn() => 'Bundled now.' ] ) + ); + + $output = $this->render_to_string( $notices ); + + $this->assertStringContainsString( 'is-dismissible', $output ); + $this->assertStringContainsString( 'Bundled now.', $output ); + } + + /** + * @dataProvider notice_severities + * + * @param string $method Method on Queue that queues the notice. + * @param string $class Expected `notice-*` class. + */ + public function test_render_uses_the_severity_of_the_notice_type( string $method, string $class ): void { + $notices = new Queue(); + $notices->{$method}( + $this->make_sub_plugin( [ 'conflict_notice_message' => static fn() => 'Something happened.' ] ) + ); + + $this->assertStringContainsString( 'notice ' . $class . ' is-dismissible', $this->render_to_string( $notices ) ); + } + + /** + * A dependency notice reports a plugin that did not load, which is an error; the conflict + * pair report something the library handled, which is a warning. + * + * @return Generator + */ + public static function notice_severities(): Generator { + yield 'merge' => [ 'queue_merge_notice', 'notice-warning' ]; + yield 'conflict' => [ 'queue_conflict_notice', 'notice-warning' ]; + yield 'dependency' => [ 'queue_dependency_notice', 'notice-error' ]; + } + + public function test_render_strips_a_script_from_the_message(): void { + $notices = new Queue(); + $notices->queue_merge_notice( + $this->make_sub_plugin( [ 'conflict_notice_message' => static fn() => 'Careful.' ] ) + ); + + $output = $this->render_to_string( $notices ); + + // `wp_kses_post()` drops the disallowed tag and keeps the text it wrapped, so the payload + // survives as inert text rather than as markup the browser would run. + $this->assertStringNotContainsString( 'assertStringContainsString( 'alert(1)', $output ); + } + + /** + * Messages come from the host's own configuration or filters rather than from user input, so a + * message is allowed to carry a link — to the knowledge-base article explaining the merge, + * typically — while the event handler a message must never be able to ship is stripped. + */ + public function test_render_keeps_a_link_but_not_an_event_handler(): void { + $notices = new Queue(); + $notices->queue_merge_notice( + $this->make_sub_plugin( + [ 'conflict_notice_message' => static fn() => 'See the docs.' ] + ) + ); + + $output = $this->render_to_string( $notices ); + + $this->assertStringContainsString( 'the docs', $output ); + $this->assertStringNotContainsString( 'onclick', $output ); + } + + public function test_render_clears_the_queue(): void { + $notices = new Queue(); + $notices->queue_merge_notice( $this->make_sub_plugin() ); + + $this->render_to_string( $notices ); + + $this->assertFalse( $this->queue_exists() ); + $this->assertSame( '', $this->render_to_string( $notices ), 'A second render must output nothing.' ); + } + + public function test_render_outputs_every_queued_notice(): void { + $notices = new Queue(); + $notices->queue_merge_notice( + $this->make_sub_plugin( [ 'conflict_notice_message' => static fn() => 'First.' ] ) + ); + $notices->queue_dependency_notice( + $this->make_sub_plugin( [ 'dependency_notice_message' => static fn() => 'Second.' ] ) + ); + + $output = $this->render_to_string( $notices ); + + $this->assertStringContainsString( 'First.', $output ); + $this->assertStringContainsString( 'Second.', $output ); + } + + public function test_render_outputs_nothing_when_the_queue_is_empty(): void { + $this->assertSame( '', $this->render_to_string( new Queue() ) ); + } + + /** + * Where notices are kept is a constructor argument, so a host can move the queue somewhere else + * without also taking on how notices are worded or drawn. Both arguments default, so + * `new Queue()` — which is what Loader::resolve() builds — is unaffected. + */ + public function test_a_replacement_store_is_used_instead_of_the_option(): void { + $store = new class() extends Store { + /** + * @var array + */ + public $written = []; + + /** + * @return array + */ + public function all(): array { + return $this->written; + } + + /** + * @param string $key Queue key. + * @param string $message Resolved message. + * + * @return void + */ + public function put( string $key, string $message ): void { + $this->written[ $key ] = $message; + } + + /** + * @return void + */ + public function clear(): void { + $this->written = []; + } + }; + + ( new Queue( $store ) )->queue_merge_notice( + $this->make_sub_plugin( [ 'conflict_notice_message' => static fn() => 'Bundled now.' ] ) + ); + + $this->assertSame( [ 'give-recurring:merge' => 'Bundled now.' ], $store->written ); + $this->assertFalse( $this->queue_exists(), 'The default option must not have been written to.' ); + } + + /** + * The other half of the same seam: different markup, same queue and same consumption rules. + */ + public function test_a_replacement_renderer_draws_the_queue(): void { + $renderer = new class() extends Renderer { + /** + * @param array $queue Queue to draw. + * + * @return void + */ + public function render( array $queue ): void { + echo '

' . count( $queue ) . '

'; + } + }; + + $notices = new Queue( null, $renderer ); + $notices->queue_merge_notice( $this->make_sub_plugin() ); + + $this->assertSame( '

1

', $this->render_to_string( $notices ) ); + $this->assertFalse( $this->queue_exists(), 'A replacement renderer still consumes the queue.' ); + } + + /** + * The capability gate guards the clearing as much as the drawing, so it has to sit in front of + * the renderer rather than inside it: a user who may not see the queue must not destroy it. + */ + public function test_a_replacement_renderer_is_never_reached_without_the_capability(): void { + $renderer = new class() extends Renderer { + /** + * @var bool + */ + public $called = false; + + /** + * @param array $queue Queue to draw. + * + * @return void + */ + public function render( array $queue ): void { + $this->called = true; + } + }; + + $notices = new Queue( null, $renderer ); + $notices->queue_merge_notice( $this->make_sub_plugin() ); + + wp_set_current_user( $this->create_user( 'subscriber' ) ); + + $this->render_to_string( $notices ); + + $this->assertFalse( $renderer->called ); + $this->assertTrue( $this->queue_exists() ); + } + + /** + * Rendering consumes the queue, so a user who cannot act on the notice must neither see it + * nor destroy it. The merge notice is raised once and never re-queued. + * + * @dataProvider users_who_cannot_activate_plugins + * + * @param string|null $role Role to render as, or null for a logged-out visitor. + */ + public function test_render_does_nothing_for_a_user_who_cannot_activate_plugins( ?string $role ): void { + $notices = new Queue(); + $notices->queue_merge_notice( + $this->make_sub_plugin( [ 'conflict_notice_message' => static fn() => 'Bundled now.' ] ) + ); + + wp_set_current_user( $role === null ? 0 : $this->create_user( $role ) ); + + $this->assertSame( '', $this->render_to_string( $notices ) ); + $this->assertTrue( $this->queue_exists(), 'The queue must survive for someone who can act on it.' ); + } + + /** + * @return Generator + */ + public static function users_who_cannot_activate_plugins(): Generator { + yield 'a subscriber' => [ 'subscriber' ]; + yield 'a logged-out visitor' => [ null ]; + } + + /** + * Surprising but intended: on multisite `activate_plugins` maps through + * `manage_network_plugins`, which only a super admin has unless the network has opened the + * plugins menu to site admins. So the person who installed the plugin on their own site is + * not the person who sees the notice — a network administrator is. + */ + public function test_a_site_administrator_on_multisite_cannot_consume_the_queue(): void { + if ( ! is_multisite() ) { + $this->markTestSkipped( 'Outside multisite an administrator simply has activate_plugins.' ); + } + + $notices = new Queue(); + $notices->queue_merge_notice( + $this->make_sub_plugin( [ 'conflict_notice_message' => static fn() => 'Bundled now.' ] ) + ); + + wp_set_current_user( $this->create_user( 'administrator' ) ); + + $this->assertSame( '', $this->render_to_string( $notices ) ); + $this->assertTrue( $this->queue_exists(), 'The queue must survive for the network administrator.' ); + } + + /** + * The resolver redirects, so the queue has to come back off a durable database row rather + * than out of the object cache the redirecting request happened to warm. Asserting the row + * itself, not just that a flush is survivable: on a site with no persistent object cache a + * transient lands in the options table too, so a flush test alone would pass for the + * transient-backed design this class exists to avoid. + */ + public function test_the_queue_is_a_durable_database_row(): void { + ( new Queue() )->queue_merge_notice( + $this->make_sub_plugin( [ 'conflict_notice_message' => static fn() => 'Bundled now.' ] ) + ); + + $this->assertStringContainsString( + 'Bundled now.', + $this->stored_row(), + 'The queue must be a row in the database, not a cache entry.' + ); + + wp_cache_flush(); + + $this->assertStringContainsString( 'Bundled now.', $this->render_to_string( new Queue() ) ); + } + + /** + * @dataProvider malformed_queues + * + * @param mixed $stored Raw option value to seed. + * @param string|null $present Substring the output must contain, or null when nothing at + * all should be rendered. + * @param array $absent Substrings the output must not contain. + */ + public function test_render_ignores_anything_that_is_not_a_message( $stored, ?string $present, array $absent ): void { + $this->seed_queue( $stored ); + + $output = $this->render_to_string( new Queue() ); + + if ( $present === null ) { + $this->assertSame( '', $output ); + } else { + $this->assertStringContainsString( $present, $output ); + } + + foreach ( $absent as $needle ) { + $this->assertStringNotContainsString( $needle, $output ); + } + } + + /** + * The first two are the likeliest real corruption: another plugin, or a host reading and + * rewriting the option, leaves something behind that is not an array at all. The rest are + * per-entry rubbish, which is dropped without taking the well-formed entries with it. + * + * @return Generator}> + */ + public static function malformed_queues(): Generator { + yield 'a scalar instead of an array' => [ 'not-a-queue', null, [ 'not-a-queue', 'notice' ] ]; + + yield 'an object instead of an array' => [ + (object) [ 'a:merge' => 'Nope.' ], + null, + [ 'Nope.', 'notice' ], + ]; + + yield 'entries that are not strings' => [ + [ + 'a:merge' => 'Fine.', + 'b:merge' => [ 'nested' ], + 'c:merge' => null, + 'd:merge' => 42, + ], + 'Fine.', + [ 'Array', '42' ], + ]; + + yield 'an empty message' => [ [ 'a:merge' => '' ], null, [ 'notice' ] ]; + + // A message that is only whitespace would otherwise print an empty notice box. + yield 'a whitespace-only message' => [ [ 'a:merge' => " \n\t" ], null, [ 'notice' ] ]; + } + + public function test_a_corrupted_queue_heals_on_the_next_write(): void { + $this->seed_queue( [ 'a:merge' => [ 'nested' ] ] ); + + ( new Queue() )->queue_merge_notice( $this->make_sub_plugin() ); + + $this->assertSame( [ 'give-recurring:merge' ], array_keys( $this->queue() ) ); + } + + public function test_the_option_is_keyed_by_the_hook_prefix(): void { + Config_State::reset(); + Config::set_hook_prefix( 'woo' ); + + $this->assertSame( self::OPTION_FOR_OTHER_PREFIX, Queue::option_name() ); + + ( new Queue() )->queue_merge_notice( $this->make_sub_plugin() ); + + $this->assertIsArray( get_site_option( self::OPTION_FOR_OTHER_PREFIX, false ) ); + $this->assertFalse( $this->queue_exists() ); + } + + public function test_queueing_needs_a_hook_prefix(): void { + Config_State::reset(); + + $this->expectException( Config_Exception::class ); + + ( new Queue() )->queue_merge_notice( $this->make_sub_plugin() ); + } + + /** + * The queue is empty on nearly every request and only ever read in the admin, so it must not + * ride along in the autoloaded bundle on every front-end request. + */ + public function test_the_queue_is_not_autoloaded(): void { + if ( is_multisite() ) { + $this->markTestSkipped( 'Network options are not part of the per-site autoload bundle.' ); + } + + ( new Queue() )->queue_merge_notice( $this->make_sub_plugin() ); + + $this->assertNotContains( self::OPTION, array_keys( wp_load_alloptions() ) ); + } + + /** + * The queue as the class stores it. Always an array, so callers can index and count it: use + * queue_exists() to ask whether there is a row at all. + * + * @return array + */ + private function queue(): array { + $queue = get_site_option( self::OPTION, [] ); + + return is_array( $queue ) ? $queue : []; + } + + /** + * Whether the option exists at all, which is what "render cleared the queue" means. + * + * @return bool + */ + private function queue_exists(): bool { + return get_site_option( self::OPTION, false ) !== false; + } + + /** + * The serialized option value straight out of the database, bypassing the object cache. + * + * The table name goes through the `%i` identifier placeholder rather than into the string, so + * the query stays a literal and nothing interpolated ever reaches the parser. + * + * @return string + */ + private function stored_row(): string { + /** @var wpdb $wpdb */ + global $wpdb; + + if ( is_multisite() ) { + $stored = $wpdb->get_var( + $wpdb->prepare( + 'SELECT meta_value FROM %i WHERE meta_key = %s AND site_id = %d', + $wpdb->sitemeta, + self::OPTION, + get_current_network_id() + ) + ); + } else { + $stored = $wpdb->get_var( + $wpdb->prepare( + 'SELECT option_value FROM %i WHERE option_name = %s', + $wpdb->options, + self::OPTION + ) + ); + } + + $this->assertIsString( $stored, 'The queue option has no row in the database.' ); + + return $stored; + } + + /** + * @param mixed $queue Raw queue contents, well-formed or not. + */ + private function seed_queue( $queue ): void { + update_site_option( self::OPTION, $queue ); + } + + private function clear_queue(): void { + delete_site_option( self::OPTION ); + } + + /** + * @param string $role Role to give the new user. + * + * @throws RuntimeException When the user cannot be created, rather than letting a later + * capability assertion fail for an unrelated reason. + * + * @return int + */ + private function create_user( string $role ): int { + $user_id = wp_insert_user( + [ + 'user_login' => uniqid( 'absorber-' ), + 'user_pass' => wp_generate_password(), + 'role' => $role, + ] + ); + + if ( $user_id instanceof WP_Error ) { + throw new RuntimeException( 'Could not create a ' . $role . ': ' . $user_id->get_error_message() ); + } + + return $user_id; + } + + private function render_to_string( Queue $notices ): string { + ob_start(); + + try { + $notices->render(); + } finally { + // In a finally block so a throw from render() cannot leave the suite's own output + // trapped in an abandoned buffer. + $output = (string) ob_get_clean(); + } + + return $output; + } +} diff --git a/tests/unit/Notices/RendererTest.php b/tests/unit/Notices/RendererTest.php new file mode 100644 index 0000000..47642b9 --- /dev/null +++ b/tests/unit/Notices/RendererTest.php @@ -0,0 +1,200 @@ +render( [ 'give-recurring:merge' => 'Bundled now.' ] ); + + $this->assertStringContainsString( 'is-dismissible', $output ); + $this->assertStringContainsString( 'Bundled now.', $output ); + } + + /** + * @dataProvider notice_severities + * + * @param string $key Queue key to render under. + * @param string $class Expected `notice-*` class. + */ + public function test_the_type_half_of_the_key_picks_the_severity( string $key, string $class ): void { + $this->assertStringContainsString( + 'notice ' . $class . ' is-dismissible', + $this->render( [ $key => 'Something happened.' ] ) + ); + } + + /** + * A dependency notice reports a plugin that did not load, which is an error; the conflict pair + * report something the library handled, which is a warning. An unrecognised type is drawn as a + * warning rather than dropped — it may have been written by an older version, or by a host + * reading and rewriting the option. + * + * @return Generator + */ + public static function notice_severities(): Generator { + yield 'merge' => [ 'give-recurring:' . Queue::TYPE_MERGE, 'notice-warning' ]; + yield 'conflict' => [ 'give-recurring:' . Queue::TYPE_CONFLICT, 'notice-warning' ]; + yield 'dependency' => [ 'give-recurring:' . Queue::TYPE_DEPENDENCY, 'notice-error' ]; + yield 'unknown type' => [ 'give-recurring:invented', 'notice-warning' ]; + yield 'no type at all' => [ 'give-recurring', 'notice-warning' ]; + } + + /** + * A slug containing a colon still resolves to the type, because the type is the last segment + * rather than the second. + */ + public function test_the_type_is_the_last_segment_of_the_key(): void { + $this->assertStringContainsString( + 'notice-error', + $this->render( [ 'give:recurring:' . Queue::TYPE_DEPENDENCY => 'Requirements not met.' ] ) + ); + } + + /** + * `wp_kses_post()` drops a disallowed tag but keeps the text it wrapped, so the payload lands + * on the page as inert text rather than as markup the browser would run. + */ + public function test_it_strips_a_script_from_a_message(): void { + $output = $this->render( [ 'a:merge' => 'Careful.' ] ); + + $this->assertStringNotContainsString( 'assertStringContainsString( 'alert(1)', $output ); + } + + /** + * Messages come from the host's own configuration or filters rather than from user input, so a + * message is allowed to carry a link — to the knowledge-base article explaining the merge, + * typically — and it has to reach the screen as an anchor. + */ + public function test_it_keeps_a_link_in_a_message(): void { + $output = $this->render( [ 'a:merge' => 'See the docs.' ] ); + + $this->assertStringContainsString( 'the docs', $output ); + } + + /** + * The allowlist is per attribute, not per tag: the anchor a host wants survives while the event + * handler it must never be able to ship does not. That split is what makes allowing markup safe. + */ + public function test_it_strips_an_event_handler_from_a_link(): void { + $output = $this->render( + [ 'a:merge' => 'See the docs.' ] + ); + + $this->assertStringNotContainsString( 'onclick', $output ); + $this->assertStringContainsString( 'the docs', $output ); + } + + /** + * A bare message still has to reach the screen as a paragraph: `.notice` styles the `

` inside + * it, and text sitting directly in the div loses that spacing. + */ + public function test_it_wraps_a_bare_message_in_a_paragraph(): void { + $this->assertStringContainsString( '

Bundled now.

', $this->render( [ 'a:merge' => 'Bundled now.' ] ) ); + } + + /** + * The same allowlist that keeps a link keeps a `

`, so a host may well send one. Wrapping that + * in another paragraph is markup no browser can honour: the parser closes the outer `

` at the + * inner one and leaves a stray `

` behind, so the notice renders with an empty leading line. + */ + public function test_it_does_not_wrap_a_paragraph_the_host_already_wrote(): void { + $output = $this->render( [ 'a:merge' => '

Bundled now.

' ] ); + + $this->assertStringNotContainsString( '

', $output ); + $this->assertSame( 1, substr_count( $output, '

' ), 'One paragraph in, one paragraph out.' ); + } + + /** + * The class docblock promises a list survives, and a `

    ` inside a `

    ` is not a list that + * survived: the browser closes the paragraph before it, orphaning the closing tag. + */ + public function test_it_keeps_a_list_out_of_the_paragraph(): void { + $output = $this->render( [ 'a:merge' => 'Requires:

    • Give
    ' ] ); + + $this->assertMatchesRegularExpression( '#

    \s*
      #', $output, 'The paragraph must close first.' ); + $this->assertStringContainsString( '
    • Give
      • ', $output ); + } + + /** + * A blank line is the only paragraph break available to a host whose message is a plain + * translated string, so it has to survive as one rather than collapse into running text. + */ + public function test_a_blank_line_starts_a_second_paragraph(): void { + $this->assertSame( 2, substr_count( $this->render( [ 'a:merge' => "One.\n\nTwo." ] ), '

      ' ) ); + } + + /** + * @dataProvider empty_messages + * + * @param string $message Message that must print nothing at all. + */ + public function test_it_skips_a_message_with_nothing_in_it( string $message ): void { + $this->assertSame( '', $this->render( [ 'a:merge' => $message ] ) ); + } + + /** + * @return Generator + */ + public static function empty_messages(): Generator { + yield 'an empty message' => [ '' ]; + + // Whitespace would otherwise print an empty notice box, which reads as a bug. + yield 'a whitespace-only message' => [ " \n\t" ]; + + // So would a message that filtering empties: `wp_kses_post()` keeps the text a disallowed + // tag wrapped, and here there is none. + yield 'a message that is only disallowed markup' => [ '' ]; + } + + public function test_an_empty_queue_prints_nothing(): void { + $this->assertSame( '', $this->render( [] ) ); + } + + public function test_it_prints_every_message_it_is_given(): void { + $output = $this->render( + [ + 'a:merge' => 'First.', + 'b:dependency' => 'Second.', + ] + ); + + $this->assertStringContainsString( 'First.', $output ); + $this->assertStringContainsString( 'Second.', $output ); + } + + /** + * @param array $queue Queue to draw. + * + * @return string + */ + private function render( array $queue ): string { + ob_start(); + + try { + ( new Renderer() )->render( $queue ); + } finally { + // In a finally block so a throw from render() cannot leave the suite's own output + // trapped in an abandoned buffer. + $output = (string) ob_get_clean(); + } + + return $output; + } +} diff --git a/tests/unit/Notices/StoreTest.php b/tests/unit/Notices/StoreTest.php new file mode 100644 index 0000000..464c0eb --- /dev/null +++ b/tests/unit/Notices/StoreTest.php @@ -0,0 +1,182 @@ +put( 'give-recurring:merge', 'Bundled now.' ); + + $this->assertSame( [ 'give-recurring:merge' => 'Bundled now.' ], ( new Store() )->all() ); + } + + /** + * A second instance reads what the first wrote: the queue lives in the database, not in the + * object that happened to write it. The resolver redirects, so the reading request is almost + * never the writing one. + */ + public function test_the_queue_outlives_the_instance_that_wrote_it(): void { + ( new Store() )->put( 'give-recurring:merge', 'Bundled now.' ); + + wp_cache_flush(); + + $this->assertSame( 'Bundled now.', ( new Store() )->all()['give-recurring:merge'] ?? '' ); + } + + public function test_writing_the_same_key_twice_replaces_rather_than_duplicates(): void { + $store = new Store(); + $store->put( 'give-recurring:merge', 'First.' ); + $store->put( 'give-recurring:merge', 'Second.' ); + + $this->assertSame( [ 'give-recurring:merge' => 'Second.' ], $store->all() ); + } + + public function test_different_keys_coexist(): void { + $store = new Store(); + $store->put( 'give-recurring:merge', 'One.' ); + $store->put( 'give-recurring:dependency', 'Two.' ); + + $this->assertCount( 2, $store->all() ); + } + + public function test_clear_removes_the_row_entirely(): void { + $store = new Store(); + $store->put( 'give-recurring:merge', 'Bundled now.' ); + + $store->clear(); + + $this->assertFalse( get_site_option( self::OPTION, false ), 'The option must be gone, not emptied.' ); + $this->assertSame( [], $store->all() ); + } + + /** + * @dataProvider malformed_queues + * + * @param mixed $stored Raw option value to seed. + * @param array $expected What all() must return for it. + */ + public function test_it_drops_anything_that_is_not_a_message( $stored, array $expected ): void { + update_site_option( self::OPTION, $stored ); + + $this->assertSame( $expected, ( new Store() )->all() ); + } + + /** + * The first two are the likeliest real corruption: another plugin, or a host reading and + * rewriting the option, leaves behind something that is not an array at all. The rest are + * per-entry rubbish, which is dropped without taking the well-formed entries with it. + * + * @return Generator}> + */ + public static function malformed_queues(): Generator { + yield 'a scalar instead of an array' => [ 'not-a-queue', [] ]; + + yield 'an object instead of an array' => [ (object) [ 'a:merge' => 'Nope.' ], [] ]; + + yield 'entries that are not strings' => [ + [ + 'a:merge' => 'Fine.', + 'b:merge' => [ 'nested' ], + 'c:merge' => null, + 'd:merge' => 42, + ], + [ 'a:merge' => 'Fine.' ], + ]; + } + + public function test_a_corrupted_queue_heals_on_the_next_write(): void { + update_site_option( self::OPTION, [ 'a:merge' => [ 'nested' ] ] ); + + ( new Store() )->put( 'give-recurring:merge', 'Bundled now.' ); + + $this->assertSame( [ 'give-recurring:merge' ], array_keys( ( new Store() )->all() ) ); + } + + public function test_the_option_is_keyed_by_the_hook_prefix(): void { + Config_State::reset(); + Config::set_hook_prefix( 'woo' ); + + $this->assertSame( self::OPTION_WOO, Store::option_name() ); + } + + /** + * The hook prefix is allowed mixed case and hyphens because it names filters. An option name + * is a storage key, so the prefix reaches the database folded rather than raw. + */ + public function test_the_option_name_normalises_the_hook_prefix(): void { + Config_State::reset(); + Config::set_hook_prefix( 'Give-Core' ); + + $this->assertSame( self::OPTION_NORMALISED, Store::option_name() ); + + ( new Store() )->put( 'give-recurring:merge', 'Bundled now.' ); + + $this->assertSame( + [ 'give-recurring:merge' => 'Bundled now.' ], + get_site_option( self::OPTION_NORMALISED ), + 'The queue must be written under the normalised name.' + ); + } + + public function test_it_needs_a_hook_prefix(): void { + Config_State::reset(); + + $this->expectException( Config_Exception::class ); + + ( new Store() )->put( 'give-recurring:merge', 'Bundled now.' ); + } + + /** + * The queue is empty on nearly every request and only ever read in the admin, so it must not + * ride along in the autoloaded bundle on every front-end request. + */ + public function test_the_queue_is_not_autoloaded(): void { + if ( is_multisite() ) { + $this->markTestSkipped( 'Network options are not part of the per-site autoload bundle.' ); + } + + ( new Store() )->put( 'give-recurring:merge', 'Bundled now.' ); + + $this->assertNotContains( self::OPTION, array_keys( wp_load_alloptions() ) ); + } +}