Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added support to multisite #690

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from
Open

Added support to multisite #690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5212be0
first attempt to multisite
davidperezgar Oct 2, 2024
ae9f280
fix link in network admin
davidperezgar Oct 2, 2024
e8c8f82
less strings
davidperezgar Oct 2, 2024
21a8df1
fixed tests
davidperezgar Oct 2, 2024
8fbf670
fix tests hooks
davidperezgar Oct 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions includes/Admin/Admin_Page.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,11 @@
* @since 1.0.0
*/
public function add_hooks() {
add_action( 'admin_menu', array( $this, 'add_and_initialize_page' ) );
add_filter( 'plugin_action_links', array( $this, 'filter_plugin_action_links' ), 10, 4 );
$admin_menu_hook = is_multisite() ? 'network_admin_menu' : 'admin_menu';
$plugin_action_link_hook = is_multisite() ? 'network_admin_plugin_action_links' : 'plugin_action_links';

add_action( $admin_menu_hook, array( $this, 'add_and_initialize_page' ) );
add_filter( $plugin_action_link_hook, array( $this, 'filter_plugin_action_links' ), 10, 4 );
Comment on lines +57 to +61
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we would want to only show the Plugin Check UI in the Network Admin just because it's a Multisite network. Maybe it should be in addition instead? I think that's worth discussing further before we make a decision and implement it.

add_action( 'admin_enqueue_scripts', array( $this, 'add_jump_to_line_code_editor' ) );

$this->admin_ajax->add_hooks();
Expand All @@ -64,13 +67,18 @@
/**
* Adds the admin page under the tools menu.
*
* @since n.e.x.t Added multisite support.
* @since 1.0.0
*/
public function add_page() {
$this->hook_suffix = add_management_page(
$admin_parent_page = is_multisite() ? 'settings.php' : 'tools.php';
$capabilities = is_multisite() ? 'manage_network_plugins' : 'activate_plugins';

Check warning on line 75 in includes/Admin/Admin_Page.php

View check run for this annotation

Codecov / codecov/patch

includes/Admin/Admin_Page.php#L74-L75 

Added lines #L74 - L75 were not covered by tests
Comment on lines +74 to +75
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above, maybe this should support both regular WP Admin and Network Admin?

Instead of checking is_multisite() (which forces it to only work for one of the two), you could check for is_network_admin() to make the decision.

Comment on lines +74 to +75
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable should probably be called $capability since it only stores one capability.


$this->hook_suffix = add_submenu_page(
$admin_parent_page,

Check warning on line 78 in includes/Admin/Admin_Page.php

View check run for this annotation

Codecov / codecov/patch

includes/Admin/Admin_Page.php#L77-L78 

Added lines #L77 - L78 were not covered by tests
__( 'Plugin Check', 'plugin-check' ),
__( 'Plugin Check', 'plugin-check' ),
'activate_plugins',
$capabilities,

Check warning on line 81 in includes/Admin/Admin_Page.php

View check run for this annotation

Codecov / codecov/patch

includes/Admin/Admin_Page.php#L81 

Added line #L81 was not covered by tests
'plugin-check',
array( $this, 'render_page' )
);
Expand Down Expand Up @@ -307,7 +315,13 @@
return $actions;
}

if ( current_user_can( 'activate_plugins' ) ) {
if ( is_multisite() && current_user_can( 'manage_network_plugins' ) ) {
$actions[] = sprintf(
'<a href="%1$s">%2$s</a>',
esc_url( network_admin_url( "settings.php?page=plugin-check&plugin={$plugin_file}" ) ),
esc_html__( 'Check this plugin', 'plugin-check' )

Check warning on line 322 in includes/Admin/Admin_Page.php

View check run for this annotation

Codecov / codecov/patch

includes/Admin/Admin_Page.php#L318-L322 

Added lines #L318 - L322 were not covered by tests
);
} elseif ( current_user_can( 'activate_plugins' ) ) {

Check warning on line 324 in includes/Admin/Admin_Page.php

View check run for this annotation

Codecov / codecov/patch

includes/Admin/Admin_Page.php#L324 

Added line #L324 was not covered by tests
Comment on lines +318 to +324
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above, this could also support both WP Admin and Network Admin UIs.

Suggested change
if ( is_multisite() && current_user_can( 'manage_network_plugins' ) ) {
$actions[] = sprintf(
'<a href="%1$s">%2$s</a>',
esc_url( network_admin_url( "settings.php?page=plugin-check&plugin={$plugin_file}" ) ),
esc_html__( 'Check this plugin', 'plugin-check' )
);
} elseif ( current_user_can( 'activate_plugins' ) ) {
if ( is_network_admin() && current_user_can( 'manage_network_plugins' ) ) {
$actions[] = sprintf(
'<a href="%1$s">%2$s</a>',
esc_url( network_admin_url( "settings.php?page=plugin-check&plugin={$plugin_file}" ) ),
esc_html__( 'Check this plugin', 'plugin-check' )
);
} elseif ( ! is_network_admin() && current_user_can( 'activate_plugins' ) ) {

$actions[] = sprintf(
'<a href="%1$s">%2$s</a>',
esc_url( admin_url( "tools.php?page=plugin-check&plugin={$plugin_file}" ) ),
Expand Down
11 changes: 8 additions & 3 deletions tests/phpunit/tests/Admin/Admin_Page_Tests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ public function set_up() {

public function test_add_hooks() {
$this->admin_page->add_hooks();
$this->assertEquals( 10, has_action( 'admin_menu', array( $this->admin_page, 'add_and_initialize_page' ) ) );
$this->assertEquals( 10, has_filter( 'plugin_action_links', array( $this->admin_page, 'filter_plugin_action_links' ) ) );
$admin_menu_hook = is_multisite() ? 'network_admin_menu' : 'admin_menu';
$plugin_action_link_hook = is_multisite() ? 'network_admin_plugin_action_links' : 'plugin_action_links';

$this->assertEquals( 10, has_action( $admin_menu_hook, array( $this->admin_page, 'add_and_initialize_page' ) ) );
$this->assertEquals( 10, has_filter( $plugin_action_link_hook, array( $this->admin_page, 'filter_plugin_action_links' ) ) );
}

public function test_add_and_initialize_page() {
Expand All @@ -35,8 +38,10 @@ public function test_add_and_initialize_page() {

$admin_user = self::factory()->user->create( array( 'role' => 'administrator' ) );

$expected_parent_page = 'tools.php';
if ( is_multisite() ) {
grant_super_admin( $admin_user );
$expected_parent_page = 'settings.php';
}

wp_set_current_user( $admin_user );
Expand All @@ -50,7 +55,7 @@ public function test_add_and_initialize_page() {
set_current_screen( $current_screen );

$this->assertArrayHasKey( 'plugin-check', $parent_pages );
$this->assertEquals( 'tools.php', $parent_pages['plugin-check'] );
$this->assertEquals( $expected_parent_page, $parent_pages['plugin-check'] );
$this->assertNotFalse( has_action( "load-{$page_hook}", array( $this->admin_page, 'initialize_page' ) ) );
}

Expand Down
Loading