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

Site Editor: old template part drafts appear in the sidebar #26402

Closed
vindl opened this issue Oct 22, 2020 · 10 comments
Closed

Site Editor: old template part drafts appear in the sidebar #26402

vindl opened this issue Oct 22, 2020 · 10 comments
Assignees
Labels
Needs Technical Feedback Needs testing from a developer perspective. [Type] Bug An existing feature does not function as intended

Comments

@vindl
Copy link
Member

vindl commented Oct 22, 2020

If you switch a couple of themes that provide block template parts you can get into a state where all of the previous ones are listed:

screen_shot_2020-10-22_at_11 55 13_am

I believe this is happening because the auto-drafts are being created for them, but there is no way to view or delete those in current wp_template_part listing, since the draft category is not shown.

I think that it would be better to avoid displaying these theme template part drafts that were not previously modified.

@vindl vindl added [Type] Bug An existing feature does not function as intended [Feature] Full Site Editing labels Oct 22, 2020
@mtias
Copy link
Member

mtias commented Oct 23, 2020

Why are we creating them in the first place?

@vindl
Copy link
Member Author

vindl commented Oct 26, 2020

Why are we creating them in the first place?

I'm not completely sure since they predate our work on FSE. I'm assuming it's partly to avoid loading from theme files directly, and to give special status to denote that it hasn't been changed compared to the base version, but there might have been other reasons at the time.

@mtias mtias added the Needs Technical Feedback Needs testing from a developer perspective. label Oct 26, 2020
@Copons
Copy link
Contributor

Copons commented Oct 28, 2020

I think that it would be better to avoid displaying these theme template part drafts that were not previously modified.

In other words: all of them. 😄

Using auto-draft as default status of templates and parts converted from files introduces a bunch of complications caused by the fact that auto-draft is a very special status that, supposedly, should be only handled automatically by the editor.

In #26394 I'm proposing a custom status for wp_template that are converted from files and not modified yet, which ideally would be propagated to wp_template_part as well.


About the issue at hand.

The conversion from file happens when several conditions are met:

  • First it searches for a corresponding wp_template_part (it has the requested slug, the current theme in meta, and is status auto-draft).
  • If there is a corresponding wp_template_part, it compare its content with the file content, and if there are changes it updates the former.
  • If there is no corresponding wp_template_part, the conversion creates one (this typically happens when activating a new theme). Since in the Site Editor sidebar we only see the slug, we don't have a way to tell "duplicates" apart.

Some ways to alleviate this confusion could be:

@gziolo
Copy link
Member

gziolo commented Oct 29, 2020

@Copons, could you point me to any document or code that would help me understand the flow for managing wp_template_parts when activating themes?

@Copons
Copy link
Contributor

Copons commented Oct 29, 2020

@gziolo AFAIK there is no flow specifically for theme activation.
Templates and template parts provided as files by the theme are converted into wp_template and wp_template_part posts (if needed) during REST requests on those post types, or when filtering the block_editor_settings.
In other words, the most reliable way to "trigger" the conversion is to open the Site Editor.

Here you can check the rest_wp_template_part_query filter:

