Skip to content

Commit 5607d91

Browse files
committed
Refine naming and wording in the existing-user link flow.
Rename `Profile_Authenticator` to `Existing_User_Authenticator`, and rename the `profile-connect` CSS class to `existing-user-connect-button` so the names match the flow. Keep the `CONNECT` prefix on the nonce constant. Rename `render_profile_admin_notices` to `render_profile_admin_errors` since the method only renders errors. Switch the profile-page tag output to use `Web_Tag::register()` instead of a separate `render_tag()` method, so the tag follows the same rendering pattern as the other flows. Add a test that confirms the existing-user flow is picked when WooCommerce is also loaded. Drop jargon from docblocks, inline comments, and assertion messages. Replace "short-circuit", "anti-vacuous", "hooked", "forces", "wired up", "piggyback", and "win" with plain English. Match project conventions by removing typed properties and void return types that weren't used anywhere else in `includes/`. Refs #9994.
1 parent a374357 commit 5607d91

8 files changed

Lines changed: 172 additions & 155 deletions

File tree

includes/Modules/Sign_In_With_Google.php

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@
3939
use Google\Site_Kit\Modules\Sign_In_With_Google\Authenticator;
4040
use Google\Site_Kit\Modules\Sign_In_With_Google\Authenticator_Interface;
4141
use Google\Site_Kit\Modules\Sign_In_With_Google\Existing_Client_ID;
42+
use Google\Site_Kit\Modules\Sign_In_With_Google\Existing_User_Authenticator;
4243
use Google\Site_Kit\Modules\Sign_In_With_Google\Hashed_User_ID;
43-
use Google\Site_Kit\Modules\Sign_In_With_Google\Profile_Authenticator;
4444
use Google\Site_Kit\Modules\Sign_In_With_Google\Profile_Reader;
4545
use Google\Site_Kit\Modules\Sign_In_With_Google\Settings;
4646
use Google\Site_Kit\Modules\Sign_In_With_Google\Sign_In_With_Google_Block;
@@ -191,8 +191,9 @@ function () {
191191
// becomes a real button on the user's own, unlinked profile.
192192
add_action( 'admin_footer', $this->get_method_proxy( 'maybe_render_profile_signinwithgoogle' ) );
193193

194-
// Surface the already-connected error from the profile-page connect flow.
195-
add_action( 'admin_notices', $this->get_method_proxy( 'render_profile_admin_notices' ) );
194+
// Show an error when a user tries to link a Google account that's
195+
// already linked to a different WordPress user.
196+
add_action( 'admin_notices', $this->get_method_proxy( 'render_profile_admin_errors' ) );
196197

197198
// Output the Sign in with Google <div> in the WooCommerce login form.
198199
add_action( 'woocommerce_login_form_start', $this->get_method_proxy( 'render_signinwithgoogle_woocommerce' ) );
@@ -262,8 +263,8 @@ private function resolve_authenticator_class( $integration ): string {
262263
return WooCommerce_Authenticator::class;
263264
}
264265

265-
if ( 'connect_existing_profile' === $integration && is_user_logged_in() ) {
266-
return Profile_Authenticator::class;
266+
if ( 'existing_user' === $integration && is_user_logged_in() ) {
267+
return Existing_User_Authenticator::class;
267268
}
268269

269270
return Authenticator::class;
@@ -773,8 +774,11 @@ public function register_tag() {
773774
/**
774775
* Builds a `Web_Tag` instance and runs the shared registration guards.
775776
*
776-
* Returns `null` when the tag is blocked or its guard refuses registration,
777-
* so callers can short-circuit before configuring setters and rendering.
777+
* Returns `null` when the tag can't be rendered. A tag is blocked when
778+
* AMP is active or another integration turns tag output off for the
779+
* current request. The guard also rejects the tag when the module's
780+
* settings are incomplete. Callers can return early before setting any
781+
* values on the tag and rendering it.
778782
*
779783
* @since n.e.x.t
780784
*
@@ -801,7 +805,7 @@ private function build_registrable_tag(): ?Web_Tag {
801805
* Outputs the Sign in with Google init script that turns the placeholder
802806
* `<div>` from `render_sign_in_with_google_profile()` into a real button.
803807
*
804-
* Hooks `admin_footer` so `did_action( 'show_user_profile' )` reliably
808+
* Runs on `admin_footer` so `did_action( 'show_user_profile' )` reliably
805809
* reports whether the section actually rendered.
806810
*
807811
* @since n.e.x.t
@@ -829,20 +833,17 @@ private function maybe_render_profile_signinwithgoogle() {
829833
}
830834

831835
$tag->set_settings( $this->get_settings()->get() );
832-
$tag->set_is_connect_existing_profile( true );
833-
834-
// Already inside `admin_footer`, so render inline instead of queuing
835-
// another callback.
836-
$tag->render_tag();
836+
$tag->set_is_existing_user_flow( true );
837+
$tag->register();
837838
}
838839

839840
/**
840-
* Renders the profile-page admin notice when the connect flow surfaces
841-
* an error (e.g. the Google account is already linked to another user).
841+
* Shows an error on the profile page when linking the Google account
842+
* fails (e.g. it is already linked to another user).
842843
*
843844
* @since n.e.x.t
844845
*/
845-
private function render_profile_admin_notices() {
846+
private function render_profile_admin_errors() {
846847
$current_screen = Current_Screen::get();
847848

848849
if (
@@ -852,8 +853,8 @@ private function render_profile_admin_notices() {
852853
return;
853854
}
854855

855-
$error_code = $this->context->input()->filter( INPUT_GET, Profile_Authenticator::ERROR_QUERY_ARG );
856-
if ( Profile_Authenticator::ERROR_ACCOUNT_ALREADY_CONNECTED !== $error_code ) {
856+
$error_code = $this->context->input()->filter( INPUT_GET, Existing_User_Authenticator::ERROR_QUERY_ARG );
857+
if ( Existing_User_Authenticator::ERROR_ACCOUNT_ALREADY_CONNECTED !== $error_code ) {
857858
return;
858859
}
859860

@@ -980,7 +981,7 @@ public function handle_disconnect_user() {
980981
*
981982
* @since 1.141.0
982983
* @since n.e.x.t Renamed from `render_disconnect_profile` and added the
983-
* the ability to connect an existing profile.
984+
* ability to connect an existing profile.
984985
*
985986
* @param WP_User $user WordPress user object.
986987
*/
@@ -1030,16 +1031,16 @@ private function render_sign_in_with_google_profile( WP_User $user ) {
10301031
</p>
10311032
<?php
10321033
/**
1033-
* Renders the Sign in with Google button for the profile-connect flow.
1034+
* Renders the Sign in with Google button for the existing-user link flow.
10341035
*
10351036
* @since n.e.x.t
10361037
*
1037-
* @param array $args Button attributes. Sets `class` for the profile-connect placement.
1038+
* @param array $args Button attributes. Sets `class` for the existing-user link placement.
10381039
*/
10391040
do_action(
10401041
'googlesitekit_render_sign_in_with_google_button',
10411042
array(
1042-
'class' => 'googlesitekit-sign-in-with-google__profile-connect-button',
1043+
'class' => 'googlesitekit-sign-in-with-google__existing-user-connect-button',
10431044
)
10441045
);
10451046
?>

includes/Modules/Sign_In_With_Google/Authenticator.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,11 @@ class Authenticator implements Authenticator_Interface {
4444
const CREATED_BY_META_KEY = 'googlesitekitpersistent_created_by';
4545

4646
/**
47-
* Nonce action used by the profile-page connect flow.
47+
* Nonce action used by the existing-user link flow.
4848
*
4949
* @since n.e.x.t
5050
*/
51-
const CONNECT_EXISTING_PROFILE_NONCE_ACTION = 'googlesitekit_connect_existing_profile';
51+
const CONNECT_EXISTING_USER_NONCE_ACTION = 'googlesitekit_connect_existing_user';
5252

5353
/**
5454
* User options instance.

includes/Modules/Sign_In_With_Google/Profile_Authenticator.php renamed to includes/Modules/Sign_In_With_Google/Existing_User_Authenticator.php

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22
/**
3-
* Class Google\Site_Kit\Modules\Sign_In_With_Google\Profile_Authenticator
3+
* Class Google\Site_Kit\Modules\Sign_In_With_Google\Existing_User_Authenticator
44
*
55
* @package Google\Site_Kit\Modules\Sign_In_With_Google
66
* @copyright 2026 Google LLC
@@ -22,7 +22,7 @@
2222
* @access private
2323
* @ignore
2424
*/
25-
class Profile_Authenticator extends Authenticator {
25+
class Existing_User_Authenticator extends Authenticator {
2626

2727
/**
2828
* Error code surfaced when the Google account is already linked to another user.
@@ -32,7 +32,7 @@ class Profile_Authenticator extends Authenticator {
3232
const ERROR_ACCOUNT_ALREADY_CONNECTED = 'googlesitekit_auth_account_already_connected';
3333

3434
/**
35-
* Query arg for the profile connect-flow error. Namespaced to avoid
35+
* Query arg for the existing-user link flow error. Namespaced to avoid
3636
* WordPress core's empty `?error=` notice on `wp-admin/profile.php` and `wp-admin/user-edit.php`.
3737
*
3838
* @since n.e.x.t
@@ -62,10 +62,10 @@ public function authenticate_user( Input $input ): string {
6262
return $this->get_error_redirect_url( self::ERROR_INVALID_REQUEST );
6363
}
6464

65-
// Tie the link request to the current WordPress session so a
66-
// cross-site request can't piggyback on the user's auth cookie.
65+
// Tie the link request to the current WordPress session so the
66+
// user's auth cookie alone can't validate a cross-site request.
6767
$nonce = $input->filter( INPUT_POST, 'connect_nonce' );
68-
if ( ! wp_verify_nonce( $nonce, self::CONNECT_EXISTING_PROFILE_NONCE_ACTION ) ) {
68+
if ( ! wp_verify_nonce( $nonce, self::CONNECT_EXISTING_USER_NONCE_ACTION ) ) {
6969
return $this->get_error_redirect_url( self::ERROR_INVALID_REQUEST );
7070
}
7171

@@ -81,7 +81,7 @@ public function authenticate_user( Input $input ): string {
8181
// Check to see if the Google user ID for the Google Account we signed in
8282
// with is already in use (eg. registered to another user on the site).
8383
//
84-
// If this is the case, we'll trigger an error instance of associating
84+
// If this is the case, we'll trigger an error instead of associating
8585
// this Google Account with the current user.
8686
$existing_user = get_users(
8787
array(
@@ -100,9 +100,7 @@ public function authenticate_user( Input $input ): string {
100100
}
101101

102102
// Link the Google account by writing the hashed user ID to the current user.
103-
$user_options = clone $this->user_options;
104-
$user_options->switch_user( $current_user->ID );
105-
$user_options->set( Hashed_User_ID::OPTION, $g_user_hid );
103+
$this->user_options->set( Hashed_User_ID::OPTION, $g_user_hid );
106104

107105
return get_edit_user_link( $current_user->ID );
108106
}
@@ -111,9 +109,9 @@ public function authenticate_user( Input $input ): string {
111109
* Builds the redirect URL used when an error needs to be surfaced.
112110
*
113111
* The base `Authenticator` redirects to `wp-login.php` for error display,
114-
* but the profile-page connect flow lives in the WordPress admin, so
115-
* we redirect back to the user's edit profile page with the error
116-
* appended as a query argument.
112+
* but the existing-user link flow lives in the WordPress admin, so we
113+
* redirect back to the user's edit profile page with the error appended
114+
* as a query argument.
117115
*
118116
* @since n.e.x.t
119117
*

includes/Modules/Sign_In_With_Google/Web_Tag.php

Lines changed: 21 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,12 @@ class Web_Tag extends Module_Web_Tag {
5353
private $redirect_to;
5454

5555
/**
56-
* Whether the tag renders for the existing-profile connect flow on `wp-admin/profile.php`.
56+
* Whether the tag renders for the existing-user link flow on `wp-admin/profile.php`.
5757
*
5858
* @since n.e.x.t
5959
* @var bool
6060
*/
61-
private bool $is_connect_existing_profile = false;
61+
private bool $is_existing_user_flow = false;
6262

6363
/**
6464
* Sets the module settings.
@@ -96,25 +96,25 @@ public function set_redirect_to( $redirect_to ) {
9696
}
9797

9898
/**
99-
* Sets whether the tag renders for the existing-profile connect flow on `wp-admin/profile.php`.
99+
* Sets whether the tag renders for the existing-user link flow on `wp-admin/profile.php`.
100100
*
101101
* @since n.e.x.t
102102
*
103-
* @param bool $is_connect_existing_profile Connect-existing-profile flag.
103+
* @param bool $is_existing_user_flow Existing-user link flow flag.
104104
*/
105-
public function set_is_connect_existing_profile( bool $is_connect_existing_profile ): void {
106-
$this->is_connect_existing_profile = $is_connect_existing_profile;
105+
public function set_is_existing_user_flow( bool $is_existing_user_flow ): void {
106+
$this->is_existing_user_flow = $is_existing_user_flow;
107107
}
108108

109109
/**
110110
* Registers tag hooks.
111111
*
112112
* @since 1.159.0
113-
* @since n.e.x.t Hooks `admin_footer` for the existing-profile connect flow.
113+
* @since n.e.x.t Runs on `admin_footer` for the existing-user link flow.
114114
*/
115115
public function register() {
116-
if ( $this->is_connect_existing_profile ) {
117-
// Output the Sign in with Google JS on the profile-page connect flow.
116+
if ( $this->is_existing_user_flow ) {
117+
// Output the Sign in with Google JS on the existing-user link flow.
118118
add_action( 'admin_footer', $this->get_method_proxy( 'render' ) );
119119
} else {
120120
// Render the Sign in with Google script that converts placeholder
@@ -127,20 +127,6 @@ public function register() {
127127
$this->do_init_tag_action();
128128
}
129129

130-
/**
131-
* Renders the Sign in with Google tag output.
132-
*
133-
* `self::render()` stays `protected` to match `Module_Tag::render()`.
134-
* Outside callers should use this public entry point instead.
135-
*
136-
* @since n.e.x.t
137-
*
138-
* @see self::render()
139-
*/
140-
public function render_tag() {
141-
$this->render();
142-
}
143-
144130
/**
145131
* Renders the Sign in with Google JS script tags, One Tap code, and
146132
* buttons.
@@ -149,7 +135,7 @@ public function render_tag() {
149135
* @since 1.144.0 Renamed to `render_signinwithgoogle` and conditionally
150136
* rendered the code to replace buttons.
151137
* @since 1.159.0 moved from main Sign_In_With_Google class to Web_Tag.
152-
* @since n.e.x.t Added support for the existing-profile connect flow.
138+
* @since n.e.x.t Added support for the existing-user link flow.
153139
*/
154140
protected function render() {
155141
$is_woocommerce = class_exists( 'WooCommerce' );
@@ -163,9 +149,12 @@ protected function render() {
163149
'shape' => $this->settings['shape'],
164150
);
165151

166-
// Treat the connect page like the WP and WooCommerce login pages,
167-
// so the callback follows the POST redirect instead of reloading.
168-
$is_login_page = $this->is_wp_login || $is_woocommerce_login || $this->is_connect_existing_profile;
152+
$is_login_page = $this->is_wp_login || $is_woocommerce_login;
153+
154+
// The existing-user link flow follows the POST redirect from the
155+
// auth callback just like the login pages do, so group it here
156+
// rather than on the login flag itself.
157+
$follows_post_redirect = $is_login_page || $this->is_existing_user_flow;
169158

170159
// Check to see if we should show the One Tap prompt on this page.
171160
//
@@ -189,9 +178,9 @@ protected function render() {
189178
( () => {
190179
async function handleCredentialResponse( response ) {
191180
<?php if ( ! is_preview() ) : // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>
192-
<?php if ( $this->is_connect_existing_profile ) : // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>
193-
response.integration = 'connect_existing_profile';
194-
response.connect_nonce = <?php echo wp_json_encode( wp_create_nonce( Authenticator::CONNECT_EXISTING_PROFILE_NONCE_ACTION ) ); ?>;
181+
<?php if ( $this->is_existing_user_flow ) : // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>
182+
response.integration = 'existing_user';
183+
response.connect_nonce = <?php echo wp_json_encode( wp_create_nonce( Authenticator::CONNECT_EXISTING_USER_NONCE_ACTION ) ); ?>;
195184
<?php elseif ( $is_woocommerce && ! $this->is_wp_login ) : // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>
196185
response.integration = 'woocommerce';
197186
<?php endif; // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>
@@ -213,7 +202,7 @@ protected function render() {
213202
sessionStorage.setItem( `siwg-comment-text-${postId}`, commentText );
214203
}
215204

216-
<?php if ( empty( $this->redirect_to ) && ! $is_login_page ) : // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>
205+
<?php if ( empty( $this->redirect_to ) && ! $follows_post_redirect ) : // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>
217206
location.reload();
218207
<?php else : // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>
219208
if ( res.ok && res.redirected ) {
@@ -241,7 +230,7 @@ protected function render() {
241230
document.getElementById( 'login' ).insertBefore( buttonDivToAddToLoginForm, document.getElementById( 'loginform' ) );
242231
<?php endif; // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>
243232

244-
<?php if ( ! is_user_logged_in() || $this->is_wp_login || is_preview() || $this->is_connect_existing_profile ) : // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>
233+
<?php if ( ! is_user_logged_in() || $this->is_wp_login || is_preview() || $this->is_existing_user_flow ) : // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect ?>
245234
<?php
246235
/**
247236
* Render SiwG buttons for all `<div>` elements with the "magic

tests/phpunit/integration/Core/Util/Current_ScreenTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public function test_returns_wp_screen_for_user_edit_screen() {
3434
$screen = Current_Screen::get();
3535

3636
$this->assertInstanceOf( WP_Screen::class, $screen, 'Should return a WP_Screen instance when the user-edit screen is active.' );
37-
$this->assertSame( 'user-edit', $screen->id, 'Returned screen ID should match the active user-edit screen.' );
37+
$this->assertSame( 'user-edit', $screen->id, 'Returned screen ID should match the user-edit screen when it is the current screen.' );
3838
}
3939

4040
public function test_returns_null_when_no_current_screen_is_set() {

0 commit comments

Comments
 (0)