Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show users without groups #14614

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions lib/private/User/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,37 @@ public function countDisabledUsersOfGroups(array $groups): int {
return $count;
}

/**
* returns how many enabled users have no groups
*
* @return int
* @since 1?.0.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

17 (as we're in feature freeze for 16) – but the annotation should go into the Interface \OCP\IUserManager

*/
public function countNotGroupedUsers(): int {
$queryBuilder = \OC::$server->getDatabaseConnection()->getQueryBuilder();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

$queryBuilder->select($queryBuilder->func()->count('*'))
->from('accounts', 'a')
->leftJoin('a', 'group_user', 'g', $queryBuilder->expr()->eq('a.uid', 'g.uid'))
->innerJoin('a', 'preferences', 'p', $queryBuilder->expr()->eq('a.uid', 'p.userid'))
->where($queryBuilder->expr()->isNull('g.uid'))
->andWhere($queryBuilder->expr()->eq('appid', $queryBuilder->createNamedParameter('core')))
->andWhere($queryBuilder->expr()->eq('configkey', $queryBuilder->createNamedParameter('enabled')))
->andWhere($queryBuilder->expr()->eq('configvalue', $queryBuilder->createNamedParameter('true'), IQueryBuilder::PARAM_STR));


$result = $queryBuilder->execute();
$count = $result->fetchColumn();
$result->closeCursor();

if ($count !== false) {
$count = (int)$count;
} else {
$count = 0;
}

return $count;
}

/**
* returns how many users have logged in once
*
Expand Down
10 changes: 9 additions & 1 deletion settings/Controller/UsersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,13 @@ public function usersList() {
});
}

$notGroupedUsers = 0;
if ($this->isAdmin) {
$disabledUsers = $isLDAPUsed ? -1 : $this->userManager->countDisabledUsers();
$userCount = $isLDAPUsed ? 0 : array_reduce($this->userManager->countUsers(), function($v, $w) {
return $v + (int)$w;
}, 0);
$notGroupedUsers = $this->userManager->countNotGroupedUsers();
chiefbrain marked this conversation as resolved.
Show resolved Hide resolved
} else {
// User is subadmin !
// Map group list to names to retrieve the countDisabledUsersOfGroups
Expand All @@ -217,6 +219,11 @@ public function usersList() {
'name' => 'Disabled users',
'usercount' => $disabledUsers
];
$notGroupedUsersGroup = [
'id' => 'notGrouped',
'name' => 'Users w/o groups',
'usercount' => $notGroupedUsers
];

/* QUOTAS PRESETS */
$quotaPreset = $this->config->getAppValue('files', 'quota_preset', '1 GB, 5 GB, 10 GB');
Expand All @@ -235,7 +242,7 @@ public function usersList() {
/* FINAL DATA */
$serverData = array();
// groups
$serverData['groups'] = array_merge_recursive($adminGroup, [$disabledUsersGroup], $groups);
$serverData['groups'] = array_merge_recursive($adminGroup, [$disabledUsersGroup], $groups, [$notGroupedUsersGroup]);
// Various data
$serverData['isAdmin'] = $this->isAdmin;
$serverData['sortGroups'] = $sortGroupsBy;
Expand Down Expand Up @@ -497,3 +504,4 @@ protected function signMessage(IUser $user, string $message): string {
return base64_encode($signature);
}
}

2 changes: 1 addition & 1 deletion settings/js/vue-7.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion settings/js/vue-7.js.map

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion settings/js/vue-settings-apps-users-management.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion settings/js/vue-settings-apps-users-management.js.map

Large diffs are not rendered by default.

