Disabling users in WebUI #413
Conversation
Added the controller functions for enabling/disabling a user.
… an additional route handler in the user controller. Finished the visuals to reflect current user status and changed user status respectively.
…re deletion and state toggling of a user is selectable.
Improved style of user action menu.
|
@Faldon, thanks for your PR! By analyzing the annotation information on this pull request, we identified @jancborchardt, @schiessle and @ringmaster to be potential reviewers |
Removed visual indicator for disabled users.
Faldon
added
3. to review
|
| } else { | ||
| return new DataResponse( | ||
| array( | ||
| 'status' => 'error', |
There was a problem hiding this comment.
Please use HTTP status codes for the response instead of a status flag.
There was a problem hiding this comment.
Ah. You just used the same style as in the code above. Okay. Then we could keep this, but should clean this up once we touch all the code here again.
|
We already have the menu CSS in place. This is for now only implemented in the sidebar, but should be reusable: https://docs.nextcloud.org/server/9/developer_manual/app/css.html#menus
Could you check this out? And also we usually use px instead of em. Maybe this already helps to make this work. |
MorrisJobke
added
2. developing
|
But nevertheless: Thanks for your contribution :) |
|
@MorrisJobke I changed my additions to the css from em to px. There are already some declarations in em, but I would rather look into it as separate task (redesigning settings maybe, than we also can change the code for the http status codes as you mentioned above.) The last mentioned problem (not displaying user) was a bug that seems to be present already. In the template resides a single tr for the javascript, that should be kept on reloading the user list but wasn't. |
…controller. Changed units for newly introduced css values from em to px. Removed unnecessary png and optimized svg with scour. Changed the userlist template to display the user action menu with correct width.
Faldon
added
3. to review
| $batch = $this->getUsersForUID($this->groupManager->displayNamesInGroup($gid, $pattern, $limit, $offset)); | ||
| } else { | ||
| $batch = $this->userManager->search($pattern, $limit, $offset); | ||
| } | ||
|
|
||
| foreach ($batch as $user) { | ||
| $users[] = $this->formatUserForIndex($user); | ||
| if( ($gid!=='disabledUsers' && $user->isEnabled()) || |
|
Beside that it already looks quite good :) |
|
For better user experience we should make the loginException extend from HintException: use OC\HintException;
class LoginException extends HintException {
}@Faldon Could you add this? @LukasReschke I don't see a risk for leaking info here. Every time the login exception is thrown it also uses a translated message. @Faldon Beside that it works really nice 👍 |
MorrisJobke
added
2. developing
|
I'm still struggling with the code style. -.- Fixes are on their way. |
Correct. Then all is fine. This is the old way to implement it but it is consistent in that class. So leave it as it is :) |
|
@Faldon There are conflicts that doesn't allow merging. Could you try to resolve those conflicts? |
Resolving conflicts for PR nextcloud#413
|
Hmpf... |
|
@Faldon I just noticed the added icons. Instead please use the classes |
|
@jancborchardt Should I extend the icons style sheet for the disabling user icon then? |
|
@Faldon we can use |
|
@jancborchardt Ok, I did the changes for the action menu. However, I left the code style for the "more" icon to comply with the other icons in the tr that are visible on hover. |
MorrisJobke
added
3. to review
|
The conflicts doesn't seem to be trivial. I will try to sort this out. Sorry for having this laying around that long 😢 |
I will close this here for now. |
|
Superseeded by #1234 |
Ability to disable users in Web UI
Base for implementing #283
TODO:
When enabling a user, site needs a reload because the groups won't be filled with users anymore.(Edit: I just fired ab my local test server after the weekend and this strange behaviour was gone. Maybe a cache problem?)The other way around seems to work fine.