#27952 closed defect (bug) (fixed)
get_blog_details()->post_count Does not subtract deleted posts
Reported by: | GSMcNamara | Owned by: | wonderboymusic |
---|---|---|---|
Milestone: | 4.0 | Priority: | normal |
Severity: | minor | Version: | 3.8.1 |
Component: | Networks and Sites | Keywords: | good-first-bug has-patch commit |
Focuses: | multisite | Cc: |
Description
The number returned for the post_count attribute is correct when posts are created, but does not decrease when posts are deleted.
Attachments (5)
Change History (20)
#1
@
11 years ago
- Keywords needs-patch good-first-bug added
- Milestone changed from Awaiting Review to Future Release
- Severity changed from normal to minor
#5
@
11 years ago
Since publish_post
and delete_post
hooks are called in transition_post_status
, I think it is enough to hook only on transition_post_status
. And comments for the function should be updated.
#6
@
11 years ago
I just ran a test on 3.9.1 and when you force delete a post (and thus skip the trash status) the transition_post_status
hook is NOT fired. I don't believe there is a way to delete without trashing first in the vanilla admin, but you can with a function call like this:
wp_delete_post( 2855, true); //setting $force_delete to true
So we will still want to hook into post_delete to catch these cases I'd think.
Also, the count will only be changing if the status is changing to or from "publish" (or being force deleted). The transition_post_status
hook passes $new_status
, $old_status
, and $post
as params. If we did a test if($new_status == "publish" || $old_status == "publish")
, we could avoid resetting the count (and what could be a decently long query on very large sites) if the status was changing from draft to trash or something like this.
If this makes sense, I can write a new patch that keeps the post_delete hook in there and possibly updated the function to optionally take $new_status
and $old_status
params to test them before updating the count.
UPDATE: Hey, MidxCat, I'm at WordCamp Philly with Nacin and talking about this one. :)
I'm told transition_post_status
will also fire if the status is "chaning" from "publish" to "publish". So we'll handle that case to NOT update the count if the status remains "publish". Working on a patch right now.
#7
@
11 years ago
Okay. Attached a new diff. Changes:
I left the update_posts_count()
function in ms-functions.php
alone, except to change the wording of the comment. It is slightly different from the first patch but I think reflects what this latest patch does.
I added two new functions to ms-blogs.php
mimicking the _update_blog_date_on_post_publish()
and _update_blog_date_on_post_delete()
functions. I wouldn't want to add new functions here if we didn't have to, but we need to do something like this so we can check if the status is really changing. Added these two functions also has the advantages of having clearer code and preserving the update_posts_count() function to simply update the count with no checks if people are using the function that way.
_update_posts_count_on_delete($post_id)
now fires onpost_delete
. If the post being deleted is not in "publish" status, we don't update the count since deleting a draft/etc wouldn't affect the count.
_update_posts_count_on_transition_post_status($new_status, $old_status)
now fires ontransition_post_status
. If the post statuses are the same or neither of them is "publish", we don't update the count since this change shouldn't affect the count.
I tested on my multisite dev and it successfully updated the count when posts were created, deleted, updated, etc, and didn't update the count in the new cases where we don't. This will avoid potentially slow queries on sites with lots of published posts.
If adding so much code for this feels icky, I'd suggest another patch that calls update_posts_count()
on post_delete
and save_post
(might as well use this since we wouldn't be checking the statuses anyway) with an update to the comment RE when update_posts_count()
is called.
#8
follow-up:
↓ 9
@
10 years ago
- Owner set to wonderboymusic
- Resolution set to fixed
- Status changed from new to closed
In 28835:
#10
@
10 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
get_post_field() is going to be a bit slower than get_post( $post_id )->post_status
because get_post_field() runs things through sanitize_post_field().
#11
@
10 years ago
attachment:27952.3.diff switches to using get_post()
instead of get_post_field()
.
Confirmed. The
update_posts_count()
function is only hooked into thepublish_post
action. It should probably be hooked intodelete_post
andtransition_post_status
too in order to catch all relevant changes.