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

Improve where statement filtering. #179

Closed
wants to merge 1 commit into from
Closed

Improve where statement filtering. #179

wants to merge 1 commit into from

Conversation

TracKer
Copy link

@TracKer TracKer commented Mar 22, 2018

In case if you want to modify default "where" statement you will fail since the query preparation is not working if there is wpp_query_where filter set. The only possible way in current implementation is to perform arguments (prefixed with "%") replacing in custom wpp_query_where filter, but arguments are not available there. So the solution is to send also arguments ($args) to filter function.

@cabrerahector
Copy link
Owner

cabrerahector commented Mar 24, 2018

Hi Andrey,

In case if you want to modify default "where" statement you will fail since the query preparation is not working if there is wpp_query_where filter set.

Could you please share more details and/or an example where this happens? (expected behavior vs current behavior)

@TracKer
Copy link
Author

TracKer commented Mar 25, 2018

I was needed to add some specific filtering (by content in meta field) to posts list. So I have added additional conditions to the where query. The version of plugin published on wordpress.org is a little different from master here (I didn't noticed this while creating this PR), so if you will check line 420 in includes/class-wordpress-popular-posts-query.php (uncommented in published version) you will see that it's blocking from applying query preparation filters to the query if where conditions were altered.

I guess this is not an issue anymore, since there are changes I didn't noticed right in the place that is causing problems in my case.

@cabrerahector
Copy link
Owner

Thanks for replying back, Andrey.

Yeah, I had a similar problem and noticed the issue as well. I changed the logic and commented out that line while I was testing it, but forgot to remove it before pushing the code to the repo.

Anyways, I'm guessing this means your PR isn't necessary anymore and so I'm closing this. Thanks for contributing, though :)

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