Skip to content

Cache the multiple admins results. #11730

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

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

Cache the multiple admins results. #11730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 8 additions & 9 deletions includes/Core/Authentication/Authentication.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ public function register() {
$this->verification_file()->register();
$this->verification_meta()->register();
$this->has_connected_admins->register();
$this->has_multiple_admins->register();
$this->owner_id->register();
$this->connected_proxy_url->register();
$this->disconnected_reason->register();
Expand Down Expand Up @@ -320,17 +321,14 @@ function () {
'googlesitekit_user_data',
function ( $user ) {
if ( $this->profile->has() ) {
$profile_data = $this->profile->get();
$user['user']['email'] = $profile_data['email'];
$user['user']['picture'] = $profile_data['photo'];
// Older versions of Site Kit (before 1.86.0) did not
// fetch the user's full name, so we need to check for
// that attribute before using it.
$profile_data = $this->profile->get();
$user['user']['email'] = $profile_data['email'];
$user['user']['picture'] = $profile_data['photo'];
$user['user']['full_name'] = isset( $profile_data['full_name'] ) ? $profile_data['full_name'] : null;
}

$user['connectURL'] = esc_url_raw( $this->get_connect_url() );
$user['hasMultipleAdmins'] = $this->has_multiple_admins->get();
$user['connectURL'] = esc_url_raw( $this->get_connect_url() );
// hasMultipleAdmins removed from here; exposed via core/site REST endpoint only.
$user['initialVersion'] = $this->initial_version->get();
$user['isUserInputCompleted'] = ! $this->user_input->are_settings_empty();
$user['verified'] = $this->verification->has();
Expand Down Expand Up @@ -735,7 +733,8 @@ public function do_refresh_user_token() {
* @since 1.32.0 Moved connect and disconnect actions to dedicated handlers.
*/
private function handle_oauth() {
if ( defined( 'WP_CLI' ) && WP_CLI ) {
// Use global WP_CLI constant without namespace issues.
if ( defined( 'WP_CLI' ) && constant( 'WP_CLI' ) ) {
return;
}

Expand Down
36 changes: 35 additions & 1 deletion includes/Core/Authentication/Has_Multiple_Admins.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,43 @@ public function get() {
$user_query = new WP_User_Query( $user_query_args );
$admins_count = $user_query->get_total();

$this->transients->get( self::OPTION, $admins_count, HOUR_IN_SECONDS );
// Cache the count for 1 week.
$this->transients->set( self::OPTION, $admins_count, WEEK_IN_SECONDS );
}

return $admins_count > 1;
}

/**
* Registers hooks to keep the cached value accurate.
*
* @since n.e.x.t
*/
public function register() {
// Invalidate when a user is registered (might be an admin) or deleted.
add_action( 'user_register', array( $this, 'invalidate' ) );
add_action( 'deleted_user', array( $this, 'invalidate' ) );

// Invalidate when a role changes to or from administrator.
add_action(
'set_user_role',
function ( $user_id, $role, $old_roles ) {
if ( 'administrator' === strtolower( $role ) || in_array( 'administrator', array_map( 'strtolower', (array) $old_roles ), true ) ) {
$this->invalidate();
}
},
10,
3
);
}

/**
* Deletes the cached admins count.
*
* @since n.e.x.t
* @return void
*/
public function invalidate() {
$this->transients->delete( self::OPTION );
}
}
2 changes: 2 additions & 0 deletions includes/Core/Authentication/REST_Authentication_Controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ private function get_rest_routes() {
'ownerID' => $this->authentication->get_owner_id_instance()->get(),
);

// hasMultipleAdmins is intentionally exposed only here (core/site) per acceptance criteria.

return new WP_REST_Response( $data );
},
'permission_callback' => $can_setup,
Expand Down
2 changes: 0 additions & 2 deletions tests/phpunit/integration/Core/Authentication/AuthenticationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ public function test_register__googlesitekit_user_data() {
'initialVersion',
'isUserInputCompleted',
'verified',
'hasMultipleAdmins',
),
array_keys( $user_data ),
'User data should contain all required authentication keys.'
Expand All @@ -123,7 +122,6 @@ public function test_register__googlesitekit_user_data() {
'isUserInputCompleted',
'verified',
'user',
'hasMultipleAdmins',
),
array_keys( $user_data ),
'User data should contain all required authentication keys including user profile data.'
Expand Down