Handle changing the connected Analytics property

[techanvil]

Feature Description

Ensure the Audience Segmentation feature is effectively reset when changing the connected Analytics property.

Additionally:

• Ensure the selected audiences are cleared when disconnecting Analytics.
• Ensure the "temporarily hidden" state for any hidden audiences is cleared.

See Analytics properties and selected audiences in the design doc.

---

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

Acceptance criteria

• When the connected Analytics property is changed by selecting a new property via the Analytics settings:
  • The dismissal state for the following items should be cleared for all users:
    • The Setup CTA Banner.
    • The Audience Creation Notice in the Selection Panel.
    • The Setup Success Notice displayed when having set the feature up on the dashboard.
    • The Setup Success Settings Notice displayed when having set the feature up on the Settings screen.
  • The "temporarily hidden" state for any hidden audiences should be cleared.
  • The "temporarily hidden" state for the No Audiences Banner should be cleared.
  • The list of cached audiences should be cleared.
  • The audienceSegmentationSetupComplete flag should be cleared (see Implement the secondary user setup #8130).
  • The audience selection should be cleared for all users.
  • The didSetAudiences flag should be cleared for all users (see Implement the secondary user variants of the "no audiences" banner. #8577).

Implementation Brief

Create the Reset_Audiences class for clearing the data

• Create a new class, Modules/Analytics_4/Reset_Audiences, which follows the standard class pattern, passing/instantiating required dependencies in the constructor, and otherwise bootstrapping the instance in a register method.
• Add a method reset_audience_data:
  • Get IDs for all users who have dismissed prompts or items using the following SQL query, ensuring the table name and meta keys are correctly prefixed as seen in this example:

SELECT DISTINCT user_id
FROM wp_usermeta
WHERE meta_key IN ( 'wp_googlesitekitpersistent_dismissed_prompts', 'wp_googlesitekitpersistent_dismissed_items' )
LIMIT 100 -- Arbitrary limit to avoid unbounded user iteration.

• Loop through these user IDs and for each user, using the switch_user method to switch the user context (e.g. here):
  • Call Dismissed_Prompts->remove for the slug audience_segmentation_setup_cta-notification.
  • Call Dismissed_Items->remove for the slugs audience-segmentation-add-group-notice, audience_segmentation_setup_success_notification, settings_visitor_groups_setup_success_notification and audience-segmentation-no-audiences-banner.
  • Call Dismissed_Items->get, use array_filter to find all of the items which have a slug that begins with audience-tile- and call Dismissed_Items->remove on them (this removes the "temporarily hidden" audiences state).
  • Call Audience_Settings -> merge to set the user configuredAudiences and didSetAudiences settings to their respective defaults of null and false.
• Switch back to the current user as in the example.
• In the register method, call Analytics_4/Settings -> on_change to set up a change handler.
  • When the propertyID value changes, call Analytics_4/Settings -> merge to set the following values to their defaults:
    • availableAudiences => null
    • availableAudiencesLastSyncedAt => 0
    • audienceSegmentationSetupComplete => false
• Remove availableAudiencesLastSyncedAt from the existing call to reset settings in Analytics_4.

Register an instance of Reset_Audiences

• Register an instance of Reset_Audiences in the Analytics_4::register method.

Test Coverage

• Add new PHPUnit tests to cover the above changes.

QA Brief

• Create multiple WordPress users, including admin and view-only users.
• Create audience settings and dismissed items/prompts for each user. You can do this by:
  • Setting up the Audience Segmentation feature for each user and configure some audiences using the audience selection panel.
  • Dismiss prompts in the UI or...
  • You can add the dismissed prompt for each user, using the googlesitekit.data.dispatch( 'core/user' ).dismissPrompt( 'audience_segmentation_setup_cta-notification' ) action in the Chrome console, only the audience_segmentation_setup_cta-notification prompt is reset in this work.
  • You can add all possible dismissed items for each user, using the googlesitekit.data.dispatch( 'core/user' ).dismissItem( '{SLUG}' ) action in the Chrome console, you can see a list of the items that this work should reset here. Note the wildcard audience-tile-* means any item slug that begins with audience-tile- should be reset by this work.
• To trigger the reset, as an admin user, go to the Analytics module settings panel and change the Google Analytics property and click save.
  • The Audience Segmentation settings, prompt and items should be reset for all users. You can check this in the UI as well as looking at the database tables wp_usermeta and wp_options tables confirming the above keys are removed and settings reset.
• This reset should also happen when the Analytics module is disconnected.

Changelog entry

• Ensure the Audience Segmentation feature is effectively reset when changing the connected Analytics property.

[techanvil]

I've moved this back to Backlog as the final in-progress changes to the design doc, relating to audience caching, will probably affect the AC for this one

[techanvil]

The audience caching aspect of the design doc has been sufficiently finalised, and I've moved this back to AC.

[benbowler]

I considered a few different approaches for this including creating a new REST endpoint, however the hooks approach seems to be to be the cleanest as it doesn't require adding Analytics specific logic to shared components like the module store or settings Footer component.

[eclarke1]

@benbowler should this one be in IBR?

[benbowler]

@eclarke1 not quite, I discussed the IB here with @aaemnnosttv last thing on Friday. I need to review it and may make some updates before putting it to IBR.

[benbowler]

My discussion with @aaemnnosttv revolved around changing the dismissed item/prompt slug, adding the account and property ID so that they would naturally be invalidated by switching the account/property.

The availableAudiences and availableAudiencesLastSyncedAt would still need to be reset as in my original IB.

Starting a thread here with @techanvil to get your thoughts before I update the IB.

[techanvil]

Hi @benbowler, this would be a good solution if we did want to retain the state when the user reverts to the same property later.

However, this is not actually the case. If the user reverts to the same property, we still want to re-show the Setup CTA Banner in order to allow them to re-setup the feature, and correspondingly show the setup success notifications once they have set it up again. We also want to re-show the Audience Creation Notice to give them another opportunity to create the SK audiences, and of course don't want to restore the "temporarily hidden" state of audiences as this would be confusing having set the feature up again.

So, we still want to delete the dismissed items/prompts when switching/resetting the property, and AFAICT it wouldn't really help to prefix the keys as suggested. Definitely a good pattern to bear in mind for other situations but I don't think it would be applicable here.

[techanvil]

Hey @benbowler, thanks for drafting this IB. A few points to consider:

• Check the user has setup permissions before continuing current_user_can( Permissions::SETUP );.

Hmm... This might be a bit of a redundant check as by the time reset_audience_settings() is called, we should be confident the user has the correct permissions. If we definitely need it let's keep it but otherwise it would be nice to drop this point.

• Get all users using get_authenticated_users, loop through these users and for each user, using the switch_user method to switch the user context (eg here):

Non-authenticated users can also "temporarily hide" tiles and have their own audience settings, so we need a different approach to iterating the users for at least these values.

Maybe, instead of reusing the get_authenticated_users() logic, we can invoke get_users() to only return users who have the googlesitekit_audience_settings user option set. This should suffice to retrieve all the users we're interested in, because all users who have set up and interacted with the Audience Segmentation feature will have set their googlesitekit_audience_settings as part of the setup process.

get_users(
   array(
      'meta_key'     => $this->user_options->get_meta_key( 'googlesitekit_audience_settings' ),
      'meta_compare' => 'EXISTS',
      'fields'       => 'ID',
   )
);

• Call Dismissed_Items->remove for the slugs audience-segmentation-add-group-notice, audience_segmentation_setup_success_notification and audienceSegmentationIntroductoryOverlayNotification.

We don't want to delete the audienceSegmentationIntroductoryOverlayNotification slug, as this is for the Introductory Popup which is (deliberately) not mentioned in the AC. However we do want to delete the slug for the Setup Success Settings Notice which is being introduced in the dependency #8178.

• Get the analytics module settings using $this->get_settings(), remove the key availableAudiences and availableAudiencesLastSyncedAt, and update the module settings using $module->get_settings()->merge( (array) $request['data'] );.

The snippet doesn't look quite right here - aside from $request['data'] not being relevant, the call to merge() doesn't seem correct for values we want to delete. That said - rather than delete them we can reset them to their default values of null and 0, in which case merge() will be useful.

• Do a new action in includes/Core/Modules/Modules.php, deactivate_module called "googlesitekit-deactivate-module-" . $slug.
  • Update includes/Modules/Analytics_4.php, use add_filter to call the new reset_audience_settings method on the googlesitekit-deactivate-module-analytics-4 hook.

We don't need a new action for this, we have the Analytics_4::on_deactivation() method that we can amend.

• Update the "googlesitekit_pre_save_settings_" . $slug action call to pass (array) $request['data'] as the first param.
  • Update includes/Modules/Analytics_4.php, create a new method reset_audience_settings_on_account_or_property_change which take one param $new_settings.
    • Get the current analytics settings using $this->get_settings.
    • If either the propertyID or accountID has changed, call the reset_audience_settings method.
  • Update includes/Modules/Analytics_4.php, use add_filter to call the new reset_audience_settings_on_account_or_property_change method on the "googlesitekit_pre_save_settings_" . $slug hook.

Rather than using the googlesitekit_pre_save_settings_ hook, we can add to the block of code in the Analytics settings' on_change() handler which runs when the property ID changes (this should be fine for the requirements here, we don't really need to look for the account ID changing as well).

[techanvil]

Update to the above:

Maybe, instead of reusing the get_authenticated_users() logic, we can invoke get_users() to only return users who have the googlesitekit_audience_settings user option set. This should suffice to retrieve all the users we're interested in, because all users who have set up and interacted with the Audience Segmentation feature will have set their googlesitekit_audience_settings as part of the setup process.

Upon reflection, I've realised this wouldn't give us all users we are interested in - users who have simply dismissed the initial setup banner, via the audience_segmentation_setup_cta-notification prompt, obviously won't have set the feature up and so won't have the googlesitekit_audience_settings entry in the usermeta table. We'd still need to iterate over the authenticated users (or, maybe an alternative approach e.g. finding all users who have dismissed prompts).

[ivonac4]

Leaving a note from the AS sync: Tom will speak with Evan about Ben's suggestions about this task, since it is touching the fundamental parts of the project.

[nfmohit]

@techanvil I've reviewed the PR for this issue and the current state LGTM! During my review, I found that a part of the IB and its consequent implementation is not correct, the following part specifically:

• In the register method, call Analytics_4/Settings -> on_change to set up a change handler.

  • When the propertyID value changes, call Analytics_4/Settings -> merge to set the following values to their defaults:

    • availableAudiences => null
    • availableAudiencesLastSyncedAt => 0
    • audienceSegmentationSetupComplete => false

• Remove availableAudiencesLastSyncedAt from the existing call to reset settings in Analytics_4.

I've mentioned about this in detail here and have changed this in the PR to correct it.

In summary, to reset the availableAudiences and audienceSegmentationSetupCompletedBy to their default value of null, we do need to use the pre_update_option_googlesitekit_analytics-4_settings hook as the Module_Settings::merge method (used in the on_change handler) does not support null as a valid setting value.

In the spirit of DRY, I've not repeated the on_change handler and the pre_update_option_googlesitekit_analytics-4_settings hook in the Reset_Audiences class, but rather left the reset of the AS-specific module settings within Analytics_4 with appropriate test coverage. Let me know if you disagree and if these should be recreated in Reset_Audiences.

As I have made some significant changes to the PR, I'm assigning this issue to you for MR. Thanks!

CC: @benbowler

[kelvinballoo]

QA Update ⚠️

Hi @benbowler on the outset, this is working as expected.
I have a couple of clarifications to be confirmed though, as listed below:

ITEM 1: ⚠️
I have used 3 users to the site.
When testiing for the googlesitekitpersistent_dismissed_items, I dismissed all the required items and changed the Analytics property.
It seems that not all of them is 100% cleared. I
Refer to the details where:

• For ID 35, it was cleared.
• For the other IDs 72 and 90, the "audienceSegmentationIntroductoryOverlayNotification" remained.
  I assume that's fine because it's not part of the items we worked on. Can you confirm?

---

ITEM 2: ⚠️
The AC references the following:
The "temporarily hidden" state for any hidden audiences should be cleared. The "temporarily hidden" state for the No Audiences Banner should be cleared.

Is there any specific properties that I should check for temporarily hidden items? Or this is part of the configuredAudiences?

---

ITEM 3: ⚠️
For iwpc986_googlesitekit_audience_settings, whenever we disconnect or change analytics properly, the values would not only be cleared but the entired rows will be deleted.
I assume that's expected. Could you help to confirm?

---

ITEM 4: ⚠️
For the WP Options table, I was expecting the following after disconnecting/changing analytics:
availableAudiences => null
availableAudiencesLastSyncedAt => 0
audienceSegmentationSetupComplete => false

The actual results were as follows:
"availableAudiences";N;s:30:"availableAudiencesLastSyncedAt";i:0;s:36:"audienceSegmentationSetupCompletedBy";N;s:14:"detectedEvents";a:0:{}}

