diff --git a/.changeset/wild-kiwis-cover.md b/.changeset/wild-kiwis-cover.md new file mode 100644 index 0000000000000..8bb6afeb72917 --- /dev/null +++ b/.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. \ No newline at end of file diff --git a/apps/meteor/app/lib/server/functions/saveCustomFieldsWithoutValidation.ts b/apps/meteor/app/lib/server/functions/saveCustomFieldsWithoutValidation.ts index e3b19b01cfc16..f99faf9aa795b 100644 --- a/apps/meteor/app/lib/server/functions/saveCustomFieldsWithoutValidation.ts +++ b/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 = 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; } diff --git a/apps/meteor/tests/e2e/admin-users-custom-fields.spec.ts b/apps/meteor/tests/e2e/admin-users-custom-fields.spec.ts new file mode 100644 index 0000000000000..833acc1b799dd --- /dev/null +++ b/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); + }); + }); +}); diff --git a/apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-users.ts b/apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-users.ts index 3bb110bceccd6..b1f284249f006 100644 --- a/apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-users.ts +++ b/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 }); + } } diff --git a/apps/meteor/tests/end-to-end/api/users.ts b/apps/meteor/tests/end-to-end/api/users.ts index f03dc46dd2c51..45beb3b499784 100644 --- a/apps/meteor/tests/end-to-end/api/users.ts +++ b/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; + + 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]', () => {