From 5dac654dcd6b1a79c65fa72a171ed4e9ed982b18 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 4 Jul 2024 15:45:55 +0530 Subject: [PATCH 01/15] Add RRM module settings. --- .../Reader_Revenue_Manager/Settings.php | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 includes/Modules/Reader_Revenue_Manager/Settings.php diff --git a/includes/Modules/Reader_Revenue_Manager/Settings.php b/includes/Modules/Reader_Revenue_Manager/Settings.php new file mode 100644 index 00000000000..6c2cddc863f --- /dev/null +++ b/includes/Modules/Reader_Revenue_Manager/Settings.php @@ -0,0 +1,123 @@ +register_owned_keys(); + } + + /** + * Returns keys for owned settings. + * + * @since n.e.x.t + * + * @return array An array of keys for owned settings. + */ + public function get_owned_keys() { + return array( + 'publicationID', + 'publicationOnboardingState', + 'publicationOnboardingStateLastSyncedAtMs', + ); + } + + /** + * Gets the default value. + * + * @since n.e.x.t + * + * @return array + */ + protected function get_default() { + return array( + 'publicationID' => '', + 'publicationOnboardingState' => '', + 'publicationOnboardingStateLastSyncedAtMs' => 0, + ); + } + + /** + * Returns keys for view-only settings. + * + * @since n.e.x.t + * + * @return array An array of keys for view-only settings. + */ + public function get_view_only_keys() { + return array(); + } + + /** + * Gets the callback for sanitizing the setting's value before saving. + * + * @since n.e.x.t + * + * @return callable|null + */ + protected function get_sanitize_callback() { + return function( $option ) { + if ( isset( $option['publicationID'] ) ) { + if ( ! preg_match( '/^[a-zA-Z0-9]+$/', $option['publicationID'] ) ) { + $option['publicationID'] = ''; + } + } + + if ( isset( $option['publicationOnboardingStateLastSyncedAtMs'] ) ) { + if ( ! is_int( $option['publicationOnboardingStateLastSyncedAtMs'] ) ) { + $option['publicationOnboardingStateLastSyncedAtMs'] = 0; + } + } + + if ( isset( $option['publicationOnboardingState'] ) ) { + + $valid_onboarding_states = array( + 'ONBOARDING_STATE_UNSPECIFIED', + 'ONBOARDING_ACTION_REQUIRED', + 'PENDING_VERIFICATION', + 'ONBOARDING_COMPLETE', + ); + + if ( ! in_array( $option['publicationOnboardingState'], $valid_onboarding_states, true ) ) { + $option['publicationOnboardingState'] = ''; + } + } + + return $option; + }; + } +} From 1b08b386adb9334bd7aaae71a0244e5ee8f048c2 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 4 Jul 2024 21:04:51 +0530 Subject: [PATCH 02/15] Add settings in base store. --- .../reader-revenue-manager/datastore/base.js | 10 ++++ includes/Modules/Reader_Revenue_Manager.php | 52 +++++++++++++++++-- 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/assets/js/modules/reader-revenue-manager/datastore/base.js b/assets/js/modules/reader-revenue-manager/datastore/base.js index da7cfb7a972..667865cedc6 100644 --- a/assets/js/modules/reader-revenue-manager/datastore/base.js +++ b/assets/js/modules/reader-revenue-manager/datastore/base.js @@ -26,4 +26,14 @@ import { validateCanSubmitChanges } from './settings'; export default Modules.createModuleStore( 'reader-revenue-manager', { storeName: MODULES_READER_REVENUE_MANAGER, validateCanSubmitChanges, + ownedSettingsSlugs: [ + 'publicationID', + 'publicationOnboardingState', + 'publicationOnboardingStateLastSyncedAtMs', + ], + settingSlugs: [ + 'publicationID', + 'publicationOnboardingState', + 'publicationOnboardingStateLastSyncedAtMs', + ], } ); diff --git a/includes/Modules/Reader_Revenue_Manager.php b/includes/Modules/Reader_Revenue_Manager.php index 4a504d82e5e..2e6ab841b4a 100644 --- a/includes/Modules/Reader_Revenue_Manager.php +++ b/includes/Modules/Reader_Revenue_Manager.php @@ -11,12 +11,19 @@ namespace Google\Site_Kit\Modules; use Google\Site_Kit\Core\Authentication\Clients\Google_Site_Kit_Client; +use Google\Site_Kit\Core\Assets\Script; use Google\Site_Kit\Core\Modules\Module; use Google\Site_Kit\Core\Modules\Module_With_Assets; use Google\Site_Kit\Core\Modules\Module_With_Scopes; use Google\Site_Kit\Core\Modules\Module_With_Assets_Trait; +use Google\Site_Kit\Core\Modules\Module_With_Deactivation; +use Google\Site_Kit\Core\Modules\Module_With_Owner; +use Google\Site_Kit\Core\Modules\Module_With_Owner_Trait; +use Google\Site_Kit\Core\Modules\Module_With_Settings; +use Google\Site_Kit\Core\Modules\Module_With_Settings_Trait; use Google\Site_Kit\Core\Modules\Module_With_Scopes_Trait; use Google\Site_Kit\Core\Modules\Module_With_Service_Entity; +use Google\Site_Kit\Modules\Reader_Revenue_Manager\Settings; use Google\Site_Kit\Core\REST_API\Data_Request; use Google\Site_Kit\Core\Util\URL; use Google\Site_Kit\Modules\Search_Console\Settings as Search_Console_Settings; @@ -26,12 +33,14 @@ /** * Class representing the Reader Revenue Manager module. * - * @since 1.130.0 + * @since n.e.x.t * @access private * @ignore */ -final class Reader_Revenue_Manager extends Module implements Module_With_Scopes, Module_With_Assets, Module_With_Service_Entity { +final class Reader_Revenue_Manager extends Module implements Module_With_Scopes, Module_With_Assets, Module_With_Service_Entity, Module_With_Deactivation, Module_With_Owner, Module_With_Settings { use Module_With_Assets_Trait; + use Module_With_Owner_Trait; + use Module_With_Settings_Trait; use Module_With_Scopes_Trait; /** @@ -42,7 +51,7 @@ final class Reader_Revenue_Manager extends Module implements Module_With_Scopes, /** * Registers functionality through WordPress hooks. * - * @since 1.130.0 + * @since n.e.x.t */ public function register() { $this->register_scopes_hook(); @@ -79,6 +88,43 @@ public function setup_services( Google_Site_Kit_Client $client ) { ); } + /** + * Checks whether the module is connected. + * + * @since n.e.x.t + * + * @return bool True if module is connected, false otherwise. + */ + public function is_connected() { + $options = $this->get_settings()->get(); + + if ( ! empty( $options['publicationID'] ) ) { + return true; + } + + return false; + } + + /** + * Sets up the module's settings instance. + * + * @since n.e.x.t + * + * @return Module_Settings + */ + protected function setup_settings() { + return new Settings( $this->options ); + } + + /** + * Cleans up when the module is deactivated. + * + * @since n.e.x.t + */ + public function on_deactivation() { + $this->get_settings()->delete(); + } + /** * Checks if the current user has access to the current configured service entity. * From 87130a1712b4f5df09e88cb6788ee3563f3a9aab Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Fri, 5 Jul 2024 10:16:12 +0530 Subject: [PATCH 03/15] Test for GET:publications. --- .../Modules/Reader_Revenue_ManagerTest.php | 43 ++++++++++++++++--- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php b/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php index 20a94e94d5b..a2e2ca92f6d 100644 --- a/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php +++ b/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php @@ -20,7 +20,8 @@ use Google\Site_Kit_Dependencies\GuzzleHttp\Psr7\Request; use Google\Site_Kit_Dependencies\GuzzleHttp\Psr7\Response; use Google\Site_Kit\Tests\Core\Modules\Module_With_Service_Entity_ContractTests; -use Google\Site_Kit_Dependencies\Google\Service\SubscribewithGoogle as Google_Service_SubscribewithGoogle; +use Google\Site_Kit_Dependencies\Google\Service\SubscribewithGoogle\ListPublicationsResponse; +use Google\Site_Kit_Dependencies\Google\Service\SubscribewithGoogle\Publication; /** * @group Modules @@ -115,7 +116,34 @@ function ( Request $request ) { switch ( $url['path'] ) { case '/v1/publications': - return new Response( 200 ); + $publications = array( + array( + 'publicationId' => 'ABCDEFGH', + 'publicationPredicates' => array( + 'businessPredicates' => array( + 'supportsSiteKit' => true, + 'canSell' => true, + ), + ), + 'verifiedDomains' => 'example.com', + 'paymentOptions' => array( + 'subscriptions' => true, + 'noPayment' => false, + 'contributions' => false, + 'thankStickers' => true, + ), + 'displayName' => 'Test Property', + 'products' => array( + array( + 'name' => 'basic', + ), + ), + 'onboardingState' => 'PENDING_VERIFICATION', + ), + ); + $response = new ListPublicationsResponse(); + $response->setPublications( $publications ); + return new Response( 200, array(), json_encode( $response ) ); } } ); @@ -126,9 +154,14 @@ function ( Request $request ) { $this->authentication->get_oauth_client()->get_required_scopes() ); - $data = $this->reader_revenue_manager->get_data( 'publications' ); - $this->assertNotWPError( $data ); - $this->assertIsArray( $data ); + $result = $this->reader_revenue_manager->get_data( 'publications' ); + $this->assertNotWPError( $result ); + $this->assertContainsOnlyInstancesOf( Publication::class, $result ); + + $publication = $result[0]; + + $this->assertEquals( 'Test Property', $publication->getDisplayName() ); + $this->assertEquals( 'ABCDEFGH', $publication->getPublicationId() ); } /** From 96da6c496553c4d4050474085bace5ce3e9438d4 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Fri, 5 Jul 2024 11:15:10 +0530 Subject: [PATCH 04/15] Validation functions. --- .../datastore/settings.js | 22 ++++++++++++++--- .../datastore/settings.test.js | 2 +- .../utils/validation.js | 24 +++++++++++++++++++ 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/assets/js/modules/reader-revenue-manager/datastore/settings.js b/assets/js/modules/reader-revenue-manager/datastore/settings.js index 57c408bef23..d105e2e3a2a 100644 --- a/assets/js/modules/reader-revenue-manager/datastore/settings.js +++ b/assets/js/modules/reader-revenue-manager/datastore/settings.js @@ -30,26 +30,42 @@ import { INVARIANT_SETTINGS_NOT_CHANGED, } from '../../../googlesitekit/data/create-settings-store'; import { createStrictSelect } from '../../../googlesitekit/data/utils'; -import { isValidPublicationID } from '../utils/validation'; +import { + isValidPublicationID, + isValidOnboardingState, +} from '../utils/validation'; // Invariant error messages. export const INVARIANT_INVALID_PUBLICATION_ID = 'a valid publicationID is required'; +export const INVARIANT_INVALID_PUBLICATION_ONBOARDING_STATE = + 'a valid publication onboarding state is required'; + export function validateCanSubmitChanges( select ) { const strictSelect = createStrictSelect( select ); // Strict select will cause all selector functions to throw an error // if `undefined` is returned, otherwise it behaves the same as `select`. // This ensures that the selector returns `false` until all data dependencies are resolved. - const { haveSettingsChanged, isDoingSubmitChanges, getPublicationID } = - strictSelect( MODULES_READER_REVENUE_MANAGER ); + const { + haveSettingsChanged, + isDoingSubmitChanges, + getPublicationID, + getPublicationOnboardingState, + } = strictSelect( MODULES_READER_REVENUE_MANAGER ); invariant( ! isDoingSubmitChanges(), INVARIANT_DOING_SUBMIT_CHANGES ); invariant( haveSettingsChanged(), INVARIANT_SETTINGS_NOT_CHANGED ); const publicationID = getPublicationID(); + const onboardingState = getPublicationOnboardingState(); invariant( isValidPublicationID( publicationID ), INVARIANT_INVALID_PUBLICATION_ID ); + + invariant( + isValidOnboardingState( onboardingState ), + INVARIANT_INVALID_PUBLICATION_ONBOARDING_STATE + ); } diff --git a/assets/js/modules/reader-revenue-manager/datastore/settings.test.js b/assets/js/modules/reader-revenue-manager/datastore/settings.test.js index f772e88a2bb..d1bbcbb0387 100644 --- a/assets/js/modules/reader-revenue-manager/datastore/settings.test.js +++ b/assets/js/modules/reader-revenue-manager/datastore/settings.test.js @@ -45,7 +45,7 @@ describe( 'modules/reader-revenue-manager settings', () => { } ); describe( 'validateCanSubmitChanges', () => { - it( 'it validates', () => { + it( 'validates', () => { // TODO: Implement tests as part of #8793. } ); } ); diff --git a/assets/js/modules/reader-revenue-manager/utils/validation.js b/assets/js/modules/reader-revenue-manager/utils/validation.js index 507a2693c06..9bf5c4a3f5e 100644 --- a/assets/js/modules/reader-revenue-manager/utils/validation.js +++ b/assets/js/modules/reader-revenue-manager/utils/validation.js @@ -31,6 +31,30 @@ export function isValidPublicationID( publicationID ) { ); } +/** + * Checks if the given publication onboarding state appears to be a valid. + * + * @since n.e.x.t + * + * @param {string} onboardingState Publication onboarding state. + * @return {boolean} `true` if the given publication ID is valid, `false` otherwise. + */ +export function isValidOnboardingState( onboardingState ) { + if ( typeof onboardingState !== 'string' ) { + return false; + } + + // List of valid onboarding states. + const validStates = [ + 'ONBOARDING_STATE_UNSPECIFIED', + 'ONBOARDING_ACTION_REQUIRED', + 'PENDING_VERIFICATION', + 'ONBOARDING_COMPLETE', + ]; + + return validStates.includes( onboardingState ); +} + /** * Checks if a given URL uses HTTPS. * From 2cca448198552925963bcd5931608b90911fc349 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Fri, 5 Jul 2024 17:56:33 +0530 Subject: [PATCH 05/15] Tests for Settings and Module class. --- includes/Modules/Reader_Revenue_Manager.php | 3 + .../Reader_Revenue_Manager/SettingsTest.php | 78 +++++++++++++++++++ .../Modules/Reader_Revenue_ManagerTest.php | 35 +++++++++ 3 files changed, 116 insertions(+) create mode 100644 tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php diff --git a/includes/Modules/Reader_Revenue_Manager.php b/includes/Modules/Reader_Revenue_Manager.php index 2e6ab841b4a..8e58fa320cc 100644 --- a/includes/Modules/Reader_Revenue_Manager.php +++ b/includes/Modules/Reader_Revenue_Manager.php @@ -16,6 +16,7 @@ use Google\Site_Kit\Core\Modules\Module_With_Assets; use Google\Site_Kit\Core\Modules\Module_With_Scopes; use Google\Site_Kit\Core\Modules\Module_With_Assets_Trait; +use Google\Site_Kit\Core\Modules\Module_With_Data_Available_State_Trait; use Google\Site_Kit\Core\Modules\Module_With_Deactivation; use Google\Site_Kit\Core\Modules\Module_With_Owner; use Google\Site_Kit\Core\Modules\Module_With_Owner_Trait; @@ -39,6 +40,7 @@ */ final class Reader_Revenue_Manager extends Module implements Module_With_Scopes, Module_With_Assets, Module_With_Service_Entity, Module_With_Deactivation, Module_With_Owner, Module_With_Settings { use Module_With_Assets_Trait; + use Module_With_Data_Available_State_Trait; use Module_With_Owner_Trait; use Module_With_Settings_Trait; use Module_With_Scopes_Trait; @@ -123,6 +125,7 @@ protected function setup_settings() { */ public function on_deactivation() { $this->get_settings()->delete(); + $this->reset_data_available(); } /** diff --git a/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php b/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php new file mode 100644 index 00000000000..7e3011bb40f --- /dev/null +++ b/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php @@ -0,0 +1,78 @@ +options = new Options( new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE ) ); + $this->settings = new Settings( $this->options ); + $this->user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); + + wp_set_current_user( $this->user_id ); + } + + public function test_get_default() { + $this->settings->register(); + + $this->assertEqualSetsWithIndex( + array( + 'publicationID' => '', + 'publicationOnboardingState' => '', + 'publicationOnboardingStateLastSyncedAtMs' => 0, + ), + get_option( Settings::OPTION ) + ); + } + + public function test_view_only_keys() { + $this->assertIsArray( $this->settings->get_view_only_keys() ); + $this->assertEmpty( $this->settings->get_view_only_keys() ); + } + + /** + * @inheritDoc + */ + protected function get_option_name() { + return Settings::OPTION; + } +} diff --git a/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php b/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php index a2e2ca92f6d..a85e55364d3 100644 --- a/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php +++ b/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php @@ -13,6 +13,7 @@ use Google\Site_Kit\Context; use Google\Site_Kit\Core\Authentication\Authentication; use Google\Site_Kit\Modules\Reader_Revenue_Manager; +use Google\Site_Kit\Modules\Reader_Revenue_Manager\Settings; use Google\Site_Kit\Tests\TestCase; use Google\Site_Kit\Tests\FakeHttp; use Google\Site_Kit\Core\Storage\User_Options; @@ -22,6 +23,7 @@ use Google\Site_Kit\Tests\Core\Modules\Module_With_Service_Entity_ContractTests; use Google\Site_Kit_Dependencies\Google\Service\SubscribewithGoogle\ListPublicationsResponse; use Google\Site_Kit_Dependencies\Google\Service\SubscribewithGoogle\Publication; +use Google\Site_Kit\Tests\Core\Modules\Module_With_Settings_ContractTests; /** * @group Modules @@ -29,6 +31,7 @@ */ class Reader_Revenue_ManagerTest extends TestCase { + use Module_With_Settings_ContractTests; use Module_With_Service_Entity_ContractTests; /** @@ -164,6 +167,38 @@ function ( Request $request ) { $this->assertEquals( 'ABCDEFGH', $publication->getPublicationId() ); } + public function test_is_connected() { + $options = new Options( $this->context ); + $rrm = new Reader_Revenue_Manager( $this->context, $options ); + + $this->assertFalse( $rrm->is_connected() ); + + $options->set( + Settings::OPTION, + array( + 'publicationID' => 'ABCDEFGH', + ) + ); + + $this->assertTrue( $rrm->is_connected() ); + } + + public function test_on_deactivation() { + $options = new Options( $this->context ); + $options->set( Settings::OPTION, 'test-value' ); + + $rrm = new Reader_Revenue_Manager( $this->context, $options ); + $rrm->set_data_available(); + $rrm->on_deactivation(); + + $this->assertOptionNotExists( Settings::OPTION ); + $this->assertFalse( $rrm->is_data_available() ); + } + + public function get_module_with_settings() { + return $this->reader_revenue_manager; + } + /** * @return Reader_Revenue_Manager */ From a4c180df014b80cc41b4ede32b7ef1cb129b11a6 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Fri, 5 Jul 2024 20:01:28 +0530 Subject: [PATCH 06/15] Add tests for validations. --- .../datastore/settings.test.js | 38 ++++++++++++++++++- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/assets/js/modules/reader-revenue-manager/datastore/settings.test.js b/assets/js/modules/reader-revenue-manager/datastore/settings.test.js index d1bbcbb0387..3eb05dbba31 100644 --- a/assets/js/modules/reader-revenue-manager/datastore/settings.test.js +++ b/assets/js/modules/reader-revenue-manager/datastore/settings.test.js @@ -24,6 +24,12 @@ import { createTestRegistry, unsubscribeFromAll, } from '../../../../../tests/js/utils'; +import { MODULES_READER_REVENUE_MANAGER } from './constants'; +import { + INVARIANT_INVALID_PUBLICATION_ID, + INVARIANT_INVALID_PUBLICATION_ONBOARDING_STATE, + validateCanSubmitChanges, +} from './settings'; describe( 'modules/reader-revenue-manager settings', () => { let registry; @@ -45,8 +51,36 @@ describe( 'modules/reader-revenue-manager settings', () => { } ); describe( 'validateCanSubmitChanges', () => { - it( 'validates', () => { - // TODO: Implement tests as part of #8793. + it( 'should throw invariant error for invalid publication ID', () => { + const settings = { + publicationID: 12345, + publicationOnboardingState: '', + publicationOnboardingStateLastSyncedAtMs: 0, + }; + + registry + .dispatch( MODULES_READER_REVENUE_MANAGER ) + .setSettings( settings ); + + expect( () => validateCanSubmitChanges( registry.select ) ).toThrow( + INVARIANT_INVALID_PUBLICATION_ID + ); + } ); + + it( 'should throw invariant error for invalid publication onboarding state', () => { + const settings = { + publicationID: 'ABCDEFGH', + publicationOnboardingState: 'invalid_state', + publicationOnboardingStateLastSyncedAtMs: 0, + }; + + registry + .dispatch( MODULES_READER_REVENUE_MANAGER ) + .setSettings( settings ); + + expect( () => validateCanSubmitChanges( registry.select ) ).toThrow( + INVARIANT_INVALID_PUBLICATION_ONBOARDING_STATE + ); } ); } ); } ); From 707b912a8b5eff223313bdb3ee2b5d3e759db3f6 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Mon, 8 Jul 2024 21:22:16 +0530 Subject: [PATCH 07/15] Test for special chars. --- .../datastore/settings.test.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/assets/js/modules/reader-revenue-manager/datastore/settings.test.js b/assets/js/modules/reader-revenue-manager/datastore/settings.test.js index 3eb05dbba31..6ca8e2b56dc 100644 --- a/assets/js/modules/reader-revenue-manager/datastore/settings.test.js +++ b/assets/js/modules/reader-revenue-manager/datastore/settings.test.js @@ -51,7 +51,7 @@ describe( 'modules/reader-revenue-manager settings', () => { } ); describe( 'validateCanSubmitChanges', () => { - it( 'should throw invariant error for invalid publication ID', () => { + it( 'should throw invariant error for invalid publication ID of type number', () => { const settings = { publicationID: 12345, publicationOnboardingState: '', @@ -67,6 +67,22 @@ describe( 'modules/reader-revenue-manager settings', () => { ); } ); + it( 'should throw invariant error for invalid publication ID with special chars', () => { + const settings = { + publicationID: 'ABCD&*12345', + publicationOnboardingState: 'ONBOARDING_ACTION_REQUIRED', + publicationOnboardingStateLastSyncedAtMs: 0, + }; + + registry + .dispatch( MODULES_READER_REVENUE_MANAGER ) + .setSettings( settings ); + + expect( () => validateCanSubmitChanges( registry.select ) ).toThrow( + INVARIANT_INVALID_PUBLICATION_ID + ); + } ); + it( 'should throw invariant error for invalid publication onboarding state', () => { const settings = { publicationID: 'ABCDEFGH', From 5a9d8a3cd333df1556e303cdd0211ae5056e88c6 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Mon, 8 Jul 2024 21:33:17 +0530 Subject: [PATCH 08/15] Fix: tests. --- .../Modules/Reader_Revenue_ManagerTest.php | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php b/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php index 09369613811..feba02f9af6 100644 --- a/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php +++ b/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php @@ -34,6 +34,13 @@ class Reader_Revenue_ManagerTest extends TestCase { use Module_With_Settings_ContractTests; use Module_With_Service_Entity_ContractTests; + /** + * Context object. + * + * @var Context + */ + private $context; + /** * Authentication object. * @@ -51,12 +58,12 @@ class Reader_Revenue_ManagerTest extends TestCase { public function set_up() { parent::set_up(); - $context = new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE ); - $options = new Options( $context ); + $this->context = new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE ); + $options = new Options( $this->context ); $user = $this->factory()->user->create_and_get( array( 'role' => 'administrator' ) ); - $user_options = new User_Options( $context, $user->ID ); - $this->authentication = new Authentication( $context, $options, $user_options ); - $this->reader_revenue_manager = new Reader_Revenue_Manager( $context, $options, $user_options, $this->authentication ); + $user_options = new User_Options( $this->context, $user->ID ); + $this->authentication = new Authentication( $this->context, $options, $user_options ); + $this->reader_revenue_manager = new Reader_Revenue_Manager( $this->context, $options, $user_options, $this->authentication ); } public function test_register() { From 635194aa5a1b396902423723661c85c48634ac40 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Tue, 9 Jul 2024 21:12:35 +0530 Subject: [PATCH 09/15] Add more test coverage. --- .../reader-revenue-manager/datastore/base.js | 6 +---- .../utils/validation.js | 2 +- includes/Modules/Reader_Revenue_Manager.php | 4 +-- .../Reader_Revenue_Manager/Settings.php | 23 +++++++++------- .../Reader_Revenue_Manager/SettingsTest.php | 27 +++++++++++++++++++ .../Modules/Reader_Revenue_ManagerTest.php | 16 +++++------ 6 files changed, 52 insertions(+), 26 deletions(-) diff --git a/assets/js/modules/reader-revenue-manager/datastore/base.js b/assets/js/modules/reader-revenue-manager/datastore/base.js index 667865cedc6..b6fd4ea0abe 100644 --- a/assets/js/modules/reader-revenue-manager/datastore/base.js +++ b/assets/js/modules/reader-revenue-manager/datastore/base.js @@ -26,11 +26,7 @@ import { validateCanSubmitChanges } from './settings'; export default Modules.createModuleStore( 'reader-revenue-manager', { storeName: MODULES_READER_REVENUE_MANAGER, validateCanSubmitChanges, - ownedSettingsSlugs: [ - 'publicationID', - 'publicationOnboardingState', - 'publicationOnboardingStateLastSyncedAtMs', - ], + ownedSettingsSlugs: [ 'publicationID' ], settingSlugs: [ 'publicationID', 'publicationOnboardingState', diff --git a/assets/js/modules/reader-revenue-manager/utils/validation.js b/assets/js/modules/reader-revenue-manager/utils/validation.js index 9bf5c4a3f5e..e65433b95c4 100644 --- a/assets/js/modules/reader-revenue-manager/utils/validation.js +++ b/assets/js/modules/reader-revenue-manager/utils/validation.js @@ -32,7 +32,7 @@ export function isValidPublicationID( publicationID ) { } /** - * Checks if the given publication onboarding state appears to be a valid. + * Checks if the given publication onboarding state is valid. * * @since n.e.x.t * diff --git a/includes/Modules/Reader_Revenue_Manager.php b/includes/Modules/Reader_Revenue_Manager.php index c7d43601340..81635d70195 100644 --- a/includes/Modules/Reader_Revenue_Manager.php +++ b/includes/Modules/Reader_Revenue_Manager.php @@ -34,7 +34,7 @@ /** * Class representing the Reader Revenue Manager module. * - * @since n.e.x.t + * @since 1.130.0 * @access private * @ignore */ @@ -53,7 +53,7 @@ final class Reader_Revenue_Manager extends Module implements Module_With_Scopes, /** * Registers functionality through WordPress hooks. * - * @since n.e.x.t + * @since 1.130.0 */ public function register() { $this->register_scopes_hook(); diff --git a/includes/Modules/Reader_Revenue_Manager/Settings.php b/includes/Modules/Reader_Revenue_Manager/Settings.php index 6c2cddc863f..73064cfb645 100644 --- a/includes/Modules/Reader_Revenue_Manager/Settings.php +++ b/includes/Modules/Reader_Revenue_Manager/Settings.php @@ -30,6 +30,14 @@ class Settings extends Module_Settings implements Setting_With_Owned_Keys_Interf const OPTION = 'googlesitekit_reader-revenue-manager_settings'; + const ONBOARDING_STATE_UNSPECIFIED = 'ONBOARDING_STATE_UNSPECIFIED'; + + const ONBOARDING_STATE_ACTION_REQUIRED = 'ONBOARDING_ACTION_REQUIRED'; + + const ONBOARDING_STATE_PENDING_VERIFICATION = 'PENDING_VERIFICATION'; + + const ONBOARDING_STATE_COMPLETE = 'ONBOARDING_COMPLETE'; + /** * Registers the setting in WordPress. * @@ -49,11 +57,7 @@ public function register() { * @return array An array of keys for owned settings. */ public function get_owned_keys() { - return array( - 'publicationID', - 'publicationOnboardingState', - 'publicationOnboardingStateLastSyncedAtMs', - ); + return array( 'publicationID' ); } /** @@ -104,12 +108,11 @@ protected function get_sanitize_callback() { } if ( isset( $option['publicationOnboardingState'] ) ) { - $valid_onboarding_states = array( - 'ONBOARDING_STATE_UNSPECIFIED', - 'ONBOARDING_ACTION_REQUIRED', - 'PENDING_VERIFICATION', - 'ONBOARDING_COMPLETE', + self::ONBOARDING_STATE_UNSPECIFIED, + self::ONBOARDING_STATE_ACTION_REQUIRED, + self::ONBOARDING_STATE_PENDING_VERIFICATION, + self::ONBOARDING_STATE_COMPLETE, ); if ( ! in_array( $option['publicationOnboardingState'], $valid_onboarding_states, true ) ) { diff --git a/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php b/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php index 7e3011bb40f..d8ba699d888 100644 --- a/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php +++ b/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php @@ -69,6 +69,33 @@ public function test_view_only_keys() { $this->assertEmpty( $this->settings->get_view_only_keys() ); } + public function data_publications() { + return array( + 'publicationID is valid string' => array( 'publicationID', 'ABCD1234', 'ABCD1234' ), + 'publicationOnboardingState is valid string' => array( 'publicationOnboardingState', 'PENDING_VERIFICATION', 'PENDING_VERIFICATION' ), + 'publicationOnboardingStateLastSyncedAtMs is valid' => array( 'publicationOnboardingStateLastSyncedAtMs', 0, 0 ), + 'publicationID is invalid string' => array( 'publicationID', 'ABCD1234&^##', '' ), + 'publicationOnboardingState is invalid string' => array( 'publicationOnboardingState', 'INVALID_STATE', '' ), + 'publicationOnboardingStateLastSyncedAtMs is invalid' => array( 'publicationOnboardingStateLastSyncedAtMs', 0.87686, 0 ), + ); + } + + /** + * @dataProvider data_publications + */ + public function test_rrm_settings_sanitization( $setting, $value, $expected_value ) { + $this->settings->register(); + + $options_key = $this->get_option_name(); + delete_option( $options_key ); + + $options = $this->settings->get(); + $options[ $setting ] = $value; + $this->settings->set( $options ); + $options = get_option( $options_key ); + $this->assertEquals( $expected_value, $options[ $setting ] ); + } + /** * @inheritDoc */ diff --git a/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php b/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php index feba02f9af6..be227be752d 100644 --- a/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php +++ b/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php @@ -166,10 +166,10 @@ function ( Request $request ) { } public function test_is_connected() { - $options = new Options( $this->context ); - $rrm = new Reader_Revenue_Manager( $this->context, $options ); + $options = new Options( $this->context ); + $reader_revenue_manager = new Reader_Revenue_Manager( $this->context, $options ); - $this->assertFalse( $rrm->is_connected() ); + $this->assertFalse( $reader_revenue_manager->is_connected() ); $options->set( Settings::OPTION, @@ -178,19 +178,19 @@ public function test_is_connected() { ) ); - $this->assertTrue( $rrm->is_connected() ); + $this->assertTrue( $reader_revenue_manager->is_connected() ); } public function test_on_deactivation() { $options = new Options( $this->context ); $options->set( Settings::OPTION, 'test-value' ); - $rrm = new Reader_Revenue_Manager( $this->context, $options ); - $rrm->set_data_available(); - $rrm->on_deactivation(); + $reader_revenue_manager = new Reader_Revenue_Manager( $this->context, $options ); + $reader_revenue_manager->set_data_available(); + $reader_revenue_manager->on_deactivation(); $this->assertOptionNotExists( Settings::OPTION ); - $this->assertFalse( $rrm->is_data_available() ); + $this->assertFalse( $reader_revenue_manager->is_data_available() ); } public function get_module_with_settings() { From cc87ccd27ac27f170f7cb730f659cd4c9c88fe3d Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Tue, 9 Jul 2024 21:29:12 +0530 Subject: [PATCH 10/15] Fix: phpcs error. --- includes/Modules/Reader_Revenue_Manager/Settings.php | 1 + 1 file changed, 1 insertion(+) diff --git a/includes/Modules/Reader_Revenue_Manager/Settings.php b/includes/Modules/Reader_Revenue_Manager/Settings.php index 73064cfb645..f79d9f72c37 100644 --- a/includes/Modules/Reader_Revenue_Manager/Settings.php +++ b/includes/Modules/Reader_Revenue_Manager/Settings.php @@ -95,6 +95,7 @@ public function get_view_only_keys() { */ protected function get_sanitize_callback() { return function( $option ) { + if ( isset( $option['publicationID'] ) ) { if ( ! preg_match( '/^[a-zA-Z0-9]+$/', $option['publicationID'] ) ) { $option['publicationID'] = ''; From 168b113bb95a34e7ad252c9045397da3d990bcb0 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Tue, 9 Jul 2024 21:38:48 +0530 Subject: [PATCH 11/15] Fix: phpcs error. --- includes/Modules/Reader_Revenue_Manager/Settings.php | 1 + 1 file changed, 1 insertion(+) diff --git a/includes/Modules/Reader_Revenue_Manager/Settings.php b/includes/Modules/Reader_Revenue_Manager/Settings.php index f79d9f72c37..b441751d6da 100644 --- a/includes/Modules/Reader_Revenue_Manager/Settings.php +++ b/includes/Modules/Reader_Revenue_Manager/Settings.php @@ -94,6 +94,7 @@ public function get_view_only_keys() { * @return callable|null */ protected function get_sanitize_callback() { + return function( $option ) { if ( isset( $option['publicationID'] ) ) { From d879772bd6ad957f15b7c761f043f00ec4e3b27a Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Tue, 9 Jul 2024 21:43:19 +0530 Subject: [PATCH 12/15] Fix: phpcs error. --- includes/Modules/Reader_Revenue_Manager/Settings.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/includes/Modules/Reader_Revenue_Manager/Settings.php b/includes/Modules/Reader_Revenue_Manager/Settings.php index b441751d6da..f5133c2cdfd 100644 --- a/includes/Modules/Reader_Revenue_Manager/Settings.php +++ b/includes/Modules/Reader_Revenue_Manager/Settings.php @@ -94,9 +94,7 @@ public function get_view_only_keys() { * @return callable|null */ protected function get_sanitize_callback() { - - return function( $option ) { - + return function ( $option ) { if ( isset( $option['publicationID'] ) ) { if ( ! preg_match( '/^[a-zA-Z0-9]+$/', $option['publicationID'] ) ) { $option['publicationID'] = ''; From 51d78368982ea35da3b7daf91f901cd2e47a3e51 Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Wed, 10 Jul 2024 10:09:41 +0100 Subject: [PATCH 13/15] Tweak formatting and add a comment. --- includes/Modules/Reader_Revenue_Manager/Settings.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/includes/Modules/Reader_Revenue_Manager/Settings.php b/includes/Modules/Reader_Revenue_Manager/Settings.php index f5133c2cdfd..32237d62869 100644 --- a/includes/Modules/Reader_Revenue_Manager/Settings.php +++ b/includes/Modules/Reader_Revenue_Manager/Settings.php @@ -30,13 +30,13 @@ class Settings extends Module_Settings implements Setting_With_Owned_Keys_Interf const OPTION = 'googlesitekit_reader-revenue-manager_settings'; - const ONBOARDING_STATE_UNSPECIFIED = 'ONBOARDING_STATE_UNSPECIFIED'; - - const ONBOARDING_STATE_ACTION_REQUIRED = 'ONBOARDING_ACTION_REQUIRED'; - + /** + * Various Reader Revenue Manager onboarding statuses. + */ + const ONBOARDING_STATE_UNSPECIFIED = 'ONBOARDING_STATE_UNSPECIFIED'; + const ONBOARDING_STATE_ACTION_REQUIRED = 'ONBOARDING_ACTION_REQUIRED'; const ONBOARDING_STATE_PENDING_VERIFICATION = 'PENDING_VERIFICATION'; - - const ONBOARDING_STATE_COMPLETE = 'ONBOARDING_COMPLETE'; + const ONBOARDING_STATE_COMPLETE = 'ONBOARDING_COMPLETE'; /** * Registers the setting in WordPress. From 40da58d75904efe5d8d9242498276fdc10189117 Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Wed, 10 Jul 2024 10:11:42 +0100 Subject: [PATCH 14/15] Rename dataProvider method. --- .../Modules/Reader_Revenue_Manager/SettingsTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php b/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php index d8ba699d888..d5b3c9e491b 100644 --- a/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php +++ b/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php @@ -69,7 +69,7 @@ public function test_view_only_keys() { $this->assertEmpty( $this->settings->get_view_only_keys() ); } - public function data_publications() { + public function data_publication_settings() { return array( 'publicationID is valid string' => array( 'publicationID', 'ABCD1234', 'ABCD1234' ), 'publicationOnboardingState is valid string' => array( 'publicationOnboardingState', 'PENDING_VERIFICATION', 'PENDING_VERIFICATION' ), @@ -81,7 +81,7 @@ public function data_publications() { } /** - * @dataProvider data_publications + * @dataProvider data_publication_settings */ public function test_rrm_settings_sanitization( $setting, $value, $expected_value ) { $this->settings->register(); From f6d4c9a052c94516b79467806ada1bf2b4418a23 Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Wed, 10 Jul 2024 10:12:18 +0100 Subject: [PATCH 15/15] Rename test method. --- .../integration/Modules/Reader_Revenue_Manager/SettingsTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php b/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php index d5b3c9e491b..f2e4ad316db 100644 --- a/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php +++ b/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php @@ -83,7 +83,7 @@ public function data_publication_settings() { /** * @dataProvider data_publication_settings */ - public function test_rrm_settings_sanitization( $setting, $value, $expected_value ) { + public function test_reader_revenue_manager_settings_sanitization( $setting, $value, $expected_value ) { $this->settings->register(); $options_key = $this->get_option_name();