fix: users.update API call erases other customFields

.changeset/wild-kiwis-cover.md

@@ -0,0 +1,5 @@
+---
+"@rocket.chat/meteor": patch
+---
+
+Fixes a bug where the `/api/v1/users.update` API call was replacing the entire `customFields` object instead of merging only the specified properties. The fix ensures that when updating custom fields, existing values are preserved while only specified fields are updated or added.

apps/meteor/app/lib/server/functions/saveCustomFieldsWithoutValidation.ts

@@ -32,22 +32,20 @@ export const saveCustomFieldsWithoutValidation = async function (
 	// configured custom fields in setting
 	const customFieldsMeta = getCustomFieldsMeta(customFieldsSetting);
 
-	const customFields: Record<string, any> = Object.keys(customFieldsMeta).reduce(
-		(acc, currentValue) => ({
-			...acc,
-			[currentValue]: formData[currentValue],
-		}),
-		{},
+	const customFields = Object.fromEntries(
+		Object.keys(formData)
+			.filter((key) => Object.hasOwn(customFieldsMeta, key))
+			.map((key) => [key, formData[key]]),
 	);
 
 	const { _updater, session } = options || {};
 
 	const updater = _updater || Users.getUpdater();
-
-	updater.set('customFields', customFields);
-
 	// add modified records to updater
 	Object.keys(customFields).forEach((fieldName) => {
+		// @ts-expect-error TODO `Updater.set` does not support `customFields.${fieldName}` syntax
+		updater.set(`customFields.${fieldName}`, customFields[fieldName]);
+
 		if (!customFieldsMeta[fieldName].modifyRecordField) {
 			return;
 		}

apps/meteor/tests/e2e/admin-users-custom-fields.spec.ts

@@ -0,0 +1,128 @@
+import { Users } from './fixtures/userStates';
+import { HomeChannel, Admin } from './page-objects';
+import { test, expect } from './utils/test';
+import { createTestUser, type ITestUser } from './utils/user-helpers';
+
+const customFieldInitial1 = 'initial1';
+const adminCustomFieldValue1 = 'admin_value1';
+const adminCustomFieldValue2 = 'admin_value2';
+const adminCustomFieldUpdated1 = 'updated_admin1';
+
+test.describe('Admin users custom fields', () => {
+	let poHomeChannel: HomeChannel;
+	let poAdmin: Admin;
+	let addTestUser: ITestUser;
+	let updateTestUser: ITestUser;
+
+	test.use({ storageState: Users.admin.state });
+
+	test.beforeAll(async ({ api }) => {
+		await api.post('/settings/Accounts_CustomFields', {
+			value: JSON.stringify({
+				customFieldText1: {
+					type: 'text',
+					required: false,
+				},
+				customFieldText2: {
+					type: 'text',
+					required: false,
+				},
+			}),
+		});
+
+		[addTestUser, updateTestUser] = await Promise.all([
+			createTestUser(api),
+			createTestUser(api, {
+				data: {
+					customFields: {
+						customFieldText1: customFieldInitial1,
+						customFieldText2: adminCustomFieldValue2,
+					},
+				},
+			}),
+		]);
+	});
+
+	test.afterAll(async ({ api }) => {
+		await Promise.all([
+			api.post('/settings/Accounts_CustomFields', {
+				value: '',
+			}),
+			addTestUser.delete(),
+			updateTestUser.delete(),
+		]);
+	});
+
+	test.beforeEach(async ({ page }) => {
+		poHomeChannel = new HomeChannel(page);
+		poAdmin = new Admin(page);
+		await page.goto('/admin/users');
+	});
+
+	test('should allow admin to add user custom fields', async () => {
+		await test.step('should find and click on add test user', async () => {
+			await poAdmin.inputSearchUsers.fill(addTestUser.data.username);
+
+			await expect(poAdmin.getUserRowByUsername(addTestUser.data.username)).toBeVisible();
+			await poAdmin.getUserRowByUsername(addTestUser.data.username).click();
+		});
+
+		await test.step('should navigate to edit user form', async () => {
+			await poAdmin.btnEdit.click();
+		});
+
+		await test.step('should fill custom fields for user', async () => {
+			await poAdmin.tabs.users.getCustomField('customFieldText1').fill(adminCustomFieldValue1);
+			await poAdmin.tabs.users.getCustomField('customFieldText2').fill(adminCustomFieldValue2);
+		});
+
+		await test.step('should save user custom fields', async () => {
+			await poAdmin.tabs.users.btnSaveUser.click();
+			await poHomeChannel.dismissToast();
+		});
+
+		await test.step('should verify custom fields were saved', async () => {
+			await poAdmin.tabs.users.btnContextualbarClose.click();
+			await poAdmin.getUserRowByUsername(addTestUser.data.username).click();
+			await poAdmin.btnEdit.click();
+
+			await expect(poAdmin.tabs.users.getCustomField('customFieldText1')).toHaveValue(adminCustomFieldValue1);
+			await expect(poAdmin.tabs.users.getCustomField('customFieldText2')).toHaveValue(adminCustomFieldValue2);
+		});
+	});
+
+	test('should allow admin to update existing user custom fields', async () => {
+		await test.step('should find and click on update test user', async () => {
+			await poAdmin.inputSearchUsers.fill(updateTestUser.data.username);
+
+			await expect(poAdmin.getUserRowByUsername(updateTestUser.data.username)).toBeVisible();
+			await poAdmin.getUserRowByUsername(updateTestUser.data.username).click();
+		});
+
+		await test.step('should navigate to edit user form', async () => {
+			await poAdmin.btnEdit.click();
+		});
+
+		await test.step('should verify existing values and update one custom field', async () => {
+			await poAdmin.tabs.users.inputName.waitFor();
+
+			await expect(poAdmin.tabs.users.getCustomField('customFieldText1')).toHaveValue(customFieldInitial1);
+			await expect(poAdmin.tabs.users.getCustomField('customFieldText2')).toHaveValue(adminCustomFieldValue2);
+
+			await poAdmin.tabs.users.getCustomField('customFieldText1').clear();
+			await poAdmin.tabs.users.getCustomField('customFieldText1').fill(adminCustomFieldUpdated1);
+		});
+
+		await test.step('should save and verify partial update', async () => {
+			await poAdmin.tabs.users.btnSaveUser.click();
+			await poHomeChannel.dismissToast();
+
+			await poAdmin.tabs.users.btnContextualbarClose.click();
+			await poAdmin.getUserRowByUsername(updateTestUser.data.username).click();
+			await poAdmin.btnEdit.click();
+
+			await expect(poAdmin.tabs.users.getCustomField('customFieldText1')).toHaveValue(adminCustomFieldUpdated1);
+			await expect(poAdmin.tabs.users.getCustomField('customFieldText2')).toHaveValue(adminCustomFieldValue2);
+		});
+	});
+});

apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-users.ts

@@ -15,6 +15,10 @@ export class AdminFlextabUsers {
 		return this.page.locator('role=button[name="Add user"]');
 	}
 
+	get btnSaveUser(): Locator {
+		return this.page.locator('role=button[name="Save user"]');
+	}
+
 	get btnMoreActions(): Locator {
 		return this.page.locator('role=button[name="More"]');
 	}
@@ -75,4 +79,8 @@ export class AdminFlextabUsers {
 	get btnContextualbarClose(): Locator {
 		return this.page.locator('button[data-qa="ContextualbarActionClose"]');
 	}
+
+	getCustomField(fieldName: string): Locator {
+		return this.page.getByRole('textbox', { name: fieldName });
+	}
 }

apps/meteor/tests/end-to-end/api/users.ts

@@ -2087,6 +2087,168 @@ describe('[Users]', () => {
 		reservedWords.forEach((name) => {
 			failUpdateUser(name);
 		});
+
+		describe('Custom Fields', () => {
+			let testUser: TestUser<IUser>;
+
+			before(async () => {
+				await setCustomFields({
+					customFieldText1: {
+						type: 'text',
+						required: false,
+					},
+					customFieldText2: {
+						type: 'text',
+						required: false,
+					},
+				});
+			});
+
+			after(async () => {
+				await clearCustomFields();
+			});
+
+			beforeEach(async () => {
+				testUser = await createUser();
+			});
+
+			afterEach(async () => {
+				await deleteUser(testUser);
+			});
+
+			it('should merge custom fields instead of replacing them when updating a user', async () => {
+				await request
+					.post(api('users.update'))
+					.set(credentials)
+					.send({
+						userId: testUser._id,
+						data: {
+							customFields: {
+								customFieldText1: 'value1',
+								customFieldText2: 'value2',
+							},
+						},
+					})
+					.expect(200);
+
+				const updateResponse = await request
+					.post(api('users.update'))
+					.set(credentials)
+					.send({
+						userId: testUser._id,
+						data: {
+							customFields: {
+								customFieldText1: 'updated1',
+							},
+						},
+					})
+					.expect(200);
+
+				expect(updateResponse.body).to.have.property('success', true);
+				expect(updateResponse.body).to.have.nested.property('user.customFields.customFieldText1', 'updated1');
+				expect(updateResponse.body).to.have.nested.property('user.customFields.customFieldText2', 'value2');
+
+				const userInfoResponse = await request.get(api('users.info')).set(credentials).query({ userId: testUser._id }).expect(200);
+
+				expect(userInfoResponse.body).to.have.property('success', true);
+				expect(userInfoResponse.body).to.have.nested.property('user.customFields.customFieldText1', 'updated1');
+				expect(userInfoResponse.body).to.have.nested.property('user.customFields.customFieldText2', 'value2');
+			});
+
+			it('should preserve existing custom fields when adding new ones', async () => {
+				await request
+					.post(api('users.update'))
+					.set(credentials)
+					.send({
+						userId: testUser._id,
+						data: {
+							customFields: {
+								customFieldText1: 'initial1',
+							},
+						},
+					})
+					.expect(200);
+
+				const updateResponse = await request
+					.post(api('users.update'))
+					.set(credentials)
+					.send({
+						userId: testUser._id,
+						data: {
+							customFields: {
+								customFieldText2: 'additional2',
+							},
+						},
+					})
+					.expect(200);
+
+				expect(updateResponse.body).to.have.property('success', true);
+				expect(updateResponse.body).to.have.nested.property('user.customFields.customFieldText1', 'initial1');
+				expect(updateResponse.body).to.have.nested.property('user.customFields.customFieldText2', 'additional2');
+			});
+
+			it('should update custom field with empty string', async () => {
+				await request
+					.post(api('users.update'))
+					.set(credentials)
+					.send({
+						userId: testUser._id,
+						data: {
+							customFields: {
+								customFieldText1: 'value1',
+							},
+						},
+					})
+					.expect(200);
+
+				const updateResponse = await request
+					.post(api('users.update'))
+					.set(credentials)
+					.send({
+						userId: testUser._id,
+						data: {
+							customFields: {
+								customFieldText1: '',
+							},
+						},
+					})
+					.expect(200);
+
+				expect(updateResponse.body).to.have.property('success', true);
+				expect(updateResponse.body).to.have.nested.property('user.customFields.customFieldText1', '');
+			});
+
+			it('should update custom field with null', async () => {
+				await request
+					.post(api('users.update'))
+					.set(credentials)
+					.send({
+						userId: testUser._id,
+						data: {
+							customFields: {
+								customFieldText1: 'value1',
+							},
+						},
+					})
+					.expect(200);
+
+				const updateResponse = await request
+					.post(api('users.update'))
+					.set(credentials)
+					.send({
+						userId: testUser._id,
+						data: {
+							customFields: {
+								customFieldText1: null,
+							},
+						},
+					})
+					.expect(200);
+
+				expect(updateResponse.body).to.have.property('success', true);
+				expect(updateResponse.body).to.have.nested.property('user.customFields.customFieldText1', null);
+			});
+		});
 	});
 
 	describe('[/users.updateOwnBasicInfo]', () => {