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 pre- and post- content filter #60

Merged
merged 9 commits into from
Apr 9, 2024
Merged

Conversation

alecgeatches
Copy link
Contributor

Description

Add a new filter for modifying post content before it is parsed, and another for after content is parsed:

vip_block_data_api__before_parse_post_content

Modify raw post content before it's parsed by the Block Data API.

/**
 * Filters content before parsing blocks in a post.
 *
 * @param string $post_content The content of the post being parsed.
 * @param int $post_id Post ID associated with the content.
 */
$post_content = apply_filters( 'vip_block_data_api__before_parse_post_content', $post_content, $post_id );

vip_block_data_api__after_parse_blocks

Modify the Block Data API REST endpoint response.

/**
 * Filters the API result before returning parsed blocks in a post.
 *
 * @param string $result The successful API result, contains 'blocks' key with an array
 *                       of block data, and optionally 'warnings' and 'debug' keys.
 * @param int $post_id Post ID associated with the content.
 */
$result = apply_filters( 'vip_block_data_api__after_parse_blocks', $result, $post_id );

Steps to Test

Tests have been added both of the new filters in tests/parser/test-parser-filters.php. To test:

$ wp-env start
$ composer install
$ composer run test

> wp-env run tests-cli --env-cwd=wp-content/plugins/vip-block-data-api ./vendor/bin/phpunit
ℹ Starting './vendor/bin/phpunit' on the tests-cli container. 

....................................................              52 / 52 (100%)

Time: 00:00.270, Memory: 56.50 MB

OK (52 tests, 122 assertions)
✔ Ran `./vendor/bin/phpunit` in 'tests-cli'. (in 3s 227ms)

@alecgeatches alecgeatches requested a review from a team as a code owner April 8, 2024 17:13
@alecgeatches
Copy link
Contributor Author

@smithjw1 Could I please have a review from you on this, especially on the README documentation for the new filters? Thank you!

@alecgeatches alecgeatches self-assigned this Apr 8, 2024
README.md Outdated

### `vip_block_data_api__before_parse_post_content`

Modify raw post content before it's parsed by the Block Data API.
Copy link
Contributor

Choose a reason for hiding this comment

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

Like you do with the other filter, I might mention that this is the very first thing called so that you are getting exactly what is in the WP database.

I also like the example in the other one, so we could add one here. The example helps people understand why they might use it.

We should also add a warning that manipulating the content directly can cause block parsing errors, and they should be careful with this filter. Here's the language I would use:

Block Markup is sensitive to changes, even changes in whitespace. We've added this filter to make the plugin flexible, but any changes to the post_content should be done with extreme care. Strongly consider adding tests to any filter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great notes, especially the warning. Will add!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smithjw1 Added an before_parse example along with a warning as you described above. Thank you!

Copy link
Contributor

@smithjw1 smithjw1 left a comment

Choose a reason for hiding this comment

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

I've made a few comments on the Readme. I think we should have someone else review the code, it looks good to me, but I didn't dig too much into the tests.

smithjw1
smithjw1 previously approved these changes Apr 8, 2024
Copy link
Contributor

@smithjw1 smithjw1 left a comment

Choose a reason for hiding this comment

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

Wonderfully done @alecgeatches

@alecgeatches
Copy link
Contributor Author

I'm going to merge this as the code changes are low-risk filter additions and also include tests.

@alecgeatches alecgeatches merged commit 1c5ef3b into trunk Apr 9, 2024
2 checks passed
@alecgeatches alecgeatches deleted the add/pre-content-filter branch April 9, 2024 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants