Upgrade legacy AdSense account and site statuses

[aaemnnosttv]

Feature Description

With the new and improved AdSense v2 setup flow, we're able to display more accurate information on a user's AdSense account and site statuses. Since #5503 this has been surfaced in the settings as well.

However, users who set up AdSense before the v2 setup flow was released will still have the older statuses in the module settings which prevent us from showing more accurate information about the current state.

---

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

Acceptance criteria

• The AdSense overview widget in the Site Kit dashboard should be adjusted to include a migration for old AdSense account and site statuses, as follows:
  • If the adsenseSetupV2 flag is enabled but the accountStatus or siteStatus settings use one of the legacy (V1) statuses, a new component should be included on top of the widget body (i.e. within the Widget component, but before the Overview component).
  • This component should run the API requests to determine the actual account and site status, similar to the setup. Roughly:
    • Look up the account object based on the current accountID.
    • Look up the AFC client for that account and check the account's state value, and based on both of these determine whether the accountStatus is ready or something else, basically a subset of how the V2 SetupAccount component does it.
    • Look up the current site within that account and check that site's state and autoAdsEnabled values, and based on both of these determine whether the siteStatus is ready or something else, basically a subset of how the V2 SetupAccountSite component does.
  • If based on those API requests both the accountStatus and siteStatus result in ready, the two module settings should be updated on the fly, and the component should simply return null (render nothing). Otherwise, it should not update the module settings, but render the following:
    • A small inline warning notice (similar to e.g. https://google.github.io/site-kit-wp/storybook/main/?path=/story/global-notices--settings-warning-notice) to inform that AdSense appears to not be fully connected, with message: You need to redo setup to complete AdSense configuration
    • A primary CTA button with message Redo setup which does the following on click:
      • Set the accountSetupComplete and siteSetupComplete flags to false.
      • Link to the AdSense setup flow. (From there the necessary logic will run to update the account/site status as needed and guide the user through the missing steps, as usual.)

Implementation Brief

• Create StatusMigration function component.
  • Get accountID, afcClient, site and adminReauthURL using getAccountID, getAFCClient, getCurrentSite and getAdminReauthURL selectors of MODULES_ADSENSE store.
  • Determine whether both account and site is in READY state using the following.
    • If either afcClient or site is undefined, set isReady as undefined.
    • if either afcClient.state !== API_STATE_READY, site.state !== API_STATE_READY or, ! site.autoAdsEnabled, set isReady as false.
    • Otherwise isReady is true.
  • Have an effect that runs when value of isReady changes:
    • If isReady is false-y, do nothing and return early.
    • Otherwise, set the accountStatus and siteStatus to ACCOUNT_STATUS_READY and SITE_STATUS_READY using setAccountStatus and setSiteStatus actions of MODULES_ADSENSE store.
  • Return null from the component if isReady is undefined or true.
  • Otherwise, render the UI with copies described in the AC using SettingsNotice and Button.
    • Clicking on the button should do the following:
    • Set accountSetupComplete and siteSetupComplete flags to false using setAccountSetupComplete and setSiteSetupComplete actions.
    • Navigate to Adsense Setup flow using navigateTo action of the CORE_LOCATION store with adminReauthURL url.
• In assets/js/modules/adsense/util/status.js:
  • Define two functions isLegacyAccountStatus and isLegacySiteStatus that returns true whenever passed {account|site}Status argument is one of the v1 {account|site} status. See here and check for the v1-only status.
• In assets/js/modules/adsense/components/module/ModuleOverviewWidget/index.js:
  • Get the accountStatus and siteStatus using getAccountStatus and getSiteStatus selectors of MODULES_ADSENSE store.
  • Using the previously created utility functions, determine whether accountStatus and siteStatus are legacy.
  • If adsenseSetupV2Enabled is true and either status is legacy, render the StatusMigration component above the Overview component.

Test Coverage

• Add tests for the newly added util functions and component.

QA Brief

• Complete AdSense Setup without enabling adsenseSetupV2 feature flag.
• Ensure both site and account has legacy account setup defined here using the following JS snippets.

googlesitekit.data.select('modules/adsense').getAccountStatus();; // approved

googlesitekit.data.select('modules/adsense').getSiteStatus(); // added

• Enable adsenseSetupV2 feature flag and go to monetization of the dashboard.
• If the site is READY, there should be no additional UI and the status will get migrated to ready for both on the fly (wait a few sec before running the snippets above to check.)
• If the site is not ready, there will be a new UI on top of the Monetization section asking user to redo setup as described in the AC, clicking the CTA should take the user to AdSense setup.

Changelog entry

• Add the migration notice to the AdSense overview widget for legacy accounts.

[felixarntz]

@aaemnnosttv Is this about sites which already have AdSense connected? Since for sites that still have to go through finishing the setup, I think the statuses should update to the new ones by itself.

Regarding sites with AdSense already connected, this is a tricky one. Of course, one thing we could do is just display the corresponding new "approved/ready/connected" account status and site status for those sites and hope that everything is fine. Or, another approach that would take more effort could be something like this:

• If a site has one of the old "approved/ready/connected" statuses and the AdSense report request returns no data, we could re-introduce something like that old AdSense special "zero data state" box, however with different content.
• In that component, we could then run checks for the AdSense account and site, and if it turns out they are not set up, we could show a notice that "You still need to do something to fully configure AdSense".
• Then there could be a button, which on click deletes the two account status and site status module options (effectively bringing the module back to "not connected" state) and then redirects the user to the AdSense setup flow where they will land in the right place.

cc @marrrmarrr

[aaemnnosttv]

@felixarntz, yes this is about updating saved v1 statuses in the AdSense settings. Not sure if sites that aren't connected yet would benefit from this or not but connected sites would primarily benefit since these would have been set up (potentially years ago) and then not touched.

I wasn't thinking we'd do anything in response to report data. Instead, I think we could do the migration on-the-fly, perhaps in the settings-view and settings-edit components. If a v1-specific status is detected, we'd run the necessary queries to "upgrade" the status to determine the current v2 status that we're already doing in module setup. It could probably be a bit simpler as the accountID would already be known.

As an aside, it might be worth thinking about explicitly versioning the schema in the settings in the future.

[felixarntz]

@aaemnnosttv For not connected sites that still have to go through module setup, this is no problem anyway as the new status will be detected as soon as they open the module setup again. So migration is only relevant for sites where AdSense is already connected.

However, the problem is that the v1 "connected" statuses for the account and site are too generic and inaccurate to map 1:1 to a v2 status, since in v1 we didn't know a whole lot of things which in v2 are now setup-blocking. We could run the queries like you're saying, but that could lead to the module being not connected anymore when it was connected before. That may be fine, but I think it would be an unexpected user experience to suddenly have the module no longer connected without having done anything.

That's why in my previous comment I suggested to do it only on demand if the report is empty, but most importantly invoked by the user explicitly with a button like I mentioned there.

[aaemnnosttv]

@felixarntz thinking about this a bit more, the connected state is based on the (account|site)SetupComplete options, which are already migrated from the v1 setupComplete option. So as long as the site is already connected, why not migrate the approved v1 account and added site statuses to v2 ready? Then I think we'd only need to run the upgrade for accounts with a disapproved|graylisted|pending status, although I would think these wouldn't reach setupComplete and thus not connected anyways, correct?

Alternatively, we should be able to safely migrate all the v1-only statuses without affecting the connected state of the module as that is determined by settings which are independent from those statuses. Then, if we later detected a mismatch (i.e. non-ready account or site which is marked setup completed), we could flag this to the user and prompt them to restart the setup for the module. Note, there is already some "legacy account status" migration in place which splits a single value into v1 account and site statuses which should also be accounted or here.

What do you think?

[felixarntz]

@marrrmarrr I discussed the approach here earlier today with @aaemnnosttv, and we aligned on what would be the best way to migrate users that are still on V1 statuses. I've captured the plan in the ACs, pending the exact wording for the notice we're envisioning. Please have a look, though it may be helpful to talk through this in person for more context.

[marrrmarrr]

@felixarntz I added suggested wording, PTAL and if it LGTY, let's go with this.

[FlicHollis]

Hi @felixarntz and @aaemnnosttv following Mariya's update are we good to get this moved to IB? Thanks!

[techanvil]

I'm still not quite so familiar with the AdSense code yet, so I had to dig into this one. Looks good to me, nice work @kuasha420!

There's just one point that I think could do with a little clarification:

• Define two functions isLegacyAccountStatus and isLegacySiteStatus that returns true whenever passed {account|site}Status argument is one of the v1 {account|site} status.

Looking at the following, I'm guessing this means that if the status is in one of the sets of constants which is marked as // V1 setup flow. (and not in, say, // V1 and V2 setup flow.), but it would be good to clarify this in the IB to save any potential confusion.

site-kit-wp/assets/js/modules/adsense/util/status.js

Lines 19 to 47 in 0eb9173

	// V1 setup flow.
	export const ACCOUNT_STATUS_DISAPPROVED = 'disapproved';
	export const ACCOUNT_STATUS_GRAYLISTED = 'graylisted';
	export const ACCOUNT_STATUS_PENDING = 'pending';
	export const ACCOUNT_STATUS_APPROVED = 'approved';
	// V2 setup flow.
	export const ACCOUNT_STATUS_NEEDS_ATTENTION = 'needs-attention';
	export const ACCOUNT_STATUS_READY = 'ready';
	export const ACCOUNT_STATUS_CLIENT_REQUIRES_REVIEW = 'client-requires-review';
	export const ACCOUNT_STATUS_CLIENT_GETTING_READY = 'client-getting-ready';
	// V1 and V2 setup flow.
	export const ACCOUNT_STATUS_NONE = 'none';
	export const ACCOUNT_STATUS_MULTIPLE = 'multiple';
	export const ACCOUNT_STATUS_NO_CLIENT = 'no-client';
	// V1 setup flow.
	export const SITE_STATUS_ADDED = 'added';
	// V2 setup flow.
	export const SITE_STATUS_NEEDS_ATTENTION = 'needs-attention';
	export const SITE_STATUS_REQUIRES_REVIEW = 'requires-review';
	export const SITE_STATUS_GETTING_READY = 'getting-ready';
	export const SITE_STATUS_READY = 'ready';
	export const SITE_STATUS_READY_NO_AUTO_ADS = 'ready-no-auto-ads';
	// V1 and V2 setup flow.
	export const SITE_STATUS_NONE = 'none';

[kuasha420]

@techanvil solid suggestion, I've added this point to the IB. Cheers.

[techanvil]

Thanks @kuasha420!

IB ✅

[kuasha420]

@wpdarren Thank you for your detailed QA report here. The first issue is not due to the changes made here but how the feature flags interact with each other. You need to set "Always Override" in the Tester Plugin and then enable dashboardSharing and toggle adsenseSetupV2 as needed.

For the second issue, it's a mistake in the code and I'll create a follow up PR fixing this momentarily. Cheers!

[aaemnnosttv]

Back to you @wpdarren for another pass 👍

[kuasha420]

@wpdarren @aaemnnosttv There's another oversight in the newly created StatusMigration component that it doesn't save the changed settings.

As a result,

• The ready state doesn't get persisted and the migration runs every time.
• When the site is not ready, user will get navigated to the setup without {account|site}SetupComplete states being saved.

Silly mistake on my part as I wasn't fully familiar with the AdSense(V2) setup flow when I wrote the initial IB. I only caught this while working for an Ad Blocker Detection IB.

I'll pull this one back from QA and open another follow up PR to address this.

Cheers.

[wpdarren]

QA Update: ⚠️

@kuasha420 @techanvil I am not sure if I am being silly here 😄 but..

The last part of the QAB:

If the site is not ready, there will be a new UI on top of the Monetization section asking user to redo setup as described in the AC, clicking the CTA should take the user to AdSense setup.

I’m a bit confused on how to trigger this to test it.

I have tried using the tester plugin to set the status to anything but 'ready' to trigger the new UI.

[wpdarren]

QA Update: ❌

@kuasha420 thank you for your help testing this. I was able to trigger the new UI with another Google account.

I have a few observations.

1. When the Monetization widget first loads no CTA appears. It takes around 5-10 seconds for it to appear. You can see it in action in the screencast below.

adsense-1.mp4

2. The styling of the new UI is a little off. I feel we could do with some padding on the left-hand side.

3. The AC mentions the new UI should look like this Story and as you can see from the screenshot above, the icon and text doesn't seem to be a placed in the centre of the CTA. It appears to be displayed at the top. The CTA button is displayed as expected. This is only minor but thought I would flag if it was an easy fix.

[kuasha420]

@wpdarren Thanks for your diligence here. I've addressed all 3 of your concern in the follow up PR. Cheers.

[wpdarren]

QA Update: ✅

Verified:

• If the site is READY, there is no additional UI and the status gets migrated to ready. I ran the snippets in the QAB to make sure that the correct statuses appeared for account and site.
• If the site is not ready, there is a new UI on top of the Monetization section asking user to redo setup as described in the AC, clicking the CTA takes the user to AdSense setup.
  • A small inline warning notice appears to inform that AdSense appears to not be fully connected, with message: You need to redo setup to complete AdSense configuration
  • A primary CTA button with message Redo setup which does the following on click
  • Link to the AdSense setup flow.

Screenshots