Refactor Analytics account and property dropdowns to source from account summaries

[aaemnnosttv]

Feature Description

The setup and settings interfaces for Analytics use dropdowns for selecting the account and properties to manage the connected property (and now datastream for tagging).

This was initially built to use endpoints based on accounts.list and properties.list which return much more data than is needed and can result in multiple calls. Account Summaries are a leaner resource for getting an overview of nearly all accounts and their properties that a user has access to in a single request. We already use account summaries for some things but the main interface still uses the initial architecture based on the less efficient resources.

Now that setup with UA is no longer possible, we can start refactoring the connection management interface to use GA4 account summaries for accounts and properties entirely which should result in a more performant experience as well.

---

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

Acceptance criteria

• The Analytics dropdowns for Account and Property should be sourced from account summaries

Implementation Brief

• In assets/js/modules/analytics-4/datastore/accounts.js:
  • Create a new selector named getAccounts that should return the state value for accountSummaries. However, it should use a resolver to auto-select matching account ID and GA4 property ID. This can be adapted from the MODULES_ANALYTICS getAccounts resolver (only the last part specific to GA4).
• In assets/js/modules/analytics-4/datastore/properties.js:
  • Create a new selector named getPropertySummaries that should accept an accountID string parameter. It should use the MODULES_ANALYTICS_4 getAccountSummaries selector to get all account summaries, and return the propertySummaries array parameter for the provided account ID.
  • Similar to the waitForProperties action, create a new waitForPropertySummaries action that should accept an accountID string parameter and use a control to await resolve select the new getPropertySummaries selector passing the provided accountID as a parameter.
  • Update the matchAccountProperty action to:
    • dispatch the waitForPropertySummaries action instead of waitForProperties.
    • use the getPropertySummaries selector instead of getProperties.
  • Create a new selector named isLoadingPropertySummaries:
    • This should be based on the isLoadingProperties selector.
    • Instead of using the isResolving getProperties selector for the isResolvingProperties variable definition, it should instead use MODULES_ANALYTICS_4 isResolving getAccountSummaries selector.
  • The waitForProperties action is no longer needed and should be removed.
  • The isLoadingProperties selector is no longer needed and should be removed.
• In assets/js/modules/analytics-4/components/settings/SettingsEnhancedMeasurementSwitch.js & assets/js/modules/analytics-4/components/setup/SetupEnhancedMeasurementSwitch.js:
  • The isLoadingProperties selector usage should be replaced with the new isLoadingPropertySummaries selector.
• Create assets/js/modules/analytics-4/components/common/AccountSelect.js:
  • This should be based on assets/js/modules/analytics/components/common/AccountSelect.js.
  • In the accounts variable definition, use the getAccounts selector from the MODULES_ANALYTICS_4 store.
  • In the hasResolvedAccounts variable definition, use the hasFinishedResolution selector from the MODULES_ANALYTICS_4 store.
  • Remove the useEffect and all other relevant unused variable definitions.
  • In the finally returned <Select> component, instead of using the id property from destructured accounts array items, use _id.
• In assets/js/modules/analytics/components/setup/SetupFormGA4.js & assets/js/modules/analytics/components/settings/GA4SettingsControls.js:
  • The AccountSelect component being used should be imported from assets/js/modules/analytics-4/components/common/AccountSelect.js.
• In assets/js/modules/analytics-4/components/common/PropertySelect.js:
  • Instead of using the MODULES_ANALYTICS getAccounts selector, use it from MODULES_ANALYTICS_4.
  • In the properties variable definition, use the MODULES_ANALYTICS_4 getPropertySummaries selector.
  • In the isLoading variable definition, use the MODULES_ANALYTICS_4 isLoadingPropertySummaries selector.

Storybook

• Fix any Storybook stories that do not function correctly by mocking data using the account summaries state.

Test Coverage

• Add test coverage for the new AccountSelect component, similar to the existing legacy version.
• Add test coverage for the new analytics-4 getAccounts selector.
• Add test coverage for the new analytics-4 getPropertySummaries selector.
• The usage of isLoadingProperties in this test case should be replaced with isLoadingWebDataStreams.
• Fix any failing tests.

