From 6a7e120f3afda5abf41abd431c455598346d5f6f Mon Sep 17 00:00:00 2001 From: Colin Stewart <79332690+costdev@users.noreply.github.com> Date: Fri, 8 Nov 2024 18:42:12 +0000 Subject: [PATCH] Branding: Add multisite support. (#160) --- .github/workflows/phpunit-tests.yml | 5 +- composer.json | 3 +- includes/class-branding.php | 35 +++++++-- phpunit.xml.dist | 5 ++ .../Branding_AdminEnqueueScriptsTest.php | 23 +++--- tests/Branding/Branding_ConstructTest.php | 76 +++++++++++++++++-- .../Branding_OutputAdminNoticeTest.php | 50 ++++++++---- tests/multisite.xml | 41 ++++++++++ 8 files changed, 197 insertions(+), 41 deletions(-) create mode 100644 tests/multisite.xml diff --git a/.github/workflows/phpunit-tests.yml b/.github/workflows/phpunit-tests.yml index 2a935a1..5df4efc 100644 --- a/.github/workflows/phpunit-tests.yml +++ b/.github/workflows/phpunit-tests.yml @@ -11,11 +11,12 @@ on: jobs: phpunit: - name: Run tests + name: Run tests (PHP ${{ matrix.php-version }}, ${{ matrix.multisite && 'Multisite' || 'Single Site' }}) runs-on: ubuntu-latest strategy: matrix: php-version: ['7.4', '8.3'] + multisite: [ true, false ] services: database: image: mysql:latest @@ -38,4 +39,4 @@ jobs: run: bash bin/install-wp-tests.sh wordpress_tests root root 127.0.0.1 latest true - name: Run tests - run: phpunit + run: XDEBUG_MODE=off phpunit${{ matrix.multisite && ' -c tests/multisite.xml' || '' }} diff --git a/composer.json b/composer.json index f7f6a35..40c90f8 100644 --- a/composer.json +++ b/composer.json @@ -26,7 +26,8 @@ "scripts": { "format": "@php ./vendor/squizlabs/php_codesniffer/bin/phpcbf --report=summary,source", "lint": "@php ./vendor/squizlabs/php_codesniffer/bin/phpcs --report=summary, source", - "test": [ "Composer\\Config::disableProcessTimeout", "@php ./vendor/phpunit/phpunit/phpunit" ] + "test": [ "Composer\\Config::disableProcessTimeout", "@php ./vendor/phpunit/phpunit/phpunit" ], + "test:multisite": [ "Composer\\Config::disableProcessTimeout", "@php ./vendor/phpunit/phpunit/phpunit -c tests/multisite.xml" ] } diff --git a/includes/class-branding.php b/includes/class-branding.php index 83c3ae3..85714d5 100644 --- a/includes/class-branding.php +++ b/includes/class-branding.php @@ -24,7 +24,8 @@ class Branding { public function __construct() { $admin_settings = Admin_Settings::get_instance(); if ( $admin_settings->get_setting( 'enable', false ) ) { - add_action( 'admin_notices', [ $this, 'output_admin_notice' ] ); + $admin_notices_hook = is_multisite() ? 'network_admin_notices' : 'admin_notices'; + add_action( $admin_notices_hook, [ $this, 'output_admin_notice' ] ); add_action( 'admin_enqueue_scripts', [ $this, 'admin_enqueue_scripts' ] ); } } @@ -53,14 +54,15 @@ public function admin_enqueue_scripts( $hook ) { } $allowed_screens = [ - 'update-core.php', - 'plugins.php', - 'plugin-install.php', - 'themes.php', - 'theme-install.php', + 'update-core', + 'plugins', + 'plugin-install', + 'themes', + 'theme-install', ]; - if ( in_array( $hook, $allowed_screens, true ) ) { + $screen = \WP_Screen::get( $hook ); + if ( in_array( $screen->id, $allowed_screens, true ) ) { wp_enqueue_style( 'aspire_update_settings_css', plugin_dir_url( __DIR__ ) . 'assets/css/aspire-update.css', [], AP_VERSION ); } } @@ -81,9 +83,15 @@ public function output_admin_notice() { } $message = ''; - switch ( $current_screen->base ) { + switch ( $current_screen->id ) { case 'plugins': case 'plugin-install': + if ( is_multisite() ) { + break; + } + // Fall-through. + case 'plugins-network': + case 'plugin-install-network': $message = sprintf( /* translators: 1: The name of the plugin, 2: The documentation URL. */ __( 'Your plugin updates are now powered by %1$s. Learn more', 'AspireUpdate' ), @@ -93,6 +101,12 @@ public function output_admin_notice() { break; case 'themes': case 'theme-install': + if ( is_multisite() ) { + break; + } + // Fall-through. + case 'themes-network': + case 'theme-install-network': $message = sprintf( /* translators: 1: The name of the plugin, 2: The documentation URL. */ __( 'Your theme updates are now powered by %1$s. Learn more', 'AspireUpdate' ), @@ -101,6 +115,11 @@ public function output_admin_notice() { ); break; case 'update-core': + if ( is_multisite() ) { + break; + } + // Fall-through. + case 'update-core-network': $message = sprintf( /* translators: 1: The name of the plugin, 2: The documentation URL. */ __( 'Your WordPress, plugin, theme and translation updates are now powered by %1$s. Learn more', 'AspireUpdate' ), diff --git a/phpunit.xml.dist b/phpunit.xml.dist index d6751ac..72d0b05 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -14,6 +14,11 @@ ./tests/ + + + ms-required + + ./aspire-update.php diff --git a/tests/Branding/Branding_AdminEnqueueScriptsTest.php b/tests/Branding/Branding_AdminEnqueueScriptsTest.php index f93c654..9176483 100644 --- a/tests/Branding/Branding_AdminEnqueueScriptsTest.php +++ b/tests/Branding/Branding_AdminEnqueueScriptsTest.php @@ -41,11 +41,11 @@ public function test_should_enqueue_style_on_certain_screens( $hook ) { public function data_hooks() { return self::text_array_to_dataprovider( [ - 'update-core.php', - 'plugins.php', - 'plugin-install.php', - 'themes.php', - 'theme-install.php', + 'update-core', + 'plugins', + 'plugin-install', + 'themes', + 'theme-install', ] ); } @@ -58,6 +58,10 @@ public function data_hooks() { * @param string $hook The current screen's hook. */ public function test_should_not_enqueue_style_on_adjacent_screens( $hook ) { + if ( is_multisite() ) { + $hook .= '-network'; + } + $branding = new AspireUpdate\Branding(); $branding->admin_enqueue_scripts( $hook ); $this->assertFalse( wp_style_is( 'aspire_update_settings_css' ) ); @@ -71,9 +75,9 @@ public function test_should_not_enqueue_style_on_adjacent_screens( $hook ) { public function data_adjacent_screens() { return self::text_array_to_dataprovider( [ - 'index.php', - 'nav-menus.php', - 'plugin-editor.php', + 'dashboard', + 'nav-menus', + 'plugin-editor', ] ); } @@ -97,8 +101,9 @@ public function test_should_not_enqueue_style_when_ap_remove_ui_is_true() { // Prevent the notice from being displayed. define( 'AP_REMOVE_UI', true ); + $hook = is_multisite() ? 'plugins-network' : 'plugins'; $branding = new AspireUpdate\Branding(); - $branding->admin_enqueue_scripts( 'plugins.php' ); + $branding->admin_enqueue_scripts( $hook ); $this->assertFalse( wp_style_is( 'aspire_update_settings_css' ) ); } } diff --git a/tests/Branding/Branding_ConstructTest.php b/tests/Branding/Branding_ConstructTest.php index 1c00791..bb05fc1 100644 --- a/tests/Branding/Branding_ConstructTest.php +++ b/tests/Branding/Branding_ConstructTest.php @@ -12,9 +12,11 @@ */ class Branding_ConstructTest extends WP_UnitTestCase { /** - * Test that hooks are added when API rewriting is enabled. + * Test that hooks are added when API rewriting is enabled in single site. * - * @dataProvider data_hooks_and_methods + * @dataProvider data_single_site_hooks_and_methods + * + * @group ms-excluded * * @runInSeparateProcess * @preserveGlobalState disabled @@ -22,7 +24,7 @@ class Branding_ConstructTest extends WP_UnitTestCase { * @string $hook The hook's name. * @string $method The method to hook. */ - public function test_should_add_hooks( $hook, $method ) { + public function test_should_add_hooks_in_single_site( $hook, $method ) { define( 'AP_ENABLE', true ); $branding = new AspireUpdate\Branding(); @@ -30,9 +32,11 @@ public function test_should_add_hooks( $hook, $method ) { } /** - * Test that hooks are not added when API rewriting is disabled. + * Test that hooks are not added when API rewriting is disabled in single-site. + * + * @dataProvider data_single_site_hooks_and_methods * - * @dataProvider data_hooks_and_methods + * @group ms-excluded * * @runInSeparateProcess * @preserveGlobalState disabled @@ -40,7 +44,7 @@ public function test_should_add_hooks( $hook, $method ) { * @string $hook The hook's name. * @string $method The method to hook. */ - public function test_should_not_add_hooks( $hook, $method ) { + public function test_should_not_add_hooks_in_single_site( $hook, $method ) { define( 'AP_ENABLE', false ); $branding = new AspireUpdate\Branding(); @@ -52,7 +56,7 @@ public function test_should_not_add_hooks( $hook, $method ) { * * @return array[] */ - public function data_hooks_and_methods() { + public function data_single_site_hooks_and_methods() { return [ 'admin_notices -> output_admin_notice' => [ 'hook' => 'admin_notices', @@ -64,4 +68,62 @@ public function data_hooks_and_methods() { ], ]; } + + /** + * Test that hooks are added when API rewriting is enabled in multisite. + * + * @dataProvider data_multisite_hooks_and_methods + * + * @group ms-required + * + * @runInSeparateProcess + * @preserveGlobalState disabled + * + * @string $hook The hook's name. + * @string $method The method to hook. + */ + public function test_should_add_hooks_in_multisite( $hook, $method ) { + define( 'AP_ENABLE', true ); + + $branding = new AspireUpdate\Branding(); + $this->assertIsInt( has_action( $hook, [ $branding, $method ] ) ); + } + + /** + * Test that hooks are not added when API rewriting is disabled in multisite. + * + * @dataProvider data_multisite_hooks_and_methods + * + * @group ms-required + * + * @runInSeparateProcess + * @preserveGlobalState disabled + * + * @string $hook The hook's name. + * @string $method The method to hook. + */ + public function test_should_not_add_hooks_in_multisite( $hook, $method ) { + define( 'AP_ENABLE', false ); + + $branding = new AspireUpdate\Branding(); + $this->assertFalse( has_action( $hook, [ $branding, $method ] ) ); + } + + /** + * Data provider. + * + * @return array[] + */ + public function data_multisite_hooks_and_methods() { + return [ + 'network_admin_notices -> output_admin_notice' => [ + 'hook' => 'network_admin_notices', + 'method' => 'output_admin_notice', + ], + 'admin_enqueue_scripts -> admin_enqueue_scripts' => [ + 'hook' => 'admin_enqueue_scripts', + 'method' => 'admin_enqueue_scripts', + ], + ]; + } } diff --git a/tests/Branding/Branding_OutputAdminNoticeTest.php b/tests/Branding/Branding_OutputAdminNoticeTest.php index 745535a..7e5ffb9 100644 --- a/tests/Branding/Branding_OutputAdminNoticeTest.php +++ b/tests/Branding/Branding_OutputAdminNoticeTest.php @@ -20,12 +20,31 @@ class Branding_OutputAdminNoticeTest extends WP_UnitTestCase { * @param string $expected The expected substring to find. */ public function test_should_output_admin_notice( $hook, $expected ) { + if ( is_multisite() ) { + $hook .= '-network'; + } set_current_screen( $hook ); $branding = new AspireUpdate\Branding(); $this->assertStringContainsString( $expected, get_echo( [ $branding, 'output_admin_notice' ] ) ); } + /** + * Test that no admin notice is output on adjacent screens. + * + * @dataProvider data_screen_specific_messages + * + * @group ms-required + * + * @param string $hook The current screen's hook. + */ + public function test_should_not_output_notice_on_single_site_screens_in_multisite( $hook ) { + set_current_screen( $hook ); + + $branding = new AspireUpdate\Branding(); + $this->assertSame( '', get_echo( [ $branding, 'output_admin_notice' ] ) ); + } + /** * Data provider. * @@ -33,24 +52,24 @@ public function test_should_output_admin_notice( $hook, $expected ) { */ public function data_screen_specific_messages() { return [ - 'update-core.php' => [ - 'hook' => 'update-core.php', + 'update-core' => [ + 'hook' => 'update-core', 'expected' => 'WordPress, plugin, theme and translation updates', ], - 'plugins.php' => [ - 'hook' => 'plugins.php', + 'plugins' => [ + 'hook' => 'plugins', 'expected' => 'plugin updates', ], - 'plugin-install.php' => [ - 'hook' => 'plugin-install.php', + 'plugin-install' => [ + 'hook' => 'plugin-install', 'expected' => 'plugin updates', ], - 'themes.php' => [ - 'hook' => 'themes.php', + 'themes' => [ + 'hook' => 'themes', 'expected' => 'theme updates', ], - 'theme-install.php' => [ - 'hook' => 'theme-install.php', + 'theme-install' => [ + 'hook' => 'theme-install', 'expected' => 'theme updates', ], ]; @@ -64,6 +83,9 @@ public function data_screen_specific_messages() { * @param string $hook The current screen's hook. */ public function test_should_not_output_notice_on_adjacent_screens( $hook ) { + if ( is_multisite() ) { + $hook .= '-network'; + } set_current_screen( $hook ); $branding = new AspireUpdate\Branding(); @@ -78,9 +100,9 @@ public function test_should_not_output_notice_on_adjacent_screens( $hook ) { public function data_adjacent_screens() { return self::text_array_to_dataprovider( [ - 'index.php', - 'nav-menus.php', - 'plugin-editor.php', + 'dashboard', + 'nav-menus', + 'plugin-editor', ] ); } @@ -108,7 +130,7 @@ public function test_should_not_output_notice_when_there_is_no_screen() { */ public function test_should_not_output_notice_when_ap_remove_ui_is_true() { // Set to a screen that should display an admin notice. - set_current_screen( 'plugins.php' ); + set_current_screen( is_multisite() ? 'plugins-network' : 'plugins' ); // Prevent the notice from being displayed. define( 'AP_REMOVE_UI', true ); diff --git a/tests/multisite.xml b/tests/multisite.xml new file mode 100644 index 0000000..1536460 --- /dev/null +++ b/tests/multisite.xml @@ -0,0 +1,41 @@ + + + + + + + + + ./ + + + + + ms-excluded + + + + + ../includes + + + ../includes/autoload.php + + + + + + +