fix: use setImmediate to handle username update

.changeset/wicked-jars-double.md

@@ -0,0 +1,5 @@
+---
+"@rocket.chat/meteor": patch
+---
+
+Handle the username update in the background

apps/meteor/app/lib/server/functions/saveUser.js

@@ -8,6 +8,7 @@ import _ from 'underscore';
 import { AppEvents, Apps } from '../../../../ee/server/apps/orchestrator';
 import { callbacks } from '../../../../lib/callbacks';
 import { trim } from '../../../../lib/utils/stringUtils';
+import { SystemLogger } from '../../../../server/lib/logger/system';
 import { getNewUserRoles } from '../../../../server/services/user/lib/getNewUserRoles';
 import { getRoles } from '../../../authorization/server';
 import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
@@ -361,17 +362,17 @@ export const saveUser = async function (userId, userData) {
 
 	// update user
 	if (userData.hasOwnProperty('username') || userData.hasOwnProperty('name')) {
-		if (
-			!(await saveUserIdentity({
+		setImmediate(async () => {
+			const isUsernameUpdated = await saveUserIdentity({
 				_id: userData._id,
 				username: userData.username,
 				name: userData.name,
-			}))
-		) {
-			throw new Meteor.Error('error-could-not-save-identity', 'Could not save user identity', {
-				method: 'saveUser',
 			});
-		}
+
+			if (!isUsernameUpdated) {
+				SystemLogger.error('Could not update username', { method: 'saveUser', userId });
+			}
+		});
 	}
 
 	if (typeof userData.statusText === 'string') {

apps/meteor/tests/end-to-end/api/01-users.js

@@ -1522,31 +1522,6 @@ describe('[Users]', function () {
 				});
 			});
 		});
