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.

Sign up for GitHub

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 meta and query parameter to fetch patterns matching a list of allowed blocks #540

Merged
merged 5 commits into from
Jan 2, 2023
Merged

Add meta and query parameter to fetch patterns matching a list of allowed blocks #540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7a40a78
Patterns: Add new meta to track which block types are used in a pattern
ryelle Nov 10, 2022
ec6462e
API: Allow filtering the pattern directory by which block types are used
ryelle Nov 16, 2022
624df8a
Directory Bin: Update all patterns with the "contains block types" me…
ryelle Nov 16, 2022
d57ee1b
Add all meta to translated patterns
ryelle Nov 16, 2022
a2c14a9
Patterns: Add the block types used as meta when a post is updated
ryelle Nov 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions public_html/wp-content/plugins/pattern-directory/bin/update-contains-block-types.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
<?php
/**
* Update all patterns with the "contains block types" meta field.
*
* To run locally, use `wp-env`, ex:
* yarn wp-env run cli "php wp-content/plugins/pattern-directory/bin/update-contains-block-types.php --all --per_page=100 --apply"
*
* To run in a sandbox, use php directly, ex:
* php ./bin/update-contains-block-types.php --all --apply
*/

namespace WordPressdotorg\Pattern_Directory;

use const WordPressdotorg\Pattern_Directory\Pattern_Post_Type\{ POST_TYPE };

// This script should only be called in a CLI environment.
if ( 'cli' != php_sapi_name() ) {
die();
}

$opts = getopt( '', array( 'post:', 'url:', 'abspath:', 'per_page:', 'all', 'apply', 'verbose' ) );

if ( empty( $opts['url'] ) ) {
$opts['url'] = 'https://wordpress.org/patterns/';
}

if ( empty( $opts['abspath'] ) && false !== strpos( __DIR__, 'wp-content' ) ) {
$opts['abspath'] = substr( __DIR__, 0, strpos( __DIR__, 'wp-content' ) );
}

$opts['apply'] = isset( $opts['apply'] );
$opts['verbose'] = isset( $opts['verbose'] );
$opts['all'] = isset( $opts['all'] );

// Bootstrap WordPress
$_SERVER['HTTP_HOST'] = parse_url( $opts['url'], PHP_URL_HOST );
$_SERVER['REQUEST_URI'] = parse_url( $opts['url'], PHP_URL_PATH );

require rtrim( $opts['abspath'], '/' ) . '/wp-load.php';

if ( ! $opts['all'] && ! isset( $opts['post'] ) ) {
fwrite( STDERR, "Error! Either specify a post ID with --post=<ID> or explicitly run over --all.\n" );
die();
}

if ( ! $opts['apply'] ) {
echo "Dry run, will not update any patterns.\n";
}

$args = array(
'post_type' => POST_TYPE,
'post_status' => array( 'publish', 'pending' ),
'posts_per_page' => isset( $opts['per_page'] ) ? $opts['per_page'] : -1,
'post_parent' => 0,
'orderby' => 'date',
'order' => 'DESC',
'meta_query' => array(
// Only update patterns without this meta.
array(
'key' => 'wpop_contains_block_types',
'compare' => 'NOT EXISTS',
),
),
);
if ( isset( $opts['post'] ) ) {
$args = array(
'post_type' => POST_TYPE,
'p' => absint( $opts['post'] ),
);
}

$query = new \WP_Query( $args );
$meta_updated = 0;

