diff --git a/assets/js/components/settings/SettingsActiveModules.stories.js b/assets/js/components/settings/SettingsActiveModules.stories.js index 2def2ae99fd..3bd0bc1e625 100644 --- a/assets/js/components/settings/SettingsActiveModules.stories.js +++ b/assets/js/components/settings/SettingsActiveModules.stories.js @@ -152,11 +152,9 @@ WithRRMActionNeeded.args = { .receiveGetSettings( { publicationID: 'test-publication-id', publicationOnboardingState: 'ONBOARDING_COMPLETE', - contentPolicyStatus: { - contentPolicyState: - CONTENT_POLICY_STATES.CONTENT_POLICY_VIOLATION_GRACE_PERIOD, - policyInfoLink: 'https://example.com/policy-info', - }, + contentPolicyState: + CONTENT_POLICY_STATES.CONTENT_POLICY_VIOLATION_GRACE_PERIOD, + policyInfoLink: 'https://example.com/policy-info', } ); }, }; diff --git a/assets/js/modules/reader-revenue-manager/components/common/PolicyViolationSettingsNotice.stories.tsx b/assets/js/modules/reader-revenue-manager/components/common/PolicyViolationSettingsNotice.stories.tsx index 8e5164ed1ee..b5885abbbc8 100644 --- a/assets/js/modules/reader-revenue-manager/components/common/PolicyViolationSettingsNotice.stories.tsx +++ b/assets/js/modules/reader-revenue-manager/components/common/PolicyViolationSettingsNotice.stories.tsx @@ -62,10 +62,8 @@ function setupRegistry( registry: Registry, contentPolicyState: string ) { publicationID: '1234', publicationOnboardingState: PUBLICATION_ONBOARDING_STATES.ONBOARDING_COMPLETE, - contentPolicyStatus: { - contentPolicyState, - policyInfoLink: POLICY_INFO_URL, - }, + contentPolicyState, + policyInfoLink: POLICY_INFO_URL, } ); } diff --git a/assets/js/modules/reader-revenue-manager/components/common/PolicyViolationSettingsNotice.test.tsx b/assets/js/modules/reader-revenue-manager/components/common/PolicyViolationSettingsNotice.test.tsx index 0acd674c126..de855a2e8ac 100644 --- a/assets/js/modules/reader-revenue-manager/components/common/PolicyViolationSettingsNotice.test.tsx +++ b/assets/js/modules/reader-revenue-manager/components/common/PolicyViolationSettingsNotice.test.tsx @@ -63,13 +63,11 @@ describe( 'PolicyViolationSettingsNotice', () => { publicationID: 'ABCDEFGH', publicationOnboardingState: PUBLICATION_ONBOARDING_STATES.ONBOARDING_COMPLETE, - contentPolicyStatus: { - contentPolicyState, - policyInfoLink: - contentPolicyState === CONTENT_POLICY_STATE_OK - ? null - : POLICY_INFO_URL, - }, + contentPolicyState, + policyInfoLink: + contentPolicyState === CONTENT_POLICY_STATE_OK + ? null + : POLICY_INFO_URL, } ); } diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/PolicyViolationNotification/index.stories.tsx b/assets/js/modules/reader-revenue-manager/components/dashboard/PolicyViolationNotification/index.stories.tsx index 003c6c4ad9a..d3f26e002fd 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/PolicyViolationNotification/index.stories.tsx +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/PolicyViolationNotification/index.stories.tsx @@ -138,10 +138,8 @@ export default { publicationID: '1234', publicationOnboardingState: PUBLICATION_ONBOARDING_STATES.ONBOARDING_COMPLETE, - contentPolicyStatus: { - contentPolicyState: parameters.contentPolicyState, - policyInfoLink: POLICY_INFO_URL, - }, + contentPolicyState: parameters.contentPolicyState, + policyInfoLink: POLICY_INFO_URL, } ); } diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/PolicyViolationNotification/index.test.tsx b/assets/js/modules/reader-revenue-manager/components/dashboard/PolicyViolationNotification/index.test.tsx index 048827cd8db..f3103db356b 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/PolicyViolationNotification/index.test.tsx +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/PolicyViolationNotification/index.test.tsx @@ -89,13 +89,11 @@ describe( 'PolicyViolationNotification', () => { .receiveGetSettings( { publicationOnboardingState: PUBLICATION_ONBOARDING_STATES.ONBOARDING_COMPLETE, - contentPolicyStatus: { - contentPolicyState, - policyInfoLink: - contentPolicyState === CONTENT_POLICY_STATE_OK - ? null - : POLICY_INFO_URL, - }, + contentPolicyState, + policyInfoLink: + contentPolicyState === CONTENT_POLICY_STATE_OK + ? null + : POLICY_INFO_URL, } ); } diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification/index.stories.tsx b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification/index.stories.tsx index ce83609e3a4..02cc940fbc4 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification/index.stories.tsx +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification/index.stories.tsx @@ -267,14 +267,14 @@ export default { publicationOnboardingState: parameters.publicationOnboardingState, productIDs: [ 'product-a', 'product-b' ], - contentPolicyStatus: parameters.contentPolicyState + ...( parameters.contentPolicyState ? { contentPolicyState: parameters.contentPolicyState, policyInfoLink: 'https://example.com/policy-info', } - : undefined, + : {} ), } ); args?.setupRegistry?.( registry ); diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification/index.test.tsx b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification/index.test.tsx index 43ee61aca0a..0d5ca44190a 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification/index.test.tsx +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification/index.test.tsx @@ -507,10 +507,8 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { publicationOnboardingState: ONBOARDING_COMPLETE, paymentOption: 'subscriptions', productID: 'basic', - contentPolicyStatus: { - contentPolicyState, - policyInfoLink: 'https://example.com/policy', - }, + contentPolicyState, + policyInfoLink: 'https://example.com/policy', } ); const { getByRole, getByText } = render( @@ -543,10 +541,8 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { publicationOnboardingState: ONBOARDING_COMPLETE, paymentOption: 'subscriptions', productID: 'basic', - contentPolicyStatus: { - contentPolicyState, - policyInfoLink: 'https://example.com/policy', - }, + contentPolicyState, + policyInfoLink: 'https://example.com/policy', } ); await registry @@ -590,10 +586,8 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { publicationOnboardingState: ONBOARDING_COMPLETE, paymentOption: 'subscriptions', productID: 'basic', - contentPolicyStatus: { - contentPolicyState, - policyInfoLink: 'https://example.com/policy', - }, + contentPolicyState, + policyInfoLink: 'https://example.com/policy', } ); await registry @@ -649,10 +643,8 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { publicationOnboardingState: ONBOARDING_COMPLETE, paymentOption: 'subscriptions', productID: 'basic', - contentPolicyStatus: { - contentPolicyState, - policyInfoLink: 'https://example.com/policy', - }, + contentPolicyState, + policyInfoLink: 'https://example.com/policy', } ); const { getByRole } = render( @@ -693,10 +685,8 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { publicationOnboardingState: ONBOARDING_COMPLETE, paymentOption: 'subscriptions', productID: 'basic', - contentPolicyStatus: { - contentPolicyState, - policyInfoLink: null, - }, + contentPolicyState, + policyInfoLink: '', } ); const { getByText, queryByRole, queryByText } = render( @@ -749,12 +739,12 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { undefined, ...Object.values( CONTENT_POLICY_STATES ), ] )( 'with contentPolicyState %s', ( contentPolicyState ) => { - const contentPolicyStatus = contentPolicyState + const contentPolicySettings = contentPolicyState ? { contentPolicyState, policyInfoLink: 'https://example.com/policy', } - : undefined; + : {}; const expectedLabelSuffix = contentPolicyState ? `:${ contentPolicyState }` @@ -780,7 +770,7 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { publicationOnboardingState: ONBOARDING_COMPLETE, paymentOption: 'subscriptions', productID: 'basic', - contentPolicyStatus, + ...contentPolicySettings, } ); const { getByRole, waitForRegistry } = render( @@ -826,7 +816,7 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { publicationOnboardingState: ONBOARDING_COMPLETE, paymentOption: 'noPayment', productID: 'basic', - contentPolicyStatus, + ...contentPolicySettings, } ); const { getByText, waitForRegistry } = render( @@ -881,7 +871,7 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { publicationOnboardingState: ONBOARDING_COMPLETE, paymentOption: 'noPayment', productID: 'advanced', - contentPolicyStatus, + ...contentPolicySettings, } ); const { getByText, waitForRegistry } = render( diff --git a/assets/js/modules/reader-revenue-manager/components/settings/SettingsEdit.stories.js b/assets/js/modules/reader-revenue-manager/components/settings/SettingsEdit.stories.js index 2c94c0fafc9..9564005dc9f 100644 --- a/assets/js/modules/reader-revenue-manager/components/settings/SettingsEdit.stories.js +++ b/assets/js/modules/reader-revenue-manager/components/settings/SettingsEdit.stories.js @@ -229,13 +229,11 @@ WithPolicyViolationPending.args = { .dispatch( MODULES_READER_REVENUE_MANAGER ) .setProductIDs( [ 'product-a', 'product-b', 'product-c' ] ); - registry - .dispatch( MODULES_READER_REVENUE_MANAGER ) - .setContentPolicyStatus( { - contentPolicyState: - CONTENT_POLICY_STATES.CONTENT_POLICY_VIOLATION_GRACE_PERIOD, - policyInfoLink: 'https://publishercenter.google.com/policy', - } ); + registry.dispatch( MODULES_READER_REVENUE_MANAGER ).setSettings( { + contentPolicyState: + CONTENT_POLICY_STATES.CONTENT_POLICY_VIOLATION_GRACE_PERIOD, + policyInfoLink: 'https://publishercenter.google.com/policy', + } ); }, }; @@ -253,13 +251,11 @@ WithPolicyViolationActive.args = { .dispatch( MODULES_READER_REVENUE_MANAGER ) .setProductIDs( [ 'product-a', 'product-b', 'product-c' ] ); - registry - .dispatch( MODULES_READER_REVENUE_MANAGER ) - .setContentPolicyStatus( { - contentPolicyState: - CONTENT_POLICY_STATES.CONTENT_POLICY_VIOLATION_ACTIVE, - policyInfoLink: 'https://publishercenter.google.com/policy', - } ); + registry.dispatch( MODULES_READER_REVENUE_MANAGER ).setSettings( { + contentPolicyState: + CONTENT_POLICY_STATES.CONTENT_POLICY_VIOLATION_ACTIVE, + policyInfoLink: 'https://publishercenter.google.com/policy', + } ); }, }; @@ -277,13 +273,11 @@ WithPolicyViolationExtreme.args = { .dispatch( MODULES_READER_REVENUE_MANAGER ) .setProductIDs( [ 'product-a', 'product-b', 'product-c' ] ); - registry - .dispatch( MODULES_READER_REVENUE_MANAGER ) - .setContentPolicyStatus( { - contentPolicyState: - CONTENT_POLICY_STATES.CONTENT_POLICY_ORGANIZATION_VIOLATION_ACTIVE_IMMEDIATE, - policyInfoLink: 'https://publishercenter.google.com/policy', - } ); + registry.dispatch( MODULES_READER_REVENUE_MANAGER ).setSettings( { + contentPolicyState: + CONTENT_POLICY_STATES.CONTENT_POLICY_ORGANIZATION_VIOLATION_ACTIVE_IMMEDIATE, + policyInfoLink: 'https://publishercenter.google.com/policy', + } ); }, }; diff --git a/assets/js/modules/reader-revenue-manager/components/settings/SettingsView.stories.js b/assets/js/modules/reader-revenue-manager/components/settings/SettingsView.stories.js index bc6554819bd..eb1332644f9 100644 --- a/assets/js/modules/reader-revenue-manager/components/settings/SettingsView.stories.js +++ b/assets/js/modules/reader-revenue-manager/components/settings/SettingsView.stories.js @@ -172,11 +172,9 @@ export default { // Add content policy status if provided. if ( args?.contentPolicyState ) { - settings.contentPolicyStatus = { - contentPolicyState: args.contentPolicyState, - policyInfoLink: - 'https://publishercenter.google.com/policy', - }; + settings.contentPolicyState = args.contentPolicyState; + settings.policyInfoLink = + 'https://publishercenter.google.com/policy'; } registry diff --git a/assets/js/modules/reader-revenue-manager/components/setup/SetupForm.test.js b/assets/js/modules/reader-revenue-manager/components/setup/SetupForm.test.js index defa5bb23f6..480c5cd0c48 100644 --- a/assets/js/modules/reader-revenue-manager/components/setup/SetupForm.test.js +++ b/assets/js/modules/reader-revenue-manager/components/setup/SetupForm.test.js @@ -46,7 +46,6 @@ describe( 'SetupForm', () => { .receiveGetSettings( { snippetMode: 'post_types', postTypes: [ 'post' ], - contentPolicyStatus: {}, } ); } ); diff --git a/assets/js/modules/reader-revenue-manager/components/setup/SetupMain.test.js b/assets/js/modules/reader-revenue-manager/components/setup/SetupMain.test.js index c5ee162c535..f1bee31f5d7 100644 --- a/assets/js/modules/reader-revenue-manager/components/setup/SetupMain.test.js +++ b/assets/js/modules/reader-revenue-manager/components/setup/SetupMain.test.js @@ -63,7 +63,6 @@ describe( 'SetupMain', () => { .receiveGetSettings( { snippetMode: 'post_types', postTypes: [ 'post' ], - contentPolicyStatus: {}, } ); } ); diff --git a/assets/js/modules/reader-revenue-manager/datastore/base.js b/assets/js/modules/reader-revenue-manager/datastore/base.js index f96580c538f..60e4b5af524 100644 --- a/assets/js/modules/reader-revenue-manager/datastore/base.js +++ b/assets/js/modules/reader-revenue-manager/datastore/base.js @@ -30,7 +30,8 @@ export default Modules.createModuleStore( MODULE_SLUG_READER_REVENUE_MANAGER, { validateCanSubmitChanges, ownedSettingsSlugs: [ 'publicationID' ], settingSlugs: [ - 'contentPolicyStatus', + 'contentPolicyState', + 'policyInfoLink', 'ownerID', 'publicationID', 'publicationOnboardingState', diff --git a/assets/js/modules/reader-revenue-manager/datastore/publications.js b/assets/js/modules/reader-revenue-manager/datastore/publications.js index 83696aae3ea..582db3616ca 100644 --- a/assets/js/modules/reader-revenue-manager/datastore/publications.js +++ b/assets/js/modules/reader-revenue-manager/datastore/publications.js @@ -72,9 +72,15 @@ const fetchGetPublicationsStore = createFetchStore( { paymentOption: getPaymentOption( publication.paymentOptions ), - contentPolicyStatus: publication.contentPolicyStatus, }; + if ( publication.contentPolicyStatus ) { + newSettings.contentPolicyState = + publication.contentPolicyStatus.contentPolicyState; + newSettings.policyInfoLink = + publication.contentPolicyStatus.policyInfoLink || ''; + } + Object.assign( state.settings, newSettings ); if ( state.savedSettings ) { @@ -288,7 +294,10 @@ const baseActions = { }; if ( contentPolicyStatus ) { - settings.contentPolicyStatus = contentPolicyStatus; + settings.contentPolicyState = + contentPolicyStatus.contentPolicyState; + settings.policyInfoLink = + contentPolicyStatus.policyInfoLink || ''; } return registry @@ -374,37 +383,13 @@ const baseSelectors = { return selectedPublication.products.map( ( product ) => product.name ); } ), - /** - * Gets the content policy state from the content policy status. - * - * @since 1.171.0 - * - * @param {Object} state Data store's state. - * @return {(string|undefined)} The content policy state; `undefined` if not available. - */ - getContentPolicyState: createRegistrySelector( ( select ) => () => { - const settings = select( MODULES_READER_REVENUE_MANAGER ).getSettings(); - - if ( ! settings ) { - return undefined; - } - - const { contentPolicyStatus } = settings; - - if ( ! contentPolicyStatus ) { - return undefined; - } - - return contentPolicyStatus.contentPolicyState; - } ), - /** * Gets the policy info URL wrapped with the account chooser URL. * * @since 1.171.0 * * @param {Object} state Data store's state. - * @return {(string|null|undefined)} The policy info URL wrapped with the account chooser URL; `null` if `policyInfoLink` is `null`; `undefined` if not available. + * @return {(string|null|undefined)} The policy info URL wrapped with the account chooser URL; `null` if `policyInfoLink` is empty; `undefined` if not available. */ getPolicyInfoURL: createRegistrySelector( ( select ) => () => { const settings = select( MODULES_READER_REVENUE_MANAGER ).getSettings(); @@ -413,20 +398,17 @@ const baseSelectors = { return undefined; } - const { contentPolicyStatus } = settings; + const { policyInfoLink } = settings; - if ( contentPolicyStatus?.policyInfoLink === undefined ) { + if ( policyInfoLink === undefined ) { return undefined; } - if ( contentPolicyStatus.policyInfoLink === null ) { - // `policyInfoLink` is `null` when `contentPolicyState` is `CONTENT_POLICY_STATE_OK`. + if ( ! policyInfoLink ) { return null; } - return select( CORE_USER ).getAccountChooserURL( - contentPolicyStatus.policyInfoLink - ); + return select( CORE_USER ).getAccountChooserURL( policyInfoLink ); } ), }; diff --git a/assets/js/modules/reader-revenue-manager/datastore/publications.test.js b/assets/js/modules/reader-revenue-manager/datastore/publications.test.js index 0299020a8a1..82c7e5dee9c 100644 --- a/assets/js/modules/reader-revenue-manager/datastore/publications.test.js +++ b/assets/js/modules/reader-revenue-manager/datastore/publications.test.js @@ -491,7 +491,7 @@ describe( 'modules/reader-revenue-manager publications', () => { ).toEqual( 'openaccess' ); } ); - it( 'should set `contentPolicyStatus` in state when provided', () => { + it( 'should set `contentPolicyState` and `policyInfoLink` in state when `contentPolicyStatus` is provided', () => { const contentPolicyStatus = { contentPolicyState: 'CONTENT_POLICY_VIOLATION_ACTIVE', policyInfoLink: 'https://example.com/policy-info', @@ -510,11 +510,12 @@ describe( 'modules/reader-revenue-manager publications', () => { .select( MODULES_READER_REVENUE_MANAGER ) .getSettings() ).toMatchObject( { - contentPolicyStatus, + contentPolicyState: 'CONTENT_POLICY_VIOLATION_ACTIVE', + policyInfoLink: 'https://example.com/policy-info', } ); } ); - it( 'should not set `contentPolicyStatus` when not provided', () => { + it( 'should not set `contentPolicyState` or `policyInfoLink` when `contentPolicyStatus` is not provided', () => { registry .dispatch( MODULES_READER_REVENUE_MANAGER ) .selectPublication( { @@ -526,7 +527,8 @@ describe( 'modules/reader-revenue-manager publications', () => { .select( MODULES_READER_REVENUE_MANAGER ) .getSettings(); - expect( settings ).not.toHaveProperty( 'contentPolicyStatus' ); + expect( settings ).not.toHaveProperty( 'contentPolicyState' ); + expect( settings ).not.toHaveProperty( 'policyInfoLink' ); } ); } ); } ); @@ -578,10 +580,12 @@ describe( 'modules/reader-revenue-manager publications', () => { ); expect( settings.productIDs ).toEqual( [ 'basic' ] ); expect( settings.paymentOption ).toEqual( 'subscriptions' ); - expect( settings.contentPolicyStatus ).toEqual( { - contentPolicyState: 'CONTENT_POLICY_STATE_OK', - policyInfoLink: `https://publishercenter.google.com/reader-revenue-manager/settings/policy?publication=${ publication.publicationId }`, - } ); + expect( settings.contentPolicyState ).toEqual( + 'CONTENT_POLICY_STATE_OK' + ); + expect( settings.policyInfoLink ).toEqual( + `https://publishercenter.google.com/reader-revenue-manager/settings/policy?publication=${ publication.publicationId }` + ); } ); it( 'should update savedSettings in state when publications are fetched', async () => { @@ -817,7 +821,7 @@ describe( 'modules/reader-revenue-manager publications', () => { expect( contentPolicyState ).toBeUndefined(); } ); - it( 'should return `undefined` if `contentPolicyStatus` is not available', () => { + it( 'should return `undefined` if `contentPolicyState` is not in settings', () => { registry .dispatch( MODULES_READER_REVENUE_MANAGER ) .receiveGetSettings( { @@ -832,17 +836,13 @@ describe( 'modules/reader-revenue-manager publications', () => { expect( contentPolicyState ).toBeUndefined(); } ); - it( 'should return the `contentPolicyState` property from `contentPolicyStatus`', () => { + it( 'should return the `contentPolicyState` setting value', () => { registry .dispatch( MODULES_READER_REVENUE_MANAGER ) .receiveGetSettings( { publicationID: 'publication-id', publicationOnboardingState: 'onboarding-state', - contentPolicyStatus: { - contentPolicyState: - 'CONTENT_POLICY_VIOLATION_ACTIVE', - policyInfoLink: 'https://example.com/policy-info', - }, + contentPolicyState: 'CONTENT_POLICY_VIOLATION_ACTIVE', } ); const contentPolicyState = registry @@ -866,7 +866,7 @@ describe( 'modules/reader-revenue-manager publications', () => { expect( policyInfoURL ).toBeUndefined(); } ); - it( 'should return `undefined` if `contentPolicyStatus` is not available', () => { + it( 'should return `undefined` if `policyInfoLink` is not in settings', () => { registry .dispatch( MODULES_READER_REVENUE_MANAGER ) .receiveGetSettings( { @@ -881,35 +881,13 @@ describe( 'modules/reader-revenue-manager publications', () => { expect( policyInfoURL ).toBeUndefined(); } ); - it( 'should return `undefined` if `policyInfoLink` is not available', () => { + it( 'should return `null` if `policyInfoLink` is empty', () => { registry .dispatch( MODULES_READER_REVENUE_MANAGER ) .receiveGetSettings( { publicationID: 'publication-id', publicationOnboardingState: 'onboarding-state', - contentPolicyStatus: { - contentPolicyState: - 'CONTENT_POLICY_VIOLATION_ACTIVE', - }, - } ); - - const policyInfoURL = registry - .select( MODULES_READER_REVENUE_MANAGER ) - .getPolicyInfoURL(); - - expect( policyInfoURL ).toBeUndefined(); - } ); - - it( 'should return `null` if `policyInfoLink` is `null`', () => { - registry - .dispatch( MODULES_READER_REVENUE_MANAGER ) - .receiveGetSettings( { - publicationID: 'publication-id', - publicationOnboardingState: 'onboarding-state', - contentPolicyStatus: { - contentPolicyState: 'CONTENT_POLICY_STATE_OK', - policyInfoLink: null, - }, + policyInfoLink: '', } ); const policyInfoURL = registry @@ -919,7 +897,7 @@ describe( 'modules/reader-revenue-manager publications', () => { expect( policyInfoURL ).toBeNull(); } ); - it( 'should return the `policyInfoLink` property from `contentPolicyStatus`, wrapped with account chooser URL', () => { + it( 'should return the `policyInfoLink` setting wrapped with account chooser URL', () => { const testEmail = 'test@example.com'; const testPolicyInfoLink = 'https://example.com/policy-info'; @@ -932,11 +910,7 @@ describe( 'modules/reader-revenue-manager publications', () => { .receiveGetSettings( { publicationID: 'publication-id', publicationOnboardingState: 'onboarding-state', - contentPolicyStatus: { - contentPolicyState: - 'CONTENT_POLICY_VIOLATION_ACTIVE', - policyInfoLink: testPolicyInfoLink, - }, + policyInfoLink: testPolicyInfoLink, } ); const policyInfoURL = registry diff --git a/assets/js/modules/reader-revenue-manager/datastore/settings.js b/assets/js/modules/reader-revenue-manager/datastore/settings.js index c20193160c9..7ab3de08b4e 100644 --- a/assets/js/modules/reader-revenue-manager/datastore/settings.js +++ b/assets/js/modules/reader-revenue-manager/datastore/settings.js @@ -20,7 +20,6 @@ * External dependencies */ import invariant from 'invariant'; -import { isPlainObject } from 'lodash'; /** * Internal dependencies @@ -59,8 +58,11 @@ export const INVARIANT_INVALID_PRODUCT_IDS = export const INVARIANT_INVALID_PAYMENT_OPTION = 'a valid payment option is required'; -export const INVARIANT_INVALID_CONTENT_POLICY_STATUS = - 'a valid content policy status object is required'; +export const INVARIANT_INVALID_CONTENT_POLICY_STATE = + 'a valid content policy state string is required'; + +export const INVARIANT_INVALID_POLICY_INFO_LINK = + 'a valid policy info link string is required'; export function validateCanSubmitChanges( select ) { const strictSelect = createStrictSelect( select ); @@ -126,13 +128,22 @@ export function validateCanSubmitChanges( select ) { INVARIANT_INVALID_PAYMENT_OPTION ); - const contentPolicyStatus = strictSelect( + const contentPolicyState = strictSelect( + MODULES_READER_REVENUE_MANAGER + ).getContentPolicyState(); + + invariant( + typeof contentPolicyState === 'string', + INVARIANT_INVALID_CONTENT_POLICY_STATE + ); + + const policyInfoLink = strictSelect( MODULES_READER_REVENUE_MANAGER - ).getContentPolicyStatus(); + ).getPolicyInfoLink(); invariant( - isPlainObject( contentPolicyStatus ), - INVARIANT_INVALID_CONTENT_POLICY_STATUS + typeof policyInfoLink === 'string', + INVARIANT_INVALID_POLICY_INFO_LINK ); } 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 e24b06b8e42..4a1e755bd04 100644 --- a/assets/js/modules/reader-revenue-manager/datastore/settings.test.js +++ b/assets/js/modules/reader-revenue-manager/datastore/settings.test.js @@ -23,8 +23,9 @@ import { setUsingCache } from 'googlesitekit-api'; import { createTestRegistry } from '../../../../../tests/js/utils'; import { MODULES_READER_REVENUE_MANAGER } from './constants'; import { - INVARIANT_INVALID_CONTENT_POLICY_STATUS, + INVARIANT_INVALID_CONTENT_POLICY_STATE, INVARIANT_INVALID_PAYMENT_OPTION, + INVARIANT_INVALID_POLICY_INFO_LINK, INVARIANT_INVALID_POST_TYPES, INVARIANT_INVALID_PRODUCT_ID, INVARIANT_INVALID_PRODUCT_IDS, @@ -45,6 +46,8 @@ describe( 'modules/reader-revenue-manager settings', () => { publicationID: 'ABCDEFGH', publicationOnboardingState: 'ONBOARDING_ACTION_REQUIRED', publicationOnboardingStateChanged: false, + contentPolicyState: '', + policyInfoLink: '', snippetMode: 'post_types', postTypes: [ 'post' ], productID: 'valid-id', @@ -246,10 +249,10 @@ describe( 'modules/reader-revenue-manager settings', () => { ); } ); - it( 'should throw invariant error for invalid content policy status', () => { + it( 'should throw invariant error for invalid content policy state', () => { const settings = { ...validSettings, - contentPolicyStatus: 'not-an-object', + contentPolicyState: 123, }; registry @@ -257,7 +260,23 @@ describe( 'modules/reader-revenue-manager settings', () => { .setSettings( settings ); expect( () => validateCanSubmitChanges( registry.select ) ).toThrow( - INVARIANT_INVALID_CONTENT_POLICY_STATUS + INVARIANT_INVALID_CONTENT_POLICY_STATE + ); + } ); + + it( 'should throw invariant error for invalid policy info link', () => { + const settings = { + ...validSettings, + contentPolicyState: '', + policyInfoLink: 123, + }; + + registry + .dispatch( MODULES_READER_REVENUE_MANAGER ) + .setSettings( settings ); + + expect( () => validateCanSubmitChanges( registry.select ) ).toThrow( + INVARIANT_INVALID_POLICY_INFO_LINK ); } ); } ); diff --git a/includes/Core/Util/Migration_N_E_X_T.php b/includes/Core/Util/Migration_N_E_X_T.php new file mode 100644 index 00000000000..7e03648eb9f --- /dev/null +++ b/includes/Core/Util/Migration_N_E_X_T.php @@ -0,0 +1,130 @@ +context = $context; + $this->options = $options ?: new Options( $context ); + $this->rrm_settings = new Reader_Revenue_Manager_Settings( $this->options ); + } + + /** + * Registers hooks. + * + * @since n.e.x.t + */ + public function register() { + add_action( 'admin_init', array( $this, 'migrate' ) ); + } + + /** + * Migrates the DB. + * + * @since n.e.x.t + */ + public function migrate() { + $db_version = $this->options->get( self::DB_VERSION_OPTION ); + + if ( ! $db_version || version_compare( $db_version, self::DB_VERSION, '<' ) ) { + $this->migrate_rrm_content_policy_status(); + + $this->options->set( self::DB_VERSION_OPTION, self::DB_VERSION ); + } + } + + /** + * Migrates the legacy nested `contentPolicyStatus` setting into + * flat `contentPolicyState` and `policyInfoLink` settings. + * + * @since n.e.x.t + */ + protected function migrate_rrm_content_policy_status() { + if ( ! $this->rrm_settings->has() ) { + return; + } + + $rrm_settings = $this->rrm_settings->get(); + + if ( ! is_array( $rrm_settings ) || empty( $rrm_settings ) ) { + return; + } + + if ( ! array_key_exists( 'contentPolicyStatus', $rrm_settings ) ) { + return; + } + + $content_policy_status = (array) $rrm_settings['contentPolicyStatus']; + + $rrm_settings['contentPolicyState'] = $content_policy_status['contentPolicyState'] ?? ''; + $rrm_settings['policyInfoLink'] = $content_policy_status['policyInfoLink'] ?? ''; + + unset( $rrm_settings['contentPolicyStatus'] ); + + $this->rrm_settings->set( $rrm_settings ); + } +} diff --git a/includes/Modules/Reader_Revenue_Manager.php b/includes/Modules/Reader_Revenue_Manager.php index 2e3c605e020..4a74438e5e7 100644 --- a/includes/Modules/Reader_Revenue_Manager.php +++ b/includes/Modules/Reader_Revenue_Manager.php @@ -517,7 +517,8 @@ function ( $pub ) use ( $publication_id ) { $content_policy_status = $publication->getContentPolicyStatus(); if ( $content_policy_status ) { - $new_settings['contentPolicyStatus'] = (array) $content_policy_status->toSimpleObject(); + $new_settings['contentPolicyState'] = $content_policy_status->getContentPolicyState() ?? ''; + $new_settings['policyInfoLink'] = $content_policy_status->getPolicyInfoLink() ?? ''; } if ( $new_onboarding_state !== $onboarding_state ) { @@ -931,16 +932,13 @@ public function get_debug_fields() { ); } - if ( isset( $settings['contentPolicyStatus'] ) ) { - $content_policy_status = (array) $settings['contentPolicyStatus']; - $content_policy_state = $content_policy_status['contentPolicyState'] ?? ''; + $content_policy_state = $settings['contentPolicyState'] ?? ''; - $debug_fields['reader_revenue_manager_content_policy_state'] = array( - 'label' => __( 'Reader Revenue Manager: Content policy state', 'google-site-kit' ), - 'value' => $content_policy_state, - 'debug' => $content_policy_state, - ); - } + $debug_fields['reader_revenue_manager_content_policy_state'] = array( + 'label' => __( 'Reader Revenue Manager: Content policy state', 'google-site-kit' ), + 'value' => $content_policy_state, + 'debug' => $content_policy_state, + ); return $debug_fields; } diff --git a/includes/Modules/Reader_Revenue_Manager/Settings.php b/includes/Modules/Reader_Revenue_Manager/Settings.php index 2361a0ed394..82ca8b55646 100644 --- a/includes/Modules/Reader_Revenue_Manager/Settings.php +++ b/includes/Modules/Reader_Revenue_Manager/Settings.php @@ -14,7 +14,6 @@ use Google\Site_Kit\Core\Storage\Setting_With_Owned_Keys_Interface; use Google\Site_Kit\Core\Storage\Setting_With_Owned_Keys_Trait; use Google\Site_Kit\Core\Storage\Setting_With_ViewOnly_Keys_Interface; -use Google\Site_Kit\Core\Util\BC_Functions; use Google\Site_Kit\Core\Util\Method_Proxy_Trait; /** @@ -70,7 +69,8 @@ public function get_owned_keys() { */ protected function get_default() { return array( - 'contentPolicyStatus' => (object) array(), + 'contentPolicyState' => '', + 'policyInfoLink' => '', 'ownerID' => 0, 'publicationID' => '', 'publicationOnboardingState' => '', @@ -183,11 +183,12 @@ protected function get_sanitize_callback() { } } - if ( isset( $option['contentPolicyStatus'] ) ) { - // If the `contentPolicyStatus` setting is not an associative array, set it to an empty object. - if ( ! ( is_array( $option['contentPolicyStatus'] ) && ! BC_Functions::array_is_list( $option['contentPolicyStatus'] ) ) ) { - $option['contentPolicyStatus'] = (object) array(); - } + if ( isset( $option['contentPolicyState'] ) && ! is_string( $option['contentPolicyState'] ) ) { + $option['contentPolicyState'] = ''; + } + + if ( isset( $option['policyInfoLink'] ) && ! is_string( $option['policyInfoLink'] ) ) { + $option['policyInfoLink'] = ''; } return $option; diff --git a/includes/Plugin.php b/includes/Plugin.php index 5281e39be00..b0faa0da68e 100644 --- a/includes/Plugin.php +++ b/includes/Plugin.php @@ -226,6 +226,7 @@ function () use ( $options, $activation_flag ) { ( new Core\Util\Migration_1_129_0( $this->context, $options ) )->register(); ( new Core\Util\Migration_1_150_0( $this->context, $options ) )->register(); ( new Core\Util\Migration_1_163_0( $this->context, $options ) )->register(); + ( new Core\Util\Migration_N_E_X_T( $this->context, $options ) )->register(); ( new Core\Dashboard_Sharing\Dashboard_Sharing( $this->context ) )->register(); ( new Core\Key_Metrics\Key_Metrics( $this->context, $user_options, $options ) )->register(); ( new Core\Prompts\Prompts( $this->context, $user_options ) )->register(); diff --git a/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php b/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php new file mode 100644 index 00000000000..639c020fd37 --- /dev/null +++ b/tests/phpunit/integration/Core/Util/Migration_N_E_X_TTest.php @@ -0,0 +1,150 @@ +context = new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE ); + $this->options = new Options( $this->context ); + $this->rrm_settings = new Reader_Revenue_Manager_Settings( $this->options ); + + $this->rrm_settings->register(); + $this->delete_db_version(); + } + + public function get_new_migration_instance() { + return new Migration_N_E_X_T( + $this->context, + $this->options + ); + } + + public function test_register() { + $migration = $this->get_new_migration_instance(); + remove_all_actions( 'admin_init' ); + + $migration->register(); + + $this->assertTrue( has_action( 'admin_init' ), 'Migration should register admin_init action.' ); + } + + public function test_migrate_content_policy_status() { + $migration = $this->get_new_migration_instance(); + + $pre_migration_settings = array( + 'publicationID' => 'test-pub-id', + 'publicationOnboardingState' => 'ONBOARDING_COMPLETE', + 'contentPolicyStatus' => array( + 'contentPolicyState' => 'CONTENT_POLICY_VIOLATION_GRACE_PERIOD', + 'policyInfoLink' => 'https://example.com/policy-info', + ), + ); + + $this->rrm_settings->set( $pre_migration_settings ); + + $migration->migrate(); + + $post_migration_settings = $this->rrm_settings->get(); + + $this->assertArrayNotHasKey( 'contentPolicyStatus', $post_migration_settings, 'Legacy contentPolicyStatus key should be removed after migration.' ); + $this->assertEquals( 'CONTENT_POLICY_VIOLATION_GRACE_PERIOD', $post_migration_settings['contentPolicyState'], 'contentPolicyState should be migrated from nested contentPolicyStatus.' ); + $this->assertEquals( 'https://example.com/policy-info', $post_migration_settings['policyInfoLink'], 'policyInfoLink should be migrated from nested contentPolicyStatus.' ); + $this->assertEquals( 'test-pub-id', $post_migration_settings['publicationID'], 'Other settings should be preserved after migration.' ); + } + + public function test_migrate_content_policy_status_with_empty_object() { + $migration = $this->get_new_migration_instance(); + + $pre_migration_settings = array( + 'publicationID' => 'test-pub-id', + 'publicationOnboardingState' => 'ONBOARDING_COMPLETE', + 'contentPolicyStatus' => array(), + ); + + $this->rrm_settings->set( $pre_migration_settings ); + + $migration->migrate(); + + $post_migration_settings = $this->rrm_settings->get(); + + $this->assertArrayNotHasKey( 'contentPolicyStatus', $post_migration_settings, 'Legacy contentPolicyStatus key should be removed after migration.' ); + $this->assertEquals( '', $post_migration_settings['contentPolicyState'], 'contentPolicyState should default to empty string when not present in legacy data.' ); + $this->assertEquals( '', $post_migration_settings['policyInfoLink'], 'policyInfoLink should default to empty string when not present in legacy data.' ); + } + + public function test_migrate_skips_when_no_rrm_settings_exist() { + $migration = $this->get_new_migration_instance(); + + $migration->migrate(); + + $this->assertOptionNotExists( Reader_Revenue_Manager_Settings::OPTION ); + } + + public function test_migrate_skips_when_no_legacy_content_policy_status() { + $migration = $this->get_new_migration_instance(); + + $pre_migration_settings = array( + 'publicationID' => 'test-pub-id', + 'publicationOnboardingState' => 'ONBOARDING_COMPLETE', + 'contentPolicyState' => 'CONTENT_POLICY_VIOLATION_ENFORCEMENT', + 'policyInfoLink' => 'https://example.com/policy-info', + ); + + $this->rrm_settings->set( $pre_migration_settings ); + + $migration->migrate(); + + $post_migration_settings = $this->rrm_settings->get(); + + $this->assertEquals( 'CONTENT_POLICY_VIOLATION_ENFORCEMENT', $post_migration_settings['contentPolicyState'], 'contentPolicyState should remain unchanged when no legacy data exists.' ); + $this->assertEquals( 'https://example.com/policy-info', $post_migration_settings['policyInfoLink'], 'policyInfoLink should remain unchanged when no legacy data exists.' ); + } + + public function test_migrate_sets_db_version() { + $migration = $this->get_new_migration_instance(); + + $migration->migrate(); + + $this->assertEquals( Migration_N_E_X_T::DB_VERSION, $this->get_db_version(), 'DB version should be set after migration.' ); + } + + protected function get_db_version() { + return $this->options->get( Migration_N_E_X_T::DB_VERSION_OPTION ); + } + + protected function delete_db_version() { + $this->options->delete( Migration_N_E_X_T::DB_VERSION_OPTION ); + } +} diff --git a/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php b/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php index b5ed27bb10f..00ec71b3379 100644 --- a/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php +++ b/tests/phpunit/integration/Modules/Reader_Revenue_Manager/SettingsTest.php @@ -56,7 +56,8 @@ public function test_get_default() { $this->assertEqualSetsWithIndex( array( - 'contentPolicyStatus' => (object) array(), + 'contentPolicyState' => '', + 'policyInfoLink' => '', 'ownerID' => 0, 'publicationID' => '', 'publicationOnboardingState' => '', @@ -129,35 +130,48 @@ public function data_revenue_manager_settings() { 'paymentOption with number' => array( 'paymentOption', 123, '' ), 'paymentOption with boolean' => array( 'paymentOption', true, '' ), - // Validate contentPolicyStatus. - 'contentPolicyStatus with populated object' => array( - 'contentPolicyStatus', - array( - 'contentPolicyState' => 'CONTENT_POLICY_VIOLATION_GRACE_PERIOD', - ), - array( - 'contentPolicyState' => 'CONTENT_POLICY_VIOLATION_GRACE_PERIOD', - ), + // Validate contentPolicyState. + 'contentPolicyState with valid string' => array( + 'contentPolicyState', + 'CONTENT_POLICY_VIOLATION_GRACE_PERIOD', + 'CONTENT_POLICY_VIOLATION_GRACE_PERIOD', ), - 'contentPolicyStatus with empty object' => array( - 'contentPolicyStatus', - (object) array(), - (object) array(), + 'contentPolicyState with empty string' => array( + 'contentPolicyState', + '', + '', ), - 'contentPolicyStatus with empty array' => array( - 'contentPolicyStatus', - array(), - (object) array(), + 'contentPolicyState with number' => array( + 'contentPolicyState', + 123, + '', + ), + 'contentPolicyState with boolean' => array( + 'contentPolicyState', + true, + '', + ), + + // Validate policyInfoLink. + 'policyInfoLink with valid string' => array( + 'policyInfoLink', + 'https://example.com/policy-info', + 'https://example.com/policy-info', + ), + 'policyInfoLink with empty string' => array( + 'policyInfoLink', + '', + '', ), - 'contentPolicyStatus with number' => array( - 'contentPolicyStatus', + 'policyInfoLink with number' => array( + 'policyInfoLink', 123, - (object) array(), + '', ), - 'contentPolicyStatus with boolean' => array( - 'contentPolicyStatus', + 'policyInfoLink with boolean' => array( + 'policyInfoLink', true, - (object) array(), + '', ), ); } diff --git a/tests/phpunit/integration/Modules/Reader_Revenue_Manager/Synchronize_PublicationTest.php b/tests/phpunit/integration/Modules/Reader_Revenue_Manager/Synchronize_PublicationTest.php index 7e01a7ef1d0..ef06dad7f3c 100644 --- a/tests/phpunit/integration/Modules/Reader_Revenue_Manager/Synchronize_PublicationTest.php +++ b/tests/phpunit/integration/Modules/Reader_Revenue_Manager/Synchronize_PublicationTest.php @@ -303,18 +303,21 @@ public function test_synchronize_content_policy_status() { $settings = $this->reader_revenue_manager->get_settings()->get(); $this->synchronize_publication->register(); - $this->assertEquals( (object) array(), $settings['contentPolicyStatus'], 'Content policy status should be empty before sync.' ); + $this->assertEquals( '', $settings['contentPolicyState'], 'Content policy state should be empty before sync.' ); + $this->assertEquals( '', $settings['policyInfoLink'], 'Policy info link should be empty before sync.' ); do_action( Synchronize_Publication::CRON_SYNCHRONIZE_PUBLICATION ); $settings = $this->reader_revenue_manager->get_settings()->get(); $this->assertEquals( - array( - 'contentPolicyState' => 'CONTENT_POLICY_VIOLATION_ACTIVE', - 'policyInfoLink' => 'https://example.com/policy-info', - ), - $settings['contentPolicyStatus'], - 'Content policy status should be updated after sync.' + 'CONTENT_POLICY_VIOLATION_ACTIVE', + $settings['contentPolicyState'], + 'Content policy state should be updated after sync.' + ); + $this->assertEquals( + 'https://example.com/policy-info', + $settings['policyInfoLink'], + 'Policy info link should be updated after sync.' ); } diff --git a/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php b/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php index 0424828fc94..beff8284b98 100644 --- a/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php +++ b/tests/phpunit/integration/Modules/Reader_Revenue_ManagerTest.php @@ -376,12 +376,14 @@ function ( Request $request ) use ( $publication_id ) { 'Payment option should be updated after fetching publications.' ); $this->assertEquals( - array( - 'contentPolicyState' => 'CONTENT_POLICY_VIOLATION_ACTIVE', - 'policyInfoLink' => 'https://example.com/policy-info', - ), - $settings['contentPolicyStatus'], - 'Content policy status should be synchronized after fetching publications.' + 'CONTENT_POLICY_VIOLATION_ACTIVE', + $settings['contentPolicyState'], + 'Content policy state should be synchronized after fetching publications.' + ); + $this->assertEquals( + 'https://example.com/policy-info', + $settings['policyInfoLink'], + 'Policy info link should be synchronized after fetching publications.' ); } @@ -942,7 +944,7 @@ public function test_get_debug_fields() { public function test_get_debug_fields__content_policy_state() { $this->reader_revenue_manager->get_settings()->register(); - // Test that the content policy state debug field is included with default `contentPolicyStatus`. + // Test that the content policy state debug field is included with default value. $this->reader_revenue_manager->get_settings()->set( array( 'publicationID' => 'test-publication-id', @@ -959,16 +961,14 @@ public function test_get_debug_fields__content_policy_state() { $this->assertEquals( '', $debug_fields['reader_revenue_manager_content_policy_state']['value'], - 'Content policy state value should be empty when contentPolicyStatus is default.' + 'Content policy state value should be empty when contentPolicyState is default.' ); - // Test that the content policy state debug field has correct values when `contentPolicyStatus` contains `contentPolicyState`. + // Test that the content policy state debug field has correct values when `contentPolicyState` is set. $this->reader_revenue_manager->get_settings()->set( array( - 'publicationID' => 'test-publication-id', - 'contentPolicyStatus' => array( - 'contentPolicyState' => 'CONTENT_POLICY_VIOLATION_GRACE_PERIOD', - ), + 'publicationID' => 'test-publication-id', + 'contentPolicyState' => 'CONTENT_POLICY_VIOLATION_GRACE_PERIOD', ) ); @@ -976,7 +976,7 @@ public function test_get_debug_fields__content_policy_state() { $this->assertArrayHasKey( 'reader_revenue_manager_content_policy_state', $debug_fields, - 'Content policy state debug field should be included when contentPolicyStatus is set.' + 'Content policy state debug field should be included when contentPolicyState is set.' ); $this->assertEquals( @@ -996,33 +996,6 @@ public function test_get_debug_fields__content_policy_state() { $debug_fields['reader_revenue_manager_content_policy_state']['debug'], 'Content policy state field should have correct debug value' ); - - // Test that the content policy state field handles missing `contentPolicyState` property gracefully. - $this->reader_revenue_manager->get_settings()->set( - array( - 'publicationID' => 'test-publication-id', - 'contentPolicyStatus' => array(), - ) - ); - - $debug_fields = $this->reader_revenue_manager->get_debug_fields(); - $this->assertArrayHasKey( - 'reader_revenue_manager_content_policy_state', - $debug_fields, - 'Content policy state field should be included even when contentPolicyState is missing' - ); - - $this->assertEquals( - '', - $debug_fields['reader_revenue_manager_content_policy_state']['value'], - 'Content policy state field should have empty string when contentPolicyState is missing' - ); - - $this->assertEquals( - '', - $debug_fields['reader_revenue_manager_content_policy_state']['debug'], - 'Content policy state field debug should have empty string when contentPolicyState is missing' - ); } public function test_check_service_entity_access_no_access_unavailable_publication() {