Core Compat: Site Environment #16338

kraftbj · 2020-06-30T17:07:11Z

Fixes #11798

Changes proposed in this Pull Request:

Uses wp_get_environment_type as the default value for the Status package is_staging_site value (by way of !== production is a staging site in how we handle them).
Uses the WP_LOCAL_DEV that Core site health does check. This activates our "Development Mode".
Renames "Development Mode" to "Offline Mode" to be clearer our intent of this mode.
Deprecation and updates relevant for ^.

Jetpack product discussion

n/a

Does this pull request change what data or activity we track or use?

n/a

Todo required

If this lands, we need to update documentation. I'm okay doing this in a folllow-up PR or adding a new commit here if this is otherwise approved.

If we're happy with this, I'll coordinate with Quill.

Testing instructions:

Ensure Offline mode works (try this on localhost without a dot in the domain, e.g. our docker without using ngrok/JT.
On a JN site running on WP trunk on a connected Jetpack site, set a constant define( 'WP_ENVIRONMENT_TYPE', 'staging' ); Does this activate staging mode?

Proposed changelog entry for your changes:

WordPress 5.5: Utilize new wp_get_environment_type function to determing Staging sites.
Rename Development Mode to Offline Mode to be clearer our intent for its use.

test-case-reminder

Here are some suggested test cases for this PR.

Gutenberg extensions

Use Core's block editor
Use latest stable Gutenberg plugin

Blocks

Extensions

Publicize
Likes

Connection

In-place connection with free plan
In-place connection with paid plan
In-place connection with product purchase
Classic connection. Use Safari, or set a constant JETPACK_SHOULD_USE_CONNECTION_IFRAME to false
Disconnect/reconnect connection
Secondary user connection
Connection on multisite

If you think that suggestions should be improved please edit the configuration file here. You can also modify/add test-suites to be used in the configuration file.

jetpackbot · 2020-06-30T17:19:55Z

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16338

Scheduled Jetpack release: August 4, 2020.
Scheduled code freeze: July 28, 2020

Generated by 🚫 dangerJS against 6bb0d46

jeherve

I think that's a good change. I only have one minor remark.

packages/status/src/class-status.php

ebinnion · 2020-06-30T20:21:20Z

I believe the Infinity ping here is for the jetpack-cli.php file. For that file, I don't see any issues. 👍

jeherve

Should we update this little utility in the React dashboard as well?

jetpack/_inc/client/state/connection/reducer.js

Lines 223 to 231 in fe5b739

    
           /** 
        
            * Checks if the site is currently in development mode. 
        
            * 
        
            * @param  {Object}  state Global state tree 
        
            * @return {boolean} True if site is in dev mode. False otherwise. 
        
            */ 
        
           export function isDevMode( state ) { 
        
           	return 'dev' === getSiteConnectionStatus( state ); 
        
           }

We also have a few "Unavailable in Dev Mode" strings in the React dashboard that may be worth updating, as well as the "Dev Mode" badge:

jetpack/_inc/client/components/masthead/index.jsx

Line 54 in 1b9b295

    
           const devNotice = this.props.siteConnectionStatus === 'dev' ? <code>Dev Mode</code> : '',

_inc/lib/admin-pages/class.jetpack-admin-page.php

matticbot · 2020-07-07T00:16:13Z

Caution: This PR has changes that must be merged to WordPress.com
Hello kraftbj! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D46012-code before merging this PR. Thank you!
This revision will be updated with each commit to this PR

jeherve

Let's add WP_LOCAL_DEV here to avoid an empty message?

jetpack/_inc/client/components/jetpack-notices/index.jsx

Lines 100 to 126 in 8cfd220

    
           if ( devMode.filter ) { 
        
           	reasons.push( 
        
           		__( '{{li}}The jetpack_development_mode filter is active{{/li}}', { 
        
           			components: { 
        
           				li: <li />, 
        
           			}, 
        
           		} ) 
        
           	); 
        
           } 
        
           if ( devMode.constant ) { 
        
           	reasons.push( 
        
           		__( '{{li}}The JETPACK_DEV_DEBUG constant is defined{{/li}}', { 
        
           			components: { 
        
           				li: <li />, 
        
           			}, 
        
           		} ) 
        
           	); 
        
           } 
        
           if ( devMode.url ) { 
        
           	reasons.push( 
        
           		__( '{{li}}Your site URL lacks a dot (e.g. http://localhost){{/li}}', { 
        
           			components: { 
        
           				li: <li />, 
        
           			}, 
        
           		} ) 
        
           	); 
        
           }

When I use the WP_LOCAL_DEV constant, I get both the staging notice and the local one:

Maybe we should set a site as staging only when it has the staging WP environment type?

…and list of staging sites. (#16590)

kraftbj · 2020-07-27T14:41:46Z

Caution: This PR has changes that must be merged to WordPress.com
Hello kraftbj! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D46012-code before merging this PR. Thank you!
This revision will be updated with each commit to this PR

Note that this can not land until after the package update has been released and deployed on WP.com.

brbrr · 2020-07-27T16:23:12Z

Sorry folks, I have commited into this branch by mistake. removed the commit and force-pushed the branch

dereksmart

Found a few things, but could be fixed in separate PRs since this one is already so big.

Found a reference to the old term "Development mode" here

When using WP_LOCAL_DEV constant, I see a notice saying I'm using a staging server (not true). This notice does not appear with the JETPACK_DEV_DEBUG constant.

Everything else seems to work as expected

Fresh build
jetpack_development_mode filter
localhost url

dereksmart

Approving despite the comments above, in case you'd prefer to fix those in separate PR.

See #16338 (review)

See Automattic/jetpack#16338

jeherve · 2020-07-28T08:31:46Z

Found a reference to the old term "Development mode" here

Fixed that in #16608

When using WP_LOCAL_DEV constant, I see a notice saying I'm using a staging server (not true).

Yes, we may have to discuss this again, since we're not exactly sure how folks are going to be using the different environment types yet (see discussion above).

See #16338 (review)

Since being added recently in r48856-core we may as well listen for the environment declaring itself `local` explicitly. https://core.trac.wordpress.org/ticket/51064 Relevant prior art: #16338

jeherve · 2021-05-13T14:26:13Z

r225659-wpcom

kraftbj added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it General [Status] Needs Review This PR is ready for review. [Package] Status labels Jun 30, 2020

kraftbj added this to the 8.8 milestone Jun 30, 2020

kraftbj requested a review from a team as a code owner June 30, 2020 17:07

kraftbj self-assigned this Jun 30, 2020

matticbot added Touches WP.com Files labels Jun 30, 2020

test-case-reminder bot reviewed Jun 30, 2020

View reviewed changes

github-actions bot added the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Jun 30, 2020

jeherve requested changes Jun 30, 2020

View reviewed changes

packages/status/src/class-status.php Show resolved Hide resolved

jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Jun 30, 2020

jeherve requested changes Jul 1, 2020

View reviewed changes

_inc/lib/admin-pages/class.jetpack-admin-page.php Outdated Show resolved Hide resolved

kraftbj force-pushed the core-compat/site-env branch 9 times, most recently from 05d2298 to 8aa6574 Compare July 6, 2020 22:15

kraftbj added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Jul 7, 2020

jeherve requested changes Jul 7, 2020

View reviewed changes

jeherve previously approved these changes Jul 27, 2020

View reviewed changes

jeherve dismissed their stale review via 676bdce July 27, 2020 12:11

jeherve force-pushed the core-compat/site-env branch from ab06973 to 676bdce Compare July 27, 2020 12:11

jeherve mentioned this pull request Jul 27, 2020

Status: add new method to check for a local development site, and expand list of staging sites. #16590

Merged

Update tests to include new connection info parameter

c8b38df

jeherve force-pushed the core-compat/site-env branch from 676bdce to c8b38df Compare July 27, 2020 12:29

Status: add new method to check for a local development site, and exp…

6bb0d46

…and list of staging sites. (#16590)

brbrr force-pushed the core-compat/site-env branch from 34df120 to 6bb0d46 Compare July 27, 2020 16:22

dereksmart reviewed Jul 27, 2020

View reviewed changes

dereksmart approved these changes Jul 27, 2020

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Jul 28, 2020

jeherve merged commit 4392a22 into master Jul 28, 2020

jeherve deleted the core-compat/site-env branch July 28, 2020 08:21

matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Jul 28, 2020

jeherve added a commit that referenced this pull request Jul 28, 2020

Admin Page: fix remaining mention of Development mode

3741ddb

See #16338 (review)

jeherve added a commit to Automattic/companion that referenced this pull request Jul 28, 2020

Switch from Development mode to Offline mode

e8583fa

See Automattic/jetpack#16338

jeherve mentioned this pull request Jul 28, 2020

Admin Page: fix remaining mention of Development mode #16608

Merged

jeherve added a commit that referenced this pull request Jul 28, 2020

Admin Page: fix remaining mention of Development mode (#16608)

bc9d081

See #16338 (review)

jeherve added [Status] Has Changelog and removed [Status] Needs Changelog labels Jul 28, 2020

jeherve added a commit that referenced this pull request Jul 28, 2020

Changelog: add #16338

e3ece94

jeherve removed the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Jul 29, 2020

georgestephanis mentioned this pull request Oct 12, 2020

Also use Core wp_get_environment_type for local #17468

Merged

	/**
	* Checks if the site is currently in development mode.
	*
	* @param {Object} state Global state tree
	* @return {boolean} True if site is in dev mode. False otherwise.
	*/
	export function isDevMode( state ) {
	return 'dev' === getSiteConnectionStatus( state );
	}

	if ( devMode.filter ) {
	reasons.push(
	__( '{{li}}The jetpack_development_mode filter is active{{/li}}', {
	components: {
	li: <li />,
	},
	} )
	);
	}
	if ( devMode.constant ) {
	reasons.push(
	__( '{{li}}The JETPACK_DEV_DEBUG constant is defined{{/li}}', {
	components: {
	li: <li />,
	},
	} )
	);
	}
	if ( devMode.url ) {
	reasons.push(
	__( '{{li}}Your site URL lacks a dot (e.g. http://localhost){{/li}}', {
	components: {
	li: <li />,
	},
	} )
	);
	}

Core Compat: Site Environment #16338

Core Compat: Site Environment #16338

Uh oh!

Conversation

kraftbj commented Jun 30, 2020

Changes proposed in this Pull Request:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Todo required

Testing instructions:

Proposed changelog entry for your changes:

Uh oh!

test-case-reminder bot left a comment

Choose a reason for hiding this comment

Gutenberg extensions

Blocks

Extensions

Connection

Uh oh!

jetpackbot commented Jun 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ebinnion commented Jun 30, 2020

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

matticbot commented Jul 7, 2020

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

kraftbj commented Jul 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brbrr commented Jul 27, 2020

Uh oh!

dereksmart left a comment

Choose a reason for hiding this comment

Uh oh!

dereksmart left a comment

Choose a reason for hiding this comment

Uh oh!

jeherve commented Jul 28, 2020

Uh oh!

jeherve commented May 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jetpackbot commented Jun 30, 2020 •

edited

Loading

kraftbj commented Jul 27, 2020 •

edited

Loading