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 ReaderRevenueManagerSetupCTABanner to not display simultaneously with other CTAs #9889

Open
13 tasks done
jimmymadon opened this issue Dec 16, 2024 · 2 comments
Open
13 tasks done
Assignees
Labels
P0 High priority Team S Issues for Squad 1 Type: Enhancement Improvement of an existing feature

Comments

@jimmymadon
Copy link
Collaborator

jimmymadon commented Dec 16, 2024

Feature Description

Since the introduction of multiple recent features and modules, their new setup CTAs can all be rendered simultaneously if their necessary feature flags and other conditions are met.

Until the introduction and overhaul of our existing notifications into perhaps a new "notifications centre", we should fix this issue to only show one CTA at a time. This issue focuses on refactoring the ReaderRevenueManagerSetupCTABanner to use the Setup CTAs queue created as part of the SignInWithGoogleSetupCTA component.

dashboard.mp4

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

Acceptance criteria

  • The ReaderRevenueManagerSetupCTABanner should be refactored to render in a separate queue of setup CTAs.
  • This CTA should be rendered with the least priority, over and above other Setup CTAs.
  • After all Setup CTAs are refactored, the order should be:
    • AudienceSegmentationSetupCTAWidget
    • ConsentModeSetupCTAWidget
    • FirstPartyModeSetupBanner
    • SignInWithGoogleSetupCTABanner
    • ReaderRevenueManagerSetupCTABanner

Implementation Brief

  • Register the ReaderRevenueManagerSetupCTABanner as a separate notification following the pattern within assets/js/modules/sign-in-with-google/index.js.

    • Use rrm-setup-notification as the notification ID as this is the ID used in for the GA tracking events.
    • Use groupID: NOTIFICATION_GROUPS.SETUP_CTAS.
    • The priority number here should be 50. Since Setup CTAs have their own queue (determined by their own groupID), having these lower priorities will not affect other notifications with the same or lower priority.
    • For the Component, use the existing ReaderRevenueManagerSetupCTABanner when registering the notification and remove the direct call from <DashboardMainApp>.
    • isDismissible should be set to true.
    • The checkRequirements callback should include all logic from within the notification which prevents it from rendering (i.e. showBanner check that return null.) This logic should be removed from the ReaderRevenueManagerSetupCTABanner component.
  • Refactor the ReaderRevenueManagerSetupCTABanner component to use the a new NotificationWithSVG component following the pattern within assets/js/modules/sign-in-with-google/index.js.

    • Remove logic for tracking GA events for the standard confirm, dismiss and view notification events.
  • Adapt the dismissNotification action to use prompts instead of dismissedItems.

    • Add one more options: dismissRetries.
    • WIthin the action, if dismissRetries is greater than 0, then fetch the getPromptDismissCount and check if this count < the number of dismissRetries. If it is, dispatch the dismissPrompt action instead of the dismissItem action, passing expiresInSeconds as normal.
    • Set the dismissRetries option in the ReaderRevenueManagerSetupCTABanner component and call the dismissNotification action instead of the dismissPrompt action directly.

Test Coverage

  • Cover the dismiss retries logic with new tests.

QA Brief

  • Enable the RRM feature flag and ensure the site being used for testing is using HTTPS. Also, connect Site Kit with Analytics set up.
  • Refresh the dashboard to check the RRM Setup Banner notification. Ensure the GA event for view_notification is correctly fired.
  • Click on the Set up Reader Revenue Manager CTA and ensure the confirm_notification GA event is correctly fired. Also test that the RRM setup is correctly finished and successful.
  • Disconnect the RRM module and deleted the wp_googlesitekitpersistent_dismissed_prompts row for your user in the database.
  • Refresh the dashboard and test the notification again by clicking on "Maybe later". This will expire the notification for 2 weeks. To speed the retesting, go to the database row for wp_googlesitekitpersistent_dismissed_prompts and set the large expires integer value to 1 for the s:22:"rrm-setup-notification";a:2:{s:7:"expires";i:1737116828;s:5:"count";i:1; value.
  • Refresh the dashboard and ensure the dismiss CTA is now Dont show again. Click on this and ensure the notification is dismissed again. All dismissals should fire the correct dismiss_notification GA tracking event.

Changelog entry

@jimmymadon jimmymadon self-assigned this Dec 16, 2024
@jimmymadon jimmymadon added P0 High priority Type: Enhancement Improvement of an existing feature labels Dec 16, 2024
@jimmymadon jimmymadon removed their assignment Dec 16, 2024
@jimmymadon jimmymadon added the Next Up Issues to prioritize for definition label Dec 16, 2024
@eugene-manuilov eugene-manuilov self-assigned this Dec 17, 2024
@eugene-manuilov
Copy link
Collaborator

AC ✔

@eugene-manuilov
Copy link
Collaborator

IB ✔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority Team S Issues for Squad 1 Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

4 participants