attached patch adds mass editing capabilities to admin/user, similar to those at admin/node. i'm particularly fond of the mass role editing feature. :) not only does it allow you to edit multiple user's roles at the same time, but the dropdowns also give you a nice overview of who has what roles.

the mass operations dropdown is generated using the same logic as found in this patch, so other modules can also add their own mass editing operations to this form.

CommentFileSizeAuthor
#22 useradmin_9.patch11.07 KBhunmonk
#19 useradmin_8.patch11.39 KBhunmonk
#17 useradmin_7.patch11.35 KBhunmonk
#16 useradmin_6.patch11.35 KBhunmonk
#13 useradmin_5.patch11.7 KBhunmonk
#11 useradmin_4.patch11.86 KBhunmonk
#7 useradmin_3.patch11.72 KBhunmonk
#6 useradmin_2.patch11.69 KBhunmonk
#5 useradmin_1.patch11.71 KBhunmonk
#3 useradmin_0.patch11.09 KBhunmonk
useradmin.patch11.18 KBhunmonk
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hunmonk’s picture

hunmonk CreditAttribution: hunmonk commented

oh, and of course a nice demo site here:

http://useradmin.xcarnated.com/

Michelle’s picture

Michelle
she/her
Primary language English
Location Wisconsin, USA
CreditAttribution: Michelle commented

Awesome! I run a site that lets anyone join but club members get a special role. This would make it much easier to see at a glance if I have everyone on the right role and correct any ones that aren't.

Michelle

hunmonk’s picture

hunmonk CreditAttribution: hunmonk commented
FileSize
useradmin_0.patch11.09 KB

some touchups to the patch. i changed the markings for roles that a user already has to a leading asterisk--seems easier to read to me. also added a 'Select/View' option to the dropdowns, as it seems to make the interface come together a little better, and allowed me to get rid of the conditional display code for the roles column.

eaton’s picture

eaton CreditAttribution: eaton commented

A major +1 for this functionality. Dealing with users in batches is a real pain, and role-adding is just as bad.

Haven't had a chance to apply the patch yet but I've used it on the test site. Will comment further when I am able to apply it.

hunmonk’s picture

hunmonk CreditAttribution: hunmonk commented
FileSize
useradmin_1.patch11.71 KB

ok, i've revamped the approach for roles, and i'm much happier with the result. roles column for users is now a collapsible fieldset which lists their current roles--this allows for a quick look at that detail w/o clogging the table with a constant display, plus all those dropdowns in the table were a bit ugly and confusing.

i moved the mass adding/deleting of roles straight to the update dropdown inside of optgroups, which seems more intuitive to me--just select the role to mass add/delete, check the users it applies to, and click update.

while this version loses the ability to mass add/delete different roles for different users at the same time, i don't think it's a big loss compared to how much cleaner and more intuitive the interface is now.

you can check out the upgrade here: http://useradmin.xcarnated.com/

hunmonk’s picture

hunmonk CreditAttribution: hunmonk commented
FileSize
useradmin_2.patch11.69 KB

stupid me--not using theme_item_list where i should... :)

attached patch corrects this.

demo site: http://useradmin.xcarnated.com/

hunmonk’s picture

hunmonk CreditAttribution: hunmonk commented
FileSize
useradmin_3.patch11.72 KB

looks like a cache_clear_all was added to the admin/node submit function. since this function basically mirrors that, i figured i should throw that in here as well...

demo site: http://useradmin.xcarnated.com/

jivyb’s picture

jivyb CreditAttribution: jivyb commented

I might have missed it on the demo, but I'd love to see an option to check all or uncheck all. I think the content admin page really needs this too...(sorry I know that is irrelevant here:)

hunmonk’s picture

hunmonk CreditAttribution: hunmonk commented

@jivyb: would be nice, but out of scope for this patch, i think.

hunmonk’s picture

hunmonk CreditAttribution: hunmonk commented

somewhere along the way i broke the updates for blocking and unblocking users--i'll attend to that shortly. also, after some discussion on irc, i'm persuaded to make some changes in the roles column display. the consensus seemed to be that if collapsible form groups are going to be used:

  1. they should be around all rows, no matter how few roles there are (at least an uncollapsed form group, to give continuity)
  2. collapsed titles should be in the form of 'X roles' and not the uninformative 'view'

i'll play with those ideas tonight and get something posted which addresses those points.

hunmonk’s picture

hunmonk CreditAttribution: hunmonk commented
FileSize
useradmin_4.patch11.86 KB

ok, latest fixes to address the issues in the post above. all role rows are wrapped in collapsible fieldsets now, which gives a lot more visual consistency. for convenience, any users w/ 2 or less roles have the fieldset open so the roles can be viewed.

demo site: http://useradmin.xcarnated.com/

Dries’s picture

Dries CreditAttribution: Dries commented

