refactor(account-management): Separate group views from internal views

Some refactoring of the route handling for the account management.
We separate internal views like *disabled* or *recent* accounts from
group views like *admin* or *custom-group-id*.

The new URL looks like:
`/settings/users/VIEW(/GROUP)` for example a real group like `admin` has
this new URL: `/settings/users/group/admin`.
While internal views like *recent* users look like:
`/settings/users/recent`.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

apps/settings/src/components/GroupListItem.vue

@@ -29,9 +29,9 @@
 		</NcModal>
 
 		<NcAppNavigationItem :key="id"
-			:exact="true"
+			exact
 			:name="name"
-			:to="{ name: 'group', params: { selectedGroup: encodeURIComponent(id) } }"
+			:to="{ name: 'group', params: { view: 'group', selectedGroup: encodeURIComponent(id) } }"
 			:loading="loadingRenameGroup"
 			:menu-open="openGroupMenu"
 			@update:menuOpen="handleGroupMenuOpen">
@@ -45,7 +45,7 @@
 				</NcCounterBubble>
 			</template>
 			<template #actions>
-				<NcActionInput v-if="id !== 'admin' && id !== 'disabled' && (settings.isAdmin || settings.isDelegatedAdmin)"
+				<NcActionInput v-if="settings.isAdmin || settings.isDelegatedAdmin"
 					ref="displayNameInput"
 					:trailing-button-label="t('settings', 'Submit')"
 					type="text"
@@ -56,7 +56,7 @@
 						<Pencil :size="20" />
 					</template>
 				</NcActionInput>
-				<NcActionButton v-if="id !== 'admin' && id !== 'disabled' && (settings.isAdmin || settings.isDelegatedAdmin)"
+				<NcActionButton v-if="settings.isAdmin || settings.isDelegatedAdmin"
 					@click="showRemoveGroupModal = true">
 					<template #icon>
 						<Delete :size="20" />

apps/settings/src/components/UserList.vue

@@ -61,7 +61,7 @@
 
 <script>
 import { mdiAccountGroup } from '@mdi/js'
-import { showError } from '@nextcloud/dialogs'
+import { showError, showWarning } from '@nextcloud/dialogs'
 import { subscribe, unsubscribe } from '@nextcloud/event-bus'
 import { Fragment } from 'vue-frag'
 
@@ -117,6 +117,10 @@ export default {
 			type: Array,
 			default: () => [],
 		},
