#39996 closed defect (bug) (fixed)
Allow passing existing template value for posts, even when template does not exist
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.9 | Priority: | normal |
Severity: | normal | Version: | |
Component: | REST API | Keywords: | has-unit-tests has-patch commit |
Focuses: | Cc: |
Description
In r40120, it became possible for a client to set format=>aside
on a post, even when when the theme doesn't support aside
as a post format.
Under this same logic, it should be possible for a client to set template=>my-file.php
, even when my-file.php
isn't a file in the theme.
The current behavior is that it's not possible for a client to set template
to an arbitrary file name (values are validated against an enum).
Related #39232
Attachments (2)
Change History (33)
#2
in reply to:
↑ 1
@
8 years ago
Replying to jnylen0:
If we apply the same logic for templates, we are essentially saying that clients can set any arbitrary data (or just an arbitrary PHP filename) in this field, which doesn't seem right. Instead, I think the change we should make here is to enable setting a post to the same
template
value that it currently has - this should be a no-op even if the current theme doesn't support that value.
I agree with this - There doesn't appear to be any value in allowing it to be set to a value which isn't supported by the theme - as it's theme-specific data (unlike post formats), however we shouldn't force a client to wipe out the existing value in order to resave a post.
@danielbachhuber If you've got a use-case, I'd love to hear it
#3
@
8 years ago
Instead, I think the change we should make here is to enable setting a post to the same
template
value that it currently has - this should be a no-op even if the current theme doesn't support that value.
This seems like a reasonable decision to me.
@dd32 I don't have a specific use case, other than the original presented: a client POSTing back the same data it received.
#4
follow-up:
↓ 5
@
8 years ago
It's a bit of a mess it seems.
WP_REST_Posts_Controller::update_item()
calls wp_update_post()
, which doesn't allow you to update the post's template to the current value. If you'd change that, the meta entry would get updated twice because the update_item()
method calls WP_REST_Posts_Controller::handle_template()
.
Ideally, we fix wp_update_post()
, remove the call to handle_template()
and add a new test for this behaviour.
#5
in reply to:
↑ 4
;
follow-up:
↓ 6
@
8 years ago
Replying to swissspidy:
Ideally, we fix
wp_update_post()
, remove the call tohandle_template()
and add a new test for this behaviour.
Couldn't we just unset $data['template']
from the update if it's the same as the current value? This would skip both passing it to wp_update_post
and calling handle_template
.
the meta entry would get updated twice because the
update_item()
method callsWP_REST_Posts_Controller::handle_template()
.
It sounds like this is probably already happening during any normal post update, no?
#6
in reply to:
↑ 5
@
8 years ago
Replying to jnylen0:
It sounds like this is probably already happening during any normal post update, no?
Upon a closer look, we never pass page_template
to wp_update_post
, so its logic for updating a page template is never triggered by the API.
So this looks like a pretty straightforward change to me, only touching WP_REST_Posts_Controller::handle_template
. @swissspidy do you agree or is there something I'm missing here?
#7
follow-up:
↓ 9
@
8 years ago
So this looks like a pretty straightforward change to me, only touching
WP_REST_Posts_Controller::handle_template
. @swissspidy do you agree or is there something I'm missing here?
IIRC this fails much earlier in the schema validation because of the enum. It never gets to this method.
This ticket was mentioned in Slack in #core by swissspidy. View the logs.
8 years ago
#9
in reply to:
↑ 7
@
8 years ago
Replying to swissspidy:
IIRC this fails much earlier in the schema validation because of the enum. It never gets to this method.
We can solve this with a similar approach to e.g. check_username
in 38739.diff. I'll get a patch posted here soon.
#11
@
8 years ago
- Summary changed from Grant clients more liberty in setting 'template' attribute to Allow passing existing template value for posts, even when template does not exist
Renaming the ticket to better match the decision here.
This ticket was mentioned in Slack in #core by desrosj. View the logs.
8 years ago
#15
@
8 years ago
- Milestone changed from 4.8 to 4.8.1
I'm going to punt this again as it's not super high priority and there is no patch yet.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
This ticket was mentioned in Slack in #core-restapi by kadamwhite. View the logs.
7 years ago
#19
@
7 years ago
- Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed
Attached a patch to use validate_callback
as suggested.
I also had to account for the built in validation that wp_insert_post()
performs for the page template. When updating the post, the original template value gets merged into the update data. This means that even if the page template was validated accounting for updating to the same value, internal validation would prevent it from being saved.
To account for this, I set page_template
to null so that template handling will be bypassed in wp_insert_post
.
Since the template is now validated at request time, I removed the extra validation in ::handle_template()
by default. However, I kept it when performing a create request because the filter on the templates can be contextual based on the new post object and we don't have the new post ID when validating at request time. It might be worth returning an error in this case as well, similar to updating terms, or meta, etc...
This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.
7 years ago
This ticket was mentioned in Slack in #core-restapi by kadamwhite. View the logs.
7 years ago
This ticket was mentioned in Slack in #core-restapi by kadamwhite. View the logs.
7 years ago
This ticket was mentioned in Slack in #core-restapi by kadamwhite. View the logs.
7 years ago
#24
@
7 years ago
- Keywords needs-patch added; has-patch removed
@TimothyBlynJacobs I think the enum
should be removed entirely in favor of completely using the validate_callback
as the available templates are dynamic.
It turns out that the list of available page templates for a given page is not necessarily a subset of all of the page templates in a theme. A plugin can dynamically make extra page templates available for certain pages, being sourced from plugins. As an example, see from Elementor. For more, see discussion on Gutenberg.
#27
@
7 years ago
+1 to the overall approach in 39996.2.diff — a dynamic callback works fine for this need.
This is pretty different than post formats - the current theme may not support all formats, but there is still a finite list of possibly supported formats.
If we apply the same logic for templates, we are essentially saying that clients can set any arbitrary data (or just an arbitrary PHP filename) in this field, which doesn't seem right. Instead, I think the change we should make here is to enable setting a post to the same
template
value that it currently has - this should be a no-op even if the current theme doesn't support that value.