• For availableAudiences, I assume N is synonymous to null
• For availableAudiencesLastSyncedAt, it became 0 as expected.
• For audienceSegmentationSetupCompletedBy, I was expecting false but it was 'N'. Is that expected?

---

Other than that, iwpc986_googlesitekitpersistent_dismissed_prompts were reset successfully

[techanvil]

Thanks @kelvinballoo, I've taken a look at your feedback.

ITEM 1: ⚠️ I have used 3 users to the site. When testiing for the googlesitekitpersistent_dismissed_items, I dismissed all the required items and changed the Analytics property. It seems that not all of them is 100% cleared. I Refer to the details where:

• For ID 35, it was cleared.
• For the other IDs 72 and 90, the "audienceSegmentationIntroductoryOverlayNotification" remained.
  I assume that's fine because it's not part of the items we worked on. Can you confirm?

That's correct, we have deliberately excluded this key from the reset logic. Once a secondary user has seen the introductory popup/banner for the feature, they have been introduced to it so don't need to see it again. We might want to think about this again during the bug bash but it's the specced behaviour for now at least.

ITEM 2: ⚠️ The AC references the following: The "temporarily hidden" state for any hidden audiences should be cleared. The "temporarily hidden" state for the No Audiences Banner should be cleared.

Is there any specific properties that I should check for temporarily hidden items? Or this is part of the configuredAudiences?

You can verify this via the plugin UI by temporarily hiding the items, changing the property, then reconnecting the previous property and confirming that the items are no longer hidden.

If you want to check it in the DB, you should check the googlesitekitpersistent_dismissed_items entry for the key audience-segmentation-no-audiences-banner for the No Audiences Banner, and any key starting with audience-tile for the hidden audiences.

ITEM 3: ⚠️ For iwpc986_googlesitekit_audience_settings, whenever we disconnect or change analytics properly, the values would not only be cleared but the entired rows will be deleted. I assume that's expected. Could you help to confirm?

Thanks for flagging this. It's actually not as expected and required a followup to fix it, because we shouldn't be completely clearing the settings - we want to retain the user's choice for the isAudienceSegmentationWidgetHidden as per the AC/IB update mentioned here. So we should actually be calling merge() on the Audience_Settings instance to set configuredAudiences and didSetAudiences back to their defaults as per the IB rather than deleting the settings.

Reviewing Audience_Settings I can see it will need a bit of additional work to allow merging null for configuredAudiences, which wasn't allowed for in the IB, so I feel we should mark this as a known issue and address it in a followup issue rather than a followup PR for this one. Please can you create an issue in Triage and I'll add AC for it?

ITEM 4: ⚠️ For the WP Options table, I was expecting the following after disconnecting/changing analytics: availableAudiences => null availableAudiencesLastSyncedAt => 0 audienceSegmentationSetupComplete => false

