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 ModuleRecoveryAlert to use the new Notifications datastore #9299

Open
3 tasks done
zutigrm opened this issue Sep 4, 2024 · 2 comments
Open
3 tasks done

Refactor ModuleRecoveryAlert to use the new Notifications datastore #9299

zutigrm opened this issue Sep 4, 2024 · 2 comments
Assignees
Labels
P2 Low priority Team S Issues for Squad 1 Type: Enhancement Improvement of an existing feature

Comments

@zutigrm
Copy link
Collaborator

zutigrm commented Sep 4, 2024

Feature Description

This issue should refactor ModuleRecoveryAlert so that it uses the new datastore infrastructure to register and queue the notification. It should also incorporate a new "layout".


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

Acceptance criteria

  • The ModuleRecoveryAlert component should be refactored so that it is registered and rendered (queued) using the new core/notifications datastore.
  • This notification component should not be called directly (i.e. in BannerNotifications) but only via the generic getQueuedNotifications selector.
  • The refactored component should not contain any business logic that hides it / prevents it from rendering. This logic should be contained in a callback function defined during registration.
  • The component should not use the bloated BannerNotification component. Instead, it should be rendered using the new Notification component wrapper and a simpler "layout" component that solely encapsulates its structure and design.
  • The "Remind me later" CTAs have a different left margin so I have updated the VRT images so we can keep the layout consistent between new banner notifications rather than adding one of CSS to keep banners consistent with the old layout.

Implementation Brief

  • Update ModuleRecoveryAlert component
    • Check the UnsatisfiedScopesAlert component for an example
    • Include 2 new props - id and Notification
    • Wrap the component with Notification component passed as the prop
    • Remove usage of BannerNotification component, and replace it with the new simple layout SimpleNotification, you can start it off from assets/js/googlesitekit/notifications/components/layout/NotificationWithSmallSVG.js and remove this part
      <Cell
      size={ 1 }
      className="googlesitekit-publisher-win__small-media"
      >
      <SmallImageSVG />
      </Cell>
      if not already present
    • Transfer the dismiss expires logic to the new SimpleNotification as well
      useMount( async () => {
      if ( dismissExpires > 0 ) {
      await expireDismiss();
      }
      if ( isDismissible ) {
      const { cacheHit } = await getItem( cacheKeyDismissed );
      setIsDismissed( cacheHit );
      }
      if ( showOnce ) {
      // Set the dismissed flag in cache without immediately hiding it.
      await persistDismissal();
      }
      } );
      so it can be used in same way it is in BannerNotification if dismissExpires prop is included
    • Use the new components from assets/js/googlesitekit/notifications/components/common/ where needed, like ActionsCTALinkDismiss.js to include actions prop to the new layout
    • Remove the isLoading logic and progress bar, as it is not needed, the conditions to determine if notification should be shown is handled by checkRequirements property in new notifications API
    • Keep the rest of the logic, determining the variation of the content that is displayed as children (excluding the progress bar)
  • Update assets/js/googlesitekit/notifications/register-defaults.js
    • Register the notification, following the existing patterns already added
    • For checkRequirements transfer the existing check from
      if (
      recoverableModules === undefined ||
      recoverableModulesList.length === 0
      ) {
      return null;
      }
      • Include isLoading check as well, to return early if conditions are not met
    • For priority, bump it by 10 from the last added general notification (not including the error type notification which start from 150)
    • Use NOTIFICATION_AREAS.BANNERS_ABOVE_NAV for areaSlug
    • Use true for isDismissible property. As notification is dismissible by default, and since progress bar is removed, that value shouldn't change.
  • Remove ModuleRecoveryAlert from the assets/js/components/notifications/BannerNotifications.js

Test Coverage

  • Update existing assets/js/components/dashboard-sharing/ModuleRecoveryAlert/index.stories.js to reflect new changes

QA Brief

Note you may need to remove your module-recovery-alert key from the wp_googlesitekitpersistent_dismissed_items key in usermeta for your user to show the error immediately for your user in subsequent tests.

  • Create a secondary, throw away admin account on your test site, reset SK and sign in as this secondary admin.
  • Set up one or multiple modules as this secondary admin (to test the different versions of this notice).
    • Enable dashboard sharing for one or more modules.
  • Login as your existing admin and delete the admin who set up and configured the SK and it's modules.
  • Refresh the dashboard and you will see the Module Recovery Alert banner (it could show after dismissing banners such as auto update).
    • "Recover" button should allow the modules to be recovered if possible, otherwise it should revert to the unrecoverable variant of the notification which can be dismissed with "Remind me later".
    • "Remind me later" CTAs should dismiss the notification for one day (This can be checked by looking at the wp_googlesitekitpersistent_dismissed_items item in the usermeta table and checking the expiry timestamp for the notification module-recovery-alert).

Changelog entry

@zutigrm zutigrm added P2 Low priority Type: Enhancement Improvement of an existing feature Team S Issues for Squad 1 labels Sep 4, 2024
@zutigrm zutigrm assigned zutigrm and unassigned zutigrm Sep 4, 2024
@eugene-manuilov eugene-manuilov self-assigned this Sep 5, 2024
@eugene-manuilov
Copy link
Collaborator

AC ✔️

@eugene-manuilov eugene-manuilov removed their assignment Sep 5, 2024
@zutigrm zutigrm assigned zutigrm and unassigned zutigrm Sep 6, 2024
@eugene-manuilov eugene-manuilov self-assigned this Sep 9, 2024
@eugene-manuilov
Copy link
Collaborator

IB ✔️

@eugene-manuilov eugene-manuilov removed their assignment Sep 9, 2024
@benbowler benbowler self-assigned this Mar 10, 2025
@benbowler benbowler assigned benbowler and unassigned benbowler Mar 10, 2025
@benbowler benbowler assigned aaemnnosttv and jimmymadon and unassigned benbowler Mar 11, 2025
@aaemnnosttv aaemnnosttv removed their assignment Mar 12, 2025
@benbowler benbowler removed their assignment Mar 13, 2025
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

5 participants