From 21301b97020fce4f24c378193a67e86045f4a7da Mon Sep 17 00:00:00 2001 From: Mohamed Marrouchi Date: Tue, 4 Feb 2025 17:57:37 +0100 Subject: [PATCH 1/2] fix: handover/handback lifecycle hook --- .../repositories/subscriber.repository.ts | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/api/src/chat/repositories/subscriber.repository.ts b/api/src/chat/repositories/subscriber.repository.ts index fa59ee70..02d0ad66 100644 --- a/api/src/chat/repositories/subscriber.repository.ts +++ b/api/src/chat/repositories/subscriber.repository.ts @@ -80,26 +80,29 @@ export class SubscriberRepository extends BaseRepository< ): Promise { const subscriberUpdates: SubscriberUpdateDto = updates?.['$set']; - const oldSubscriber = await this.findOne(criteria); + if ('assignedTo' in subscriberUpdates) { + // In case of a handover or handback, emit events + const oldSubscriber = await this.findOne(criteria); - if (!oldSubscriber) { - throw new Error('Something went wrong: subscriber does not exist'); - } + if (!oldSubscriber) { + throw new Error('Something went wrong: subscriber does not exist'); + } - if (subscriberUpdates.assignedTo !== oldSubscriber?.assignedTo) { - this.eventEmitter.emit( - 'hook:subscriber:assign', - subscriberUpdates, - oldSubscriber, - ); - - if (!(subscriberUpdates.assignedTo && oldSubscriber?.assignedTo)) { + if (subscriberUpdates.assignedTo !== oldSubscriber?.assignedTo) { this.eventEmitter.emit( - 'hook:analytics:passation', + 'hook:subscriber:assign', + subscriberUpdates, oldSubscriber, - !!subscriberUpdates?.assignedTo, ); - subscriberUpdates.assignedAt = new Date(); + + if (!(subscriberUpdates.assignedTo && oldSubscriber?.assignedTo)) { + this.eventEmitter.emit( + 'hook:analytics:passation', + oldSubscriber, + !!subscriberUpdates?.assignedTo, + ); + subscriberUpdates.assignedAt = new Date(); + } } } } From a607247d567a554be1f29063a2b8777474765ea1 Mon Sep 17 00:00:00 2001 From: Mohamed Marrouchi Date: Tue, 4 Feb 2025 18:26:46 +0100 Subject: [PATCH 2/2] test: add unit tests --- .../subscriber.repository.spec.ts | 73 ++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/api/src/chat/repositories/subscriber.repository.spec.ts b/api/src/chat/repositories/subscriber.repository.spec.ts index fd553e5f..1e101057 100644 --- a/api/src/chat/repositories/subscriber.repository.spec.ts +++ b/api/src/chat/repositories/subscriber.repository.spec.ts @@ -1,5 +1,5 @@ /* - * Copyright © 2024 Hexastack. All rights reserved. + * Copyright © 2025 Hexastack. All rights reserved. * * Licensed under the GNU Affero General Public License v3.0 (AGPLv3) with the following additional terms: * 1. The name "Hexabot" is a trademark of Hexastack. You may not use this name in derivative works without express written permission. @@ -52,6 +52,7 @@ describe('SubscriberRepository', () => { let allSubscribers: Subscriber[]; let allAttachments: Attachment[]; let subscribersWithPopulatedFields: SubscriberFull[]; + let eventEmitter: EventEmitter2; beforeAll(async () => { const module = await Test.createTestingModule({ @@ -94,6 +95,7 @@ describe('SubscriberRepository', () => { allUsers.find(({ id }) => subscriber.assignedTo === id) || null, avatar: allAttachments.find(({ id }) => subscriber.avatar === id) || null, })); + eventEmitter = module.get(EventEmitter2); }); afterEach(jest.clearAllMocks); @@ -155,4 +157,73 @@ describe('SubscriberRepository', () => { ); }); }); + + describe('updateOne', () => { + it('should execute preUpdate hook and emit events on assignedTo change', async () => { + // Arrange: Set up a mock subscriber + const oldSubscriber = { + ...subscriberFixtures[0], // Mocked existing subscriber + assignedTo: null, + } as Subscriber; + + const updates = { assignedTo: '9'.repeat(24) }; // Change assigned user; + + jest + .spyOn(subscriberRepository, 'findOne') + .mockResolvedValue(oldSubscriber); + jest.spyOn(eventEmitter, 'emit'); + + await subscriberRepository.updateOne(oldSubscriber.id, updates); + + expect(eventEmitter.emit).toHaveBeenNthCalledWith( + 3, + 'hook:subscriber:assign', + expect.anything(), + expect.anything(), + ); + expect(eventEmitter.emit).toHaveBeenNthCalledWith( + 4, + 'hook:analytics:passation', + expect.anything(), + true, // Because assignedTo has changed + ); + }); + + it('should not emit events if assignedTo remains unchanged', async () => { + const oldSubscriber = { + ...subscriberFixtures[0], + assignedTo: '8'.repeat(24), + } as Subscriber; + + const updates = { assignedTo: '8'.repeat(24) }; // Same user; + + jest + .spyOn(subscriberRepository, 'findOne') + .mockResolvedValue(oldSubscriber); + jest.spyOn(eventEmitter, 'emit'); + + await subscriberRepository.updateOne(oldSubscriber.id, updates); + + expect(eventEmitter.emit).not.toHaveBeenCalledWith( + 'hook:subscriber:assign', + expect.anything(), + expect.anything(), + ); + expect(eventEmitter.emit).not.toHaveBeenCalledWith( + 'hook:analytics:passation', + expect.anything(), + expect.anything(), + ); + }); + + it('should throw an error if the subscriber does not exist', async () => { + jest.spyOn(subscriberRepository, 'findOne').mockResolvedValue(null); + + await expect( + subscriberRepository.updateOne('0'.repeat(24), { + $set: { assignedTo: 'user-456' }, + }), + ).rejects.toThrow(); + }); + }); });