WordPress.org

Make WordPress Core

Opened 11 months ago

Closed 11 months ago

Last modified 10 months ago

#38916 closed defect (bug) (fixed)

REST API: Can't set existing post format if theme does not support it

Reported by: iseulde Owned by: joehoyle
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.7
Component: REST API Keywords:
Focuses: Cc:

Description

The schema for the post format depends on the theme. If the theme does not support e.g. "standard", and an existing post has that format, then trying to update that post with "standard" as the format won't work. You get the error Invalid parameter(s): format.

In addition, the documentation says standard, aside, chat, gallery, link, image, quote, status, video, audio are all allowed, while it actually depends on the theme.

http://v2.wp-api.org/reference/posts/

Related: https://github.com/WP-API/WP-API/issues/1448

Change History (14)

#1 @jnylen0
11 months ago

  • Milestone changed from Awaiting Review to 4.7

We don't yet have a strategy for documenting parameters that have a dynamic list of values like this:

'enum' => array_values( get_post_format_slugs() ),

We can ignore the update to the format field if the value is the same as what's currently saved. Does that make sense as a way to address the issue?

#2 @iseulde
11 months ago

I think we should at least always support "standard" (as a way to unset). Additionally we could support the existing value as well so it is kept on update. If not, I think we should unset it to "standard" and not throw an error.

#3 @iseulde
11 months ago

Best to leave the existing value though, I don't like the idea of quietly changing the content.

#4 @danielbachhuber
11 months ago

We can ignore the update to the format field if the value is the same as what's currently saved. Does that make sense as a way to address the issue?

I'm not sure introducing conditional behavior in our validation framework is the best way to go about this.

I'm also not sure this is a bug. If a client doesn't want to update a field, it can and should omit the field from the request. If the client is trying to the field to a value that isn't supported, the API should rightfully error.

As a corollary example to consider, what should happen if you try to update template for a Page to a template value that doesn't exist in the theme?

#5 @iseulde
11 months ago

HTML forms don't let you omit the field without the help of JS.

Shouldn't you be able to POST the data you got back to the server without errors?

Isn't it now conditional if it's based on the theme?

#6 @danielbachhuber
11 months ago

HTML forms don't let you omit the field without the help of JS.

How does a HTML form handle a JSON response in the first place?

A HTML form POSTing to an API endpoint will always need JavaScript if it wants to avoid displaying JSON to the end user.

Shouldn't you be able to POST the data you got back to the server without errors?

Ideally yes, but I don't think the API can guarantee this.

Isn't it now conditional if it's based on the theme?

Sure, it's conditional in the sense that any PHP code can modify the enum. Validation doesn't change from request to request though, which is one of the principles of REST.

I'm open to always supporting 'standard' as a value though.

#7 @iseulde
11 months ago

Of course not straight to the endpoint. But then the server passing it along will need to handle this, which is okay I guess.

#9 @joehoyle
11 months ago

My 2c:

Shouldn't you be able to POST the data you got back to the server without errors?

Broadly speaking: yes. However, there are some edgecases when this isn't exactly possible., following to:

Isn't it now conditional if it's based on the theme?

I think the distinction here is site wide conditional (say for the theme) or conditional based off a specific post. Right now, it's implement as:

"Allow any of these values" (valid formats), however, if you have a non-valid format already on the resource, then you are actually saying "Allow any of these values, or the specific value that might already be stored for that specific post". Right now, most of the validation is not specific post aware.

We only recently changed the validation to only support the values allowed by the current theme (as that's broadly what the admin does), however I thinking maybe that was a mistake and we should always support _all_ the post formats in the API. Even if we fixed the validation issue, it's somewhat annoying that you can get a post in the API which has an post format that you, as an api client, never knew the existence of.

This ticket was mentioned in Slack in #core by helen. View the logs.


11 months ago

#11 @joehoyle
11 months ago

  • Owner set to joehoyle
  • Resolution set to fixed
  • Status changed from new to closed

In 39353:

REST API: Special case the “standard” post format to always be allowed.

Fixes #38916.

#12 follow-up: @Drivingralle
11 months ago

I think the behavior to just allow the post formats of the current theme is bad for a long term use of user data.

The current theme maybe doesn't support the format "aside" but the theme used two month later does.
If a user publishes the same content using a client via REST API suddenly gets different results. Because the newer posts suddenly have the format "aside" and the older once are "standard".

If we always allow to set the default post formats a client can write the full set of information into the post. If a theme leverages the data or not.
This way we make it pain-free to switch themes, something we should aim for.

We also allow to post comments into the API even if the theme doesn't show/support them.

#13 in reply to: ↑ 12 @Bueltge
10 months ago

+1

We also allow to post comments into the API even if the theme doesn't show/support them.

Other issue, but same context and really right.

#14 @iseulde
10 months ago

Since this ticket is closed and has commits, but doesn't really address the problem fully, I've created a new ticket about the same issue, even if it's just to figure out the definitive answer.

Note: See TracTickets for help on using tickets.