Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions assets/js/modules/reader-revenue-manager/datastore/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
* Internal dependencies
*/
import Modules from 'googlesitekit-modules';
import { isFeatureEnabled } from '@/js/features';
import { MODULE_SLUG_READER_REVENUE_MANAGER } from '@/js/modules/reader-revenue-manager/constants';
import { MODULES_READER_REVENUE_MANAGER } from './constants';
import { submitChanges, validateCanSubmitChanges } from './settings';
Expand All @@ -41,5 +42,8 @@ export default Modules.createModuleStore( MODULE_SLUG_READER_REVENUE_MANAGER, {
'productID',
'productIDs',
'paymentOption',
...( isFeatureEnabled( 'rrmExpressSetup' )
? [ 'organizationID', 'configuredCTAs' ]
: [] ),
],
} );
69 changes: 68 additions & 1 deletion includes/Modules/Reader_Revenue_Manager/Settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
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\Feature_Flags;
use Google\Site_Kit\Core\Util\Method_Proxy_Trait;

/**
Expand Down Expand Up @@ -51,6 +52,31 @@ public function register() {
$this->register_owned_keys();
}

/**
* Gets the value of the setting.
*
* Overrides the parent to ensure empty configuredCTAs arrays are cast
* to objects so that json_encode produces {} instead of [].
*
* @since n.e.x.t
*
* @return mixed Value set for the option, or registered default if not set.
*/
public function get() {
$settings = parent::get();

if (
is_array( $settings )
&& Feature_Flags::enabled( 'rrmExpressSetup' )
&& isset( $settings['configuredCTAs'] )
&& array() === $settings['configuredCTAs']
) {
$settings['configuredCTAs'] = (object) array();
}

return $settings;
}

/**
* Returns keys for owned settings.
*
Expand All @@ -66,11 +92,12 @@ public function get_owned_keys() {
* Gets the default value.
*
* @since 1.132.0
* @since n.e.x.t Added the `organizationID` and `configuredCTAs` settings.
*
* @return array
*/
protected function get_default() {
return array(
$defaults = array(
'contentPolicyState' => '',
'policyInfoLink' => '',
'ownerID' => 0,
Expand All @@ -83,6 +110,13 @@ protected function get_default() {
'postTypes' => array( 'post' ),
'productID' => 'openaccess',
);

if ( Feature_Flags::enabled( 'rrmExpressSetup' ) ) {
$defaults['organizationID'] = '';
$defaults['configuredCTAs'] = array();
Comment thread
nfmohit marked this conversation as resolved.
}

return $defaults;
}

/**
Expand All @@ -107,6 +141,7 @@ public function get_view_only_keys() {
* Gets the callback for sanitizing the setting's value before saving.
*
* @since 1.132.0
* @since n.e.x.t Added sanitization for the `organizationID` and `configuredCTAs` settings.
*
* @return callable|null
*/
Expand Down Expand Up @@ -193,7 +228,39 @@ protected function get_sanitize_callback() {
$option['policyInfoLink'] = '';
}

$option = $this->sanitize_express_setup_settings( $option );

return $option;
};
}

/**
* Sanitizes the express setup settings.
*
* @since n.e.x.t
*
* @param array $option Settings array.
* @return array Sanitized settings array.
*/
private function sanitize_express_setup_settings( $option ) {
if ( isset( $option['organizationID'] ) && ! is_string( $option['organizationID'] ) ) {
$option['organizationID'] = '';
}

if ( isset( $option['configuredCTAs'] ) ) {
if ( ! is_array( $option['configuredCTAs'] ) ) {
$option['configuredCTAs'] = array();
} else {
$option['configuredCTAs'] = array_filter(
$option['configuredCTAs'],
function ( $value, $key ) {
return is_string( $key ) && is_string( $value );
},
ARRAY_FILTER_USE_BOTH
);
}
}

return $option;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,79 @@ public function test_get_default() {
);
}

public function test_get_default__with_express_setup_flag() {
$this->enable_feature( 'rrmExpressSetup' );
$this->settings->register();

$this->assertEqualSetsWithIndex(
array(
'contentPolicyState' => '',
'policyInfoLink' => '',
'ownerID' => 0,
'organizationID' => '',
'publicationID' => '',
'publicationOnboardingState' => '',
'publicationOnboardingStateChanged' => false,
'snippetMode' => 'post_types',
'postTypes' => array( 'post' ),
'productID' => 'openaccess',
'productIDs' => array(),
'paymentOption' => '',
'configuredCTAs' => array(),
),
get_option( Settings::OPTION ),
'RRM Settings should include the express setup settings when the flag is enabled.'
);
}

public function test_get__casts_empty_configured_ctas_to_object() {
$this->enable_feature( 'rrmExpressSetup' );
$this->settings->register();

$settings = $this->settings->get();

$this->assertEquals(
(object) array(),
$settings['configuredCTAs'],
'Empty configuredCTAs should be cast to an object on get().'
);

$this->assertEquals(
'{}',
wp_json_encode( $settings['configuredCTAs'] ),
'Empty configuredCTAs should JSON-encode as an object.'
);

// Stored value remains an empty array.
$this->assertSame(
array(),
get_option( Settings::OPTION )['configuredCTAs'],
'Empty configuredCTAs should remain stored as an array.'
);
}

public function test_get__preserves_non_empty_configured_ctas() {
$this->enable_feature( 'rrmExpressSetup' );
$this->settings->register();

$configured_ctas = array(
'9d2418415-ab3a' => 'newsletter-signup',
'8j8152411-cd4b' => 'survey',
);

$options = $this->settings->get();
$options['configuredCTAs'] = $configured_ctas;
$this->settings->set( $options );

$settings = $this->settings->get();

$this->assertSame(
$configured_ctas,
$settings['configuredCTAs'],
'Non-empty configuredCTAs should be returned as an associative array.'
);
}

public function test_view_only_keys() {
$this->assertEqualSets(
array(
Expand Down Expand Up @@ -173,6 +246,41 @@ public function data_revenue_manager_settings() {
true,
'',
),

// Validate organizationID.
'organizationID with valid string' => array( 'organizationID', 'ABCD1234', 'ABCD1234' ),
'organizationID with empty string' => array( 'organizationID', '', '' ),
'organizationID with number' => array( 'organizationID', 123, '' ),
'organizationID with boolean' => array( 'organizationID', true, '' ),
'organizationID with array' => array( 'organizationID', array(), '' ),

// Validate configuredCTAs.
'configuredCTAs with valid entries' => array(
'configuredCTAs',
array(
'9d2418415-ab3a' => 'newsletter-signup',
'8j8152411-cd4b' => 'survey',
),
array(
'9d2418415-ab3a' => 'newsletter-signup',
'8j8152411-cd4b' => 'survey',
),
Comment thread
nfmohit marked this conversation as resolved.
),
'configuredCTAs with empty array' => array( 'configuredCTAs', array(), array() ),
'configuredCTAs with invalid type' => array( 'configuredCTAs', 'not-an-array', array() ),
'configuredCTAs with number' => array( 'configuredCTAs', 123, array() ),
'configuredCTAs with invalid entries dropped' => array(
'configuredCTAs',
array(
'9d2418415-ab3a' => 'newsletter-signup',
'8j8152411-cd4b' => 123,
'7f1042311-ef5c' => array( 'survey' ),
0 => 'numeric-key',
),
array(
'9d2418415-ab3a' => 'newsletter-signup',
),
),
);
}

Expand Down
Loading