Merge pull request #53050 from nextcloud/backport/52833/stable30

[stable30] fix(settings): Send update request when clearing user manager

Author: nfebe

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

@@ -249,16 +249,17 @@
 					data-cy-user-list-input-manager
 					:data-loading="loading.manager || undefined"
 					:input-id="'manager' + uniqueId"
-					:close-on-select="true"
 					:disabled="isLoadingField"
-					:append-to-body="false"
 					:loading="loadingPossibleManagers || loading.manager"
-					label="displayname"
 					:options="possibleManagers"
 					:placeholder="managerLabel"
+					label="displayname"
+					:filterable="false"
+					:internal-search="false"
+					:clearable="true"
 					@open="searchInitialUserManager"
 					@search="searchUserManager"
-					@option:selected="updateUserManager" />
+					@update:model-value="updateUserManager" />
 			</template>
 			<span v-else-if="!isObfuscated">
 				{{ user.manager }}
@@ -508,7 +509,6 @@ export default {
 			return this.languages[0].languages.concat(this.languages[1].languages)
 		},
 	},
-
 	async beforeMount() {
 		if (this.user.manager) {
 			await this.initManager(this.user.manager)
@@ -629,11 +629,12 @@ export default {
 			})
 		},
 
-		async updateUserManager(manager) {
-			if (manager === null) {
-				this.currentManager = ''
-			}
+		async updateUserManager() {
 			this.loading.manager = true
+
+			// Store the current manager before making changes
+			const previousManager = this.user.manager
+
 			try {
 				await this.$store.dispatch('setUserData', {
 					userid: this.user.id,
@@ -643,7 +644,10 @@ export default {
 			} catch (error) {
 				// TRANSLATORS This string describes a line manager in the context of an organization
 				showError(t('settings', 'Failed to update line manager'))
-				console.error(error)
+				logger.error('Failed to update manager:', { error })
+
+				// Revert to the previous manager in the UI on error
+				this.currentManager = previousManager
 			} finally {
 				this.loading.manager = false
 			}

apps/settings/src/store/users.js

@@ -766,24 +766,25 @@ const actions = {
 	 */
 	async setUserData(context, { userid, key, value }) {
 		const allowedEmpty = ['email', 'displayname', 'manager']
-		if (['email', 'language', 'quota', 'displayname', 'password', 'manager'].indexOf(key) !== -1) {
-			// We allow empty email or displayname
-			if (typeof value === 'string'
-				&& (
-					(allowedEmpty.indexOf(key) === -1 && value.length > 0)
-					|| allowedEmpty.indexOf(key) !== -1
-				)
-			) {
-				try {
-					await api.requireAdmin()
-					await api.put(generateOcsUrl('cloud/users/{userid}', { userid }), { key, value })
-					return context.commit('setUserData', { userid, key, value })
-				} catch (error) {
-					context.commit('API_FAILURE', { userid, error })
-				}
-			}
+		const validKeys = ['email', 'language', 'quota', 'displayname', 'password', 'manager']
+
+		if (!validKeys.includes(key)) {
+			throw new Error('Invalid request data')
+		}
+
+		// If value is empty and the key doesn't allow empty values, throw error
+		if (value === '' && !allowedEmpty.includes(key)) {
+			throw new Error('Value cannot be empty for this field')
+		}
+
+		try {
+			await api.requireAdmin()
+			await api.put(generateOcsUrl('cloud/users/{userid}', { userid }), { key, value })
+			return context.commit('setUserData', { userid, key, value })
+		} catch (error) {
+			context.commit('API_FAILURE', { userid, error })
+			throw error
 		}
-		return Promise.reject(new Error('Invalid request data'))
 	},
 
 	/**

cypress/e2e/settings/users_manager.cy.ts

@@ -0,0 +1,121 @@
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+import { User } from '@nextcloud/cypress'
+import { getUserListRow, handlePasswordConfirmation, toggleEditButton, waitLoading } from './usersUtils'
+import { clearState } from '../../support/commonUtils'
+
+const admin = new User('admin', 'admin')
+
+describe('Settings: User Manager Management', function() {
+	let user: User
+	let manager: User
+
+	beforeEach(function() {
+		clearState()
+		cy.createRandomUser().then(($user) => {
+			manager = $user
+			return cy.createRandomUser()
+		}).then(($user) => {
+			user = $user
+			cy.login(admin)
+			cy.intercept('PUT', `/ocs/v2.php/cloud/users/${user.userId}*`).as('updateUser')
+		})
+	})
+
+	it('Can assign and remove a manager through the UI', function() {
+		cy.visit('/settings/users')
+
+		toggleEditButton(user, true)
+
+		// Scroll to manager cell and wait for it to be visible
+		getUserListRow(user.userId)
+			.find('[data-cy-user-list-cell-manager]')
+			.scrollIntoView()
+			.should('be.visible')
+
+		// Assign a manager
+		getUserListRow(user.userId).find('[data-cy-user-list-cell-manager]').within(() => {
+			// Verify no manager is set initially
+			cy.get('.vs__selected').should('not.exist')
+
+			// Open the dropdown menu
+			cy.get('[role="combobox"]').click({ force: true })
+
+			// Wait for the dropdown to be visible and initialized
+			waitLoading('[data-cy-user-list-input-manager]')
+
+			// Type the manager's username to search
+			cy.get('input[type="search"]').type(manager.userId, { force: true })
+
+			// Wait for the search results to load
+			waitLoading('[data-cy-user-list-input-manager]')
+		})
+
+		// Now select the manager from the filtered results
+		// Since the dropdown is floating, we need to search globally
+		cy.get('.vs__dropdown-menu').find('li').contains('span', manager.userId).should('be.visible').click({ force: true })
+
+		// Handle password confirmation if needed
+		handlePasswordConfirmation(admin.password)
+
+		// Verify the manager is selected in the UI
+		cy.get('.vs__selected').should('exist').and('contain.text', manager.userId)
+
+		// Verify the PUT request was made to set the manager
+		cy.wait('@updateUser').then((interception) => {
+			// Verify the request URL and body
+			expect(interception.request.url).to.match(/\/cloud\/users\/.+/)
+			expect(interception.request.body).to.deep.equal({
+				key: 'manager',
+				value: manager.userId
+			})
+			expect(interception.response?.statusCode).to.equal(200)
+		})
+
+		// Wait for the save to complete
+		waitLoading('[data-cy-user-list-input-manager]')
+
+		// Verify the manager is set in the backend
+		cy.getUserData(user).then(($result) => {
+			expect($result.body).to.contain(`<manager>${manager.userId}</manager>`)
+		})
+
+		// Now remove the manager
+		getUserListRow(user.userId).find('[data-cy-user-list-cell-manager]').within(() => {
+			// Clear the manager selection
+			cy.get('.vs__clear').click({ force: true })
+
+			// Verify the manager is cleared in the UI
+			cy.get('.vs__selected').should('not.exist')
+
+			// Handle password confirmation if needed
+			handlePasswordConfirmation(admin.password)
+		})
+
+		// Verify the PUT request was made to clear the manager
+		cy.wait('@updateUser').then((interception) => {
+			// Verify the request URL and body
+			expect(interception.request.url).to.match(/\/cloud\/users\/.+/)
+			expect(interception.request.body).to.deep.equal({
+				key: 'manager',
+				value: '',
+			})
+			expect(interception.response?.statusCode).to.equal(200)
+		})
+
+		// Wait for the save to complete
+		waitLoading('[data-cy-user-list-input-manager]')
+
+		// Verify the manager is cleared in the backend
+		cy.getUserData(user).then(($result) => {
+			expect($result.body).to.not.contain(`<manager>${manager.userId}</manager>`)
+			expect($result.body).to.contain('<manager></manager>')
+		})
+
+		// Finish editing the user
+		toggleEditButton(user, false)
+	})
+})

cypress/e2e/settings/users_modify.cy.ts

@@ -181,47 +181,6 @@ describe('Settings: Change user properties', function() {
 		})
 	})
 
-	it('Can set manager of a user', function() {
-		// create the manager
-		let manager: User
-		cy.createRandomUser().then(($user) => { manager = $user })
-
-		// open the User settings as admin
-		cy.login(admin)
-		cy.visit('/settings/users')
-
-		// toggle edit button into edit mode
-		toggleEditButton(user, true)
-
-		getUserListRow(user.userId)
-			.find('[data-cy-user-list-cell-manager]')
-			.scrollIntoView()
-
-		getUserListRow(user.userId).find('[data-cy-user-list-cell-manager]').within(() => {
-			// see that the user has no manager
-			cy.get('.vs__selected').should('not.exist')
-			// Open the dropdown menu
-			cy.get('[role="combobox"]').click({ force: true })
-			// select the manager
-			cy.contains('li', manager.userId).click({ force: true })
-
-			// Handle password confirmation on time out
-			handlePasswordConfirmation(admin.password)
-
-			// see that the user has a manager set
-			cy.get('.vs__selected').should('exist').and('contain.text', manager.userId)
-		})
-
-		// see that the changes are loading
-		waitLoading('[data-cy-user-list-input-manager]')
-
-		// finish editing the user
-		toggleEditButton(user, false)
-
-		// validate the manager is set
-		cy.getUserData(user).then(($result) => expect($result.body).to.contain(`<manager>${manager.userId}</manager>`))
-	})
-
 	it('Can make user a subadmin of a group', function() {
 		// create a group
 		const groupName = 'userstestgroup'