From fcf42c4f8623592b5c8a7c63c1a8ff482d71ee43 Mon Sep 17 00:00:00 2001
From: Christopher Ng <chrng8@gmail.com>
Date: Tue, 25 Mar 2025 14:31:44 -0700
Subject: [PATCH] fix(settings): Fix infinitely loading account management page
 with pagination of groups - Includes searching

Signed-off-by: Christopher Ng <chrng8@gmail.com>
---
 .../src/components/AppNavigationGroupList.vue | 212 ++++++++++++++++++
 .../settings/src/components/GroupListItem.vue |   1 +
 apps/settings/src/store/users.js              |  62 +++--
 apps/settings/src/views/UserManagement.vue    |   5 -
 .../src/views/UserManagementNavigation.vue    |  90 +-------
 apps/settings/src/views/user-types.d.ts       |  17 ++
 6 files changed, 282 insertions(+), 105 deletions(-)
 create mode 100644 apps/settings/src/components/AppNavigationGroupList.vue

diff --git a/apps/settings/src/components/AppNavigationGroupList.vue b/apps/settings/src/components/AppNavigationGroupList.vue
new file mode 100644
index 00000000000..d5362d6cf4c
--- /dev/null
+++ b/apps/settings/src/components/AppNavigationGroupList.vue
@@ -0,0 +1,212 @@
+<!--
+	- SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+	- SPDX-License-Identifier: AGPL-3.0-or-later
+-->
+
+<template>
+	<Fragment>
+		<NcAppNavigationCaption :name="t('settings', 'Groups')"
+			:disabled="loadingAddGroup"
+			:aria-label="loadingAddGroup ? t('settings', 'Creating group…') : t('settings', 'Create group')"
+			force-menu
+			is-heading
+			:open.sync="isAddGroupOpen">
+			<template v-if="isAdminOrDelegatedAdmin" #actionsTriggerIcon>
+				<NcLoadingIcon v-if="loadingAddGroup" />
+				<NcIconSvgWrapper v-else :path="mdiPlus" />
+			</template>
+			<template v-if="isAdminOrDelegatedAdmin" #actions>
+				<NcActionText>
+					<template #icon>
+						<NcIconSvgWrapper :path="mdiAccountGroup" />
+					</template>
+					{{ t('settings', 'Create group') }}
+				</NcActionText>
+				<NcActionInput :label="t('settings', 'Group name')"
+					data-cy-users-settings-new-group-name
+					:label-outside="false"
+					:disabled="loadingAddGroup"
+					:value.sync="newGroupName"
+					:error="hasAddGroupError"
+					:helper-text="hasAddGroupError ? t('settings', 'Please enter a valid group name') : ''"
+					@submit="createGroup" />
+			</template>
+		</NcAppNavigationCaption>
+
+		<NcAppNavigationSearch v-model="groupsSearchQuery"
+			:label="t('settings', 'Search groups…')" />
+
+		<p id="group-list-desc" class="hidden-visually">
+			{{ t('settings', 'List of groups. This list is not fully populated for performance reasons. The groups will be loaded as you navigate or search through the list.') }}
+		</p>
+		<NcAppNavigationList class="account-management__group-list"
+			aria-describedby="group-list-desc"
+			data-cy-users-settings-navigation-groups="custom">
+			<GroupListItem v-for="group in filteredGroups"
+				:id="group.id"
+				ref="groupListItems"
+				:key="group.id"
+				:active="selectedGroupDecoded === group.id"
+				:name="group.title"
+				:count="group.count" />
+			<div v-if="loadingGroups" role="note">
+				<NcLoadingIcon :name="t('settings', 'Loading groups…')" />
+			</div>
+		</NcAppNavigationList>
+	</Fragment>
+</template>
+
+<script setup lang="ts">
+import { computed, ref, watch, onBeforeMount } from 'vue'
+import { Fragment } from 'vue-frag'
+import { useRoute, useRouter } from 'vue-router/composables'
+import { useElementVisibility } from '@vueuse/core'
+import { showError } from '@nextcloud/dialogs'
+import { mdiAccountGroup, mdiPlus } from '@mdi/js'
+
+import NcActionInput from '@nextcloud/vue/components/NcActionInput'
+import NcActionText from '@nextcloud/vue/components/NcActionText'
+import NcAppNavigationCaption from '@nextcloud/vue/components/NcAppNavigationCaption'
+import NcAppNavigationList from '@nextcloud/vue/components/NcAppNavigationList'
+import NcAppNavigationSearch from '@nextcloud/vue/components/NcAppNavigationSearch'
+import NcIconSvgWrapper from '@nextcloud/vue/components/NcIconSvgWrapper'
+import NcLoadingIcon from '@nextcloud/vue/components/NcLoadingIcon'
+
+import GroupListItem from './GroupListItem.vue'
+
+import { useFormatGroups } from '../composables/useGroupsNavigation.ts'
+import { useStore } from '../store'
+import logger from '../logger.ts'
+
+interface Group {
+		id: string
+    displayname: string
+    usercount: number
+    disabled: number
+    canAdd: boolean
+    canRemove: boolean
+}
+
+const store = useStore()
+const route = useRoute()
+const router = useRouter()
+
+onBeforeMount(async () => {
+	await loadGroups()
+})
+
+/** Current active group in the view - this is URL encoded */
+const selectedGroup = computed(() => route.params?.selectedGroup)
+/** Current active group - URL decoded  */
+const selectedGroupDecoded = computed(() => selectedGroup.value ? decodeURIComponent(selectedGroup.value) : null)
+/** All available groups */
+const groups = computed(() => store.getters.getSortedGroups)
+/** User groups */
+const { userGroups } = useFormatGroups(groups)
+/** Server settings for current user */
+const settings = computed(() => store.getters.getServerData)
+/** True if the current user is a (delegated) admin */
+const isAdminOrDelegatedAdmin = computed(() => settings.value.isAdmin || settings.value.isDelegatedAdmin)
+
+/** True if the 'add-group' dialog is open - needed to be able to close it when the group is created */
+const isAddGroupOpen = ref(false)
+/** True if the group creation is in progress to show loading spinner and disable adding another one */
+const loadingAddGroup = ref(false)
+/** Error state for creating a new group */
+const hasAddGroupError = ref(false)
+/** Name of the group to create (used in the group creation dialog) */
+const newGroupName = ref('')
+
+/** True if groups are loading */
+const loadingGroups = ref(false)
+/** Search offset */
+const offset = ref(0)
+/** Search query for groups */
+const groupsSearchQuery = ref('')
+/** Filtered groups */
+const filteredGroups = computed(() => userGroups.value.filter((group) => {
+	return group.title.toLocaleLowerCase().includes(groupsSearchQuery.value.toLocaleLowerCase())
+}))
+
+const groupListItems = ref([])
+const lastGroupListItem = computed(() => {
+	return groupListItems.value
+		.findLast(component => component?.$vnode?.key === userGroups.value?.at(-1)?.id) // Order of refs is not guaranteed to match source array order
+		?.$refs?.listItem?.$el
+})
+const isLastGroupVisible = useElementVisibility(lastGroupListItem)
+watch(isLastGroupVisible, async () => {
+	if (!isLastGroupVisible.value) {
+		return
+	}
+	await loadGroups()
+})
+
+watch(groupsSearchQuery, async () => {
+	store.commit('resetGroups')
+	offset.value = 0
+	await loadGroups()
+})
+
+/**
+ * Load groups
+ */
+async function loadGroups() {
+	loadingGroups.value = true
+	try {
+		const { data } = await store.dispatch('searchGroups', {
+			search: groupsSearchQuery.value,
+			offset: offset.value,
+			limit: 25,
+		})
+		const groups: Group[] = data.ocs?.data?.groups ?? []
+		if (groups.length > 0) {
+			offset.value += 25
+		}
+		for (const group of groups) {
+			store.commit('addGroup', {
+				id: group.id,
+				name: group.displayname,
+				usercount: group.usercount,
+				disabled: group.disabled,
+				canAdd: group.canAdd,
+				canRemove: group.canRemove,
+			})
+		}
+	} catch (error) {
+		logger.error(t('settings', 'Failed to load groups'), { error })
+	}
+	loadingGroups.value = false
+}
+
+/**
+ * Create a new group
+ */
+async function createGroup() {
+	hasAddGroupError.value = false
+	const groupId = newGroupName.value.trim()
+	if (groupId === '') {
+		hasAddGroupError.value = true
+		return
+	}
+
+	isAddGroupOpen.value = false
+	loadingAddGroup.value = true
+
+	try {
+		await store.dispatch('addGroup', groupId)
+		await router.push({
+			name: 'group',
+			params: {
+				selectedGroup: encodeURIComponent(groupId),
+			},
+		})
+		const newGroupListItem = groupListItems.value.findLast(component => component?.$vnode?.key === groupId)
+		newGroupListItem?.$refs?.listItem?.$el?.scrollIntoView({ behavior: 'smooth', block: 'nearest' })
+		newGroupName.value = ''
+	} catch {
+		showError(t('settings', 'Failed to create group'))
+	}
+	loadingAddGroup.value = false
+}
+</script>
diff --git a/apps/settings/src/components/GroupListItem.vue b/apps/settings/src/components/GroupListItem.vue
index 19786507b48..65d46136ec1 100644
--- a/apps/settings/src/components/GroupListItem.vue
+++ b/apps/settings/src/components/GroupListItem.vue
@@ -29,6 +29,7 @@
 		</NcModal>
 
 		<NcAppNavigationItem :key="id"
