Here is a patch that changes the 'Add CAS user' page to use a textarea where a comma-delimited list of users can be added in bulk. This work was done by jadhavdevendra at MPhasis for Yale University, originally as a separate module that we have not deployed yet. A discussion in IRC led to others thinking this would be a useful addition to the CAS module so I've attached a patch which is a first stab at moving this code from a submodule into CAS core. The patch includes attribution to jadhavdevendra. It needs some work so I wanted to put it out here for feedback so we can get it integrated. Thanks!
Potential issues/todos
- This code went away from
cas_add_user_form_submit()and I'm not sure if/how it can be brought back, or if it should.
// Set these in case another module needs the values. $form_state['user'] = $account; $form_state['values']['uid'] = $account->uid; - It appears that
_cas_name_element_validate()should be modified since it is expecting a single user now, and maybe then it won't be necessary to check if the account exists incas_batch_user_add().
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | cas-batch_user_create-2238973-19.patch | 7.77 KB | vinmassaro |
Comments
Comment #1
vinmassaro commentedComment #3
vinmassaro commentedHere's a fix for the tests (hopefully).
Comment #4
vinmassaro commentedComment #5
vinmassaro commentedComment #7
yalet commentedThe patch is failing the add user test because the screen you get after adding users has changed. You'll need to update the test (the relevant test is
testCasUserAddwhich starts at about line 332 of cas.test) for the new expected behavior of the page after you've added users.Comment #8
vinmassaro commentedI think rewriting this portion of the test is over my head, so I hope someone else can help out with it.
Comment #9
cilefen commentedTests.
Comment #10
cilefen commentedBad patch - this should work.
Comment #12
yalet commentedThe patch in #10 worked for me. Multiple users added correctly.
Comment #13
vinmassaro commentedComment #14
vinmassaro commented@cilefen: thanks for updating this patch!
Comment #15
cilefen commented10: cas-batch_user_create-2238973-10.patch queued for re-testing.
Comment #16
bkosborneRan tests a few times, and tested myself with a few hundred names. Works well, good work.
I wonder if it would be better to split usernames with a newline character instead of comma separated? I think that's the instinct most users will have when seeing a multi-lined text area where they are used to pasting in lists of things. Not a deal breaker though - setting RTBC
Also, I'm not familiar with the batch API, but I see that another approach would have been to use one operation (instead of an operation for each registration) and pass all of the usernames to that operation. Then the single operation would manage the batches, and state would be passed from one batch to another (like what index in the array of CAS names have already been processed). I wonder if anyone has an opinion on this and if it really matters one way or another.
Comment #17
metzlerd commentedI would generally agree that new lines would be better than commas to delineate user names as it opens the door to adding other data a standard .CSV format (e.g. having the drupal username be different than the cas username). Also wondering if the author has an objection, especially given the early indication that the module has not been deployed. Tempted to put this back to needs work. Being able to paste from spreadsheets, etc seems like an important versatility to anticipate.
Comment #18
vinmassaro commentedThanks for the feedback everyone. Here is an updated patch that switches to new lines. It accounts for new lines and carriage returns as well as cleans up multiple new lines.
Comment #20
vinmassaro commentedOops, this one should pass.
Comment #21
cilefen commentedI was just going to suggest changing that line in the test but you beat me to it.
Comment #22
bkosborneLooks good
Comment #24
bkosborne