+		view: {
+			type: String,
+			required: true,
+		},
 	},
 
 	setup() {
@@ -166,7 +170,7 @@ export default {
 		},
 
 		filteredUsers() {
-			if (this.selectedGroup === 'disabled') {
+			if (this.view === 'disabled') {
 				return this.users.filter(user => user.enabled === false)
 			}
 			return this.users.filter(user => user.enabled !== false)
@@ -198,22 +202,6 @@ export default {
 			return quotaPreset
 		},
 
-		usersOffset() {
-			return this.$store.getters.getUsersOffset
-		},
-
-		usersLimit() {
-			return this.$store.getters.getUsersLimit
-		},
-
-		disabledUsersOffset() {
-			return this.$store.getters.getDisabledUsersOffset
-		},
-
-		disabledUsersLimit() {
-			return this.$store.getters.getDisabledUsersLimit
-		},
-
 		usersCount() {
 			return this.users.length
 		},
@@ -231,16 +219,24 @@ export default {
 				},
 			]
 		},
+
+		userFilter() {
+			return [this.view, this.selectedGroup]
+		},
 	},
 
 	watch: {
 		// watch url change and group select
-		async selectedGroup(val) {
+		async userFilter() {
 			this.isInitialLoad = true
 			// if selected is the disabled group but it's empty
 			await this.redirectIfDisabled()
 			this.$store.commit('resetUsers')
 			await this.loadUsers()
+			this.setNewUserDefaultGroup(this.selectedGroup)
+		},
+
+		selectedGroup(val) {
 			this.setNewUserDefaultGroup(val)
 		},
 
@@ -255,7 +251,7 @@ export default {
 
 	async mounted() {
 		if (!this.settings.canChangePassword) {
-			OC.Notification.showTemporary(t('settings', 'Password change is disabled because the master key is disabled'))
+			showWarning(t('settings', 'Password change is disabled because the master key is disabled'))
 		}
 
 		/**
@@ -286,24 +282,23 @@ export default {
 		},
 
 		async loadUsers() {
+			logger.debug('Loading users', { view: this.view, group: this.selectedGroup })
 			this.loading.users = true
 			try {
-				if (this.selectedGroup === 'disabled') {
-					await this.$store.dispatch('getDisabledUsers', {
-						offset: this.disabledUsersOffset,
-						limit: this.disabledUsersLimit,
+				if (this.view === 'all') {
+					await this.$store.dispatch('getUsers', {
 						search: this.searchQuery,
 					})
-				} else if (this.selectedGroup === '__nc_internal_recent') {
+				} else if (this.view === 'disabled') {
+					await this.$store.dispatch('getDisabledUsers', {
+						search: this.searchQuery,
+					})
+				} else if (this.view === 'recent') {
 					await this.$store.dispatch('getRecentUsers', {
-						offset: this.usersOffset,
-						limit: this.usersLimit,
 						search: this.searchQuery,
 					})
 				} else {
 					await this.$store.dispatch('getUsers', {
-						offset: this.usersOffset,
-						limit: this.usersLimit,
 						group: this.selectedGroup,
 						search: this.searchQuery,
 					})
@@ -386,11 +381,11 @@ export default {
 		 */
 		async redirectIfDisabled() {
 			const allGroups = this.$store.getters.getGroups
-			if (this.selectedGroup === 'disabled'
-						&& allGroups.findIndex(group => group.id === 'disabled' && group.usercount === 0) > -1) {
+			if (this.view === 'disabled'
+				&& allGroups.findIndex(group => group.id === 'disabled' && group.usercount === 0) > -1
+			) {
 				// disabled group is empty, redirection to all users
-				this.$router.push({ name: 'users' })
-				await this.loadUsers()
+				this.$router.replace({ name: 'users' })
 			}
 		},
 	},

apps/settings/src/router/index.ts

@@ -19,4 +19,23 @@ const router = new Router({
 	routes,
 })
 
+const ALL_VIEWS = [
+	'all',
+	'disabled',
+	'group',
+	'recent',
+]
+
+router.beforeEach((to, from, next) => {
+	// make sure old URLs without the `/group/` part keep working
+	if (to.name === 'users-view' && !ALL_VIEWS.includes(to.params.view)) {
+		return next({ name: 'group', params: { selectedGroup: to.params.view } })
+	}
+	// if there is no group selected redirect to all accounts
+	if (to.name === 'users-view' && to.params.view === 'group' && !to.params.selectedGroup) {
+		return next({ name: 'users-view', params: { view: 'all' } })
+	}
+	next()
+})
+
 export default router

apps/settings/src/router/routes.ts

@@ -24,11 +24,23 @@ const routes: RouteConfig[] = [
 			navigation: UserManagementNavigation,
 		},
 		props: true,
+
+		redirect: {
+			name: 'users-view',
+			params: {
+				view: 'all',
+			},
+		},
+
 		children: [
 			{
-				path: ':selectedGroup',
+				path: ':view(group)/:selectedGroup',
 				name: 'group',
 			},
+			{
+				path: ':view',
+				name: 'users-view',
+			},
 		],
 	},
 	{

apps/settings/src/store/users.js

@@ -338,12 +338,18 @@ const actions = {
 	 * @param {string} options.group Get users from group
 	 * @return {Promise}
 	 */
-	getUsers(context, { offset, limit, search, group }) {
+	getUsers(context, options) {
+		let { offset, limit, search, group } = {
+			offset: context.getters.getUsersOffset,
+			limit: context.getters.getUsersLimit,
+			search: '',
+			...options,
+		}
+
 		if (searchRequestCancelSource) {
 			searchRequestCancelSource.cancel('Operation canceled by another search request.')
 		}
 		searchRequestCancelSource = CancelToken.source()
-		search = typeof search === 'string' ? search : ''
 
 		/**
 		 * Adding filters in the search bar such as in:files, in:users, etc.
@@ -398,7 +404,12 @@ const actions = {
 	 * @param {string} options.search Search query
 	 * @return {Promise<number>}
 	 */
-	async getRecentUsers(context, { offset, limit, search }) {
+	async getRecentUsers(context, options) {
+		const { offset, limit, search } = {
+			limit: context.getters.getUsersLimit,
+			offset: context.getters.getUsersOffset,
+			...options,
+		}
 		const url = generateOcsUrl('cloud/users/recent?offset={offset}&limit={limit}&search={search}', { offset, limit, search })
 		try {
 			const response = await api.get(url)
@@ -419,10 +430,16 @@ const actions = {
 	 * @param {object} options destructuring object
 	 * @param {number} options.offset List offset to request
 	 * @param {number} options.limit List number to return from offset
-	 * @param options.search
+	 * @param {string} options.search
 	 * @return {Promise<number>}
 	 */
-	async getDisabledUsers(context, { offset, limit, search }) {
+	async getDisabledUsers(context, options) {
+		const { offset, limit, search } = {
+			limit: context.getters.getDisabledUsersLimit,
+			offset: context.getters.getDisabledUsersOffset,
+			...options,
+		}
+
 		const url = generateOcsUrl('cloud/users/disabled?offset={offset}&limit={limit}&search={search}', { offset, limit, search })
 		try {
 			const response = await api.get(url)

apps/settings/src/views/UserManagement.vue

@@ -5,8 +5,9 @@
 
 <template>
 	<NcAppContent :page-heading="pageHeading">
-		<UserList :selected-group="selectedGroupDecoded"
-			:external-actions="externalActions" />
+		<UserList :external-actions="externalActions"
+			:selected-group="selectedGroupDecoded"
+			:view="currentView" />
 	</NcAppContent>
 </template>
 
@@ -34,15 +35,23 @@ export default defineComponent({
 	},
 
 	computed: {
+		currentView() {
+			return this.$route.params.view ?? 'all'
+		},
+
 		pageHeading() {
-			if (this.selectedGroupDecoded === null) {
+			if (this.currentView === 'all') {
 				return t('settings', 'All accounts')
+			} else if (this.currentView === 'recent') {
+				return t('settings', 'Recently active accounts')
+			} else if (this.currentView === 'disabled') {
+				return t('settings', 'Disabled acounts')
+			} else {
+				if (this.selectedGroupDecoded === 'admin') {
+					return t('settings', 'Admins')
+				}
+				return t('settings', 'Account group: {group}', { group: this.selectedGroupDecoded })
 			}
-			const matchHeading = {
-				admin: t('settings', 'Admins'),
-				disabled: t('settings', 'Disabled accounts'),
-			}
-			return matchHeading[this.selectedGroupDecoded] ?? t('settings', 'Account group: {group}', { group: this.selectedGroupDecoded })
 		},
 
 		selectedGroup() {

apps/settings/src/views/UserManagementNavigation.vue

@@ -16,15 +16,15 @@
 
 		<NcAppNavigationList class="account-management__system-list"
 			data-cy-users-settings-navigation-groups="system">
-			<NcAppNavigationItem id="everyone"
+			<NcAppNavigationItem id="view-all"
 				:exact="true"
 				:name="t('settings', 'All accounts')"
-				:to="{ name: 'users' }">
+				:to="{ name: 'users-view', params: { view: 'all' } }">
 				<template #icon>
 					<NcIconSvgWrapper :path="mdiAccount" />
 				</template>
 				<template #counter>
-					<NcCounterBubble v-if="userCount" :type="!selectedGroupDecoded ? 'highlighted' : undefined">
+					<NcCounterBubble v-if="userCount" :type="currentView === 'all' ? 'highlighted' : undefined">
 						{{ userCount }}
 					</NcCounterBubble>
 				</template>
@@ -32,9 +32,9 @@
 
 			<NcAppNavigationItem v-if="settings.isAdmin"
 				id="admin"
-				:exact="true"
+				exact
 				:name="t('settings', 'Admins')"
-				:to="{ name: 'group', params: { selectedGroup: 'admin' } }">
+				:to="{ name: 'group', params: { view: 'group', selectedGroup: 'admin' } }">
 				<template #icon>
 					<NcIconSvgWrapper :path="mdiShieldAccount" />
 				</template>
@@ -47,16 +47,16 @@
 			</NcAppNavigationItem>
 
 			<NcAppNavigationItem v-if="isAdminOrDelegatedAdmin"
-				id="recent"
+				id="view-recent"
 				:exact="true"
 				:name="t('settings', 'Recently active')"
-				:to="{ name: 'group', params: { selectedGroup: '__nc_internal_recent' } }">
+				:to="{ name: 'users-view', params: { view: 'recent' } }">
 				<template #icon>
 					<NcIconSvgWrapper :path="mdiHistory" />
 				</template>
 				<template #counter>
 					<NcCounterBubble v-if="recentGroup?.usercount"
-						:type="selectedGroupDecoded === '__nc_internal_recent' ? 'highlighted' : undefined">
+						:type="currentView === 'recent' ? 'highlighted' : undefined">
 						{{ recentGroup.usercount }}
 					</NcCounterBubble>
 				</template>
@@ -64,15 +64,15 @@
 
 			<!-- Hide the disabled if none, if we don't have the data (-1) show it -->
 			<NcAppNavigationItem v-if="disabledGroup && (disabledGroup.usercount > 0 || disabledGroup.usercount === -1)"
-				id="disabled"
+				id="view-disabled"
 				:exact="true"
 				:name="t('settings', 'Disabled accounts')"
-				:to="{ name: 'group', params: { selectedGroup: 'disabled' } }">
+				:to="{ name: 'users-view', params: { view: 'disabled' } }">
 				<template #icon>
 					<NcIconSvgWrapper :path="mdiAccountOff" />
 				</template>
 				<template v-if="disabledGroup.usercount > 0" #counter>
-					<NcCounterBubble :type="selectedGroupDecoded === 'disabled' ? 'highlighted' : undefined">
+					<NcCounterBubble :type="currentView === 'disabled' ? 'highlighted' : undefined">
 						{{ disabledGroup.usercount }}
 					</NcCounterBubble>
 				</template>
@@ -161,6 +161,8 @@ const store = useStore()
 /** State of the 'new-account' dialog */
 const isDialogOpen = ref(false)
 
+const currentView = computed(() => route.params.view ?? 'all')
+
 /** Current active group in the view - this is URL encoded */
 const selectedGroup = computed(() => route.params?.selectedGroup)
 /** Current active group - URL decoded  */