QA Brief

• Verify that the account and property dropdowns in the Analytics module setup and settings screens are populated with data sourced from GA4 account summaries.
• Check the network tab to verify the API calls were made to the account-summaries endpoint.
• Verify the property is correctly selected for the account and the site URL from the account summaries on the initial load. This can be checked from the account-summaries endpoint response from the network tab.
• Check that no API calls were made to the accounts-properties-profiles and properties endpoints to load the accounts and properties.
• Note that an API call to the properties-profiles endpoint will still be made upon changing the account as it was called by the selectAccount action.

Additional Notes:

• For accounts with UA properties matching the URL, the matched property is determined on the server using the property's website URL. This matching is implemented in the accounts-properties-profiles endpoint.
• If the account includes only GA4 properties, the matched property is found on the client-side logic through the web data stream. In this case, the account will be selected based on the web data stream's matched property ID.
• Consequently, there will be a difference in the account selection compared to the accounts-properties-profiles endpoint. This difference is expected and should not be considered a bug.

Update

• Verify that new GA4 properties can be created from the settings and setup screens. Please refer to this comment for more details and ensure both the mentioned issues are fixed.

Changelog entry

• Update Analytics components to use data from account summaries.

[techanvil]

Hi @nfmohit, this IB is looking good. A few points to consider:

• It would be good to clean up unused code here where we can identify it. For example it looks like the GA4 waitForProperties() action would no longer be needed and can be removed.
• You've specced a new AccountSelect component, but not specified where it should be used. Presumably it should replace existing use of the current Analytics AccountSelect, which can also be removed.
• The isLoadingProperties() selector exists to ensure the loading state of PropertySelect can be shared with the SettingsEnhancedMeasurementSwitch and SetupEnhancedMeasurementSwitch components, so these components should also be updated to use isLoadingPropertySummaries(). It looks like isLoadingProperties() will also be able to be removed.
• Some additional testing should be specified - the new datastore code will need test coverage, and the current AccountSelect component has test coverage so it could be appropriate to cover the new one too.
• While reviewing the current use of isLoadingProperties() I've noticed an error in a test where isLoadingProperties() is called in place of isLoadingWebDataStreams(). Although we could fix this in a separate issue, it might be worth simply fixing it as part of this one while removing isLoadingProperties().

[nfmohit]

Hi @nfmohit, this IB is looking good. A few points to consider:

• It would be good to clean up unused code here where we can identify it. For example it looks like the GA4 waitForProperties() action would no longer be needed and can be removed.
• You've specced a new AccountSelect component, but not specified where it should be used. Presumably it should replace existing use of the current Analytics AccountSelect, which can also be removed.
• The isLoadingProperties() selector exists to ensure the loading state of PropertySelect can be shared with the SettingsEnhancedMeasurementSwitch and SetupEnhancedMeasurementSwitch components, so these components should also be updated to use isLoadingPropertySummaries(). It looks like isLoadingProperties() will also be able to be removed.
• Some additional testing should be specified - the new datastore code will need test coverage, and the current AccountSelect component has test coverage so it could be appropriate to cover the new one too.
• While reviewing the current use of isLoadingProperties() I've noticed an error in a test where isLoadingProperties() is called in place of isLoadingWebDataStreams(). Although we could fix this in a separate issue, it might be worth simply fixing it as part of this one while removing isLoadingProperties().

Thank you for the kind and detailed review here, @techanvil. I've updated the IB accordingly, and have also bumped up the estimate by one notch. Please let me know if it looks good now, thanks!

[techanvil]

Thanks @nfmohit, that's great. IB LGTM ✅

[wpdarren]

QA Update: ⚠️

@hussain-t This looks good, but I have an observation for an account with a UA and GA4 property.

Should I see the accounts-properties-profiles endpoint in the scenario above? I don't, but I do see the GA4 in the account-summaries endpoint. It might be my understanding, but just checking based on the QAB note.

