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

BUG: Blocks that use postId context crash indexing #3325

Closed
1 task done
tomjn opened this issue Feb 20, 2023 · 3 comments · Fixed by #3333
Closed
1 task done

BUG: Blocks that use postId context crash indexing #3325

tomjn opened this issue Feb 20, 2023 · 3 comments · Fixed by #3333
Labels
bug Something isn't working
Milestone

Comments

@tomjn
Copy link
Contributor

tomjn commented Feb 20, 2023

Describe the bug

  1. I have a dynamic block that requires the postId context to render post specific content.
  2. This context is only provided if the global $post object is set. This is fine because my block is only available in posts, and is inside a post.
  3. ElasticPress does not set the current post so $post is undefined
  4. ElasticPress uses the_content filter to retrieve my posts content
  5. WP tries to render the block and the callback tries to use postId expecting an integer
  6. the block renderer recieves a null and the application crashes

Steps to Reproduce

Attempt to index a site that uses the postId in a block renderer

Screenshots, screen recording, code snippet

No response

Environment information

No response

WordPress and ElasticPress information

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@tomjn tomjn added the bug Something isn't working label Feb 20, 2023
@tomjn
Copy link
Contributor Author

tomjn commented Feb 20, 2023

Noting that adding global $post to the top of the post document preparer allowed indexing to complete:

	public function prepare_document( $post_id ) {
		global $post;

@felipeelia
Copy link
Member

Good catch @tomjn. Do you mind opening a Pull Request with that change? Thanks

@felipeelia felipeelia added this to the 4.5.0 milestone Feb 21, 2023
@tomjn
Copy link
Contributor Author

tomjn commented Feb 21, 2023

note that while this does improve things, I'm not 100% sure it solves the problem. My initial attempt also involved trying to set the current post the same way the_post does which did nothing

tomjn added a commit to tomjn/ElasticPress that referenced this issue Feb 21, 2023
When indexing a post, the content gets rendered, and shortcodes/blocks/etc may rely on the global `$post` variable to provide context that will be missing when performing an ES index.

In my case it was a block which did not handle the value being missing, but context dependent content would generate PHP warnings/errors or incorrect data
felipeelia added a commit that referenced this issue Feb 23, 2023
Fix #3325 Set the current post when indexing posts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants