Copy over analytics module settings to analytics-4

[nfmohit]

Feature Description

Module settings in the analytics module that are still relevant after the removal of UA should be copied over to the analytics-4 module. Namely:

1. accountID
2. adsConversionID
3. canUseSnippet
4. ownerID
5. trackingDisabled
6. useSnippet

A runtime process migration should be introduced in the Analytics_4 class that migrates the settings data from analytics to analytics-4. For example, accountID setting data in googlesitekit_analytics_settings should be migrated over to googlesitekit_analytics-4_settings.

---

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

Acceptance criteria

• The following module settings in the analytics module should be copied over to the analytics-4 module:
  • accountID
  • adsConversionID
  • canUseSnippet
  • trackingDisabled
  • adsenseLinked
• When the value for one of these copied settings is queried and it doesn't exist in the googlesitekit_analytics-4_settings option, it should fallback to the data stored in the googlesitekit_analytics_settings option (if it exists there).
  • The existing ownerID setting should also be treated in the same way, instead of how it is currently always synced between the two modules.

Implementation Brief

• In includes/Modules/Analytics_4/Settings.php:
  • Update the get_owned_keys method to include the accountID module setting.
  • Update the get_default method to include defaults for the module settings mentioned in the ACs. The default values can be copied from the Modules\Analytics\Settings class.
  • Update the get_sanitize_callback method to add sanitization for the canUseSnippet, trackingDisabled, and adsenseLinked module settings. These sanitization rules can be copied from the Modules\Analytics\Settings class.
  • Remove the merge_initial_owner_id method.
  • Remove the update_owner_id_in_settings method.
  • Update the get method:
    • Remove the existing logic that returns the analytics module setting value for the ownerID setting.
    • For all the module settings that are being copied as a part of this issue, add a fallback for them so that if data for them do not exist in the analytics-4 module but does exist in the googlesitekit_analytics_settings option, the value from the googlesitekit_analytics_settings option is served.
      • Instead of using the Analytics_Settings class as a way to access the googlesitekit_analytics_settings option, use the always available get_option WordPress function. This will ensure that this fallback works even after the Modules\Analytics class is removed in the future (currently planned as a part of #7932).
• In assets/js/modules/analytics-4/datastore/base.js:
  • Include accountID in the ownedSettingsSlugs array.
  • Include all the module settings mentioned in the ACs to the settingSlugs array.

Test Coverage

• In tests/phpunit/integration/Modules/Analytics_4/SettingsTest.php:
  • Add test case(s) for the get method to verify the fallback behaviour for the copied settings.

QA Brief

• Use existing website where Analytics was previously connected
• Go to Analytics settings
• Verify that data is still correct
• Open console log, and paste googlesitekit.data.select('modules/analytics-4').getSettings(), hit enter. Expand the array, verify that keys listed in AC are present and have correct data
• Edit settings, update something (like excluding/including logged in users in tracking) and save it. Verify that settings are still correctly saved
• Refresh the page, paste the same snippet in console log, ensure that they reflect the change (Focus on new properties listed in AC)
• Reset Site Kit, and set it up again with analytics, verify that all settings are correctly saved

Changelog entry

• Update Analytics 4 settings to use Analytics settings if they are not migrated yet.

[tofumatt]

ACs here are good, moving to IB 👍🏻

[nfmohit]

Update: After a discussion with @aaemnnosttv, I have updated the ACs of this issue so that instead of doing a once-off migration of the module settings, we fallback to the data stored in googlesitekit_analytics_settings option. This will prevent any data discrepancy until the two modules have been entirely separated, as this is an iterative process.

CC: @tofumatt

[eugene-manuilov]

IB ✔️

[wpdarren]

QA Update: ✅

Verified:

• Used an existing website where Analytics was previously connected
  • The data is still correct when updating the branch to develop
  • The array, verify that keys listed in AC are present and have correct data
  • Edited settings and are still correctly saved
• Refreshed the page, pasted the same snippet in the console log, and they reflect the change as per AC
• Reset Site Kit, and set it up again with analytics, and all settings are correctly saved.

Screenshots

[nfmohit]

Hi @wpdarren.

I was just working on reviewing some of the SAM work that is going out as part of this release, and I think this issue isn't working fully as expected.

The new GA4 settings are being correctly migrated if Analytics was set up before the PR for this issue was merged. However, if the Analytics module is connected after the PR is merged, the GA4 settings are not migrated. See:

The idea behind this issue was for the new analytics-4 settings to always fall back to whatever is set for their analytics counterparts if they are not set explicitly.

CC: @aaemnnosttv

[wpdarren]

@nfmohit, thanks for finding and sharing this issue. I left my thoughts on Slack about this. In short, I feel that we will rely a lot on the QAB for this epic because the changes are in the background. I'll bring it up in our retro to see what lessons we (I) can learn from this.

[nfmohit]

Hi @wpdarren.

I was just working on reviewing some of the SAM work that is going out as part of this release, and I think this issue isn't working fully as expected.

The new GA4 settings are being correctly migrated if Analytics was set up before the PR for this issue was merged. However, if the Analytics module is connected after the PR is merged, the GA4 settings are not migrated. See:

The idea behind this issue was for the new analytics-4 settings to always fall back to whatever is set for their analytics counterparts if they are not set explicitly.

CC: @aaemnnosttv

We have taken a deeper look into this and found out that there isn't a simple way for this to work for new Analytics module setups. As a result, I have opened a new issue to perform a DB migration of the settings just when we ship the singular Analytics module - #8082.

[aaemnnosttv]

Thanks @nfmohit!