-
-		function failUpdateUser(name) {
-			it(`should not update an user if the new username is the reserved word ${name}`, (done) => {
-				request
-					.post(api('users.update'))
-					.set(credentials)
-					.send({
-						userId: targetUser._id,
-						data: {
-							username: name,
-						},
-					})
-					.expect('Content-Type', 'application/json')
-					.expect(400)
-					.expect((res) => {
-						expect(res.body).to.have.property('success', false);
-						expect(res.body).to.have.property('error', 'Could not save user identity [error-could-not-save-identity]');
-					})
-					.end(done);
-			});
-		}
-
-		reservedWords.forEach((name) => {
-			failUpdateUser(name);
-		});
 	});
 
 	describe('[/users.updateOwnBasicInfo]', () => {

apps/meteor/tests/end-to-end/api/09-rooms.js

@@ -4,6 +4,7 @@ import path from 'path';
 import { expect } from 'chai';
 import { after, afterEach, before, beforeEach, describe, it } from 'mocha';
 
+import { sleep } from '../../../lib/utils/sleep';
 import { getCredentials, api, request, credentials } from '../../data/api-data.js';
 import { sendSimpleMessage, deleteMessage } from '../../data/chat.helper';
 import { imgURL } from '../../data/interactions.js';
@@ -1543,29 +1544,30 @@ describe('[Rooms]', function () {
 			roomId = result.body.room.rid;
 		});
 
-		it('should update group name if user changes username', (done) => {
-			updateSetting('UI_Use_Real_Name', false).then(() => {
-				request
-					.post(api('users.update'))
-					.set(credentials)
-					.send({
-						userId: testUser._id,
-						data: {
-							username: `changed.username.${testUser.username}`,
-						},
-					})
-					.end(() => {
-						request
-							.get(api('subscriptions.getOne'))
-							.set(credentials)
-							.query({ roomId })
-							.end((err, res) => {
-								const { subscription } = res.body;
-								expect(subscription.name).to.equal(`rocket.cat,changed.username.${testUser.username}`);
-								done();
-							});
-					});
-			});
+		it('should update group name if user changes username', async () => {
+			await updateSetting('UI_Use_Real_Name', false);
+			await request
+				.post(api('users.update'))
+				.set(credentials)
+				.send({
+					userId: testUser._id,
+					data: {
+						username: `changed.username.${testUser.username}`,
+					},
+				});
+
+			// need to wait for the username update finish
+			await sleep(300);
+
+			await request
+				.get(api('subscriptions.getOne'))
+				.set(credentials)
+				.query({ roomId })
+				.send()
+				.expect((res) => {
+					const { subscription } = res.body;
+					expect(subscription.name).to.equal(`rocket.cat,changed.username.${testUser.username}`);
+				});
 		});
 
 		it('should update group name if user changes name', (done) => {

apps/meteor/tests/unit/server/users/saveUserIdentity.spec.ts

@@ -0,0 +1,134 @@
+import { expect } from 'chai';
+import proxyquire from 'proxyquire';
+import sinon from 'sinon';
+
+// Create stubs for dependencies
+const stubs = {
+	findOneUserById: sinon.stub(),
+	updateUsernameAndMessageOfMentionByIdAndOldUsername: sinon.stub(),
+	updateUsernameOfEditByUserId: sinon.stub(),
+	updateAllUsernamesByUserId: sinon.stub(),
+	updateDirectNameAndFnameByName: sinon.stub(),
+	updateUserReferences: sinon.stub(),
+	setUsername: sinon.stub(),
+	setRealName: sinon.stub(),
+	validateName: sinon.stub(),
+	FileUpload: sinon.stub(),
+};
+
+const { saveUserIdentity } = proxyquire.noCallThru().load('../../../../app/lib/server/functions/saveUserIdentity', {
+	'@rocket.chat/models': {
+		Users: {
+			findOneById: stubs.findOneUserById,
+		},
+		Messages: {
+			updateUsernameAndMessageOfMentionByIdAndOldUsername: stubs.updateUsernameAndMessageOfMentionByIdAndOldUsername,
+			updateUsernameOfEditByUserId: stubs.updateUsernameOfEditByUserId,
+			updateAllUsernamesByUserId: stubs.updateAllUsernamesByUserId,
+		},
+		Subscriptions: {
+			updateDirectNameAndFnameByName: stubs.updateDirectNameAndFnameByName,
+		},
+		VideoConference: {
+			updateUserReferences: stubs.updateUserReferences,
+		},
+	},
+	'meteor/meteor': {
+		'Meteor': sinon.stub(),
+		'@global': true,
+	},
+	'../../../../app/file-upload/server': {
+		FileUpload: stubs.FileUpload,
+	},
+	'../../../../app/lib/server/functions/setRealName': {
+		_setRealName: stubs.setRealName,
+	},
+	'../../../../app/lib/server/functions/setUsername': {
+		_setUsername: stubs.setUsername,
+	},
+	'../../../../app/lib/server/functions/updateGroupDMsName': {
+		updateGroupDMsName: sinon.stub(),
+	},
+	'../../../../app/lib/server/functions/validateName': {
+		validateName: stubs.validateName,
+	},
+});
+
+describe('Users - saveUserIdentity', () => {
+	beforeEach(() => {
+		// Reset stubs before each test
+		Object.values(stubs).forEach((stub) => stub.reset());
+	});
+
+	it('should return false if _id is not provided', async () => {
+		const result = await saveUserIdentity({ _id: undefined });
+
+		expect(stubs.findOneUserById.called).to.be.false;
+		expect(result).to.be.false;
+	});
+
+	it('should return false if user does not exist', async () => {
+		stubs.findOneUserById.returns(undefined);
+		const result = await saveUserIdentity({ _id: 'valid_id' });
+
+		expect(stubs.findOneUserById.calledWith('valid_id')).to.be.true;
+		expect(result).to.be.false;
+	});
+
+	it('should return false if username is not allowed', async () => {
+		stubs.findOneUserById.returns({ username: 'oldUsername' });
+		stubs.validateName.returns(false);
+		const result = await saveUserIdentity({ _id: 'valid_id', username: 'admin' });
+
+		expect(stubs.validateName.calledWith('admin')).to.be.true;
+		expect(result).to.be.false;
+	});
+
+	it('should return false if username is invalid or unavailable', async () => {
+		stubs.findOneUserById.returns({ username: 'oldUsername' });
+		stubs.validateName.returns(true);
+		stubs.setUsername.returns(false);
+		const result = await saveUserIdentity({ _id: 'valid_id', username: 'invalidUsername' });
+
+		expect(stubs.validateName.calledWith('invalidUsername')).to.be.true;
+		expect(stubs.setUsername.calledWith('valid_id', 'invalidUsername', { username: 'oldUsername' })).to.be.true;
+		expect(result).to.be.false;
+	});
+
+	it("should not update the username if it's not changed", async () => {
+		stubs.findOneUserById.returns({ username: 'oldUsername', name: 'oldName' });
+		stubs.validateName.returns(true);
+		stubs.setUsername.returns(true);
+		await saveUserIdentity({ _id: 'valid_id', username: 'oldUsername', name: 'oldName' });
+
+		expect(stubs.validateName.called).to.be.false;
+		expect(stubs.setUsername.called).to.be.false;
+		expect(stubs.updateUsernameOfEditByUserId.called).to.be.false;
+		expect(stubs.updateAllUsernamesByUserId.called).to.be.false;
+		expect(stubs.updateUsernameAndMessageOfMentionByIdAndOldUsername.called).to.be.false;
+		expect(stubs.updateDirectNameAndFnameByName.called).to.be.false;
+		expect(stubs.updateUserReferences.called).to.be.false;
+	});
+
+	it('should return false if _setName fails', async () => {
+		stubs.findOneUserById.returns({ name: 'oldName' });
+		stubs.setRealName.returns(false);
+		const result = await saveUserIdentity({ _id: 'valid_id', name: 'invalidName' });
+
+		expect(stubs.setRealName.calledWith('valid_id', 'invalidName', { name: 'oldName' })).to.be.true;
+		expect(result).to.be.false;
+	});
+
+	it('should update Subscriptions and VideoConference if name changes', async () => {
+		stubs.findOneUserById.returns({ name: 'oldName', username: 'oldUsername' });
+		stubs.setRealName.returns(true);
+		const result = await saveUserIdentity({ _id: 'valid_id', name: 'name', username: 'oldUsername' });
+
+		expect(stubs.setUsername.called).to.be.false;
+		expect(stubs.setRealName.called).to.be.true;
+		expect(stubs.updateUsernameOfEditByUserId.called).to.be.false;
+		expect(stubs.updateDirectNameAndFnameByName.called).to.be.true;
+		expect(stubs.updateUserReferences.called).to.be.true;
+		expect(result).to.be.true;
+	});
+});