Image Studio: Add isDevMode property to imageStudioData#48084
Image Studio: Add isDevMode property to imageStudioData#48084
Conversation
Adds a is_dev_mode() check that mirrors the Agents_Manager logic to detect local/dev/proxied environments. The resulting isDevMode flag is passed to Calypso so analytics events can be tagged as test traffic.
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
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! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 2 files.
Full summary · PHP report · JS report Coverage check overridden by
I don't care about code coverage for this PR
|
Adds unit tests covering each early-return branch: production domain (false), localhost, jurassic.ninja, jurassic.tube, and wpcom_is_proxied_request().
b2218b7 to
b1c8392
Compare
…nager The jetpack-mu-wpcom package is only available on WordPress.com hosted sites, so Image Studio needs its own copy for self-hosted Jetpack sites.
There was a problem hiding this comment.
Pull request overview
Adds a dev/test environment signal to Image Studio so Calypso can distinguish test traffic from production when emitting analytics.
Changes:
- Add
is_dev_mode()detection (localhost/Jurassic, proxied A8C requests, Atomic proxied requests) to Image Studio. - Expose the result as
isDevModeon theimageStudioDatainline script object. - Add PHPUnit coverage for the new inline property and some
is_dev_mode()scenarios, plus a changelog fragment.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| projects/plugins/jetpack/extensions/plugins/image-studio/image-studio.php | Introduces is_dev_mode() and adds isDevMode into the inline imageStudioData. |
| projects/plugins/jetpack/tests/php/extensions/plugins/image-studio/Image_Studio_Test.php | Adds tests asserting isDevMode exists and basic URL-based dev mode detection. |
| projects/plugins/jetpack/changelog/image-studio-devmode-prop | Documents the change in Jetpack’s changelog fragments. |
| $domain = wp_parse_url( get_site_url(), PHP_URL_HOST ); | ||
| if ( | ||
| $domain === 'localhost' || | ||
| '.jurassic.tube' === stristr( $domain, '.jurassic.tube' ) || | ||
| '.jurassic.ninja' === stristr( $domain, '.jurassic.ninja' ) |
There was a problem hiding this comment.
wp_parse_url( get_site_url(), PHP_URL_HOST ) can return false/null, which then gets passed into stristr(). On newer PHP versions this can raise warnings/deprecations for non-string arguments. Consider normalizing $domain to a string (or early-returning false when no host is present) before calling stristr().
| $this->assertFalse( ImageStudio\is_dev_mode() ); | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
is_dev_mode() includes multiple proxy/constant-based detection paths, but the unit tests only cover localhost/Jurassic and the default false case. Add coverage for at least the $_SERVER['A8C_PROXIED_REQUEST'] path (and ideally the wpcom_is_proxied_request() and AT_PROXIED_REQUEST/ATOMIC_CLIENT_ID paths) to prevent regressions in environments where the site URL is production-like but requests are proxied.
| /** | |
| /** | |
| * Test is_dev_mode returns true for proxied production requests. | |
| */ | |
| public function test_is_dev_mode_returns_true_for_a8c_proxied_request() { | |
| update_option( 'siteurl', 'https://example.com' ); | |
| $had_proxied_request = array_key_exists( 'A8C_PROXIED_REQUEST', $_SERVER ); | |
| $original_value = $had_proxied_request ? $_SERVER['A8C_PROXIED_REQUEST'] : null; | |
| try { | |
| $_SERVER['A8C_PROXIED_REQUEST'] = '1'; | |
| $this->assertTrue( ImageStudio\is_dev_mode() ); | |
| } finally { | |
| if ( $had_proxied_request ) { | |
| $_SERVER['A8C_PROXIED_REQUEST'] = $original_value; | |
| } else { | |
| unset( $_SERVER['A8C_PROXIED_REQUEST'] ); | |
| } | |
| } | |
| } | |
| /** |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Prevents any risk of siteurl state leaking between tests, independent of the WP_UnitTestCase transaction rollback.
…dev_mode() Moves the dev/test environment detection helper out of the Image Studio extension and into functions.global.php so other parts of the Jetpack plugin can reuse it. Tests live alongside other functions.global.php tests in Functions_Global_Test.php.
Avoids passing false/null to stristr() in edge cases where the site URL has no host component.
…nt() The previous name overlapped with Jetpack's legacy "Development Mode" terminology (now Status::is_offline_mode()) and could mislead developers searching for dev-mode helpers. The new name makes clear this is about detecting A8C-internal testing infrastructure for analytics tagging.
kat3samsin
left a comment
There was a problem hiding this comment.
Thanks for adding this!
Fixes FORNO-337
Proposed changes
is_dev_mode()function to Image Studio that detects dev/test environments (localhost, Jurassic Ninja/Tube, proxied A8C requests, Atomic proxied requests)isDevModein theimageStudioDatainline script object, so the Calypso side (wp-calypso#109983) can tag analytics events as test trafficAgents_Manager::is_dev_mode()so Image Studio works independently of Big SkyRelated product discussion/links
Does this pull request change what data or activity we track or use?
No new tracking. This adds a flag that allows the Calypso consumer to distinguish dev/test traffic from production traffic in existing analytics.
Testing instructions
imageStudioData— confirmisDevModeistrue