WordPress.org

Make WordPress Core

Opened 10 years ago

Closed 7 years ago

Last modified 7 years ago

#16603 closed enhancement (fixed)

Add hooks to wp_count_posts()

Reported by: mikeschinkel Owned by: wonderboymusic
Milestone: 3.7 Priority: normal
Severity: normal Version:
Component: Query Keywords: has-patch 3.6-early
Focuses: Cc:

Description

http://mikeschinkel.com/websnaps/Posts_%E2%80%B9_Watermark_Associates_Newsletter_%E2%80%94_WordPress-20110220-142441.png

The use-case where this is needed is when the sites is using roles & capabilities to limit access to viewing posts to only those who have the proper capabilities to see them.

For example, assume we have a system with a "Manager" role and a taxonomy called "Post Visibility" where terms are "Visible to All" and "Visible to Managers Only." We add a 'see_managers_posts' capability to "Manager." We then filter outs posts with the "Visible to Managers Only" term using the 'posts_where' and 'posts_join' hooks when viewed by users whose role does not have the 'see_managers_posts' capability.

With that configuration now assume for example we have 10 posts with 3 of them "Visible to Managers Only." The post list in /wp-admin/edit.php will use wp_count_posts() to show that we have 10 posts when we can only see 7 of them. Currently to fix it so the post counts display 7 for non-managers we have to hook the 'query' hook, which is a hook of last resort.

So, the attached patch adds two hooks to wp_count_posts():

  • 'wp_count_posts_sql' - To allow modifications of the SQL used to count posts, and
  • 'wp_count_posts' - To allow modifications of the array returned by the function.

I decided to add two (2) hooks because not having the former would mean we'd need to make two SQL queries if we need to modify the counts, and not having the latter would mean that we'd have the complexity of modifying SQL in use-cases where the a SQL query modification is not what is needed to to determine the proper counts.

Attachments (5)

wp_count_posts_hooks.diff (1.7 KB) - added by mikeschinkel 10 years ago.
Hooks for wp_post_counts()
count-posts_3.2.patch (641 bytes) - added by kevinB 10 years ago.
filter wp_count_posts() return value
16603-wp_count_posts-filter.diff (474 bytes) - added by hardy101 8 years ago.
Filter wp_count_posts before populating the cache
16603.diff (1.4 KB) - added by nacin 8 years ago.
Something like this.
16603.2.diff (2.6 KB) - added by DrewAPicture 7 years ago.
filter docs

Download all attachments as: .zip

Change History (24)

@mikeschinkel
10 years ago

Hooks for wp_post_counts()

#1 @scribu
10 years ago

How about just making that portion of the admin screen hookable instead?

#2 follow-up: @scribu
10 years ago

Actually, it's already possible to overwrite that method by extending the WP_Posts_List_Table class, but alas those classes aren't public as of WP 3.1.

#3 in reply to: ↑ 2 @mikeschinkel
10 years ago

Replying to scribu:

How about just making that portion of the admin screen hookable instead?

Isn't that more extreme? Why not just the simple hooks proposed? The hooks add no perceptible overhead. Why force someone to recreate the entire wp_count_posts() function?

Replying to scribu:

Actually, it's already possible to overwrite that method by extending the WP_Posts_List_Table class, but alas those classes aren't public as of WP 3.1.

Extending a class is not composable with other plugins; only one plugin can extend WP_Posts_List_Table so extending classes really should be left to use within core and not by plugins.

#4 @kevinB
10 years ago

  • Cc kevin@… added

+1 for API consistency.

The appeal here is bring the wp_count_posts() API into line with filtering that is already supported for the corresponding paged result set that those counts are supposed to summarize. Without this, plugins/themes that use the WP_Query hooks to filter editing access (on any basis) must choose between an amateurish user experience (counts unfiltered / jQuery-filtered) or "hacky" code (counts filtered via the 'query' hook).

I'm submitting an alternate patch against 3.2-bleeding which also applies the filter on the cached return value. Without that, filtering is bypassed if persistent caching is implemented via object-cache.php. With it, persistent caching eliminates the redundant query.

I'm all for filtering the count query itself but left it out of this patch in case the results filter is an easier sell, and due to implications with persistent caching. By the way, I don't see where the 'counts' cache is ever cleared on post save.

@kevinB
10 years ago

filter wp_count_posts() return value

#5 @mikeschinkel
10 years ago

