Make WordPress Core

Opened 15 years ago

Closed 10 years ago

Last modified 5 years ago

#12668 closed enhancement (fixed)

Better support for custom comment types

Reported by: ptahdunbar's profile ptahdunbar Owned by: ptahdunbar's profile ptahdunbar
Milestone: 4.1 Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch
Focuses: Cc:

Description

Similar to the improved support for custom post types #9674, we really need better support for custom comment types. At the database layer, this is completely supported and is used for post type comments, trackbacks and pingbacks. The code just needs to be decoupled into standard APIs.

Specifically, we'll need APIs for registering comment types, and comment statuses so the admin interface (edit-comments.php) can support them.

I'll be working on a patch, although feel free to provide your own patches, ideas and insights :)

Attachments (14)

12668.patch (982 bytes) - added by mordauk 10 years ago.
Add comment_type support to get_approved_comments()
12668-2.patch (2.0 KB) - added by mordauk 10 years ago.
12668-3.patch (2.0 KB) - added by mordauk 10 years ago.
12668-dashboard.patch (490 bytes) - added by mordauk 10 years ago.
12668-widgets.patch (558 bytes) - added by mordauk 10 years ago.
12668-comments-list-table.patch (921 bytes) - added by mordauk 10 years ago.
12668-comments-query.patch (1.7 KB) - added by mordauk 10 years ago.
12668-comments-query.2.patch (1.6 KB) - added by dancameron 10 years ago.
cleaning up previous patch
12668-get_approved_comments-WP_COMMENT_QUERY.patch (3.4 KB) - added by dancameron 10 years ago.
Use WP_Comment_Query to improve get_approved_comments since we like caching and all that.
12668-comments-query.3.patch (1.5 KB) - added by dancameron 10 years ago.
array_map( 'esc_sql', $that ). Also Includes 12668-get_approved_comments-WP_COMMENT_QUERY.patch by default.
12668-comments-query.4.patch (1.6 KB) - added by dancameron 10 years ago.
noticed how I created confusion by removing the check for the comment type.
12668-comments-query-new-tests.patch (7.1 KB) - added by dancameron 10 years ago.
New tests for comments query and get_approved_comments prior to any patch.
12668-v2-comments-query.patch (3.7 KB) - added by dancameron 10 years ago.
This patch addresses all points of the new scope
12668-comments-query-new-tests.2.patch (19.8 KB) - added by dancameron 10 years ago.
21 tests in all. I'm sure there's some variation that I missed but the coverage these tests have is fairly extensive

Download all attachments as: .zip

Change History (100)

#1 follow-up: @mikeschinkel
15 years ago

  • Cc mikeschinkel@… added

Interesting idea. If comments were moved to be in the wp_posts table with post_type='comment' then a lot of functionality would come for free?

#2 follow-up: @nacin
15 years ago

Cross-referencing #10856. Should be handled together.

#3 @unsalkorkmaz
15 years ago

  • Cc unsalkorkmaz added

#4 @johnbillion
14 years ago

  • Cc johnbillion@… added

#5 @alex-ye
14 years ago

Good idea , I will try to creating my patch also ...

#6 @bainternet
14 years ago

  • Cc bainternet added

#7 @gruvii
13 years ago

  • Cc gruvii added

#8 follow-up: @gruvii
13 years ago

Would love to see comments in the wp_posts table. There are infinite possiblities especially taxonomies for the comments etc.

#9 in reply to: ↑ 8 @nacin
13 years ago

Replying to gruvii:

Would love to see comments in the wp_posts table. There are infinite possiblities especially taxonomies for the comments etc.