+			ref="listItem"
 			:exact="true"
 			:name="name"
 			:to="{ name: 'group', params: { selectedGroup: encodeURIComponent(id) } }"
diff --git a/apps/settings/src/store/users.js b/apps/settings/src/store/users.js
index 61473528ba6..c997b04b40c 100644
--- a/apps/settings/src/store/users.js
+++ b/apps/settings/src/store/users.js
@@ -8,15 +8,21 @@ import { getCapabilities } from '@nextcloud/capabilities'
 import { parseFileSize } from '@nextcloud/files'
 import { showError } from '@nextcloud/dialogs'
 import { generateOcsUrl, generateUrl } from '@nextcloud/router'
+import { loadState } from '@nextcloud/initial-state'
 import axios from '@nextcloud/axios'
 
 import { GroupSorting } from '../constants/GroupManagement.ts'
 import api from './api.js'
 import logger from '../logger.ts'
 
+const usersSettings = loadState('settings', 'usersSettings', {})
+
 const localStorage = getBuilder('settings').persist(true).build()
 
 const defaults = {
+	/**
+	 * @type {import('../views/user-types').IGroup}
+	 */
 	group: {
 		id: '',
 		name: '',
@@ -29,14 +35,14 @@ const defaults = {
 
 const state = {
 	users: [],
-	groups: [],
-	orderBy: GroupSorting.UserCount,
+	groups: [...usersSettings.systemGroups],
+	orderBy: usersSettings.sortGroups,
 	minPasswordLength: 0,
 	usersOffset: 0,
 	usersLimit: 25,
 	disabledUsersOffset: 0,
 	disabledUsersLimit: 25,
-	userCount: 0,
+	userCount: usersSettings.userCount,
 	showConfig: {
 		showStoragePath: localStorage.getItem('account_settings__showStoragePath') === 'true',
 		showUserBackend: localStorage.getItem('account_settings__showUserBackend') === 'true',
@@ -63,21 +69,17 @@ const mutations = {
 	setPasswordPolicyMinLength(state, length) {
 		state.minPasswordLength = length !== '' ? length : 0
 	},
-	initGroups(state, { groups, orderBy, userCount }) {
-		state.groups = groups.map(group => Object.assign({}, defaults.group, group))
-		state.orderBy = orderBy
-		state.userCount = userCount
-	},
-	addGroup(state, { gid, displayName }) {
+	/**
+	 * @param {object} state store state
+	 * @param {import('../views/user-types.js').IGroup} newGroup new group
+	 */
+	addGroup(state, newGroup) {
 		try {
-			if (typeof state.groups.find((group) => group.id === gid) !== 'undefined') {
+			if (typeof state.groups.find((group) => group.id === newGroup.id) !== 'undefined') {
 				return
 			}
 			// extend group to default values
-			const group = Object.assign({}, defaults.group, {
-				id: gid,
-				name: displayName,
-			})
+			const group = Object.assign({}, defaults.group, newGroup)
 			state.groups.unshift(group)
 		} catch (e) {
 			console.error('Can\'t create group', e)
@@ -215,6 +217,15 @@ const mutations = {
 		state.disabledUsersOffset = 0
 	},
 
+	/**
+	 * Reset group list
+	 *
+	 * @param {object} state the store state
+	 */
+	resetGroups(state) {
+		state.groups = [...usersSettings.systemGroups]
+	},
+
 	setShowConfig(state, { key, value }) {
 		localStorage.setItem(`account_settings__${key}`, JSON.stringify(value))
 		state.showConfig[key] = value
@@ -312,6 +323,25 @@ const actions = {
 		})
 	},
 
+	/**
+	 * search groups
+	 *
+	 * @param {object} context Store context
+	 * @param {object} options Options
+	 * @param {string} options.search Search query
+	 * @param {number} options.offset List offset
+	 * @param {number} options.limit List limit
+	 * @return {Promise}
+	 */
+	searchGroups(context, { search, offset, limit }) {
+		return api.get(generateOcsUrl('cloud/groups/details?search={search}&offset={offset}&limit={limit}', { search, offset, limit }))
+			.catch((error) => {
+				if (!axios.isCancel(error)) {
+					context.commit('API_FAILURE', error)
+				}
+			})
+	},
+
 	/**
 	 * Get user details
 	 *
@@ -444,7 +474,7 @@ const actions = {
 			.then((response) => {
 				if (Object.keys(response.data.ocs.data.groups).length > 0) {
 					response.data.ocs.data.groups.forEach(function(group) {
-						context.commit('addGroup', { gid: group, displayName: group })
+						context.commit('addGroup', { id: group, name: group })
 					})
 					return true
 				}
@@ -511,7 +541,7 @@ const actions = {
 		return api.requireAdmin().then((response) => {
 			return api.post(generateOcsUrl('cloud/groups'), { groupid: gid })
 				.then((response) => {
-					context.commit('addGroup', { gid, displayName: gid })
+					context.commit('addGroup', { id: gid, name: gid })
 					return { gid, displayName: gid }
 				})
 				.catch((error) => { throw error })
diff --git a/apps/settings/src/views/UserManagement.vue b/apps/settings/src/views/UserManagement.vue
index d09f0a76f81..9ab76f921a0 100644
--- a/apps/settings/src/views/UserManagement.vue
+++ b/apps/settings/src/views/UserManagement.vue
@@ -55,11 +55,6 @@ export default defineComponent({
 	},
 
 	beforeMount() {
-		this.$store.commit('initGroups', {
-			groups: this.$store.getters.getServerData.groups,
-			orderBy: this.$store.getters.getServerData.sortGroups,
-			userCount: this.$store.getters.getServerData.userCount,
-		})
 		this.$store.dispatch('getPasswordPolicyMinLength')
 	},
 
diff --git a/apps/settings/src/views/UserManagementNavigation.vue b/apps/settings/src/views/UserManagementNavigation.vue
index f4c8c646ae2..7588429fbce 100644
--- a/apps/settings/src/views/UserManagementNavigation.vue
+++ b/apps/settings/src/views/UserManagementNavigation.vue
@@ -79,42 +79,7 @@
 			</NcAppNavigationItem>
 		</NcAppNavigationList>
 
-		<NcAppNavigationCaption :name="t('settings', 'Groups')"
-			:disabled="loadingAddGroup"
-			:aria-label="loadingAddGroup ? t('settings', 'Creating group…') : t('settings', 'Create group')"
-			force-menu
-			is-heading
-			:open.sync="isAddGroupOpen">
-			<template v-if="isAdminOrDelegatedAdmin" #actionsTriggerIcon>
-				<NcLoadingIcon v-if="loadingAddGroup" />
-				<NcIconSvgWrapper v-else :path="mdiPlus" />
-			</template>
-			<template v-if="isAdminOrDelegatedAdmin" #actions>
-				<NcActionText>
-					<template #icon>
-						<NcIconSvgWrapper :path="mdiAccountGroup" />
-					</template>
-					{{ t('settings', 'Create group') }}
-				</NcActionText>
-				<NcActionInput :label="t('settings', 'Group name')"
-					data-cy-users-settings-new-group-name
-					:label-outside="false"
-					:disabled="loadingAddGroup"
-					:value.sync="newGroupName"
-					:error="hasAddGroupError"
-					:helper-text="hasAddGroupError ? t('settings', 'Please enter a valid group name') : ''"
-					@submit="createGroup" />
-			</template>
-		</NcAppNavigationCaption>
-
-		<NcAppNavigationList class="account-management__group-list" data-cy-users-settings-navigation-groups="custom">
-			<GroupListItem v-for="group in userGroups"
-				:id="group.id"
-				:key="group.id"
-				:active="selectedGroupDecoded === group.id"
-				:name="group.title"
-				:count="group.count" />
-		</NcAppNavigationList>
+		<AppNavigationGroupList />
 
 		<template #footer>
 			<NcButton class="account-management__settings-toggle"
@@ -131,31 +96,26 @@
 </template>
 
 <script setup lang="ts">
-import { mdiAccount, mdiAccountGroup, mdiAccountOff, mdiCog, mdiPlus, mdiShieldAccount, mdiHistory } from '@mdi/js'
-import { showError } from '@nextcloud/dialogs'
+import { mdiAccount, mdiAccountOff, mdiCog, mdiPlus, mdiShieldAccount, mdiHistory } from '@mdi/js'
 import { translate as t } from '@nextcloud/l10n'
 import { computed, ref } from 'vue'
 
-import NcActionInput from '@nextcloud/vue/components/NcActionInput'
-import NcActionText from '@nextcloud/vue/components/NcActionText'
 import NcAppNavigation from '@nextcloud/vue/components/NcAppNavigation'
-import NcAppNavigationCaption from '@nextcloud/vue/components/NcAppNavigationCaption'
 import NcAppNavigationItem from '@nextcloud/vue/components/NcAppNavigationItem'
 import NcAppNavigationList from '@nextcloud/vue/components/NcAppNavigationList'
 import NcAppNavigationNew from '@nextcloud/vue/components/NcAppNavigationNew'
 import NcButton from '@nextcloud/vue/components/NcButton'
 import NcCounterBubble from '@nextcloud/vue/components/NcCounterBubble'
 import NcIconSvgWrapper from '@nextcloud/vue/components/NcIconSvgWrapper'
-import NcLoadingIcon from '@nextcloud/vue/components/NcLoadingIcon'
 
-import GroupListItem from '../components/GroupListItem.vue'
 import UserSettingsDialog from '../components/Users/UserSettingsDialog.vue'
+import AppNavigationGroupList from '../components/AppNavigationGroupList.vue'
+
 import { useStore } from '../store'
-import { useRoute, useRouter } from 'vue-router/composables'
+import { useRoute } from 'vue-router/composables'
 import { useFormatGroups } from '../composables/useGroupsNavigation'
 
 const route = useRoute()
-const router = useRouter()
 const store = useStore()
 
 /** State of the 'new-account' dialog */
@@ -170,51 +130,13 @@ const selectedGroupDecoded = computed(() => selectedGroup.value ? decodeURICompo
 const userCount = computed(() => store.getters.getUserCount)
 /** All available groups */
 const groups = computed(() => store.getters.getSortedGroups)
-const { adminGroup, recentGroup, disabledGroup, userGroups } = useFormatGroups(groups)
+const { adminGroup, recentGroup, disabledGroup } = useFormatGroups(groups)
 
 /** Server settings for current user */
 const settings = computed(() => store.getters.getServerData)
 /** True if the current user is a (delegated) admin */
 const isAdminOrDelegatedAdmin = computed(() => settings.value.isAdmin || settings.value.isDelegatedAdmin)
 
-/** True if the 'add-group' dialog is open - needed to be able to close it when the group is created */
-const isAddGroupOpen = ref(false)
-/** True if the group creation is in progress to show loading spinner and disable adding another one */
-const loadingAddGroup = ref(false)
-/** Error state for creating a new group */
-const hasAddGroupError = ref(false)
-/** Name of the group to create (used in the group creation dialog) */
-const newGroupName = ref('')
-
-/**
- * Create a new group
- */
-async function createGroup() {
-	hasAddGroupError.value = false
-	const groupId = newGroupName.value.trim()
-	if (groupId === '') {
-		hasAddGroupError.value = true
-		return
-	}
-
-	isAddGroupOpen.value = false
-	loadingAddGroup.value = true
-
-	try {
-		await store.dispatch('addGroup', groupId)
-		await router.push({
-			name: 'group',
-			params: {
-				selectedGroup: encodeURIComponent(groupId),
-			},
-		})
-		newGroupName.value = ''
-	} catch {
-		showError(t('settings', 'Failed to create group'))
-	}
-	loadingAddGroup.value = false
-}
-
 /**
  * Open the new-user form dialog
  */
diff --git a/apps/settings/src/views/user-types.d.ts b/apps/settings/src/views/user-types.d.ts
index b8cd30e53d9..21c63a13b03 100644
--- a/apps/settings/src/views/user-types.d.ts
+++ b/apps/settings/src/views/user-types.d.ts
@@ -3,7 +3,14 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 export interface IGroup {
+	/**
+	 * Id
+	 */
 	id: string
+
+	/**
+	 * Display name
+	 */
 	name: string
 
 	/**
@@ -15,4 +22,14 @@ export interface IGroup {
 	 * Number of disabled users
 	 */
 	disabled: number
+
+	/**
+	 * True if users can be added to this group
+	 */
+	canAdd?: boolean
+
+	/**
+	 * True if users can be removed from this group
+	 */
+	canRemove?: boolean
 }