Full Site Editing: add a is_fse_active property to the Site object in the REST API

[jeherve]

Changes proposed in this Pull Request:

Adding a property to the Site object since 1. the blog-stickers endpoint is only for automattians, and 2. to avoid duplicating theme support logic on the backend and in Calypso

Also introduce the wpcom_is_full_site_editing_active function and use it to hide the Customizer menu in wp-admin

Reviewers: #cylon_team, #serenity_team, helpingcat

Reviewed By: #serenity_team, helpingcat

Subscribers: helpingcat

Tags: #touches_jetpack_files

Differential Revision: D31148-code, D31319-code

This commit syncs r194971-wpcom and r195033-wpcom.

Testing instructions:

Instructions in Automattic/wp-calypso#35106

also: the Customizer menu link should likewise be hidden under the same circumstance in wp-admin

Proposed changelog entry for your changes:

• WordPress.com API: add option to manage Full Site Editing.

[jetpackbot]

	Warnings
⚠️	pre-commit hook was skipped for one or more commits

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 2bf46e2

[jeherve]

@mattwiebe Would we need to edit sal/class.json-api-site-base.php as well to add the new is_fse_active function there?

[mattwiebe]

Would we need to edit sal/class.json-api-site-base.php as well to add the new is_fse_active function there?

I'm not certain, possibly? MY SAL-fu is weak. We're going to need to detect this on the JP site's side via having the FSE plugin (full-site-editing/full-site-editing.php I believe) active, and likely also a supported theme. That should be detectable via current_theme_supports( 'full-site-editing' ).

[jeherve]

Alright, thanks for the explanation. I can't seem to be able to get the Customizer link to disappear, even on a WordPress.com site, so I am probably not the best person to keep looking into this. :)

I'll let y'all figure this out as you keep working on this and add the conditions you've mentioned for Jetpack sites.

[mattwiebe]

Yup we'll figure it out, thanks for the ping. :)

[noahtallen]

What should our next steps be here in terms of testing? Since that other PR has landed, is there something different we are testing here?

[gwwar]

I need to walk though these steps, but for reviewers I think this can be tested with the following

• Spin up a new Jetpack site with https://jurassic.ninja/create?jetpack-beta&branch=fusion-sync/jeherve/r194971-wpcom-1565160115
• Verify that jetpack-beta is pointing at this branch instead of stable or master
• Install the full site editing plugin from the dotorg repository, and activate
• Install the Gutenberg plugin from the dotorg repository and activate
• Install the Modern Business theme from https://wordpress.com/themes/ and activate
• Page editing now looks like in full screen mode:

- Requests to site APIs should default to remote, so I think then all we need to do is verify that we see the new property on the wpcom sites response.

[gwwar]

So I'm seeing the following behavior:

Single Site Request

A 400 when we forward to the remote site in a
/sites/{site} request:

@jeherve should I not be testing with Jurassic Ninja here, or should it work? I've connected the site.

@mattwiebe would it be possible to please check for server errors here?

All Sites

On /me/sites we use cached site data on WordPress.com and don't request information from the remote. Here, we see the new is_fse_active property but this is always false, even with all the steps above.

@mattwiebe any other steps to follow? Maybe I need to simulate an Atomic site here?

[gwwar]

I wonder if there is a difference here between theme names, eg "pub/modern-business" and "modern-business" on the whitelist checks.

[jeherve]

A 400 when we forward to the remote site in a /sites/{site} request
should I not be testing with Jurassic Ninja here, or should it work? I've connected the site.

It should work with any site really, JN or an Atomic site. However, as you can see in the error message you get a 500 error from the site. I assume this is because of this:
#13196 (comment)

Once you add this function to sal/class.json-api-site-base.php, you should see is_fse_active in the response from the API, and it will return whatever you've set in your function.

On /me/sites we use cached site data on WordPress.com and don't request information from the remote. Here, we see the new is_fse_active property but this is always false

I believe that's to be expected, since that's the value that's currently returned in public.api/rest/sal/class.json-api-site-jetpack-shadow.php.

I wonder if there is a difference here between theme names, eg "pub/modern-business" and "modern-business" on the whitelist checks.

This would definitely have an impact. If you install a theme from WordPress.com on a Jetpack site, from the Themes screen in Calypso, it will have a slug like modern-business-wpcom. You would have to take that into account in your checks.

[mattwiebe]

Had some fun last week with deploy fatals but I'm circling back to this :)

[mattwiebe]

Ok, 223c5c7268966b607898826e69d2c56f3510a573 should get the job done. I don't know if this has made a mess of sync @jeherve, please let me know if there's a more preferred way of doing this. :)

[gwwar]

If the files are already whitelisted, I think the whole fusion dance is happier starting from a Jetpack PR, which then creates the diff/updates it.

[gwwar]

Still getting a 400 on the single site endpoint and false for me/sites. There might be a separate issues to work on for getting this value to be true, in the right cases for Jetpack sites.

[mattwiebe]

Hmm I definitely don't get a 400 on a single site endpoint with this branch active, but D31808-code should take care of /me/sites.

[jeherve]

I don't know if this has made a mess of sync @jeherve

You should be all good on that front 👍.

• /me/sites and /me/site/%s work well for me now, when I am sandboxed with D31808-code applied. They both return true for is_fse_active.
• I still see the Customize link in Calypso. That seems like a bug, or maybe I am missing something.
• If I try to edit a page, I get the full site editing experience.

---

Nitpicking and not blocking, but a few minor changes would help us get rid of PHPCS warnings here:

diff --git a/sal/class.json-api-site-jetpack.php b/sal/class.json-api-site-jetpack.php
index 0af76af2a..f13fbbd88 100644
--- a/sal/class.json-api-site-jetpack.php
+++ b/sal/class.json-api-site-jetpack.php
@@ -189,10 +189,11 @@ class Jetpack_Site extends Abstract_Jetpack_Site {
 
 	function is_fse_active() {
 		$fse_enabled = is_plugin_active( 'full-site-editing/full-site-editing-plugin.php' );
-		$has_method = method_exists( '\A8C\FSE\Full_Site_Editing', 'is_supported_theme' );
+		$has_method  = method_exists( '\A8C\FSE\Full_Site_Editing', 'is_supported_theme' );
 		if ( $fse_enabled && $has_method ) {
-			$fse = \A8C\FSE\Full_Site_Editing::get_instance();
+			$fse  = \A8C\FSE\Full_Site_Editing::get_instance();
 			$slug = get_option( 'stylesheet' );
+
 			return $fse->is_supported_theme( $slug );
 		}
 		return false;

[mattwiebe]

I still see the Customize link in Calypso. That seems like a bug, or maybe I am missing something.

Hmm yeah, as long as we get a true response from the API, we should be able to fix things on the Calypso side. Shouldn't block this.

Anyway both your suggestions have landed @jeherve

[jeherve]

LGTM now. 👍 I'll let someone else review; I can't approve on my own since I created the PR :)

[gwwar]

🤔 Maybe just something with my setup then I'm still seeing 400s. @mattwiebe maybe add a screenshot of a good response, and ✅ on the review?

[mattwiebe]

Here's a good response from my Jetpack site with this PR checked out:

[gwwar]

Go ahead and approve then @mattwiebe, I can see if we can get a +1 on manual testing, but I think this is probably okay

[mattwiebe]

Looks like @jeherve needs someone else from Automattic/jetpack-crew to approve but I'm happy with it :)

[oskosk]

LGTM