For accounts with UA properties matching the URL, the matched property is determined on the server using the property's website URL. This matching is implemented in the accounts-properties-profiles endpoint.

[hussain-t]

@wpdarren, no, a network call shouldn't be made to the accounts-properties-profile endpoint. That note clarified that we determined UA's matched account property for the URL from the server side. However, currently, it happens in the client. Hence, you may see some differences in the matching account/property. If that occurs, it is not a bug.

cc: @bethanylang

[wpdarren]

QA Update: ✅

Verified:

• The account and property dropdowns in the Analytics module setup and settings screens are populated with data sourced from GA4 account summaries.
• The network tab includes the API calls to the account-summaries endpoint.
• The property is correctly selected for the account and the site URL from the account summaries on the initial load.
• No API calls were made to the accounts-properties-profiles and properties endpoints.
• Noted that an API call to the properties-profiles endpoint will still be made upon changing the account as it was called by the selectAccount action.

Screenshots

[hussain-t]

Issue 1: @mohitwp raised an issue on Slack that creating a GA4 new property from the settings screen throws an error. This is because the account summaries weren't loaded after creating a new property. Hence, the newly created property was only available after reloading the page.

Recording.723.mp4

---

Issue 2: While investigating the above issue, I found that the propertyID is not set in the property dropdown even after refetching the account summaries. That's because the loading time wasn't enough in the PropertySelect component due to data synchronization/availability delay. This issue was noticeable in the Setup form as the settings edit screen shows the ProgressBar when submitting the form.

Screen.Recording.2024-01-10.at.5.25.43.PM.mov

I have fixed the above issues in this follow-up PR.

cc: @aaemnnosttv @bethanylang @nfmohit

[wpdarren]

QA Update: ❌

@hussain-t I see strange UX/UI behavior when creating a new property and web data stream on the initial setup. I haven't tested the settings yet, but will do so and report back.

1. The setup flow for creating a new property is strange because the property dropdown appears blank after returning from oAuth, and the behavior is unexpected. The screencast shows you the process of creating a new property. A new property does seem to be created, but I feel it's confusing when going through the setup flow.

new-property.mp4

2. An error message appears when creating only a new web data stream and returning from Oauth. Error: Requested entity already exists. You can see it in action in the screencast.

new-web.mp4

[hussain-t]

@wpdarren, I'm looking into the first issue. As for creating a new web data stream error, it was from the API. No server-side changes were made as part of this ticket. Let's create a new ticket to address it.

[hussain-t]

As discussed on Slack, the issue occurred due to the build issue.

[wpdarren]

QA Update: ✅

@hussain-t thanks for the investigation! Just to clarify, point 2 I will create a new ticket and point 1 is not an issue now the site has an updated main branch. I will raise this issue with Evan on our next QA sync as it is rather annoying that this happens as it involves additional work for you and me. I did refresh the main numerous times, but it wasn't until we selected the latest and then the main that the build was completed properly.

Verified:

• The account and property dropdowns in the Analytics module setup and settings screens are populated with data sourced from GA4 account summaries.
• The network tab includes the API calls to the account-summaries endpoint.
• The property is correctly selected for the account and the site URL from the account summaries on the initial load.
• No API calls were made to the accounts-properties-profiles and properties endpoints.
• Noted that an API call to the properties-profiles endpoint will still be made upon changing the account as it was called by the selectAccount action.

Additional testing around setting up Analytics and creating new properties in the connect Analytics screen and also in Analytics settings. The issues reported above are now fixed.

Screenshots

[aaemnnosttv]

2. An error message appears when creating only a new web data stream and returning from Oauth. Error: Requested entity already exists. You can see it in action in the screencast.

This is a known issue; see #6727

As for the calls to properties-profiles, that should really be unnecessary but that is a bit out of scope for this issue. I think we probably need to reimplement selectAccount for GA4. cc @nfmohit

[nfmohit]

As for the calls to properties-profiles, that should really be unnecessary but that is a bit out of scope for this issue. I think we probably need to reimplement selectAccount for GA4. cc @nfmohit

The GA4 selectAccount action is being implemented as a part of #7927.