REST API: Add /gutenberg/available-extensions endpoint by ockham · Pull Request #10710 · Automattic/jetpack

ockham · 2018-11-23T12:55:07Z

Based on @roccotripaldi's D21225-code. As I commented there,

Since we'll want to use the same logic to determine block availability in Gutenblocks both on WP.com, and Jetpack (which means using this endpoint and dropping the Jetpack_Editor_Initial_State based approach), I think it'd make sense to add this endpoint to Jetpack and sync it to WP.com (similar to what Mike did for the Publicize endpoint(s), see #10605)

Changes proposed in this Pull Request:

Add /gutenberg/available-extensions endpoint to expose Gutenberg block and plugin availabilty

Testing instructions:

Manual Tests

(stolen from Mike's for #10605)

The best way to test is to make REST API requests to a test site. The simplest way to do that is to install either the REST API Console plugin or the Basic Authentication plugin.
As a user with contributor privileges or higher, make an authenticated REST API request: GET /wp-json/wpcom/v2/gutenberg/available-extensions. Verify that you get a list of blocks, with available bool attributes (and unavailable_reason strings if available is false). Verify that block/plugin availability corresponds to module activations status/plan/etc (also try changing them and verify that the endpoint's response changes accordingly).
As a user with subscriber privileges, make an authenticated REST API request: GET /wp-json/wpcom/v2/gutenberg/available-extensions, and verify that you get a 403 error. Try the same unauthenticated

Proposed changelog entry for your changes:

Add /gutenberg/available-extensions endpoint to expose Gutenberg block and plugin availabilty

Follow-up TODO

Sync to dotcom

jetpackbot · 2018-11-23T12:59:31Z

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.

Scheduled Jetpack release: November 26, 2018.
Scheduled code freeze: November 19, 2018

Generated by 🚫 dangerJS

lezama

Thanks for porting this @ockham !

lezama · 2018-11-23T13:36:29Z

Leaving a not here that we discussed with @ockham that it might make sense to also pass a post_id.

tyxla

Thanks for working on this, Bernie ❤️

Haven't tested this, but it looks really great at first glance! 💯

Left some suggestions and questions - let's discuss.

tyxla · 2018-11-23T13:59:03Z

_inc/lib/core-api/wpcom-endpoints/gutenberg-available-extensions.php

+					'type'        => 'boolean',
+				),
+				'unavailable_reason' => array(
+					'description' => __( 'Human readable reason for the extensiongutenberg/available-extensions not being available', 'jetpack' ),


Something looks a little wrong with this description

tyxla · 2018-11-23T14:01:09Z

_inc/lib/core-api/wpcom-endpoints/gutenberg-available-extensions.php

+	/**
+	 * Ensure the user has proper permissions
+	 *
+	 * @return WP_Error|boolean


current_user_can will always return a boolean AFAIK. Are we filtering it so it would return a WP_Error under some circumstances?

Nah (or at least that I'm aware of). JSDoc copypasta.

tyxla · 2018-11-23T14:03:02Z

_inc/lib/core-api/wpcom-endpoints/gutenberg-available-extensions.php

+	 * @return WP_Error|boolean
+	 */
+	public function get_items_permission_check() {
+		return current_user_can( 'edit_posts' );


Noting that usually for doing anything with modules (retrieving activation status or (de)activating), we require the jetpack_manage_modules capability. Is there any reason why we use a different one here?

I think a user can have edit_posts but not jetpack_manage_modules

Okay, but then: if we don't want the users to have access to manage modules, we don't give them the jetpack_manage_modules and the jetpack_admin_page capabilities, right? But then, with the check being like this, we expose the module information to them, regardless of whether they have the capability. That feels a bit off to me.

Your rationale makes sense to me, @tyxla. Changing to jetpack_manage_modules.

Not so fast @ockham 😆

FWIW, the modules endpoint needs the jetpack_admin_page permission for reading module activation state. It needs jetpack_manage_modules for changing the state.

So if we go by intent (display available blocks in the block picker, i.e. when editing a post) rather than implementation (check module activation state -- but also some additional information for individual blocks), edit_posts seems to make sense again... 🤔

Note that even though jetpack_admin_page falls back to read, they're still different capabilities, and if we use read instead of jetpack_admin_page, we're still introducing subtle differences. One difference could be the fact that in with Jetpack dev mode enabled, jetpack_admin_page will default to manage_options. Another could be any plugin or custom code that tinkers with the custom capability.

Is the TL;DR of the above convo that contributors can still see their blocks? 😅

Block availability isn't perfectly congruent with module activation status, since individual blocks can also depend on plans etc.

Totally. But then, since module activation status is one of the requirements, I'd expect the minimum needed capability there to be jetpack_admin_page. Any user should be able to read the current site plan, at least that's how it currently works for the Jetpack React page.

As discussed via Slack, this endpoint won't expose module activation state directly, so using edit_posts makes sense 👍

tyxla · 2018-11-23T14:03:59Z

_inc/lib/core-api/wpcom-endpoints/gutenberg-available-extensions.php

+<?php
+
+/*
+ * Plugin Name: Available Gutenberg Extensions (Blocks and Plugins) for wpcom/v2 WP-API


Is this a plugin? I didn't expect to see a plugin docblock here.

I quit as a reviewer

Copypasta strikes again 🙄

ockham · 2018-11-23T14:47:56Z

Leaving a not here that we discussed with @ockham that it might make sense to also pass a post_id.

I gave this some more thought. I thought of a scenario:

Start a new post (no post_id yet), request available blocks
Insert blocks, save post (post_id assigned)
Request available blocks again, this time passing the post_id

I can't seem to think of any plausible cases where the list of available blocks would change in such a scenario. What seems somewhat more realistic is different blocks available depending on post type, but since AFAIK even Gutenberg itself doesn't take that case into account (yet), I'm inclined to say it's safe enough to not have the endpoint accept any arguments. (Should Gutenberg change dramatically in that regard it's probably okay to bump the API version anyway.)

Unless you feel strongly about this @lezama 😄

lezama · 2018-11-23T14:51:40Z

Unless you feel strongly about this @lezama 😄

let's keep it simple for now 👍

ockham · 2018-11-23T14:58:21Z

Addressed feedback. This should be ready for another look.

tyxla

LGTM, nice work 👍

ockham · 2018-11-23T16:09:23Z

WP.com counterpart: D21280-code

enejb · 2018-11-23T18:45:45Z

_inc/lib/core-api/wpcom-endpoints/gutenberg-available-extensions.php

+ * [
+ *   { # Availabilty Object. See schema for more detail.
+ *     available:          (boolean) Whether the extension is available
+ *     unavailable_reason: (string)  Human readable reason for the extension not being available


Human readable reason for the extension not being available
is not really true. since reason usally looks like missing_module or missing_plan

oskosk

LGTM!

ockham · 2018-11-23T18:59:25Z

Thanks everyone!

* Add first version of the Changelog and testing list for 6.9 * Changelog: add #10710 * changelog: add #10538 * changelog: add #10741 * changelog: add #10749 * changelog: add #10664 * changelog: add #10224 * changelog: add #10788 * Changelog: add #10560 * Chanegelog: add #10812 * changelog: add #10556 * Changelog: add #10668 * Changelog: add #10846 * Changelog: add #10947 * Changelog: add #10962 * Changelog: add #10956 * Changelog: add #10940 * Changelog: add #10934 * Changelog: add #10912 * changelog: add #10866 * changelog: add #10924 * Changelog: add #10936 * Changelog: add #10833 * changelog: add #10867 * Changelog: add #10960 * Changelog: add #10888 * changelog: add #10840 * changelog: add #10972 * Changelog: add #10979 * changelog: add #10909 * Changelog: add #10958 * Changelog: add #10981 * Changelog: add #10564 * Changelog: add #10809 * Changelog: add #10982 * Changelog: add #10706 * Changelog: add #10978 * Changelog: add #10132 * Changelog: add #11022 * Changelog: add #11024 * Changelog: add #10875 * Changelog: add #11030 * Changelog: add #11053 * Changelog: add #10880 * Changelog: add #9359 * Changelog: add #11037 * Update block list * Changelog: add #11060 * Changelog: add #10755 * changelog: add #11000 * Changelog: add #10786 * Changelog: add #10945 * Changelog: add #10597

ockham added [Status] In Progress [Feature] WP REST API [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Nov 23, 2018

ockham self-assigned this Nov 23, 2018

ockham requested a review from a team November 23, 2018 12:55

REST API: Add /gutenberg/available-extensions endpoint

64bf1e4

ockham force-pushed the add/gutenberg-available-extensions-endpoint branch from 3d6465e to 64bf1e4 Compare November 23, 2018 13:06

Remove spurious wpcom_is_wpcom_only_endpoint flag

d2a515e

ockham added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Nov 23, 2018

ockham requested review from a team November 23, 2018 13:15

lezama reviewed Nov 23, 2018

View reviewed changes

tyxla reviewed Nov 23, 2018

View reviewed changes

Address feedback

aa34483

lezama previously approved these changes Nov 23, 2018

View reviewed changes

Change permission back to edit_posts

d9882ee

ockham dismissed lezama’s stale review via d9882ee November 23, 2018 15:56

tyxla previously approved these changes Nov 23, 2018

View reviewed changes

enejb reviewed Nov 23, 2018

View reviewed changes

oskosk previously approved these changes Nov 23, 2018

View reviewed changes

fix wording

4659e2c

lezama dismissed stale reviews from oskosk and tyxla via 4659e2c November 23, 2018 18:48

fix wording

1dbbd14

oskosk approved these changes Nov 23, 2018

View reviewed changes

lezama merged commit 58f991a into master Nov 23, 2018

matticbot added the [Status] Needs Changelog label Nov 23, 2018

lezama deleted the add/gutenberg-available-extensions-endpoint branch November 23, 2018 18:58

jeherve added [Status] Has Changelog and removed [Status] Needs Changelog [Status] Needs Review This PR is ready for review. labels Dec 7, 2018

matticbot added the Touches WP.com Files label Dec 7, 2018

jeherve added a commit that referenced this pull request Dec 19, 2018

Changelog: add #10710

a3a1b54

jeherve added a commit that referenced this pull request Jan 3, 2019

Changelog: add #10710

04c016f

Conversation

ockham commented Nov 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes proposed in this Pull Request:

Testing instructions:

Manual Tests

Proposed changelog entry for your changes:

Follow-up TODO

Uh oh!

jetpackbot commented Nov 23, 2018

Uh oh!

lezama left a comment

Choose a reason for hiding this comment

Uh oh!

lezama commented Nov 23, 2018

Uh oh!

tyxla left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ockham commented Nov 23, 2018

Uh oh!

lezama commented Nov 23, 2018

Uh oh!

ockham commented Nov 23, 2018

Uh oh!

tyxla left a comment

Choose a reason for hiding this comment

Uh oh!

ockham commented Nov 23, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oskosk left a comment

Choose a reason for hiding this comment

Uh oh!

ockham commented Nov 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

ockham commented Nov 23, 2018 •

edited

Loading