diff --git a/README.md b/README.md index a463080..fe5bd41 100644 --- a/README.md +++ b/README.md @@ -94,6 +94,11 @@ Config::set_container( $container ); | Interface | Default | Responsibility | |---|---|---| | `Contracts\Registrar_Interface` | `Registrar` | Holds the registered sub-plugins. | +| `Contracts\Notices_Interface` | `Notices` | Notice queue and rendering. | + +The default notices queue into the option `{prefix}_plugin_absorber_notices` — a network option on +multisite — and render for users who can `activate_plugins`. Read `Notices::option_name()` if you +want to render them yourself without replacing the queue. **Set the container before your first `Loader::register()` call.** Resolution is memoized, so a container set afterwards is not consulted. `Loader::reset()` does discard the memo — but it also diff --git a/docs/superpowers/plans/2026-07-31-plugin-absorber.md b/docs/superpowers/plans/2026-07-31-plugin-absorber.md index 3904005..9a14bd5 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 `"{$hook_prefix}_plugin_absorber_activations"`, option `"{$hook_prefix}_plugin_absorber_notices"`. **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: @@ -2802,7 +2802,32 @@ Lands before the load path and the resolver because both call into it. 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 `"{$hook_prefix}_plugin_absorber_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. - 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. @@ -3249,7 +3274,7 @@ gh pr create --base 09-loader-resolve --title "Notice queue" --body 'What: the t Usage: Loader::notices()->queue_merge_notice( $sub_plugin ); - Loader::notices()->render(); // hooked to admin_notices by boot() + Loader::notices()->render(); // hooked to all_admin_notices by boot() // Or supply your own copy: "conflict_notice_message" => static fn() => __( "Now bundled with Give.", "give" ), @@ -3290,7 +3315,7 @@ redirect (queue with one instance, render with another).' Task 12 adds the `plugins_loaded` @1 hook to `boot()`; Task 13 adds the activation call to `load()`. -**Design note:** `boot()` wires only the @2 load hook and `admin_notices` in this PR. The @1 conflict-resolution hook arrives in Task 12 with the resolver it delegates to — wiring a trampoline to a collaborator that does not exist yet would not run. +**Design note:** `boot()` wires only the @2 load hook and `all_admin_notices` in this PR. The @1 conflict-resolution hook arrives in Task 12 with the resolver it delegates to — wiring a trampoline to a collaborator that does not exist yet would not run. - [ ] **Step 1: Cut the branch** @@ -3519,7 +3544,7 @@ class LoaderBootTest extends WPTestCase { public function tearDown(): void { remove_all_actions( 'plugins_loaded' ); - remove_all_actions( 'admin_notices' ); + remove_all_actions( 'all_admin_notices' ); Loader::reset(); Config::reset(); parent::tearDown(); @@ -3548,7 +3573,7 @@ class LoaderBootTest extends WPTestCase { Loader::boot(); - $this->assertNotFalse( has_action( 'admin_notices', [ Loader::class, 'render_notices' ] ) ); + $this->assertNotFalse( has_action( 'all_admin_notices', [ Loader::class, 'render_notices' ] ) ); set_current_screen( 'front' ); } @@ -3595,7 +3620,11 @@ Append these methods, and extend `reset()` as shown at the end: add_action( 'plugins_loaded', [ self::class, 'load_all' ], 2 ); if ( is_admin() ) { - add_action( 'admin_notices', [ self::class, 'render_notices' ] ); + // all_admin_notices, not admin_notices. WordPress dispatches admin_notices, + // network_admin_notices and user_admin_notices as mutually exclusive branches, so a + // superadmin working in the network admin -- exactly where a network-wide + // deactivation gets noticed -- would never see the queue rendered. + add_action( 'all_admin_notices', [ self::class, 'render_notices' ] ); } } @@ -3747,7 +3776,7 @@ gh pr create --base 10-notices-queue --title "Loader boot and load path" --body Usage: Loader::register( [ ... ] ); - Loader::boot(); // wires plugins_loaded @2 and admin_notices + Loader::boot(); // wires plugins_loaded @2 and all_admin_notices add_filter( "give/plugin_absorber/should_load", function ( $should_load, $sub_plugin ) { return $should_load; @@ -5054,11 +5083,11 @@ Expected: FAIL — `Call to undefined method Nexcess\PluginAbsorber\Notices::fil } ``` -And inside `boot()`'s `is_admin()` block, beside the existing `admin_notices` line: +And inside `boot()`'s `is_admin()` block, beside the existing `all_admin_notices` line: ```php if ( is_admin() ) { - add_action( 'admin_notices', [ self::class, 'render_notices' ] ); + add_action( 'all_admin_notices', [ self::class, 'render_notices' ] ); add_filter( 'wp_admin_notice_markup', [ self::class, 'filter_activation_error_markup' ] ); } ``` diff --git a/src/Contracts/Notices_Interface.php b/src/Contracts/Notices_Interface.php new file mode 100644 index 0000000..a75c04e --- /dev/null +++ b/src/Contracts/Notices_Interface.php @@ -0,0 +1,76 @@ +queue( + $sub_plugin, + self::TYPE_MERGE, + $sub_plugin->get_conflict_notice_message( + sprintf( + '%s has been deactivated because it is now bundled and loaded automatically.', + $sub_plugin->get_slug() + ) + ) + ); + } + + /** + * The default differs from the merge notice's on purpose: this one asks the user to act, + * where that one reports something already done. + * + * @since 1.0.0 + * + * @param Sub_Plugin $sub_plugin Sub-plugin concerned. + * + * @throws Config_Exception When no hook prefix has been set. + * + * @return void + */ + public function queue_conflict_notice( Sub_Plugin $sub_plugin ): void { + $this->queue( + $sub_plugin, + self::TYPE_CONFLICT, + $sub_plugin->get_conflict_notice_message( + sprintf( + '%s is now bundled and loaded automatically. You can safely deactivate the standalone plugin.', + $sub_plugin->get_slug() + ) + ) + ); + } + + /** + * @since 1.0.0 + * + * @param Sub_Plugin $sub_plugin Sub-plugin concerned. + * + * @throws Config_Exception When no hook prefix has been set. + * + * @return void + */ + public function queue_dependency_notice( Sub_Plugin $sub_plugin ): void { + $this->queue( $sub_plugin, self::TYPE_DEPENDENCY, $sub_plugin->get_dependency_notice_message() ); + } + + /** + * @since 1.0.0 + * + * @throws Config_Exception When no hook prefix has been set. + * + * @return void + */ + public function render(): void { + if ( ! current_user_can( self::CAPABILITY ) ) { + return; + } + + $queue = $this->get_queue(); + + if ( $queue === [] ) { + return; + } + + foreach ( $queue as $message ) { + if ( $message === '' ) { + continue; + } + + printf( + '

%s

', + esc_html( $message ) + ); + } + + $this->clear_queue(); + } + + /** + * The option name backing the queue. Read it directly to render these notices yourself. + * + * @since 1.0.0 + * + * @throws Config_Exception When no hook prefix has been set. + * + * @return string + */ + public static function option_name(): string { + return Config::get_hook_prefix() . '_plugin_absorber_notices'; + } + + /** + * Store one notice, keyed by slug and type so different types can coexist. + * + * A sub-plugin can legitimately earn a merge notice while the conflict is resolved and a + * dependency notice while the load is attempted, in the same request. Keying by slug alone + * would silently drop one of them. + * + * @since 1.0.0 + * + * @param Sub_Plugin $sub_plugin Sub-plugin concerned. + * @param string $type Notice type. + * @param string $message Resolved message. + * + * @throws Config_Exception When no hook prefix has been set. + * + * @return void + */ + private function queue( Sub_Plugin $sub_plugin, string $type, string $message ): void { + $queue = $this->get_queue(); + + $queue[ $sub_plugin->get_slug() . ':' . $type ] = $message; + + if ( is_multisite() ) { + update_site_option( self::option_name(), $queue ); + + return; + } + + // Not autoloaded: the queue is empty on almost every request, and only the admin reads it. + update_option( self::option_name(), $queue, false ); + } + + /** + * @since 1.0.0 + * + * @throws Config_Exception When no hook prefix has been set. + * + * @return array + */ + private function get_queue(): array { + $queue = is_multisite() + ? get_site_option( self::option_name(), [] ) + : get_option( self::option_name(), [] ); + + if ( ! is_array( $queue ) ) { + return []; + } + + // Anything that is not a string message is dropped rather than printed. queue() writes + // the filtered array back, so a corrupted entry heals itself. + return array_filter( $queue, 'is_string' ); + } + + /** + * @since 1.0.0 + * + * @throws Config_Exception When no hook prefix has been set. + * + * @return void + */ + private function clear_queue(): void { + if ( is_multisite() ) { + delete_site_option( self::option_name() ); + + return; + } + + delete_option( self::option_name() ); + } +} diff --git a/tests/unit/NoticesTest.php b/tests/unit/NoticesTest.php new file mode 100644 index 0000000..2a9d7d5 --- /dev/null +++ b/tests/unit/NoticesTest.php @@ -0,0 +1,353 @@ +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->factory()->user->create( [ 'role' => 'administrator' ] ); + + // On multisite, activate_plugins maps through manage_network_plugins unless the network + // has opened the plugins menu to site admins, so a plain administrator is not enough. + if ( is_multisite() ) { + ( new WP_User( $user_id ) )->add_cap( 'manage_network_plugins' ); + } + + wp_set_current_user( $user_id ); + } + + public function tearDown(): void { + $this->clear_queue(); + delete_option( 'woo_plugin_absorber_notices' ); + delete_site_option( 'woo_plugin_absorber_notices' ); + Loader::reset(); + Config::reset(); + parent::tearDown(); + } + + private function clear_queue(): void { + delete_option( self::OPTION ); + delete_site_option( self::OPTION ); + } + + /** + * @return array|false + */ + private function queue() { + return is_multisite() + ? get_site_option( self::OPTION, false ) + : get_option( self::OPTION, false ); + } + + /** + * @param array $queue Raw queue contents. + */ + private function seed_queue( array $queue ): void { + if ( is_multisite() ) { + update_site_option( self::OPTION, $queue ); + + return; + } + + update_option( self::OPTION, $queue, false ); + } + + /** + * @param array $overrides Config overrides. + */ + private function make_sub_plugin( array $overrides = [] ): Sub_Plugin { + return new Sub_Plugin( + array_merge( + [ + 'slug' => 'give-recurring', + 'bundled_plugin_file' => '/tmp/give-recurring.php', + 'plugin_loaded_constant' => 'GIVE_RECURRING_VERSION_NOTICES', + ], + $overrides + ) + ); + } + + private function render_to_string( Notices $notices ): string { + ob_start(); + $notices->render(); + + return (string) ob_get_clean(); + } + + public function test_the_loader_resolves_the_default_notices(): void { + $this->assertInstanceOf( Notices::class, Loader::notices() ); + } + + public function test_the_default_notices_satisfy_the_contract(): void { + $this->assertInstanceOf( Notices_Interface::class, new Notices() ); + } + + public function test_it_queues_a_merge_notice(): void { + ( new Notices() )->queue_merge_notice( $this->make_sub_plugin( [ 'conflict_notice_message' => 'Bundled now.' ] ) ); + + $queue = $this->queue(); + + $this->assertIsArray( $queue ); + $this->assertArrayHasKey( 'give-recurring:merge', $queue ); + $this->assertSame( 'Bundled now.', $queue['give-recurring:merge'] ); + } + + public function test_the_merge_notice_falls_back_to_a_default_message(): void { + ( new Notices() )->queue_merge_notice( $this->make_sub_plugin() ); + + $queue = $this->queue(); + + $this->assertStringContainsString( 'give-recurring', $queue['give-recurring:merge'] ); + $this->assertNotSame( '', $queue['give-recurring:merge'] ); + } + + public function test_it_queues_a_conflict_notice(): void { + ( new Notices() )->queue_conflict_notice( $this->make_sub_plugin() ); + + $this->assertArrayHasKey( 'give-recurring:conflict', $this->queue() ); + } + + /** + * 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 Notices(); + $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 Notices(); + $notices->queue_merge_notice( $this->make_sub_plugin( [ 'conflict_notice_message' => 'Ours.' ] ) ); + $notices->queue_conflict_notice( $this->make_sub_plugin( [ 'conflict_notice_message' => 'Ours.' ] ) ); + + $queue = $this->queue(); + + $this->assertSame( 'Ours.', $queue['give-recurring:merge'] ); + $this->assertSame( 'Ours.', $queue['give-recurring:conflict'] ); + } + + public function test_it_queues_a_dependency_notice_using_the_sub_plugin_message(): void { + ( new Notices() )->queue_dependency_notice( $this->make_sub_plugin( [ 'dependency_notice_message' => 'Needs Give.' ] ) ); + + $this->assertSame( 'Needs Give.', $this->queue()['give-recurring:dependency'] ); + } + + public function test_the_dependency_notice_falls_back_to_the_sub_plugin_default(): void { + ( new Notices() )->queue_dependency_notice( $this->make_sub_plugin() ); + + $this->assertSame( + 'give-recurring could not be loaded because its requirements are not met.', + $this->queue()['give-recurring:dependency'] + ); + } + + public function test_queueing_the_same_slug_and_type_twice_does_not_duplicate(): void { + $notices = new Notices(); + $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 Notices(); + $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 Notices(); + $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_warning_markup(): void { + $notices = new Notices(); + $notices->queue_merge_notice( $this->make_sub_plugin( [ 'conflict_notice_message' => 'Bundled now.' ] ) ); + + $output = $this->render_to_string( $notices ); + + $this->assertStringContainsString( 'notice notice-warning is-dismissible', $output ); + $this->assertStringContainsString( 'Bundled now.', $output ); + } + + public function test_render_escapes_the_message(): void { + $notices = new Notices(); + $notices->queue_merge_notice( $this->make_sub_plugin( [ 'conflict_notice_message' => '' ] ) ); + + $output = $this->render_to_string( $notices ); + + $this->assertStringNotContainsString( '