From 5453c1d7fa5a96393c8ab2f7046ec95fe1c43cfa Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Mon, 25 Mar 2024 23:45:39 +0100 Subject: [PATCH] feat(settings): Allow to sort groups in the account management alphabetically We can do this purly in the frontend - but when enforced from the backend using the existing system config, we need to follow the requirement. We then show a warning about the configuration. Signed-off-by: Ferdinand Thiessen --- .../lib/Controller/UsersController.php | 12 ++- .../components/Users/UserSettingsDialog.vue | 51 ++++++++++++ .../settings/src/constants/GroupManagement.ts | 29 +++++++ apps/settings/src/store/users.js | 56 ++++++++----- .../src/views/UserManagementNavigation.vue | 2 +- config/config.sample.php | 1 + cypress/e2e/settings/users_groups.cy.ts | 81 ++++++++++++++++++- 7 files changed, 207 insertions(+), 25 deletions(-) create mode 100644 apps/settings/src/constants/GroupManagement.ts diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index dbf86db20ac..10b8f690812 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -125,7 +125,7 @@ class UsersController extends Controller { /* SORT OPTION: SORT_USERCOUNT or SORT_GROUPNAME */ $sortGroupsBy = \OC\Group\MetaData::SORT_USERCOUNT; $isLDAPUsed = false; - if ($this->config->getSystemValue('sort_groups_by_name', false)) { + if ($this->config->getSystemValueBool('sort_groups_by_name', false)) { $sortGroupsBy = \OC\Group\MetaData::SORT_GROUPNAME; } else { if ($this->appManager->isEnabledForUser('user_ldap')) { @@ -212,13 +212,19 @@ class UsersController extends Controller { /* LANGUAGES */ $languages = $this->l10nFactory->getLanguages(); + /** Using LDAP or admins (system config) can enfore sorting by group name, in this case the frontend setting is overwritten */ + $forceSortGroupByName = $sortGroupsBy === \OC\Group\MetaData::SORT_GROUPNAME; + /* FINAL DATA */ $serverData = []; // groups $serverData['groups'] = array_merge_recursive($adminGroup, [$disabledUsersGroup], $groups); // Various data $serverData['isAdmin'] = $isAdmin; - $serverData['sortGroups'] = $sortGroupsBy; + $serverData['sortGroups'] = $forceSortGroupByName + ? \OC\Group\MetaData::SORT_GROUPNAME + : (int)$this->config->getAppValue('core', 'group.sortBy', (string)\OC\Group\MetaData::SORT_USERCOUNT); + $serverData['forceSortGroupByName'] = $forceSortGroupByName; $serverData['quotaPreset'] = $quotaPreset; $serverData['allowUnlimitedQuota'] = $allowUnlimitedQuota; $serverData['userCount'] = $userCount; @@ -247,7 +253,7 @@ class UsersController extends Controller { * @return JSONResponse */ public function setPreference(string $key, string $value): JSONResponse { - $allowed = ['newUser.sendEmail']; + $allowed = ['newUser.sendEmail', 'group.sortBy']; if (!in_array($key, $allowed, true)) { return new JSONResponse([], Http::STATUS_FORBIDDEN); } diff --git a/apps/settings/src/components/Users/UserSettingsDialog.vue b/apps/settings/src/components/Users/UserSettingsDialog.vue index 8580fe5e3fb..d8db67514c4 100644 --- a/apps/settings/src/components/Users/UserSettingsDialog.vue +++ b/apps/settings/src/components/Users/UserSettingsDialog.vue @@ -48,6 +48,32 @@ + + + {{ t('settings', 'The system config enforces sorting the groups by name. This also disables showing the member count.') }} + +
+ {{ t('settings', 'Group list sorting') }} + + {{ t('settings', 'By member count') }} + + + {{ t('settings', 'By name') }} + +
+ + + + diff --git a/apps/settings/src/constants/GroupManagement.ts b/apps/settings/src/constants/GroupManagement.ts new file mode 100644 index 00000000000..573c8691c70 --- /dev/null +++ b/apps/settings/src/constants/GroupManagement.ts @@ -0,0 +1,29 @@ +/** + * @copyright Copyright (c) 2024 Ferdinand Thiessen + * + * @author Ferdinand Thiessen + * + * @license AGPL-3.0-or-later + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +/** + * https://github.com/nextcloud/server/blob/208e38e84e1a07a49699aa90dc5b7272d24489f0/lib/private/Group/MetaData.php#L34 + */ +export enum GroupSorting { + UserCount = 1, + GroupName = 2 +} diff --git a/apps/settings/src/store/users.js b/apps/settings/src/store/users.js index 7fbd6cd7230..bfc0f4e119f 100644 --- a/apps/settings/src/store/users.js +++ b/apps/settings/src/store/users.js @@ -30,26 +30,16 @@ import { getBuilder } from '@nextcloud/browser-storage' import { getCapabilities } from '@nextcloud/capabilities' import { parseFileSize } from '@nextcloud/files' -import { generateOcsUrl } from '@nextcloud/router' +import { showError } from '@nextcloud/dialogs' +import { generateOcsUrl, generateUrl } from '@nextcloud/router' import axios from '@nextcloud/axios' +import { GroupSorting } from '../constants/GroupManagement.ts' import api from './api.js' import logger from '../logger.ts' const localStorage = getBuilder('settings').persist(true).build() -const orderGroups = function(groups, orderBy) { - /* const SORT_USERCOUNT = 1; - * const SORT_GROUPNAME = 2; - * https://github.com/nextcloud/server/blob/208e38e84e1a07a49699aa90dc5b7272d24489f0/lib/private/Group/MetaData.php#L34 - */ - if (orderBy === 1) { - return groups.sort((a, b) => a.usercount - a.disabled < b.usercount - b.disabled) - } else { - return groups.sort((a, b) => a.name.localeCompare(b.name)) - } -} - const defaults = { group: { id: '', @@ -64,7 +54,7 @@ const defaults = { const state = { users: [], groups: [], - orderBy: 1, + orderBy: GroupSorting.UserCount, minPasswordLength: 0, usersOffset: 0, usersLimit: 25, @@ -100,8 +90,6 @@ const mutations = { state.groups = groups.map(group => Object.assign({}, defaults.group, group)) state.orderBy = orderBy state.userCount = userCount - state.groups = orderGroups(state.groups, state.orderBy) - }, addGroup(state, { gid, displayName }) { try { @@ -114,7 +102,6 @@ const mutations = { name: displayName, }) state.groups.unshift(group) - state.groups = orderGroups(state.groups, state.orderBy) } catch (e) { console.error('Can\'t create group', e) } @@ -125,7 +112,6 @@ const mutations = { const updatedGroup = state.groups[groupIndex] updatedGroup.name = displayName state.groups.splice(groupIndex, 1, updatedGroup) - state.groups = orderGroups(state.groups, state.orderBy) } }, removeGroup(state, gid) { @@ -143,7 +129,6 @@ const mutations = { } const groups = user.groups groups.push(gid) - state.groups = orderGroups(state.groups, state.orderBy) }, removeUserGroup(state, { userid, gid }) { const group = state.groups.find(groupSearch => groupSearch.id === gid) @@ -154,7 +139,6 @@ const mutations = { } const groups = user.groups groups.splice(groups.indexOf(gid), 1) - state.groups = orderGroups(state.groups, state.orderBy) }, addUserSubAdmin(state, { userid, gid }) { const groups = state.users.find(user => user.id === userid).subadmin @@ -254,6 +238,23 @@ const mutations = { localStorage.setItem(`account_settings__${key}`, JSON.stringify(value)) state.showConfig[key] = value }, + + setGroupSorting(state, sorting) { + const oldValue = state.orderBy + state.orderBy = sorting + + // Persist the value on the server + axios.post( + generateUrl('/settings/users/preferences/group.sortBy'), + { + value: String(sorting), + }, + ).catch((error) => { + state.orderBy = oldValue + showError(t('settings', 'Could not set group sorting')) + logger.error(error) + }) + }, } const getters = { @@ -267,6 +268,21 @@ const getters = { // Can't be subadmin of admin or disabled return state.groups.filter(group => group.id !== 'admin' && group.id !== 'disabled') }, + getSortedGroups(state) { + const groups = [...state.groups] + if (state.orderBy === GroupSorting.UserCount) { + return groups.sort((a, b) => { + const numA = a.usercount - a.disabled + const numB = b.usercount - b.disabled + return (numA < numB) ? 1 : (numB < numA ? -1 : a.name.localeCompare(b.name)) + }) + } else { + return groups.sort((a, b) => a.name.localeCompare(b.name)) + } + }, + getGroupSorting(state) { + return state.orderBy + }, getPasswordPolicyMinLength(state) { return state.minPasswordLength }, diff --git a/apps/settings/src/views/UserManagementNavigation.vue b/apps/settings/src/views/UserManagementNavigation.vue index 4959040a1bf..51a5744543d 100644 --- a/apps/settings/src/views/UserManagementNavigation.vue +++ b/apps/settings/src/views/UserManagementNavigation.vue @@ -149,7 +149,7 @@ const selectedGroupDecoded = computed(() => selectedGroup.value ? decodeURICompo /** Overall user count */ const userCount = computed(() => store.getters.getUserCount) /** All available groups */ -const groups = computed(() => store.getters.getGroups) +const groups = computed(() => store.getters.getSortedGroups) const { adminGroup, disabledGroup, userGroups } = useFormatGroups(groups) /** True if the current user is an administrator */ diff --git a/config/config.sample.php b/config/config.sample.php index 13a4d99f1e3..280334477a6 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -1353,6 +1353,7 @@ $CONFIG = [ * Sort groups in the user settings by name instead of the user count * * By enabling this the user count beside the group name is disabled as well. + * @deprecated since Nextcloud 29 - Use the frontend instead or set the app config value `group.sortBy` for `core` to `2` */ 'sort_groups_by_name' => false, diff --git a/cypress/e2e/settings/users_groups.cy.ts b/cypress/e2e/settings/users_groups.cy.ts index 7f9052800a1..fd56c558b4f 100644 --- a/cypress/e2e/settings/users_groups.cy.ts +++ b/cypress/e2e/settings/users_groups.cy.ts @@ -21,7 +21,7 @@ */ import { User } from '@nextcloud/cypress' -import { getUserListRow, handlePasswordConfirmation, toggleEditButton } from './usersUtils' +import { assertNotExistOrNotVisible, getUserListRow, handlePasswordConfirmation, toggleEditButton } from './usersUtils' // eslint-disable-next-line n/no-extraneous-import import randomString from 'crypto-random-string' @@ -223,3 +223,82 @@ describe('Settings: Delete a non empty group', () => { }) }) }) + +describe.only('Settings: Sort groups in the UI', () => { + before(() => { + // Clear state + cy.runOccCommand('group:list --output json').then((output) => { + const groups = Object.keys(JSON.parse(output.stdout)).filter((group) => group !== 'admin') + groups.forEach((group) => { + cy.runOccCommand(`group:delete "${group}"`) + }) + }) + + // Add two groups and add one user to group B + cy.runOccCommand('group:add A') + cy.runOccCommand('group:add B') + cy.createRandomUser().then((user) => { + cy.runOccCommand(`group:adduser B "${user.userId}"`) + }) + + // Visit the settings as admin + cy.login(admin) + cy.visit('/settings/users') + }) + + it('Can set sort by member count', () => { + // open the settings dialog + cy.contains('button', 'Account management settings').click() + + cy.contains('.modal-container', 'Account management settings').within(() => { + cy.get('[data-test="sortGroupsByMemberCount"] input[type="radio"]').scrollIntoView() + cy.get('[data-test="sortGroupsByMemberCount"] input[type="radio"]').check({ force: true }) + // close the settings dialog + cy.get('button.modal-container__close').click() + }) + cy.waitUntil(() => cy.get('.modal-container').should(el => assertNotExistOrNotVisible(el))) + }) + + it('See that the groups are sorted by the member count', () => { + cy.get('ul[data-cy-users-settings-navigation-groups="custom"]').within(() => { + cy.get('li').eq(0).should('contain', 'B') // 1 member + cy.get('li').eq(1).should('contain', 'A') // 0 members + }) + }) + + it('See that the order is preserved after a reload', () => { + cy.reload() + cy.get('ul[data-cy-users-settings-navigation-groups="custom"]').within(() => { + cy.get('li').eq(0).should('contain', 'B') // 1 member + cy.get('li').eq(1).should('contain', 'A') // 0 members + }) + }) + + it('Can set sort by group name', () => { + // open the settings dialog + cy.contains('button', 'Account management settings').click() + + cy.contains('.modal-container', 'Account management settings').within(() => { + cy.get('[data-test="sortGroupsByName"] input[type="radio"]').scrollIntoView() + cy.get('[data-test="sortGroupsByName"] input[type="radio"]').check({ force: true }) + // close the settings dialog + cy.get('button.modal-container__close').click() + }) + cy.waitUntil(() => cy.get('.modal-container').should(el => assertNotExistOrNotVisible(el))) + }) + + it('See that the groups are sorted by the user count', () => { + cy.get('ul[data-cy-users-settings-navigation-groups="custom"]').within(() => { + cy.get('li').eq(0).should('contain', 'A') + cy.get('li').eq(1).should('contain', 'B') + }) + }) + + it('See that the order is preserved after a reload', () => { + cy.reload() + cy.get('ul[data-cy-users-settings-navigation-groups="custom"]').within(() => { + cy.get('li').eq(0).should('contain', 'A') + cy.get('li').eq(1).should('contain', 'B') + }) + }) +})