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 findMatchedPublication() action #8795

Closed
nfmohit opened this issue Jun 4, 2024 · 11 comments
Closed

Implement RRM findMatchedPublication() action #8795

nfmohit opened this issue Jun 4, 2024 · 11 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 findMatchedPublication() action should be implemented for the Reader Revenue Manager module that finds a matching publication from the list of all publications based on the following criteria:

  • If there is no publication in the list, it will return null early as there is no publication to select.
  • Otherwise, if there is only one publication in the list, it will return that publication.
  • Otherwise, if there is more than one publication in the list, it will return one as follows:
    • It will prefer a publication that has completed setup (i.e. its onboardingState property is ONBOARDING_COMPLETE) and return that publication.
    • Otherwise, it will just return the first publication in the list.

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

Acceptance criteria

  • The modules/reader-revenue-manager data store should have a findMatchedPublication() action added, that finds a matching publication from the list of available publications based on the following criteria:
    • If there is no publication in the list, it should return null early as there is no publication to select.
    • Otherwise, if there is only one publication in the list, it should return that publication.
    • Otherwise, if there is more than one publication in the list, it should return one as follows:
      • It should prefer a publication that has completed setup (i.e. its onboardingState property is ONBOARDING_COMPLETE) and return that publication.
      • Otherwise, it should just return the first publication in the list.

Implementation Brief

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

  • Add a new action, findMatchedPublication() that finds a matching publication with the following logic:
    • Get the list of all publications from the getPublications selector.
    • Return undefined if the publications have not been loaded yet.
    • If there is no publication in the list, return null.
    • If there is only one publication in the list, return that publication.
    • If there is more than one publication in the list, return one as follows:
      • Find a publication that has completed setup (i.e., publication.onboardingState property is set to ONBOARDING_COMPLETE) and return that publication.
      • Otherwise, return the first publication in the list.

Test Coverage

  • Add test coverage for the new action findMatchedPublication() in assets/js/module/reader-revenue-manager/datastore/publications.test.js.

QA Brief

  • Follow the QAB from #8791 to create new publications.
  • Enable "Add code to your site" within the survey setup for the publications created above for the publications to be fetched in Site Kit. Please refer to the "Note" in this comment.
    Screenshot 2024-07-23 at 4 52 47 PM
  • Clear the browser session storage and run the following command in the browser console:
await googlesitekit.data.dispatch('modules/reader-revenue-manager').findMatchedPublication()
  • Verify that the action returns the expected publication based on the AC above.
  • For any reason the publications are not fetched, we can simulate by providing the publications mock data.
  • To verify the action returns null when there are no publications, run the following command in the browser console:
googlesitekit.data.dispatch('modules/reader-revenue-manager').receiveGetPublications( [] )
await googlesitekit.data.dispatch('modules/reader-revenue-manager').findMatchedPublication()
  • To verify the action returns the only publication when there is only one publication, run the following command in the browser console:
let publications = [
    {
        "publicationId": "IJKLMNOP",
        "publicationPredicates": {
            "businessPredicates": {
                "supportsSiteKit": true,
                "canSell": true
            }
        },
        "verifiedDomains": "example.com",
        "displayName": "Test Publication 1",
        "onboardingState": "ONBOARDING_COMPLETE"
    }
]
googlesitekit.data.dispatch('modules/reader-revenue-manager').receiveGetPublications( publications )
await googlesitekit.data.dispatch('modules/reader-revenue-manager').findMatchedPublication()
  • To verify the action returns the publication that has completed setup when there are multiple publications, run the following command in the browser console:
let publications = [
	{
		"publicationId": "PQRSTUV",
		"publicationPredicates": {
			"businessPredicates": {
				"supportsSiteKit": true,
				"canSell": false
			}
		},
		"verifiedDomains": "example.com",
		"displayName": "Test Publication 2",
		"onboardingState": "PENDING_VERIFICATION"
	},
	{
		"publicationId": "IJKLMNOP",
		"publicationPredicates": {
			"businessPredicates": {
				"supportsSiteKit": true,
				"canSell": true
			}
		},
		"verifiedDomains": "example.com",
		"displayName": "Test Publication 3",
		"onboardingState": "ONBOARDING_ACTION_REQUIRED"
	},
	{
		"publicationId": "QRSTUVWX",
		"publicationPredicates": {
			"businessPredicates": {
				"supportsSiteKit": false,
				"canSell": true
			}
		},
		"verifiedDomains": "example.com",
		"displayName": "Test Publication 4",
		"onboardingState": "ONBOARDING_COMPLETE"
	}
]
googlesitekit.data.dispatch('modules/reader-revenue-manager').receiveGetPublications( publications )
await googlesitekit.data.dispatch('modules/reader-revenue-manager').findMatchedPublication()
  • To verify the action returns the first publication when there are multiple publications and none of them have completed setup, run the following command in the browser console:
