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

Move on-demand onboarding state synchronization to server side #9363

Closed
3 tasks done
nfmohit opened this issue Sep 16, 2024 · 4 comments
Closed
3 tasks done

Move on-demand onboarding state synchronization to server side #9363

nfmohit opened this issue Sep 16, 2024 · 4 comments
Labels
Module: RRM Reader Revenue Manager module related issues P1 Medium priority Team M Issues for Squad 2 Type: Enhancement Improvement of an existing feature

Comments

@nfmohit
Copy link
Collaborator

nfmohit commented Sep 16, 2024

Feature Description

As pointed out here, we should look into moving the on-demand synchronization of publication onboarding state from the client-side to the server for a better UX, like we did for the periodic synchronization.

Ideally, we should add a POST:sync-publication-onboarding-state endpoint so we handle the sync on the backend, and then call fetchGetSettings() after this to update the saved settings state.


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

Acceptance criteria

  • On the RRM setup and settings edit screens, having clicked on the CTA to visit the Publisher Center from one of the notifications that indicate the publication's onboarding state, and having then returned to the Site Kit tab after 15 seconds have elapsed (see Remove RRM onboarding state notice when state changes #9262), the onboarding state and corresponding CTA should be updated without showing the respective screen in a loading state.
  • Additionally, for the settings edit case, the Save button should not momentarily change to Confirm changes during the onboarding state update.

Implementation Brief

Server side

  • Create a POST:sync-publication-onboarding-state endpoint as a new datapoint in the Reader_Revenue_Manager class:
    • Require $publicationID and $publicationOnboardingState parameters to be passed in $data. We need to pass these in, rather than simply use the current values in settings, because the client may be requesting a sync for an unsaved setting in the case where a new publication has been selected from the dropdown.
    • Retrieve the list of $publications via a call to $this->get_data( 'publications' ).
    • Find the $publication in the list where $publication->getPublicationId() matches $publicationID.
    • If $publication is not found, return a WP_Error with a message that the publication was not found.
    • Retrieve $newOnboardingState from $publication->getOnboardingState().
    • If $newOnboardingState is already $publicationOnboardingState, return an empty object, indicating that the state has not changed.
    • Retrieve the current module $settings from $this->get_settings()->get().
    • If the $publicationID parameter matches $settings['publicationID'], indicating that we are syncing the currently saved publication:
      • Merge publicationOnboardingState => $newOnboardingState into $settings.
      • Return an object with the following values:
          {
            publicationOnboardingState: $newOnboardingState
            isSavedSetting: true
          }
        
    • Otherwise, return an object with the following values:
        {
          publicationOnboardingState: $newOnboardingState,
        }
      
    • The endpoint does not need to be shareable.

Client side

  • In the publications datastore:
    • Create a new fetchGetSyncPublicationOnboardingStateStore store with baseName: getSyncPublicationOnboardingState via createFetchStore() that calls the new POST:sync-publication-onboarding-state endpoint.
      • If publicationOnboardingState is set in the response:
        • The reducer should update settings.publicationOnboardingState in state.
        • It should also should update savedSettings.publicationOnboardingState if isSavedSetting is true.
    • Update the *syncPublicationOnboardingState() action:
  • Update the syncPublication() callback in the RRMSetupSuccessSubtleNotification component:
    • Prior to the call to syncPublicationOnboardingState(), retrieve currentOnboardingState via the getPublicationOnboardingState() selector.
    • Await syncPublicationOnboardingState() to retrieve the result, which is of the shape { response, error }.
    • If response?.publicationOnboardingState is truthy, execute the following block of code which can be moved from syncPublicationOnboardingState(). The value of response.publicationOnboardingState should be used for newOnboardingState:
      // If the onboarding state changes to complete from a non-empty state, set
      // the key in CORE_UI to trigger the notification.
      if (
      !! currentOnboardingState &&
      newOnboardingState !== currentOnboardingState &&
      newOnboardingState ===
      PUBLICATION_ONBOARDING_STATES.ONBOARDING_COMPLETE
      ) {
      registry
      .dispatch( CORE_UI )
      .setValue(
      UI_KEY_READER_REVENUE_MANAGER_SHOW_PUBLICATION_APPROVED_NOTIFICATION,
      true
      );
      }

Test Coverage

  • Add PHPUnit test coverage for the server side changes.
  • Update the JS test coverage for the syncPublicationOnboardingState() action and RRMSetupSuccessSubtleNotification component.

QA Brief

