Implement RRM `getPublications()` selector
Feature Description
The getPublications() selector should be implemented for the Reader Revenue Manager module that should interact with the GET:publications REST endpoint and return a list of publications.
- See relevant section in the design doc.
- This selector will be quite similar (if not the same) to the TwG getPublications() selector.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- A
getPublications()selector should be added to the Reader Revenue Manager module data store. - This selector should fetch the
GET:publicationsREST endpoint (being implemented in #8791) to return the list of available publications for the site.
Implementation Brief
- [x] Create
assets/js/module/reader-revenue-manager/datastore/publications.js. This would be a data store to fetch the publications.- [x] Add this store in
Data.combineStoresinassets/js/module/reader-revenue-manager/index.jsadded in #8786 - [x] Use
createFetchStoreto create the publication store (getPublicationStore). This should useGET:publicationsendpoint (being implemented in #8791) to return the list of available publications for the site. - [x]
baseNamefor the storegetPublications. - [x] Base initial state (
baseInitialState) should have thepublicationsproperty which would beundefinedinitially and will be set to the list of publications. It should be array of publication objects. - [x] In
reducerCallbackinsidecreateFetchStore, set thepublicationsproperty instate. - [x] Create a
baseResolversobject.- [x] Add
*getPublicationsgenerator inside it. It should callgetPublicationsselector. If it is undefined, it will yield the result by callinggetPublicationStore.actions.fetchGetPublications
- [x] Add
- [x] Create
baseSelectorsobject.- [x] Add
getPublicationswhich should returnstate.publications.
- [x] Add
- [x] Add this store in
- [x] Use
Data.combineStoresand pass the following- [x]
baseInitialStateasinitialState - [x]
baseActions,reduceras empty object. - [x] Set
resolversand passbaseResolvers. - [x] Set
selectorsand passbaseSelectors.
- [x]
Test Coverage
- [x] Add tests for
publicationsdata store. We can usefetchMockto mock the API calls.
QA Brief
Changelog entry
IB ✅
Hi @ankitrox . About the IB, I've removed the requirement of only including the publicationID and displayName properties in the publication object. Ideally, it should include all properties that the API returns. Sorry for not noticing that earlier. I hope that's okay. Thanks!
HI @nfmohit ,
That completely makes sense. In fact I realised this yesterday when I was working on #8837
Thank you for making that change.
I've created the PR: https://github.com/google/site-kit-wp/pull/8950 for this issue, but keeping this issue in Execution because its dependencies #8786 #8791 are yet to be merged.
Once merged, the base branch for this PR needs to be changed to develop and test everything and move this to CR.
QA Update: ⚠️
I've had an initial test and here are the results and queries:
-
I've created the site and triggered the script in browser console and it rightly pulled nothing as there was no Publications. The request was made with 200 status ✅
-
After the first script execution, I went ahead to create a publication for the website and triggered the script again. However, this round, there was no request made to the publications endpoint. And also, the publication was not retrieved. I would assume we should have triggered and pulled the new publication. Could you review whether any fixes is required here? ⚠️
Publication is not retrieved:https://github.com/user-attachments/assets/02aeff5a-3b3b-4a03-8448-b133e9c8ffba
-
One thing to flag is after setting up the RRM module, at the module settings, it shows: "Complete setup for Reader Revenue Management" Question is: is that expected currently due to the implementation is not complete yet or it should have been successful? ⚠️
- After the first script execution, I went ahead to create a publication for the website and triggered the script again. However, this round, there was no request made to the publications endpoint. And also, the publication was not retrieved. I would assume we should have triggered and pulled the new publication. Could you review whether any fixes is required here? ⚠️
getPublications is a selector which queries the API only when the data is not available. When we trigger the script for the first time, data is gathered from the API and is available for subsequent runs, so it will not call the API further.
We have a separate ticket where maybeSyncPublicationOnboardingState selector will periodically sync and update the publications in the database.
One thing to flag is after setting up the RRM module, at the module settings, it shows: "Complete setup for Reader Revenue Management" Question is: is that expected currently due to the implementation is not complete yet or it should have been successful?
Yes, this is expected! The reason for that is we do not have the #8800 ready yet, but you can refer to #8796 IB where step 2 would help to resolve this issue.
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();
QA Update ✅
-
I've created the site and triggered the script in browser console and it rightly pulled nothing as there was no Publications. The request was made with 200 status ✅
-
I've also created another side and created a publication for it. After running the script in browser console, it rightly pulled the publication with the proper endpoint with 200 status. ✅
Moving ticket to Approval.