/**
* Filter for supporting the `resolved`, `template`, and `theme` parameters in `wp_template_part` queries.
*
* @param array $args The query arguments.
* @param WP_REST_Request $request The request object.
* @return array Filtered $args.
*/
function filter_rest_wp_template_part_query( $args, $request ) {
/**
* Unlike `filter_rest_wp_template_query`, we resolve queries also if there's only a `template` argument set.
* The difference is that in the case of templates, we can use the `slug` field that already exists (as part
* of the entities endpoint, wheras for template parts, we have to register the extra `template` argument),
* so we need the `resolved` flag to convey the different semantics (only return 'resolved' templates that match
* the `slug` vs return _all_ templates that match it (e.g. including all auto-drafts)).
*
* A template parts query with a `template` arg but not a `resolved` one is conceivable, but probably wouldn't be
* very useful: It'd be all template parts for all templates matching that `template` slug (including auto-drafts etc).
*
* @see filter_rest_wp_template_query
* @see filter_rest_wp_template_part_collection_params
* @see https://github.com/WordPress/gutenberg/pull/21878#discussion_r436961706
*/
if ( $request['resolved'] || $request['template'] ) {
$template_part_ids = array( 0 ); // Return nothing by default (the 0 is needed for `post__in`).
$template_types = $request['template'] ? array( $request['template'] ) : get_template_types();
foreach ( $template_types as $template_type ) {
// Skip 'embed' for now because it is not a regular template type.
if ( in_array( $template_type, array( 'embed' ), true ) ) {
continue;
}
$current_template = gutenberg_find_template_post_and_parts( $template_type );
if ( isset( $current_template ) ) {
$template_part_ids = $template_part_ids + $current_template['template_part_ids'];
}
}
$args['post__in'] = $template_part_ids;
$args['post_status'] = array( 'publish', 'auto-draft' );
}
if ( $request['theme'] ) {
$meta_query = isset( $args['meta_query'] ) ? $args['meta_query'] : array();
$meta_query[] = array(
'key' => 'theme',
'value' => $request['theme'],
);
// Ensure auto-drafts of all theme supplied template parts are created.
if ( wp_get_theme()->stylesheet === $request['theme'] ) {
/**
* Finds all nested template part file paths in a theme's directory.
*
* @param string $base_directory The theme's file path.
* @return array $path_list A list of paths to all template part files.
*/
function get_template_part_paths( $base_directory ) {
$path_list = array();
if ( file_exists( $base_directory . '/block-template-parts' ) ) {
$nested_files = new RecursiveIteratorIterator( new RecursiveDirectoryIterator( $base_directory . '/block-template-parts' ) );
$nested_html_files = new RegexIterator( $nested_files, '/^.+\.html$/i', RecursiveRegexIterator::GET_MATCH );
foreach ( $nested_html_files as $path => $file ) {
$path_list[] = $path;
}
}
return $path_list;
}
// Get file paths for all theme supplied template parts.
$template_part_files = get_template_part_paths( get_stylesheet_directory() );
if ( is_child_theme() ) {
$template_part_files = array_merge( $template_part_files, get_template_part_paths( get_template_directory() ) );
}
// Build and save each template part.
foreach ( $template_part_files as $template_part_file ) {
$content = file_get_contents( $template_part_file );
// Infer slug from filepath.
$slug = substr(
$template_part_file,
// Starting position of slug.
strpos( $template_part_file, 'block-template-parts/' ) + 21,
// Subtract ending '.html'.
-5
);
// Wrap content with the template part block, parse, and create auto-draft.
$template_part_string = '<!-- wp:template-part {"slug":"' . $slug . '","theme":"' . wp_get_theme()->get( 'TextDomain' ) . '"} -->' . $content . '<!-- /wp:template-part -->';
$template_part_block = parse_blocks( $template_part_string )[0];
create_auto_draft_for_template_part_block( $template_part_block );
}
};
$args['meta_query'] = $meta_query;
}
return $args;
}
add_filter( 'rest_wp_template_part_query', 'filter_rest_wp_template_part_query', 99, 2 );

Here the block_editor_settings filter:

/**
* Extends default editor settings to enable template and template part editing.
*
* @param array $settings Default editor settings.
*
* @return array Filtered editor settings.
*/
function gutenberg_template_loader_filter_block_editor_settings( $settings ) {
global $post;
if ( ! $post ) {
return $settings;
}
// If this is the Site Editor, auto-drafts for template parts have already been generated
// through `filter_rest_wp_template_part_query`, when called via the REST API.
if ( isset( $settings['editSiteInitialState'] ) ) {
return $settings;
}
// Otherwise, create template part auto-drafts for the edited post.
$post = get_post();
foreach ( parse_blocks( $post->post_content ) as $block ) {
create_auto_draft_for_template_part_block( $block );
}
// TODO: Set editing mode and current template ID for editing modes support.
return $settings;
}
add_filter( 'block_editor_settings', 'gutenberg_template_loader_filter_block_editor_settings' );

And here is the function that performs the conversion:

