Survicate: skip internal P2 sites and add is_big_sky_site visitor trait#48588
Open
paulopmt1 wants to merge 4 commits into
Open
Survicate: skip internal P2 sites and add is_big_sky_site visitor trait#48588paulopmt1 wants to merge 4 commits into
paulopmt1 wants to merge 4 commits into
Conversation
Contributor
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryCoverage changed in 1 file.
|
Atomic powers Automattic's internal P2s, so the same Survicate code that ships to wpcom user sites was loading on internal P2s too. Hard-block in should_load() so the SDK never enqueues there. For Big Sky, surveys like "how easy was it to edit your site template today" don't apply to AI-driven flows. Emit an is_big_sky_site visitor trait (true when the big-sky-enabled or big-sky-free-trial sticker is set) so the targeting rule can be configured in Survicate's UI without a code change. Extracts a reusable wpcom_is_p2_site() helper to src/utils.php; the same inline pattern is duplicated in ~8 other call sites and can be migrated in follow-up work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Survicate calls the canonical Automattic\Jetpack\Status\Host::is_p2_site() in src/features/survicate/class-survicate.php directly. Drops the short-lived wpcom_is_p2_site() helper from src/utils.php — the same package already wraps Status\Host for is_woa_site() but the dedicated P2 helper makes its own wrapper unnecessary. Status\Host::is_p2_site() short-circuits to false when there is no wpcom site id, so the P2 stylesheet/WPForTeams tests now define IS_WPCOM in their isolated process to route get_wpcom_site_id() through get_current_blog_id() (matching the pattern in projects/packages/status/tests/php/Host_Test.php). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
is_admin() returns true for /wp-admin/network/* and /wp-admin/user/* too, so the existing should_load() guard was still enqueuing Survicate on wpcom's network admin (e.g. wordpress.com/wp-admin/network/admin.php) — an internal tools surface that the P2 check doesn't cover because the main wpcom blog isn't a P2. Adds an is_network_admin() || is_user_admin() short-circuit and tests that route through WP_Screen via the -network / -user hook suffix, since the screen-based branch of is_network_admin() takes precedence over the WP_NETWORK_ADMIN define once a current_screen is set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The three fixture files only existed to dodge PHP's "function already declared" rule across tests. Each stub is now declared inline inside its test method — namespaced (\WPForTeams\is_wpforteams_site) and global (has_blog_sticker) stubs both go through an eval'd namespace block since the test file itself lives in namespace A8C\FSE. @runInSeparateProcess on each test keeps the function table fresh. Removes tests/php/features/survicate/fixtures/. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
7cb49b5 to
863d5e3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes DOTOBRD-431
Proposed changes
We want to skip Survicate on three surfaces: Internal P2s, Big Sky, and WPCOM internal-tools admin screens.
should_load()now bails early when\Automattic\Jetpack\Status\Host::is_p2_site()returns true, which covers bothpub/p2-*stylesheets and the\WPForTeams\is_wpforteams_site()signal. No new local helper — we call the canonicalStatus\Hostmethod directly.is_admin()istruefor/wp-admin/network/*and/wp-admin/user/*too, so the existing guard was still enqueuing Survicate on wpcom's network admin (e.g.wordpress.com/wp-admin/network/admin.php) — a wpcom internal-tools surface that the P2 check doesn't cover because the main wpcom blog isn't a P2.should_load()now also short-circuits onis_network_admin() || is_user_admin().is_big_sky_sitevisitor trait. The CES survey "how easy was it to edit your site template today" doesn't apply to AI-driven Big Sky flows. Emit a newis_big_sky_sitevisitor trait (string"true"/"false") — true when either thebig-sky-enabledorbig-sky-free-trialblog sticker is set. The actual exclusion is configured in Survicate's targeting UI, so future Big Sky-targeted surveys are still possible without a code change.Related product discussion/links
Does this pull request change what data or activity we track or use?
No new data is collected. A new visitor trait (
is_big_sky_site) is sent to Survicate alongside the traits we already pass (email,site_id,site_type,editor_context). Its only purpose is to let Survicate's targeting UI exclude Big Sky users.Survicate dashboard follow-up (post-deploy)
The trait alone doesn't filter anything — the targeting rule lives in Survicate's UI. After this lands and rolls out:
is_big_sky_siteequals"true".site_typetargeting that would reach internal P2 sites — the P2 hard-block already handles this server-side, but worth confirming nothing else is misconfigured.Testing instructions
Unit tests
cd projects/packages/jetpack-mu-wpcom composer phpunit -- tests/php/features/survicate/Survicate_Test.phpShould pass 27 tests, including 7 new ones:
test_should_load_returns_false_on_p2_site_via_stylesheettest_should_load_returns_false_on_p2_site_via_wpforteamstest_should_load_returns_false_on_network_admintest_should_load_returns_false_on_user_admintest_get_visitor_traits_returns_is_big_sky_site_false_by_defaulttest_get_visitor_traits_returns_is_big_sky_site_true_for_big_sky_enabled_stickertest_get_visitor_traits_returns_is_big_sky_site_true_for_big_sky_free_trial_stickerManual verification (Jurassic Ninja)
P2 hard-block:
pub/p2-2020), log in as an English user.survey.survicate.com. The Survicate inline script is not enqueued.Network / user admin hard-block (wpcom sandbox):
https://wordpress.com/wp-admin/network/admin.php(or any/wp-admin/user/*URL).survey.survicate.comrequests and the page source for the inline Survicate bootstrap./wp-admin/*on a user site should still enqueue as before.Big Sky trait:
SurvicateReady→ inspect the_svacalls or usewindow._sva.getVisitorTraits?.().is_big_sky_site: "false".big-sky-enabledblog sticker (or filteringwpcom_has_blog_stickerfor the test environment).is_big_sky_site: "true".is_big_sky_site = "true". Reload the page.Regression check
Confirm Survicate still loads as before on:
editor_context,site_type, etc.should_load()short-circuits (logged out, non-admin, non-English locale) still return false.