diff --git a/projects/plugins/jetpack/_inc/client/newsletter/newsletter-categories.jsx b/projects/plugins/jetpack/_inc/client/newsletter/newsletter-categories.jsx index 9eeb45c8d0723..b01c64d9bee8b 100644 --- a/projects/plugins/jetpack/_inc/client/newsletter/newsletter-categories.jsx +++ b/projects/plugins/jetpack/_inc/client/newsletter/newsletter-categories.jsx @@ -54,7 +54,10 @@ function NewsletterCategories( props ) { dispatch, } = props; - const checkedCategoriesIds = newsletterCategories.map( mapCategoriesIds ); + const checkedCategoriesIds = useMemo( + () => newsletterCategories?.map( mapCategoriesIds ) ?? [], + [ newsletterCategories ] + ); const mappedCategories = useMemo( () => diff --git a/projects/plugins/jetpack/_inc/lib/class-jetpack-newsletter-category-helper.php b/projects/plugins/jetpack/_inc/lib/class-jetpack-newsletter-category-helper.php new file mode 100644 index 0000000000000..96488541c8c15 --- /dev/null +++ b/projects/plugins/jetpack/_inc/lib/class-jetpack-newsletter-category-helper.php @@ -0,0 +1,90 @@ + (int) $id ); + } + } + } + + // Check if it is an array of arrays with term_id keys. + // [{term_id: 123}, {term_id: 456}] + if ( isset( $newsletter_categories[0] ) && is_array( $newsletter_categories[0] ) && isset( $newsletter_categories[0]['term_id'] ) ) { + foreach ( $newsletter_categories as $category ) { + if ( isset( $category['term_id'] ) && is_numeric( $category['term_id'] ) ) { + $formatted_categories[] = array( 'term_id' => (int) $category['term_id'] ); + } + } + } + + if ( empty( $formatted_categories ) ) { + return false; + } + + return update_option( self::NEWSLETTER_CATEGORIES_OPTION, $formatted_categories ) + ? $formatted_categories + : false; + } +} diff --git a/projects/plugins/jetpack/_inc/lib/core-api/class.jetpack-core-api-module-endpoints.php b/projects/plugins/jetpack/_inc/lib/core-api/class.jetpack-core-api-module-endpoints.php index 280b89e35fb7f..7edc3bc64aa5c 100644 --- a/projects/plugins/jetpack/_inc/lib/core-api/class.jetpack-core-api-module-endpoints.php +++ b/projects/plugins/jetpack/_inc/lib/core-api/class.jetpack-core-api-module-endpoints.php @@ -504,6 +504,12 @@ public function get_all_options() { $response['newsletter_has_active_plan'] = count( Jetpack_Memberships::get_all_newsletter_plan_ids( false ) ) > 0; } + // Make sure we are returning a consistent type + if ( ! class_exists( 'Jetpack_Newsletter_Category_Helper' ) ) { + require_once JETPACK__PLUGIN_DIR . '_inc/lib/class-jetpack-newsletter-category-helper.php'; + } + $response['wpcom_newsletter_categories'] = Jetpack_Newsletter_Category_Helper::get_category_ids(); + return rest_ensure_response( $response ); } @@ -650,6 +656,10 @@ public function update_data( $request ) { } } + if ( ! class_exists( 'Jetpack_Newsletter_Category_Helper' ) ) { + require_once JETPACK__PLUGIN_DIR . '_inc/lib/class-jetpack-newsletter-category-helper.php'; + } + foreach ( $params as $option => $value ) { // Used if there was an error. Can be overwritten with specific error messages. @@ -1050,43 +1060,24 @@ function ( &$value ) { } break; - case 'wpcom_newsletter_categories': - $sanitized_category_ids = (array) $value; - - array_walk_recursive( - $sanitized_category_ids, - function ( &$value ) { - if ( is_int( $value ) && $value > 0 ) { - return; - } - - $value = (int) $value; - if ( $value <= 0 ) { - $value = null; - } - } - ); - - $sanitized_category_ids = array_unique( - array_filter( - $sanitized_category_ids, - function ( $category_id ) { - return $category_id !== null; - } - ) - ); + case Jetpack_Newsletter_Category_Helper::NEWSLETTER_CATEGORIES_OPTION: + if ( ! is_array( $value ) || empty( $value ) ) { + break; + } - $new_value = array_map( - function ( $category_id ) { - return array( 'term_id' => $category_id ); - }, - $sanitized_category_ids - ); + // If we are already current, do nothing + $current_value = Jetpack_Newsletter_Category_Helper::get_category_ids(); + if ( $value === $current_value ) { + break; + } - if ( ! update_option( $option, $new_value ) ) { + if ( Jetpack_Newsletter_Category_Helper::save_category_ids( $value ) ) { + $updated = true; + } else { $updated = false; - $error = esc_html__( 'WPCOM Newsletter Categories failed to process.', 'jetpack' ); + $error = esc_html__( 'Newsletter category did not update.', 'jetpack' ); } + break; default: diff --git a/projects/plugins/jetpack/changelog/arcanagelinis-newsletter-fix-v2 b/projects/plugins/jetpack/changelog/arcanagelinis-newsletter-fix-v2 new file mode 100644 index 0000000000000..8248ae0157b01 --- /dev/null +++ b/projects/plugins/jetpack/changelog/arcanagelinis-newsletter-fix-v2 @@ -0,0 +1,4 @@ +Significance: patch +Type: bugfix + +Newsletter: fix bug in category settings diff --git a/projects/plugins/jetpack/json-endpoints/class.wpcom-json-api-site-settings-endpoint.php b/projects/plugins/jetpack/json-endpoints/class.wpcom-json-api-site-settings-endpoint.php index bf9bf97532610..f07a8eaaf3ebb 100644 --- a/projects/plugins/jetpack/json-endpoints/class.wpcom-json-api-site-settings-endpoint.php +++ b/projects/plugins/jetpack/json-endpoints/class.wpcom-json-api-site-settings-endpoint.php @@ -374,22 +374,11 @@ public function get_settings_response() { ) ); - $newsletter_categories = maybe_unserialize( get_option( 'wpcom_newsletter_categories', array() ) ); - $newsletter_category_ids = array_filter( - array_map( - function ( $newsletter_category ) { - if ( is_array( $newsletter_category ) && isset( $newsletter_category['term_id'] ) ) { - // This is the expected format. - return (int) $newsletter_category['term_id']; - } elseif ( is_numeric( $newsletter_category ) ) { - // This is a previous format caused by a bug. - return (int) $newsletter_category; - } - return null; - }, - $newsletter_categories - ) - ); + // Make sure we are returning a consistent type + if ( ! class_exists( 'Jetpack_Newsletter_Category_Helper' ) ) { + require_once JETPACK__PLUGIN_DIR . '_inc/lib/class-jetpack-newsletter-category-helper.php'; + } + $newsletter_category_ids = Jetpack_Newsletter_Category_Helper::get_category_ids(); $api_cache = $site->is_jetpack() ? (bool) get_option( 'jetpack_api_cache_enabled' ) : true; @@ -673,6 +662,10 @@ public function update_settings() { $sharing_options = array(); $updated = array(); + if ( ! class_exists( 'Jetpack_Newsletter_Category_Helper' ) ) { + require_once JETPACK__PLUGIN_DIR . '_inc/lib/class-jetpack-newsletter-category-helper.php'; + } + foreach ( $input as $key => $value ) { if ( ! is_array( $value ) ) { @@ -1070,42 +1063,12 @@ function ( &$value ) { $updated[ $key ] = (int) (bool) $value; break; - case 'wpcom_newsletter_categories': - $sanitized_category_ids = (array) $value; - - array_walk_recursive( - $sanitized_category_ids, - function ( &$value ) { - if ( is_int( $value ) && $value > 0 ) { - return; - } - - $value = (int) $value; - if ( $value <= 0 ) { - $value = null; - } - } - ); - - $sanitized_category_ids = array_unique( - array_filter( - $sanitized_category_ids, - function ( $category_id ) { - return $category_id !== null; - } - ) - ); - - $new_value = array_map( - function ( $category_id ) { - return array( 'term_id' => $category_id ); - }, - $sanitized_category_ids - ); - - if ( update_option( $key, $new_value ) ) { - $updated[ $key ] = $sanitized_category_ids; + case Jetpack_Newsletter_Category_Helper::NEWSLETTER_CATEGORIES_OPTION: + $update_newsletter_categories = Jetpack_Newsletter_Category_Helper::save_category_ids( (array) $value ); + if ( $update_newsletter_categories ) { + $updated[ $key ] = $update_newsletter_categories; } + break; case 'wpcom_newsletter_categories_enabled': diff --git a/projects/plugins/jetpack/tests/php/_inc/lib/Jetpack_Newsletter_Category_Helper_Test.php b/projects/plugins/jetpack/tests/php/_inc/lib/Jetpack_Newsletter_Category_Helper_Test.php new file mode 100644 index 0000000000000..8e7bebe848fbd --- /dev/null +++ b/projects/plugins/jetpack/tests/php/_inc/lib/Jetpack_Newsletter_Category_Helper_Test.php @@ -0,0 +1,195 @@ +assertEquals( array(), $result ); + } + + /** + * Test get_category_ids() returns empty array when option is empty. + */ + public function test_get_category_ids_empty_option() { + update_option( 'wpcom_newsletter_categories', array() ); + $result = Jetpack_Newsletter_Category_Helper::get_category_ids(); + $this->assertEquals( array(), $result ); + } + + /** + * Test get_category_ids() returns empty array when option is not an array. + */ + public function test_get_category_ids_invalid_option() { + update_option( 'wpcom_newsletter_categories', 'not-an-array' ); + $result = Jetpack_Newsletter_Category_Helper::get_category_ids(); + $this->assertEquals( array(), $result ); + } + + /** + * Test get_category_ids() handles array of integers format. + */ + public function test_get_category_ids_integer_array() { + $categories = array( 123, 456, 789 ); + update_option( 'wpcom_newsletter_categories', $categories ); + $result = Jetpack_Newsletter_Category_Helper::get_category_ids(); + $this->assertEquals( $categories, $result ); + } + + /** + * Test get_category_ids() handles array of arrays with term_id format. + */ + public function test_get_category_ids_term_id_array() { + $categories = array( + array( 'term_id' => 123 ), + array( 'term_id' => 456 ), + array( 'term_id' => 789 ), + ); + update_option( 'wpcom_newsletter_categories', $categories ); + $result = Jetpack_Newsletter_Category_Helper::get_category_ids(); + $this->assertEquals( array( 123, 456, 789 ), $result ); + } + + /** + * Test get_category_ids() filters out non-numeric term_ids. + */ + public function test_get_category_ids_filters_invalid_term_ids() { + $categories = array( + array( 'term_id' => 123 ), + array( 'term_id' => 'not-a-number' ), + array( 'term_id' => 456 ), + ); + update_option( 'wpcom_newsletter_categories', $categories ); + $result = Jetpack_Newsletter_Category_Helper::get_category_ids(); + $this->assertEquals( array( 123, 456 ), $result ); + } + + /** + * Test get_category_ids() handles serialized data. + */ + public function test_get_category_ids_serialized_data() { + $categories = array( 123, 456 ); + update_option( 'wpcom_newsletter_categories', serialize( $categories ) ); + $result = Jetpack_Newsletter_Category_Helper::get_category_ids(); + $this->assertEquals( $categories, $result ); + } + + /** + * Test save_category_ids() returns false for empty array. + */ + public function test_save_category_ids_empty_array() { + $result = Jetpack_Newsletter_Category_Helper::save_category_ids( array() ); + $this->assertFalse( $result ); + } + + /** + * Test save_category_ids() formats array of integers correctly. + */ + public function test_save_category_ids_integer_array() { + $input = array( 123, 456, 789 ); + $expected = array( + array( 'term_id' => 123 ), + array( 'term_id' => 456 ), + array( 'term_id' => 789 ), + ); + $result = Jetpack_Newsletter_Category_Helper::save_category_ids( $input ); + $this->assertEquals( $expected, $result ); + + $saved_option = get_option( 'wpcom_newsletter_categories' ); + $this->assertEquals( $expected, $saved_option ); + } + + /** + * Test save_category_ids() handles string numeric values. + */ + public function test_save_category_ids_string_numeric() { + $input = array( '123', '456', '789' ); + $expected = array( + array( 'term_id' => 123 ), + array( 'term_id' => 456 ), + array( 'term_id' => 789 ), + ); + $result = Jetpack_Newsletter_Category_Helper::save_category_ids( $input ); + $this->assertEquals( $expected, $result ); + } + + /** + * Test save_category_ids() filters out non-numeric values. + */ + public function test_save_category_ids_filters_non_numeric() { + $input = array( 123, 'not-a-number', 456 ); + $expected = array( + array( 'term_id' => 123 ), + array( 'term_id' => 456 ), + ); + $result = Jetpack_Newsletter_Category_Helper::save_category_ids( $input ); + $this->assertEquals( $expected, $result ); + } + + /** + * Test save_category_ids() handles array of arrays with term_id. + */ + public function test_save_category_ids_term_id_array() { + $input = array( + array( 'term_id' => 123 ), + array( 'term_id' => 456 ), + array( 'term_id' => 789 ), + ); + + $result = Jetpack_Newsletter_Category_Helper::save_category_ids( $input ); + $this->assertEquals( $input, $result ); + } + + /** + * Test save_category_ids() returns false when no valid categories. + */ + public function test_save_category_ids_no_valid_categories() { + $input = array( 'not-a-number', 'also-not-a-number' ); + $result = Jetpack_Newsletter_Category_Helper::save_category_ids( $input ); + $this->assertFalse( $result ); + } + + /** + * Test integration between save and get methods. + */ + public function test_save_and_get_integration() { + $input = array( 123, 456, 789 ); + Jetpack_Newsletter_Category_Helper::save_category_ids( $input ); + $result = Jetpack_Newsletter_Category_Helper::get_category_ids(); + $this->assertEquals( $input, $result ); + } +}