Add author__not_in for comment queries

[chriscct7]

It would be useful to have the author__not_in parameter that we have in WP_Query on comment queries, that way you can easily (for example) query all comments made on a particular post not written by a specific user.

[nofearinc]

Add authorin and authornot_in for WP_Comment_Query

[nofearinc]

Sample implementation added for author__in and author__not_in

[boonebgorges]

author__in is ambiguous between comment author and post author. I think post_author__in is clearer.

Can we get some unit tests for these please?

[nofearinc]

Tests are ready, just need to override the name of the field.

[nofearinc]

rename post_author_ fields

[nofearinc]

phpunit tests

[chriscct7]

While I definitely see the usefulness of the ability to limit the comments returned by the author of the post, the ticket (which I apologize since it wasn't clear enough), was to be able to limit the comments returned by the author of the comments. For example, a query might be return all comments of post 14 except for ones written by user id 4.

While the patch for this issue adds post_author__in, I think the more helpful comment_author__in should also be added (since it was what I was actually proposing). I extremely apologize for any confusion.

[nofearinc]

author_email allows for filtering by email in the comments query, and there is user_id support as well - would that work for you?

The only problem I can see is the lack of user_id__not_in which would also break the convention with user_id.

[chriscct7]

[ignore this comment...Trac went crazy]

[chriscct7]

Replying to nofearinc:

author_email allows for filtering by email in the comments query, and there is user_id support as well - would that work for you?

The only problem I can see is the lack of user_id__not_in which would also break the convention with user_id.

The problem with author_email is I want to do exactly opposite of what it does. For example, your patch adds post_author__not_in which, if passed a single int id, does the exact opposite of using post_author. I'm looking for the ability to get comments that are not written by a specific person, so we'd probably need a comment_author__in and comment_author__not_in.

Does that make sense?

[nofearinc]

I agree with your concern and I see where it could be applied. The only problem I see is still convention as we currently have author_email and user_id and what would be the proper naming for the opposite filters (we can't rename those due to backwards compatibility).

[chriscct7]

Its highly unlikely someone would be using both one of those and not_in. What we could do to get around this is if the parameter comment_author__in isset, then we use that parameter and ignore the email and the id one. If its not then we use what we already have. This would let a person using Comments Query to write a query where if its on WordPress 4.1, it would use the more powerful single param, else if its on the old version fallback (complete backwards compatibility) to the old params.

[boonebgorges]

chriscct7 - Thanks for chiming in. I think the post_author__ arguments are still useful, so let's not discard them.

Re parameter names: We should aim to parallel WP_Query as much as possible, to make things easier for developers. So that suggests author__in and author__not_in. As noted, we have to keep user_id around.

Re internal logic: let's parse the existing user_id together with author__in and use the parsed value to build the IN clause.

What we could do to get around this is if the parameter comment_authorin isset, then we use that parameter and ignore the email and the id one.

No, I don't think we can do this. Currently you can pass user_id=5 and author_email=boone@example.com and you'll get back comments from *both* user 5 and boone@…. We can't have one of these params cancel the other one out. I do think we can parse together user_id and author__in in PHP, as noted above, but even this is optional - MySQL will figure out what to do with AND user_id IN (3,4,5) AND user_id IN (3,4).

[chriscct7]

Hi Boone,
I by no any means intended to suggest to discard the existing patch. I'm actually a really big fan of what it does. I just wanted to point out that while its really useful it wasn't the intended goal of the ticket, and suggested we instead append to it so we get both the intended functionality of the ticket and the functionality in the patch. I don't really care as to how its implemented, but somehow, we need to add the ability to be able to get all comments that aren't written by a specific author or array of authors.

While the goal is to parallel WP_Query, it should be noted that the author parameters always correspond to the author of the given data, in the normal case a post. On one hand while I get how author__not_in referring to the comment author, might be confusing, at the end of the day the author parameters always point to the data, not what its displayed on. Its the way that most developers will end up expecting comment querying to work, and actually the whole reason I opened this ticket, was because I mistakenly assumed author__not_in already was an existing parameter for comment queries and had pushed code assuming it was. While my mistake alone shouldn't be contrived to be a barometer of sorts for the developer community at whole by a long shot, I think its safe to assume when someone sees the author parameter in WP_Query and interprets it to be the author of the post, they'll see the same parameter for comment querying as the author of the comment. It could also be implemented as proposed above using a comment_author__not_in name.

What is your thoughts?

[boonebgorges]

What is your thoughts?

Yup, I totally agree. Sorry if that didn't come across in the above. I'm suggesting four new params:

'author__in', // comment author
'author__not_in', // comment author
'post_author__in', // post author
'post_author__not_in', // post author

[chriscct7]

Awesome, fantastic news!

Quick Trac question, when a patch exists for a ticket, and it adds functionality, but the patch needs to be updated to add more functionality (like in this case) is that needs-patch or refresh-patch?

Unless @nofearinc is already working on it, I'd like to take a stab at writing the update for the patch, and then ideally someone could review it. Would that be okay?

[nofearinc]

Happy to co-work on that, so I'll wait for the update if you'd like to tackle it yourself.

[boonebgorges]

Quick Trac question, when a patch exists for a ticket, and it adds functionality, but the patch needs to be updated to add more functionality (like in this case) is that needs-patch or refresh-patch?

needs-patch is correct. needs-refresh is for patches that no longer apply cleanly (I actually used it incorrectly above - sorry about that). See ​http://codex.wordpress.org/Reporting_Bugs#Trac_Keywords

Unless @nofearinc is already working on it, I'd like to take a stab at writing the update for the patch, and then ideally someone could review it. Would that be okay?

Sounds great. Thanks!

[chriscct7]

First attempt at patch that implements all 4 parameters

[chriscct7]

Okay, uploaded my patch for it, which someone should look over. Marking as needs unit tests, as if the new patch is accepted, the 2 parameters added in it will need to have tests written for it to be added to the existing unit test patch file. With the new patch, all 4 new parameters listed in comment 16 are implemented

[boonebgorges]

chriscct7 - Patch looks good to me, though we do need unit tests to move forward. If you've never written unit tests, this is a good time to learn :) ​https://make.wordpress.org/core/handbook/automated-testing/

[nofearinc]

+1 for the patch, and I would "clone" the other unit test attached here for starters. Chris, let me know if you need help (or want me to submit the tests for your case as well).

[chriscct7]

Will have an updated test case patch file later today. Didn't want to write the unit tests until the implementation was accepted, so that I knew what I was testing :)

[chriscct7]

My sincere apologies. I got so offtrack with other WordPress trac tickets I forgot about this one. Will update with updated testing patch file tomorrow

[nofearinc]

post_author_ and author_ unit tests

[nofearinc]

post_author_ and author_ unit tests

[nofearinc]

Doing some gardening, submitting the unit tests for the author__in and author__not_in arguments.

[boonebgorges]

In 29935:

Comment/post author in/not_in for WP_Comment_Query.

Props nofearinc, chriscct7.
Fixes #29885.