From b9db88535f714dc98de3c07ccb9913327bf27fb7 Mon Sep 17 00:00:00 2001 From: Bogdan Ungureanu Date: Thu, 30 Jan 2025 14:56:01 +0200 Subject: [PATCH] RDV: Fix a performance issue and assign a12s treatment (#41378) * RDV: Fix a performance issue and assign a12s treatment Fixed an issue that caused to make an API request to ExPlat on each page request for users that were in control or if they were a12s. This PR also forces treatment to all a12s on both Simple and Atomic sites. On Simple sites, we use is_automattician since that's more accurate than relying on proxy. Although not needed for performance, the result is also cached in the user option so that a12s to be able to use the escape hatch. On Atomic sites, the logic is a bit more complicated. The switch for a12s is handled with two conditions: Calypo: if there's an admin_menu_a11n query param (which is passed by WPCOM admin-menu API mapper WP-Admin: Rely on the is proxy check. Since this PR comes after a revert, we also need to change the caching key to pull again the experiment assignment from ExPlat. * Restrict the parameter to the admin-menu endpoint and fix a linting issue --- .../jetpack-mu-wpcom/.phan/config.php | 1 + .../changelog/fix-rdv-caching-and-a12s | 4 +++ .../wpcom-admin-interface.php | 33 +++++++++++++++++-- 3 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 projects/packages/jetpack-mu-wpcom/changelog/fix-rdv-caching-and-a12s diff --git a/projects/packages/jetpack-mu-wpcom/.phan/config.php b/projects/packages/jetpack-mu-wpcom/.phan/config.php index 4d75c36cf968d..7ff30780d5d8c 100644 --- a/projects/packages/jetpack-mu-wpcom/.phan/config.php +++ b/projects/packages/jetpack-mu-wpcom/.phan/config.php @@ -26,6 +26,7 @@ __DIR__ . '/../../../plugins/jetpack/modules/custom-css/custom-css.php', // class Jetpack_Custom_CSS_Enhancements __DIR__ . '/../../../plugins/jetpack/class-jetpack-stats-dashboard-widget.php', // class Jetpack_Stats_Dashboard_Widget __DIR__ . '/../../../plugins/wpcomsh/wpcomsh.php', // function wpcomsh_record_tracks_event + __DIR__ . '/../../../plugins/wpcomsh/support-session.php', ), 'exclude_analysis_directory_list' => array( 'src/features/custom-css/csstidy/', diff --git a/projects/packages/jetpack-mu-wpcom/changelog/fix-rdv-caching-and-a12s b/projects/packages/jetpack-mu-wpcom/changelog/fix-rdv-caching-and-a12s new file mode 100644 index 0000000000000..a2789e233a15f --- /dev/null +++ b/projects/packages/jetpack-mu-wpcom/changelog/fix-rdv-caching-and-a12s @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Caching fixes for RDV project diff --git a/projects/packages/jetpack-mu-wpcom/src/features/wpcom-admin-interface/wpcom-admin-interface.php b/projects/packages/jetpack-mu-wpcom/src/features/wpcom-admin-interface/wpcom-admin-interface.php index 4e037bb5755cf..5e4d3928df0c3 100644 --- a/projects/packages/jetpack-mu-wpcom/src/features/wpcom-admin-interface/wpcom-admin-interface.php +++ b/projects/packages/jetpack-mu-wpcom/src/features/wpcom-admin-interface/wpcom-admin-interface.php @@ -390,7 +390,7 @@ function wpcom_show_admin_interface_notice() { /** * Option to force and cache the Remove duplicate Views experiment assigned variation. */ -const RDV_EXPERIMENT_FORCE_ASSIGN_OPTION = 'remove_duplicate_views_experiment_assignment'; +const RDV_EXPERIMENT_FORCE_ASSIGN_OPTION = 'remove_duplicate_views_experiment_assignment_160125'; /** * Check if the duplicate views experiment is enabled. @@ -409,6 +409,11 @@ function wpcom_is_duplicate_views_experiment_enabled() { $variation = get_user_option( RDV_EXPERIMENT_FORCE_ASSIGN_OPTION, get_current_user_id() ); + /** + * We cache it for both AT and Simple because we want to give a12s to be able to switch between variations for their accounts - this can be useful during support. + * + * If we don't cache it, the is_automattician conditions will force treatment every time. + */ if ( false !== $variation ) { $is_enabled = 'treatment' === $variation; return $is_enabled; @@ -417,9 +422,33 @@ function wpcom_is_duplicate_views_experiment_enabled() { if ( ( new Host() )->is_wpcom_simple() ) { \ExPlat\assign_current_user( $aa_test_name ); $is_enabled = 'treatment' === \ExPlat\assign_current_user( $experiment_name ); + + if ( is_automattician() ) { + $is_enabled = true; + update_user_option( get_current_user_id(), RDV_EXPERIMENT_FORCE_ASSIGN_OPTION, 'treatment', true ); + } + return $is_enabled; } + $is_proxy_atomic = defined( 'AT_PROXIED_REQUEST' ) && AT_PROXIED_REQUEST; + $is_support_session = WPCOMSH_Support_Session_Detect::is_probably_support_session(); + $admin_menu_is_a11n = isset( $_GET['admin_menu_is_a11n'] ) && function_exists( 'wpcomsh_is_admin_menu_api_request' ) && wpcomsh_is_admin_menu_api_request(); + + /** + * This handles two contexts: Calypso and WP-Admin. + * + * Calypso: WPCOM admin-menu API endpoint mapper sends a "admin_menu_is_a11n" param for a12s. If the param exists, then we'll switch to treatment. + * WP-Admin: We check if the user is proxied and if it's not in a support session. + */ + + if ( $admin_menu_is_a11n || ( $is_proxy_atomic && ! $is_support_session ) ) { + update_user_option( get_current_user_id(), RDV_EXPERIMENT_FORCE_ASSIGN_OPTION, 'treatment', true ); + $is_enabled = true; + + return true; + } + if ( ! ( new Jetpack_Connection() )->is_user_connected() ) { $is_enabled = false; return $is_enabled; @@ -451,7 +480,7 @@ function wpcom_is_duplicate_views_experiment_enabled() { $data = json_decode( wp_remote_retrieve_body( $response ), true ); - if ( isset( $data['variations'] ) && isset( $data['variations'][ $experiment_name ] ) ) { + if ( isset( $data['variations'] ) && array_key_exists( $experiment_name, $data['variations'] ) ) { $variation = $data['variations'][ $experiment_name ]; update_user_option( get_current_user_id(), RDV_EXPERIMENT_FORCE_ASSIGN_OPTION, $variation, true );