Refactor setup success notification to take module-specific copy adjustments from datastore

[felixarntz]

A googlesitekit.SetupWinNotification-${ slug } filter is currently applied to allow other code to replace content in the setup success notification. This should be replaced by a mechanism relying on the respective module's data store.

---

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

Acceptance criteria

• The modules/analytics and modules/optimize data stores should receive a new selector getSetupSuccessContent that returns an object with the same data that is currently called winData in the two modules' respective usage of the googlesitekit.SetupWinNotification-${ slug } filter.
• Instead of applying the googlesitekit.SetupWinNotification-${ slug } filter, the DashboardSetupAlerts component should rely on the new selector in the respective module store if it exists, to get that information from there.
• The usage of the googlesitekit.SetupWinNotification-${ slug } filter in the two modules mentioned above should be removed as well.

Implementation Brief

• Create assets/js/modules/optimize/datastore/notification.js which contains the getSetupSuccessContent selector. The latter should return an object containing the same winData data as in assets/js/modules/optimize/index.js.
  • Remove the use of addFilter in assets/js/modules/optimize/index.js.
• Update assets/js/modules/optimize/datastore/index.js to add the new data store partial using combineStores.
• Create assets/js/modules/analytics/datastore/notification.js which contains the getSetupSuccessContent selector. The latter should return an object containing the same winData data as in assets/js/modules/analytics-4/index.js.
  • Remove the use of addFilter in assets/js/modules/analytics-4/index.js.
• Update assets/js/modules/analytics/datastore/index.js to add the new data store partial using combineStores.
• Using assets/js/components/legacy-notifications/dashboard-setup-alerts.js, remove the use of googlesitekit.SetupWinNotification-${ slug }, to instead rely on the getSetupSuccessContent selector if it exists for the module to get the data.
  • There is an additional check when requesting data for the analytics data store, by checking if the ga4setup feature flag is enabled. Refer to assets/js/modules/analytics-4/index.js.

Test Coverage

• Add new tests for the getSetupSuccessContent selectors for both data stores.

QA Brief

• Setup the "Optimize" module and check if the success notification renders as usual.
• Setup the "Analytics" module with the ga4setup feature flag off. Check the notification component if it's displayed correctly.
• Setup "Analytics" module again with the ga4setup feature flag on. Check if the notification component displays correctly, with the text "You’ll only see Universal Analytics data for now. Learn more"

Changelog entry

• Remove legacy googlesitekit.SetupWinNotification-${ slug } filter.

[tofumatt]

IB ✅

[tofumatt]

The ACs here mention Analytics and PageSpeed Insights, but the IB deals with Analytics and Optimize. I totally missed that when reviewing 🤦🏻

I think it should be fine to change to PageSpeed Insights from Optimize, and mentioned as much in my PR review. Is that alright @asvinb?

Sorry about that! 😓

[asvinb]

@tofumatt Actually is there a possibility that the AC is wrong because the googlesitekit.SetupWinNotification-${ slug } filter is used twice in the codebase:

• In the Analytics module
• In the Optimize module

If you check the PSI module, it's not there.

I completely missed that as well, with me focusing on where the filter is being used and it's used twice, not paying much attention to which modules exactly.

What do you think?

[tofumatt]

Hmm, that makes sense. From my perspective this code is good-to-go, so I'll assign @felixarntz to do the merge review.

If it turns out the ACs here were wrong can you update them @felixarntz and merge this? Just wanna make sure 🙂

[felixarntz]

@asvinb @tofumatt Good catch, you're right! I've updated the ACs accordingly, will check the PR now.

[wpdarren]

QA update: ⚠️

@tofumatt @asvinb an observation:

Setup the "Analytics" module with the ga4setup feature flag off. Check the notification component if it's displayed correctly.

I started at this step because you cannot set up Optimize without Analytics. When I connect Google Analytics account, and then I am redirected back to the dashboard, no notification message appears. I have unifiedDashboard activated and on the develop branch. Am I understanding the QAB correctly, should the success notification appear?

[tofumatt]

Ah, the QA brief here is… well, it's complete in that it doesn't mention needing the Unified Dashboard feature flag enabled, but that's a bit strange for a "Unified Dashboard" epic feature.

This issue is about laying the groundwork to do these notifications without filters, which will be used by the Unified Dashboard, but we don't actually have those notifications in the Unified Dashboard yet.

So the QA steps outlined here should be done with the Unified Dashboard feature flag disabled. Sorry for the confusion! 😅

[wpdarren]

QA Update: ✅

Verified:

• When setting up the "Optimize" module the success notification renders as usual.
• When setting up the "Analytics" module with the ga4setup feature flag off, the notification component is displayed

• When setting up "Analytics" module with the ga4setup feature flag on, the notification component displays