/**
* Recursively traverses a block tree, creating auto drafts
* for any encountered template parts without a fixed post.
*
* @access private
*
* @param array $block The root block to start traversing from.
* @return int[] A list of template parts IDs for the given block.
*/
function create_auto_draft_for_template_part_block( $block ) {
$template_part_ids = array();
if ( 'core/template-part' === $block['blockName'] && isset( $block['attrs']['slug'] ) ) {
if ( isset( $block['attrs']['postId'] ) ) {
// Template part is customized.
$template_part_id = $block['attrs']['postId'];
} else {
// A published post might already exist if this template part
// was customized elsewhere or if it's part of a customized
// template. We also check if an auto-draft was already created
// because preloading can make this run twice, so, different code
// paths can end up with different posts for the same template part.
// E.g. The server could send back post ID 1 to the client, preload,
// and create another auto-draft. So, if the client tries to resolve the
// post ID from the slug and theme, it won't match with what the server sent.
$template_part_query = new WP_Query(
array(
'post_type' => 'wp_template_part',
'post_status' => array( 'publish', 'auto-draft' ),
'title' => $block['attrs']['slug'],
'meta_key' => 'theme',
'meta_value' => $block['attrs']['theme'],
'posts_per_page' => 1,
'no_found_rows' => true,
)
);
$template_part_post = $template_part_query->have_posts() ? $template_part_query->next_post() : null;
if ( $template_part_post && 'auto-draft' !== $template_part_post->post_status ) {
$template_part_id = $template_part_post->ID;
} else {
// Template part is not customized, get it from a file and make an auto-draft for it, unless one already exists
// and the underlying file hasn't changed.
$template_part_file_path = get_stylesheet_directory() . '/block-template-parts/' . $block['attrs']['slug'] . '.html';
if ( ! file_exists( $template_part_file_path ) ) {
$template_part_file_path = false;
}
if ( $template_part_file_path ) {
$file_contents = file_get_contents( $template_part_file_path );
if ( $template_part_post && $template_part_post->post_content === $file_contents ) {
$template_part_id = $template_part_post->ID;
} else {
$template_part_id = wp_insert_post(
array(
'post_content' => $file_contents,
'post_title' => $block['attrs']['slug'],
'post_status' => 'auto-draft',
'post_type' => 'wp_template_part',
'post_name' => $block['attrs']['slug'],
'meta_input' => array(
'theme' => $block['attrs']['theme'],
),
)
);
}
}
}
}
$template_part_ids[ $block['attrs']['slug'] ] = $template_part_id;
}
foreach ( $block['innerBlocks'] as $inner_block ) {
$template_part_ids = array_merge( $template_part_ids, create_auto_draft_for_template_part_block( $inner_block ) );
}
return $template_part_ids;
}

@gziolo
Copy link
Member

gziolo commented Oct 29, 2020

Thank you @Copons for all the pointers, much appreciated 💯

@gziolo
Copy link
Member

gziolo commented Nov 2, 2020

I'm able to reproduce the same issue. It looks like it displays template parts from all previously activated themes:

Screen Shot 2020-11-02 at 14 28 10

I activated 3 themes that support FSE and contain template parts.

For full context, I customized only one of them:

Screen Shot 2020-11-02 at 14 30 24

Aside: It's a bit unclear for me why template parts used by the current theme aren't listed in the Template Parts section as they technically exist for the current theme, even if it's only a virtual item sourced from the template provided by the theme.

@fklein-lu
Copy link
Contributor

Aside: It's a bit unclear for me why template parts used by the current theme aren't listed in the Template Parts section as they technically exist for the current theme, even if it's only a virtual item sourced from the template provided by the theme.

This is likely because unpublished template parts have the auto-draft status. This post status is excluded from the Post List view.

Saved but unpublished template parts should be drafts instead.

@Copons
Copy link
Contributor

Copons commented Nov 4, 2020

Yes, can confirm it's because auto-draft items are automatically excluded from most (all?) queries.

In #26636 I've modified the wp_template (not template parts, but the logic is similar) admin list query to also include auto-draft items.
Major reason: it was becoming extremely hard — at least for me — to work with templates without having a unambiguous source of truth, and a simple way to clear them when doing new activations tests.

I don't have a strong opinion about keeping auto-draft in the admin list when the Site Editor is stable, or hide them again to be consistent with all the other lists.
Or if we even intend to keep the hide the templates/parts lists as well, and only rely on the Site Editor.
Either way, those are long term discussions that aren't super relevant at the moment.

@vindl
Copy link
Member Author

vindl commented Nov 20, 2020

This was fixed in #26948.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Technical Feedback Needs testing from a developer perspective. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

6 participants