site-kit-wp icon indicating copy to clipboard operation
site-kit-wp copied to clipboard

Re-check `registerDefaults` of the notification API if there is any resolver that needs to be resolved

Open zutigrm opened this issue 1 year ago • 2 comments

Feature Description

Go over the assets/js/googlesitekit/notifications/register-defaults.js and check if there are resolvers, or selectors that are using different resolvers (like in the case of hasZeroData) in checkRequirements that might be missed, and are retrieved using select instead of resolveSelect. One such example is isModuleConnected which is currently not resolved. There might be more


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

Acceptance criteria

  • All the data used in checkRequirements property of currently registered notifications should be properly resolved where needed - specifically when retrieving the data from resolvers.
    • One such example is isModuleConnected which is currently not resolved

Implementation Brief

  • [x] Update assets/js/googlesitekit/notifications/register-defaults.js
    • Data from isModuleConnected resolver should be retrieved with resolveSelect and awaited
    • There are few selectors, which rely on data from different resolver, which can't guarantee having data available at the point when notifications API is invoked, although there is good chance this data is already resolved and available due to requests for this mandatory data is done very early.
      • isAuthenticated and getUnsatisfiedScopes selectors are both using getAuthentication resolver, which fetches the needed data.
      • canViewSharedModule selector, is relying on having data from getModule resolver.
      • Invoke these dependency resolvers and await them before the selectors are called, to ensure data is resolved properly
  • Check if there is any other resolver that might be missed, in case new notifications are added after this IB has been written.

Test Coverage

  • No updates are needed

QA Brief

  • The following notifications need to be re-tested:
    • Gathering Data
    • Zero Data
    • UnsatisfiedScopesAlert
    • UnsatisfiedScopesAlertGTE
    • GA4AdSenseLinked

Changelog entry

  • Update requirement checks for notifications to have all selectors resolved correctly and efficiently.

zutigrm avatar Sep 13 '24 10:09 zutigrm

AC ✔️

eugene-manuilov avatar Sep 13 '24 18:09 eugene-manuilov

IB ✔️

eugene-manuilov avatar Sep 18 '24 17:09 eugene-manuilov

QA Update ✅

  • Tested on dev environment.
  • Verified that the following notifications showing correctly and associated events getting trigger successfully.
    • Gathering Data
    • Zero Data
    • UnsatisfiedScopesAlert
    • UnsatisfiedScopesAlertGTE
    • GA4AdSenseLinked

Zero and Gathering data state notifications

SC is in zero data state

Image

Analytics is in gathering state but SC is in zero state

Image

AC and Analytics both in zero data state

Image

SC and analytics both in gathering data state

Image

Both SC and analytics is in zero data state

Image

SC is in gathering data state and Analytics is in zero data state

Image

UnsatisfiedScopeAlert

Image

Image

UnsatisfiedScopeAlertGTE

Image

GA4AdSenseLinked

Image

mohitwp avatar Oct 30 '24 00:10 mohitwp