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

Fix for issue #625 #628

Merged
merged 3 commits into from
Nov 29, 2018
Merged

Fix for issue #625 #628

merged 3 commits into from
Nov 29, 2018

Conversation

hyperionjrw
Copy link
Contributor

A previous change #624 broke the regEx which then broke the SQL see #625 for full details.

A previous change broke the regEx which then broke the SQL which then broke the front end.
@hyperionjrw hyperionjrw changed the title Fix for issue #625 from the original plugin. Fix for issue #625 Nov 27, 2018
Copy link
Contributor

@rebeccahum rebeccahum left a comment

Choose a reason for hiding this comment

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

Hi there! Please see the below that needs to be addressed:

function posts_where_filter( $where, $query ) {
global $wpdb;

if ( $query->is_author() ) {
$post_type = $query->query_vars['post_type'];
if ( 'any' === $post_type ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How come we are removing this line here? This will cause a syntax error and throw off the support for 'any' post types.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't think I was. 🤔
I think I may have messed up when selecting which bits to commit as I had a heap of white space changes show up due to my automatic tendency to run a code formatting shortcut after almost all edits and also doing this between other things.


//If we have an ID but it's not a "real" ID that means that this isn't the first time the filter has fired and the object_id has already been replaced by a previous run of this filter. We therefore need to replace the 0
// This happens when wp_query::get_posts() is run multiple times.
if ( false === get_user_by( 'id', $id ) ){
$id = 0;
if ( $id !== '\d+' && false === get_user_by( 'id', $id ) ){
Copy link
Contributor

Choose a reason for hiding this comment

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

The check for $id !== '\d+' is probably not necessary. I think we can just have $id assigned to '\d+' if get_user_by( 'id', $id ) returns false since $id will not equal '\d+' if $id ends up equating to get_queried_object_id() (which will not be '\d+').

Copy link
Contributor Author

@hyperionjrw hyperionjrw Nov 29, 2018

Choose a reason for hiding this comment

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

The check for the string was to avoid doing an unnecessary user query that's obviously going to fail. String check is far quicker than a db query.

@@ -713,12 +717,12 @@ function posts_where_filter( $where, $query ) {
}
$terms_implode = rtrim( $terms_implode, ' OR' );

$id = is_author() ? get_queried_object_id() : '\d+';
$id = is_author() && $query->is_main_query() ? get_queried_object_id() : '\d+';
Copy link
Contributor

Choose a reason for hiding this comment

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

This fix works without the check for $query->is_main_query() so I think we don't need to add it for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you query a second user from author.php to show something like, people in same dept or other authors you may like, you won't get the correct posts, you'll only get those where the user is in the post_author column.

e.g. If I add $x = new WP_Query( array ( 'post_status' => 'publish', 'author' => 9099999999 ) ); to author.php after the main loop. Go to /?author=1 the ID that's passed to the regex is 1 not 9099999999 so our custom query is missing any posts where the queried user isn't in the post_author column.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thinking this through you also need this check to avoid further risk of WordPress throwing db errors. If your custom queried user ID contains the author page user ID somewhere before the 3rd char from the end then we'd end up with a split user ID and invalid SQL.

e.g Go to /?author=1 and sub-query for author 10123456. That'd produce regex that's looking through the the SQL for \s*(.*1.)/ which would match 1012 from our ID and put the new SQL in the middle of the ID and thus a db error would be thrown.

Added some comments and fixed a wonky commit.
Changing the order to match previous commit.
@rebeccahum rebeccahum merged commit d4490a8 into Automattic:master Nov 29, 2018
rebeccahum added a commit that referenced this pull request Mar 26, 2019
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