Gutenberg Integration: load editor with a new post #27074

rodrigoi · 2018-09-07T18:51:27Z

This PR adds the bare minimum wiring to Calypso to support the API v2 Post format, so Gutenberg can somewhat start to function.

Depends on D18064-code

Gutenberg relies heavily on the provided post prop, with a schema as returned by the wp/v2 API when the edit context is provided. Without a valid post, Gutenberg fails to load most of the sidebar tools.
This PR adds as little code as possible to wire the wp/v2 endpoints needed to create a draft and load the post so Gutenberg can use the returned post.

This PR is based on the createNewPost function on p2-gutenberg:

export const createNewPost = () => {
	return new Promise( ( resolve ) => {
		jQuery.wpcom_proxy_request( {
			path: `/sites/${ window._currentSiteId }/p2/post`,
			apiNamespace: 'wpcom/v2',
		} ).then( ( { ID } ) => {
			wp.apiRequest( {
				path: `/wp/v2/posts/${ ID }?context=edit`,
			} ).then( resolve );
		} );
	} );
};

~~### State~~

The post stored on Calypso's redux state is insufficient for Gutenberg, that relies on fields returned by the API when querying for a post and the edit context is provided. In particular, the sidebar uses the _links metadata returned by the API to decide whether or not to populate hierarchical and flat lists like categories and taxonomies, despite the fact that those values are queried and stored on core and core/data redux store.
Gutenberg stores the current post information on the currentPost key on the core/editor store. Calypso has to do something similar, so, on the gutenberg.currentPost branch on Calypso's redux store, we keep the information returned from the API call.

~~### Query Components~~

~~This PR also incorporates two query components, QueryGutenbergCreatePost to create a new auto draft, and QueryGutenbergSitePost to query any post using the wp/v2 endpoint.~~

The App Component holding the Gutenberg instance implements only QueryGutenbergCreatePost. When the page load, a new draft is created, fetched, loaded into state and sent over the props to Gutenberg. Refresh the page, and a new one is created.

~~### Data Layer~~

This PR also breaks new ground because it implements the first two endpoints for the wp/v2 API namespace for calypso. I've made the controversial decision of creating a new handler (wpHandlers) and a new top level folder structure to keep things apart.

The requestGutenbergPostDraft request handler dispatches the fetch of the newly created draft on success, and there are no state management associated with that action. I initially implemented tracking on different active drafts, but removed all that complexity in favor of fixing the rest of the integration problems. We can worry about multiple instances of Gutenberg in the future.

How To Test and What to Expect

Do not expect much 😎

~~There are some basic unit tests for the state management and the data layer that can be run with:~~

To test the Gutenberg integration, you'll have to apply D18064-code and sandbox public-api.wordpress.com to use the new create draft endpoint. Then, navigate to /gutenberg/post/{site-slug}.

Gutenberg should load with all the sidebar elements active. The Categories panel should load all the available categories, and the authors should load on the Status and Visibility panel. Taxonomies load on the store, but for reason to explore on another PR they are not loading on the token input.

The Categories block should load all the categories, but autosave and the rest of the functionality is sadly broken. This PR should enable work on those features.

matticbot · 2018-09-07T18:51:28Z

Test live: https://calypso.live/?branch=add/gutenberg-api-site-post

dmsnell · 2018-09-07T19:28:33Z

client/components/data/query-gutenberg-create-post/index.jsx

+export default connect(
+	null,
+	{ createGutenbergPostDraft }
+)( QueryGutenbergCreatePost );


this is a pretty unexpected use of a query component - is the CreatePostDraft a kind of resource we might imagine polling for or is it intended to be a kind of action like "create this post draft"?

if it's the first would it make more sense to call it something like <QueryPostDraft /> and if it's the second would it make sense to dispatch it as an action instead and use some declarative query component to retrieve it?

Generally query components are reserved for GETs and intended to be idempotent (can be called multiple times without side-effects). Unless I'm misreading this, let's not use a query component and instead dispatch this in the parent component.

client/gutenberg/editor/main.jsx

dmsnell · 2018-09-07T19:36:00Z

These new resources look an awful lot like "shove data"

they are requested directly from an API endpoint
a new reducer does nothing more than opaquely load it into state
a new selector plainly grabs it from state

In this case I think that httpData might be a valid use case here which could trim down the PR size, reduce the surface area of the changes, and provide a more declarative API for the code

import { requestGutenbergDraftPost as requestDraft } from 'state/data-getters';

