-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Help Center: Support Experience to 50% of ALL users #97119
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~2269 bytes removed 📉 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~35647 bytes added 📈 [gzipped])
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Async-loaded Components (~2748 bytes added 📈 [gzipped])
React components that are loaded lazily, when a certain part of UI is displayed for the first time. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
Why don't we go straight to 100%? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems this logic doesn't run in the editor, would that break chats when people switch from Calypso to the editor?
@robfelty it is just because when we switched to 50% of users, we saw a surge in Zendesk API usage and we had to work a bit to improve how we handle the calls. So I want to see how API usage reacts to this increment first ✌️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! I see title and description differs (50% and 75%) I know it's 75% but for the records, we should update the description 😆
Secondly, this is not using any AB test, is that intended? If that's intended, then I'm fine with this code, but if we have set up and experiment to be run in the backend, then this is not the correct way to run an AB test :)
@@ -139,3 +139,10 @@ export const matchSupportInteractionId = ( | |||
return foundMatch; | |||
} | |||
}; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this way, but are we sure, we don't do anything in the backend?
I mean, this will enable the frontend, but are we using any different treatment in the backend?
Example, like users being assigned in the backend to use a different version (if so, we can do also in Calypso by adding a new version parameter in the sendMessage hook stating a different version when user is assigned to the experiment)
Secondly, this is not using a real A/B experiment, is just balancing users to be using either one experience or the other. Is that intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Daniel!
This is not using an experiment! It simply shows it to 75% of users 👍
to use a different version
Yes, we already do this here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And thanks, I updated the description!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works in Calypso, wp-admin, and the editor. It makes sense to keep it at 50% since we include the editor now, which is 40% more Help Center users! This is already a huge bump.
if ( ! userId || userId % 100 > 75 ) { | ||
return false; | ||
} | ||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if ( ! userId || userId % 100 > 75 ) { | |
return false; | |
} | |
return true; | |
return userId && userId % 100 < 75 |
@AllTerrainDeveloper @robfelty, we only shipped to 50% since this PR also introduces the changes to wp-admin and the editor, so already a lot of users. |
This reverts commit c13354e.
Proposed Changes
This removes the experiment and shows the new Support Experience to 50% of ALL users, so now including wp-admin and the editor.
Testing Instructions
yarn start
and test it in Calypso.cd apps/help-center
->yarn dev --sync
)flags=help-center-experience
, is still working on Calypso, WP-Admin, and Editor.