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

Implement RRM syncPublicationOnboardingState() action #8796

Closed
16 tasks done
nfmohit opened this issue Jun 4, 2024 · 6 comments
Closed
16 tasks done

Implement RRM syncPublicationOnboardingState() action #8796

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

Comments

@nfmohit
Copy link
Collaborator

nfmohit commented Jun 4, 2024

Feature Description

The syncPublicationOnboardingState() action should be implemented for the Reader Revenue Manager module that syncs the value of the publicationOnboardingState module setting with the publication's onboarding state in the API.


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

Acceptance criteria

  • A syncPublicationOnboardingState() action should be implemented for the modules/reader-revenue-manager data store that syncs the value of the publicationOnboardingState module setting with the publication's onboarding state in the API if a publication is connected to the module.
  • This should also update the publicationOnboardingStateLastSyncedAtMs module setting to the current time, in the form of the number of milliseconds elapsed since the epoch (using Date.now() for example).
  • If the publicationOnboardingState module setting is updated from any other non-empty state to ONBOARDING_COMPLETE, as a side effect, the publication approved overlay notification (being implemented in #8843) should be displayed.
  • The syncPublicationOnboardingState() action should be dispatched whenever the getPublications() selector is resolved.

Implementation Brief

In assets/js/modules/reader-revenue-manager/datastore/publications.js:

  • Define the *syncPublicationOnboardingState() action with the following logic:
    • Load the modules using the getModules() selector.
    • Check if the reader-revenue-manager module is connected using the isModuleConnected selector.
    • If the module is not connected, return early.
    • Load the module settings using the getSettings() selector of the modules/reader-revenue-manager data store.
    • Retrieve the publicationID module setting using the getPublicationID() selector.
    • If publicationID is not set, return early.
    • Use the getPublications() selector to obtain the publications data.
    • Find the publication matching the publicationID module setting in the publications data.
    • Compare the onboardingState of the matched publication with the publicationOnboardingState module setting.
    • If they differ, update the publicationOnboardingState module setting with the new value from the API i.e. the onboardingState of the matched publication using the setSettings() action.
    • Update the publicationOnboardingStateLastSyncedAtMs module setting with the current time using Date.now() using the setSettings() action.
    • Save the updated settings using the saveSettings() action.
    • If the publicationOnboardingState module setting changes to ONBOARDING_COMPLETE from any other non-empty state, set the UI_KEY_SHOW_RRM_PUBLICATION_APPROVED_NOTIFICATION key in the core/ui store to display the <PublicationApprovedOverlayNotification>. (Note: This key may be different during the execution phase of #8843.)
  • Dispatch the syncPublicationOnboardingState() action:
    • Dispatch the syncPublicationOnboardingState action within the *getPublications() resolver.

Test Coverage

  • Add test coverage for the syncPublicationOnboardingState action.

QA Brief

Note 1: Ensure that you have publications setup in publisher center. Refer QAB of this issue on how to setup publications. Also, there should be atleast two publications setup for testing this.

Note 2: Ensure that "Custom Site URL" is NOT set in "Tester Settings" (Site Kit by Google Tester plugin).

  1. Activate the module.
  2. Run the following command in browser console. Remember to replace publicationID value with your publication ID in publisher center. It can be found in publisher center URL.
  const settings = {
      publicationID: "CAow6J6vDA",
      publicationOnboardingState: "ONBOARDING_STATE_UNSPECIFIED",
      publicationOnboardingStateLastSyncedAtMs: 0
    };

    googlesitekit.data.dispatch('modules/reader-revenue-manager').setSettings( settings );
    googlesitekit.data.dispatch('modules/reader-revenue-manager').saveSettings();

refresh the page.

  1. This will set the publication data in settings. Note that current value of onboarding state is ONBOARDING_STATE_UNSPECIFIED and publicationOnboardingStateLastSyncedAtMs is zero. This can be verified using following command in browser console.
googlesitekit.data.select('modules/reader-revenue-manager').getSettings();
  1. Now, run following command in browser console. This will trigger a request to publications endpoint to publisher centre. Let that request complete. For the first time, it will output undefined. Once request gets completed and then running the same command will output the publication data.
 googlesitekit.data.select('modules/reader-revenue-manager').getPublications();
  1. Run the command
 googlesitekit.data.select('modules/reader-revenue-manager').getSettings();

notice that publicationOnboardingState and publicationOnboardingStateLastSyncedAtMs must have changed from the previous values to reflect the current onboarding state of the publication and the timestamp in milliseconds which should be the current time.

Changelog entry

  • Add infrastructure for synchronizing the onboarding state of a publication in the Reader Revenue Manager module.
@nfmohit nfmohit self-assigned this Jun 4, 2024
@nfmohit nfmohit added Type: Enhancement Improvement of an existing feature Team M Issues for Squad 2 Module: RRM Reader Revenue Manager module related issues labels Jun 4, 2024
@nfmohit nfmohit changed the title Implement RRM publication onboarding state sync actions Implement RRM syncPublicationOnboardingState() action Jun 4, 2024
@ivonac4 ivonac4 added the P0 High priority label Jun 5, 2024
@nfmohit nfmohit removed their assignment Jun 8, 2024
@ivonac4 ivonac4 added Module: RRM Reader Revenue Manager module related issues and removed Module: RRM Reader Revenue Manager module related issues labels Jun 11, 2024
@nfmohit nfmohit assigned nfmohit and unassigned nfmohit Jun 14, 2024
@hussain-t hussain-t self-assigned this Jun 26, 2024
@hussain-t hussain-t removed their assignment Jul 11, 2024
@nfmohit nfmohit self-assigned this Jul 11, 2024
@nfmohit
Copy link
Collaborator Author

nfmohit commented Jul 12, 2024

IB ✅

@nfmohit nfmohit removed their assignment Jul 12, 2024
@ankitrox ankitrox self-assigned this Jul 15, 2024
@ankitrox ankitrox removed their assignment Jul 16, 2024
@techanvil techanvil assigned techanvil and ankitrox and unassigned techanvil Jul 17, 2024
@ankitrox ankitrox assigned techanvil and unassigned ankitrox Jul 19, 2024
@techanvil techanvil removed their assignment Jul 19, 2024
@wpdarren wpdarren self-assigned this Jul 22, 2024
@wpdarren
Copy link
Collaborator

QA Update: ⚠️

@ankitrox I have a few observations that I would like to run by you.

  1. When I run googlesitekit.data.select('modules/reader-revenue-manager').getPublications(); I see undefined but when I run it a second time, I do see a response.
  • Should it take two runs of the command for the API to provide us with the publication data?
  • I don't see any mention of the publication name or ID, when the command is run a second time. The expected behaviour of this command isn't included in the QAB, so I wasn't sure. Is this correct? See screenshot below.

image

  1. The QAB says this after running googlesitekit.data.select('modules/reader-revenue-manager').getSettings();

notice that publicationOnboardingState and publicationOnboardingStateLastSyncedAtMs must have changed from the previous values to reflect the current onboarding state of the publication and the timestamp in milliseconds which should be the current time.

The ouput is the same as when I ran the same command previously.

image

Please could you clarify the two points above. Thank you!

@ankitrox
Copy link
Collaborator

Thank you @wpdarren for testing this and raising your queries. Please find the answers below to the questions.

When I run googlesitekit.data.select('modules/reader-revenue-manager').getPublications(); I see undefined but when I run it a second time, I do see a response.
Should it take two runs of the command for the API to provide us with the publication data?
I don't see any mention of the publication name or ID, when the command is run a second time. The expected behaviour of this command isn't included in the QAB, so I wasn't sure. Is this correct? See screenshot below.

I have amended the description in QAB regarding this command. When the command is run for the first time, it will output undefined as the request is yet to be resolved. Once the request is successful and after that we run the same command, it will output the publication data.

The ouput is the same as when I ran the same command previously.

I see that there are no publications fetched from the API. So you will need to create couple of publications as mentioned in QAB. Also, note that when running the first command given in QAB instructions, you must replace the publication ID with the one you created in publisher centre.

@ankitrox ankitrox removed their assignment Jul 23, 2024
@wpdarren
Copy link
Collaborator

wpdarren commented Jul 23, 2024

QA Update: ⚠️

@ankitrox You can see on the screenshot all of he commands that I have run.

I have two publications. Used the correct ID.

  1. No publication data appears when I run googlesitekit.data.select('modules/reader-revenue-manager').getPublications();
  2. When I run googlesitekit.data.select('modules/reader-revenue-manager').getSettings();
    in step 5, the data appears the same as step 3.

Could you confirm if you see the same on your side?

image

Just to add this is a screenshot of the publication centre.

image

The URLs match the site I am testing with.

image

@hussain-t
Copy link
Collaborator

@wpdarren, you see results for getSettings because you have set the settings using the setSettings action and saved it.

@wpdarren
Copy link
Collaborator

QA Update: ✅

Verified:

  • Running the commands provided the data in the QAB including the publications.

image

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 P0 High priority Team M Issues for Squad 2 Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

7 participants