Skip to content

Commit

Permalink
fix(core): Fix user telemetry bugs (n8n-io#10293)
Browse files Browse the repository at this point in the history
  • Loading branch information
ivov authored Aug 5, 2024
1 parent 7fb3f62 commit 42a0b59
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 3 deletions.
7 changes: 6 additions & 1 deletion packages/cli/src/controllers/__tests__/me.controller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,15 @@ describe('MeController', () => {
it('should update the user in the DB, and issue a new cookie', async () => {
const user = mock<User>({
id: '123',
email: 'valid@email.com',
password: 'password',
authIdentities: [],
role: 'global:owner',
});
const reqBody = { email: 'valid@email.com', firstName: 'John', lastName: 'Potato' };
const req = mock<MeRequest.UserUpdate>({ user, body: reqBody, browserId });
const res = mock<Response>();
userRepository.findOneByOrFail.mockResolvedValue(user);
userRepository.findOneOrFail.mockResolvedValue(user);
jest.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token');
userService.toPublic.mockResolvedValue({} as unknown as PublicUser);
Expand All @@ -66,7 +68,10 @@ describe('MeController', () => {
]);

expect(userService.update).toHaveBeenCalled();

expect(eventService.emit).toHaveBeenCalledWith('user-updated', {
user,
fieldsChanged: ['firstName', 'lastName'], // email did not change
});
expect(res.cookie).toHaveBeenCalledWith(
AUTH_COOKIE_NAME,
'signed-token',
Expand Down
52 changes: 52 additions & 0 deletions packages/cli/src/controllers/__tests__/users.controller.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { mock } from 'jest-mock-extended';
import { UsersController } from '../users.controller';
import type { UserRequest } from '@/requests';
import type { EventService } from '@/events/event.service';
import type { User } from '@/databases/entities/User';
import type { UserRepository } from '@/databases/repositories/user.repository';
import type { ProjectService } from '@/services/project.service';

describe('UsersController', () => {
const eventService = mock<EventService>();
const userRepository = mock<UserRepository>();
const projectService = mock<ProjectService>();
const controller = new UsersController(
mock(),
mock(),
mock(),
mock(),
userRepository,
mock(),
mock(),
mock(),
mock(),
mock(),
projectService,
eventService,
);

beforeEach(() => {
jest.restoreAllMocks();
});

describe('changeGlobalRole', () => {
it('should emit event user-changed-role', async () => {
const request = mock<UserRequest.ChangeRole>({
user: { id: '123' },
params: { id: '456' },
body: { newRoleName: 'global:member' },
});
userRepository.findOne.mockResolvedValue(mock<User>({ id: '456' }));
projectService.getUserOwnedOrAdminProjects.mockResolvedValue([]);

await controller.changeGlobalRole(request);

expect(eventService.emit).toHaveBeenCalledWith('user-changed-role', {
userId: '123',
targetUserId: '456',
targetUserNewRole: 'global:member',
publicApi: false,
});
});
});
});
6 changes: 5 additions & 1 deletion packages/cli/src/controllers/me.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ export class MeController {

await this.externalHooks.run('user.profile.beforeUpdate', [userId, currentEmail, payload]);

const preUpdateUser = await this.userRepository.findOneByOrFail({ id: userId });
await this.userService.update(userId, payload);
const user = await this.userRepository.findOneOrFail({
where: { id: userId },
Expand All @@ -98,7 +99,10 @@ export class MeController {

this.authService.issueCookie(res, user, req.browserId);

const fieldsChanged = Object.keys(payload);
const fieldsChanged = (Object.keys(payload) as Array<keyof UserUpdatePayload>).filter(
(key) => payload[key] !== preUpdateUser[key],
);

this.eventService.emit('user-updated', { user, fieldsChanged });

const publicUser = await this.userService.toPublic(user);
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/controllers/users.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ export class UsersController {
this.eventService.emit('user-changed-role', {
userId: req.user.id,
targetUserId: targetUser.id,
targetUserNewRole: ['global', payload.newRoleName].join(' '),
targetUserNewRole: payload.newRoleName,
publicApi: false,
});

Expand Down

0 comments on commit 42a0b59

Please sign in to comment.