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

Image block and post_mime_type #1604

Closed
ethanclevenger91 opened this issue Dec 17, 2019 · 6 comments · Fixed by #2222
Closed

Image block and post_mime_type #1604

ethanclevenger91 opened this issue Dec 17, 2019 · 6 comments · Fixed by #2222

Comments

@ethanclevenger91
Copy link

ethanclevenger91 commented Dec 17, 2019

Describe the bug
The Gutenberg image block adds 'post_mime_type' => 'image' to the media modal's search queries (the query-attachments AJAX action). The classic editor didn't add this parameter.

ElasticPress translates post_mime_type to an exact match, but in wp_post_mime_type_where(), which WP_Query uses to parse that argument, if there's no / in the post_mime_type, the resulting SQL wildcards the second half of the mime type, so LIKE 'image/%' (for this specific case).

While admin-side functionality isn't the default for this plugin, updating this logic would make it more seamless.

Steps to Reproduce

  1. Install/configure ElasticPress, Classic Editor plugin
  2. Enable document indexing
  3. Enable admin and AJAX via ep_admin_wp_query_integration and ep_ajax_wp_query_integration
  4. Add an image to the media library
  5. Create a new post
  6. Edit with classic editor
  7. Use the "Add Media" button to open the media modal
  8. See your image
  9. Use the search bar to search for your image
  10. Still see your image
  11. Close out. Edit a post with the block editor.
  12. Add a new image block to bring up the media modal
  13. See your image
  14. Use the search bar to search for your image
  15. See no results returned

Expected behavior
Image search results in the admin should be consistent.

Environment information

  • Device: ThinkPad
  • OS: Windows
  • Browser and version: Firefox
  • Plugins and version: ElasticPress, latest
  • Theme and version: Custom
  • Other installed plugin(s) and version(s): I can provide this if the above steps fail to reproduce

Additional context
Current workaround is a pre_get_posts filter:

public function query_attachments( &$query ) {
	if( !wp_doing_ajax() || $_POST['action'] != 'query-attachments' ) {
		return;
	}

	$query->set('post_mime_type', '');
}
@ethanclevenger91 ethanclevenger91 added the type:bug Something isn't working. label Dec 17, 2019
@brandwaffle
Copy link
Contributor

@ethanclevenger91 agreed this would be a good thing to get working. Currently, the only way I'm aware of to create a partial match in Elasticsearch is to map this particular field with the ngram analyzer, which enables partial string matches. That could create some indexing overhead, so I'm also wondering if perhaps we could shortcut this by splitting the 'image/' string into an array of each part, so that we could exact match on just 'image' across any images regardless of format. @tlovett1 any thoughts?

@tlovett1
Copy link
Member

This is admin search correct @ethanclevenger91? I assume you have the Protected Content feature enabled?

@ethanclevenger91
Copy link
Author

Admin - correct. I didn't have the Protected Content feature enabled, though I'm enabling it now after better understanding what it does.

@tlovett1 tlovett1 added confirmed bug high priority and removed type:bug Something isn't working. labels Mar 10, 2020
@felipeelia
Copy link
Member

I'm working on it.

@felipeelia felipeelia self-assigned this May 25, 2020
@roborourke
Copy link
Contributor

I just ran into this - doesn't seem to be fixed in the latest version. The issue is that the plugin assumes if a string value is passed then it's a truncated / prefix mime type and if it's an array then they're fully qualified:

https://github.com/10up/ElasticPress/blob/develop/includes/classes/Indexable/Post/Post.php#L907-L934

This isn't exactly reliable as the image block shows but WP will accept any combination of those truncated and fully qualified values.

I've got a temporary workaround here:

https://github.com/humanmade/altis-enhanced-search/pull/98/files

The latter part of it will nicely handle whatever is passed to the post_mime_type WP_Query argument (after converting that value to an array if a string was given:

https://github.com/humanmade/altis-enhanced-search/pull/98/files#diff-2d9bc91244a2cb57445498ad1c971f7dR318-R349

I'll make a PR as soon as I can.

roborourke added a commit to roborourke/ElasticPress that referenced this issue Sep 10, 2020
As noted in issue 10up#1604 the image block sends a mime type value of `image` through as an array which goes against the convention this part of the query filter relied on. This update makes the filter more agnostic, working with a single string (as `wp_post_mime_type_where()` does) or an array, plus it will then separate the values into full terms and prefixes.

This has the benefit of supporting almost any combination of values given in the `post_mime_type` argument and avoids the use of a `regexp` query which can be slower to run.
@mckdemps mckdemps assigned Rahmon and unassigned felipeelia May 28, 2021
@felipeelia
Copy link
Member

We can use wp_get_mime_types to check which mime types exist and search for all of them. So, for image/% it would mean [ 'image/jpeg', 'image/gif', 'image/png', 'image/bmp', 'image/tiff', 'image/x-icon', 'image/heic' ]

@felipeelia felipeelia added this to the 3.6.0 milestone Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants