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 getServiceURL() selector #8848

Closed
6 tasks done
nfmohit opened this issue Jun 8, 2024 · 7 comments
Closed
6 tasks done

Implement RRM getServiceURL() selector #8848

nfmohit opened this issue Jun 8, 2024 · 7 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 8, 2024

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

  • Create a new store definition in assets/js/modules/reader-revenue-manager/datastore/service.js.
  • The store definition only needs to provide the selectors object, which in turn contains the getServiceURL() selector.
  • getServiceURL() should accept an optional publicationId parameter and implement the logic as defined in the AC to determine the link.
  • Use getAccountChooserURL() to wrap the link with the Google account chooser URL.
  • See the Analytics getServiceURL() implementation for some pointers.
  • Merge the store with the main RRM datastore introduced via Add JS entry point for RRM #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( { publicationID: '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=your-email@get10up.com

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.

  2. Again, in the browser console, run the following script:

googlesitekit.data.select( 'modules/reader-revenue-manager' ).getServiceURL()
  1. The link that will receive should take you to the publisher center homepage, i.e. https://publishercenter.google.com/

Changelog entry

  • Add Reader Revenue Manager data store functionality to get service URL.
@nfmohit nfmohit added P0 High priority Type: Enhancement Improvement of an existing feature Team M Issues for Squad 2 Module: RRM Reader Revenue Manager module related issues labels 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
@ivonac4 ivonac4 added the Next Up Issues to prioritize for definition label Jun 17, 2024
@nfmohit nfmohit assigned nfmohit and unassigned nfmohit Jun 18, 2024
@techanvil techanvil assigned techanvil and unassigned techanvil Jun 19, 2024
@nfmohit nfmohit self-assigned this Jun 19, 2024
@nfmohit
Copy link
Collaborator Author

nfmohit commented Jun 20, 2024

  • 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 nfmohit assigned techanvil and unassigned nfmohit Jun 20, 2024
@techanvil
Copy link
Collaborator

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

@techanvil techanvil assigned nfmohit and unassigned techanvil Jun 20, 2024
@nfmohit
Copy link
Collaborator Author

nfmohit commented Jun 20, 2024

Brilliant, thanks @techanvil! IB ✅

@nfmohit nfmohit removed their assignment Jun 20, 2024
@ivonac4 ivonac4 added Sp Wk 2 Issues to be completed in the second week of the assigned sprint and removed Next Up Issues to prioritize for definition labels Jun 21, 2024
@ankitrox ankitrox self-assigned this Jul 2, 2024
@ankitrox
Copy link
Collaborator

ankitrox commented Jul 3, 2024

I've created the PR: #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 ankitrox removed their assignment Jul 4, 2024
@nfmohit nfmohit assigned nfmohit and ankitrox and unassigned nfmohit Jul 4, 2024
@ankitrox ankitrox assigned nfmohit and unassigned ankitrox Jul 8, 2024
@nfmohit nfmohit assigned ankitrox and unassigned nfmohit Jul 9, 2024
@ankitrox ankitrox assigned nfmohit and unassigned ankitrox Jul 10, 2024
@nfmohit nfmohit removed their assignment Jul 10, 2024
@eclarke1 eclarke1 removed the Sp Wk 2 Issues to be completed in the second week of the assigned sprint label Jul 11, 2024
@kelvinballoo
Copy link
Collaborator

QA Update ⚠️

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

@ankitrox
Copy link
Collaborator

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 ankitrox removed their assignment Jul 18, 2024
@kelvinballoo
Copy link
Collaborator

QA Update ✅

Thanks for the update @ankitrox

Moving this ticket to approval.

Screenshot 2024-07-15 at 19 58 55

@kelvinballoo kelvinballoo removed their assignment Jul 18, 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 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