Make WordPress Core

Opened 13 years ago

Last modified 6 years ago

#19826 assigned defect (bug)

Error behavior for deleting trashed posts is different for Bulk Delete versus Empty Trash

Reported by: jpbellona's profile jpbellona Owned by: piontkowski's profile piontkowski
Milestone: Future Release Priority: normal
Severity: minor Version: 3.1.2
Component: Posts, Post Types Keywords: has-patch, needs-refresh
Focuses: Cc:

Description

Bug testing a custom blog build, but I may have located a bug with core (wp-die) Trash error messages.

Background:
*The blog has different users, with different roles.
*Issue occurs on trash page of a custom post type (post status = trash .../edit.php?post_status=trash&post_type=subject)
*Trash contains posts by both an admin role (who can add/edit/delete all custom posts) and an author role (who can add/edit/delete his/her own posts).
*Logged in as an author.
*Using Capability Manager Plug-in. (Tested bug without plugin too)
*Issue is specific to behavior after normal WP-error is thrown.

Issue:
Same WP error. Two different behaviors.

Different functionality occurs between using the Bulk Actions (Delete Permanently) > "Apply" button and the "Empty Trash" button, after a normal WP error is thrown. In the first case (Bulk Actions > Apply [1a below]), after I return from the error page and refresh the Trash page, all of the selected author pages have been deleted. No admin pages have been deleted.
In the second case ("Empty Trash" [1b below]), after I return from the error page and refresh the Trash page, all of the selected author pages have NOT been deleted. No admin pages have been deleted.

Behavior:
1a. When I am logged in as an author and I highlight posts by both author and admin, and I attempt to delete using Bulk Actions (Delete Per.) > "Apply", I receive the WordPress error "You are not allowed to delete this item." /After I return from the error page and refresh the Trash page, all of the selected author pages have been deleted. No admin pages have been deleted.

1b. When I am logged in as an author and I highlight posts by both author and admin, and I attempt to delete using "Empty Trash" button, I receive the WordPress error "You are not allowed to delete this item." /I return from the error page and refresh the Trash page, all of the selected author pages have NOT been deleted. No admin pages have been deleted.

returning to the Trash page. Tested both scenarios.

  1. I hit back on the browser, and return to the Trash page. I refresh the page.
  2. Move to a different WP admin page, not the Trash page, then return to the Trash page.

Question, because the same text/error is thrown for both buttons, why is the functionality different? Both single functions seem appropriate, however, having both functions exist together instead of picking one behavior seems problematic.

Attachments (1)

19826.diff (6.1 KB) - added by piontkowski 11 years ago.

Download all attachments as: .zip

Change History (13)

#1 follow-up: @nacin
12 years ago

  • Summary changed from wp-die:: Same WP error. Two different behaviors. to Error behavior for deleting trashed posts is different for Bulk Delete versus Empty Trash

Renaming this ticket to reflect what sounds like is the actual bug being reported.

After looking through the code, it appears you are seeing this behavior by pure coincidence. They both use the same code. When you use the checkboxes, PHP receives the post IDs in the order they are on the page. This is frequently going to be newest first. Chances are, you have let's say 5 author posts in a row (5 newest), followed by 5 admin posts (5 oldest). Thus, it deletes all of the posts authored by the author, before stopping at a post authored by an admin.

When you use the Empty Trash button, it does a raw query for all post IDs that are in the trash. This results in deleting the *oldest* posts first. Thus, it hits a post authored by the admin immediately and nothing is deleted.

It would make the most sense to delete all relevant posts, then issue a proper admin notice once we determine that X posts were skipped and could not be deleted. It is not fair to issue a wp_die() when the user is presented with both a checkbox and a 'Delete Permanently' bulk action — they shouldn't need to know that they can't combine them for certain posts.

Likewise, if you try to Empty Trash but you are not allowed to delete certain posts, you should be able to delete all of the ones you can delete. For what it's worth, though, I am not sure why there is an Empty Trash button shown for an author -- authors do not have the edit_others_posts capability. Perhaps you have altered this using the Capability Manager plugin.

#3 @bpetty
12 years ago

  • Cc bpetty added

#4 in reply to: ↑ 1 @helen
11 years ago

  • Keywords needs-patch good-first-bug added; ui-feedback dev-feedback removed

Replying to nacin:

It would make the most sense to delete all relevant posts, then issue a proper admin notice once we determine that X posts were skipped and could not be deleted.

Sounds like a patch is needed that does this.

#5 @nacin
11 years ago

  • Component changed from Trash to Posts, Post Types

#6 @aaronholbrook
11 years ago

Attempted to reproduce this with both regular posts and a custom post type - I found that the author did in fact not have the option to 'Empty Trash' and therefore I was unable to reproduce this bug.

#7 @piontkowski
11 years ago

Could not reproduce on a fresh install since an author cannot empty trash or highlight other's posts by default. Was able to reproduce in 3.1.2 and trunk only after installing Capability Manager Plug-in and granting edit_others_posts capability to Author. After deactivating and deleting the plugin but leaving the modified capability, Author still retains the capability to edit other's posts.

The bug appears when a user can edit but not delete other's posts. Is a patch still needed?

@piontkowski
11 years ago

#8 @piontkowski
11 years ago

  • Keywords has-patch added; needs-patch removed

Ignore my last question. I uploaded a patch to implement the behavior described in comment 1. The code will take a count of how many posts were not able to be deleted and, if applicable, return the counts formatted as an error.

While fixing the behavior when deleting a post, I noticed that the behavior is the same for trashing and restoring posts. Same, as in, the code issues a wp_die() when it comes across the first post that is not able to be trashed or restored by the user. So I also modified the trash and restore behavior too.

Tested on regular posts and custom post types.

#9 @nacin
11 years ago

  • Owner set to piontkowski
  • Status changed from new to assigned

Thanks piontkowski! I'll take a look at this.

#10 @ilonaF
10 years ago

  • Keywords close added

We could not reproduce the bug on a fresh install since an author cannot empty trash or highlight other's posts by default.
The bug probably appears only after installing Capability Manager Plug-in and granting edit_others_posts capability to Author.

Should we close this bug as invalid? Or treat piontkowski's patch as enhancement?

#11 @wonderboymusic
9 years ago

  • Keywords needs-refresh added; close removed
  • Milestone changed from Awaiting Review to Future Release

This probably needs a refresh

#12 @flixos90
6 years ago

  • Keywords good-first-bug removed
Note: See TracTickets for help on using tickets.