Skip to content

Commit 57c6a3a

Browse files
cardosojessicaschelly
cardoso
and
jessicaschelly
authored
fix: users.update API call erases other customFields (#36578)
Co-authored-by: Jessica Schelly Souza <[email protected]>
1 parent 1142862 commit 57c6a3a

File tree

5 files changed

+310
-9
lines changed

5 files changed

+310
-9
lines changed

‎.changeset/wild-kiwis-cover.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@rocket.chat/meteor": patch
3+
---
4+
5+
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‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,22 +32,20 @@ export const saveCustomFieldsWithoutValidation = async function (
3232
// configured custom fields in setting
3333
const customFieldsMeta = getCustomFieldsMeta(customFieldsSetting);
3434

35-
const customFields: Record<string, any> = Object.keys(customFieldsMeta).reduce(
36-
(acc, currentValue) => ({
37-
...acc,
38-
[currentValue]: formData[currentValue],
39-
}),
40-
{},
35+
const customFields = Object.fromEntries(
36+
Object.keys(formData)
37+
.filter((key) => Object.hasOwn(customFieldsMeta, key))
38+
.map((key) => [key, formData[key]]),
4139
);
4240

4341
const { _updater, session } = options || {};
4442

4543
const updater = _updater || Users.getUpdater();
46-
47-
updater.set('customFields', customFields);
48-
4944
// add modified records to updater
5045
Object.keys(customFields).forEach((fieldName) => {
46+
// @ts-expect-error TODO `Updater.set` does not support `customFields.${fieldName}` syntax
47+
updater.set(`customFields.${fieldName}`, customFields[fieldName]);
48+
5149
if (!customFieldsMeta[fieldName].modifyRecordField) {
5250
return;
5351
}

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

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
import { Users } from './fixtures/userStates';
2+
import { HomeChannel, Admin } from './page-objects';
3+
import { test, expect } from './utils/test';
4+
import { createTestUser, type ITestUser } from './utils/user-helpers';
5+
6+
const customFieldInitial1 = 'initial1';
7+
const adminCustomFieldValue1 = 'admin_value1';
8+
const adminCustomFieldValue2 = 'admin_value2';
9+
const adminCustomFieldUpdated1 = 'updated_admin1';
10+
11+
test.describe('Admin users custom fields', () => {
12+
let poHomeChannel: HomeChannel;
13+
let poAdmin: Admin;
14+
let addTestUser: ITestUser;
15+
let updateTestUser: ITestUser;
16+
17+
test.use({ storageState: Users.admin.state });
18+
19+
test.beforeAll(async ({ api }) => {
20+
await api.post('/settings/Accounts_CustomFields', {
21+
value: JSON.stringify({
22+
customFieldText1: {
23+
type: 'text',
24+
required: false,
25+
},
26+
customFieldText2: {
27+
type: 'text',
28+
required: false,
29+
},
30+
}),
31+
});
32+
33+
[addTestUser, updateTestUser] = await Promise.all([
34+
createTestUser(api),
35+
createTestUser(api, {
36+
data: {
37+
customFields: {
38+
customFieldText1: customFieldInitial1,
39+
customFieldText2: adminCustomFieldValue2,
40+
},
41+
},
42+
}),
43+
]);
44+
});
45+
46+
test.afterAll(async ({ api }) => {
47+
await Promise.all([
48+
api.post('/settings/Accounts_CustomFields', {
49+
value: '',
50+
}),
51+
addTestUser.delete(),
52+
updateTestUser.delete(),
53+
]);
54+
});
55+
56+
test.beforeEach(async ({ page }) => {
57+
poHomeChannel = new HomeChannel(page);
58+
poAdmin = new Admin(page);
59+
await page.goto('/admin/users');
60+
});
61+
62+
test('should allow admin to add user custom fields', async () => {
63+
await test.step('should find and click on add test user', async () => {
64+
await poAdmin.inputSearchUsers.fill(addTestUser.data.username);
65+
66+
await expect(poAdmin.getUserRowByUsername(addTestUser.data.username)).toBeVisible();
67+
await poAdmin.getUserRowByUsername(addTestUser.data.username).click();
68+
});
69+
70+
await test.step('should navigate to edit user form', async () => {
71+
await poAdmin.btnEdit.click();
72+
});
73+
74+
await test.step('should fill custom fields for user', async () => {
75+
await poAdmin.tabs.users.getCustomField('customFieldText1').fill(adminCustomFieldValue1);
76+
await poAdmin.tabs.users.getCustomField('customFieldText2').fill(adminCustomFieldValue2);
77+
});
78+
79+
await test.step('should save user custom fields', async () => {
80+
await poAdmin.tabs.users.btnSaveUser.click();
81+
await poHomeChannel.dismissToast();
82+
});
83+
84+
await test.step('should verify custom fields were saved', async () => {
85+
await poAdmin.tabs.users.btnContextualbarClose.click();
86+
await poAdmin.getUserRowByUsername(addTestUser.data.username).click();
87+
await poAdmin.btnEdit.click();
88+
89+
await expect(poAdmin.tabs.users.getCustomField('customFieldText1')).toHaveValue(adminCustomFieldValue1);
90+
await expect(poAdmin.tabs.users.getCustomField('customFieldText2')).toHaveValue(adminCustomFieldValue2);
91+
});
92+
});
93+
94+
test('should allow admin to update existing user custom fields', async () => {
95+
await test.step('should find and click on update test user', async () => {
96+
await poAdmin.inputSearchUsers.fill(updateTestUser.data.username);
97+
98+
await expect(poAdmin.getUserRowByUsername(updateTestUser.data.username)).toBeVisible();
99+
await poAdmin.getUserRowByUsername(updateTestUser.data.username).click();
100+
});
101+
102+
await test.step('should navigate to edit user form', async () => {
103+
await poAdmin.btnEdit.click();
104+
});
105+
106+
await test.step('should verify existing values and update one custom field', async () => {
107+
await poAdmin.tabs.users.inputName.waitFor();
108+
109+
await expect(poAdmin.tabs.users.getCustomField('customFieldText1')).toHaveValue(customFieldInitial1);
110+
await expect(poAdmin.tabs.users.getCustomField('customFieldText2')).toHaveValue(adminCustomFieldValue2);
111+
112+
await poAdmin.tabs.users.getCustomField('customFieldText1').clear();
113+
await poAdmin.tabs.users.getCustomField('customFieldText1').fill(adminCustomFieldUpdated1);
114+
});
115+
116+
await test.step('should save and verify partial update', async () => {
117+
await poAdmin.tabs.users.btnSaveUser.click();
118+
await poHomeChannel.dismissToast();
119+
120+
await poAdmin.tabs.users.btnContextualbarClose.click();
121+
await poAdmin.getUserRowByUsername(updateTestUser.data.username).click();
122+
await poAdmin.btnEdit.click();
123+
124+
await expect(poAdmin.tabs.users.getCustomField('customFieldText1')).toHaveValue(adminCustomFieldUpdated1);
125+
await expect(poAdmin.tabs.users.getCustomField('customFieldText2')).toHaveValue(adminCustomFieldValue2);
126+
});
127+
});
128+
});

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ export class AdminFlextabUsers {
1515
return this.page.locator('role=button[name="Add user"]');
1616
}
1717

18+
get btnSaveUser(): Locator {
19+
return this.page.locator('role=button[name="Save user"]');
20+
}
21+
1822
get btnMoreActions(): Locator {
1923
return this.page.locator('role=button[name="More"]');
2024
}
@@ -75,4 +79,8 @@ export class AdminFlextabUsers {
7579
get btnContextualbarClose(): Locator {
7680
return this.page.locator('button[data-qa="ContextualbarActionClose"]');
7781
}
82+
83+
getCustomField(fieldName: string): Locator {
84+
return this.page.getByRole('textbox', { name: fieldName });
85+
}
7886
}

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

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2087,6 +2087,168 @@ describe('[Users]', () => {
20872087
reservedWords.forEach((name) => {
20882088
failUpdateUser(name);
20892089
});
2090+
2091+
describe('Custom Fields', () => {
2092+
let testUser: TestUser<IUser>;
2093+
2094+
before(async () => {
2095+
await setCustomFields({
2096+
customFieldText1: {
2097+
type: 'text',
2098+
required: false,
2099+
},
2100+
customFieldText2: {
2101+
type: 'text',
2102+
required: false,
2103+
},
2104+
});
2105+
});
2106+
2107+
after(async () => {
2108+
await clearCustomFields();
2109+
});
2110+
2111+
beforeEach(async () => {
2112+
testUser = await createUser();
2113+
});
2114+
2115+
afterEach(async () => {
2116+
await deleteUser(testUser);
2117+
});
2118+
2119+
it('should merge custom fields instead of replacing them when updating a user', async () => {
2120+
await request
2121+
.post(api('users.update'))
2122+
.set(credentials)
2123+
.send({
2124+
userId: testUser._id,
2125+
data: {
2126+
customFields: {
2127+
customFieldText1: 'value1',
2128+
customFieldText2: 'value2',
2129+
},
2130+
},
2131+
})
2132+
.expect(200);
2133+
2134+
const updateResponse = await request
2135+
.post(api('users.update'))
2136+
.set(credentials)
2137+
.send({
2138+
userId: testUser._id,
2139+
data: {
2140+
customFields: {
2141+
customFieldText1: 'updated1',
2142+
},
2143+
},
2144+
})
2145+
.expect(200);
2146+
2147+
expect(updateResponse.body).to.have.property('success', true);
2148+
expect(updateResponse.body).to.have.nested.property('user.customFields.customFieldText1', 'updated1');
2149+
expect(updateResponse.body).to.have.nested.property('user.customFields.customFieldText2', 'value2');
2150+
2151+
const userInfoResponse = await request.get(api('users.info')).set(credentials).query({ userId: testUser._id }).expect(200);
2152+
2153+
expect(userInfoResponse.body).to.have.property('success', true);
2154+
expect(userInfoResponse.body).to.have.nested.property('user.customFields.customFieldText1', 'updated1');
2155+
expect(userInfoResponse.body).to.have.nested.property('user.customFields.customFieldText2', 'value2');
2156+
});
2157+
2158+
it('should preserve existing custom fields when adding new ones', async () => {
2159+
await request
2160+
.post(api('users.update'))
2161+
.set(credentials)
2162+
.send({
2163+
userId: testUser._id,
2164+
data: {
2165+
customFields: {
2166+
customFieldText1: 'initial1',
2167+
},
2168+
},
2169+
})
2170+
.expect(200);
2171+
2172+
const updateResponse = await request
2173+
.post(api('users.update'))
2174+
.set(credentials)
2175+
.send({
2176+
userId: testUser._id,
2177+
data: {
2178+
customFields: {
2179+
customFieldText2: 'additional2',
2180+
},
2181+
},
2182+
})
2183+
.expect(200);
2184+
2185+
expect(updateResponse.body).to.have.property('success', true);
2186+
expect(updateResponse.body).to.have.nested.property('user.customFields.customFieldText1', 'initial1');
2187+
expect(updateResponse.body).to.have.nested.property('user.customFields.customFieldText2', 'additional2');
2188+
});
2189+
2190+
it('should update custom field with empty string', async () => {
2191+
await request
2192+
.post(api('users.update'))
2193+
.set(credentials)
2194+
.send({
2195+
userId: testUser._id,
2196+
data: {
2197+
customFields: {
2198+
customFieldText1: 'value1',
2199+
},
2200+
},
2201+
})
2202+
.expect(200);
2203+
2204+
const updateResponse = await request
2205+
.post(api('users.update'))
2206+
.set(credentials)
2207+
.send({
2208+
userId: testUser._id,
2209+
data: {
2210+
customFields: {
2211+
customFieldText1: '',
2212+
},
2213+
},
2214+
})
2215+
.expect(200);
2216+
2217+
expect(updateResponse.body).to.have.property('success', true);
2218+
expect(updateResponse.body).to.have.nested.property('user.customFields.customFieldText1', '');
2219+
});
2220+
2221+
it('should update custom field with null', async () => {
2222+
await request
2223+
.post(api('users.update'))
2224+
.set(credentials)
2225+
.send({
2226+
userId: testUser._id,
2227+
data: {
2228+
customFields: {
2229+
customFieldText1: 'value1',
2230+
},
2231+
},
2232+
})
2233+
.expect(200);
2234+
2235+
const updateResponse = await request
2236+
.post(api('users.update'))
2237+
.set(credentials)
2238+
.send({
2239+
userId: testUser._id,
2240+
data: {
2241+
customFields: {
2242+
customFieldText1: null,
2243+
},
2244+
},
2245+
})
2246+
.expect(200);
2247+
2248+
expect(updateResponse.body).to.have.property('success', true);
2249+
expect(updateResponse.body).to.have.nested.property('user.customFields.customFieldText1', null);
2250+
});
2251+
});
20902252
});
20912253

20922254
describe('[/users.updateOwnBasicInfo]', () => {

0 commit comments

Comments
 (0)
⚡