12 changes: 11 additions & 1 deletion settings/src/components/userList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,25 @@ export default {
}
return disabledUsers;
}
else if (this.selectedGroup === 'notGrouped') {
let noGroupsUsers = this.users.filter(user => user.groups.length === 0 && user.enabled === true);
if (noGroupsUsers.length===0 && this.$refs.infiniteLoading && this.$refs.infiniteLoading.isComplete) {
// noGroups group is empty, redirection to all users
this.$router.push({name: 'users'});
this.$refs.infiniteLoading.stateChanger.reset()
}
return noGroupsUsers;
}
if (!this.settings.isAdmin) {
// we don't want subadmins to edit themselves
return this.users.filter(user => user.enabled !== false && user.id !== oc_current_user);
}
return this.users.filter(user => user.enabled !== false);
},
groups() {
// data provided php side + remove the disabled group
// data provided php side + remove the disabled and the notGrouped group
return this.$store.getters.getGroups
.filter(group => group.id !== 'notGrouped')
.filter(group => group.id !== 'disabled')
.sort((a, b) => a.name.localeCompare(b.name));
},
Expand Down
15 changes: 13 additions & 2 deletions settings/src/store/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ const getters = {
},
getSubadminGroups(state) {
// Can't be subadmin of admin or disabled
return state.groups.filter(group => group.id !== 'admin' && group.id !== 'disabled');
return state.groups.filter(group => group.id !== 'admin' && group.id !== 'disabled' && group.id !== 'notGrouped');
},
getPasswordPolicyMinLength(state) {
return state.minPasswordLength;
Expand Down Expand Up @@ -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))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be identical to the following section… why?

.then((response) => {
if (Object.keys(response.data.ocs.data.users).length > 0) {
context.commit('appendUsers', response.data.ocs.data.users);
return true;
}
return false;
})
.catch((error) => context.commit('API_FAILURE', error));
}
else if (group !== '') {
return api.get(OC.linkToOCS(`cloud/groups/${group}/users/details?offset=${offset}&limit=${limit}&search=${search}`, 2))
.then((response) => {
if (Object.keys(response.data.ocs.data.users).length > 0) {
Expand Down
23 changes: 19 additions & 4 deletions settings/src/views/Users.vue
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,8 @@ export default {
menu() {
// Data provided php side
let self = this;

// Entry: add "groups"
let groups = this.$store.getters.getGroups;
groups = Array.isArray(groups) ? groups : [];

Expand All @@ -338,7 +340,7 @@ export default {
item.utils.counter = group.usercount - group.disabled;
}

if (item.id !== 'admin' && item.id !== 'disabled' && this.settings.isAdmin) {
if (item.id !== 'admin' && item.id !== 'disabled' && this.settings.isAdmin && item.id !== 'notGrouped') {
// add delete button on real groups
item.utils.actions = [{
icon: 'icon-delete',
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the right-most seperator is unnecessary as it is a special group like the others imho


// Add separator
let realGroups = groups.find((group) => {return group.id !== 'disabled' && group.id !== 'admin'});
Expand All @@ -366,7 +368,18 @@ export default {
groups.unshift(separator);
}

// Adjust admin and disabled groups
// Entry: adjust "no groups"
let notGroupedGroup = groups.find(group => group.id == 'notGrouped');

if (notGroupedGroup && notGroupedGroup.text) {
// 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

groups.unshift(notGroupedGroup); // re-add notGrouped group
}

// Entry: adjust "admin and disabled groups"
let adminGroup = groups.find(group => group.id == 'admin');
let disabledGroup = groups.find(group => group.id == 'disabled');

Expand All @@ -390,14 +403,15 @@ export default {
}


// Add everyone group
// Entry: add "everyone group"
let everyoneGroup = {
id: 'everyone',
key: 'everyone',
icon: 'icon-contacts-dark',
router: {name:'users'},
text: t('settings', 'Everyone'),
};

// users count
if (this.userCount > 0) {
Vue.set(everyoneGroup, 'utils', {
Expand All @@ -406,6 +420,7 @@ export default {
}
groups.unshift(everyoneGroup);

// Entry: add "add group"
let addGroup = {
id: 'addgroup',
key: 'addgroup',
Expand Down
15 changes: 15 additions & 0 deletions tests/acceptance/features/users.feature
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,21 @@ Feature: users
# When I open the User settings
# Then I see that the section "Disabled users" is not shown

Scenario: users navigation without not grouped users
Given I act as Jane
And I am logged in as the admin
And I open the User settings
And I open the "Users w/o groups" section
And I see that the list of users contains the user notGroupedUser
# disabled because we need the TAB patch:
# https://github.com/minkphp/MinkSelenium2Driver/pull/244
# When I assign the user notGroupedUser to the group admin
# Then I see that the section "Users w/o groups" is not shown
# check again after reloading the settings
# When I open the User settings
# Then I see that the section "Users w/o groups" is not shown


Scenario: assign user to a group
Given I act as Jane
And I am logged in as the admin
Expand Down