The actual results were as follows: "availableAudiences";N;s:30:"availableAudiencesLastSyncedAt";i:0;s:36:"audienceSegmentationSetupCompletedBy";N;s:14:"detectedEvents";a:0:{}}

• For availableAudiences, I assume N is synonymous to null
• For availableAudiencesLastSyncedAt, it became 0 as expected.
• For audienceSegmentationSetupCompletedBy, I was expecting false but it was 'N'. Is that expected?

This is as expected, yes - the audienceSegmentationSetupCompleted setting was renamed to audienceSegmentationSetupCompletedBy with a default value of null after the IB was drafted so it's a little out of date.

Btw - you're correct with regard to the N => null mapping. It is possible to use the WP CLI on InstaWP to retrieve the mapped version of the data - it takes a little setting up, you need to go to https://app.instawp.io/commands, create a command which runs wp option get googlesitekit_analytics-4_settings (or whichever setting you want to query), then run it via the Commands menu item for a site:

[kelvinballoo]

QA Update ✅

Thanks @techanvil , noted on items 1, 2, 4 and 5.
For item 3, I've raised an issue here: #9432 and assigned to you.

For ITEM 2, I've done a test as you suggested and the 'Temporarily Hidden' badge is not carried through after changing analytics property or disconnecting the Analytics module and adding back the Analytics property.
YOu may refer below for the video.

• Also verified per the video, when we change analytics property, the tiles are reset and they don't appear anymore.
• Also verified that when we disconnect analytics property, the tiles are gone too.

8180.item.2.480p.mov

Other items were also verified good : ✅

• googlesitekitpersistent_dismissed_items and googlesitekitpersistent_dismissed_prompts were reset successfully when changing analytics property.

  

• For the WP Options table, the 3 parameters: AvailableAudiences, availableAudiencesLastSyncedAt and audienceSegmentationSetupCompletedBy were all reset successfully when changing analytics property or disconnecting it.

  Actual results were as follows in the DB editor table after reset: "availableAudiences";N;s:30:"availableAudiencesLastSyncedAt";i:0;s:36:"audienceSegmentationSetupCompletedBy";N;s:14:"detectedEvents";a:0:{}}

• This was tested on a couple of WP Users : 2 admins and one editor and results were consistent

Moving ticket to Approval.