I don't see this ever happening. That said, you can already do taxonomies on comments. That it's difficult (and that you don't really know about it) is more about deficiencies in the taxonomy system rather than the comments system.

#10 in reply to: ↑ 2 @mitchoyoshitaka
13 years ago

  • Cc mitcho@… added

Replying to nacin:

Cross-referencing #10856. Should be handled together.

I don't see why these two should be handled together. #10856 involves the migration/removal of a few columns from the comments table. This bug is about API's for comment type registration and handling... the comment_type column already exists.

#11 @jane
13 years ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Two years without a patch, so I'm closing. Reopen if there's a patch to be reviewed.

#12 @nacin
13 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

Re-closing as maybelater. If we do a comment/feedback loop in 3.5 or 3.6, this will almost certainly get hit.

#13 @nacin
13 years ago

  • Resolution set to maybelater
  • Status changed from reopened to closed

#14 follow-up: @mordauk
12 years ago

  • Cc pippin@… added

I'd love to see this discussed again. Here's an example of why I'd like to use it:

In Easy Digital Downloads, we use comments on our payments post type to track info related to individual purchases, and also for allowing store owners to leave notes about individual orders. Comments work exceptionally well for this, except they start showing up in recent comment widgets, which is not optimal, especially when notes may contain sensitive information.

I'd love to be able to register a custom comment type that would be automatically excluded from recent comment streams (since they would only pull in core comment types by default). This would give us a much more granular level of control.

#15 in reply to: ↑ 14 @MikeSchinkel
12 years ago

  • Cc mike@… added

Replying to mordauk:

I'd love to see this discussed again.

+1. I'd like to build a plugin that uses comments for Twitter status updates tied to a Twitter Account periodic post (daily, weekly, monthly) with a parent post for Twitter account.

I'd also like to build another plugin that uses comments to manage support requests for a support ticketing system.

And another plugin that uses comments for suggested improvements to pages like how php.net has comments, but with the goal of improving the page, marking the suggestion complete, and then hiding the suggestion during normal page view. My use-case is for internal use for using WordPress for creating user documentation.

#16 @dcowgill
12 years ago

  • Cc dcowgill@… added

#17 @gvenk
11 years ago

  • Cc gvenk added

#18 @joshlevinson
11 years ago

  • Cc joshlevinson added

I would also like to see custom comment types. I will probably end up using comment taxonomies for now (if I can), but I could see types becoming very useful. For example, along with registering a post type, I could register a comment type ( think Products with Product FAQs and Product Reviews ). Querying, submitting, and segregating these different types of comments would be easier done as a comment type IMHO.

Version 0, edited 11 years ago by joshlevinson (next)

#19 @andymacb
11 years ago

  • Cc am@… added

#20 @SergeyBiryukov
11 years ago

#25674 was marked as a duplicate.

#21 @sc0ttkclark
11 years ago

  • Cc lol@… added
  • Resolution maybelater deleted
  • Status changed from closed to reopened

I opened #25674 because I couldn't find this one.

We could port most of the Post Type handling internals like register_post_type > register_comment_type and get_post_type > get_comment_type which would make this implementation in core easier. We could also register the built-in types like comment/pingback etc

Then, comment_type could be implemented like post_type is in edit.php, for the comments page, and allow for the new comment types to be added into their own menus.

I love the potential of what Custom Comment Types can bring, but unfortunately not having an API to work with them can make it cumbersome to build with.

I can make a first pass at a patch on this if anyone is interested in moving this forward.

I ended up making a new GitHub repo for a functionality proposal. It's a work in progress and is open to contributors and pull requests.

http://github.com/sc0ttkclark/wp-custom-comment-types

#22 @sbrajesh
11 years ago

  • Cc brajesh@… added

#23 @SergeyBiryukov
11 years ago

  • Milestone set to Awaiting Review

#24 @sbruner
11 years ago

  • Cc sbruner@… added

#25 @meloniq
11 years ago

  • Cc meloniq@… added

#26 @judgej
11 years ago

+1

I would love to see this functionality expanded too. We already have popular plugins such as WooCommerce that use their own custom comment type ("order_notes", against the "shop_order" post type) and fill up the comments table quickly. This slows a site down substantially, as the core WP does not recognise these custom comment types as distinct from the standard comments against public posts when doing its counts of comments to be approved/latest comments etc.

I've raised it also as a question here:

http://wordpress.stackexchange.com/questions/130809/what-is-the-point-of-get-comment-count-if-you-cannot-limit-by-a-comment-type

#27 @mordauk
11 years ago

For plugins that store things like order notes, or other private data, there are three filters that have to be used if the plugin wants to:

  1. Remove the custom comment type from the general comment queries (recent comments widget for example)
  2. Remove the custom comment type from comment feeds
  3. Remove the custom comment type from comment counts

To do this, plugins have to use 3 separate filters, the last one being pretty expensive (causes a lot of slow queries on sites with a lot of comments):

<?php
/**
 * Exclude notes (comments) on edd_payment post type from showing in Recent
 * Comments widgets
 *
 * @since 1.4.1
 * @param array $clauses Comment clauses for comment query
 * @param obj $wp_comment_query WordPress Comment Query Object
 * @return array $clauses Updated comment clauses
 */
function edd_hide_payment_notes( $clauses, $wp_comment_query ) {
    global $wpdb;

        $clauses['where'] .= ' AND comment_type != "edd_payment_note"';
    return $clauses;
}
add_filter( 'comments_clauses', 'edd_hide_payment_notes', 10, 2 );


/**
 * Exclude notes (comments) on edd_payment post type from showing in comment feeds
 *
 * @since 1.5.1
 * @param array $where
 * @param obj $wp_comment_query WordPress Comment Query Object
 * @return array $where
 */
function edd_hide_payment_notes_from_feeds( $where, $wp_comment_query ) {
    global $wpdb;

        $where .= $wpdb->prepare( " AND comment_type != %s", 'edd_payment_note' );
        return $where;
}
add_filter( 'comment_feed_where', 'edd_hide_payment_notes_from_feeds', 10, 2 );


/**
 * Remove EDD Comments from the wp_count_comments function
 *
 * @access public
 * @since 1.5.2
 * @param array $stats (empty from core filter)
 * @param int $post_id Post ID
 * @return array Array of comment counts
*/
function edd_remove_payment_notes_in_comment_counts( $stats, $post_id ) {
        global $wpdb, $pagenow;

        if( 'index.php' != $pagenow ) {
                return $stats;
        }

        $post_id = (int) $post_id;

        if ( apply_filters( 'edd_count_payment_notes_in_comments', false ) )
                return $stats;

        $stats = wp_cache_get( "comments-{$post_id}", 'counts' );

        if ( false !== $stats )
                return $stats;

        $where = 'WHERE comment_type != "edd_payment_note"';

        if ( $post_id > 0 )
                $where .= $wpdb->prepare( " AND comment_post_ID = %d", $post_id );

        $count = $wpdb->get_results( "SELECT comment_approved, COUNT( * ) AS num_comments FROM {$wpdb->comments} {$where} GROUP BY comment_approved", ARRAY_A );

        $total = 0;
        $approved = array( '0' => 'moderated', '1' => 'approved', 'spam' => 'spam', 'trash' => 'trash', 'post-trashed' => 'post-trashed' );
        foreach ( (array) $count as $row ) {
                // Don't count post-trashed toward totals
                if ( 'post-trashed' != $row['comment_approved'] && 'trash' != $row['comment_approved'] )
                        $total += $row['num_comments'];
                if ( isset( $approved[$row['comment_approved']] ) )
                        $stats[$approved[$row['comment_approved']]] = $row['num_comments'];
        }

        $stats['total_comments'] = $total;
        foreach ( $approved as $key ) {
                if ( empty($stats[$key]) )
                        $stats[$key] = 0;
        }

        $stats = (object) $stats;
        wp_cache_set( "comments-{$post_id}", $stats, 'counts' );

        return $stats;
}
add_filter( 'wp_count_comments', 'edd_remove_payment_notes_in_comment_counts', 10, 2 );

These are the filters we use in Easy Digital Downloads. You may notice that in the last one we included a check that causes it to just exit early if not on index.php. We discovered that this was resulting in a lot of slow queries and so had to disable it everywhere but index.php

If we had a true way of handling custom comment types, we could avoid all of this mess.

Last edited 11 years ago by DrewAPicture (previous) (diff)

#28 @amolv
11 years ago

#27089 was marked as a duplicate.

This ticket was mentioned in IRC in #wordpress-dev by DrewAPicture. View the logs.


10 years ago

#30 @mordauk
10 years ago

I'd love to see more traction on this for 4.1? More than happy to contribute towards getting patches written up once some more direction is decided on.

I like where @sc0ttkclark is going but I wonder if it's too much. I don't personally see a real need for complete UI support for custom comment types yet. If a plugin wants to register and display custom comment types, the display can be left up to them. Just being able to register a comment type and query it (and exclude it from normal queries) would be more than sufficient for most cases.

#31 @sc0ttkclark
10 years ago

I'm cool with API first, UI second approach.

#32 @ocean90
10 years ago

#29929 was marked as a duplicate.

#33 @mark-k
10 years ago

As I said in the duplicate ticket, IMO trying to support new comment types is the wrong approach as we are way to late for that. Comment just another content (post) type that like attachments can be attached to other content, so the natural way to register new comment types should be with register_post_type.

After all, the real issue behind the ticket is to provide "comment" style admin to some content type. because it is convenient, and not because we are talking about some "real" comments.

Bonus points to have them on different tables as the edd payment notes, the form post types and others just bloat the post and comment tables and reducing their performance without being relevant to any front-end functionality.

#34 follow-up: @judgej
10 years ago

@mark-k The whole point of this is that comments are *not* another post type. They live in a table all of their own, and that table can grow enormously when used by some plugins (e.g. WooCommerce uses a comment for each status change to an order, which usually amounts to six or more comments on each order, of which there can be hundreds of thousands on even a reasonably sized shop).

The fact that there are no "types" used to group comments together by use, means that when this table grows, the queries to count comments on that table become very slow. A plugin can't say, "give me the count of all comments *I* have anything to do with" - it gets the counts of all comments globally across all parts of the application).

Alternative approaches would be for plugins to stop using comments to store comments.

#35 in reply to: ↑ 34 @mark-k
10 years ago

Replying to judgej:

@mark-k The whole point of this is that comments are *not* another post type. They live in a table all of their own, and that table can grow enormously when used by some plugins (e.g. WooCommerce uses a comment for each status change to an order, which usually amounts to six or more comments on each order, of which there can be hundreds of thousands on even a reasonably sized shop).

  1. I think that at this stage what table is being used is an implementation detail. Right now the way I understand the ticket it is not about how to reduce the bloat in the comments table.
  1. which is why wC and EDD should not use the comments table at all, and should have their own table. See my bonus points section.

Alternative approaches would be for plugins to stop using comments to store comments.

Exactly my point, WC and EDD store data as comments because it results in convenient admin UI and API, not because they are anything like a conventional comment.

#36 @dancameron
10 years ago

Maybe I'm hearing this wrong but I don't think a solution recommendation should ever be create custom tables. That seems like a slippery slop of doing nothing to improve WP and many WP hosts don't allow it.

#37 follow-up: @judgej
10 years ago

TBH, I'm torn.

Yes, the comments are being used for various purposes (mainly logging stuff) on non-blog posts due to their administration convenience. That to me feels right and in scope for what the comments are for.

And yes, creating tables is a slippery slope.

However, custom tables can often be a much better fit for some purposes. Back to WooCommerce again, each order has a dozens of custom fields - order options, each part of each postal address etc. The rate at which the wp_postmeta grows as a result is frightening. I don't think WP was ever designed to be scaled in the direction that some of these plugins are taking it, and I hate to say it, but they really should be putting their data into more optimised (i.e. flatter and wider) tables of their own.

I think this is always going to be a problem as WP is pushed more away from its simple blogging roots; the people pushing it to its limits are moving faster than WP ever will, and the limits causing immense performance issues on some sites are being exposed.

Last edited 10 years ago by judgej (previous) (diff)

#38 in reply to: ↑ 37 @MikeSchinkel
10 years ago

Replying to mark-k:

Exactly my point, WC and EDD store data as comments because it results in convenient admin UI and API, not because they are anything like a conventional comment.

Replying to judgej:

However, custom tables can often be a much better fit for some purposes.

One of the big issues with custom tables is that any plugin developer who chooses to use custom tables will (effectively) never be able see his or her plugin used on WordPress.com or on WordPress VIP. I know a vendor of a very well known plugin that has lamented this fact and discussed how I might be able to help them get around the issue using RESTfully accessed storage-as-a-service solutions.

While the former is only a pipe dream for most plugin developers, the latter is a real consideration for anyone wanting to build a plugin to address the enterprise market.

I think this is always going to be a problem as WP is pushed more away from its simple blogging roots; the people pushing it to its limits are moving faster than WP ever will, and the limits causing immense performance issues on some sites are being exposed.

True, there are always tradeoffs. Anyone who sees their site gain success will have to deal with issues no matter what solution is chosen.

#39 follow-up: @mordauk
10 years ago

Regardless of whether storing things like payment notes in comments is "proper", I still think better support for custom comment types has a lot of merit.

Pingbacks and trackbacks are custom comment types. Why not make it easier to support more comment types like that?

The issue right now isn't so much that we don't have support for custom comment types, it's that we don't have support for separating comment types out from one another.

Anyone can go and create a custom comment type right now, but any comment inserted with a custom type will still be included in the default comment tables and lists, and removing a comment type from those queries is rather expensive, as shown above.

#40 in reply to: ↑ 39 @mark-k
10 years ago

Replying to mordauk:

I fully agree with anything that will improve on the current situation even if it will end up in just introducing new filters to be able to better customize the admin side. Whatever I write below is not that important to me

Regardless of whether storing things like payment notes in comments is "proper", I still think better support for custom comment types has a lot of merit.

Pingbacks and trackbacks are custom comment types. Why not make it easier to support more comment types like that?

<pedantic> semantically Pingback === trackback, they are both remote mentions and the underlying way by which they were discovered doesn't make any semantical difference nor is there any point in presenting them differently </pedantic>

comments are just content type that represent content that might have been submitted by users with no publishing permission. EDD and WC use them for content that implicitly is always generated by authorized user, it doesn't need to go through a spam detection, doesn't need to be displayed in threads, doesn't need avatar.... it is not a comment in any semantical way.

The issue right now isn't so much that we don't have support for custom comment types, it's that we don't have support for separating comment types out from one another.

and the easiest way to achieve that is not to call "comments" things which are not comments. Why doesn't EDD use CPT for that? maybe it will be easier to fix that limitation than having better support for custom comment types.

Anyone can go and create a custom comment type right now, but any comment inserted with a custom type will still be included in the default comment tables and lists, and removing a comment type from those queries is rather expensive, as shown above.

What about theme support? I can't right now think of any example (which might mean that the whole argument is invalid ;) ) but all the comment related code I have seen assumes that if it is not pingback or trackback it is a comment therefor it is very likely that all comment types will be displayed as part of the comment stream.

#41 follow-ups: @judgej
10 years ago

the easiest way to achieve that is not to call "comments" things which are not comments.

Quite often they *are* comments, just not specifically "user comments on blog posts". Status changes on a shop order are logically comments on that shop order, a copy of which may be emailed to customer, or may be kept for administrator reference only. Some of those comments may be visible to the customer (but not any other customer) and some may not be.

Now, if the official line from WP were that "comments" are ONLY "public user-provided comments on just the "post" post type", then fine, we can see there is no scope to use it for anything else and can move on. But I think for now there is some disagreement on exactly what a comment is, so different developers expect to be able to use comments for a range of different purposes. So is there an official statement or documentation about the scope of "comments" beyond what we can see in the code? Should this FR start with an agreement on the scope first? It's been tossed around for five years now, so a different approach is needed to either move it forward, or release us hopefuls so we can create an alternative as a plugin.

#42 in reply to: ↑ 41 @mordauk
10 years ago

Replying to judgej:

the easiest way to achieve that is not to call "comments" things which are not comments.

Quite often they *are* comments, just not specifically "user comments on blog posts". Status changes on a shop order are logically comments on that shop order, a copy of which may be emailed to customer, or may be kept for administrator reference only. Some of those comments may be visible to the customer (but not any other customer) and some may not be.

Now, if the official line from WP were that "comments" are ONLY "public user-provided comments on just the "post" post type", then fine, we can see there is no scope to use it for anything else and can move on. But I think for now there is some disagreement on exactly what a comment is, so different developers expect to be able to use comments for a range of different purposes. So is there an official statement or documentation about the scope of "comments" beyond what we can see in the code? Should this FR start with an agreement on the scope first? It's been tossed around for five years now, so a different approach is needed to either move it forward, or release us hopefuls so we can create an alternative as a plugin.

For EDD and Woo, they are most definitely comments. While there are automated changes, such as payment statuses, logged, they are also used for manually entered admin and customer notes/comments on order.

#43 in reply to: ↑ 41 ; follow-up: @mark-k
10 years ago

Replying to judgej:

the easiest way to achieve that is not to call "comments" things which are not comments.

Quite often they *are* comments, just not specifically "user comments on blog posts". Status changes on a shop order are logically comments on that shop order, a copy of which may be emailed to customer, or may be kept for administrator reference only. Some of those comments may be visible to the customer (but not any other customer) and some may not be.

I took a look now at what EDD does. AFAICT all you can do is create a note and delete it. This is more of a log then a collection of comments.

Now, if the official line from WP were that "comments" are ONLY "public user-provided comments on just the "post" post type", then fine, we can see there is no scope to use it for anything else and can move on. But I think for now there is some disagreement on exactly what a comment is, so different developers expect to be able to use comments for a range of different purposes. So is there an official statement or documentation about the scope of "comments" beyond what we can see in the code? Should this FR start with an agreement on the scope first? It's been tossed around for five years now, so a different approach is needed to either move it forward, or release us hopefuls so we can create an alternative as a plugin.

I fail to see any real disagreement. If it works for WC and EDD to store logs as comments then why should anyone object, but the more interesting aspects of having actual different comment types is by supporting also questions, answers, reviews, likes/dislikes, etc... mixed for the same content.
For example there might be a post type of "test" used to publish online test. You might be able to ask a question to clarify the test, submit an answer, and after the test is over review or comment on it.

#44 in reply to: ↑ 43 @mordauk
10 years ago

Replying to mark-k:

I fail to see any real disagreement. If it works for WC and EDD to store logs as comments then why should anyone object, but the more interesting aspects of having actual different comment types is by supporting also questions, answers, reviews, likes/dislikes, etc... mixed for the same content.
For example there might be a post type of "test" used to publish online test. You might be able to ask a question to clarify the test, submit an answer, and after the test is over review or comment on it.

I think that's a perfect example of where custom comment types would be useful :)

#45 @dancameron
10 years ago

If a real world example of using comments outside of e-commerce and in a traditional post comment scope is necessary:

I'm in a situation were I'd like to have clients comment on a document (CPT). My users (of my plugin) won't necessarily require that these comments be filtered from the management screen but I'd expect it to be a support and UX issue because WordPress doesn't allow for the comments to be distinguished by there type, let alone a custom status. Instead I'd like to handle management on my own gracefully, with the help of a WP API to filter these comments from the default comments admin, then build my own comment management UI within the CPT edit screen (or some other mngt screen that I develop). ATM this isn't supported, as many have already address, and IMO I'm using comments within a traditional sense.


I'm interested in working with someone (or a group) to get a patch going on this. I'm not a fan of developing a patch before it's clear that the contribution will be accepted but at this point I think a patch would be more productive and put more weight behind our attempts to validate the improvement to core. I agree that a two phase approach would be best where phase one includes the filtering out of CCTs and the ability to set a custom comment status).

As far as approach for custom comment statuses, I think @nacin was right to associate #10856 to this issue since the current comment status column is "comment_approved" and creating a true comment status implementation would be warranted (aside from this enhancement).

Hope this helps!

#46 follow-up: @mordauk
10 years ago

I'll work with you on it @dancameron

#47 in reply to: ↑ 46 @norcross
10 years ago

Replying to mordauk:

I'll work with you on it @dancameron

FWIW I also have a real-world example of this in use right now. A site uses a CPT for companies in a particular business sector, and I used comments to handle user-driven reviews. the comment_type was set to review and works as expected, however, takes a lot of mashing and workarounds as @mordauk has outlined above to get any sort of aggregate data.

#48 follow-up: @mordauk
10 years ago

It would make sense to me if WP core only ever displayed pingbacks, tracbacks, and default comments, then plugins can handle the rest of the display for custom types.

#49 in reply to: ↑ 48 @norcross
10 years ago

Replying to mordauk:

It would make sense to me if WP core only ever displayed pingbacks, tracbacks, and default comments, then plugins can handle the rest of the display for custom types.

agree 100%. sorry I wasn't clear on that, I just wanted to show another instance of comments being used outside of the vanilla "blog post comments" model.

@mordauk
10 years ago

Add comment_type support to get_approved_comments()

@mordauk
10 years ago

#50 @mordauk
10 years ago

12668-2.patch is a first pass at adding some preliminary support. It has two changes:

  1. Adds a $comment_type parameter to get_approved_comments() (an old function not really used any more but there for backwards compatibility).
  1. Adds support for passing an array of comment types to WP_Comment_Query instead of just a single type.

@mordauk
10 years ago

#51 @mordauk
10 years ago

  • Keywords has-patch added

12668-dashboard.patch makes the recent comments widget on the dashboard display default types only.

12668-widgets.patch makes the Recent Comments widget display default types only.

#52 @mordauk
10 years ago

12668-comments-query.patch is an updated version of patch-1,2, and 3 that properly accounts for the fact that default comments don't have an explicit comment type.

I also went ahead and separated each modified file into separate patches. Not sure if that makes it easier or more difficult to review :)

