Block version gating #14703

glendaviesnz · 2020-02-16T21:23:50Z

Adds a version gating option block registration.

Changes proposed in this Pull Request:

Some of the new blocks being created require specific versions of gutenberg. This PR introduces the ability to include min WP and gutenberg plugin versions in jetpack_register_block $args
It also runs set_extension_unavailable if block is version gated in order to allow a version upgrade nudge to be added a later point.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

New feature.

Testing instructions:

Check out this branch and build blocks
Update extensions/blocks/markdown/markdown.php block registration to the following:

jetpack_register_block( 'jetpack/markdown', array( 'version_requirements' => array( 'gutenberg' => '8', 'wp' => '7')) );

Load the build in your local jetpack dev setup and check that window.Jetpack_Editor_Initial_State.available_blocks; shows

markdown:
available: false
unavailable_reason: "incorrect_gutenberg_version"
details:
required_feature: "jetpack/markdown"
required_version: {gutenberg: "8", wp: "7"}
current_version: {wp: "5.3", gutenberg: "7.5.0"}

Play around with modifying the gutenberg and wp versions, and the versions you are running and check block shows available or not.

N.B. Your local dev may set GUTENBERG_DEVELOPMENT_MODE in which case the block will always show available, in which case you can comment out the following in class.jetpack-gutenberg.php to test:

if ( defined( 'GUTENBERG_DEVELOPMENT_MODE' ) && GUTENBERG_DEVELOPMENT_MODE ) {
	return true;
}

matticbot · 2020-02-16T21:23:57Z

Caution: This PR has changes that must be merged to WordPress.com
Hello glendaviesnz! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D38972-code before merging this PR. Thank you!

jetpackbot · 2020-02-16T21:26:11Z

	Warnings
⚠️	`pre-commit` hook was skipped for one or more commits
⚠️	"Proposed changelog entry" is missing for this PR. Please include any meaningful changes

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against e07aa21

simison · 2020-02-16T21:28:40Z

class.jetpack-gutenberg.php

