Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/wild-kiwis-cover.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
128 changes: 128 additions & 0 deletions apps/meteor/tests/e2e/admin-users-custom-fields.spec.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
});
4 changes: 2 additions & 2 deletions apps/meteor/tests/e2e/containers/saml/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
FROM php:7.1-apache
FROM php:7.3-apache

# Utilities
RUN apt-get update && \
apt-get -y install apt-transport-https git curl vim --no-install-recommends && \
rm -r /var/lib/apt/lists/*

# SimpleSAMLphp
ARG SIMPLESAMLPHP_VERSION=1.15.2
ARG SIMPLESAMLPHP_VERSION=1.15.4
RUN curl -s -L -o /tmp/simplesamlphp.tar.gz https://github.com/simplesamlphp/simplesamlphp/releases/download/v$SIMPLESAMLPHP_VERSION/simplesamlphp-$SIMPLESAMLPHP_VERSION.tar.gz && \
tar xzf /tmp/simplesamlphp.tar.gz -C /tmp && \
rm -f /tmp/simplesamlphp.tar.gz && \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@ 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"]');
}

get btnDeleteUser(): Locator {
return this.page.locator('role=menuitem[name="Delete"]');
}

get btnInvite(): Locator {
return this.page.locator('role=button[name="Invite"]');
}
Expand Down Expand Up @@ -67,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 });
}
}
77 changes: 77 additions & 0 deletions apps/meteor/tests/e2e/utils/user-helpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { faker } from '@faker-js/faker';
import type { APIResponse } from '@playwright/test';
import type { IUser } from '@rocket.chat/core-typings';

import type { BaseTest } from './test';
import { DEFAULT_USER_CREDENTIALS } from '../config/constants';

export interface ICreateUserOptions {
username?: string;
email?: string;
name?: string;
password?: string;
roles?: string[];
data?: Record<string, any>;
}

export interface ITestUser {
response: APIResponse;
data: IUser & { username: string };
deleted: boolean;
delete: () => Promise<APIResponse | undefined>;
markAsDeleted: () => void;
}

/**
* Creates a test user with optional customizations
*/
export async function createTestUser(api: BaseTest['api'], options: ICreateUserOptions = {}): Promise<ITestUser> {
const userData = {
email: options.email || faker.internet.email(),
name: options.name || faker.person.fullName(),
password: options.password || DEFAULT_USER_CREDENTIALS.password,
username: options.username || `test-user-${faker.string.uuid()}`,
roles: options.roles || ['user'],
...options.data,
};

const response = await api.post('/users.create', userData);

if (response.status() !== 200) {
throw new Error(`Failed to create user: ${response.status()}, response: ${await response.text()}`);
}

const { user } = await response.json();

return {
response,
data: user,
deleted: false,
markAsDeleted(this: ITestUser) {
this.deleted = true;
},
async delete(this: ITestUser) {
if (this.deleted) {
return;
}

const response = await api.post('/users.delete', { userId: user._id });
this.markAsDeleted();
return response;
},
};
}

/**
* Creates multiple test users at once
*/
export async function createTestUsers(api: BaseTest['api'], count: number, options: ICreateUserOptions = {}): Promise<ITestUser[]> {
const promises = Array.from({ length: count }, (_, i) =>
createTestUser(api, {
...options,
username: options.username ? `${options.username}-${i + 1}` : undefined,
}),
);

return Promise.all(promises);
}
Loading
Loading