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 <opensource@fthiessen.de>

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);
 		}

apps/settings/src/components/Users/UserSettingsDialog.vue

@@ -48,6 +48,32 @@
 			</NcCheckboxRadioSwitch>
 		</NcAppSettingsSection>
 
+		<NcAppSettingsSection id="groups-sorting"
+			:name="t('settings', 'Sorting')">
+			<NcNoteCard v-if="isGroupSortingEnforced" type="warning">
+				{{ t('settings', 'The system config enforces sorting the groups by name. This also disables showing the member count.') }}
+			</NcNoteCard>
+			<fieldset>
+				<legend>{{ t('settings', 'Group list sorting') }}</legend>
+				<NcCheckboxRadioSwitch type="radio"
+					:checked.sync="groupSorting"
+					data-test="sortGroupsByMemberCount"
+					:disabled="isGroupSortingEnforced"
+					name="group-sorting-mode"
+					value="member-count">
+					{{ t('settings', 'By member count') }}
+				</NcCheckboxRadioSwitch>
+				<NcCheckboxRadioSwitch type="radio"
+					:checked.sync="groupSorting"
+					data-test="sortGroupsByName"
+					:disabled="isGroupSortingEnforced"
+					name="group-sorting-mode"
+					value="name">
+					{{ t('settings', 'By name') }}
+				</NcCheckboxRadioSwitch>
+			</fieldset>
+		</NcAppSettingsSection>
+
 		<NcAppSettingsSection id="email-settings"
 			:name="t('settings', 'Send email')">
 			<NcCheckboxRadioSwitch type="switch"
@@ -81,8 +107,10 @@ import axios from '@nextcloud/axios'
 import NcAppSettingsDialog from '@nextcloud/vue/dist/Components/NcAppSettingsDialog.js'
 import NcAppSettingsSection from '@nextcloud/vue/dist/Components/NcAppSettingsSection.js'
 import NcCheckboxRadioSwitch from '@nextcloud/vue/dist/Components/NcCheckboxRadioSwitch.js'
+import NcNoteCard from '@nextcloud/vue/dist/Components/NcNoteCard.js'
 import NcSelect from '@nextcloud/vue/dist/Components/NcSelect.js'
 
+import { GroupSorting } from '../../constants/GroupManagement.ts'
 import { unlimitedQuota } from '../../utils/userUtils.ts'
 
 export default {
@@ -92,6 +120,7 @@ export default {
 		NcAppSettingsDialog,
 		NcAppSettingsSection,
 		NcCheckboxRadioSwitch,
+		NcNoteCard,
 		NcSelect,
 	},
 
@@ -110,6 +139,22 @@ export default {
 	},
 
 	computed: {
+		groupSorting: {
+			get() {
+				return this.$store.getters.getGroupSorting === GroupSorting.GroupName ? 'name' : 'member-count'
+			},
+			set(sorting) {
+				this.$store.commit('setGroupSorting', sorting === 'name' ? GroupSorting.GroupName : GroupSorting.UserCount)
+			},
+		},
+
+		/**
+		 * Admin has configured `sort_groups_by_name` in the system config
+		 */
+		isGroupSortingEnforced() {
+			return this.$store.getters.getServerData.forceSortGroupByName
+		},
+
 		isModalOpen: {
 			get() {
 				return this.open
@@ -261,3 +306,9 @@ export default {
 	},
 }
 </script>
+
+<style scoped lang="scss">
+fieldset {
+	font-weight: bold;
+}
+</style>

apps/settings/src/constants/GroupManagement.ts

@@ -0,0 +1,29 @@
+/**
+ * @copyright Copyright (c) 2024 Ferdinand Thiessen <opensource@fthiessen.de>
+ *
+ * @author Ferdinand Thiessen <opensource@fthiessen.de>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+/**
+ * https://github.com/nextcloud/server/blob/208e38e84e1a07a49699aa90dc5b7272d24489f0/lib/private/Group/MetaData.php#L34
+ */
+export enum GroupSorting {
+	UserCount = 1,
+	GroupName = 2
+}

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
 	},

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 */

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,
 

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')
+		})
+	})
+})