Rely strictly on AdSense API to determine account status

[felixarntz]

Follow-up to #4758, #4759, #5106: The logic in the AdSense setup is now generally correct, however there is something we need to enhance: Currently, some of the states are determined based on the accountID and clientID settings, and there are a few situations where we don't update them based on the current API responses. This can lead to inconsistent behavior, especially when mocking different statuses with the tester plugin.

It can certainly have real-world implications though too: for example if you first start the AdSense setup with an account that has an AFC client set up but then delete it, when you go back to the AdSense setup, Site Kit still has that old clientID set, which is incorrect and leads to the wrong status being set and the wrong UI showing up.

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

• In the AdSense V2 SetupMain component:
  • If there are no accounts in the API response, but accountID is set to a non-empty value, the setting should be updated to be an empty string again (in the respective useEffect).
• In the AdSense V2 SetupAccount component:
  • If there is no AFC client in the API response, but clientID is set to a non-empty value, the setting should be updated to be an empty string again (in the respective useEffect).
  • Cleanup: The component should no longer use getClients which is unnecessary. That selector is internally used by getAFCClient, and its return value alone is sufficient for any of the conditions where the getClients result is currently used (e.g. loading state).

Implementation Brief

• In assets/js/modules/adsense/components/setup/v2/SetupMain.js:
  • Update the existing useEffect hook that sets the accountID setting to set an empty string for the accountID setting if accounts is an empty array and current value of the accountID setting is not empty.

    site-kit-wp/assets/js/modules/adsense/components/setup/v2/SetupMain.js

    Lines 138 to 148 in 00a2fdf

    	// Update current account ID setting on-the-fly.
    	useEffect( () => {
    	if (
    	accounts?.length === 1 &&
    	( ! accountID || accounts[ 0 ]._id !== accountID )
    	) {
    	setAccountID( accounts[ 0 ]._id );
    	// Set flag to await background submission.
    	setIsAwaitingBackgroundSubmit( true );
    	}
    	}, [ accounts, accountID, setAccountID ] );

• In assets/js/modules/adsense/components/setup/v2/SetupAccount.js:
  • Update the existing useEffect hook that sets the clientID setting to set an empty string for the clientID setting if afcClient is null and the current value of the clientID setting is not empty.

    site-kit-wp/assets/js/modules/adsense/components/setup/v2/SetupAccount.js

    Lines 77 to 81 in 00a2fdf

    	useEffect( () => {
    	if ( afcClient?._id && clientID !== afcClient._id ) {
    	setClientID( afcClient._id );
    	}
    	}, [ afcClient, clientID, setClientID ] );

  • Remove the getClients selector call and all checks of the clients variable.
• Update corresponding storybook stories if any is broken due to the aforementioned changes.

Test Coverage

• N/A

QA Brief

For all tests below: Set the tester plugin AdSense site status to "ready".

Ensure you have at least 1.8.2 of the Tester plugin installed.

Ensuring that account is reset

