Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gutenberg: use v1.2 endpoint to create auto-draft #27673

Merged
merged 2 commits into from
Oct 9, 2018
Merged

Conversation

gwwar
Copy link
Contributor

@gwwar gwwar commented Oct 9, 2018

Fixes #27430 by updating our API call to use our v1.1 endpoint to create an auto-draft The Core wp/v2 endpoint does not allow creation of auto-drafts. Our previous custom endpoint did not allow us to fetch all post types from Jetpack sites.

Note that autosaves for custom post types don't appear to work

screen shot 2018-10-08 at 6 53 51 pm
screen shot 2018-10-08 at 6 58 40 pm

cc @mdawaffe I've added #27674

Testing Instructions

  • No regressions on posts, when navigating to http://calypso.localhost:3000/gutenberg/post/<site>
  • No regressions on pages when navigating to http://calypso.localhost:3000/gutenberg/page/<site>
  • Custom post type may be created, but autosaves fail when visiting http://calypso.localhost:3000/gutenberg/edit/<cpt>/<site>

screen shot 2018-10-08 at 6 54 00 pm

@gwwar gwwar added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Goal] Gutenberg Working towards full integration with Gutenberg labels Oct 9, 2018
@matticbot
Copy link
Contributor

@gwwar gwwar requested a review from a team October 9, 2018 02:03
@gwwar gwwar self-assigned this Oct 9, 2018
@ghost
Copy link

ghost commented Oct 9, 2018

That's a great PR description, thank you so much for your effort!

Generated by 🚫 dangerJS

export const requestSitePost = ( siteId, postId, postType ) => {
//post and page types are plural except for custom post types
//eg /sites/<siteId>/posts/1234 vs /sites/<siteId>/jetpack-testimonial/4
const s = postType === 'page' || postType === 'post' ? 's' : '';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a clever one, but I'm personally not a big fan of adding the plural s this way, and I'd rather have a more straightforward if/else-if/else, with the additional advantage of being self-explanatory.

Copy link
Contributor

@Copons Copons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No matter how we pluralize posts and pages, this works as expected.
@gwwar feel free to change the pluralization logic if you think it makes sense, but it's no blocker for :shipit:

@gwwar
Copy link
Contributor Author

gwwar commented Oct 9, 2018

I made the path logic less clever in f2383b3! Thanks for the review @Copons

@gwwar gwwar merged commit 2640086 into master Oct 9, 2018
@gwwar gwwar deleted the update/auto-draft branch October 9, 2018 18:48
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Goal] Gutenberg Working towards full integration with Gutenberg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants