Misleading message when deleting a user with no sites

[danielbachhuber]

In multisite, when deleting a user with no sites, the following message appears:

Transfer or delete posts and links before deleting users.

This is a confusing message because there are no posts or links to transfer/delete. The message should be changed to something else.

[brianlayman]

Improved wording for the delete confirmation screen

[brianlayman]

Added possible wording change. The new wording explains what the screen does and states that the deletion cannot be undone.

[chriscct7]

Refresh to remove link references

[jeremyfelt]

The proposed text change in the patch is to:

You have chosen to delete the following user(s) from all networks and sites.
Once you hit "Confirm Delete", the user(s) will be permanently removed. There is no undo.

I think this is close. We can probably scratch "There is no undo." and use _n() for user(s).

[HarishChaudhari]

Refreshed user deletion message for multisite

[HarishChaudhari]

Refreshed user deletion message, included _n() as suggested by @jeremyfelt.
"following" word is not used in case of single user as there is no username listed below that.

[SergeyBiryukov]

A couple of issues with 18132_refreshed.diff​:

• We can't have 'user' and 'following users' as separate strings injected into other strings. Some languages require the whole string to be translated differently in each case.
• _n() is not actually an ideal solution here. In some languages, the first form ("the user") can be used for 21, 31, etc., but it doesn't necessarily mean it's a singular form (it needs to be translated differently when dealing with 1 user vs. 21 users). See comment:5:ticket:28502.

We should just check if count( $users ) > 1 and display separate strings for each case.

[Idealien]

check if count( $users ) > 1 and display separate strings for each case.

[Idealien]

Replying to SergeyBiryukov:

We should just check if count( $users ) > 1 and display separate strings for each case.

Changes incorporated into 18132_checkCount.diff and tested on a fresh Network installation.

Ready for has-patch and milestone 4.2 perhaps?

[jeremyfelt]

Great changes so far. It looks like now that we're doing a better job with the text, we need to change some of the other output a bit to match the experience. We don't need to be too worried about style, this screen is very utilitarian already, but we can provide some of the info already available to us.

A few screenshots with 18132_checkCount.diff applied:

• Deleting a single user belonging to no sites - ​https://cloudup.com/cZh_-bDViFb
• Deleting a single user belonging to one site - ​https://cloudup.com/cFqovmePV-j
• Deleting two users, one belonging to a site - ​https://cloudup.com/ctQpM4fPBVr
• Deleting two users, neither belonging to a site - ​https://cloudup.com/c9nTL-eSEg5

I think we can maneuver some HTML around and split up those strings a bit to provide a structure that's more readable.

You have chosen to delete the following users from all networks and sites.

   - username1
   - username2

Once you hit "Confirm Deletion", these users will be permanently removed.

and

You have chosen to delete the following users from all networks and sites.

    - username1
    - username2

        What should be done with content owned by username2?

            * Site: Site Two
                Delete all content.
                Attribute all content to: ----
             * Site: Site Three
                Delete all content.
                Attribute all content to: ----

Once you hit "Confirm Deletion", these users will be permanently removed.

[Idealien]

Extends 18132_checkCount.diff to include changes @jeremyfelt recommended

[Idealien]

Replying to jeremyfelt:

Great changes so far. I think we can maneuver some HTML around and split up those strings a bit to provide a structure that's more readable.

Screenshot of changes in 18132_listUsers.diff
​https://cloudup.com/cNktPCjCQpq

I agree in general it is both utilitarian and a little better. A couple of questions that come to mind:

• I setup the users as a standard unordered list. Is there a better markup to be using for their display? I can't recall another scenario atm in admin where there is such a list. Even bulk moving pages to trash just gives "2 pages moved to the Trash." + undo.

• If admin is deleting more than a few users, this setup pushes the actionable parts of form + the Confirm Delete button well down the page. That okay?

[jeremyfelt]

@Idealien - This is looking really good, thanks! I'm going to think about the markup a bit and test with a long list of users, but we're almost there. Depending on how the list looks with ~20 users, we may want to merge the two displays instead of having two user lists.

[Idealien]

Replying to jeremyfelt:

@Idealien - This is looking really good, thanks! I'm going to think about the markup a bit and test with a long list of users, but we're almost there.

Thx. This is my first real core code effort and enjoying the right sized scope for it.

Do either of the two options mocked up below feel like a better approach?

A) Inline User Notice​https://cloudup.com/cohAeZh7SSf

• Removes the extra loop at top listing all users set for deletion at the top.
• Display a line identifying each user that has no content in the existing set with a message indicating they will be deleted.

From code perspective it'd be adding an else condition to

$blogs = get_blogs_of_user( $user_id, true );
if ( !empty( $blogs ) ) {
   //current per user output
}

B) Isolated Cases at the Bottom​https://cloudup.com/c4A59cpujjQ
During the main loop of this function store the list of user_ids that have no blogs as array and display them at the bottom indicating they will also be deleted.

Both involve another string for translation and seem like similar amount of code. My $0.02 would vote for A) because it keeps the list alphabatized and consistent experience. But I also don't use Network form of WP so would value others opinions more than my own on this.

[jeremyfelt]

Replying to Idealien:

My $0.02 would vote for A) because it keeps the list alphabatized and consistent experience.

Exactly my thinking, let's go with A. It may also be good to add some margin to the site listings under each user to help separate the sections.

[Idealien]

Replying to jeremyfelt:

Exactly my thinking, let's go with A. It may also be good to add some margin to the site listings under each user to help separate the sections.

Implementation of A) is uploading in 18132_FormTable.diff now. See ​https://cloudup.com/c5xlPhHVezn for screenshot of it in action. A few comments:

• I could not find any existing CSS that just provided indent margin/padding so I used table with class="form-table" that seems to be common through other parts of admin. Really makes the username in th of each row stand-out!

• My draft of new message for scenario when get_blogs_of_user returns no results: "User has no sites or content and will be deleted". Reasonable? Got Better?

• I wasn't clear if the hidden field with user_id was needed in this non-action case but included it for consistency.

[Idealien]

Adds formTable style + "non-action" note per row / user

[DrewAPicture]

Here's a couple of things with 18132_formTable.diff​:

• Just before the first if/else on the $users count, we should probably adjust to also checking whether $users is empty in addition to not being an array. Otherwise, the "not greater than 1" condition could theoretically spit out a string for a zero $users count there and further down
• $current_user->ID in the option value should be escaped with esc_attr()
• !empty( $blogs ) should be ! empty( $blogs )
• "User has no sites or content and will be deleted" should end with a period
• The double quotes in the final two strings should instead use “ and ” respectively

[Idealien]

Screens from test with 18132_DrewReview.diff applied

[Idealien]

Thanks @DrewAPicture. Updates applied in latest .diff.

I think the text in multiple delete scenario reads well-enough. Deleting a single user reads a little bit like a bad episode of The Brady Bunch to me ("Marsha, Marsha, Marsha! User, User, USER!"). The only suggestion I have atm is to remove the first line "You have chosen to delete the user from all networks and sites." in that else case.

I trust whoever is to commit to make the call on that.

[DrewAPicture]

Minor cleanup

[DrewAPicture]

Did some minor cleanup in 18132.2.diff​. Looks good to go.

[jeremyfelt]

Really close on this, but I'd like to spend a few more minutes looking at the formatting that the table layout gives us before committing. This is great work @idealien, we'll definitely get it in for 4.2.

[jeremyfelt]

In 31656:

Improve experience when deleting users from a multisite network.

When deleting a user who is not associated with any sites, the current messaging can be confusing as only users associated with at least one site actually appear on the confirmation page for deletion.

This experience can be improved by showing all users being deleted as well as their current site associations.

• If an empty array of users is passed, don't attempt to confirm deletion.
• If one user is passed, show a message crafted for a user of one.
• If multiple users are passed, show a message crafted for many.
• Show the pending results of all users to be deleted.
• Update messaging around the deletion/confirmation process to be less misleading.

Props Idealien, HarishChaudhari, DrewAPicture.

Fixes #18132.

[TobiasBg]

Should the last if-conditional in [31656] use _n() instead?