const mapStateToProps = ( state, { siteId } ) => ( {
	draftPost: requestDraft( siteId ).data
} )

export const requestGutenbergDraftPost = siteId => requestHttpData(
	`gutenberg-draft-post-${ siteId }`,
	http( {
 		path: `/sites/${ siteId }/posts/create-draft`,
 		method: 'GET',
 		apiNamespace: 'wp/v2',
 	} ),
	{
		fromApi: () => post => [ [ `gutenberg-draft-post-${ siteId }`, post ] ]
	}
);

gwwar · 2018-09-08T01:06:36Z

client/state/action-types.js

@@ -262,6 +262,9 @@ export const GUIDED_TRANSFER_STATUS_REQUEST = 'GUIDED_TRANSFER_STATUS_REQUEST';
 export const GUIDED_TRANSFER_STATUS_REQUEST_FAILURE = 'GUIDED_TRANSFER_STATUS_REQUEST_FAILURE';
 export const GUIDED_TRANSFER_STATUS_REQUEST_SUCCESS = 'GUIDED_TRANSFER_STATUS_REQUEST_SUCCESS';
 export const GUTENBERG_OPT_IN_DIALOG_IS_SHOWING = 'GUTENBERG_OPT_IN_DIALOG_IS_SHOWING';
+export const GUTENBERG_CREATE_POST_DRAFT = 'GUTENBERG_CREATE_POST_DRAFT';
+export const GUTENBERG_SITE_POST_REQUEST = 'GUTENBERG_SITE_POST_REQUEST';
+export const GUTENBERG_SITE_POST_RECEIVE = 'GUTENBERG_SITE_POST_RECEIVE';


In terms of naming should we be prefixing with Gutenberg? These are core v2 schema posts right?

I'll also recommend a more descriptive name that would allow some of these actions to make sense out of the context of any server action:
*POST_ADD vs *_SITE_POST_RECEIVE
*POST_DRAFT_ADD vs * _CREATE_POST_DRAFT (it sounds weird but actions should be postfixed).

In terms of naming

the core project has been starting to eschew the name Gutenberg in code review. I hope this makes your day more interesting. they are moving to internalize the name "editor" and the like.

I ran into this while working on the parser stuff - renames from things like Gutenberg_Block to WP_Block

just wanted to throw that out there 😬

gwwar · 2018-09-08T01:08:31Z

client/state/gutenberg/test/reducer.js

+/**
+ * External dependencies
+ */
+import { expect } from 'chai';


Remove this :D Let's use jest. No need to import chai.

wasn't sure exactly what we were using, so I defaulted to chai. Fixed on e2a414c.

gwwar · 2018-09-08T01:18:20Z

client/state/index.js

