Implement RRM PHP module foundation

[nfmohit]

Feature Description

The PHP module foundation should be implemented for the Reader Revenue Manager module with a slug of reader-revenue-manager.

• See relevant section in the design doc.
• See #5364 which was a similar effort for the former Thank with Google epic.

---

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

Acceptance criteria

• A module class, Modules\Reader_Revenue_Manager should be added extending Core\Modules\Module.
• The module slug should be reader-revenue-manager.
• The module name should be "Reader Revenue Manager".
• The module description should be "Reader Revenue Manager helps publishers grow, retain, and engage their audiences, creating new revenue opportunities".
• The module homepage should be https://readerrevenue.withgoogle.com/.
• The module class should require the https://www.googleapis.com/auth/subscribewithgoogle.publications.readonly scope.
• The module class should be added to Core\Modules\Modules only if the rrmModule feature flag is enabled.

Implementation Brief

• Create includes/Modules/Reader_Revenue_Manager.php:
  • The Google\Site_Kit\Modules\Reader_Revenue_Manager class will reside here.
  • The Class will extend Google\Site_Kit\Core\Modules\Module and implement Google\Site_Kit\Core\Modules\Module_With_Scopes interface.
  • Add the MODULE_SLUG const with the value from AC.
  • Add module information as described in AC inside setup_info function.
    • Set the order to 5 so it is placed between PageSpeed Insights and Tag Manager (alphabetical sorting, )
  • Add the scope defined in AC inside get_scopes function.
• In includes/Core/Modules/Modules.php:
  • Add the new Reader_Revenue_Manager module to the core_modules when rrmModule is enabled. See here for example.

Test Coverage

• Add test coverage for newly added module in a new Google\Site_Kit\Tests\Modules\Reader_Revenue_ManagerTest class.
  • Add test for get_scopes function.
  • Add a test_magic_methods to test basic module attributes. See similarly named tests for other modules for examples.
• Add test coverage for the addition of the newly added module when rrmModule feature flag is enabled in Google\Site_Kit\Tests\Core\Modules\ModulesTest class.

QA Brief

• Enable the rrmModule feature flag via the tester plugin.
• Execute the following commands in the browser console to verify the module has been added:
  • googlesitekit.data.select('core/modules').getModules()
  • Ensure that the Reader Revenue Manager module is included in the list of modules.
  • googlesitekit.data.select('core/modules').getModule('reader-revenue-manager')
  • Verify that the Reader Revenue Manager module details (description, homepage, order) are correctly displayed.
• Disable the rrmModule feature flag via the tester plugin.
• Execute the above commands in the browser console.
• Verify that the Reader Revenue Manager module is not included in the list of modules and that the module details are not displayed.

Changelog entry

• Add foundation for new Reader Revenue Manager feature (PHP).

[kuasha420]

AC ✅

[kuasha420]

Hi @nfmohit, while working on this IB, I realized that we should move a few points around the ACs to make things a bit smother and self contained. Namely,

1. We should Add a note about adding module settings for RRM here. In that way, we can flesh out is_connected. register and on_deactivation (using Module_With_Deactivation) here since these parts are fundamental as well.
2. We should remove the Module_With_Service_Entity requirement from here and add it to the REST_Route issue in #8791 since that's where we are going to really going to use the service. Since the logic around determining property id from SC and everything else will have to be explored in the rest route issue anyways, this seems like a logical thing to do to not bloat this issue unnecessarily!
3. By doing the above changes, we can keep this one self contained and remove #8783 blocker for this one (and add 8783 as additional blocker for 8791).

I'm sending this back to AC to you for another look over, let me know what you think, and if you agree, make the amendments in AC and send it back to me straight to IB again.

Sorry for not catching this earlier while doing ACR. Hope you don't mind. <3

[nfmohit]

Thank you @kuasha420. As discussed internally, the first point will be handled by #8793, but the other points have been addressed. This is back with you in IB.

[nfmohit]

The IB looks nice, thanks @kuasha420 . Do you think a Tests\Modules\Reader_Revenue_ManagerTest test suite should be added as part of this issue as well that acts as a starting point for test coverage for this class? It can include basic tests like Tests\Modules\AdsTest::test_magic_methods() and possibly scope verifications. Let me know what you think, thanks!

Note: The estimate may change based on the update.

[kuasha420]

Thank you @nfmohit. Sorry for forgetting about tests! I've added your suggestion, another point about checking the existence of RRM Module when the feature flag is active, and bumped up the estimate.

Cheers.

[nfmohit]

Thanks @kuasha420 ! IB ✅

[kelvinballoo]

QA Update ⚠️

This is pretty much good to go. I only have one question at the end.

Tested the following as good:

• Verified that the Reader Revenue Manager module details (description, homepage, order) are correctly displayed when the flag is on.

  • The module slug should be reader-revenue-manager.
  • The module name should be "Reader Revenue Manager".
  • The module description should be "Reader Revenue Manager helps publishers grow, retain, and engage their audiences, creating new revenue opportunities".
  • The module homepage should be https://readerrevenue.withgoogle.com/.
  

• Verified that the Reader Revenue Manager module doesn't appear if rrmModule feature flag is disabled via the tester plugin.

  

Question:
All the QAB points are fine.
I only noticed the following point in the AC and I was wondering if there is a way to verify it.
The module class should require the https://www.googleapis.com/auth/subscribewithgoogle.publications.readonly scope.
If this cannot be verified from QA perspective and it's a technical detail that is already taken care of, then it's fine.

[hussain-t]

@kelvinballoo, the scopes cannot be tested with the changes made in this ticket. However, you can view the code and the PHP unit tests to verify the necessary scope.

Code Changes:

site-kit-wp/includes/Modules/Reader_Revenue_Manager.php

Lines 48 to 52 in 110a5b0

	public function get_scopes() {
	return array(
	'https://www.googleapis.com/auth/subscribewithgoogle.publications.readonly',
	);
	}

PHP Unit Test:

site-kit-wp/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php

Lines 62 to 69 in e25f7e3

	public function test_get_scopes() {
	$this->assertEqualSets(
	array(
	'https://www.googleapis.com/auth/subscribewithgoogle.publications.readonly',
	),
	$this->reader_revenue_manager->get_scopes()
	);
	}

[kelvinballoo]

QA Update ✅

Thanks for the clarification @hussain-t .

This is good to go.

• Verified that the Reader Revenue Manager module details (description, homepage, order) are correctly displayed when the flag is on.

  • The module slug should be reader-revenue-manager.
  • The module name should be "Reader Revenue Manager".
  • The module description should be "Reader Revenue Manager helps publishers grow, retain, and engage their audiences, creating new revenue opportunities".
  • The module homepage should be https://readerrevenue.withgoogle.com/.
  

• Verified that the Reader Revenue Manager module doesn't appear if rrmModule feature flag is disabled via the tester plugin.