WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#20045 closed enhancement (fixed)

Delete User should not default to "Delete all posts and links"

Reported by: lachlanj Owned by: GhostToast
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.3.1
Component: Users Keywords: has-patch commit dev-feedback
Focuses: Cc:

Description

When deleting a user in WordPress the radio button defaults to "Delete all posts and links." If a user does not read or understand what they are doing, the results can be a potential loss of all posts.

A better user experience would be to default to "Attribute all posts and links to:" then even if the data is attributed to the wrong user, at least no data is lost if the person does not read the options.

Although this is a minor change the result would save a lot of pain for the end user. An example of where this may happen is when a web developer sets up a website for a client, after handing over the website the client deletes the web developers user account without reading or understanding the options.

Attachments (4)

20045_confirm_delete_user.patch (901 bytes) - added by GhostToast 3 years ago.
Removed 'checked="checked"' from confirm deletion area of users.php
20045-default-to-you.diff (1.4 KB) - added by Ipstenu 3 years ago.
Change checked to 'assign posts to another user' and assign that user to whomever is deleting.
20045_disable_user_delete_submit.patch (1.8 KB) - added by Dan Rivera 3 years ago.
jquery disables confirm deletion button until radio is selected.
20045.diff (3.9 KB) - added by nacin 3 years ago.
Based on the patches from Dan, Ben, and GhostToast.

Download all attachments as: .zip

Change History (19)

comment:1 @lachlanj3 years ago

  • Summary changed from Delete User should default to "Attribute all posts and links to:" to Delete User should not default to "Delete all posts and links"

comment:2 @lachlanj3 years ago

  • Cc lachlanj added

After some discussion it was pointed out to me that defaulting to "Attribute all posts and links to:" and not selecting the correct user would cause some user experience issues as well. ie. potentially assigning many posts to a user that should not have access to edit or even view certain posts.

Perhaps a better option would be to not have either option selected by default so if a user did click "confirm deletion" they would be prompted to choose an option. Then a second confirmation if the users selects to delete all posts with a message that says "clicking ok will permanently delete all posts for this user, are you sure you want to delete?"

comment:3 @dd323 years ago

Having heard of this before, it does make sense to me that there we may need more of a warning on what's about to happen.

Perhaps a message of "This will cause 9 Pages, and 30 posts to be permanently deleted" for the delete term, and default the new-user for the posts to the current user, Either could be left as the default, but making it clear (Perhaps a "Are you sure" JS popup) could be worthwhile to prevent such accidental deletions.

Not having a default selected and forcing the user to choose one or the other is also a potential solution to force the user to read what they're about to do.

comment:4 @GhostToast3 years ago

  • Owner set to GhostToast
  • Status changed from new to reviewing

comment:5 @GhostToast3 years ago

Going to attempt this. Simply removing checked="checked" from default value will not disable the button of course, and page will load without incident. This will at least slow the user down hopefully long enough for them to realize what they are doing. Ideally, I think this combined with the greying out of the "Confirm Deletion" button might work. Otherwise, the "This will delete X posts and Y pages" might be nice, but could potentially be difficult to take into account custom post types.

@GhostToast3 years ago

Removed 'checked="checked"' from confirm deletion area of users.php

comment:6 @GhostToast3 years ago

For now, just removed checked="checked" from the first radio "Delete all posts and links". Still resolves to "user deleted"... continuing to work on feedback instead of false positive.

@Ipstenu3 years ago

Change checked to 'assign posts to another user' and assign that user to whomever is deleting.

comment:7 @Ipstenu3 years ago

Uploaded a different take on it.

I changed the checked to assign it to a different user, and then changed the user to be whomever's actioning the delete.

@Dan Rivera3 years ago

jquery disables confirm deletion button until radio is selected.

comment:8 @Dan Rivera3 years ago

jquery disables confirm deletion button until radio is selected. Collaborated with Ben Brooks and GhostToast at Hack Day san fran 2012

comment:9 @nacin3 years ago

  • Milestone changed from Awaiting Review to 3.5

comment:10 @wonderboymusic3 years ago

  • Keywords has-patch added

@nacin3 years ago

Based on the patches from Dan, Ben, and GhostToast.

comment:11 @nacin3 years ago

  • Keywords commit added

I took this for a spin and ended up with 20045.diff. Simplifies the JS, adds a no-JS "ERROR" fallback.

Also removes redundant current_user_can() calls, adds a space after the "Attribute all posts to:" label, and does better (and earlier) sanitization of passed users.

comment:12 @nacin3 years ago

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

In [22166]:

Force the user to explicitly choose between content deletion and reassignment when deleting users. props Dan Rivera, Ben Brooks, GhostToast. fixes #20045.

comment:13 follow-up: @johnbillion3 years ago

  • Keywords dev-feedback added

Should the same approach be taken when deleting users on the Network Users screen on a Multisite install?

comment:14 in reply to: ↑ 13 ; follow-up: @nacin3 years ago

Replying to johnbillion:

Should the same approach be taken when deleting users on the Network Users screen on a Multisite install?

It's weird because that only works for the current (main) site. Other sites in the network are not touched. I thought we had gotten rid of that form a while ago. New ticket for porting this over and cleaning that up?

comment:15 in reply to: ↑ 14 @SergeyBiryukov2 years ago

Replying to nacin:

New ticket for porting this over and cleaning that up?

Follow-up: #23361

Note: See TracTickets for help on using tickets.