• Set the tester plugin AdSense account status to "needs-attention". Go to the AdSense setup (it should show the correct UI, but that's not the point here).
• Set the tester plugin AdSense account status to "none". Go to the AdSense setup (it should show the correct UI as well, still not the thing to mostly look for).
• Set the tester plugin AdSense account status to "multiple". Go to the AdSense setup. You should now see the dropdown to select an account. If you land in another situation (where your account seems as if it was already selected), it would mean the problem was not fixed as expected.

Ensuring the client is reset

• Set the tester plugin AdSense account status to "needs-attention". Go to the AdSense setup (it should show the correct UI, but that's not the point here).
• Set the tester plugin AdSense account status to "no-client". Go to the AdSense setup. You should now see the UI where it says that you need to upgrade your AdSense account to support "AdSense for Content". If you land in another situation (where your client seems as if it was set), it would mean the problem was not fixed as expected.

Changelog entry

• Ensure AdSense account ID and client ID are always set based on API response during setup.

[felixarntz]

Added a QA Brief for this already based on how I encountered this bug.

[eugene-manuilov]

• In the AdSense V2 SetupMain component:

  • If there are no accounts in the API response, but accountID is set to a non-empty value, the setting should be updated to be an empty string again (in the respective useEffect).

• In the AdSense V2 SetupAccount component:

  • If there is no AFC client in the API response, but clientID is set to a non-empty value, the setting should be updated to be an empty string again (in the respective useEffect).

@felixarntz I wonder whether we need to save settings after we reset either accountID or clientID. What do you think?

[felixarntz]

@eugene-manuilov

@felixarntz I wonder whether we need to save settings after we reset either accountID or clientID. What do you think?

Yes, we do. Would that not be already the case though based on the auto-submission logic from changed settings?

[eugene-manuilov]

@eugene-manuilov

@felixarntz I wonder whether we need to save settings after we reset either accountID or clientID. What do you think?

Yes, we do. Would that not be already the case though based on the auto-submission logic from changed settings?

I don't think so. All set{SettingSlug} actions just update settings in the store state and not submit changes. I have updated IB to explicitly say that we need save settings after reseting. Do you mind reviewing IB?

[felixarntz]

@eugene-manuilov There is this logic around background submit in AdSense setup, it has been there forever also in the old/current flow. Have you checked that code?

We should preferably not call saveSettings on every individual occurrence, because several of these changes can happen around the same time, and we shouldn't try to dispatch a bunch of individual POST requests at a similar time.

[eugene-manuilov]

Ah.. yes, you are right, @felixarntz. I forgot about that logic. Yes, it will automatically submit changes because the setting is changed.

[eugene-manuilov]

Removed unnecessary saveSettings calls from IB.

[felixarntz]

@eugene-manuilov Thanks, 2 more things about the IB:

• Instead of two new useEffect hooks, I suggest we put that extra code into the two existing useEffect hooks that call setAccountID / setClientID based on that API data.
• You say "reset" the settings, maybe it would be more clear to say "set it to an empty string"?

[eugene-manuilov]

@eugene-manuilov Thanks, 2 more things about the IB:

• Instead of two new useEffect hooks, I suggest we put that extra code into the two existing useEffect hooks that call setAccountID / setClientID based on that API data.
• You say "reset" the settings, maybe it would be more clear to say "set it to an empty string"?

Yes, sounds good to me. Updated.

[felixarntz]

IB ✅

[eugene-manuilov]

@felixarntz Matthew has a question regarding QA steps in this comment. Could you please take a look at it and help him since you wrote the original QAB?

[felixarntz]

@eugene-manuilov Replied on the PR.

[wpdarren]

QA Update: ⚠️

@felixarntz @eugene-manuilov I have an observation:

For both of the 'Needs Attention' tests in the QAB:

Set the tester plugin AdSense account status to "needs-attention". Go to the AdSense setup (it should show the correct UI, but that's not the point here).

I was expecting to see the Your account isn’t ready to show ads yet. You need to fix some things before we can connect Site Kit to your AdSense account. UI for Needs Attention. What I see is Add this site to your AdSense account We’ve detected that you haven’t added this site to your AdSense account yet. Is this because I have selected 'None' as the site status? Wasn't 100% sure, so thought I'd check.

[eugene-manuilov]

@wpdarren looks like you have the same problem as Matthew had during code review (see here: #5195 (review)). He solved the problem by installing the latest version of the tester plugin. Could you please also confirm that you have the 1.8.2 tester plugin?

[wpdarren]

@eugene-manuilov yes, I am using the latest version 1.8.2.

Let me run through it again and I will let you know if this is still an issue for me.

[wpdarren]

QA Update: ❌

@eugene-manuilov I am still seeing the issue. I tested it on the previous test site and a new one.

It looks like Matthew's issue was to do with multiple but this is with needs attention account status.

• Set the tester plugin AdSense site status to none
• Set the tester plugin AdSense account status to needs attention
• Set up AdSense and what I see is Add this site to your AdSense account. We’ve detected that you haven’t added this site to your AdSense account yet.
• Using the latest version of the tester plugin 1.8.2
• On the develop branch with adsenseSetupV2 feature flag enabled.

Everything else looks fine.

[felixarntz]

@wpdarren Apologies, I think that small but crucial detail about the site status to set in the tester plugin was wrong the way I had originally specified it in the QA Brief. What you're seeing is in fact expected because not having a site takes precedence over some of the other relevant criteria. So I've now updated the QA Brief to have the site status always set to "ready", so that the site status cannot "conflict" with the account status you set here, which is what this issue is exclusively about.

[wpdarren]

QA Update: ✅

Verified:

Ensuring that account is reset

• When the tester plugin AdSense account status to needs-attention, the correct UI loads when setting up AdSense
• When the tester plugin AdSense account status is set to none, the correct UI loads when setting up AdSense.
• When the tester plugin AdSense account status is set to multiple. the dropdown to select an account appears when setting up AdSense.

Ensuring the client is reset

• When the tester plugin AdSense account status to needs-attention, the correct UI loads when setting up AdSense
• When the tester plugin AdSense account status to no-client, the correct UI appears where it says you need to upgrade your AdSense account to support "AdSense for Content", when setting up AdSense.

Screenshots