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

[iseulde]

Follow up ticket on #38916.

The schema for the post format depends on the theme. Should it be?

Some highlights:

Replying to danielbachhuber:

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?

Replying to joehoyle:

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.

Replying to Drivingralle:

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.

[mark-k]

+1. the behavior of post formats was always broken for anyone that thought about using them. Replicating the broken behavior might be consistent but isn't it better to fix the core issue? If I have a post with one image the post format I should set is an image whether the theme I currently use does something special for it or not.

In the context of the rest api - I can use the internal API to change the post format if the theme supports it or not, why shouldn't I be able to do it from the rest api?

[jnylen0]

Replying to @iseulde from ticket:38916#comment:5

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

I think this is an excellent design principle for an API. Cases where this is not possible often point to problems in the API or elsewhere.

Proposed solution for this ticket:

• Always allow the standard post formats to be set; show them as part of the format enum in the arguments and schema.
• Additionally allow the current post format to be set, even if the theme does not support it. This should be a no-op.

I'm fine with addressing this in the API interface first, then addressing the larger issue in core (as @Drivingralle explained) separately.

[iseulde]

Replying to joehoyle:

I think the distinction here is site wide conditional (say for the theme) or conditional based off a specific post.

As far as I understand, this is the main argument against the change, right? That it would be introducing conditional logic that is specific to the post? The API isn't consistent with this either though. From what I saw in the post controller, there is password logic that depends on the sticky value.

Replying to iseulde:

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

On second thought, this would have much bigger consequences. When you have an invalid thumbnail ID, what should happen? Or are we then differentiating between values that depend on the theme rather than the database?

[Drivingralle]

When you have an invalid thumbnail ID, what should happen? Or are we then differentiating between values that depend on the theme rather than the database?

An invalid thumbnail ID would lead to broken data and needs to be prevented. No theme or plugin can work with a wrong/broken post_thumbnail.

But a post_format not used by the current theme, but supported by core is not a wrong/broken piece of information. It's just not used at this point in time.

If a theme adds a custom post_format the API should validate the value correct. And after a theme switch the format is gone because it's part of the theme. The default formats are still inside WP.

[Drivingralle]

Is there anything I can do to bring this ticket forward?

[jnylen0]

We need consensus on an approach, and we need a patch with unit tests. I still think the approach I outlined in comment:2 is sensible.

[JPry]

Replying to jnylen0:

Proposed solution for this ticket:

• Always allow the standard post formats to be set; show them as part of the format enum in the arguments and schema.
• Additionally allow the current post format to be set, even if the theme does not support it. This should be a no-op.

jnylen0:

We need consensus on an approach, and we need a patch with unit tests. I still think the approach I outlined in comment:2 is sensible.

I think I'm +1 to your first bullet point. However, I don't agree with the second bullet point. Instead, I agree with this:

Drivingralle:

But a post_format not used by the current theme, but supported by core is not a wrong/broken piece of information. It's just not used at this point in time.

I think it makes sense to still set the post format, even if the theme doesn't support their use. If a theme isn't using post formats, or isn't using certain kinds of post formats, then it should just ignore them.

However, I disagree with this portion from Drivingralle:

If a theme adds a custom post_format the API should validate the value correct. And after a theme switch the format is gone because it's part of the theme. The default formats are still inside WP.

Themes aren't supposed to add their own custom post formats. By extension, I don't think that the API should support any "custom" post formats. Although, if others decided that it should, then we could put a filter in place where the post formats are added to the enum field.

[JPry]

Changes with theme support being ignored

[jnylen0]

Themes aren't supposed to add their own custom post formats.

I wasn't aware of this, but on its own isn't a good enough reason to remove support.

Do themes add post formats anyway? I'm not sure; we'll need someone who's more familiar with this feature to weigh in.

[Drivingralle]

Replying to jnylen0:

Do themes add post formats anyway? I'm not sure; we'll need someone who's more familiar with this feature to weigh in.

Themes can announce that they support a post format.
​https://codex.wordpress.org/Post_Formats#Adding_Theme_Support

The info like icon and labels for the format are included inside core.

[davidakennedy]

Replying to jnylen0:

Themes aren't supposed to add their own custom post formats.

I wasn't aware of this, but on its own isn't a good enough reason to remove support.

Do themes add post formats anyway? I'm not sure; we'll need someone who's more familiar with this feature to weigh in.

Responding to the post in Core Themes Slack.

Themes cannot add "custom" post formats. See: ​https://codex.wordpress.org/Post_Formats

New formats cannot be introduced by themes or even plugins.

As for point two in 2:

Additionally allow the current post format to be set, even if the theme does not support it. This should be a no-op.

I'd say I agree with this:

But a post_format not used by the current theme, but supported by core is not a wrong/broken piece of information. It's just not used at this point in time.

Because if you use post formats and switch themes, that data – what format it is still exists. A theme declaring it is just the theme's decision because it's usually connected to a front end display difference.

But I can imagine API clients wanting to set and show that data differently. To some degree, they could be "themes" in a loose sense, as in not a traditional WordPress theme, but still wanting to use that data in a creative way.

That's just my take. Let me know if I missed anything, or you need anything else.

[jnylen0]

Replying to davidakennedy:

Themes cannot add "custom" post formats. See: ​https://codex.wordpress.org/Post_Formats

New formats cannot be introduced by themes or even plugins.

This is the piece I was missing here, thank you. There is a big difference between "should not" and "cannot" :)

Given that, I agree with the approach in 39232.diff​. I don't think we should remove the tests though - let's make sure the requests succeed instead.

[Drivingralle]

Does it make sense to include this patch into 4.7.3?

This way currently beeing developed API-clients don't need to implement soon superfluous checks. Other API misbehavior are also changed in that minor release as far as I understood.

[jnylen0]

Thanks for the reminder, @Drivingralle. I had forgotten about this ticket. I'll try to land it for 4.7.3 today.

[jnylen0]

Modify the existing "unsupported post format" tests instead of removing them.

[jnylen0]

This is good to go with 39232.2.diff​.

[jnylen0]

In 40120:

REST API: Allow setting post formats even if they are not supported by the theme.

A post_format not used by the current theme, but supported by core is not a wrong/broken piece of information. It's just not used at this point in time. Therefore we should allow setting and retrieving any of the standard post formats supported in core, even if the current theme doesn't use them.

After this commit, a post's format value can survive a round trip through the API, which is a good general design principle for an API.

Props JPry, iseulde, davidakennedy, Drivingralle.
Fixes #39232.

[jnylen0]

Reopening for backport to 4.7 branch. Edit: Two commits to backport - [40120] and [40121].

[jnylen0]

In 40121:

REST API: Update the client test fixtures after changes to post formats.

This commit updates the wp-api-generated.js fixture file after recent changes to the way post formats work in the API.

See #39232.

[ocean90]

In 40137:

REST API: Allow setting post formats even if they are not supported by the theme.

A post_format not used by the current theme, but supported by core is not a wrong/broken piece of information. It's just not used at this point in time. Therefore we should allow setting and retrieving any of the standard post formats supported in core, even if the current theme doesn't use them.

After this commit, a post's format value can survive a round trip through the API, which is a good general design principle for an API.

Merge of [40120] and [40121] to the 4.7 branch.

Props JPry, iseulde, davidakennedy, Drivingralle.
Fixes #39232.