+	public static function is_block_version_gated( $core_wp_version, $plugin_version ) {
+		global $wp_version;
+
+		if ( defined( 'IS_WPCOM' ) && IS_WPCOM ) {


Does this check true on Atomic sites? (Those can sometimes run older versions of Gutenberg.)

good question, will try and find out

Apparently this will only return true for simple sites.

matticbot · 2020-02-16T22:42:36Z

glendaviesnz, Your synced wpcom patch D38972-code has been updated.

matticbot · 2020-02-16T23:09:28Z

glendaviesnz, Your synced wpcom patch D38972-code has been updated.

matticbot · 2020-02-17T01:04:24Z

glendaviesnz, Your synced wpcom patch D38972-code has been updated.

pento · 2020-02-17T01:37:10Z

class.jetpack-gutenberg.php

+	public static function is_block_version_gated( $core_wp_version, $plugin_version ) {
+		global $wp_version;
+
+		if ( defined( 'IS_WPCOM' ) && IS_WPCOM ) {


Is there a reason why WPCOM will always return false? It seems like there are reasonable situations where it may not have the necessary version of Core or Gutenberg (for example, I believe some VIPs are running older versions of Gutenberg).

I was thinking that wpcom simple sites currently have the latest gutenberg release - but if there are VIP sites running older versions which have IS_WPCOM set then I will take a look and see how we can identify which gutenberg version is active for any site which has IS_WPCOM true.

class.jetpack-gutenberg.php

pablinos

Looking good so far. Should we add the nudge to this PR or create a separate one?

pablinos · 2020-02-17T11:04:33Z

class.jetpack-gutenberg.php

+			'incorrect_gutenberg_version',
+			array(
+				'required_feature' => $slug,
+				'required_version' => $args['version_requirements']['plugin'],


Would we need all the version requirements for the nudge? And would it be beneficial to know the versions detected?

class.jetpack-gutenberg.php

matticbot · 2020-02-17T18:36:50Z

glendaviesnz, Your synced wpcom patch D38972-code has been updated.

matticbot · 2020-02-17T21:59:21Z

glendaviesnz, Your synced wpcom patch D38972-code has been updated.

glendaviesnz · 2020-02-17T22:02:34Z

@pablinos I made some changes based on your feedback, let me know what you think about the latest iteration.

glendaviesnz · 2020-02-17T22:03:39Z

Should we add the nudge to this PR or create a separate one?

I was figuring we would keep it simple and do the nudge as a separate PR.

matticbot · 2020-02-17T22:45:31Z

glendaviesnz, Your synced wpcom patch D38972-code has been updated.

pablinos · 2020-02-18T19:14:51Z

@pablinos I made some changes based on your feedback, let me know what you think about the latest iteration.

It's looking good at first glance. I think on reflection it would be good to keep this PR as just the logic to check versions and we can add the nudge in a separate one.

pablinos · 2020-02-18T19:19:20Z

I was figuring we would keep it simple and do the nudge as a separate PR.

Sorry only just saw this reply. I agree!

class.jetpack-gutenberg.php

scruffian · 2020-02-19T10:42:25Z

This looks good to me, just a few thoughts:

We should update the documentation of jetpack_register_block to add this option and explain how to use it.
From the name (is_editor_version_available) and the code itself, I don't think it's 100% clear whether this is just a check for the Gutenberg version that is running, or a generic check for any plugin. Maybe we should change the plugin array key to gutenberg and the function to is_gutenberg_version_available. I can see a case in the future of extending this to check for any plugin, but lets not add that at this stage :)

class.jetpack-gutenberg.php

jeherve · 2020-02-19T11:05:31Z

class.jetpack-gutenberg.php

+		}
+
+		if ( defined( 'GUTENBERG_VERSION' ) ) {
+			$version_available = GUTENBERG_VERSION >= $version_requirements['plugin'];


I wonder if we should check if plugin and wp are set before to call them here and below, to avoid any errors if someone registers a block but only specifies one or the other?

Done - but wasn't sure on bailing if we just return false or true - open to suggestions on best approach here.

class.jetpack-gutenberg.php

matticbot · 2020-02-20T00:11:41Z

glendaviesnz, Your synced wpcom patch D38972-code has been updated.

matticbot · 2020-02-20T00:12:20Z

glendaviesnz, Your synced wpcom patch D38972-code has been updated.

matticbot · 2020-02-20T00:40:28Z

glendaviesnz, Your synced wpcom patch D38972-code has been updated.

matticbot · 2020-02-20T01:00:44Z

glendaviesnz, Your synced wpcom patch D38972-code has been updated.

class.jetpack-gutenberg.php

jeherve

This seems to work nicely in my tests with the Markdown block:

It may be useful to add some testing instructions to your PR so others can test.

For example, I am not sure what one should use as value for the wp requirement when registering a block, though. For example right now I can develop a block that uses features introduced in Gutenberg 7.5.0. I don't necessarily know when that version of Gutenberg will make it to WordPress Core though, do I? In this case what should one use? A random high WordPress version number that we're not likely to reach anytime soon?

class.jetpack-gutenberg.php

…quired version of gutenberg will go out it

glendaviesnz · 2020-02-23T23:23:01Z

I am not sure what one should use as value for the wp requirement when registering a block, though. For example right now I can develop a block that uses features introduced in Gutenberg 7.5.0. I don't necessarily know when that version of Gutenberg will make it to WordPress Core though, do I

Ah, nice catch, hadn't thought about it from that angle. Making up a random future core release number seems a bit hacky to me though - I have pushed an alternative approach that makes the wp version optional if it is know, and updated the comments to explain this - let me know what you think of this approach - the main downside I can see is that teams will need to be disciplined about circling back and adding wp version to the version requirements array once it is known if the version gate is still required at that point.

matticbot · 2020-02-23T23:32:18Z

glendaviesnz, Your synced wpcom patch D38972-code has been updated.

glendaviesnz · 2020-02-23T23:34:09Z

It may be useful to add some testing instructions to your PR so others can test.

Done.

jeherve

This works well for me right now. 👍

scruffian · 2020-02-24T17:15:04Z

r203276-wpcom

* 8.3 release: changelog * Changelog: add #14516 * Changelog: add #14574 * Bring in changes from 8.2.1 and 8.2.2 * Update stable version * Bring in 8.2.3 changes * Changelog: add #14714 * Changelog: add #14639 * Changelog: add #14678 * Changelog: add #14673 * Changelog: add #14687 * Changelog: add #14704 * Changelog: add #14702 * Changelog: add #14541 * Changelog: add #14657 * Changelog: add #14622 * Changelog: add #14582 * Changelog: add #14638 * Changelog: add #14633 * Changelog: add #14571 * Changelog: add #14592 * Changelog: add #14539 * Changelog: add #14514 * Changelog: add #14643 * Changelog: add #14494 * Changelog: add #13739 * Changelog: add #14707 * Changelog: add #14736 * Changelog: add #14706 * Changelog: add #14730 * Changelog: add #14685 * Changelog: add #14727 * Changelog: add #14711 * Changelog: add #14742 * Changelog: add #14746 * Changelog: add #14725 * Changelog: add #13999 * Changelog: add #14740 * Changelog: add #14759 * Changelog: add #14703 * Changelog: add #14753 * Changelog: add #14754 * Changelog: add #14645 * Cahngelog: add #14599

matticbot added the Touches WP.com Files label Feb 16, 2020

simison reviewed Feb 16, 2020

View reviewed changes

glendaviesnz changed the title ~~Initial commit of block version gating~~ [WIP] Block version gating Feb 17, 2020

pento reviewed Feb 17, 2020

View reviewed changes

jeherve added [Status] In Progress [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Feb 17, 2020

jeherve reviewed Feb 17, 2020

View reviewed changes

class.jetpack-gutenberg.php Outdated Show resolved Hide resolved

pablinos reviewed Feb 17, 2020

View reviewed changes

glendaviesnz changed the title ~~[WIP] Block version gating~~ Block version gating Feb 18, 2020

glendaviesnz marked this pull request as ready for review February 18, 2020 19:29

glendaviesnz requested a review from a team February 18, 2020 19:29

pablinos reviewed Feb 18, 2020

View reviewed changes

class.jetpack-gutenberg.php Outdated Show resolved Hide resolved

scruffian reviewed Feb 19, 2020

View reviewed changes

class.jetpack-gutenberg.php Show resolved Hide resolved

jeherve requested changes Feb 19, 2020

View reviewed changes

jeherve assigned glendaviesnz Feb 19, 2020

jeherve added this to the 8.3 milestone Feb 19, 2020

Initial commit of block version gating

a5e9238

Improve naming and comments

8daaa77

[not verified] Fix tests

6604ce7

Fix broken test

dace3be

add fullstop to comment

ef7fd20

jeherve requested changes Feb 20, 2020

View reviewed changes

class.jetpack-gutenberg.php Outdated Show resolved Hide resolved

jeherve reviewed Feb 21, 2020

View reviewed changes

class.jetpack-gutenberg.php Outdated Show resolved Hide resolved

jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] In Progress labels Feb 21, 2020

Make WP version optional as we may not know which core release the re…

e07aa21

…quired version of gutenberg will go out it

jeherve approved these changes Feb 24, 2020

View reviewed changes

glendaviesnz merged commit 0ea25dc into master Feb 24, 2020

glendaviesnz deleted the add/block-plan-version-gating branch February 24, 2020 09:35

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

scruffian mentioned this pull request Feb 24, 2020

Update the tests for block gating so that they don't fail on wpcom #14778

Merged

jeherve added [Status] Has Changelog and removed [Status] Needs Changelog labels Feb 25, 2020

jeherve added a commit that referenced this pull request Feb 25, 2020

Changelog: add #14703

f1f196a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Block version gating #14703

Block version gating #14703

glendaviesnz commented Feb 16, 2020 •

edited

Loading

matticbot commented Feb 16, 2020

jetpackbot commented Feb 16, 2020 •

edited

Loading

simison Feb 16, 2020

glendaviesnz Feb 16, 2020

glendaviesnz Feb 16, 2020

matticbot commented Feb 16, 2020

matticbot commented Feb 16, 2020

matticbot commented Feb 17, 2020

pento Feb 17, 2020

glendaviesnz Feb 17, 2020

pablinos left a comment

pablinos Feb 17, 2020

matticbot commented Feb 17, 2020

matticbot commented Feb 17, 2020

glendaviesnz commented Feb 17, 2020

glendaviesnz commented Feb 17, 2020

matticbot commented Feb 17, 2020

pablinos commented Feb 18, 2020

pablinos commented Feb 18, 2020

scruffian commented Feb 19, 2020

jeherve Feb 19, 2020

glendaviesnz Feb 20, 2020

matticbot commented Feb 20, 2020

matticbot commented Feb 20, 2020

matticbot commented Feb 20, 2020

matticbot commented Feb 20, 2020

jeherve left a comment

glendaviesnz commented Feb 23, 2020

matticbot commented Feb 23, 2020

glendaviesnz commented Feb 23, 2020

jeherve left a comment

scruffian commented Feb 24, 2020

Block version gating #14703

Block version gating #14703

Conversation

glendaviesnz commented Feb 16, 2020 • edited Loading

Changes proposed in this Pull Request:

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Testing instructions:

matticbot commented Feb 16, 2020

jetpackbot commented Feb 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matticbot commented Feb 16, 2020

matticbot commented Feb 16, 2020

matticbot commented Feb 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pablinos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matticbot commented Feb 17, 2020

matticbot commented Feb 17, 2020

glendaviesnz commented Feb 17, 2020

glendaviesnz commented Feb 17, 2020

matticbot commented Feb 17, 2020

pablinos commented Feb 18, 2020

pablinos commented Feb 18, 2020

scruffian commented Feb 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matticbot commented Feb 20, 2020

matticbot commented Feb 20, 2020

matticbot commented Feb 20, 2020

matticbot commented Feb 20, 2020

jeherve left a comment

Choose a reason for hiding this comment

glendaviesnz commented Feb 23, 2020

matticbot commented Feb 23, 2020

glendaviesnz commented Feb 23, 2020

jeherve left a comment

Choose a reason for hiding this comment

scruffian commented Feb 24, 2020

glendaviesnz commented Feb 16, 2020 •

edited

Loading

jetpackbot commented Feb 16, 2020 •

edited

Loading