Remove "the hackiest hack that ever did hack" from add_user()

[duck_]

"The hackiest hack that ever did hack" was introduced in [3677] as part of #2624 to "[allow] adding new users of any defined role straight from users.php (i.e. without having to create a user of the default role and the promoting to the desired level)".

This seems unnecessary now as wp_insert_user() does this anyway:

if ( isset($role) )
    $user->set_role($role);
elseif ( !$update )
    $user->set_role(get_option('default_role'));

$role being set in edit_user() from $_POST['role'].

Back at revision 3677 this was a different story:

if ($update && !empty($role)) {
    $user = new WP_User($user_id);
    $user->set_role($role);
}
	
if ( !$update ) {
    $user = new WP_User($user_id);
    $user->set_role(get_settings('default_role'));
}

New users would always have the default role set on insert.

The add-user ajax action isn't actually used any more, but I tested the hack removal by using Chromium's JavaScript console to send AJAX requests.

At the very least the documentation should be updated to reflect the actual reasoning for the code rather than just "seems that [it] is for backwards compatibility".

Issue found during testing

The add-user admin-ajax case doesn't set the current screen so several notices are thrown by the list table code:

Notice: Trying to get property of non-object in wp-admin/includes/class-wp-users-list-table.php on line 17
Notice: Trying to get property of non-object in wp-admin/includes/class-wp-list-table.php on line 89
Notice: Trying to get property of non-object in wp-admin/includes/template.php on line 238
Notice: Trying to get property of non-object in wp-admin/includes/template.php on line 239
Notice: Trying to get property of non-object in wp-admin/includes/template.php on line 239
Notice: Trying to get property of non-object in wp-admin/includes/template.php on line 242
Notice: Trying to get property of non-object in wp-admin/includes/template.php on line 257
Notice: Trying to get property of non-object in wp-admin/includes/class-wp-list-table.php on line 602

See #9891 and [11442] for the change to wp_insert_user().

Mmm... hacky.

How about also deprecating add_user()?

[19686]

Ryan suggested switching to edit_user() exclusively in core, but leaving add_user() as is for now, see patch.

I agree.

Grain of salt, but I prefer ! $user_id = edit_user() to ! ( $user_id = edit_user() ), the latter looks like a situation where you're trying to invert what's inside. The former looks like a deliberate assignment and a check against the value of the assignment.

Replying to nacin:

Grain of salt, but I prefer ! $user_id = edit_user() to ! ( $user_id = edit_user() ), the latter looks like a situation where you're trying to invert what's inside. The former looks like a deliberate assignment and a check against the value of the assignment.

FWIW I was thinking that somebody might read ! $user_id = edit_user() as "$user_id doesn't equal edit_user()", but parens removed and left as is.

[19689]

How about just doing the assignment separately:

$user_id = edit_user();
if ( !$user_id ) {

}

It's fine, but separate assignment is best general.