while ( $query->have_posts() ) {
$query->the_post();
$pattern = get_post();
$pattern_id = $pattern->ID;
$blocks = parse_blocks( $pattern->post_content );
$all_blocks = _flatten_blocks( $blocks );

// Get the list of block names and convert it to a single string.
$block_names = wp_list_pluck( $all_blocks, 'blockName' );
Copy link

Choose a reason for hiding this comment

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

I think it's better to remove the core/ prefix from the blocks and the reason is that we can end up with a quite big GET request that could exceed the limit. If every core block is allowed it seems we have 60 of them.

If in the future we add support in pattern directory for third party blocks, we can revisit and I don't think a solution would be that hard.

For now in my GB PR, I'm stripping the core/ prefix: WordPress/gutenberg@0b3d6e0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If every core block is allowed it seems we have 60 of them.

If every block is allowed, you could just not send the parameter, and we won't need to do any filtering.

If in the future we add support in pattern directory for third party blocks

Yeah, this is trying to be future-friendly, so the full block name is necessary in the meta (otherwise we'll need another script to update it later…).

we can end up with a quite big GET request that could exceed the limit.

I think we're safe for now with just core blocks - are you running into the limit in your PR?

Copy link

@ntsekouras ntsekouras Nov 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

If every block is allowed, you could just not send the parameter, and we won't need to do any filtering.

That would be more convoluted as we would need to have operations in sync with the Blocks API(like parent, ancestor, etc..), so for now let's not do that. Even with having the above though, he problem remains if for example we have just one disallowed block.

I think we're safe for now with just core blocks - are you running into the limit in your PR?

With the prefix the length of allowed_blocks is 962 characters. It's quite long but I think we will be in the limit for now at least. Let's try that.

Yeah, this is trying to be future-friendly, so the full block name is necessary in the meta (otherwise we'll need another script to update it later…).

We could just modify the script to add the prefix in non-core blocks, but as I said above we can try with the prefix for now and let's see if we need something different in the future.

I have updated the GB PR to send the prefix.

Copy link

Choose a reason for hiding this comment

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

That would be more convoluted as we would need to have operations in sync with the Blocks API(like parent, ancestor, etc..), so for now let's not do that.

Hm... I just realized that there are some nuances in this. What I'm sending you right now doesn't include the blocks for example that have a specific parent(ex List item and List), but your parsing of the content will probably have the List item as well, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

your parsing of the content will probably have the List item as well, correct?

Yes, it collapses down all the blocks used in a pattern, for example: core/column,core/columns,core/group,core/heading,core/list,core/list-item,core/paragraph,core/separator,core/social-link,core/social-links

This comment was marked as outdated.

Sign in to view

Copy link

Choose a reason for hiding this comment

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

I have added the logic to include the parent/ancestor relationship and if all available core blocks are allowed to be inserted, I'm sending the all value as a string.

Copy link
Member

Choose a reason for hiding this comment

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

If every core block is allowed it seems we have 60 of them.

This is going to affect the cacheability of the API significantly if every API request sends a list of their installed blocks that we then filter by. Due to the Pattern APIs using WordPress to serve the API requests with nginx caching in-front, this could be a longer-term performance issue.

We should require/suggest strongly, at a minimum, that the allowed_blocks parameter should be sorted consistently to avoid two sites with different block orders getting different cache keys.

I can't see that clients could reasonably send a "I don't support this block" flag though, as it doesn't know what it doesn't know about.

$block_names = array_filter( $block_names );
$block_names = array_unique( $block_names );
sort( $block_names );
Copy link

Choose a reason for hiding this comment

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

Do you think it will help performance sorting the block names?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly this is mostly for devex, so it's easier to check the value against the expected value.

$used_blocks = implode( ',', $block_names );

if ( $opts['apply'] ) {
$result = update_post_meta( $pattern_id, 'wpop_contains_block_types', $used_blocks );
if ( $result ) {
$meta_updated++;
} else if ( $opts['verbose'] ) {
echo "Error updating {$pattern_id}.\n"; // phpcs:ignore
}
} else if ( $opts['verbose'] ) {
echo "Will update {$pattern_id} with '{$used_blocks}'.\n"; // phpcs:ignore
}
}

echo "Updated {$meta_updated} patterns.\n"; // phpcs:ignore
echo "Done.\n\n"; // phpcs:ignore
57 changes: 57 additions & 0 deletions public_html/wp-content/plugins/pattern-directory/includes/pattern-post-type.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
add_action( 'rest_api_init', __NAMESPACE__ . '\register_rest_fields' );
add_action( 'init', __NAMESPACE__ . '\register_post_statuses' );
add_action( 'transition_post_status', __NAMESPACE__ . '\status_transitions', 10, 3 );
add_action( 'post_updated', __NAMESPACE__ . '\update_contains_block_types_meta' );
add_action( 'enqueue_block_editor_assets', __NAMESPACE__ . '\enqueue_editor_assets' );
add_filter( 'allowed_block_types_all', __NAMESPACE__ . '\remove_disallowed_blocks', 10, 2 );
add_action( 'enqueue_block_editor_assets', __NAMESPACE__ . '\disable_block_directory', 0 );
Expand Down Expand Up @@ -249,6 +250,24 @@ function register_post_type_data() {
),
)
);

