Comment queries should ignore comments associated with non-active custom post types

[Clorith]

As of 4.4 we introduced the _doing_it_wrong() (r34091) when checking meta capabilities on custom post types that aren't registered.

This also spread and affected comments, primarily on the dashboard where we show comments and try to add links for these and check the users capability against them, they will produce this wonderful output:

PHP Notice: map_meta_cap was called <strong>incorrectly</strong>. The post type shop_order is not registered, so it may not be reliable to check the capability "edit_post" against a post of that type. Please see <a href="​https://codex.wordpress.org/Debugging_in_WordPress">Debugging in WordPress</a> for more information. (This message was added in version 4.4.0.) in /wordpress/wp-includes/functions.php on line 3827

Ideally, WP_Comment_Query should bypass comments associated to non-existing post types as well.

In the attached patch I've introduced both post_type__in and post_type__not_in which accepts an array of post type string names.

I've also added the default value for post_type__in to get_post_types(), as by default you'd never want to query for non-existing data any way, but this allows the query to be overwritten via filters, or directly with a new construct for those who have the need.

I am wondering if the use of post_type = 'any' would need to short-circuit the new arguments to avoid breaking BC (current tests all pass with the patch applied though) ?

Related #16956

What this looks like (from #34918):

It also shows up on the Dashboard in the comments widget:

[SergeyBiryukov]

Related: #34918, #36058.

[rachelbaker]

I wonder if a more straight-forward approach would be to add support for post_type => active? Downside would be there would be a conflict with any post_types named active.

[Clorith]

I wanted to avoid any potential collision from the use of keywords, and instead use a format that is more in line with, and subconsciously expected, across the other query classes.

[rachelbaker]

Suggestion from @boonebgorges ​via Slack:

Or distinguish between all (whitelist) and any (ignored)

[rachelbaker]

post_type or post_status = any should match WP_Query behavior

[rachelbaker]

In 36208.diff​ I am matching the behavior of the any value for the post_type and post_status arguments to the WP_Query class, which only includes valid types/statuses that are not excluded from search.

Needs more unit tests and testing.

[rachelbaker]

This issue isn't as straightforward as I first thought. My approach in 36208.diff​ won't work as written.

Comments don't have to be related to posts. Defaulting to only valid post_types or valid post_statuses would actually exclude comments with a comment_post_ID of 0 which we don't want to do.

We need to somehow limit hiding comments from related posts if the comment_post_ID > 0 AND the post_type of the related post is registered.

[Clorith]

What if we explicitly look for the any keyword to trigger the addition of 36208.diff, and keep the empty check as it currently is with just a simple continue (so essentially spit up the if or into separate if statements on line 786).

That would avoid us breaking any existing queries that use the comment_post_ID as 0, yet give the enhancements we'd like, or am I missing something deeper perhaps?

[chriscct7]

In #34918, a exact duplicate, there was discussion that followed a different approach than the one on this ticket. Therefore, while closing #34918 as duplicate of this ticket, I've updated this ticket's description with some information the other had, and will quickly summarize the patches + discussions of the other ticket, as it took a different approach to solving the issue.

It was pointed out that hiding the comments of post types that don't exist/inactive would make them inpossible to edit or remove. It was then noted that while this was likely by design, WordPress does not show posts from unregistered post types, and thus this served as an argument to hide comments that are orphaned.

The ticket consensus then agreed that it made sense to hide posts if the post type they are connected to is "gone", particularly from a consistency standpoint.

Component maintainer, @boonebgorges agreed to this approach (https://core.trac.wordpress.org/ticket/34918#comment:10), and then a patch was produced (attaching it below) that hides the errors in the comments list table by checking to see if the post type object exists. This patch however, did not address the implications of a plugin using a wp comment query or the dashboard widget.

Notifications from old ticket: @Funkatronic, @jdgrimes, @lukecavanagh, @nekojira, @obrienlabs, @wesm87

[chriscct7]

#34918 was marked as a duplicate.

[chriscct7]

Patch from #34918

[kal123]

See this issue on 5.1 today:

[11-Mar-2019 00:16:54 UTC] PHP Notice: map_meta_cap was called <strong>incorrectly</strong>. The post type tribe-ea-record is not registered, so it may not be reliable to check the capability "read_post" against a post of that type. Please see <a href="​https://codex.wordpress.org/Debugging_in_WordPress">Debugging in WordPress</a> for more information. (This message was added in version 4.4.0.) in [wordpress path]/wp-includes/functions.php on line 4667

I have removed the tribe plugins, and still see this message 98 times in the debug log.
Happens every time I load the page. About about 20 times per page load.

First, print it ONCE, do NOT fill up the log, or the entire screen. Rate Limit all your messages.

[kal123]

I have 100 entries from 09 26 2018. Why are these messages been printed for old data in the database, even after the plugin removal using wp cli ?
These kind of messages should not be printed by just searching the database. If the plugin in no longer installed, it should stop.

Have a way to acknowledge the message, and disable further log spam.

[SergeyBiryukov]

Related: #47652

[jjazzbel]

I am having this problem as well. it really seems to be dragging our site down to the point where it is barely responsive. Any suggestions would be welcome.

[SergeyBiryukov]

#49266 was marked as a duplicate.

[donmhico]

36208.4.diff - Cleaned up the patch a little.

[ocean90]

#49437 was marked as a duplicate.

[ocean90]

#50150 was marked as a duplicate.

Fixing Comment queries should ignore comments associated with non-active custom post types for wp_dashboard_recent_comments function

Trac ticket: https://core.trac.wordpress.org/ticket/36208

[techjewel]

I have made a pull request regarding this issue. This PR will solve the PHP notice

PHP Notice: map_meta_cap was called <strong>incorrectly</strong>. The post type shop_order is not registered, so it may not be reliable to check the capability "edit_post" against a post of that type.

Now it will pass the post_type array that is only registered to WordPress. I am open to discussion to resolve this issue as it's annoying notice when we have custom post type registered before.

[helgatheviking]

What can we do to finally get this fixed? @techjewel's [PR](​https://github.com/WordPress/wordpress-develop/pull/3045) looks promising to me, though it could also be argued that the issue traces back to get_comments() which should perhaps only return comments attached to active post types. However, easy to see how that would have much broader implications than the suggested PR.

[techjewel]

Yes, It's really frustrating bug. As a plugin developer I installs lots of plugins and those plugin create different posts (cpts) and once I deactivate any I get PHP warning all the time and my debug log file get messy. If anyone have any better idea, we should implement that or a reviewer can review my pull request and merge.

[techjewel]

Unit Tests for Ticket #36208

[techjewel]

Just added Unit Tests for this ticket.

Thanks to @imath for helping about this test.

I have a PR from the last few months here: ​https://github.com/WordPress/wordpress-develop/pull/3045

Looking forward to seeing this issue resolved.

[desrosj]

Readding a milestone to help surface this better in reports.

[techjewel]

From Slack Conversation:

This one will not be considered for 6.1.1, and at the surface, this seems like something that may be best suited for a major release (6.2 being the next one)

[helgatheviking]

Not clear to me why a 7+ year old bug needs to wait for a major release... when a patch is present.

[imath]

Alternative approach

[imath]

Hi @techjewel you're very welcome 👍.

If your PR is the selected fix, I'll be very happy.

I've added an alternative approach a bit similar to the one @chriscct7 shared above.

If removing the "unregistered post type" comments from the comments query is pretty straightforward, imho it's looking like a developer's way of dealing with the problem. It's probably a very efficient way (simple is beautiful) but, even if I may overthink this issue, I'm not feeling comfortable about "hiding" some information/content from the site's administrator because it simply generates doing it wrong notices.

I believe an admin should be able to at least remove these comments or keep them and eventually reactivate the plugin registering the post type.

I hope this will be solved soon 👋

Quick check! How many more years to go before we get this fixed?