@kevinB - Thanks for the improved patch.

@hardy101
8 years ago

Filter wp_count_posts before populating the cache

#6 @hardy101
8 years ago

+1 for this filter. I do a lot of post filtering based on permissions, and these status counts are always out of sync. My attached patch (16603-wp_count_posts-filter.diff) is a one-liner: If we filter the array of counts before it populates the cache, we should get that filtered value back (as long as our plugin fires before anyone else caches the count).

#7 @scribu
8 years ago

  • Keywords dev-feedback removed
  • Milestone changed from Awaiting Review to 3.5

Since WP_List_Tables don't seem to be getting extensible any time soon, let's do this.

#8 follow-up: @nacin
8 years ago

Filters should occur outside caching. (Otherwise we end up #7213 and [8225] which results in #21267, not to mention many other side effects.)

count-posts_3.2.patch looks good. I'd suggest we rename a bunch of variables in that function so both filters receive $counts, rather than the dubious $count and $stats variables, and it is clear what is going on.

If there is truly a desire to alter the query directly, then a pre_* filter should be added. I don't think that is particularly necessary.

A side note, the counts cache bucket is non-persistent, which means a persistent cache won't eliminate the query. The counts cache bucket is used for things that are annoying to invalidate or would be invalidated constantly, but is still good to hold onto for the pageload.

Also, unrelated, that wp_cache_set() should technically be a wp_cache_add().

@nacin
8 years ago

Something like this.

#9 @nacin
8 years ago

  • Keywords commit added

Tossed up a patch.

#10 @nacin
8 years ago

  • Keywords 3.6-early added
  • Milestone changed from 3.5 to Future Release

#11 @Offereins
8 years ago

  • Cc lmoffereins@… added

+1

#12 @itthinx
8 years ago

I was just looking at wp_count_posts() in 3.5.1 looking for filters that unfortunately don't exist. What has been proposed here, seems to be in line with what I was looking for. By the way, wp_count_attachments() seems to be lacking those filters as well.

A way to modify the outcome of this function is important when you are restricting the visibility of posts - my Groups plugin allows to do that and when a user is viewing the list of posts in the back end, the counts reflect the total number of posts that exist, but if that user is only allowed to see a subset of these, the number of posts that is in the list is less. Moreover, the small indicator on top right would say 'X items' where X is the correct number of posts for that user, while the 'Published (Y)' indicator would give a higher number in Y.

Seeing that this ticket has been open since 2 years and there's no (reasonably efficient) way to interact with the outcome of wp_count_posts() and wp_count_attachments(), here's one more who would appreciate a solution to this. I haven't tried the patches but from what has been said here, more than just +1.

#13 in reply to: ↑ 8 @creativeinfusion
7 years ago

  • Cc wptrac@… added

Replying to nacin:

Filters should occur outside caching.

There's a filter on wp_count_comments that has a similar function to this ticket and it's applied before the cache is used at all. This does work when wanting to modify the query, though a direct filter on the query being used would be better as it would avoid replicating the function.

If there is truly a desire to alter the query directly, then a pre_* filter should be added. I don't think that is particularly necessary.

I have sites where we only allow some admin users to see their own posts (custom post types typically) which needs a post_author to be added to the query.

At present we use the 'views_edit-{post_type}' filter with a custom count posts function that replicates wp_count_posts except for the query change (I really didn't want to use the 'query' hook). It's far from ideal not least because if this is wanted on regular posts the counts in other places (e.g. the right_now dashboard widget) are of course off.

#14 @wonderboymusic
7 years ago

  • Milestone changed from Future Release to 3.7

Patch from @nacin still applies cleanly

#15 @mordauk
7 years ago

  • Cc pippin@… added

#16 @wonderboymusic
7 years ago

  • Keywords needs-docs added; commit removed

There's a new filter

@DrewAPicture
7 years ago

filter docs

#17 @DrewAPicture
7 years ago

  • Keywords needs-docs removed

16603.2.diff adds a docblock for the count_posts filter hook.

#18 @wonderboymusic
7 years ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

In 25554:

Add hooks to wp_count_posts(). Adds filter docs. Adds unit test to test count_posts filter.

Props nacin, DrewAPicture.
Fixes #16603.

#19 @nacin
7 years ago

In 25578:

In wp_count_posts(), rename 'count_posts' hook to 'wp_count_posts', for clarity. see #16603.

Note: See TracTickets for help on using tickets.