Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RDV: Respect overriding preference for Untangling Calypso Settings #99388

Merged
merged 13 commits into from
Feb 10, 2025
27 changes: 5 additions & 22 deletions client/controller/index.web.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@ import { RouteProvider } from 'calypso/components/route';
import Layout from 'calypso/layout';
import LayoutLoggedOut from 'calypso/layout/logged-out';
import { isE2ETest } from 'calypso/lib/e2e';
import { loadExperimentAssignment } from 'calypso/lib/explat';
import { navigate } from 'calypso/lib/navigate';
import { createAccountUrl, login } from 'calypso/lib/paths';
import { CalypsoReactQueryDevtools } from 'calypso/lib/react-query-devtools-helper';
import { getIsRemoveDuplicateViewsExperimentEnabled } from 'calypso/lib/remove-duplicate-views-experiment';
import { isRemoveDuplicateViewsExperimentEnabled } from 'calypso/lib/remove-duplicate-views-experiment';
import { addQueryArgs, getSiteFragment } from 'calypso/lib/route';
import {
getProductSlugFromContext,
Expand All @@ -32,7 +31,6 @@ import {
getImmediateLoginEmail,
getImmediateLoginLocale,
} from 'calypso/state/immediate-login/selectors';
import { getPreference } from 'calypso/state/preferences/selectors';
import { canCurrentUser } from 'calypso/state/selectors/can-current-user';
import { getSiteAdminUrl, getSiteHomeUrl, getSiteOption } from 'calypso/state/sites/selectors';
import { setSelectedSiteId } from 'calypso/state/ui/actions/set-sites.js';
Expand Down Expand Up @@ -320,9 +318,8 @@ export async function redirectToHostingPromoIfNotAtomic( context, next ) {

if ( ! isAtomicSite || site.plan?.expired ) {
// Keep the user within the Settings tab
const isRemoveDuplicateViewsExperimentEnabled =
await getIsRemoveDuplicateViewsExperimentEnabled();
if ( isRemoveDuplicateViewsExperimentEnabled ) {
const isUntangled = await isRemoveDuplicateViewsExperimentEnabled( context.store.getState() );
if ( isUntangled ) {
return page.redirect( '/sites/settings/site/' + context.params.site_id );
}

Expand Down Expand Up @@ -403,23 +400,9 @@ export const ssrSetupLocale = ( _context, next ) => {
};

export const redirectIfDuplicatedView = ( wpAdminPath ) => async ( context, next ) => {
const aaTestName = 'calypso_post_onboarding_aa_150125';
const isUntangled = await isRemoveDuplicateViewsExperimentEnabled( context.store.getState() );

loadExperimentAssignment( aaTestName );
const isRemoveDuplicateViewsExperimentEnabled =
await getIsRemoveDuplicateViewsExperimentEnabled();

const overrideAssignment = getPreference(
context.store.getState(),
'remove_duplicate_views_experiment_assignment_160125'
);

if ( 'control' === overrideAssignment ) {
next();
return;
}

if ( isE2ETest() || isRemoveDuplicateViewsExperimentEnabled ) {
if ( isE2ETest() || isUntangled ) {
const state = context.store.getState();
const siteId = getSelectedSiteId( state );
const wpAdminUrl = getSiteAdminUrl( state, siteId, wpAdminPath );
Expand Down
12 changes: 5 additions & 7 deletions client/hosting/overview/controller.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import i18n from 'i18n-calypso';
import HostingActivate from 'calypso/hosting/server-settings/hosting-activate';
import Hosting from 'calypso/hosting/server-settings/main';
import PageViewTracker from 'calypso/lib/analytics/page-view-tracker';
import { getIsRemoveDuplicateViewsExperimentEnabled } from 'calypso/lib/remove-duplicate-views-experiment';
import { isRemoveDuplicateViewsExperimentEnabled } from 'calypso/lib/remove-duplicate-views-experiment';
import { PanelWithSidebar } from 'calypso/sites/components/panel-sidebar';
import HostingOverview from 'calypso/sites/overview/components/hosting-overview';
import { SettingsSidebar } from 'calypso/sites/settings/controller';
Expand Down Expand Up @@ -35,9 +35,8 @@ export async function hostingConfiguration( context: PageJSContext, next: () =>
removeQueryArgs( window.location.href, 'hosting_features' )
);
}
const isRemoveDuplicateViewsExperimentEnabled =
await getIsRemoveDuplicateViewsExperimentEnabled();
context.primary = isRemoveDuplicateViewsExperimentEnabled ? (
const isUntangled = await isRemoveDuplicateViewsExperimentEnabled( context.store.getState() );
context.primary = isUntangled ? (
<PanelWithSidebar>
<SettingsSidebar />
<div className="hosting-configuration">
Expand All @@ -53,9 +52,8 @@ export async function hostingConfiguration( context: PageJSContext, next: () =>
}

export async function hostingActivate( context: PageJSContext, next: () => void ) {
const isRemoveDuplicateViewsExperimentEnabled =
await getIsRemoveDuplicateViewsExperimentEnabled();
context.primary = isRemoveDuplicateViewsExperimentEnabled ? (
const isUntangled = await isRemoveDuplicateViewsExperimentEnabled( context.store.getState() );
context.primary = isUntangled ? (
<PanelWithSidebar>
<SettingsSidebar />
<div className="hosting-configuration">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
useSetEdgeCacheMutation,
useClearEdgeCacheMutation,
} from 'calypso/data/hosting/use-cache';
import { useRemoveDuplicateViewsExperimentEnabled } from 'calypso/lib/remove-duplicate-views-experiment';
import CacheCard from 'calypso/sites/settings/caching/form';
import { clearWordPressCache } from 'calypso/state/hosting/actions';
import getRequest from 'calypso/state/selectors/get-request';
Expand Down Expand Up @@ -47,6 +48,7 @@ jest.mock( 'calypso/data/hosting/use-cache' );
jest.mock( 'calypso/state/selectors/get-request' );
jest.mock( 'calypso/state/selectors/should-rate-limit-atomic-cache-clear' );
jest.mock( 'calypso/state/hosting/actions' );
jest.mock( 'calypso/lib/remove-duplicate-views-experiment' );

describe( 'CacheCard component', () => {
beforeEach( () => {
Expand All @@ -63,6 +65,7 @@ describe( 'CacheCard component', () => {
} );
jest.mocked( getRequest ).mockReturnValue( { isLoading: false } );
jest.mocked( shouldRateLimitAtomicCacheClear ).mockReturnValue( false );
jest.mocked( useRemoveDuplicateViewsExperimentEnabled ).mockReturnValue( false );
} );

function renderWithProvider() {
Expand Down
3 changes: 3 additions & 0 deletions client/hosting/server-settings/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ const createTestStore = ( {
[ TEST_SITE_ID ]: true,
},
},
explatExperiments: {
experimentAssignments: {},
},
} );
return store;
};
Expand Down
52 changes: 42 additions & 10 deletions client/lib/remove-duplicate-views-experiment/index.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,52 @@
import { useEffect, useState } from 'react';
import { useEffect } from 'react';
import { loadExperimentAssignment } from 'calypso/lib/explat';
import { useDispatch, useSelector } from 'calypso/state';
import { getRemoveDuplicateViewsExperimentAssignment } from 'calypso/state/explat-experiments/actions';
import {
getIsRemoveDuplicateViewsExperimentOverride,
getIsRemoveDuplicateViewsExperimentEnabled,
} from 'calypso/state/explat-experiments/selectors';
import type { AppState } from 'calypso/types';

const EXPERIMENT_NAME = 'calypso_post_onboarding_holdout_160125';
export const REMOVE_DUPLICATE_VIEWS_EXPERIMENT = 'calypso_post_onboarding_holdout_160125';
export const REMOVE_DUPLICATE_VIEWS_EXPERIMENT_OVERRIDE =
'remove_duplicate_views_experiment_assignment_160125';
const REMOVE_DUPLICATE_VIEWS_EXPERIMENT_AA_TEST = 'calypso_post_onboarding_aa_150125';

export const getIsRemoveDuplicateViewsExperimentEnabled = async (): Promise< boolean > => {
const experimentAssignment = await loadExperimentAssignment( EXPERIMENT_NAME );
return experimentAssignment?.variationName === 'treatment';
export const loadRemoveDuplicateViewsExperimentAssignment = async ( state: AppState ) => {
/**
* This is for escape hatch users to override the experiment assignment: p7DVsv-m73-p2
*/
const overrideAssignment = getIsRemoveDuplicateViewsExperimentOverride( state );
if ( overrideAssignment ) {
return overrideAssignment;
}

/**
* REMOVE_DUPLICATE_VIEWS_EXPERIMENT_AA_TEST should be called exactly the same number of times as REMOVE_DUPLICATE_VIEWS_EXPERIMENT.
* It helps ExPlat to know that the experiment is running as expected.
*/
const aaTestName = REMOVE_DUPLICATE_VIEWS_EXPERIMENT_AA_TEST;
loadExperimentAssignment( aaTestName );

const experimentAssignment = await loadExperimentAssignment( REMOVE_DUPLICATE_VIEWS_EXPERIMENT );
return experimentAssignment?.variationName;
};

export const isRemoveDuplicateViewsExperimentEnabled = async ( state: AppState ) => {
const experimentAssignment = await loadRemoveDuplicateViewsExperimentAssignment( state );
return experimentAssignment === 'treatment';
};

export const useRemoveDuplicateViewsExperimentEnabled = (): boolean => {
const [ isRemoveDuplicateViewsExperimentEnabled, setIsRemoveDuplicateViewsExperimentEnabled ] =
useState( false );
const isEnabled = useSelector( ( state: AppState ) => {
return getIsRemoveDuplicateViewsExperimentEnabled( state );
} );
const dispatch = useDispatch();

useEffect( () => {
getIsRemoveDuplicateViewsExperimentEnabled().then( setIsRemoveDuplicateViewsExperimentEnabled );
}, [] );
dispatch( getRemoveDuplicateViewsExperimentAssignment() );
}, [ dispatch, isEnabled ] );

return isRemoveDuplicateViewsExperimentEnabled;
return isEnabled;
};
35 changes: 35 additions & 0 deletions client/lib/remove-duplicate-views-experiment/test/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { loadExperimentAssignment } from 'calypso/lib/explat';
import { loadRemoveDuplicateViewsExperimentAssignment } from '../index';
import type { AppState } from 'calypso/types';

jest.mock( 'calypso/lib/explat', () => ( {
loadExperimentAssignment: jest.fn(),
} ) );

describe( 'loadRemoveDuplicateViewsExperimentAssignment', () => {
it( 'returns override if present', async () => {
const mockState = {
preferences: {
localValues: {
remove_duplicate_views_experiment_assignment_160125: 'control',
},
},
} as AppState;
const result = await loadRemoveDuplicateViewsExperimentAssignment( mockState );
expect( result ).toBe( 'control' );
} );

it( 'calls experiment assignment when no override', async () => {
const mockState = {} as AppState;

( loadExperimentAssignment as jest.Mock ).mockResolvedValue( { variationName: 'treatment' } );

const result = await loadRemoveDuplicateViewsExperimentAssignment( mockState );

expect( loadExperimentAssignment ).toHaveBeenCalledWith( 'calypso_post_onboarding_aa_150125' );
expect( loadExperimentAssignment ).toHaveBeenCalledWith(
'calypso_post_onboarding_holdout_160125'
);
expect( result ).toBe( 'treatment' );
} );
} );
62 changes: 29 additions & 33 deletions client/my-sites/site-settings/form-general.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ import SiteLanguagePicker from 'calypso/components/language-picker/site-language
import Notice from 'calypso/components/notice';
import NoticeAction from 'calypso/components/notice/notice-action';
import Timezone from 'calypso/components/timezone';
import { getIsRemoveDuplicateViewsExperimentEnabled } from 'calypso/lib/remove-duplicate-views-experiment';
import scrollToAnchor from 'calypso/lib/scroll-to-anchor';
import { domainManagementEdit } from 'calypso/my-sites/domains/paths';
import SettingsSectionHeader from 'calypso/my-sites/site-settings/settings-section-header';
import SiteSettingsForm from 'calypso/sites/settings/site/form';
import { getRemoveDuplicateViewsExperimentAssignment } from 'calypso/state/explat-experiments/actions';
import { getIsRemoveDuplicateViewsExperimentEnabled } from 'calypso/state/explat-experiments/selectors';
import getTimezonesLabels from 'calypso/state/selectors/get-timezones-labels';
import isAtomicSite from 'calypso/state/selectors/is-site-automated-transfer';
import isSiteWpcomStaging from 'calypso/state/selectors/is-site-wpcom-staging';
Expand All @@ -40,22 +41,9 @@ import SiteIconSetting from './site-icon-setting';
import wrapSettingsForm from './wrap-settings-form';

export class SiteSettingsFormGeneral extends Component {
state = {
isRemoveDuplicateViewsExperimentEnabled: false,
};

componentDidMount() {
setTimeout( () => scrollToAnchor( { offset: 15 } ) );
getIsRemoveDuplicateViewsExperimentEnabled().then(
( isRemoveDuplicateViewsExperimentEnabled ) => {
if (
this.state.isRemoveDuplicateViewsExperimentEnabled !==
isRemoveDuplicateViewsExperimentEnabled
) {
this.setState( { isRemoveDuplicateViewsExperimentEnabled } );
}
}
);
this.props.getRemoveDuplicateViewsExperimentAssignment();
}

getIncompleteLocaleNoticeMessage = ( language ) => {
Expand Down Expand Up @@ -389,14 +377,14 @@ export class SiteSettingsFormGeneral extends Component {
render() {
const {
handleSubmitForm,
isRemoveDuplicateViewsExperimentEnabled,
isRequestingSettings,
isSavingSettings,
site,
siteIsJetpack,
translate,
adminInterfaceIsWPAdmin,
} = this.props;
const { isRemoveDuplicateViewsExperimentEnabled } = this.state;
const classes = clsx( 'site-settings__general-settings', {
'is-loading': isRequestingSettings,
} );
Expand Down Expand Up @@ -440,23 +428,31 @@ export class SiteSettingsFormGeneral extends Component {
}
}

const connectComponent = connect( ( state ) => {
const siteId = getSelectedSiteId( state );
return {
isAtomicAndEditingToolkitDeactivated:
isAtomicSite( state, siteId ) &&
getSiteOption( state, siteId, 'editing_toolkit_is_active' ) === false,
adminInterfaceIsWPAdmin: isAdminInterfaceWPAdmin( state, siteId ),
isUnlaunchedSite: isUnlaunchedSite( state, siteId ),
isWPForTeamsSite: isSiteWPForTeams( state, siteId ),
isWpcomStagingSite: isSiteWpcomStaging( state, siteId ),
selectedSite: getSelectedSite( state ),
siteIsJetpack: isJetpackSite( state, siteId ),
siteIsWpcom: isWpcomSite( state, siteId ),
siteSlug: getSelectedSiteSlug( state ),
timezonesLabels: getTimezonesLabels( state ),
};
} );
const connectComponent = connect(
( state ) => {
const siteId = getSelectedSiteId( state );
const isRemoveDuplicateViewsExperimentEnabled =
getIsRemoveDuplicateViewsExperimentEnabled( state );
return {
isAtomicAndEditingToolkitDeactivated:
isAtomicSite( state, siteId ) &&
getSiteOption( state, siteId, 'editing_toolkit_is_active' ) === false,
adminInterfaceIsWPAdmin: isAdminInterfaceWPAdmin( state, siteId ),
isRemoveDuplicateViewsExperimentEnabled,
isUnlaunchedSite: isUnlaunchedSite( state, siteId ),
isWPForTeamsSite: isSiteWPForTeams( state, siteId ),
isWpcomStagingSite: isSiteWpcomStaging( state, siteId ),
selectedSite: getSelectedSite( state ),
siteIsJetpack: isJetpackSite( state, siteId ),
siteIsWpcom: isWpcomSite( state, siteId ),
siteSlug: getSelectedSiteSlug( state ),
timezonesLabels: getTimezonesLabels( state ),
};
},
{
getRemoveDuplicateViewsExperimentAssignment,
}
);

const getFormSettings = ( settings ) => {
const defaultSettings = {
Expand Down
19 changes: 8 additions & 11 deletions client/my-sites/site-settings/settings-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import titlecase from 'to-title-case';
import { redirectIfDuplicatedView } from 'calypso/controller';
import { recordPageView } from 'calypso/lib/analytics/page-view';
import { navigate } from 'calypso/lib/navigate';
import { getIsRemoveDuplicateViewsExperimentEnabled } from 'calypso/lib/remove-duplicate-views-experiment';
import { isRemoveDuplicateViewsExperimentEnabled } from 'calypso/lib/remove-duplicate-views-experiment';
import { sectionify } from 'calypso/lib/route';
import { canCurrentUser } from 'calypso/state/selectors/can-current-user';
import isSiteWpcomAtomic from 'calypso/state/selectors/is-site-wpcom-atomic';
Expand Down Expand Up @@ -39,10 +39,9 @@ export function redirectToJetpackNewsletterSettingsIfNeeded( context, next ) {
* @returns
*/
export const redirectToolsIfRemoveDuplicateViewsExperimentEnabled = async ( context, next ) => {
const isRemoveDuplicateViewsExperimentEnabled =
await getIsRemoveDuplicateViewsExperimentEnabled();
const isUntangled = await isRemoveDuplicateViewsExperimentEnabled( context.store.getState() );

if ( isRemoveDuplicateViewsExperimentEnabled ) {
if ( isUntangled ) {
const slug = context.path.split( '/' )[ 2 ];
if ( ! slug ) {
return next();
Expand Down Expand Up @@ -70,11 +69,10 @@ export const redirectToolsIfRemoveDuplicateViewsExperimentEnabled = async ( cont
*
* Previously /settings redirected to /settings/general which now redirects to /wp-admin/options-general.php
*/
export const redirectSettingsIfDuplciatedViewsEnabled = async () => {
const isRemoveDuplicateViewsExperimentEnabled =
await getIsRemoveDuplicateViewsExperimentEnabled();
export const redirectSettingsIfDuplciatedViewsEnabled = async ( context ) => {
const isUntangled = await isRemoveDuplicateViewsExperimentEnabled( context.store.getState() );

if ( isRemoveDuplicateViewsExperimentEnabled ) {
if ( isUntangled ) {
return page.redirect( `/sites/settings/site` );
}

Expand All @@ -90,11 +88,10 @@ export async function redirectGeneralSettingsIfDuplicatedViewsEnabled( context,
const siteId = getSelectedSiteId( state );
const siteSlug = getSelectedSiteSlug( state );

const isRemoveDuplicateViewsExperimentEnabled =
await getIsRemoveDuplicateViewsExperimentEnabled();
const isUntangled = await isRemoveDuplicateViewsExperimentEnabled( context.store.getState() );
const hasClassicAdminInterfaceStyle =
getSiteOption( state, siteId, 'wpcom_admin_interface' ) === 'wp-admin';
if ( isRemoveDuplicateViewsExperimentEnabled && hasClassicAdminInterfaceStyle ) {
if ( isUntangled && hasClassicAdminInterfaceStyle ) {
return page.redirect( `/sites/settings/site/${ siteSlug }` );
}

Expand Down
Loading
Loading