provide commands for dealing with users

This sounds pretty good. I'm just worried about the command listing getting unmanageable. Feedback welcome both about how to handle huge command listing and about the code here.

I'd love to see an option to set a user's password added. I know it's reasonably easy to do via SQL but it seems like a natural fit for this command set.

I love this submission and think it's a great idea. I wonder, though, if this code should use user_save rather than making SQL calls directly. The advantage of this is that all of the built-in Drupal special handling would also be done; hook_user would be called, deleted users would be kicked off the system, etc.

@acrollet: You rock. I reviewed your patch to quickly; this is great as it is.

+++ commands/user/user.drush.inc	2009-12-17 15:49:13.361762264 -0600
@@ -0,0 +1,401 @@
+      'drush info 2,3,someguy,somegal,billgates@microsoft.com' => 

mixing uid and name and email is great. i guess it is not 100% reliable and thats why we disallow for other commands. thats reasonable for now. an alternative is to ask the user to disambuate use the same pattern as vset and clear cache commands (in HEAD).

+++ commands/user/user.drush.inc	2009-12-17 15:49:13.361762264 -0600
@@ -0,0 +1,401 @@
+  $items['user create'] = array(

perhaps too ambitious, but it would be great to be able to populate any fields that are attached to the user entity.

+++ commands/user/user.drush.inc	2009-12-17 15:49:13.361762264 -0600
@@ -0,0 +1,401 @@
+  $items['user cancel'] = array(

How hard would it be to support the other cancel modules like 'delete all content by this user? I think thats the most common, because of spam.

+++ commands/user/user.drush.inc	2009-12-17 15:49:13.361762264 -0600
@@ -0,0 +1,401 @@
+      user_multiple_role_edit($uids, 'remove_role', $rid);

Is this available for D5/D6/D7?

+++ commands/user/user.drush.inc	2009-12-17 15:49:13.361762264 -0600
@@ -0,0 +1,401 @@
+  if (empty($mail)) {
+    drush_die("You must specify an email address!");
+  }
+  elseif (empty($pass)) {
+    drush_die("You must specify a password!");
+  }

Don't use drush_die() - use drush_set_error() and bail. But in this case, I'd just as soon not check these conditions. Drupal will happily save an account without an email address. Not sure about password. If it won't (doubtful), lets create one. Developers often just need accounts, even if they are bare.

+++ commands/user/user.drush.inc	2009-12-17 15:49:13.361762264 -0600
@@ -0,0 +1,401 @@
+function _drush_user_print_info($uid) {

This is an OK presentation, but consider how much better the dev/load tab is on a user. I think we should do drush_print_r($userinfo);

+++ commands/user/user.drush.inc	2009-12-17 15:49:13.361762264 -0600
@@ -0,0 +1,401 @@
+  if (count($uids) == 0) {
+    return FALSE;
+  }
+  else {
+    return $uids;
+  }

Lets do

return empty($uids)? FALSE : $uids;

My wishlists:

Add a user list command. This isn't terribly important, as I can always use sqlq "select name from users;". I think it would be a nice as a convenience function, though.

Add a function to add permissions to roles. The downside here is that there is no Drupal API to do this; you'd have to use SQL queries directly, so again, this feature would be only a convenience function, as sqlq works nearly as well.

@moshe: $userinfo is an object, so drush_print_r($userinfo) produces some ugly output. I would recommend this presentation:

function _drush_user_print_info($uid) {
  $info = user_load(array('uid' => $uid));
  $userinfo = (array)$info;
  unset($userinfo['data']);
  unset($userinfo['block']);
  unset($userinfo['form_build_id']);

  var_export($userinfo);
}

Optionally, drush_print_r instead of var_export, and optionally keep $userinfo['block'], but overall, a little bit of clean-up makes the whole thing easier to read.

For the user list command: drush user list would list the name of all of the users on the system, per sqlq "select name from users;" (minus the initial 'name' column). The user could use 'grep' to filter or 'more' to paginate. The purpose of the command would be discoverability; drush help user list would say 'identical to drush sql query "select name from users;".' Rather than add more features for filtering and such, the user could switch to sqlq if a 'where' clause was needed. I think this command would mostly be used in test sites with a small number of users. The bash "alias" command could also be used to provide this functionality, but again, the advantage of building it in to drush is that it is more discoverable.

My logic for the add permissions to roles features is pretty much the same. I like to script basic site setup operations, and setting up basic permissions is a useful part of module setup. This might be better done with features, which I really haven't started using yet. An add permissions command would then naturally beg for a list / add / remove role command. The main reason for adding these would be completeness.

My use case for editing permissions is as follows. I have a drupal-backed static html site. On the edit site, the "anonymous user" role has no permissions. When the site is published, I copy all of the permissions from a template role over the anonymous user role to grant the anonymous user the right to read content on the live site. The script is like this:

customer_perms=`drush edit.dickensfair.com sqlq "select perm from permission where rid=5;" | grep -v '^perm'`
drush dickensfair.com sqlq "insert into permission (rid,perm) values ('1', '$customer_perms');"

Sqlq does work just fine, so this is an optional feature. It would be nice, though. It would be even nicer to just add a role to the anonymous user with the existing "add role" command; unfortunately, the anonymous user is a role, not a user in Drupal, so that won't work.

I have also briefly contemplated user migration. I sometimes migrate all users wholesale like this:

drush --tables-list="users,role,users_roles,permission,fckeditor_role" sql sync live dev

If the user info command would output a cvs list, you might be able to send it to node_import to do individual user migration. I don't have a use case for this yet; it's just a thought.

I think we can skip the user list and permission editing for now.

I really want disambiguation on the write commands in addition to just info command. Thats how we do it for the vset command, for example. Its just so handy. We can add something like the always-set option which was just added to vset command.

By default, drush commands run after a full drupal bootstrap. But we can skip this request for now - I think profile fields will open a small can of worms.

I really want the cancel methods for d7. if we can support them in d6 using user_delete module, thats icing on the cake. we could consider shipping that code with user_delete module.

drush_choice() is quite new code. it's quite possible that less than a dozen people have seen it. i welcome improvements there.

'user info' will need to change to 'user-info' if #550522: Change white spaces in commands by another symbol lands first. I don't want to have to redo that patch if it can be avoided, because it was a bit of a pain to roll; this issue won't cause any problems there, though, (since it's all in its own new file) so I'm fine with either commit order, & will even fix this one up if it commits first.

The code looks pretty good on quick inspection, but unfortunately, my home system died this morning (motherboard went, maybe), and at the moment I'm on a Windows laptop that doesn't even have cygwin installed. :p I'll give this a spin at work if I have a chance today; this patch will be useful to us.

Yes, that's correct. I tried to roll a patch to change $command['command'] on the fly, but doing that in drush help dispatching was too complicated -- it turned out to not be worth the effort. So, all commands will need to be changed (spaces to dashes) by hand.

Here is a patch that makes two modification:

1. The quoting of the SQL statements is adjusted in several places for Postgres compatibility (please try on mysql to make sure I didn't break anything)
2. The call to drush_choice is updated to use the patch I posted in comment #3 of #674940: drush_choice() uses array_unshift(), which re-indexes numerical keys.

This patch therefore does not work correctly unless you first apply the patch to drush_choice mentioned above.

Nah, there are tons of modules that work with both. Sorry I messed up the syntax for mysql -- give me some time and I'll pull up something that works both places. Actually, I think this might be a D7-specific problem; perhaps I shouldn't have added the quotes on the D7 code. I'll have to install D7 to try it; might not get to that for a couple of days.

Here is a patch for experimentation. I've updated the call to drush_choice to match the latest patch over there, and rolled out all of the D7 changes I made. I haven't tested on D7 yet, nor have I tested on D6 + mysql, but this works great on D6 + postgres (and I presume D7 + mysql), provided that you also take patch #6 from #674940: drush_choice() uses array_unshift(), which re-indexes numerical keys. Give it a try if you have a chance. I will test it myself on D7 & mysql when I can, but I still haven't recovered my home system with the dead motherboard yet. (At least I've got the internet gateway back up, using my son's computer, and I'm on a proper Linux laptop with Drupal installed now... ;) ).

most recent patches are some sort of incremental patches. lets post full patches if possible (i.e. the whole user.drush.inc)

Edit: This patch is broken, don't use it...

1. I think we can omit 'drupal dependencies'. user is a required module.
2. Command names should replace spaces with hyphens since #550522: Change white spaces in commands by another symbol has landed.

3. 'options' => array(
   +      'delete-content' => 'Delete all content created by the user (d7 only)',
   +    ),
   

   Lets wrap this option in a conditional which checks: drush_drupal_major_version() >= 7

4. drush_get_option('delete-content')) == 'true'. i think we can just check for drush_get_option('delete-content')). It will be TRUE if present. If we think thats too dangerous, we could add a drush_confirm().
5. I really like the drush_choice() flow in user_info command. I would like to see that available for all user commands. Its a little onerous to pass in --mail=foo@example.com instead of foo@example.com. This sort of simplicity makes drush fans weep with joy. Its magic (in a good way).
6. Perhaps parse_args and its error handling should move into drush_user_init() which is an implementation of hook_drush_init(). For example:
   

   // Implement hook_drush_init().
   function user_drush_init() {
     $args = func_get_args();
     $command = array_shift($args);
     $needs_parse_args = array('user-block', 'user-unblock', ...);
     if (in_array($command, $needs_parse_args) {
        // parse args and call drush_set_option for --uids
       // If no uids found, call drush_set_error(). This aborts command processing. Thus, all callbacks can expect to receive valid UIDs.
     }
   }
   

I fixed my mangling from #26, and did item 2. from #27 in the attached patch.

@acrollet: Can I pass this back to you for the other items?

Any chance we can revive this.

In there interest of fewer commands, I think we should cobine add-role and remove-role into a user-role command. same for block and unblock

Haven't heard from acrollet for a while, so I'll pick this up. No complaints from me if someone else does it, though.

Bump... but only to mention that I'm not planning on fixing this for 3.0. It's nearly done, though, so please feel free to add a drush-3.0 tag if you feel like holding up the drush-3.0-stable release for this.

Hi

Is the drush user-info implemented correctly? From the example I cant actually get a response to tell me weather the user even exists. I using the latest patch and running : "drush user-info --basic targetusername" and getting a response telling me its missing an arugment and prints the details for user '' (ie no information passed in) whats the correct format for passing in a user/users?

Thanks

DF

Thanks for jumping back on this. A few minor notes below.

+++ commands/user/user.drush.inc	2010-06-18 17:23:53.000000000 -0500
@@ -0,0 +1,530 @@
+      '--detail' => 'show basic, extended, or all information about the user',

should we just re-use the verbose option here? if there are more than 1 level of detail, lets itemize each one. note that the 'sa' command has --full and --short options. might be worth considering here instead of inventing own names.

+++ commands/user/user.drush.inc	2010-06-18 17:23:53.000000000 -0500
@@ -0,0 +1,530 @@
+    'core' => array('5','6','7'),

omit this line (and similar below) when not needed. we expect this command to work on d8 until some api change breaks it. thats ok.

+++ commands/user/user.drush.inc	2010-06-18 17:23:53.000000000 -0500
@@ -0,0 +1,530 @@
+      '--users' => 'A comma delimited list of user uids, names and mail addresses to block (you will be prompted to resolve any ambiguities between uids and names)',

Isn't this option a duplicate of the argument?

Looks like we should print basic user info using same approach as status command. this makes for better wrapping of long values. search for drush_print_table(drush_key_value_to_array_table($status_table)); in core.drush.inc

thanks. just reviewed yet again and found:

drush_die("You must specify a password!"); should be return drush_set_error() for better compatibility with backend calls.

looks to me like --pipe only works with basic user info which is surprising.

need command aliases. i propose

usin
usbl
usubl
usar
usrr
uscr
usca
usp

Regarding aliases, I'd rather see one less character from "user", and one more character from the verb. uins, ublk, unblk, etc.

Finally trying these out ...

I get 4 warnings similar to below when doing a user-create on d7. similar for uinf so i think the problem is in a common subroutine.

WD php: Warning: array_flip(): Can only flip STRING and INTEGER values! in                     [warning]
DrupalDefaultEntityController->load() (line 104 of /Users/mw/htd/prfr/includes/entity.inc).
WD php: Warning: array_flip(): Can only flip STRING and INTEGER values! in                     [warning]
DrupalDefaultEntityController->cacheGet() (line 264 of /Users/mw/htd/prfr/includes/entity.inc).

I passed an invalid string to user-block and got no error back. Same for uinf. Lets do descriptive confirm and error messages for these commands.

Thanks for your persistence ...

1. drush_get_option('delete-content') is undocumented
2. it would be nice if role and block commands checked to see if they make sense before running. for example, i can unblock an active user without any error. thats not too sensible. this is a nice to have, not a requirement.

3. drush_set_error("Could not find a uid for the search term '" . $search . "'!");
   return FALSE;
   

   you can just do return drush_set_error("Could not find a uid for the search term '" . $search . "'!");

4. i think that the commands which support disambiguation should mention this feature in their help text.
5. lets do success messages with type='success' instead of notice. this will make them show up in non verbose mode which is good IMO. for example, drush_log(dt("Added the %role role to uid %uid", array('%role' => $role, '%uid' => $uid)), 'notice');. In general, I'd like to see more success messages in these commands. i know the unix way is to be quiet on success but thats pretty disconcerting IMO. Not a big deal.
6. Is seems that we have no support for --simulate. When we do something that changes DB, we should run it through drush_op(). That gets you simulate support for free.

I implemented --simulate and committed this. I also fixed #836910: drush_drupal_version() is broken on d7 in a later commit.

thanks to acrollet for persisting on this. very nice first patch for drush4.

i wil leave it to anarcat to decide if this should get into drush3.

We have discussed the policy of the drush 3.x branch at a BOF at Drupalcon CPH and it was agreed to only include critical and security fixes on the branch, to make it easier to sync in the official distributions (e.g. Debian). New features and small bug reports like this one, unless really critical or security, will therefore be refused for the 3.x branch.

Actually, i'm reconsidering this one, since Debian is still in a "sludge" rather than a full freeze... Can we get a complete patch here? Moshe, you mentionned adding --simulate in there... Should I just slap in the user.drush.inc file?

That would be great to have it ported to Drush 3.x, it is extremely useful stuff.

[EDIT] It seems this change Component: Code » PM (dl, en, up ...) was forced without my intention

Just tried the patch for Drush 3.3 from http://drupal.org/project/drush_user and it works great.

3.x gets security fixes only. this is in 4.