Cool feature but the fieldset looks really,really weird (please take it out, if the user filtering patch in is available, it shouldn't really matter). I'm traveling so I don't have time to review the code.

hunmonk’s picture

hunmonk CreditAttribution: hunmonk commented
FileSize
useradmin_5.patch11.7 KB

attached patch removes the collapsible fieldsets from around the role column. i also removed the auth user from the roles listing--seems wasteful as every user has the role.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Needs review » Needs work

Code looks good, functionality is great. Your test site is down though.

The only thing that looks a little bit weird is:

+  if ($_POST['edit']['accounts'] && $_POST['edit']['operation'] == 'delete') {
+    return user_multiple_delete_confirm();
+  }

Is this proper use of the forms API?

Instead of writing:

+      $row = array();
+      $row[] = form_render($form['accounts'][$key]);
+      $row[] = form_render($form['name'][$key]);
+      $row[] = form_render($form['status'][$key]);
+      $row[] = form_render($form['roles'][$key]);
+      $row[] = form_render($form['member_for'][$key]);
+      $row[] = form_render($form['last_access'][$key]);
+      $row[] = form_render($form['operations'][$key]);
+      $rows[] = $row;

write:

+      $row = array(
+        form_render($form['accounts'][$key]),
+        form_render($form['name'][$key]),
+        form_render($form['status'][$key]),
+        form_render($form['roles'][$key]),
+        form_render($form['member_for'][$key]),
+        form_render($form['last_access'][$key]),
+        form_render($form['operations'][$key]));
hunmonk’s picture

hunmonk CreditAttribution: hunmonk commented
Status: Needs work » Needs review

Code looks good, functionality is great. Your test site is down though.

Thanks :) sorry about the test site--i'm out of town, the server is down, and won't be able to get it back up until later tonight.

Is this proper use of the forms API?

this is the exact same implementation that admin/node uses, and i'm not really sure there's a better way to do it. the problem is that since it's a delete op, there's a confirmation screen, which kind of makes this page a two-stage form for deletions only. it doesn't really work very well to put the delete stuff in the _submit function, as you can't call another drupal_get_form while you're in the middle of executing another one... :) i'm open for suggestions for a better implementation...

Instead of writing... write...

attached patch corrects this.

hunmonk’s picture

hunmonk CreditAttribution: hunmonk commented
FileSize
useradmin_6.patch11.35 KB

...and the patch... :)

hunmonk’s picture

hunmonk CreditAttribution: hunmonk commented
FileSize
useradmin_7.patch11.35 KB

broken by the admin patch. updated patch attached. apologies for the test site not being up. i'll get that remedied shortly...

Dries’s picture

Dries CreditAttribution: Dries commented

Please spit the user_operation callbacks in separate functions and document them properly (cfr. node operations). Thanks.

hunmonk’s picture

hunmonk CreditAttribution: hunmonk commented
FileSize
useradmin_8.patch11.39 KB

ok, broke out the mass ops into seperate functions, and properly doc'd. got rid of the call back arg stuff in the submit function. also fixed some bad paths in the delete confirm form b/c of the admin patch. i think she's ready to go!

test site is working again as well: http://useradmin.xcarnated.com/

chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Needs work

For some reason, sorting on test site is by uid and not name though from teh code it looks OK.

array_diff($edit['accounts'], array(0));

isn't this a baroque way of saying array_filter($edit['accounts']) ?

form_set_error('', t('Please select some users to perform the delete operation.')); we sure that there is no better way to this? looks a very little bit hackish.

db_query('INSERT INTO {users_roles} VALUES( there missing a space.

these are just small things. nice job.

hunmonk’s picture

hunmonk CreditAttribution: hunmonk commented

@chx: thanks for these. i'm busy today and tomorrow, but will attend to the patch the next day.

hunmonk’s picture

hunmonk CreditAttribution: hunmonk commented
FileSize
useradmin_9.patch11.07 KB

@chx: ok, all of your concerns have been addressed.

the sorting problem had to do w/ the fact that i wasn't including the data key in the $header array during the form building function, since i thought it wasn't really needed until the theming function. however, the tablesorting code does in fact need that key in order to process the sorting request. given the fact that sometimes the form building code and theming code are in different functions now, we might want to examine the tablesort code and streamline it for these cases--it seems wasteful to me to have to include the entire $header array in both the form building and form theming functions. for now, though, it does seem like the correct approach.

i decided to get rid of that whole optional message thing in the validation code--it seems like overkill to me. now if no users are selected, you simply get a "No users selected" warning message. btw, that dual message approach was copied from the node admin code, so we might want to clean that up as well. :)

updated patch attached.

test site: http://useradmin.xcarnated.com/

hunmonk’s picture

hunmonk CreditAttribution: hunmonk commented
Status: Needs work » Needs review
chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Reviewed & tested by the community

The test site seems working. I found no glaring code errors.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD.

Anonymous’s picture

(not verified) CreditAttribution: commented
Status: Fixed » Closed (fixed)