Show users without groups #14614
Show users without groups #14614
Conversation
Signed-off-by: Oliver Wittkopf <[email protected]>
Signed-off-by: Oliver Wittkopf <[email protected]>
Signed-off-by: Oliver Wittkopf <[email protected]>
Signed-off-by: Oliver Wittkopf <[email protected]>
Signed-off-by: Oliver Wittkopf <[email protected]>
Signed-off-by: Oliver Wittkopf <[email protected]>
Signed-off-by: Oliver Wittkopf <[email protected]>
MorrisJobke
added
enhancement
3. to review
There was a problem hiding this comment.
See comments.
There is a further issue that complicates this scenario: the user backend actually does not have a clue about the memberships, and there might be more than one group backend enabled and users from one backend might belong to groups from a different one. This makes determining all ungrouped users very expensive.
| * returns how many enabled users have no groups | ||
| * | ||
| * @return int | ||
| * @since 1?.0.0 |
There was a problem hiding this comment.
17 (as we're in feature freeze for 16) – but the annotation should go into the Interface \OCP\IUserManager
| * @since 1?.0.0 | ||
| */ | ||
| public function countNotGroupedUsers(): int { | ||
| $queryBuilder = \OC::$server->getDatabaseConnection()->getQueryBuilder(); |
There was a problem hiding this comment.
this is the wrong place to gather the information for the database, as the UserManager forwards all requests to the registered Backends. This implementation would go to OC\User\Database. The manager instead would cycle through all registered backends implementing this method (→ needs an Interface, e.g. '\OCP\User\ICountUngroupUsers`), and sum up what each backend returned.
| @@ -201,7 +201,18 @@ const actions = { | |||
| getUsers(context, { offset, limit, search, group }) { | |||
| search = typeof search === 'string' ? search : ''; | |||
| group = typeof group === 'string' ? group : ''; | |||
| if (group !== '') { | |||
| if (group === 'notGrouped') { | |||
| return api.get(OC.linkToOCS(`cloud/users/details?offset=${offset}&limit=${limit}&search=${search}`, 2)) | |||
There was a problem hiding this comment.
This seems to be identical to the following section… why?
| @@ -352,7 +354,7 @@ export default { | |||
| }); | |||
|
|
|||
| // Every item is added on top of the array, so we're going backward | |||
| // Groups, separator, disabled, admin, everyone | |||
| // Groups, separator, no groups, separator, admin, disabled, everyone | |||
There was a problem hiding this comment.
the right-most seperator is unnecessary as it is a special group like the others imho
| // filter out to re-add later to correct position | ||
| groups = groups.filter(group => notGroupedGroup != group); | ||
|
|
||
| notGroupedGroup.text = t('settings', 'Users w/o groups'); // rename notGrouped group |
There was a problem hiding this comment.
Let's write out "w/o", for many, especially non-native speaks it might not be clear, and it means different things in different fields (e.g. banking), not making sense, necessarily, but perhaps enough to irritate.
|
cc @skjnldsv |
|
Closing as there are many unresolved review comments that never got any attention. |
see #14612