Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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 @@ -499,6 +499,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
9 changes: 8 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 = $isLDAPUsed ? -1 : $this->userManager->countNotGroupedUsers();
} 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
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