@@ -136,6 +137,7 @@ const reducers = {
 	form,
 	googleAppsUsers,
 	googleMyBusiness,
+	gutenberg,


Should this be gutenberg? wpv2 maybe?

gwwar · 2018-09-08T01:21:38Z

client/gutenberg/editor/main.jsx


 		return (
-			<Editor settings={ editorSettings } hasFixedToolbar={ true } post={ post } onError={ noop } />
+			<Fragment>
+				{ isEmpty( post ) && <QueryGutenbergCreatePost siteId={ siteId } /> }


Can this not be done on componentDidUpdate/componentDidMount? I think it's fine to copy pasta those lifecycle methods from QueryComponent and get rid of it. An HOC might be another alternative if we have many usages.

gwwar · 2018-09-08T01:35:02Z

Promising start @rodrigoi

@dmsnell if you'd like us to use a new pattern here, don't forget to link to additional context. I reviewed #22549 myself and I had to refresh myself.

I do think we'll end up using this wp/v2 post by multiple components (for example, imagine a draft selector), but we can maybe start with this if @rodrigoi is up for this.

@dmsnell I believe using a key of gutenberg-draft-post-${ siteId } will return an incorrectly cached response if we visit the post page of a site twice.

dmsnell · 2018-09-08T11:52:18Z

don't forget to link to additional context

@gwwar thanks for the link! the geolocation data refactor is a simple example of the conversion too.

I do think we'll end up using this wp/v2 post by multiple components

If I understand what you are saying here then it shouldn't be a problem. With httpData it's convenient for one-off data but if we start using it in multiple places then our method of abstraction is just creating a separate function in a separate module - the data-getters directory. Each component can use it and even provide their own freshness constraints and the freshest constraint will hold.

a key of gutenberg-draft-post-${ siteId } will return an incorrectly cached response if we visit the post page of a site twice.

this suggestion may come from my general confusion mentioned in comments above about what we're actually trying to fetch. if it is clear what we're fetching we can create a clear key. I'm not sure how to change this though in my ignorance of the goal.

are you saying that this endpoint isn't cacheable? if so we can look at ways to take that into account.

the suggestion about httpData here is mainly to prevent adding code we don't need such as the new action types and Redux mechanics. would be happy to share more if you find it valuable, just ask!

vindl · 2018-09-10T22:03:43Z

@rodrigoi just a note that I've merged Gutenberg package updates (#27022) and API middleware refactor (#27009) which could influence the work here (especially the new package versions). I'd suggest rebasing sooner rather than later to avoid encountering strange errors further down the road. :)

rodrigoi · 2018-09-11T15:56:04Z

ok, I've went ahead with @dmsnell recommendation and re-wrote all the HTTP calls using functions that dispatch a requestHttpData on state/data-setters. This has reduced considerably the size of the PR (from 500 lines to 41), but most importantly, has removed all the boilerplate code needed to wire in the data-layer. As much as I love the data-layer, it's a bit verbose and setting up a request there adds a lot of redux boilerplate.
So the requestHttpData pattern is a great addition for transient data. In this particular case, since Gutenberg has its own state management, this will remove the unnecessary duplication of Calypso having to keep track of the current post.

As of today, the only section using the wp/v2 namespace in Calypso is the post revisions dialog, that fetches /sites/${ siteId }/users. I think we'll have to consider how to structure our data-layer for wp/v2 migration in the future (naming collision was an issue for this PR), it's nice to remove that responsibility from this PR.

Anyway, this is ready for another pass.

kwight · 2018-09-11T17:57:49Z

I gave this a run, but had odd behaviour.

there's some sort of autosave action loop; autosave network calls keep getting made, while the UI flaps between "Autosaving" and "Autosaved"
however, only one draft was saved to the database

I was properly sandboxed with D18064 (I could see the error_logs from that patch in my tail.)

kwight · 2018-09-11T18:15:20Z

Oh, noting I also saw a bunch of these, not sure if that's expected or not:

gwwar · 2018-09-11T20:55:48Z

client/state/data-getters/index.js

+
+export const requestGutenbergDraftPost = siteId =>
+	requestHttpData(
+		`gutenberg-draft-post-${ siteId }`,


I don't believe this key is sufficient. For example, what happens when we load the new post page multiple times? We probably need to assign a temporary draft id, until the response comes back.

If we look at media for example, I believe the transient object we create is uniqueId( 'media-' ),

We only need to make this unique to the client, so we can do something similar here with:

uniqueId( `gutenberg-draft-post-${ siteId }-` )

Careful! It looks like we're also creating a new post from a GET request.

Fixed the GET verb on d77f521 and updated D18064-code also. Please apply the patch again before testing.

About the requestId, I don't think it'll be a problem to use that key. I haven't found any problems (blocked requests) when reloading the page or loading multiple pages. But using an uniqueId is always a good idea.

I've implemented two versions, one using the route's controller (d2d95b6fcdf978ecefc68db4fe792e299a10daa3):

export const post = ( context, next ) => { const postId = getPostID( context ); const uniqueDraftId = uniqueId( 'gutenberg-draft-' ); //reserved space for adding the post to context see post-editor/controller.js context.primary = <GutenbergEditor { ...{ postId, uniqueDraftId } } />; next(); };

and another using a HOC (fd77025afdb0c1d372155fb900c0fc036da7e10b, e8899f785f64af9402488b287312536dc800e608 and 714beb69a514e8ec2d9a71a038a4198d9925155c):

export const withUniqueDraftId = WrappedComponent => { return class extends Component { constructor( props ) { super( props ); this.state = { uniqueDraftId: uniqueId( 'gutenberg-draft-' ), }; } render() { const { uniqueDraftId } = this.state; return <WrappedComponent { ...{ ...this.props, uniqueDraftId } } />; } }; };

export default withUniqueDraftId( connect( mapStateToProps )( GutenbergEditor ) );

You can verify that the requestId is passed down to the HTTP_DATA_REQUEST action on the redux inspector:

which version do we prefer? I'm leaning towards using the controller just because it's far less code, but composing things is nice too.

gwwar

Thanks for the updates here @rodrigoi Let's make this a bit easier on ourselves and ONLY target one case. Eg, can I open an existing post? Eg /gutenberg/post/myid

Let's also remove the autosaveMonitor like we do in #27060 for now and no-op on the new post path. I can open up a new issue specific to the new post case, and another one to get autosave going.

Added #27140 to track this.

rodrigoi · 2018-09-11T22:42:45Z

@kwight thanks for taking the time to test this! The autosave problem is expected and will have it's own diff. The same with the lifecycle warnings.

The sign that this works is authors and categories loading correctly on the sidebar and the cagetories block. Publish is broken, and will most likely need changes on the <BrowserURL /> component, but then again, that's for another PR to fix.

gwwar · 2018-09-11T23:05:16Z

client/state/data-getters/index.js

+			{}
+		),
+		{ formApi: () => ( { ID } ) => [ [ `gutenberg-draft-post-${ siteId }`, ID ] ] }
+	);


We can probably remove this for now and revisit how to handle autosave drafts in a follow up PR. requestHttpData here feels incorrect for this POST case and is even more unexpected in mapStateToProps. Overall I think we'd want to trigger specific requests to create the draft only after we have some user input.

If we want to keep this pretty lightweight later I think we'd end up making a create action, but we could potentially still keep a poll going of the post object using requestHttpData.

Thoughts here @dmsnell ?

…p/v2 API Endpoint

…/v2 site post.

…/onFailure" warnings.

…nique draft id, in case it's needed

…unique draft ids

pairing!

gwwar · 2018-09-12T17:00:35Z

Had a good pairing session with @rodrigoi ✨ to help untangle why we needed a new endpoint. To our understanding underneath the hood all Gutenberg instances need an autodraft. Normally, this is provided by the PHP shell in wp-admin as a hidden form field, but needs to be requested in a standalone instance like Calypso or p2.

We're reusing an internal endpoint here (limited to p2 sites + a8c users) for now while we discuss where new custom v2 endpoints and how this should be shaped. Expect posts and additional issues to follow.

Work here should at least uncover new classes of errors to start digging into that we can get folks started on. :D

vindl · 2018-09-18T01:55:13Z

The same with the lifecycle warnings.

@kwight @rodrigoi these are coming from Gutenberg packages and can't be fixed in Calypso. You can check out #26643 for more details.

vindl · 2018-09-18T02:03:47Z

To our understanding underneath the hood all Gutenberg instances need an autodraft. Normally, this is provided by the PHP shell in wp-admin as a hidden form field, but needs to be requested in a standalone instance like Calypso or p2.

@gwwar It sounds like a better way to fix this would be in Gutenberg core - adding a new endpoint there and using it during editor initialization instead of hidden form fields? Otherwise it looks like this could add maintenance burden (errors) during updates.

gwwar · 2018-09-18T02:08:08Z

Let's follow up with an issue to see if we can help make changes in core to avoid this. :) Do try and take your afk though @vindl !

rodrigoi added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Data [Goal] Gutenberg Working towards full integration with Gutenberg labels Sep 7, 2018

rodrigoi added this to the Calypso Gutenberg Integration milestone Sep 7, 2018

rodrigoi self-assigned this Sep 7, 2018

rodrigoi requested a review from a team September 7, 2018 18:51

rodrigoi changed the title ~~Gutenberg Integration: create and load auto draft~~ Gutenberg Integration: load editor with a new post Sep 7, 2018

dmsnell reviewed Sep 7, 2018

View reviewed changes

client/gutenberg/editor/main.jsx Outdated Show resolved Hide resolved

gwwar requested a review from a team September 8, 2018 00:57

gwwar reviewed Sep 8, 2018

View reviewed changes

gwwar requested a review from a team September 8, 2018 01:46

rodrigoi force-pushed the add/gutenberg-api-site-post branch from 90ba14b to d7d826b Compare September 11, 2018 16:01

gwwar reviewed Sep 11, 2018

View reviewed changes

gwwar previously requested changes Sep 11, 2018

View reviewed changes

gwwar reviewed Sep 11, 2018

View reviewed changes

Rodrigo Iloro added 2 commits September 12, 2018 13:35

Gutenberg Integration: Adds support to load Post contents using the w…

c05cfb9

…p/v2 API Endpoint

Adds selector to retrieve the current loaded post for Gutenberg.

653455f

Rodrigo Iloro added 14 commits September 12, 2018 13:37

Removes query components in favor of using plain data getters

2b0379c

Removing all the boylerplate no longer needed to request draft and wp…

e862e50

…/v2 site post.

Removes test code (http -> rawHttp) on requestSiteAlerts.

c460f8a

Adds empty action to http action creator to renive "missing onSuccess…

aaa78e8

…/onFailure" warnings.

Fixes some missing imports after rebasing with master

070b4cd

Removes unused siteId prop from connect()

df5abb4

Updates the request post draft method to use POST verb instead of GET

d51c397

Extremely basic support for postId from the route

4ecc235

New component to inject a unique draft Id into the editor instance

6029852

Request Gutenberg draft now accepts a draft Id

1bb3de6

wraps the Gutenberg editor connected component so it can access the u…

64e793e

…nique draft id, in case it's needed

Adds a unique draft Id on the controller

94869f6

Removes unused HOC component and refactors mapStateToProps to handle …

c1392de

…unique draft ids

Reuses the p2-gutenberg create auto-draft endpoint

816e624

rodrigoi force-pushed the add/gutenberg-api-site-post branch from c231c66 to 816e624 Compare September 12, 2018 16:46

rodrigoi added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Sep 12, 2018

gwwar approved these changes Sep 12, 2018

View reviewed changes

rodrigoi merged commit 4264c7f into master Sep 12, 2018

rodrigoi deleted the add/gutenberg-api-site-post branch September 12, 2018 20:26

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 12, 2018

This was referenced Sep 13, 2018

Gutenberg: fix missing siteId white screen of death #27191

Merged

Gutenberg: Open an existing post #27140

Closed

This was referenced Sep 14, 2018

Data Layer: Correct fromApi typo in draft post requests. #27221

Merged

Data Layer: Correct fromApi typo in draft post requests (take 2). #27258

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gutenberg Integration: load editor with a new post #27074

Gutenberg Integration: load editor with a new post #27074

rodrigoi commented Sep 7, 2018 •

edited

Loading

matticbot commented Sep 7, 2018

dmsnell Sep 7, 2018

gwwar Sep 8, 2018 •

edited

Loading

dmsnell commented Sep 7, 2018 •

edited

Loading

gwwar Sep 8, 2018

dmsnell Sep 8, 2018

gwwar Sep 8, 2018

rodrigoi Sep 8, 2018

gwwar Sep 8, 2018

gwwar Sep 8, 2018

gwwar commented Sep 8, 2018 •

edited

Loading

dmsnell commented Sep 8, 2018

vindl commented Sep 10, 2018

rodrigoi commented Sep 11, 2018

kwight commented Sep 11, 2018

kwight commented Sep 11, 2018

gwwar Sep 11, 2018

gwwar Sep 11, 2018 •

edited

Loading

gwwar Sep 11, 2018 •

edited

Loading

rodrigoi Sep 12, 2018

gwwar left a comment •

edited

Loading

rodrigoi commented Sep 11, 2018

gwwar Sep 11, 2018

gwwar commented Sep 12, 2018 •

edited

Loading

vindl commented Sep 18, 2018

vindl commented Sep 18, 2018 •

edited

Loading

gwwar commented Sep 18, 2018

Gutenberg Integration: load editor with a new post #27074

Gutenberg Integration: load editor with a new post #27074

Conversation

rodrigoi commented Sep 7, 2018 • edited Loading

How To Test and What to Expect

matticbot commented Sep 7, 2018

Choose a reason for hiding this comment

gwwar Sep 8, 2018 • edited Loading

Choose a reason for hiding this comment

dmsnell commented Sep 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gwwar commented Sep 8, 2018 • edited Loading

dmsnell commented Sep 8, 2018

vindl commented Sep 10, 2018

rodrigoi commented Sep 11, 2018

kwight commented Sep 11, 2018

kwight commented Sep 11, 2018

Choose a reason for hiding this comment

gwwar Sep 11, 2018 • edited Loading

Choose a reason for hiding this comment

gwwar Sep 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gwwar left a comment • edited Loading

Choose a reason for hiding this comment

rodrigoi commented Sep 11, 2018

Choose a reason for hiding this comment

gwwar commented Sep 12, 2018 • edited Loading

vindl commented Sep 18, 2018

vindl commented Sep 18, 2018 • edited Loading

gwwar commented Sep 18, 2018

rodrigoi commented Sep 7, 2018 •

edited

Loading

gwwar Sep 8, 2018 •

edited

Loading

dmsnell commented Sep 7, 2018 •

edited

Loading

gwwar commented Sep 8, 2018 •

edited

Loading

gwwar Sep 11, 2018 •

edited

Loading

gwwar Sep 11, 2018 •

edited

Loading

gwwar left a comment •

edited

Loading

gwwar commented Sep 12, 2018 •

edited

Loading

vindl commented Sep 18, 2018 •

edited

Loading