Skip to content

Commit b6d95f2

Browse files
authored
SEO: Record a Tracks event when Google one-click verify saves the code (#53798)
* SEO: Record a Tracks event when Google one-click verify saves the code Google auto-verify saved the verification code through its own request, so it never fired jetpack_seo_setting_updated, and only mirrored the code into unsaved form state. The next verification save then re-sent the code and recorded it late. The settings form now saves the code for it (saveVerification), records it like a form save (only when the code changed, or the failure event if the save fails), and adopts it as saved in the form and the settings store. * SEO: Keep the auto-verified Google code when a form save finishes later saveValues adopted its whole start-of-save snapshot as the new baseline and store state. A Google code saveVerification adopted while that save was in flight was rolled back, so the code looked unsaved again and the next verification save recorded it a second time. Adopt only the fields the save sent. Also cover the hook's save-then-verify order and its stop on a failed save, and correct wording that claimed later saves don't re-send the code: the section save still sends the full map; it no longer counts the code as a change. * SEO: Send only changed verification codes and patch the latest store snapshot A verification-section save sent the whole code map and adopted it as saved, so finishing after a Google auto-verify could overwrite the new code on the server and roll it back in the form. The server merges submitted codes over the stored ones, so send and adopt only the codes that changed. Every write to the settings store now patches its latest snapshot instead of replacing it with the writing form's own baseline, so a form left on another tab mid-save can't roll back what a newer one saved. * SEO: Serialize Google auto-verify with form saves The server merges verification codes without a lock, so a form save overlapping saveVerification could drop the new code, and a form save's post-save re-read arriving after it could roll the code back in the form and store. saveVerification now waits for the save in flight, re-read included, and holds isSaving during its own request so the form's controls stay locked. A re-read also applies only the codes its save changed, which covers a form left on another tab, where the two don't share a queue. * SEO: Queue form saves behind Google auto-verify too The isSaving lock saveVerification takes disables a focused code field, and in Chrome that fires blur, which starts a section save while the Google save is still running. saveValues didn't wait for it, and either save's finally could clear isSaving while the other still ran. Form saves now queue behind the save in flight as well (their payload is still computed when called, and applied only to the codes they changed), and isSaving holds until the last queued save settles. * SEO: Lock the manual Google code field during auto-verify A code typed into the manual Google field while auto-verify was running got saved by the blur the form lock fires, queued right after the token, and replaced it, so Google could not find the token it was asked to verify. Disable that field while auto-verify runs; clicking Verify already blurs it, so a code typed before the click saves first. * SEO: Lock the manual Google field only once the popup returns Locking it for the whole auto-verify left it disabled until reload when the browser blocked the popup, which never returns, taking away the manual fallback. Lock it from the moment the popup returns a keyring instead: a code typed while the popup was open is then saved by the lock's blur before the token save is queued, so the token still wins. * SEO: Word the Google field lock comment without relying on blur Whether disabling a focused field fires blur differs by browser, so state only what the lock guarantees.
1 parent 6d11b2b commit b6d95f2

14 files changed

Lines changed: 644 additions & 72 deletions

‎projects/packages/seo/_inc/data/build-payload.ts‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,34 @@ export function buildCorePayload(
3939
if ( local.front_page_description !== baseline.front_page_description ) {
4040
payload.advanced_seo_front_page_description = local.front_page_description;
4141
}
42-
// The codes are sub-keys of one option, so the whole map goes when any of them
43-
// changes; the server merges it over the codes already stored.
44-
if (
45-
VERIFICATION_KEYS.some( key => local.verification[ key ] !== baseline.verification[ key ] )
46-
) {
47-
payload.verification_services_codes = local.verification;
42+
// The codes are sub-keys of one option, merged server-side over the stored ones, so
43+
// only changed codes go: a whole map would overwrite a code saved meanwhile.
44+
const codes = changedCodes( baseline, local );
45+
if ( Object.keys( codes ).length > 0 ) {
46+
payload.verification_services_codes = codes;
4847
}
4948

5049
return payload;
5150
}
5251

52+
/**
53+
* The verification codes that differ from the baseline.
54+
*
55+
* @param baseline - The last-saved server state.
56+
* @param local - The current form state.
57+
* @return The changed codes, keyed by service.
58+
*/
59+
export function changedCodes(
60+
baseline: SettingsResponse,
61+
local: SettingsResponse
62+
): Partial< SettingsResponse[ 'verification' ] > {
63+
return Object.fromEntries(
64+
VERIFICATION_KEYS.filter(
65+
key => local.verification[ key ] !== baseline.verification[ key ]
66+
).map( key => [ key, local.verification[ key ] ] )
67+
);
68+
}
69+
5370
/**
5471
* Read a `/wp/v2/settings` response back into form fields — the inverse of
5572
* `buildCorePayload()`, mapped the way the server's own settings read does.

‎projects/packages/seo/_inc/data/test/build-payload.test.ts‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -83,18 +83,12 @@ describe( 'buildCorePayload', () => {
8383
expect( buildCorePayload( baseline, local ) ).toEqual( {} );
8484
} );
8585

86-
it( 'sends the whole verification map when any code changed', () => {
87-
const baseline = makeSettings();
88-
const verification = {
89-
google: 'g-code',
90-
bing: '',
91-
pinterest: '',
92-
yandex: 'y-code',
93-
facebook: '',
94-
};
95-
const local = makeSettings( { verification } );
86+
it( 'sends only the verification codes that changed, cleared ones included', () => {
87+
const codes = { google: 'g-code', bing: 'b-code', pinterest: '', yandex: '', facebook: '' };
88+
const baseline = makeSettings( { verification: codes } );
89+
const local = makeSettings( { verification: { ...codes, bing: '', yandex: 'y-code' } } );
9690
expect( buildCorePayload( baseline, local ) ).toEqual( {
97-
verification_services_codes: verification,
91+
verification_services_codes: { bing: '', yandex: 'y-code' },
9892
} );
9993
} );
10094

@@ -107,7 +101,7 @@ describe( 'buildCorePayload', () => {
107101
} );
108102
expect( buildCorePayload( baseline, local ) ).toEqual( {
109103
advanced_seo_front_page_description: 'Desc',
110-
verification_services_codes: verification,
104+
verification_services_codes: { google: 'g' },
111105
} );
112106
} );
113107

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
import { jest } from '@jest/globals';
2+
import { act, renderHook, waitFor } from '@testing-library/react';
3+
4+
type Request = { path: string; method?: string };
5+
6+
const mockApiFetch = jest.fn< ( options: Request ) => Promise< unknown > >();
7+
const saveCode = jest.fn< ( code: string ) => Promise< void > >();
8+
9+
jest.unstable_mockModule( '@wordpress/api-fetch', () => ( { default: mockApiFetch } ) );
10+
jest.unstable_mockModule( '@automattic/request-external-access', () => ( {
11+
default: ( _url: string, callback: ( result: { keyring_id: number } ) => void ) =>
12+
callback( { keyring_id: 7 } ),
13+
} ) );
14+
jest.unstable_mockModule( '@automattic/jetpack-script-data', () => ( {
15+
getScriptData: () => ( {
16+
seo: { google_verify: { connect_url: 'https://example.com/connect', is_connected: true } },
17+
} ),
18+
} ) );
19+
jest.unstable_mockModule( '@wordpress/notices', () => ( { store: 'core/notices' } ) );
20+
const createErrorNotice = jest.fn();
21+
22+
jest.unstable_mockModule( '@wordpress/data', () => ( {
23+
useDispatch: () => ( {
24+
createInfoNotice: jest.fn(),
25+
createSuccessNotice: jest.fn(),
26+
createErrorNotice,
27+
} ),
28+
} ) );
29+
30+
const { useGoogleVerify } = await import( '../use-google-verify' );
31+
32+
const VERIFY_POST = expect.objectContaining( {
33+
path: '/jetpack/v4/verify-site/google',
34+
method: 'POST',
35+
} );
36+
37+
/**
38+
* Answer the verify-site status reads, and acknowledge every POST.
39+
*
40+
* @param alreadyVerified - Whether Google already considers the site verified.
41+
*/
42+
function respond( alreadyVerified = false ) {
43+
mockApiFetch.mockImplementation( ( { path, method } ) =>
44+
Promise.resolve(
45+
method === 'POST'
46+
? {}
47+
: {
48+
verified: alreadyVerified && path.endsWith( '/7' ),
49+
is_owner: true,
50+
google_search_console_url: '',
51+
token: 'tok',
52+
}
53+
)
54+
);
55+
}
56+
57+
/**
58+
* Mount the hook and wait for its status read.
59+
*
60+
* @return The rendered hook.
61+
*/
62+
async function mountHook() {
63+
const view = renderHook( () => useGoogleVerify( { saveCode } ) );
64+
await waitFor( () => expect( view.result.current.state ).not.toBe( 'loading' ) );
65+
return view;
66+
}
67+
68+
describe( 'useGoogleVerify', () => {
69+
beforeEach( () => {
70+
jest.clearAllMocks();
71+
saveCode.mockResolvedValue( undefined );
72+
} );
73+
74+
it.each( [
75+
[ 'saves the token through the settings form', false, [ [ 'tok' ] ] ],
76+
[ 'saves nothing when the site is already verified', true, [] ],
77+
] )( '%s', async ( _label, alreadyVerified, calls ) => {
78+
respond( alreadyVerified );
79+
const { result } = await mountHook();
80+
81+
act( () => result.current.autoVerify() );
82+
await waitFor( () => expect( result.current.isVerifying ).toBe( false ) );
83+
expect( saveCode.mock.calls ).toEqual( calls );
84+
expect( mockApiFetch ).not.toHaveBeenCalledWith(
85+
expect.objectContaining( { path: '/wp/v2/settings' } )
86+
);
87+
} );
88+
89+
it( 'asks Google to verify only once the code is saved', async () => {
90+
let finishSave: () => void = () => undefined;
91+
saveCode.mockImplementation(
92+
() =>
93+
new Promise< void >( resolve => {
94+
finishSave = resolve;
95+
} )
96+
);
97+
respond();
98+
const { result } = await mountHook();
99+
100+
act( () => result.current.autoVerify() );
101+
await waitFor( () => expect( saveCode ).toHaveBeenCalled() );
102+
expect( mockApiFetch ).not.toHaveBeenCalledWith( VERIFY_POST );
103+
expect( result.current.isConfirming ).toBe( true );
104+
105+
await act( async () => finishSave() );
106+
107+
await waitFor( () => expect( result.current.isVerifying ).toBe( false ) );
108+
expect( mockApiFetch ).toHaveBeenCalledWith( VERIFY_POST );
109+
expect( result.current.isConfirming ).toBe( false );
110+
} );
111+
112+
it( 'stops with an error when the code save fails', async () => {
113+
saveCode.mockRejectedValue( { message: 'Nope.' } );
114+
respond();
115+
const { result } = await mountHook();
116+
117+
act( () => result.current.autoVerify() );
118+
await waitFor( () => expect( result.current.isVerifying ).toBe( false ) );
119+
expect( mockApiFetch ).not.toHaveBeenCalledWith( VERIFY_POST );
120+
expect( createErrorNotice ).toHaveBeenCalledWith( 'Nope.', expect.anything() );
121+
} );
122+
} );

0 commit comments

Comments
 (0)