On Site Kit Dashboard

  1. Make sure you have at least one publication setup in RRM publication center with the status ONBOARDING_COMPLETE.

  2. In tester plugin, RRM > Force Reader Revenue Manager publication onboarding state, set it to ONBOARDING_ACTION_REQUIRED.

  3. Enable RRM feature and module. Make sure publications are setup for the site and they are available in settings screen.

  4. Select the publication with complete state in publication center and complete the setup. This will setup the publication state as ONBOARDING_ACTION_REQUIRED in database.

  5. Visit the Site Kit dashboard. You will be able to see the publication onboarding state notification with the Complete publication setup CTA.

  6. Click on CTA which will open the publisher center. Stay on that page (away from Site Kit dashboard tab) for more than 15 seconds.

  7. Return to the tab, ensure that notification has been changed to setup success notification and the Publication Approved Overlay notification is also visible.

On Settings Screen

  1. Make sure you have at least one publication setup in RRM publication center with the status ONBOARDING_COMPLETE.

  2. In tester plugin, RRM > Force Reader Revenue Manager publication onboarding state, set it to ONBOARDING_ACTION_REQUIRED.

  3. Enable RRM feature and module. Make sure publications are setup for the site and they are available in settings screen.

  4. Select the publication with complete state in publication center.

  5. You will be able to see the publication onboarding state notification with the Complete publication setup CTA on settings screen.

  6. Click on CTA which will open the publisher center. Stay on that page (away from Site Kit dashboard tab) for more than 15 seconds.

  7. Return to the tab on settings edit screen, the publication onboarding state notification should disappear.

Changelog entry

  • Move on-demand onboarding state synchronization to the server side, avoiding the need to reload the full list of publications.
@nfmohit nfmohit added P1 Medium priority Type: Enhancement Improvement of an existing feature Team M Issues for Squad 2 Module: RRM Reader Revenue Manager module related issues labels Sep 16, 2024
@techanvil techanvil assigned techanvil and unassigned techanvil Sep 17, 2024
@nfmohit
Copy link
Collaborator Author

nfmohit commented Sep 22, 2024

AC ✅

@ankitrox ankitrox assigned ankitrox and unassigned ankitrox Sep 25, 2024
@ivonac4 ivonac4 added the Next Up Issues to prioritize for definition label Oct 31, 2024
@techanvil techanvil assigned techanvil and unassigned techanvil Oct 31, 2024
@nfmohit nfmohit self-assigned this Nov 6, 2024
@nfmohit
Copy link
Collaborator Author

nfmohit commented Nov 8, 2024

IB ✅, thanks @techanvil!

Just a note that I made an edit to correct the selector name from getOnboardingState() to getPublicationOnboardingState(). @techanvil Please let me know if you think that's not correct. Thank you!

@nfmohit nfmohit removed their assignment Nov 8, 2024
@techanvil
Copy link
Collaborator

That's spot on @nfmohit, thanks for fixing the typo!

@ankitrox ankitrox self-assigned this Nov 11, 2024
@ankitrox ankitrox removed their assignment Nov 13, 2024
@techanvil techanvil assigned techanvil and ankitrox and unassigned techanvil Nov 13, 2024
@ankitrox ankitrox assigned techanvil and unassigned ankitrox Nov 14, 2024
@ivonac4 ivonac4 removed the Next Up Issues to prioritize for definition label Nov 14, 2024
@techanvil techanvil removed their assignment Nov 14, 2024
@ankitrox ankitrox self-assigned this Nov 15, 2024
@ankitrox ankitrox assigned techanvil and unassigned ankitrox Nov 15, 2024
@techanvil techanvil removed their assignment Nov 15, 2024
@mohitwp mohitwp self-assigned this Nov 25, 2024
@mohitwp
Copy link
Collaborator

mohitwp commented Nov 25, 2024

QA Update ✅

  • Tested on dev environment.
  • Verified that on SK dashboard ensure that notification has been changed to setup success notification and the Publication Approved Overlay notification is also visible.
  • Verified that on settings edit screen, the publication onboarding state notification gets disappear.
  • Verified that on settings edit page s, the Save button not momentarily change to Confirm changes during the onboarding state update.

Settings screen

Recording.1623.mp4

Main dashboard

Recording.1624.mp4

Edit case

Recording.1625.mp4

@mohitwp mohitwp removed their assignment Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: RRM Reader Revenue Manager module related issues P1 Medium priority Team M Issues for Squad 2 Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

6 participants