diff --git a/classes/Visualizer/Module/Admin.php b/classes/Visualizer/Module/Admin.php index e003f2e6..e7ef36e5 100644 --- a/classes/Visualizer/Module/Admin.php +++ b/classes/Visualizer/Module/Admin.php @@ -81,7 +81,6 @@ public function __construct( Visualizer_Plugin $plugin ) { $this->_addFilter( 'media_view_strings', 'setupMediaViewStrings' ); $this->_addFilter( 'plugin_action_links', 'getPluginActionLinks', 10, 2 ); $this->_addFilter( 'plugin_row_meta', 'getPluginMetaLinks', 10, 2 ); - $this->_addFilter( 'visualizer_logger_data', 'getLoggerData' ); $this->_addFilter( 'visualizer_feedback_review_trigger', 'feedbackReviewTrigger' ); $this->_addFilter( 'themeisle_sdk_blackfriday_data', 'add_black_friday_data' ); diff --git a/classes/Visualizer/Module/Setup.php b/classes/Visualizer/Module/Setup.php index f0cc81f4..2ef61c60 100644 --- a/classes/Visualizer/Module/Setup.php +++ b/classes/Visualizer/Module/Setup.php @@ -113,7 +113,7 @@ public function getUsage( $data, $meta_keys = array() ) { $lib = get_post_meta( $chart_id, Visualizer_Plugin::CF_CHART_LIBRARY, true ); $charts['library'][ $lib ] = isset( $charts['library'][ $lib ] ) ? $charts['library'][ $lib ] + 1 : 1; $settings = get_post_meta( $chart_id, Visualizer_Plugin::CF_SETTINGS, true ); - if ( array_key_exists( 'manual', $settings ) && ! empty( $settings['manual'] ) ) { + if ( is_array( $settings ) && ! empty( $settings['manual'] ) ) { $charts['manual_config'] = $charts['manual_config'] + 1; } @@ -124,7 +124,7 @@ public function getUsage( $data, $meta_keys = array() ) { if ( Visualizer_Module::is_pro() ) { $permissions = get_post_meta( $chart_id, Visualizer_Pro::CF_PERMISSIONS, true ); - if ( empty( $permissions ) ) { + if ( ! is_array( $permissions ) || empty( $permissions['permissions'] ) || ! is_array( $permissions['permissions'] ) ) { continue; } $permissions = $permissions['permissions']; @@ -132,7 +132,7 @@ public function getUsage( $data, $meta_keys = array() ) { foreach ( $default_perms as $key => $val ) { if ( ! is_array( $val ) && ! is_null( $val ) && isset( $permissions[ $key ] ) && $permissions[ $key ] !== $val ) { $customized = true; - } elseif ( is_array( $val ) && ! is_null( $val ) && isset( $permissions[ $key ] ) && count( $permissions[ $key ] ) !== count( $val ) ) { + } elseif ( is_array( $val ) && ! is_null( $val ) && isset( $permissions[ $key ] ) && is_array( $permissions[ $key ] ) && count( $permissions[ $key ] ) !== count( $val ) ) { $customized = true; } } diff --git a/tests/e2e/config/mu-plugins/plant-chart-settings.php b/tests/e2e/config/mu-plugins/plant-chart-settings.php index 5222e9de..5a07d4a0 100644 --- a/tests/e2e/config/mu-plugins/plant-chart-settings.php +++ b/tests/e2e/config/mu-plugins/plant-chart-settings.php @@ -38,5 +38,21 @@ function () { }, ) ); + + // Runs the SDK usage logger on demand, so specs can verify it + // tolerates whatever chart meta they planted (issue #1359). + register_rest_route( + 'visualizer-e2e/v1', + '/usage', + array( + 'methods' => 'GET', + 'permission_callback' => function () { + return current_user_can( 'manage_options' ); + }, + 'callback' => function () { + return apply_filters( 'visualizer_logger_data', array() ); + }, + ) + ); } ); diff --git a/tests/e2e/specs/usage-logger.spec.js b/tests/e2e/specs/usage-logger.spec.js new file mode 100644 index 00000000..d502e15b --- /dev/null +++ b/tests/e2e/specs/usage-logger.spec.js @@ -0,0 +1,69 @@ +/** + * WordPress dependencies + */ +const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' ); + +/** + * Internal dependencies + */ +const { deleteAllCharts } = require( '../utils/common' ); + +/** + * Regression tests for https://github.com/Codeinwp/visualizer/issues/1359 + * + * A published chart whose `visualizer-settings` meta is a string (instead of + * the sanitized settings array) crashed `Visualizer_Module_Setup::getUsage()` + * with a PHP 8 TypeError, aborting the whole SDK usage collection request. + * The logger must tolerate such charts and still report the others. + */ +test.describe( 'Usage logger', () => { + let corruptedId; + let manualId; + + test.beforeAll( async ( { requestUtils } ) => { + // The assertions below count charts, so start from a clean library. + await deleteAllCharts( requestUtils ); + + // A chart whose settings meta is a corrupted string value. + const corrupted = await requestUtils.rest( { + method: 'POST', + path: '/wp/v2/visualizer', + data: { title: 'Corrupted settings chart', status: 'publish' }, + } ); + corruptedId = corrupted.id; + await requestUtils.rest( { + method: 'POST', + path: `/visualizer-e2e/v1/chart-settings/${ corruptedId }`, + data: { settings: 'corrupted string settings' }, + } ); + + // A healthy chart with a manual configuration, which must still be counted. + const manual = await requestUtils.rest( { + method: 'POST', + path: '/wp/v2/visualizer', + data: { title: 'Manual config chart', status: 'publish' }, + } ); + manualId = manual.id; + await requestUtils.rest( { + method: 'POST', + path: `/visualizer-e2e/v1/chart-settings/${ manualId }`, + data: { settings: { manual: '{"colors": ["#000"]}' } }, + } ); + } ); + + test.afterAll( async ( { requestUtils } ) => { + for ( const id of [ corruptedId, manualId ] ) { + if ( id ) { + await requestUtils.rest( { method: 'DELETE', path: `/wp/v2/visualizer/${ id }`, params: { force: true } } ); + } + } + } ); + + test( 'survives a chart whose settings meta is a string', async ( { requestUtils } ) => { + // Before the fix this request died with a TypeError (HTTP 500). + const usage = await requestUtils.rest( { method: 'GET', path: '/visualizer-e2e/v1/usage' } ); + + expect( usage.manual_config ).toBe( 1 ); + expect( Object.values( usage.types ).reduce( ( a, b ) => a + b, 0 ) ).toBe( 2 ); + } ); +} ); diff --git a/tests/test-usage-logger.php b/tests/test-usage-logger.php new file mode 100644 index 00000000..2abf25a9 --- /dev/null +++ b/tests/test-usage-logger.php @@ -0,0 +1,107 @@ +post->create( + array( + 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, + 'post_status' => 'publish', + 'post_content' => wp_slash( serialize( array( array( 'Label' ), array( 'Value' ) ) ) ), + ) + ); + update_post_meta( $chart_id, Visualizer_Plugin::CF_CHART_TYPE, 'line' ); + update_post_meta( $chart_id, Visualizer_Plugin::CF_SETTINGS, $settings ); + return $chart_id; + } + + /** + * A chart whose settings meta is a string must not abort usage collection. + */ + public function test_string_settings_meta_does_not_crash_logger() { + $this->create_chart( 'corrupted string settings' ); + + $usage = apply_filters( 'visualizer_logger_data', array() ); + + $this->assertIsArray( $usage ); + $this->assertSame( 0, $usage['manual_config'] ); + } + + /** + * A chart with no settings meta at all must not abort usage collection. + */ + public function test_missing_settings_meta_does_not_crash_logger() { + $chart_id = $this->create_chart( array() ); + delete_post_meta( $chart_id, Visualizer_Plugin::CF_SETTINGS ); + + $usage = apply_filters( 'visualizer_logger_data', array() ); + + $this->assertIsArray( $usage ); + $this->assertSame( 0, $usage['manual_config'] ); + } + + /** + * On pro, permission meta that is not shaped like a map of arrays must not + * abort usage collection — a string entry fatals on count(), and a nested + * object fatals on the array offset read. + */ + public function test_malformed_permissions_meta_does_not_crash_logger() { + // The stub stays defined for the rest of the PHPUnit process. That only + // affects code gating on class_exists( 'Visualizer_Pro' ) — the legacy + // license fallback in proFeaturesEnabled() — which no test exercises. + if ( ! class_exists( 'Visualizer_Pro' ) ) { + eval( 'class Visualizer_Pro { const CF_PERMISSIONS = "visualizer-permissions"; }' ); + } + + $chart_id = $this->create_chart( array() ); + update_post_meta( + $chart_id, + Visualizer_Pro::CF_PERMISSIONS, + array( 'permissions' => array( 'edit-specific' => 'administrator' ) ) + ); + + $object_chart_id = $this->create_chart( array() ); + update_post_meta( $object_chart_id, Visualizer_Pro::CF_PERMISSIONS, array( 'permissions' => new stdClass() ) ); + + add_filter( 'visualizer_is_pro', '__return_true' ); + $usage = apply_filters( 'visualizer_logger_data', array() ); + remove_filter( 'visualizer_is_pro', '__return_true' ); + + $this->assertIsArray( $usage ); + $this->assertSame( 0, $usage['permissions'] ); + } + + /** + * Valid array settings still count manual configurations. + */ + public function test_manual_config_still_counted_for_array_settings() { + $this->create_chart( array( 'manual' => '{"colors": ["#000"]}' ) ); + $this->create_chart( 'corrupted string settings' ); + + $usage = apply_filters( 'visualizer_logger_data', array() ); + + $this->assertSame( 1, $usage['manual_config'] ); + $this->assertSame( 2, $usage['types']['line'] ); + } +}