diff --git a/php/class-admin.php b/php/class-admin.php index 70b2ebb3e..9bec1020b 100644 --- a/php/class-admin.php +++ b/php/class-admin.php @@ -488,16 +488,27 @@ protected function save_settings( $submission, $data ) { $analytics = $this->plugin->get_component( 'analytics' ); if ( $analytics ) { - $analytics->track( - 'settings_saved', - 'settings', - null, - array( - 'page' => $submission, - 'changed_keys' => $changed_keys, - ) + $params = array( + 'page' => $submission, + 'changed_keys' => $changed_keys, ); + // Values only for allowlisted feature settings; secrets + // and free text never leave the site. Read back after the + // save so the value reflects any save filters. + $feature_values = array(); + foreach ( $changed_keys as $key ) { + $normalized = $analytics->normalize_feature_value( $submission, $key, $this->settings->get_value( $key ) ); + if ( null !== $normalized ) { + $feature_values[ $key ] = $normalized; + } + } + if ( ! empty( $feature_values ) ) { + $params['changed_values'] = $feature_values; + } + + $analytics->track( 'settings_saved', 'settings', null, $params ); + if ( 'gallery' === $submission ) { $this->track_gallery_configured( $analytics, $data ); } diff --git a/php/class-analytics.php b/php/class-analytics.php index 61df3b29c..27d1f1ad5 100644 --- a/php/class-analytics.php +++ b/php/class-analytics.php @@ -65,6 +65,7 @@ public function __construct( Plugin $plugin ) { add_action( 'admin_enqueue_scripts', array( $this, 'enqueue_script_data' ) ); add_action( 'admin_init', array( $this, 'maybe_send_smoke_event' ) ); add_action( 'admin_init', array( $this, 'maybe_send_pending_activation' ) ); + add_action( 'admin_init', array( $this, 'maybe_send_feature_snapshot' ) ); add_action( 'cloudinary_uploaded_asset', array( $this, 'maybe_first_api_consumption' ), 10, 2 ); } @@ -75,6 +76,182 @@ public function __construct( Plugin $plugin ) { const LAST_ACTIVE = '_cloudinary_last_active'; const FIRST_API_FLAG = '_cloudinary_first_api_emitted'; + /** + * Option key marking the feature-config snapshot as sent (per cloud_name). + */ + const FEATURE_SNAPSHOT_FLAG = '_cloudinary_feature_snapshot_emitted'; + + /** + * Allowlist of feature settings whose values may be emitted, per settings + * page, mapped to how each value is normalised before it leaves the site. + * + * Anything not listed here is never sent as a value. Free-text fields are + * reduced to a presence boolean, since they can hold public IDs, overlay + * text or URLs. + * + * - bool: `on`/`off` toggle → true/false. + * - int: numeric field → int. + * - string: select/radio enum → string. + * - css_length: free-text CSS length → the length, or dropped when it is + * anything else. + * - presence: free text → whether it is non-empty. + * - crops: per-size crop map → number of sizes with a custom crop. + */ + const FEATURE_SETTINGS = array( + 'image_settings' => array( + 'image_delivery' => 'bool', + 'image_optimization' => 'bool', + 'image_format' => 'string', + 'image_quality' => 'string', + 'image_freeform' => 'presence', + 'svg_support' => 'bool', + 'crop_sizes' => 'crops', + ), + 'lazy_loading' => array( + 'use_lazy_load' => 'bool', + 'lazy_threshold' => 'css_length', + 'lazy_placeholder' => 'string', + 'lazy_animate' => 'bool', + 'dpr' => 'string', + ), + 'responsive' => array( + 'enable_breakpoints' => 'bool', + 'pixel_step' => 'int', + 'breakpoints' => 'int', + 'max_width' => 'int', + 'min_width' => 'int', + ), + 'video_settings' => array( + 'video_delivery' => 'bool', + 'video_player' => 'string', + 'adaptive_streaming' => 'bool', + 'adaptive_streaming_mode' => 'string', + 'video_controls' => 'bool', + 'video_loop' => 'bool', + 'video_autoplay_mode' => 'string', + 'video_optimization' => 'bool', + 'video_format' => 'string', + 'video_quality' => 'string', + 'video_freeform' => 'presence', + ), + ); + + /** + * Normalises a feature setting value for emission. + * + * @param string $page The settings page slug the setting belongs to. + * @param string $key The setting slug. + * @param mixed $value The raw setting value. + * + * @return bool|int|string|null The normalised value, or null when the + * setting is not allowlisted for that page or + * its value is missing or unusable. + */ + public function normalize_feature_value( $page, $key, $value ) { + if ( ! isset( self::FEATURE_SETTINGS[ $page ][ $key ] ) || null === $value ) { + return null; + } + + switch ( self::FEATURE_SETTINGS[ $page ][ $key ] ) { + case 'bool': + return 'on' === $value || true === $value; + case 'int': + return is_numeric( $value ) ? (int) $value : null; + case 'css_length': + $value = is_string( $value ) ? trim( $value ) : ''; + + return preg_match( '/^\d+(\.\d+)?(px|%|r?em|vh|vw)?$/', $value ) ? $value : null; + case 'presence': + return is_string( $value ) && '' !== trim( $value ); + case 'crops': + // Each size maps to '' (default crop), '--' (crop disabled) or + // a custom transformation string. + if ( ! is_array( $value ) ) { + return 0; + } + + return count( + array_filter( + $value, + static function ( $crop ) { + return is_string( $crop ) && '' !== trim( $crop ) && '--' !== $crop; + } + ) + ); + default: + return is_scalar( $value ) ? sanitize_text_field( (string) $value ) : null; + } + } + + /** + * Emits the one-time `feature_config_snapshot` adoption baseline. + * + * Captures the full state of the delivery features, including those left + * at their defaults, which `settings_saved` never reports. Sent on the + * first admin page load by an administrator once the site is connected, + * then suppressed per cloud_name. That covers wizard setups, sites + * connected through the `CLOUDINARY_CONNECTION_STRING` constant, existing + * installs upgrading to this version, and account switches. + * + * @return void + */ + public function maybe_send_feature_snapshot() { + // `admin_init` also runs for admin-ajax requests, including logged-out + // ones, and for non-admin users visiting their profile. + if ( wp_doing_ajax() || ! Utils::user_can( 'analytics', 'manage_options' ) || ! $this->is_enabled() ) { + return; + } + + $connect = $this->plugin->get_component( 'connect' ); + if ( ! $connect || ! $connect->is_connected() ) { + return; + } + + $cloud = (string) $connect->get_cloud_name(); + if ( '' === $cloud ) { + return; + } + + // Autoloaded, so this check costs no query on later admin loads. + $sent = get_option( self::FEATURE_SNAPSHOT_FLAG ); + if ( $sent === $cloud ) { + return; + } + + if ( false === $sent ) { + // Only one of several concurrent first requests sends: add_option() + // upserts, and a request writing the same cloud name as an earlier + // one changes no row, so add_option() returns false for it. + if ( ! add_option( self::FEATURE_SNAPSHOT_FLAG, $cloud, '', true ) ) { + return; + } + } else { + // Account switched. + update_option( self::FEATURE_SNAPSHOT_FLAG, $cloud, true ); + } + + $this->track( 'feature_config_snapshot', 'features', null, $this->get_feature_snapshot() ); + } + + /** + * Reads the current, normalised value of every allowlisted feature setting. + * + * @return array + */ + public function get_feature_snapshot() { + $snapshot = array(); + foreach ( self::FEATURE_SETTINGS as $page => $settings ) { + foreach ( array_keys( $settings ) as $key ) { + $value = $this->normalize_feature_value( $page, $key, $this->plugin->settings->get_value( $key ) ); + if ( null !== $value ) { + $snapshot[ $key ] = $value; + } + } + } + + return $snapshot; + } + /** * Records the activation type on plugin activation (funnel step 1). * diff --git a/php/class-deactivation.php b/php/class-deactivation.php index 04cf82d69..a60fa7ea0 100644 --- a/php/class-deactivation.php +++ b/php/class-deactivation.php @@ -630,6 +630,8 @@ protected function cleanup_options() { '_cld_disable_http_upload', Report::REPORT_KEY, Media::GLOBAL_VIDEO_TRANSFORMATIONS, + Analytics::FIRST_API_FLAG, + Analytics::FEATURE_SNAPSHOT_FLAG, self::CLEANING_KEY, ) ); diff --git a/tests/e2e/features-analytics.spec.js b/tests/e2e/features-analytics.spec.js index 16e83f6f8..24ada2df3 100644 --- a/tests/e2e/features-analytics.spec.js +++ b/tests/e2e/features-analytics.spec.js @@ -6,7 +6,7 @@ const { test, expect } = require( './fixtures' ); /** * Internal dependencies */ -const { wpEvalFile } = require( './utils/wizard' ); +const { wpCli, wpEvalFile } = require( './utils/wizard' ); const { fakeCloudinaryConnected } = require( './utils/connection' ); const { clearAnalyticsEvents, @@ -82,3 +82,35 @@ test.describe( 'Extensions & gallery analytics', () => { expect( events[ 0 ].media_count ).toBe( 2 ); } ); } ); + +// @serial: the snapshot's one-time flag is shared by the whole site, so a +// parallel worker's admin load could send it into that worker's log instead. +// The sending rules are covered by PHPUnit; this checks the admin wiring. +test.describe( 'Feature config snapshot', { tag: '@serial' }, () => { + test.beforeEach( () => { + fakeCloudinaryConnected(); + // Earlier admin loads already sent the snapshot for this cloud_name, + // so reset the one-time flag to simulate a fresh setup. + wpCli( [ 'option', 'delete', '_cloudinary_feature_snapshot_emitted' ] ); + clearAnalyticsEvents(); + } ); + + test( 'the first admin load once connected emits feature_config_snapshot once', async ( { + admin, + } ) => { + await admin.visitAdminPage( 'admin.php', 'page=cloudinary' ); + await admin.visitAdminPage( 'admin.php', 'page=cloudinary' ); + + const events = findAnalyticsEvents( + readAnalyticsEvents(), + 'feature_config_snapshot' + ); + expect( events.length ).toBe( 1 ); + expect( events[ 0 ].event_category ).toBe( 'features' ); + + // Default values are reported even though they were never saved. + expect( typeof events[ 0 ].use_lazy_load ).toBe( 'boolean' ); + expect( typeof events[ 0 ].pixel_step ).toBe( 'number' ); + expect( typeof events[ 0 ].image_format ).toBe( 'string' ); + } ); +} ); diff --git a/tests/e2e/settings-analytics.spec.js b/tests/e2e/settings-analytics.spec.js index 27e294816..4ea7ff984 100644 --- a/tests/e2e/settings-analytics.spec.js +++ b/tests/e2e/settings-analytics.spec.js @@ -79,8 +79,72 @@ test.describe( 'Settings & navigation analytics', () => { expect( savedEvents.length ).toBe( 1 ); expect( savedEvents[ 0 ].page ).toBe( 'image_settings' ); expect( savedEvents[ 0 ].changed_keys ).toContain( 'image_quality' ); + expect( savedEvents[ 0 ].changed_values.image_quality ).toBe( + nextValue + ); } ); + // One feature page each, flipping a field no other spec writes to, so + // parallel workers can't change it between this test's read and save. + for ( const { page: slug, tag, field, values, expected } of [ + { + page: 'video_settings', + tag: 'select', + field: 'video_quality', + values: [ 'auto', '80' ], + expected: ( value ) => value, + }, + { + page: 'lazy_loading', + tag: 'select', + field: 'dpr', + values: [ '2X', 'max' ], + expected: ( value ) => value, + }, + { + page: 'responsive', + tag: 'input', + field: 'breakpoints', + values: [ '10', '12' ], + expected: ( value ) => Number( value ), + }, + ] ) { + test( `saving the ${ slug } page emits settings_saved with its values`, async ( { + admin, + page, + } ) => { + await admin.visitAdminPage( + 'admin.php', + `page=cloudinary_${ slug }` + ); + + // Set through the DOM: some of these fields are hidden until + // their feature is toggled on, but are still submitted. + const input = page.locator( `${ tag }[name$="[${ field }]"]` ); + const current = await input.inputValue(); + const nextValue = + values[ 0 ] === current ? values[ 1 ] : values[ 0 ]; + await input.evaluate( ( el, value ) => { + el.value = value; + el.dispatchEvent( new Event( 'input', { bubbles: true } ) ); + el.dispatchEvent( new Event( 'change', { bubbles: true } ) ); + }, nextValue ); + + await page.locator( SEL.saveButton ).click(); + await page.waitForLoadState( 'networkidle' ); + + const savedEvents = findAnalyticsEvents( + readAnalyticsEvents(), + 'settings_saved' + ); + expect( savedEvents.length ).toBe( 1 ); + expect( savedEvents[ 0 ].page ).toBe( slug ); + expect( savedEvents[ 0 ].changed_values[ field ] ).toBe( + expected( nextValue ) + ); + } ); + } + test( 'dismissing an admin notice emits notice_dismissed', async ( { admin, page, diff --git a/tests/phpunit/tests/test-analytics-feature-snapshot.php b/tests/phpunit/tests/test-analytics-feature-snapshot.php new file mode 100644 index 000000000..bdac8e62b --- /dev/null +++ b/tests/phpunit/tests/test-analytics-feature-snapshot.php @@ -0,0 +1,237 @@ +analytics = $plugin->get_component( 'analytics' ); + + $this->original_connected = $plugin->settings->get_param( 'connected', null ); + $this->original_cloud_name = $plugin->get_component( 'connect' )->get_cloud_name(); + $this->connect_as( 'first-cloud' ); + + delete_option( Analytics::FEATURE_SNAPSHOT_FLAG ); + wp_set_current_user( self::factory()->user->create( array( 'role' => 'administrator' ) ) ); + + $this->sent = array(); + add_filter( 'pre_http_request', array( $this, 'capture_event' ), 1, 3 ); + } + + /** + * Restores the connection state. + * + * @return void + */ + public function tear_down() { + remove_filter( 'pre_http_request', array( $this, 'capture_event' ), 1 ); + + $plugin = get_plugin_instance(); + $plugin->settings->set_param( 'connected', $this->original_connected ); + $plugin->get_component( 'connect' )->set_credentials( array( 'cloud_name' => $this->original_cloud_name ) ); + + parent::tear_down(); + } + + /** + * Captures analytics events instead of sending them. + * + * @param false|array $preempt Whether to preempt the request. + * @param array $args The request arguments. + * @param string $url The request URL. + * + * @return false|array + */ + public function capture_event( $preempt, $args, $url ) { + if ( CLOUDINARY_ENDPOINTS_ANALYTICS !== $url ) { + return $preempt; + } + $this->sent[] = json_decode( $args['body'], true ); + + return array( + 'headers' => array(), + 'body' => '', + 'response' => array( + 'code' => 200, + 'message' => 'OK', + ), + 'cookies' => array(), + ); + } + + /** + * Marks the plugin as connected to the given cloud. + * + * @param string $cloud_name The cloud name. + * + * @return void + */ + protected function connect_as( $cloud_name ) { + $plugin = get_plugin_instance(); + $plugin->settings->set_param( 'connected', true ); + $plugin->get_component( 'connect' )->set_credentials( array( 'cloud_name' => $cloud_name ) ); + } + + /** + * Returns the captured snapshot events. + * + * @return array[] + */ + protected function snapshots() { + return array_values( + array_filter( + $this->sent, + static function ( $event ) { + return 'feature_config_snapshot' === $event['event_name']; + } + ) + ); + } + + /** + * The snapshot is sent once per account, with the feature values. + * + * @return void + */ + public function test_sends_once_per_account() { + $this->analytics->maybe_send_feature_snapshot(); + $this->analytics->maybe_send_feature_snapshot(); + + $snapshots = $this->snapshots(); + $this->assertCount( 1, $snapshots ); + $this->assertSame( 'features', $snapshots[0]['event_category'] ); + $this->assertIsBool( $snapshots[0]['use_lazy_load'] ); + $this->assertArrayNotHasKey( 'cloudinary_url', $snapshots[0] ); + $this->assertSame( 'first-cloud', get_option( Analytics::FEATURE_SNAPSHOT_FLAG ) ); + } + + /** + * Switching to another account sends a new snapshot. + * + * @return void + */ + public function test_sends_again_after_an_account_switch() { + $this->analytics->maybe_send_feature_snapshot(); + $this->connect_as( 'second-cloud' ); + $this->analytics->maybe_send_feature_snapshot(); + + $this->assertCount( 2, $this->snapshots() ); + $this->assertSame( 'second-cloud', get_option( Analytics::FEATURE_SNAPSHOT_FLAG ) ); + } + + /** + * Nothing is sent before the site is connected. + * + * @return void + */ + public function test_not_sent_while_disconnected() { + get_plugin_instance()->settings->set_param( 'connected', false ); + $this->analytics->maybe_send_feature_snapshot(); + + $this->assertCount( 0, $this->snapshots() ); + $this->assertFalse( get_option( Analytics::FEATURE_SNAPSHOT_FLAG ) ); + } + + /** + * With analytics disabled nothing is sent, and the snapshot is still sent + * once they are enabled again. + * + * @return void + */ + public function test_deferred_while_analytics_are_disabled() { + add_filter( 'cloudinary_analytics_enabled', '__return_false' ); + $this->analytics->maybe_send_feature_snapshot(); + remove_filter( 'cloudinary_analytics_enabled', '__return_false' ); + + $this->assertCount( 0, $this->snapshots() ); + + $this->analytics->maybe_send_feature_snapshot(); + $this->assertCount( 1, $this->snapshots() ); + } + + /** + * Admin loads by non-administrators don't send the snapshot. + * + * @return void + */ + public function test_not_sent_for_non_administrators() { + wp_set_current_user( self::factory()->user->create( array( 'role' => 'subscriber' ) ) ); + $this->analytics->maybe_send_feature_snapshot(); + + $this->assertCount( 0, $this->snapshots() ); + } + + /** + * Admin-ajax requests don't send the snapshot. + * + * @return void + */ + public function test_not_sent_during_ajax_requests() { + add_filter( 'wp_doing_ajax', '__return_true' ); + $this->analytics->maybe_send_feature_snapshot(); + remove_filter( 'wp_doing_ajax', '__return_true' ); + + $this->assertCount( 0, $this->snapshots() ); + } + + /** + * When another request already added the flag, this one doesn't send. + * + * @return void + */ + public function test_only_the_request_that_adds_the_flag_sends() { + // Simulates a concurrent request having added the flag after this one + // read it: the add_option() gate must fail and nothing be sent. + add_filter( 'pre_option_' . Analytics::FEATURE_SNAPSHOT_FLAG, '__return_false' ); + add_option( Analytics::FEATURE_SNAPSHOT_FLAG, 'first-cloud' ); + $this->analytics->maybe_send_feature_snapshot(); + remove_filter( 'pre_option_' . Analytics::FEATURE_SNAPSHOT_FLAG, '__return_false' ); + + $this->assertCount( 0, $this->snapshots() ); + } +} diff --git a/tests/phpunit/tests/test-analytics-feature-values.php b/tests/phpunit/tests/test-analytics-feature-values.php new file mode 100644 index 000000000..f205e0f83 --- /dev/null +++ b/tests/phpunit/tests/test-analytics-feature-values.php @@ -0,0 +1,134 @@ +analytics = get_plugin_instance()->get_component( 'analytics' ); + } + + /** + * Settings outside the allowlist, including secrets, are never emitted. + * + * @return void + */ + public function test_non_allowlisted_settings_are_dropped() { + $this->assertNull( $this->analytics->normalize_feature_value( 'connect', 'cloudinary_url', 'cloudinary://key:secret@demo' ) ); + $this->assertNull( $this->analytics->normalize_feature_value( 'connect', 'cloudinary_folder', 'my-folder' ) ); + $this->assertNull( $this->analytics->normalize_feature_value( 'lazy_loading', 'lazy_custom_color', 'rgba(0,0,0,1)' ) ); + } + + /** + * A setting is only allowlisted on its own settings page. + * + * @return void + */ + public function test_settings_are_allowlisted_per_page() { + $this->assertSame( 'avif', $this->analytics->normalize_feature_value( 'image_settings', 'image_format', 'avif' ) ); + $this->assertNull( $this->analytics->normalize_feature_value( 'video_settings', 'image_format', 'avif' ) ); + } + + /** + * A missing value is left out rather than reported as off. + * + * @return void + */ + public function test_missing_values_are_dropped() { + $this->assertNull( $this->analytics->normalize_feature_value( 'lazy_loading', 'use_lazy_load', null ) ); + $this->assertNull( $this->analytics->normalize_feature_value( 'image_settings', 'image_freeform', null ) ); + $this->assertNull( $this->analytics->normalize_feature_value( 'image_settings', 'crop_sizes', null ) ); + } + + /** + * On/off toggles become booleans. + * + * @return void + */ + public function test_toggles_become_booleans() { + $this->assertTrue( $this->analytics->normalize_feature_value( 'lazy_loading', 'use_lazy_load', 'on' ) ); + $this->assertFalse( $this->analytics->normalize_feature_value( 'lazy_loading', 'use_lazy_load', 'off' ) ); + $this->assertFalse( $this->analytics->normalize_feature_value( 'video_settings', 'adaptive_streaming', '' ) ); + } + + /** + * Numeric fields become ints, and unreadable ones are dropped. + * + * @return void + */ + public function test_numbers_become_ints() { + $this->assertSame( 200, $this->analytics->normalize_feature_value( 'responsive', 'pixel_step', '200' ) ); + $this->assertNull( $this->analytics->normalize_feature_value( 'responsive', 'breakpoints', '' ) ); + } + + /** + * Enum values pass through as strings. + * + * @return void + */ + public function test_enums_stay_strings() { + $this->assertSame( 'avif', $this->analytics->normalize_feature_value( 'image_settings', 'image_format', 'avif' ) ); + $this->assertSame( 'auto:good', $this->analytics->normalize_feature_value( 'image_settings', 'image_quality', 'auto:good' ) ); + } + + /** + * The lazy-loading threshold is only sent when it is a CSS length. + * + * @return void + */ + public function test_lazy_threshold_only_keeps_css_lengths() { + $this->assertSame( '100px', $this->analytics->normalize_feature_value( 'lazy_loading', 'lazy_threshold', ' 100px ' ) ); + $this->assertSame( '1.5em', $this->analytics->normalize_feature_value( 'lazy_loading', 'lazy_threshold', '1.5em' ) ); + $this->assertSame( '20%', $this->analytics->normalize_feature_value( 'lazy_loading', 'lazy_threshold', '20%' ) ); + $this->assertNull( $this->analytics->normalize_feature_value( 'lazy_loading', 'lazy_threshold', 'jane@example.com' ) ); + $this->assertNull( $this->analytics->normalize_feature_value( 'lazy_loading', 'lazy_threshold', '' ) ); + } + + /** + * Free-text transformations are reduced to a presence flag. + * + * @return void + */ + public function test_freeform_is_reduced_to_presence() { + $this->assertTrue( $this->analytics->normalize_feature_value( 'image_settings', 'image_freeform', 'l_text:Arial_20:Jane Doe' ) ); + $this->assertFalse( $this->analytics->normalize_feature_value( 'video_settings', 'video_freeform', ' ' ) ); + } + + /** + * Crop sizes are summarised as the number of sizes with a custom crop. + * + * @return void + */ + public function test_crop_sizes_count_custom_crops() { + $crops = array( + '150x150' => 'c_thumb,g_face', + '300x300' => '', + '1024x768' => '--', + '768x0' => 'c_fill,g_auto', + ); + + $this->assertSame( 2, $this->analytics->normalize_feature_value( 'image_settings', 'crop_sizes', $crops ) ); + $this->assertSame( 0, $this->analytics->normalize_feature_value( 'image_settings', 'crop_sizes', '' ) ); + } +}