site-kit-wp icon indicating copy to clipboard operation
site-kit-wp copied to clipboard

Implement RRM `getServiceURL()` selector

Open nfmohit opened this issue 1 year ago • 4 comments

Feature Description

The getServiceURL() selector should be implemented for the Reader Revenue Manager module that should return a prepared link to the Reader Revenue Manager platform that meets this criteria.


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

Acceptance criteria

  • A getServiceURL() selector should be added to the Reader Revenue Manager module data store.
  • This selector should return a link to the Reader Revenue Manager platform based on the following structure:
    • Link to the Reader Revenue Manager platform, i.e. publisher center should look like: https://publishercenter.google.com/.
    • Link to a specific publication in the Reader Revenue Manager platform should look like: https://publishercenter.google.com/reader-revenue-manager?publication=PUBLICATION_ID.
    • All links to the Reader Revenue Manager platform should be wrapped with the Google account chooser URL.
    • All links should include a query parameter to indicate Site Kit as a source, i.e. utm_source=sitekit.

Implementation Brief

  • [x] Create a new store definition in assets/js/modules/reader-revenue-manager/datastore/service.js.
  • [x] The store definition only needs to provide the selectors object, which in turn contains the getServiceURL() selector.
  • [x] getServiceURL() should accept an optional publicationId parameter and implement the logic as defined in the AC to determine the link.
  • [x] Use getAccountChooserURL() to wrap the link with the Google account chooser URL.
  • [x] See the Analytics getServiceURL() implementation for some pointers.
  • [x] Merge the store with the main RRM datastore introduced via #8786.

Test Coverage

  • Add JS test coverage for the new selector.

QA Brief

  1. Activate the Reader Revenue Manager module.
  2. Run following command in browser console.
    googlesitekit.data.select('modules/reader-revenue-manager').getServiceURL('ABCDEFGH');

You must get the URL like following

https://accounts.google.com/accountchooser?continue=https://publishercenter.google.com/reader-revenue-manager?publication=ABCDEFGH&utm_source=sitekit&[email protected]

Notice that email will be replaced with your account email.

  1. Clicking on the above link should take you to the publisher center, but page will open in publisher center only if the publication ID is valid (in this case ABCDEFGH which is invalid). You can create a publication in https://publishercenter.google.com/ to get the publication ID.

Changelog entry

nfmohit avatar Jun 08 '24 13:06 nfmohit

  • Create a new store definition in assets/js/modules/analytics-4/datastore/service.js.

Hi @techanvil, just to confirm, shouldn't this be assets/js/modules/reader-revenue-manager/datastore/service.js instead?

nfmohit avatar Jun 20 '24 13:06 nfmohit

Thanks @nfmohit, apologies for the copy/paste fail! This is now fixed, cheers.

techanvil avatar Jun 20 '24 13:06 techanvil

Brilliant, thanks @techanvil! IB ✅

nfmohit avatar Jun 20 '24 13:06 nfmohit

I've created the PR: https://github.com/google/site-kit-wp/pull/8953 for this issue, but keeping this issue in Execution because its dependencies #8786 is 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.

ankitrox avatar Jul 03 '24 13:07 ankitrox

QA Update ⚠️

I did some tests and results and queries are as follows:

  • Executed with the following scenarios:

    • Invalid publication ID - Link will go the the publisher page but would not open. It's expected ✅
    • Valid publication ID - Link will go to the correct Publisher publication page. ✅
    • Only getserviceURL() - While the link goes to the Publisher central page initially, it goes directly to the publication afterwards. ⚠️ Refer to the attached video : https://github.com/user-attachments/assets/0c9a4552-0b3b-4216-aa49-1eddf24219e1
  • It's worth noting that the URLs have a lot of symbols with %, etc.. In the AC, there is no mention of those symbols and add ons. The link works but are we expecting these? ⚠️

    e.g. https://accounts.google.com/accountchooser?continue=https%3A%2F%2Fpublishercenter.google.com%3Futm_source%3Dsitekit%26publication%3DCAownMayDA&Email=kelvin.balloo%40get10up.com

    It's not @get10up.com but %40get10up.com

kelvinballoo avatar Jul 15 '24 16:07 kelvinballoo

Hi @kelvinballoo ,

Thank you for testing this issue.

Only getserviceURL() - While the link goes to the Publisher central page initially, it goes directly to the publication afterwards. ⚠️ Refer to the attached video :

the publication ID query parameter is added by publisher centre automatically and we do not have the control over it. As you demonstrated in your video, even if we simply visit https://publishercenter.google.com/, that query parameter is getting appended in the URL.

It's worth noting that the URLs have a lot of symbols with %, etc.. In the AC, there is no mention of those symbols and add ons. The link works but are we expecting these? ⚠️

This is fine as the getServiceURL selector will encode the URL so that all the parameters and values can be passed safely.

ankitrox avatar Jul 18 '24 14:07 ankitrox

QA Update ✅

Thanks for the update @ankitrox

  • Executed with the following scenarios:
    • Invalid publication ID - Link will go the the publisher page but would not open. It's expected ✅

    • Valid publication ID - Link will go to the correct Publisher publication page. ✅

    • Only getserviceURL() - Link goes to the Publisher central page initially and goes directly to the publication afterwards but this is fine as that's the way the RRM works. ✅ Video for reference: https://github.com/user-attachments/assets/0c9a4552-0b3b-4216-aa49-1eddf24219e1

    • Link to the RRM platform includes the following: ✅

      • https://publishercenter.google.com/.
      • Publication portion: https://publishercenter.google.com/reader-revenue-manager?publication=PUBLICATION_ID.
      • Links wrapped with the Google account chooser URL.
      • Query parameter includes Site Kit as a source,utm_source=sitekit.
      • URL tested: https://accounts.google.com/accountchooser?continue=https%3A%2F%2Fpublishercenter.google.com%3Futm_source%3Dsitekit%26publication%3DCAownMayDA&Email=kelvin.balloo%40get10up.com

      Symbols with the URL encoding is fine as clarified.

Moving this ticket to approval.

Screenshot 2024-07-15 at 19 58 55

kelvinballoo avatar Jul 18 '24 15:07 kelvinballoo