Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the ZeroDataNotification to use the new Notifications datastore #8977

Closed
6 tasks done
jimmymadon opened this issue Jul 8, 2024 · 6 comments
Closed
6 tasks done
Labels
P2 Low priority Team S Issues for Squad 1 Type: Enhancement Improvement of an existing feature

Comments

@jimmymadon
Copy link
Collaborator

jimmymadon commented Jul 8, 2024

Feature Description

This issue will "use" the new datastore infrastructure to queue the ZeroDataNotification using the new datastore infrastructure.


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

Acceptance criteria

  • The ZeroDataNotification should be refactored and rendered (queued) using the new core/notifications datastore. This means that the component should not be called directly but via the generic getQueuedNotifications selector.
  • The refactored component should be its own standalone component without any business logic that prevents it from rendering. This component should be passed through when registering the new notification.

Implementation Brief

  • Wait for the dependency issues to be merged.
  • In assets/js/googlesitekit/notifications/register-defaults.js:
    • Add the registration of the ZeroDataNotification notification similar to the GatheringDataNotification component.
    • Priority should be 10 more than the GatheringDataNotification.
    • The check requirements call back function should be similar to GatheringDataNotification. However, return false if either hasSearchConsoleGatheringData or hasAnalyticsGatheringData params are true. This is so that, either the GatheringDataNotification and the ZeroDataNnotification NEVER get queued together.
    • Within BannerNotifications and EntityBannerNotifications, remove the call to ZeroDataNotification.

Test Coverage

  • No new tests required.

QA Brief

Ideally - WAIT for this issue to be merged and then QA both #8976 and this issue together as they are exactly the same pattern in code.

  • Thoroughly smoke test the GatheringData and ZeroData notifications:
    • Test when only Search Console is gathering data (and analytics is disconnected).
    • Test when both, Search Console and Analytics is Gathering Data.
    • Test when Search Console has finished gathering data but Analytics is still Gathering Data.
    • Test when either and both, SC and Analytics are in Zero data state.
    • Test all of the above for the Entity Dashboard as well.
  • Test when any other notification, say any other SetupSuccessBannerNotification is active - they should take precedence over the Gathering and Zero Data notifications.
  • Smoke test the remaining notifications to ensure they still work normally.
  • Test the new Storybook stories in Components -> Notifications.

Changelog entry

  • Refactor the ZeroDataNotification to use the new Notifications approach.
@jimmymadon jimmymadon added Type: Enhancement Improvement of an existing feature Team S Issues for Squad 1 labels Jul 8, 2024
@binnieshah binnieshah added the Next Up Issues to prioritize for definition label Jul 8, 2024
@eclarke1 eclarke1 added the P2 Low priority label Jul 8, 2024
@jimmymadon jimmymadon self-assigned this Jul 15, 2024
@binnieshah binnieshah removed the Next Up Issues to prioritize for definition label Jul 18, 2024
@eugene-manuilov
Copy link
Collaborator

AC and IB ✔️

@jimmymadon
Copy link
Collaborator Author

@mohitwp This issue is now merged, so I have assigned you this one as you were assigned to #8976. Both these issues can be QAed now together as per their QAB. c.c. @wpdarren

@mohitwp
Copy link
Collaborator

mohitwp commented Aug 16, 2024

QA Update ❌

  • Tested on dev environment.
  • Verified zero data notifications on main, entity and view only dashboard.
  • Smoke tested plugin notification and verified that other notifiations - take precedence over the Gathering and Zero Data notifications.
  • Tested storybook component >notifications stories.

@jimmymadon
Issue > On View only dashboard zero and gathering data notifications are not showing. On latest environment zero and gathering data state notifications are showing.

Recording.1304.mp4

Question > The "AdSense - Earnings at risk" notice reappears when we open the dashboard in a new tab, even after clicking "OK, Got It!" This occurs in both the latest and development environments. Is this expected?

image

Recording.1305.mp4

PASS CASES

SC gathering Data

image

SC and Analytics gathering data

image

Entity Dashboard (SC and Analytics gathering data)

image

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

image

Entity Dashboard ( SC is in gathering data state and Analytics is in zero data state.)

image

Both SC and analytics are in zero data state

image

Entity Dashboard ( Both SC and analytics are in zero data state)

image

Only Analytics is in gathering data state

image

Entity dashboard (Only Analytics is in gathering data state)

image

Other notifications- Another Admin

image

image

@mohitwp mohitwp assigned jimmymadon and unassigned mohitwp Aug 16, 2024
@jimmymadon
Copy link
Collaborator Author

Holding this issue in CR with me until #8976 has its follow up PR merged as it fixes this issue too. Then will move this straight to QA.

@jimmymadon jimmymadon assigned mohitwp and unassigned jimmymadon Aug 19, 2024
@jimmymadon
Copy link
Collaborator Author

@mohitwp Thanks - back to you for another pass!

@mohitwp
Copy link
Collaborator

mohitwp commented Aug 21, 2024

QA Update ✅

image

image

image

image

Recording.1324.mp4
Recording.1325.mp4

@mohitwp mohitwp removed their assignment Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Low priority Team S Issues for Squad 1 Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

6 participants