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 search_meta key param support to ES_Query - see #17 #34

Merged
merged 3 commits into from
Jul 3, 2014

Conversation

tlovett1
Copy link
Member

Should close #17 . This PR allows you to pass "search_meta" to ES_Query that supports an array of post meta keys to be searched. @AaronHolbrook #reviewmerge

@tlovett1
Copy link
Member Author

I don't think a filter makes sense. Filters are difficult to use. The purpose of the query object is to be able to customize which posts your receive. Empowering the user with the query object is much more valuable than forcing them to use a filter.

Regarding using meta_query, I'm undecided. The intention of meta_query is different from that of search_meta. meta_query let's you specify keys and values. search_meta does not let you specify a value but rather relies on 's'. If you created a WP_Query object with arguments 's' and 'meta_query', the meta query part would not use 's'. The intention of my pull request is to allow 's' to be used in a meta query thus necessitating a new argument, search_meta.

@AaronHolbrook
Copy link
Contributor

Instead of defining values in the meta_query you could simply define keys / terms to include.

I was referring to this for the filter instead of an action prior to the search request firing: 3357674#classes-class-ep-api-php-P4

I don't think it makes sense to only offer an action in that spot, why provide the ability to execute functionality but not modify what is about to be executed by the plugin? I think the filter makes more sense in this case. Also - not trying to force anyone to use a filter, simply providing a way to modify behavior in unforeseen and unplanned ways.

@tlovett1
Copy link
Member Author

I 100% agree on only adding an action. The only reason I added that was to make unit testing easier. I'm totally open to adding a filter in another PR.

@tlovett1
Copy link
Member Author

tlovett1 commented Jul 2, 2014

@AaronHolbrook anymore issues with this? Would be nice to get this merged

@AaronHolbrook
Copy link
Contributor

Any concerns over whether or not we should sanitize the $key of the meta field being passed in?

AaronHolbrook added a commit that referenced this pull request Jul 3, 2014
Add search_meta key param support to ES_Query - see #17
@AaronHolbrook AaronHolbrook merged commit 4ef5a89 into master Jul 3, 2014
@AaronHolbrook AaronHolbrook deleted the feature/post-meta-param branch July 3, 2014 02:09
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