let publications = [
    {
        "publicationId": "PQRSTUV",
        "publicationPredicates": {
            "businessPredicates": {
                "supportsSiteKit": true,
                "canSell": false
            }
        },
        "verifiedDomains": "example.com",
        "displayName": "Test Publication 2",
        "onboardingState": "PENDING_VERIFICATION"
    },
    {
        "publicationId": "IJKLMNOP",
        "publicationPredicates": {
            "businessPredicates": {
                "supportsSiteKit": true,
                "canSell": true
            }
        },
        "verifiedDomains": "example.com",
        "displayName": "Test Publication 3",
        "onboardingState": "ONBOARDING_ACTION_REQUIRED"
    },
    {
        "publicationId": "QRSTUVWX",
        "publicationPredicates": {
            "businessPredicates": {
                "supportsSiteKit": false,
                "canSell": true
            }
        },
        "verifiedDomains": "example.com",
        "displayName": "Test Publication 4",
        "onboardingState": "PENDING_VERIFICATION"
    }
]
googlesitekit.data.dispatch('modules/reader-revenue-manager').receiveGetPublications( publications )
await googlesitekit.data.dispatch('modules/reader-revenue-manager').findMatchedPublication()

Changelog entry

  • Add Reader Revenue Manager data store functionality to find a matching publication.
@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
@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
@nfmohit nfmohit assigned nfmohit and unassigned nfmohit Jun 18, 2024
@hussain-t hussain-t self-assigned this Jun 26, 2024
@ivonac4 ivonac4 added the Next Up Issues to prioritize for definition label Jun 27, 2024
@hussain-t hussain-t assigned nfmohit and unassigned hussain-t Jul 3, 2024
@nfmohit
Copy link
Collaborator Author

nfmohit commented Jul 4, 2024

IB LGTM 👍 ✅

@nfmohit nfmohit removed their assignment Jul 4, 2024
@eclarke1 eclarke1 removed the Next Up Issues to prioritize for definition label Jul 8, 2024
@hussain-t hussain-t self-assigned this Jul 14, 2024
@hussain-t hussain-t removed their assignment Jul 15, 2024
@nfmohit nfmohit assigned nfmohit and hussain-t and unassigned nfmohit Jul 16, 2024
@hussain-t hussain-t assigned nfmohit and unassigned hussain-t Jul 16, 2024
nfmohit added a commit that referenced this issue Jul 16, 2024
@nfmohit nfmohit removed their assignment Jul 16, 2024
@kelvinballoo
Copy link
Collaborator

QA Update ⚠️

I've tested this and here are the results:

  • I was able to test for 0 publications and it would return null.

    0 Publication
  • I tested with 1 Publication and it would return accordingly.

    1 Publication
  • The problem I am having is with multiple publications and how to define 'ONBOARDING STATE = COMPLETED' in the RRM platform. ⚠️

    • I set up 3 publications

      • One without checking the 'Add to site' checkbox in the RRM platform
      • One adding all the checkboxes in the RRM platform but without proceeding it there
      • One with all checkboxes in RRM platform and proceeded it
        • I am assumed this would set the onboarding state to 'Completed' but in vain
    • When I ran the script, the publication 3 comes up but with onboardingstate as 'ONBOARDING ACTION REQUIRED'

      Screenshot 2024-07-18 at 15 25 09
    • ⚠️ There are to main things I want to unpick from here:

      • How do I get an onboarding action completed in the RRM platform?
      • Per the AC, if there are no publication with complete onboarding, the first publication on the list would return.
        My question is, how do I validated which one is the first publication? How do I verify the API list?

@hussain-t
Copy link
Collaborator

