Opened 8 years ago
Last modified 8 years ago
#40073 new enhancement
WP_Comment_Query should support a comment type of "comment"
Reported by: | rogerlos | Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 4.8 |
Component: | Comments | Keywords: | |
Focuses: | Cc: |
Description
Scenario: User is leveraging the comment_type
field in their theme or a plugin, and desires all comments to have a type. They assign "regular" comments a comment_type
of "comment" as that seems to make sense.
If they then ask for those comments using get_comments
or another function which leverages WP_Comment_Query
:
$comments = get_comments( [ 'type' => 'comment', ] );
The returned list will not return any comments which have comment_type
set to "comment", only those with no type at all.
As this behavior (the swallowing of "comment" as a type entirely, rather than looking for ['','comment']
) is undocumented, perhaps WP_Comment_Query
could be altered as such:
715 case 'comment': 716 case 'comments': 717 $comment_types[ $operator ][] = "''"; ___ $comment_types[ $operator ][] = "'comment'"; ___ $comment_types[ $operator ][] = "'comments'"; 718 break; 719 720 case 'pings': 721 $comment_types[ $operator ][] = "'pingback'"; 722 $comment_types[ $operator ][] = "'trackback'"; ___ $comment_types[ $operator ][] = "'pings'"; 723 break;
A sounder approach would probably be to allow a flag to be passed:
708 foreach ( $_raw_types as $type ) { ___ if ( empty( $this->query_vars['ignore_default_types'] ) ) { 709 switch ( $type ) { ... 729 } ___ } else { ___ $comment_types[ $operator ][] = $wpdb->prepare( '%s', $type ); ___ } ... 735 }
An alternative might be to throw an exception when setting comment_type
to one of the default values using wp_new_comment
, wp_insert_comment
and similar functions.
(I feel since there is no way to retrieve a comment when the type is one of these "reserved" words with standard WP functions, comments should probably not be able to be saved with a "reserved" type using standard functions if WP_Comment_Query
is not altered.)
At the very least, the documentation for comment "insert" functions should mention that there are reserved keywords on comment_type
, note what they are, and that if you use one of them you will need to use the comments_clauses
filter to allow them to be retrieved.
Change History (2)
#2
@
8 years ago
Thanks for looking at this.
I agree that throwing an exception is the least desirable solution, and that passing a flag is probably the most flexible and backwards-compatible.
I also agree we should let people set their comment types to 'comment' or 'all' or 'pings' if that's what they want to do, as long as it's documented somewhere how to get them back.
Naming parameters and variables is something I'm awful at, so I'm confident there is a better name for the flag which might actually even tell a user what it does just by reading the var name.
One thing I did not note about get_comments
:
Even when the type
parameter failed, I expected type__in
to NOT fail, as I assumed it would be "literal", ie, outside of whatever magic was happening that caused type
to fail. I think my line of reasoning was "if someone is going to specifically generate an array of types, surely the class will take them all literally".
I have no basis for that other than my own prejudices, but it might also be a way forward if a flag is decided against. Seeing as there are only two magic types, plus a choice which returns everything, it seems unlikely a user sending type__in
would want anything other than literal types.
Finally, if WP_Comment_Query
is modified, I suggest WP consider that core start setting a comment type when saving (how about "comment"!).
There are a number of interesting things which can be done by saving appropriate content as "comments" even when they're not actual comments (for example, gaining access to spam checks, flood prevention, etc), and having no type by default can make it difficult to work within the data saved in the table.
@rogerlos Thanks very much for the clear and sensible ticket.
I agree that the current situation is not ideal. If we're going to allow
comment_type
to be set to 'comment', then we need to be able to allow these comments to be queried by type as well. As you indicate, this is a problem not only for 'comment', but for 'pings' and 'all' as well.Comment query type handling is already too "magic" for my taste. The fact that 'comment' fetches and 'pings' fetches 'pingback'/'trackback' is confusing, and is there only for legacy reasons. Let's not make things worse by trying to lump more into these clauses, as in your first suggestion.
I think I like the idea of combining a few of your suggestions:
ignore_default_types
, but maybe we can come up with a better name, since it's less about "ignoring" than about translating reserved kewords) that causestype
to be interpreted literally.wp_insert_comment()
andwp_update_comment()
documentation to strongly discourage the use of comment type 'comment', 'pings', 'all'.I don't think we can actually *prevent* the use of these comment types, for backward-compatibility reasons. I can imagine that there are many cases like yours where developers are passing 'comment' and expecting it to be recorded in the database.
I'd like a second opinion from @rachelbaker about this.