@dancameron
10 years ago

cleaning up previous patch

#53 @dancameron
10 years ago

12668-comments-query.2.patch doesn't do much more than change the formatting of the code that @mordauk submitted, in order to conform to whatever standards are applied to the rest of that query method.

I really want to change the get_approved_comments function, I don't know how to describe my angst towards it. Firstly, it's not even being used by anything in core than comments-popup (which brings back memories); Secondly, I feel that it should be using WP_Comment_Query...I'll create a totally separate patch.

Replying to mordauk:

I also went ahead and separated each modified file into separate patches. Not sure if that makes it easier or more difficult to review :)

In order to keep track it's gonna be a real hassle if the diff/patch files get changed. So let's try to keep them as is (although I wouldn't be apposed to naming the files based off the file names).

#54 follow-up: @boonebgorges
10 years ago

  • Keywords needs-unit-tests added

This thread is long and the scope of the proposed changes is unclear. There are pieces of this that I would be happy to help shepherd through the process - in particular, changes to the WP_Comment_Query API that'll allow for filtering by comment type, and the addition of filters as necessary to allow plugins to manipulate default comment querying behavior, eg on the Recent Comments dashboard widget. That said, now that patches are starting to roll in, it would be nice if they were accompanied by a succinct statement about what just what it is that we're trying to accomplish.

In order to keep track it's gonna be a real hassle if the diff/patch files get changed. So let's try to keep them as is (although I wouldn't be apposed to naming the files based off the file names).

+1

I really want to change the get_approved_comments function, I don't know how to describe my angst towards it. Firstly, it's not even being used by anything in core than comments-popup (which brings back memories); Secondly, I feel that it should be using WP_Comment_Query...I'll create a totally separate patch.

Yes, this. Adding piecemeal support to get_approved_comments() is unappealing. We'll need unit tests for that function before rewriting its internals, though. (Also, in WP_Comment_Query, be sure to array_map( 'esc_sql', $this->query_vars['type'] ).)

@dancameron
10 years ago

Use WP_Comment_Query to improve get_approved_comments since we like caching and all that.

@dancameron
10 years ago

array_map( 'esc_sql', $that ). Also Includes 12668-get_approved_comments-WP_COMMENT_QUERY.patch by default.

#55 follow-up: @mordauk
10 years ago

If I can propose one, this is the scope I'd like to see for this ticket:

  1. Exclude non-default comment types from the list table and all recent comments widgets. Basically, WP core should never show a non-default comment tye
  2. Add support to WP_Comment_Query for retrieving multiple comment types

I think that's it.

#56 in reply to: ↑ 54 @dancameron
10 years ago

Replying to boonebgorges:

There are pieces of this that I would be happy to help shepherd through the process

Thank you!

...it would be nice if they were accompanied by a succinct statement about what just what it is that we're trying to accomplish.

I'll try my best to be as descriptive as possible with my patches.

Yes, this. Adding piecemeal support to get_approved_comments() is unappealing. We'll need unit tests for that function before rewriting its internals, though.

I'd be happy to do this since I've never done WP core unit tests before (forgive me later).

(Also, in WP_Comment_Query, be sure to array_map( 'esc_sql', $this->query_vars['type'] ).)

Done. Thanks, this is something I actually forgot to bring up because I didn't know what callback to use.

#57 in reply to: ↑ 55 @dancameron
10 years ago

Replying to mordauk:

If I can propose one, this is the scope I'd like to see for this ticket:

  1. Exclude non-default comment types from the list table and all recent comments widgets. Basically, WP core should never show a non-default comment tye
  2. Add support to WP_Comment_Query for retrieving multiple comment types

Agreed. I would love to do more but in order to KISS so we can see this in core soon the "more" can come later.

#58 follow-up: @boonebgorges
10 years ago

Thanks for the clarification on scope, mordauk and dancameron. I agree that it's best to start with a modest scope.

The API-level changes proposed in https://core.trac.wordpress.org/attachment/ticket/12668/12668-get_approved_comments-WP_COMMENT_QUERY.patch (plus the esc_sql() later) look reasonable at a glance to me. I'd like to see unit tests both for get_approved_comments() and for array-formatted 'type' param in WP_Comment_Query.

Some thoughts about core and default settings. Current behavior of the Dashboard widget, edit-comments.php, WP_Widget_Recent_Comments is to show all comment types. This is, of course, the very thing we're trying to "fix". But there are probably many cases where the existing behavior is being relied upon by users and by plugins. Changing the defaults for these UIs so that only core comments appear has the potential to confuse. And in the case of edit-comments.php, it's likely that in many cases, removing non-core comments from this display will make it impossible for users to moderate/edit custom comments. Some plugins will add their own interface, or will filter admin_comment_types_dropdown, but many will not. We might want to consider a more conservative route of keeping the current default behavior, but providing more robust filters, so that plugins that add custom comment types can easily exclude them from these interfaces. Do others have thoughts about this?

#59 in reply to: ↑ 58 @mordauk
10 years ago

Replying to boonebgorges:

Some thoughts about core and default settings. Current behavior of the Dashboard widget, edit-comments.php, WP_Widget_Recent_Comments is to show all comment types. This is, of course, the very thing we're trying to "fix". But there are probably many cases where the existing behavior is being relied upon by users and by plugins. Changing the defaults for these UIs so that only core comments appear has the potential to confuse. And in the case of edit-comments.php, it's likely that in many cases, removing non-core comments from this display will make it impossible for users to moderate/edit custom comments. Some plugins will add their own interface, or will filter admin_comment_types_dropdown, but many will not. We might want to consider a more conservative route of keeping the current default behavior, but providing more robust filters, so that plugins that add custom comment types can easily exclude them from these interfaces. Do others have thoughts about this?

For all intensive purposes, "all comment types" is the same as showing pingbacks, trackbacks, and standard comments (no explicit type).

I'd propose we keep it that way by having WP_Comment_Query default to showing just those three types if no type/s is set.

#60 follow-ups: @boonebgorges
10 years ago

For all intensive purposes, "all comment types" is the same as showing pingbacks, trackbacks, and standard comments (no explicit type).

'All Comment Types' sets the type to '', which results in a SQL query that doesn't filter by type at all. On a vanilla install, this is the same thing as pingbacks+trackbacks+comments. But if a plugin is creating comments with a custom type, they also show up on this filter. If we make the default behavior to show only the three core comment types, there'll be no way to see these comments at edit-comments.php.

#61 in reply to: ↑ 60 ; follow-up: @mordauk
10 years ago

Replying to boonebgorges:

For all intensive purposes, "all comment types" is the same as showing pingbacks, trackbacks, and standard comments (no explicit type).

'All Comment Types' sets the type to '', which results in a SQL query that doesn't filter by type at all. On a vanilla install, this is the same thing as pingbacks+trackbacks+comments. But if a plugin is creating comments with a custom type, they also show up on this filter. If we make the default behavior to show only the three core comment types, there'll be no way to see these comments at edit-comments.php.

12668-comments-query.patch addresses that issue.

#62 in reply to: ↑ 61 ; follow-up: @joshlevinson
10 years ago

Replying to mordauk:

12668-comments-query.patch addresses that issue.

I may be wrong, but I don't think that patch (or any so far on this ticket) addresses what boonebgorges is discussing. I think boonebgorges is questioning the first item of the scope you defined. That is, whether it's best to remove all but the default comment types from display/administration or to keep them in comment queries and allow—via filters—the opposite: explicit removal of specific types from the queries.

Replying to boonebgorges:

...there are probably many cases where the existing behavior is being relied upon by users and by plugins. Changing the defaults for these UIs so that only core comments appear has the potential to confuse. ... Do others have thoughts about this?

IMO, users and plugins that do rely on built-in administration/display for custom comment types are relying on an unsupported ability that isn't documented or promoted anywhere. Defining the correct behavior here is new territory, so it may be best to just make a clean break and require anything built on current behavior to conform.

#63 in reply to: ↑ 62 ; follow-up: @mordauk
10 years ago

Replying to joshlevinson:

Replying to mordauk:

12668-comments-query.patch addresses that issue.

I may be wrong, but I don't think that patch (or any so far on this ticket) addresses what boonebgorges is discussing. I think boonebgorges is questioning the first item of the scope you defined. That is, whether it's best to remove all but the default comment types from display/administration or to keep them in comment queries and allow—via filters—the opposite: explicit removal of specific types from the queries.

They do. To remove the non-default comments from the list table, two things have to be done:

  1. The comment type needs to be set to array( 'pingback', 'trackback', 'comment' ) as the default in the prepare_items() method of class-wp-comments-list-table.php. 12668-comments-list-table.patch addresses this.
  1. WP_Comment_Query has to be modified slightly so that it can accept an array of comment types. 12668-comments-query.patch addresses this.

If a plugin wants to change the comment types that are included by default, all it would need to do is modify the comment query args with pre_get_comments.

@dancameron
10 years ago

noticed how I created confusion by removing the check for the comment type.

#64 in reply to: ↑ 63 @dancameron
10 years ago

Sorry guys...I think I know why there's a bit of confusion. I mistakenly removed a very important check for an explicit 'comment' argument (now on line number 458).

12668-comments-query.4.patch brings back the contribution that mordauk had already made in 12668-comments-query.patch with some formatting differences and the sanitization callback of esc_sql.

Also, I mentioned that I would love to do the Unit Tests but I might not get around to it before WCSF so please do not wait for me.

Last edited 10 years ago by dancameron (previous) (diff)

#65 follow-up: @mark-k
10 years ago

If you think that changes to comment_form to handle a comment type as one of its parameters and the comment submitting handling should be part of the scope for this iteration, I can handle that side.

#66 in reply to: ↑ 65 @dancameron
10 years ago

Replying to mark-k:

If you think that changes to comment_form to handle a comment type as one of its parameters and the comment submitting handling should be part of the scope for this iteration, I can handle that side.

At this point I agree with he limited scope mordauk proposed (below) in order to get support into core soon. Working on additional features/enhancements is something I'd like to be involved with but just not at this time (baby steps).

  1. Exclude non-default comment types from the list table and all recent comments widgets. Basically, WP core should never show a non-default comment ty[p]e
  2. Add support to WP_Comment_Query for retrieving multiple comment types

AFAIK these are already done but unit tests need to be made, which I hope to complete today (at #WCSF).

#67 in reply to: ↑ 60 @dancameron
10 years ago

Firstly thank you boonebgorges for taking the time yesterday to focus on this ticket.

What we concluded was that the latest scope from mordauk (although limiting) was far too reaching because it would impose a regressing for those (hypothetical) comment types that are expected within the admin edit screen after 4.1. At this point building support to register comment types (similar to CPTs) is unlikely for an immediate release and a broader discussion should be had.

That said the pain points mordauk, judgej and others brought up should be addressed now.

Here's the scope that I'm working on providing:

  • Add support to WP_Comment_Query for retrieving multiple comment types
    • Add array support for the type
    • Add type__in and merge with type.
  • Add support to exclude custom comment_types from WP_Comment_Query::query
    • By adding type__not_in, pre_get_comments should/can be used to hide comment_types from all comment queries.
  • Rewrite the internals for get_approved_comments to use WP_Comment_Query::query and allow for query args
  • Create new unit tests for comments query pre-patch
  • Create new unit tests for any comment query enhancements

At the moment I have all of these items (except a few new tests) complete. I will wrap everything soon and upload the pre-patch tests separately than the patch(s).

I do hope the need for this new scope is apparent and the benefits are clear.

#68 @mordauk
10 years ago

Sounds like a plan to me.

@dancameron
10 years ago

New tests for comments query and get_approved_comments prior to any patch.

@dancameron
10 years ago

This patch addresses all points of the new scope

@dancameron
10 years ago

21 tests in all. I'm sure there's some variation that I missed but the coverage these tests have is fairly extensive

#69 @jared_smith
10 years ago

  • Keywords needs-unit-tests removed

Removing the "needs-unit-tests" keyword, as this ticket obviously includes unit tests.

#70 @boonebgorges
10 years ago

In 30096:

Better flexibility for 'type' in WP_Comment_Query.

  • Add support for an array of values in 'type'.
  • Introduce type__in parameter. This duplicates 'type' but is added for better consistency with other query classes.
  • Introduce type__not_in.

Among other things, these changes will make it easier for plugin authors to
manage the appearance of custom comment types in various WP interfaces.

Props dancameron, mordauk.
See #12668.

#71 @boonebgorges
10 years ago

In 30098:

Use WP_Comment_Query to query comments in get_approved_comments().

Props dancameron.
See #12668.

#72 @boonebgorges
10 years ago

  • Milestone changed from Awaiting Review to 4.1
  • Resolution set to fixed
  • Status changed from reopened to closed

dancameron - Thanks so much for the updated patch.

These changes should allow plugin authors to work around the fact that we don't currently have a robust method of registering custom comment types. I think we should explore what a register_comment_type() API would look accomplish, but I think it should probably happen somewhere other than in this Trac ticket. If this is something that others are interested in, maybe a proposal could be worked up and shared on, eg, make.wordpress.org/core, and we could have a broader community discussion about goals. (sc0ttclark's github experiments described here https://core.trac.wordpress.org/ticket/12668#comment:21 might be a good technical starting point)

In the meantime, in the interest of having a ticket to close against this milestone, I'm going to mark this ticket as resolved. When there's a more concrete proposal for a full comment type API, let's open a new ticket that references this one.

#73 @mordauk
10 years ago

While a complete comment types API will be great, these changes give us support for fixing some of the original problems brought up as reasons for this ticket, so I'm happy :)

#74 @dancameron
10 years ago

You're very welcome, glad I could help; I learned a lot in the process.

Also, I look forward to the comment type API in the future. For now I'm happy to see our resolution in core.

#75 @sc0ttkclark
10 years ago

+1 for iterative progress, excited to see what comes from here.

#76 @DrewAPicture
10 years ago

In 30109:

Update the changelog for get_approved_comments() to reflect that the function was refactored to leverage WP_Comment_Query in [30098].

Also updates the $args parameter description with a reference to see WP_Comment_Query for argument information.

See #12668.

#77 @DrewAPicture
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#78 @DrewAPicture
10 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 30110:

Update the changelog for WP_Comment_Query::query() to reflect two more new arguments added in [30096].

[30096] introduced the type__in and type__not_in arguments. See #30111 for documentation on those new arguments.

Fixes #12668.

This ticket was mentioned in Slack in #core by dancameron. View the logs.


10 years ago

This ticket was mentioned in Slack in #core by drew. View the logs.


10 years ago

This ticket was mentioned in Slack in #core by desaiuditd. View the logs.


10 years ago

#82 @boonebgorges
10 years ago

#21571 was marked as a duplicate.

This ticket was mentioned in Slack in #core by boone. View the logs.


9 years ago

This ticket was mentioned in Slack in #core by mike. View the logs.


9 years ago

This ticket was mentioned in Slack in #core by rachelbaker. View the logs.


8 years ago

#86 in reply to: ↑ 1 @zithemes
5 years ago

Replying to mikeschinkel:

Interesting idea. If comments were moved to be in the wp_posts table with post_type='comment' then a lot of functionality would come for free?

Note: See TracTickets for help on using tickets.