Hi @kelvinballoo,

As we discussed during our call, the Google service API is returning only a single publication despite multiple publications. This may be due to a configuration issue in how the publication was created within the RRM console or the API needing more time to sync the data. However, this is outside the scope of this issue and is not an issue from Site Kit.

You can mock the data directly in your browser console to test the behavior with multiple publications. You can copy the mock JSON data from this file and use it with the receiveGetPublications action. This will allow the data to be available for testing in the getPublications selector.

Run the following command in the browser console:

googlesitekit.data.dispatch( 'modules/reader-revenue-manager' ).receiveGetPublications(mockJsonData);

Replace mockJsonData with the actual mock data you copied. This should help you simulate and test multiple publications.

cc: @wpdarren @nfmohit

@wpdarren
Copy link
Collaborator

@hussain-t thanks for the explanation, I just want to highlight one point.

the Google service API is returning only a single publication despite multiple publications. This may be due to a configuration issue in how the publication was created within the RRM console or the API needing more time to sync the data. However, this is outside the scope of this issue and is not an issue from Site Kit.

While I realise this is an API issue, shouldn't we be flagging this with someone? The AC says we have to test with multiple publications, and that's hard to do if the API is not functioning as expected.

That aside, I am going to look at this ticket and sync with @kelvinballoo when I have looked at it.

@wpdarren wpdarren self-assigned this Jul 18, 2024
@hussain-t
Copy link
Collaborator

Thanks, @wpdarren.

While I realise this is an API issue, shouldn't we be flagging this with someone? The AC says we have to test with multiple publications, and that's hard to do if the API is not functioning as expected.

As mentioned in my previous comment, I'm not 100% sure if it's an API issue or a configuration issue. Please try creating a few more publications and see if they pull.

@wpdarren
Copy link
Collaborator

QA Update: ⚠️

@hussain-t I am wondering if the issue we're having here is linked to the problem I am seeing on #8796 where when googlesitekit.data.select('modules/reader-revenue-manager').getPublications(); is run, no publications are appearing. I have run the code in the QAB on this ticket, await googlesitekit.data.dispatch('modules/reader-revenue-manager').findMatchedPublication() and the response is still null. I have two publications created.

image
image

@hussain-t
Copy link
Collaborator

Hi @wpdarren, I see the results when executing the browser console action.

Screenshot 2024-07-23 at 4 16 07 PM

As explained here. The publications weren't being fetched from the API call. The action returns null because the publications API's result is an empty array.

@hussain-t
Copy link
Collaborator

@wpdarren, I've updated the QAB with clearer instructions. Please let me know if that's easy to follow.

@hussain-t hussain-t removed their assignment Jul 23, 2024
@wpdarren
Copy link
Collaborator

QA Update: ⚠️

@hussain-t thank you for the additional QAB instructions!

When I run await googlesitekit.data.dispatch('modules/reader-revenue-manager').findMatchedPublication() if no publication is created then null appears ✅ and if I have just one publication then that appears ✅

My publication has 3 set up, and I have set up RRM / Survey with code on site for each one.

The issue is when I have 2 or more publications, only one publication appears.

image

I run the mock code in the QAB for more than one publication and only 1 publication appears.

I am assuming all 4 should have appeared?

image

@hussain-t
Copy link
Collaborator

Hi @wpdarren, it seems you misunderstood the findMatchedPublication action with the getPublications selector.

  • getPublications - returns the list of all the publications
  • findMatchedPublication - find the matched ONLY ONE publication based on the criteria.

I hope that clears your concerns.

@wpdarren
Copy link
Collaborator

@hussain-t in the QAB on this ticket it mentions findMatchedPublication in all of the code examples. I found the correct code for more than one publication and now see them all as expected.

QA Update: ✅

Verified:

The modules/reader-revenue-manager data store has a findMatchedPublication() action added, that finds a matching publication from the list of available publications based on the following criteria:

  • If there is no publication in the list, it returns null as there is no publication to select.
  • If there is only one publication in the list, it returns that publication.
  • If there is more than one publication in the list, it returns one as follows:
    • It prefers a publication that has completed setup and returns that publication.
    • Otherwise, it returns the first publication in the list.
Screenshots

image
image
image

@wpdarren wpdarren removed their assignment Jul 24, 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