register_post_meta(
POST_TYPE,
'wpop_contains_block_types',
array(
'type' => 'string',
'description' => 'A list of block types used in this pattern',
'single' => true,
'default' => '',
'sanitize_callback' => 'sanitize_text_field',
'auth_callback' => __NAMESPACE__ . '\can_edit_this_pattern',
'show_in_rest' => array(
'schema' => array(
'type' => 'string',
),
),
)
);
}

/**
Expand Down Expand Up @@ -490,6 +509,26 @@ function status_transitions( $new_status, $old_status, $post ) {
}
}

/**
* Given a post ID, parse out the block types and update the `wpop_contains_block_types` meta field.
*
* @param int $pattern_id Pattern ID.
*/
function update_contains_block_types_meta( $pattern_id ) {
$pattern = get_post( $pattern_id );
$blocks = parse_blocks( $pattern->post_content );
$all_blocks = _flatten_blocks( $blocks );

// Get the list of block names and convert it to a single string.
$block_names = wp_list_pluck( $all_blocks, 'blockName' );
$block_names = array_filter( $block_names ); // Filter out null values (extra line breaks).
$block_names = array_unique( $block_names );
sort( $block_names );
$used_blocks = implode( ',', $block_names );

update_post_meta( $pattern_id, 'wpop_contains_block_types', $used_blocks );
}

/**
* Determines if the current user can edit the given pattern post.
*
Expand Down Expand Up @@ -635,6 +674,14 @@ function filter_patterns_collection_params( $query_params ) {
'type' => 'string',
);

$query_params['allowed_blocks'] = array(
'description' => __( 'Filter the request to only return patterns with blocks on this list.', 'wporg-patterns' ),
'type' => 'array',
'items' => array(
'type' => 'string',
),
);

return $query_params;
}

Expand Down Expand Up @@ -700,6 +747,16 @@ function filter_patterns_rest_query( $args, $request ) {
);
}

$allowed_blocks = $request->get_param( 'allowed_blocks' );
if ( $allowed_blocks ) {
// Only return a pattern if all contained blocks are in the allowed blocks list.
$args['meta_query']['allowed_blocks'] = array(
'key' => 'wpop_contains_block_types',
'compare' => 'REGEXP',
'value' => '^((' . implode( '|', $allowed_blocks ) . '),?)+$',
Copy link
Member

Choose a reason for hiding this comment

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

This isn't going to be amazing performance wise, but I'm not sure I can see a better way to achieve this, other than filtering post-query.

);
}

return $args;
}

Expand Down
12 changes: 8 additions & 4 deletions public_html/wp-content/plugins/pattern-translations/pattern-translations.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,14 @@ function create_or_update_translated_pattern( Pattern $pattern ) {
'post_author' => $parent->post_author ?? 0,
'post_status' => $parent->post_status ?? 'pending',
'meta_input' => [
'wpop_description' => $pattern->description,
'wpop_locale' => $pattern->locale,
'wpop_viewport_width' => $parent->wpop_viewport_width,
'wpop_is_translation' => true,
'wpop_description' => $pattern->description,
'wpop_locale' => $pattern->locale,
'wpop_keywords' => $pattern->keywords,
'wpop_viewport_width' => $parent->wpop_viewport_width,
'wpop_block_types' => $parent->wpop_block_types,
'wpop_contains_block_types' => $parent->wpop_contains_block_types,
'wpop_wp_version' => $parent->wpop_wp_version,
'wpop_is_translation' => true,
],
];

Expand Down