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

Add author support to templates and template parts #1937

Conversation

talldan
Copy link

@talldan talldan commented Nov 24, 2021

Related - WordPress/gutenberg#36763

Adds support for two nest REST API field for the templates controller to meet the requirements here - WordPress/gutenberg#36597 (comment)

It adds:

  • author. When a user creates a complete new template (or template part) the author is now stored. The author can then be shown in the list of templates.
  • origin. When a file template is customized and a post template created, this stores what the original 'source' of that template was (usually either 'theme' or 'plugin').

I'll leave a disclaimer that making these kind of changes is not my strength, so this could do with some critique.

To test the full feature here you need both WordPress and Gutenberg dev environments, with this checked out for the WordPress dev environment and WordPress/gutenberg#36763 checked for the gutenberg environment.

Testing steps

  1. Visit wp-admin/themes.php?page=gutenberg-edit-site&postType=wp_template_part
  2. Click Add new and name the template part
  3. Go back to wp-admin/themes.php?page=gutenberg-edit-site&postType=wp_template_part
  4. Your author should show up in the Added By column for the newly created template part

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

src/wp-includes/class-wp-block-template.php Outdated Show resolved Hide resolved
@@ -516,6 +516,7 @@ function _build_block_template_result_from_post( $post ) {
$template->status = $post->post_status;
$template->has_theme_file = $has_theme_file;
$template->is_custom = true;
$template->post_author = $post->post_author;
Copy link
Member

Choose a reason for hiding this comment

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

Should we set $template->post_author to something in _build_block_template_result_from_file? as well Or is 0 correct in that case?

Copy link
Author

Choose a reason for hiding this comment

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

It's a good question. File templates won't have an author, but I wasn't sure what a good default value was here. I leaned on convention by using the same value posts do.

But maybe it'd be better to return no author property in the response for file templates, which could also be reflected in the WP_Block_Template class by not giving it a default of 0.

Copy link
Member

Choose a reason for hiding this comment

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

I think 0 as no author makes sense for file templates.

src/wp-includes/class-wp-block-template.php Show resolved Hide resolved
if ( ! empty( $request['author'] ) ) {
$post_author = (int) $request['author'];

if ( get_current_user_id() !== $post_author ) {
Copy link
Member

Choose a reason for hiding this comment

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

What's this check for? Because wp_insert_post() will set post_author to get_current_user_id() if we don't specify?

Copy link
Author

@talldan talldan Nov 25, 2021

Choose a reason for hiding this comment

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

I lifted it directly from the rest posts controller. 😄

My understanding is that it's a quick way to check if the provided author value is valid. If the id matches the current user it bypasses the more expensive check a few lines below. Seems to be based on the assumption that the current user is valid, which I think is fair enough.

Copy link
Member

Choose a reason for hiding this comment

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

This is fine as it is a copy and paste from the post controller.

@noisysocks
Copy link
Member

noisysocks commented Nov 25, 2021

We'll need to make these changes to the REST API endpoint in Gutenberg as well so that everything works when running Gutenberg on a site with WordPress 5.7 or 5.8. It might be easier to land it that way, too. (This PR can serve as a reference for me when I backport WordPress/gutenberg#36763.)

edit: Never mind, I see you already did that in WordPress/gutenberg#36763 😀

@talldan talldan force-pushed the add/author-support-to-templates-and-template-parts branch from 2bb7197 to 8888852 Compare November 25, 2021 08:53
@@ -69,6 +69,7 @@ class WP_Block_Template {
*/
public $description = '';


Copy link
Member

Choose a reason for hiding this comment

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

Remove this line.

@noisysocks
Copy link
Member

This was merged into #1967 and committed.

@noisysocks noisysocks closed this Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants