From 90a7c2f5c2769a76ccd6cd1a8b8f8cef03414f96 Mon Sep 17 00:00:00 2001 From: yassinedorbozgithub Date: Thu, 9 Jan 2025 17:58:26 +0100 Subject: [PATCH 01/12] fix: apply strict null checks updates to the Chat Module --- api/src/channel/channel.service.ts | 4 +- .../chat/controllers/block.controller.spec.ts | 61 ++++++++++--------- api/src/chat/controllers/block.controller.ts | 9 ++- .../context-var.controller.spec.ts | 39 ++++++------ .../controllers/message.controller.spec.ts | 48 +++++++-------- .../controllers/subscriber.controller.spec.ts | 25 ++++---- .../chat/controllers/subscriber.controller.ts | 4 +- api/src/chat/dto/block.dto.ts | 2 +- .../repositories/block.repository.spec.ts | 21 ++++--- .../repositories/context-var.repository.ts | 2 +- .../repositories/message.repository.spec.ts | 19 +++--- .../subscriber.repository.spec.ts | 10 +-- .../repositories/subscriber.repository.ts | 10 +-- api/src/chat/schemas/block.schema.ts | 6 +- api/src/chat/schemas/subscriber.schema.ts | 6 +- api/src/chat/services/block.service.spec.ts | 50 +++++++-------- api/src/chat/services/bot.service.spec.ts | 20 +++--- api/src/chat/services/bot.service.ts | 20 +++--- api/src/chat/services/context-var.service.ts | 2 +- api/src/chat/services/conversation.service.ts | 15 +++-- api/src/chat/services/message.service.spec.ts | 47 +++++++------- api/src/chat/services/message.service.ts | 4 +- .../chat/services/subscriber.service.spec.ts | 10 +-- api/src/chat/services/subscriber.service.ts | 2 +- .../channels/web/base-web-channel.ts | 37 ++++++++--- api/src/nlp/services/nlp.service.ts | 19 ++++-- api/src/utils/test/fixtures/block.ts | 3 +- api/src/utils/test/mocks/block.ts | 32 +++++----- api/src/utils/test/sort.ts | 6 +- api/src/utils/types/filter.types.ts | 8 ++- api/types/event-emitter.d.ts | 2 +- 31 files changed, 302 insertions(+), 241 deletions(-) diff --git a/api/src/channel/channel.service.ts b/api/src/channel/channel.service.ts index 66745c97..4170bdd7 100644 --- a/api/src/channel/channel.service.ts +++ b/api/src/channel/channel.service.ts @@ -163,8 +163,8 @@ export class ChannelService { { id: req.session.passport.user.id, foreign_id: req.session.passport.user.id, - first_name: req.session.passport.user.first_name, - last_name: req.session.passport.user.last_name, + first_name: req.session.passport.user.first_name || 'Anonymous', + last_name: req.session.passport.user.last_name || 'Anonymous', locale: '', language: '', gender: '', diff --git a/api/src/chat/controllers/block.controller.spec.ts b/api/src/chat/controllers/block.controller.spec.ts index 403744b0..a68dc7d9 100644 --- a/api/src/chat/controllers/block.controller.spec.ts +++ b/api/src/chat/controllers/block.controller.spec.ts @@ -61,11 +61,11 @@ describe('BlockController', () => { let blockController: BlockController; let blockService: BlockService; let categoryService: CategoryService; - let category: Category; - let block: Block; - let blockToDelete: Block; - let hasNextBlocks: Block; - let hasPreviousBlocks: Block; + let category: Category | null; + let block: Block | null; + let blockToDelete: Block | null; + let hasNextBlocks: Block | null; + let hasPreviousBlocks: Block | null; const FIELDS_TO_POPULATE = [ 'trigger_labels', 'assign_labels', @@ -162,9 +162,9 @@ describe('BlockController', () => { const result = await blockController.find([], {}); const blocksWithCategory = blockFixtures.map((blockFixture) => ({ ...blockFixture, - category: category.id, + category: category!.id, nextBlocks: - blockFixture.name === 'hasNextBlocks' ? [hasPreviousBlocks.id] : [], + blockFixture.name === 'hasNextBlocks' ? [hasPreviousBlocks!.id] : [], })); expect(blockService.find).toHaveBeenCalledWith({}, undefined); @@ -185,6 +185,7 @@ describe('BlockController', () => { blockFixture.name === 'hasPreviousBlocks' ? [hasNextBlocks] : [], nextBlocks: blockFixture.name === 'hasNextBlocks' ? [hasPreviousBlocks] : [], + attachedToBlock: null, })); expect(blockService.findAndPopulate).toHaveBeenCalledWith({}, undefined); @@ -195,13 +196,13 @@ describe('BlockController', () => { describe('findOne', () => { it('should find one block by id', async () => { jest.spyOn(blockService, 'findOne'); - const result = await blockController.findOne(hasNextBlocks.id, []); - expect(blockService.findOne).toHaveBeenCalledWith(hasNextBlocks.id); + const result = await blockController.findOne(hasNextBlocks!.id, []); + expect(blockService.findOne).toHaveBeenCalledWith(hasNextBlocks!.id); expect(result).toEqualPayload( { - ...blockFixtures.find(({ name }) => name === hasNextBlocks.name), - category: category.id, - nextBlocks: [hasPreviousBlocks.id], + ...blockFixtures.find(({ name }) => name === hasNextBlocks!.name), + category: category!.id, + nextBlocks: [hasPreviousBlocks!.id], }, [...IGNORED_TEST_FIELDS, 'attachedToBlock'], ); @@ -210,16 +211,17 @@ describe('BlockController', () => { it('should find one block by id, and populate its category and previousBlocks', async () => { jest.spyOn(blockService, 'findOneAndPopulate'); const result = await blockController.findOne( - hasPreviousBlocks.id, + hasPreviousBlocks!.id, FIELDS_TO_POPULATE, ); expect(blockService.findOneAndPopulate).toHaveBeenCalledWith( - hasPreviousBlocks.id, + hasPreviousBlocks!.id, ); expect(result).toEqualPayload({ ...blockFixtures.find(({ name }) => name === 'hasPreviousBlocks'), category, previousBlocks: [hasNextBlocks], + attachedToBlock: null, }); }); @@ -227,14 +229,15 @@ describe('BlockController', () => { jest.spyOn(blockService, 'findOneAndPopulate'); block = await blockService.findOne({ name: 'attachment' }); const result = await blockController.findOne( - block.id, + block!.id, FIELDS_TO_POPULATE, ); - expect(blockService.findOneAndPopulate).toHaveBeenCalledWith(block.id); + expect(blockService.findOneAndPopulate).toHaveBeenCalledWith(block!.id); expect(result).toEqualPayload({ ...blockFixtures.find(({ name }) => name === 'attachment'), category, previousBlocks: [], + attachedToBlock: null, }); }); }); @@ -244,12 +247,12 @@ describe('BlockController', () => { jest.spyOn(blockService, 'create'); const mockedBlockCreateDto: BlockCreateDto = { name: 'block with nextBlocks', - nextBlocks: [hasNextBlocks.id], + nextBlocks: [hasNextBlocks!.id], patterns: ['Hi'], trigger_labels: [], assign_labels: [], trigger_channels: [], - category: category.id, + category: category!.id, options: { typing: 0, fallback: { @@ -281,15 +284,17 @@ describe('BlockController', () => { describe('deleteOne', () => { it('should delete block', async () => { jest.spyOn(blockService, 'deleteOne'); - const result = await blockController.deleteOne(blockToDelete.id); + const result = await blockController.deleteOne(blockToDelete!.id); - expect(blockService.deleteOne).toHaveBeenCalledWith(blockToDelete.id); + expect(blockService.deleteOne).toHaveBeenCalledWith(blockToDelete!.id); expect(result).toEqual({ acknowledged: true, deletedCount: 1 }); }); it('should throw NotFoundException when attempting to delete a block by id', async () => { - await expect(blockController.deleteOne(blockToDelete.id)).rejects.toThrow( - new NotFoundException(`Block with ID ${blockToDelete.id} not found`), + await expect( + blockController.deleteOne(blockToDelete!.id), + ).rejects.toThrow( + new NotFoundException(`Block with ID ${blockToDelete!.id} not found`), ); }); }); @@ -300,16 +305,16 @@ describe('BlockController', () => { const updateBlock: BlockUpdateDto = { name: 'modified block name', }; - const result = await blockController.updateOne(block.id, updateBlock); + const result = await blockController.updateOne(block!.id, updateBlock); expect(blockService.updateOne).toHaveBeenCalledWith( - block.id, + block!.id, updateBlock, ); expect(result).toEqualPayload( { - ...blockFixtures.find(({ name }) => name === block.name), - category: category.id, + ...blockFixtures.find(({ name }) => name === block!.name), + category: category!.id, ...updateBlock, }, [...IGNORED_TEST_FIELDS, 'attachedToBlock'], @@ -322,9 +327,9 @@ describe('BlockController', () => { }; await expect( - blockController.updateOne(blockToDelete.id, updateBlock), + blockController.updateOne(blockToDelete!.id, updateBlock), ).rejects.toThrow( - new NotFoundException(`Block with ID ${blockToDelete.id} not found`), + new NotFoundException(`Block with ID ${blockToDelete!.id} not found`), ); }); }); diff --git a/api/src/chat/controllers/block.controller.ts b/api/src/chat/controllers/block.controller.ts index 945032f6..949410d3 100644 --- a/api/src/chat/controllers/block.controller.ts +++ b/api/src/chat/controllers/block.controller.ts @@ -219,9 +219,12 @@ export class BlockController extends BaseController< this.validate({ dto: block, allowedIds: { - category: (await this.categoryService.findOne(block.category))?.id, - attachedBlock: (await this.blockService.findOne(block.attachedBlock)) - ?.id, + category: block.category + ? (await this.categoryService.findOne(block.category))?.id + : null, + attachedBlock: block.attachedBlock + ? (await this.blockService.findOne(block.attachedBlock))?.id + : null, nextBlocks: ( await this.blockService.find({ _id: { diff --git a/api/src/chat/controllers/context-var.controller.spec.ts b/api/src/chat/controllers/context-var.controller.spec.ts index 6098ac05..11a88eb8 100644 --- a/api/src/chat/controllers/context-var.controller.spec.ts +++ b/api/src/chat/controllers/context-var.controller.spec.ts @@ -36,8 +36,8 @@ import { ContextVarController } from './context-var.controller'; describe('ContextVarController', () => { let contextVarController: ContextVarController; let contextVarService: ContextVarService; - let contextVar: ContextVar; - let contextVarToDelete: ContextVar; + let contextVar: ContextVar | null; + let contextVarToDelete: ContextVar | null; beforeAll(async () => { const module = await Test.createTestingModule({ @@ -91,11 +91,11 @@ describe('ContextVarController', () => { describe('findOne', () => { it('should return the existing contextVar', async () => { jest.spyOn(contextVarService, 'findOne'); - const result = await contextVarController.findOne(contextVar.id); + const result = await contextVarController.findOne(contextVar!.id); - expect(contextVarService.findOne).toHaveBeenCalledWith(contextVar.id); + expect(contextVarService.findOne).toHaveBeenCalledWith(contextVar!.id); expect(result).toEqualPayload( - contextVarFixtures.find(({ label }) => label === contextVar.label), + contextVarFixtures.find(({ label }) => label === contextVar!.label)!, ); }); }); @@ -121,11 +121,11 @@ describe('ContextVarController', () => { it('should delete a contextVar by id', async () => { jest.spyOn(contextVarService, 'deleteOne'); const result = await contextVarController.deleteOne( - contextVarToDelete.id, + contextVarToDelete!.id, ); expect(contextVarService.deleteOne).toHaveBeenCalledWith( - contextVarToDelete.id, + contextVarToDelete!.id, ); expect(result).toEqual({ acknowledged: true, @@ -135,10 +135,10 @@ describe('ContextVarController', () => { it('should throw a NotFoundException when attempting to delete a contextVar by id', async () => { await expect( - contextVarController.deleteOne(contextVarToDelete.id), + contextVarController.deleteOne(contextVarToDelete!.id), ).rejects.toThrow( new NotFoundException( - `Context var with ID ${contextVarToDelete.id} not found.`, + `Context var with ID ${contextVarToDelete!.id} not found.`, ), ); }); @@ -152,12 +152,12 @@ describe('ContextVarController', () => { .spyOn(contextVarService, 'deleteMany') .mockResolvedValue(deleteResult); const result = await contextVarController.deleteMany([ - contextVarToDelete.id, - contextVar.id, + contextVarToDelete!.id, + contextVar!.id, ]); expect(contextVarService.deleteMany).toHaveBeenCalledWith({ - _id: { $in: [contextVarToDelete.id, contextVar.id] }, + _id: { $in: [contextVarToDelete!.id, contextVar!.id] }, }); expect(result).toEqual(deleteResult); }); @@ -175,7 +175,10 @@ describe('ContextVarController', () => { }); await expect( - contextVarController.deleteMany([contextVarToDelete.id, contextVar.id]), + contextVarController.deleteMany([ + contextVarToDelete!.id, + contextVar!.id, + ]), ).rejects.toThrow( new NotFoundException('Context vars with provided IDs not found'), ); @@ -189,16 +192,16 @@ describe('ContextVarController', () => { it('should return updated contextVar', async () => { jest.spyOn(contextVarService, 'updateOne'); const result = await contextVarController.updateOne( - contextVar.id, + contextVar!.id, contextVarUpdatedDto, ); expect(contextVarService.updateOne).toHaveBeenCalledWith( - contextVar.id, + contextVar!.id, contextVarUpdatedDto, ); expect(result).toEqualPayload({ - ...contextVarFixtures.find(({ label }) => label === contextVar.label), + ...contextVarFixtures.find(({ label }) => label === contextVar!.label), ...contextVarUpdatedDto, }); }); @@ -206,12 +209,12 @@ describe('ContextVarController', () => { it('should throw a NotFoundException when attempting to update an non existing contextVar by id', async () => { await expect( contextVarController.updateOne( - contextVarToDelete.id, + contextVarToDelete!.id, contextVarUpdatedDto, ), ).rejects.toThrow( new NotFoundException( - `ContextVar with ID ${contextVarToDelete.id} not found`, + `ContextVar with ID ${contextVarToDelete!.id} not found`, ), ); }); diff --git a/api/src/chat/controllers/message.controller.spec.ts b/api/src/chat/controllers/message.controller.spec.ts index 0105343b..b7dad706 100644 --- a/api/src/chat/controllers/message.controller.spec.ts +++ b/api/src/chat/controllers/message.controller.spec.ts @@ -53,10 +53,10 @@ describe('MessageController', () => { let messageService: MessageService; let subscriberService: SubscriberService; let userService: UserService; - let sender: Subscriber; - let recipient: Subscriber; - let user: User; - let message: Message; + let sender: Subscriber | null; + let recipient: Subscriber | null; + let user: User | null; + let message: Message | null; let allMessages: Message[]; let allUsers: User[]; let allSubscribers: Subscriber[]; @@ -129,9 +129,9 @@ describe('MessageController', () => { subscriberService = module.get(SubscriberService); messageController = module.get(MessageController); message = await messageService.findOne({ mid: 'mid-1' }); - sender = await subscriberService.findOne(message.sender); - recipient = await subscriberService.findOne(message.recipient); - user = await userService.findOne(message.sentBy); + sender = await subscriberService.findOne(message!.sender!); + recipient = await subscriberService.findOne(message!.recipient!); + user = await userService.findOne(message!.sentBy!); allSubscribers = await subscriberService.findAll(); allUsers = await userService.findAll(); allMessages = await messageService.findAll(); @@ -153,31 +153,31 @@ describe('MessageController', () => { describe('findOne', () => { it('should find message by id, and populate its corresponding sender and recipient', async () => { jest.spyOn(messageService, 'findOneAndPopulate'); - const result = await messageController.findOne(message.id, [ + const result = await messageController.findOne(message!.id, [ 'sender', 'recipient', ]); expect(messageService.findOneAndPopulate).toHaveBeenCalledWith( - message.id, + message!.id, ); expect(result).toEqualPayload({ - ...messageFixtures.find(({ mid }) => mid === message.mid), + ...messageFixtures.find(({ mid }) => mid === message!.mid), sender, recipient, - sentBy: user.id, + sentBy: user!.id, }); }); it('should find message by id', async () => { jest.spyOn(messageService, 'findOne'); - const result = await messageController.findOne(message.id, []); + const result = await messageController.findOne(message!.id, []); - expect(messageService.findOne).toHaveBeenCalledWith(message.id); + expect(messageService.findOne).toHaveBeenCalledWith(message!.id); expect(result).toEqualPayload({ - ...messageFixtures.find(({ mid }) => mid === message.mid), - sender: sender.id, - recipient: recipient.id, - sentBy: user.id, + ...messageFixtures.find(({ mid }) => mid === message!.mid), + sender: sender!.id, + recipient: recipient!.id, + sentBy: user!.id, }); }); }); @@ -189,10 +189,10 @@ describe('MessageController', () => { const result = await messageController.findPage(pageQuery, [], {}); const messagesWithSenderAndRecipient = allMessages.map((message) => ({ ...message, - sender: allSubscribers.find(({ id }) => id === message['sender']).id, - recipient: allSubscribers.find(({ id }) => id === message['recipient']) - .id, - sentBy: allUsers.find(({ id }) => id === message['sentBy']).id, + sender: allSubscribers.find(({ id }) => id === message.sender)?.id, + recipient: allSubscribers.find(({ id }) => id === message.recipient) + ?.id, + sentBy: allUsers.find(({ id }) => id === message.sentBy)?.id, })); expect(messageService.find).toHaveBeenCalledWith({}, pageQuery); @@ -208,9 +208,9 @@ describe('MessageController', () => { ); const messages = allMessages.map((message) => ({ ...message, - sender: allSubscribers.find(({ id }) => id === message['sender']), - recipient: allSubscribers.find(({ id }) => id === message['recipient']), - sentBy: allUsers.find(({ id }) => id === message['sentBy']).id, + sender: allSubscribers.find(({ id }) => id === message.sender), + recipient: allSubscribers.find(({ id }) => id === message.recipient), + sentBy: allUsers.find(({ id }) => id === message.sentBy)?.id, })); expect(messageService.findAndPopulate).toHaveBeenCalledWith( diff --git a/api/src/chat/controllers/subscriber.controller.spec.ts b/api/src/chat/controllers/subscriber.controller.spec.ts index b5a2e6b7..af810f15 100644 --- a/api/src/chat/controllers/subscriber.controller.spec.ts +++ b/api/src/chat/controllers/subscriber.controller.spec.ts @@ -48,7 +48,7 @@ describe('SubscriberController', () => { let subscriberService: SubscriberService; let labelService: LabelService; let userService: UserService; - let subscriber: Subscriber; + let subscriber: Subscriber | null; let allLabels: Label[]; let allSubscribers: Subscriber[]; let allUsers: User[]; @@ -114,38 +114,39 @@ describe('SubscriberController', () => { describe('findOne', () => { it('should find one subscriber by id', async () => { jest.spyOn(subscriberService, 'findOne'); - const result = await subscriberService.findOne(subscriber.id); + const result = await subscriberService.findOne(subscriber!.id); const labelIDs = allLabels - .filter((label) => subscriber.labels.includes(label.id)) + .filter((label) => subscriber!.labels.includes(label.id)) .map(({ id }) => id); - expect(subscriberService.findOne).toHaveBeenCalledWith(subscriber.id); + expect(subscriberService.findOne).toHaveBeenCalledWith(subscriber!.id); expect(result).toEqualPayload({ ...subscriberFixtures.find( - ({ first_name }) => first_name === subscriber.first_name, + ({ first_name }) => first_name === subscriber!.first_name, ), labels: labelIDs, - assignedTo: allUsers.find(({ id }) => subscriber.assignedTo === id).id, + assignedTo: allUsers.find(({ id }) => subscriber!.assignedTo === id) + ?.id, }); }); it('should find one subscriber by id, and populate its corresponding labels', async () => { jest.spyOn(subscriberService, 'findOneAndPopulate'); - const result = await subscriberController.findOne(subscriber.id, [ + const result = await subscriberController.findOne(subscriber!.id, [ 'labels', ]); expect(subscriberService.findOneAndPopulate).toHaveBeenCalledWith( - subscriber.id, + subscriber!.id, ); expect(result).toEqualPayload({ ...subscriberFixtures.find( - ({ first_name }) => first_name === subscriber.first_name, + ({ first_name }) => first_name === subscriber!.first_name, ), labels: allLabels.filter((label) => - subscriber.labels.includes(label.id), + subscriber!.labels.includes(label.id), ), - assignedTo: allUsers.find(({ id }) => subscriber.assignedTo === id), + assignedTo: allUsers.find(({ id }) => subscriber!.assignedTo === id), }); }); }); @@ -177,7 +178,7 @@ describe('SubscriberController', () => { ({ labels, ...rest }) => ({ ...rest, labels: allLabels.filter((label) => labels.includes(label.id)), - assignedTo: allUsers.find(({ id }) => subscriber.assignedTo === id), + assignedTo: allUsers.find(({ id }) => subscriber!.assignedTo === id), }), ); diff --git a/api/src/chat/controllers/subscriber.controller.ts b/api/src/chat/controllers/subscriber.controller.ts index f7e53806..d0688df0 100644 --- a/api/src/chat/controllers/subscriber.controller.ts +++ b/api/src/chat/controllers/subscriber.controller.ts @@ -145,7 +145,9 @@ export class SubscriberController extends BaseController< * @returns A streamable file containing the avatar image. */ @Get(':id/profile_pic') - async getAvatar(@Param('id') id: string): Promise { + async getAvatar( + @Param('id') id: string, + ): Promise { const subscriber = await this.subscriberService.findOneAndPopulate(id); if (!subscriber) { diff --git a/api/src/chat/dto/block.dto.ts b/api/src/chat/dto/block.dto.ts index 7892f424..349df9d4 100644 --- a/api/src/chat/dto/block.dto.ts +++ b/api/src/chat/dto/block.dto.ts @@ -89,7 +89,7 @@ export class BlockCreateDto { @IsNotEmpty() @IsString() @IsObjectId({ message: 'Category must be a valid objectId' }) - category: string; + category: string | null; @ApiPropertyOptional({ description: 'Block has started conversation', diff --git a/api/src/chat/repositories/block.repository.spec.ts b/api/src/chat/repositories/block.repository.spec.ts index 472a3659..0948e427 100644 --- a/api/src/chat/repositories/block.repository.spec.ts +++ b/api/src/chat/repositories/block.repository.spec.ts @@ -31,9 +31,9 @@ describe('BlockRepository', () => { let blockRepository: BlockRepository; let categoryRepository: CategoryRepository; let blockModel: Model; - let category: Category; - let hasPreviousBlocks: Block; - let hasNextBlocks: Block; + let category: Category | null; + let hasPreviousBlocks: Block | null; + let hasNextBlocks: Block | null; let validIds: string[]; let validCategory: string; @@ -67,16 +67,19 @@ describe('BlockRepository', () => { it('should find one block by id, and populate its trigger_labels, assign_labels, nextBlocks, attachedBlock, category,previousBlocks', async () => { jest.spyOn(blockModel, 'findById'); - const result = await blockRepository.findOneAndPopulate(hasNextBlocks.id); + const result = await blockRepository.findOneAndPopulate( + hasNextBlocks!.id, + ); expect(blockModel.findById).toHaveBeenCalledWith( - hasNextBlocks.id, + hasNextBlocks!.id, undefined, ); expect(result).toEqualPayload({ - ...blockFixtures.find(({ name }) => name === hasNextBlocks.name), + ...blockFixtures.find(({ name }) => name === hasNextBlocks!.name), category, nextBlocks: [hasPreviousBlocks], previousBlocks: [], + attachedToBlock: null, }); }); }); @@ -93,6 +96,7 @@ describe('BlockRepository', () => { blockFixture.name === 'hasPreviousBlocks' ? [hasNextBlocks] : [], nextBlocks: blockFixture.name === 'hasNextBlocks' ? [hasPreviousBlocks] : [], + attachedToBlock: null, })); expect(blockModel.find).toHaveBeenCalledWith({}, undefined); @@ -110,6 +114,7 @@ describe('BlockRepository', () => { blockFixture.name === 'hasPreviousBlocks' ? [hasNextBlocks] : [], nextBlocks: blockFixture.name === 'hasNextBlocks' ? [hasPreviousBlocks] : [], + attachedToBlock: null, })); expect(blockModel.find).toHaveBeenCalledWith({}, undefined); @@ -191,7 +196,7 @@ describe('BlockRepository', () => { category: validCategory, nextBlocks: [], attachedBlock: null, - } as Block); + } as unknown as Block); const mockUpdateOne = jest.spyOn(blockRepository, 'updateOne'); @@ -233,7 +238,7 @@ describe('BlockRepository', () => { attachedBlock: null, nextBlocks: [validIds[0], validIds[1]], }, - ] as Block[]; + ] as unknown as Block[]; const mockUpdateOne = jest.spyOn(blockRepository, 'updateOne'); diff --git a/api/src/chat/repositories/context-var.repository.ts b/api/src/chat/repositories/context-var.repository.ts index f2125252..c6ef75cb 100644 --- a/api/src/chat/repositories/context-var.repository.ts +++ b/api/src/chat/repositories/context-var.repository.ts @@ -32,7 +32,7 @@ export class ContextVarRepository extends BaseRepository { @Optional() blockService?: BlockService, ) { super(eventEmitter, model, ContextVar); - this.blockService = blockService; + if (blockService) this.blockService = blockService; } /** diff --git a/api/src/chat/repositories/message.repository.spec.ts b/api/src/chat/repositories/message.repository.spec.ts index 7c3b2e62..9793f2e3 100644 --- a/api/src/chat/repositories/message.repository.spec.ts +++ b/api/src/chat/repositories/message.repository.spec.ts @@ -63,19 +63,22 @@ describe('MessageRepository', () => { it('should find one message by id, and populate its sender and recipient', async () => { jest.spyOn(messageModel, 'findById'); const message = await messageRepository.findOne({ mid: 'mid-1' }); - const sender = await subscriberRepository.findOne(message['sender']); + const sender = await subscriberRepository.findOne(message!['sender']); const recipient = await subscriberRepository.findOne( - message['recipient'], + message!['recipient'], ); - const user = await userRepository.findOne(message['sentBy']); - const result = await messageRepository.findOneAndPopulate(message.id); + const user = await userRepository.findOne(message!['sentBy']); + const result = await messageRepository.findOneAndPopulate(message!.id); - expect(messageModel.findById).toHaveBeenCalledWith(message.id, undefined); + expect(messageModel.findById).toHaveBeenCalledWith( + message!.id, + undefined, + ); expect(result).toEqualPayload({ - ...messageFixtures.find(({ mid }) => mid === message.mid), + ...messageFixtures.find(({ mid }) => mid === message!.mid), sender, recipient, - sentBy: user.id, + sentBy: user!.id, }); }); }); @@ -92,7 +95,7 @@ describe('MessageRepository', () => { ...message, sender: allSubscribers.find(({ id }) => id === message['sender']), recipient: allSubscribers.find(({ id }) => id === message['recipient']), - sentBy: allUsers.find(({ id }) => id === message['sentBy']).id, + sentBy: allUsers.find(({ id }) => id === message['sentBy'])?.id, })); expect(messageModel.find).toHaveBeenCalledWith({}, undefined); diff --git a/api/src/chat/repositories/subscriber.repository.spec.ts b/api/src/chat/repositories/subscriber.repository.spec.ts index b497002f..2b54b03d 100644 --- a/api/src/chat/repositories/subscriber.repository.spec.ts +++ b/api/src/chat/repositories/subscriber.repository.spec.ts @@ -107,20 +107,20 @@ describe('SubscriberRepository', () => { }); const allLabels = await labelRepository.findAll(); const result = await subscriberRepository.findOneAndPopulate( - subscriber.id, + subscriber!.id, ); const subscriberWithLabels = { ...subscriberFixtures.find( - ({ first_name }) => first_name === subscriber.first_name, + ({ first_name }) => first_name === subscriber!.first_name, ), labels: allLabels.filter((label) => - subscriber.labels.includes(label.id), + subscriber!.labels.includes(label.id), ), - assignedTo: allUsers.find(({ id }) => subscriber.assignedTo === id), + assignedTo: allUsers.find(({ id }) => subscriber!.assignedTo === id), }; expect(subscriberModel.findById).toHaveBeenCalledWith( - subscriber.id, + subscriber!.id, undefined, ); expect(result).toEqualPayload(subscriberWithLabels); diff --git a/api/src/chat/repositories/subscriber.repository.ts b/api/src/chat/repositories/subscriber.repository.ts index a3ee6cd1..f12b6949 100644 --- a/api/src/chat/repositories/subscriber.repository.ts +++ b/api/src/chat/repositories/subscriber.repository.ts @@ -134,7 +134,7 @@ export class SubscriberRepository extends BaseRepository< */ async findOneByForeignIdAndPopulate(id: string): Promise { const query = this.findByForeignIdQuery(id).populate(this.populate); - const [result] = await this.execute(query, this.clsPopulate); + const [result] = await this.execute(query, SubscriberFull); return result; } @@ -149,7 +149,7 @@ export class SubscriberRepository extends BaseRepository< async updateOneByForeignIdQuery( id: string, updates: SubscriberUpdateDto, - ): Promise { + ): Promise { return await this.updateOne({ foreign_id: id }, updates); } @@ -160,7 +160,9 @@ export class SubscriberRepository extends BaseRepository< * * @returns The updated subscriber entity. */ - async handBackByForeignIdQuery(foreignId: string): Promise { + async handBackByForeignIdQuery( + foreignId: string, + ): Promise { return await this.updateOne( { foreign_id: foreignId, @@ -183,7 +185,7 @@ export class SubscriberRepository extends BaseRepository< async handOverByForeignIdQuery( foreignId: string, userId: string, - ): Promise { + ): Promise { return await this.updateOne( { foreign_id: foreignId, diff --git a/api/src/chat/schemas/block.schema.ts b/api/src/chat/schemas/block.schema.ts index ab9a0b7a..7b144ad7 100644 --- a/api/src/chat/schemas/block.schema.ts +++ b/api/src/chat/schemas/block.schema.ts @@ -143,13 +143,13 @@ export class Block extends BlockStub { attachedBlock?: string; @Transform(({ obj }) => obj.category.toString()) - category: string; + category: string | null; @Exclude() previousBlocks?: never; @Exclude() - attachedToBlock?: never | null; + attachedToBlock?: never; } @Schema({ timestamps: true }) @@ -161,7 +161,7 @@ export class BlockFull extends BlockStub { assign_labels: Label[]; @Type(() => Block) - nextBlocks?: Block[]; + nextBlocks: Block[]; @Type(() => Block) attachedBlock?: Block; diff --git a/api/src/chat/schemas/subscriber.schema.ts b/api/src/chat/schemas/subscriber.schema.ts index 5302df41..f2a8e1cf 100644 --- a/api/src/chat/schemas/subscriber.schema.ts +++ b/api/src/chat/schemas/subscriber.schema.ts @@ -86,7 +86,7 @@ export class SubscriberStub extends BaseSchema { type: Date, default: null, }) - assignedAt?: Date; + assignedAt?: Date | null; @Prop({ type: Date, @@ -132,10 +132,10 @@ export class Subscriber extends SubscriberStub { labels: string[]; @Transform(({ obj }) => (obj.assignedTo ? obj.assignedTo.toString() : null)) - assignedTo?: string; + assignedTo?: string | null; @Transform(({ obj }) => obj.avatar?.toString() || null) - avatar?: string; + avatar?: string | null; } @Schema({ timestamps: true }) diff --git a/api/src/chat/services/block.service.spec.ts b/api/src/chat/services/block.service.spec.ts index e15cbc75..6e46a30f 100644 --- a/api/src/chat/services/block.service.spec.ts +++ b/api/src/chat/services/block.service.spec.ts @@ -74,10 +74,10 @@ import { CategoryService } from './category.service'; describe('BlockService', () => { let blockRepository: BlockRepository; let categoryRepository: CategoryRepository; - let category: Category; - let block: Block; + let category: Category | null; + let block: Block | null; let blockService: BlockService; - let hasPreviousBlocks: Block; + let hasPreviousBlocks: Block | null; let contentService: ContentService; let contentTypeService: ContentTypeService; let settingService: SettingService; @@ -168,10 +168,10 @@ describe('BlockService', () => { describe('findOneAndPopulate', () => { it('should find one block by id, and populate its trigger_labels, assign_labels,attachedBlock,category,nextBlocks', async () => { jest.spyOn(blockRepository, 'findOneAndPopulate'); - const result = await blockService.findOneAndPopulate(block.id); + const result = await blockService.findOneAndPopulate(block!.id); expect(blockRepository.findOneAndPopulate).toHaveBeenCalledWith( - block.id, + block!.id, undefined, ); expect(result).toEqualPayload({ @@ -179,6 +179,7 @@ describe('BlockService', () => { category, nextBlocks: [hasPreviousBlocks], previousBlocks: [], + attachedToBlock: null, }); }); }); @@ -194,6 +195,7 @@ describe('BlockService', () => { blockFixture.name === 'hasPreviousBlocks' ? [block] : [], nextBlocks: blockFixture.name === 'hasNextBlocks' ? [hasPreviousBlocks] : [], + attachedToBlock: null, })); expect(blockRepository.findAndPopulate).toHaveBeenCalledWith( @@ -380,7 +382,7 @@ describe('BlockService', () => { }, blockGetStarted, ); - expect(result).toEqual(blockGetStarted.patterns[3]); + expect(result).toEqual(blockGetStarted.patterns?.[3]); }); it("should match payload when it's an attachment file", () => { @@ -396,7 +398,7 @@ describe('BlockService', () => { }, blockGetStarted, ); - expect(result).toEqual(blockGetStarted.patterns[4]); + expect(result).toEqual(blockGetStarted.patterns?.[4]); }); }); @@ -439,7 +441,7 @@ describe('BlockService', () => { describe('processMessage', () => { it('should process list message (with limit = 2 and skip = 0)', async () => { const contentType = await contentTypeService.findOne({ name: 'Product' }); - blockProductListMock.options.content.entity = contentType.id; + blockProductListMock.options!.content!.entity = contentType!.id; const result = await blockService.processMessage( blockProductListMock, { @@ -451,27 +453,27 @@ describe('BlockService', () => { 'conv_id', ); const elements = await contentService.findPage( - { status: true, entity: contentType.id }, + { status: true, entity: contentType!.id }, { skip: 0, limit: 2, sort: ['createdAt', 'desc'] }, ); const flattenedElements = elements.map(Content.toElement); - expect(result.format).toEqualPayload( - blockProductListMock.options.content?.display, + expect(result!.format).toEqualPayload( + blockProductListMock.options!.content!.display, ); expect( - (result.message as StdOutgoingListMessage).elements, + (result!.message as StdOutgoingListMessage).elements, ).toEqualPayload(flattenedElements); - expect((result.message as StdOutgoingListMessage).options).toEqualPayload( - blockProductListMock.options.content, - ); expect( - (result.message as StdOutgoingListMessage).pagination, + (result!.message as StdOutgoingListMessage).options, + ).toEqualPayload(blockProductListMock.options!.content!); + expect( + (result!.message as StdOutgoingListMessage).pagination, ).toEqualPayload({ total: 4, skip: 0, limit: 2 }); }); it('should process list message (with limit = 2 and skip = 2)', async () => { const contentType = await contentTypeService.findOne({ name: 'Product' }); - blockProductListMock.options.content.entity = contentType.id; + blockProductListMock.options!.content!.entity = contentType!.id; const result = await blockService.processMessage( blockProductListMock, { @@ -483,20 +485,20 @@ describe('BlockService', () => { 'conv_id', ); const elements = await contentService.findPage( - { status: true, entity: contentType.id }, + { status: true, entity: contentType!.id }, { skip: 2, limit: 2, sort: ['createdAt', 'desc'] }, ); const flattenedElements = elements.map(Content.toElement); - expect(result.format).toEqual( - blockProductListMock.options.content?.display, + expect(result!.format).toEqual( + blockProductListMock.options!.content?.display, ); - expect((result.message as StdOutgoingListMessage).elements).toEqual( + expect((result!.message as StdOutgoingListMessage).elements).toEqual( flattenedElements, ); - expect((result.message as StdOutgoingListMessage).options).toEqual( - blockProductListMock.options.content, + expect((result!.message as StdOutgoingListMessage).options).toEqual( + blockProductListMock.options!.content, ); - expect((result.message as StdOutgoingListMessage).pagination).toEqual({ + expect((result!.message as StdOutgoingListMessage).pagination).toEqual({ total: 4, skip: 2, limit: 2, diff --git a/api/src/chat/services/bot.service.spec.ts b/api/src/chat/services/bot.service.spec.ts index bf71c0b3..dcebfa35 100644 --- a/api/src/chat/services/bot.service.spec.ts +++ b/api/src/chat/services/bot.service.spec.ts @@ -197,7 +197,7 @@ describe('BlockService', () => { foreign_id: 'foreign-id-web-1', }); - event.setSender(webSubscriber); + event.setSender(webSubscriber!); let hasBotSpoken = false; const clearMock = jest @@ -210,15 +210,15 @@ describe('BlockService', () => { isFallback: boolean, ) => { expect(actualConversation).toEqualPayload({ - sender: webSubscriber.id, + sender: webSubscriber!.id, active: true, next: [], context: { user: { - first_name: webSubscriber.first_name, - last_name: webSubscriber.last_name, + first_name: webSubscriber!.first_name, + last_name: webSubscriber!.last_name, language: 'en', - id: webSubscriber.id, + id: webSubscriber!.id, }, user_location: { lat: 0, @@ -263,7 +263,7 @@ describe('BlockService', () => { const webSubscriber = await subscriberService.findOne({ foreign_id: 'foreign-id-web-1', }); - event.setSender(webSubscriber); + event.setSender(webSubscriber!); const clearMock = jest .spyOn(botService, 'handleIncomingMessage') @@ -278,10 +278,10 @@ describe('BlockService', () => { active: true, context: { user: { - first_name: webSubscriber.first_name, - last_name: webSubscriber.last_name, + first_name: webSubscriber!.first_name, + last_name: webSubscriber!.last_name, language: 'en', - id: webSubscriber.id, + id: webSubscriber!.id, }, user_location: { lat: 0, lon: 0 }, vars: {}, @@ -317,7 +317,7 @@ describe('BlockService', () => { const webSubscriber = await subscriberService.findOne({ foreign_id: 'foreign-id-web-2', }); - event.setSender(webSubscriber); + event.setSender(webSubscriber!); const captured = await botService.processConversationMessage(event); expect(captured).toBe(false); diff --git a/api/src/chat/services/bot.service.ts b/api/src/chat/services/bot.service.ts index 93cb4f1b..6926b2ad 100644 --- a/api/src/chat/services/bot.service.ts +++ b/api/src/chat/services/bot.service.ts @@ -25,6 +25,7 @@ import { IncomingMessageType, StdOutgoingEnvelope, } from '../schemas/types/message'; +import { SubscriberContext } from '../schemas/types/subscriberContext'; import { BlockService } from './block.service'; import { ConversationService } from './conversation.service'; @@ -70,14 +71,13 @@ export class BotService { ); // Process message : Replace tokens with context data and then send the message const recipient = event.getSender(); - const envelope: StdOutgoingEnvelope = - await this.blockService.processMessage( - block, - context, - recipient?.context, - fallback, - conservationId, - ); + const envelope = (await this.blockService.processMessage( + block, + context, + recipient?.context as SubscriberContext, + fallback, + conservationId, + )) as StdOutgoingEnvelope; // Send message through the right channel const response = await event @@ -252,13 +252,13 @@ export class BotService { assign_labels: [], trigger_labels: [], attachedBlock: undefined, - category: undefined, + category: undefined as any, previousBlocks: [], }; convo.context.attempt++; fallback = true; } else { - convo.context.attempt = 0; + if (convo.context) convo.context.attempt = 0; fallbackBlock = undefined; } diff --git a/api/src/chat/services/context-var.service.ts b/api/src/chat/services/context-var.service.ts index dfcdd164..d467c304 100644 --- a/api/src/chat/services/context-var.service.ts +++ b/api/src/chat/services/context-var.service.ts @@ -30,7 +30,7 @@ export class ContextVarService extends BaseService { block: Block | BlockFull, ): Promise> { const vars = await this.find({ - name: { $in: block.capture_vars.map(({ context_var }) => context_var) }, + name: { $in: block.capture_vars?.map(({ context_var }) => context_var) }, }); return vars.reduce((acc, cv) => { acc[cv.name] = cv; diff --git a/api/src/chat/services/conversation.service.ts b/api/src/chat/services/conversation.service.ts index fdd65cd2..da9f7a7a 100644 --- a/api/src/chat/services/conversation.service.ts +++ b/api/src/chat/services/conversation.service.ts @@ -68,6 +68,9 @@ export class ConversationService extends BaseService< ) { const msgType = event.getMessageType(); const profile = event.getSender(); + + if (!convo.context) throw new Error('Missing conversation context'); + // Capture channel specific context data convo.context.channel = event.getHandler().getName(); convo.context.text = event.getText(); @@ -81,7 +84,7 @@ export class ConversationService extends BaseService< // Capture user entry in context vars if (captureVars && next.capture_vars && next.capture_vars.length > 0) { next.capture_vars.forEach((capture) => { - let contextValue: string | Payload; + let contextValue: string | Payload | undefined; const nlp = event.getNLP(); @@ -103,7 +106,7 @@ export class ConversationService extends BaseService< if (capture.entity === -1) { // Capture the whole message contextValue = - ['message', 'quick_reply'].indexOf(msgType) !== -1 + msgType && ['message', 'quick_reply'].indexOf(msgType) !== -1 ? event.getText() : event.getPayload(); } else if (capture.entity === -2) { @@ -113,13 +116,16 @@ export class ConversationService extends BaseService< contextValue = typeof contextValue === 'string' ? contextValue.trim() : contextValue; - if (contextVars[capture.context_var]?.permanent) { + if ( + profile.context?.vars && + contextVars[capture.context_var]?.permanent + ) { Logger.debug( `Adding context var to subscriber: ${capture.context_var} = ${contextValue}`, ); profile.context.vars[capture.context_var] = contextValue; } else { - convo.context.vars[capture.context_var] = contextValue; + convo.context!.vars[capture.context_var] = contextValue; } }); } @@ -158,6 +164,7 @@ export class ConversationService extends BaseService< // Deal with load more in the case of a list display if ( + next.options && next.options.content && (next.options.content.display === OutgoingMessageFormat.list || next.options.content.display === OutgoingMessageFormat.carousel) diff --git a/api/src/chat/services/message.service.spec.ts b/api/src/chat/services/message.service.spec.ts index e464cb2e..07f2b8e9 100644 --- a/api/src/chat/services/message.service.spec.ts +++ b/api/src/chat/services/message.service.spec.ts @@ -48,11 +48,11 @@ describe('MessageService', () => { let allMessages: Message[]; let allSubscribers: Subscriber[]; let allUsers: User[]; - let message: Message; - let sender: Subscriber; - let recipient: Subscriber; + let message: Message | null; + let sender: Subscriber | null; + let recipient: Subscriber | null; let messagesWithSenderAndRecipient: Message[]; - let user: User; + let user: User | null; beforeAll(async () => { const module = await Test.createTestingModule({ @@ -91,15 +91,14 @@ describe('MessageService', () => { allUsers = await userRepository.findAll(); allMessages = await messageRepository.findAll(); message = await messageRepository.findOne({ mid: 'mid-1' }); - sender = await subscriberRepository.findOne(message['sender']); - recipient = await subscriberRepository.findOne(message['recipient']); - user = await userRepository.findOne(message['sentBy']); + sender = await subscriberRepository.findOne(message!.sender!); + recipient = await subscriberRepository.findOne(message!.recipient!); + user = await userRepository.findOne(message!.sentBy!); messagesWithSenderAndRecipient = allMessages.map((message) => ({ ...message, - sender: allSubscribers.find(({ id }) => id === message['sender']).id, - recipient: allSubscribers.find(({ id }) => id === message['recipient']) - .id, - sentBy: allUsers.find(({ id }) => id === message['sentBy']).id, + sender: allSubscribers.find(({ id }) => id === message.sender)?.id, + recipient: allSubscribers.find(({ id }) => id === message.recipient)?.id, + sentBy: allUsers.find(({ id }) => id === message.sentBy)?.id, })); }); @@ -109,17 +108,17 @@ describe('MessageService', () => { describe('findOneAndPopulate', () => { it('should find message by id, and populate its corresponding sender and recipient', async () => { jest.spyOn(messageRepository, 'findOneAndPopulate'); - const result = await messageService.findOneAndPopulate(message.id); + const result = await messageService.findOneAndPopulate(message!.id); expect(messageRepository.findOneAndPopulate).toHaveBeenCalledWith( - message.id, + message!.id, undefined, ); expect(result).toEqualPayload({ - ...messageFixtures.find(({ mid }) => mid === message.mid), + ...messageFixtures.find(({ mid }) => mid === message!.mid), sender, recipient, - sentBy: user.id, + sentBy: user!.id, }); }); }); @@ -131,9 +130,9 @@ describe('MessageService', () => { const result = await messageService.findPageAndPopulate({}, pageQuery); const messagesWithSenderAndRecipient = allMessages.map((message) => ({ ...message, - sender: allSubscribers.find(({ id }) => id === message['sender']), - recipient: allSubscribers.find(({ id }) => id === message['recipient']), - sentBy: allUsers.find(({ id }) => id === message['sentBy']).id, + sender: allSubscribers.find(({ id }) => id === message.sender), + recipient: allSubscribers.find(({ id }) => id === message.recipient), + sentBy: allUsers.find(({ id }) => id === message.sentBy)?.id, })); expect(messageRepository.findPageAndPopulate).toHaveBeenCalledWith( @@ -150,7 +149,7 @@ describe('MessageService', () => { new Date().setMonth(new Date().getMonth() + 1), ); const result = await messageService.findHistoryUntilDate( - sender, + sender!, until, 30, ); @@ -166,16 +165,16 @@ describe('MessageService', () => { it('should return history since given date', async () => { const since: Date = new Date(); const result = await messageService.findHistorySinceDate( - sender, + sender!, since, 30, ); const messagesWithSenderAndRecipient = allMessages.map((message) => ({ ...message, - sender: allSubscribers.find(({ id }) => id === message['sender']).id, - recipient: allSubscribers.find(({ id }) => id === message['recipient']) - .id, - sentBy: allUsers.find(({ id }) => id === message['sentBy']).id, + sender: allSubscribers.find(({ id }) => id === message.sender)?.id, + recipient: allSubscribers.find(({ id }) => id === message.recipient) + ?.id, + sentBy: allUsers.find(({ id }) => id === message.sentBy)?.id, })); const historyMessages = messagesWithSenderAndRecipient.filter( (message) => message.createdAt > since, diff --git a/api/src/chat/services/message.service.ts b/api/src/chat/services/message.service.ts index 67474daa..6ee4463c 100644 --- a/api/src/chat/services/message.service.ts +++ b/api/src/chat/services/message.service.ts @@ -46,8 +46,8 @@ export class MessageService extends BaseService< @Optional() gateway?: WebsocketGateway, ) { super(messageRepository); - this.logger = logger; - this.gateway = gateway; + if (logger) this.logger = logger; + if (gateway) this.gateway = gateway; } /** diff --git a/api/src/chat/services/subscriber.service.spec.ts b/api/src/chat/services/subscriber.service.spec.ts index bae61da4..ce93c156 100644 --- a/api/src/chat/services/subscriber.service.spec.ts +++ b/api/src/chat/services/subscriber.service.spec.ts @@ -93,17 +93,17 @@ describe('SubscriberService', () => { const subscriber = await subscriberRepository.findOne({ first_name: 'Jhon', }); - const result = await subscriberService.findOneAndPopulate(subscriber.id); + const result = await subscriberService.findOneAndPopulate(subscriber!.id); expect(subscriberService.findOneAndPopulate).toHaveBeenCalledWith( - subscriber.id, + subscriber!.id, ); expect(result).toEqualPayload({ ...subscriber, labels: allLabels.filter((label) => - subscriber.labels.includes(label.id), + subscriber!.labels.includes(label.id), ), - assignedTo: allUsers.find(({ id }) => subscriber.assignedTo === id), + assignedTo: allUsers.find(({ id }) => subscriber!.assignedTo === id), }); }); }); @@ -139,7 +139,7 @@ describe('SubscriberService', () => { expect(result).toEqualPayload({ ...subscriber, labels: allLabels - .filter((label) => subscriber.labels.includes(label.id)) + .filter((label) => subscriber!.labels.includes(label.id)) .map((label) => label.id), }); }); diff --git a/api/src/chat/services/subscriber.service.ts b/api/src/chat/services/subscriber.service.ts index d5b688a9..aa17028c 100644 --- a/api/src/chat/services/subscriber.service.ts +++ b/api/src/chat/services/subscriber.service.ts @@ -51,7 +51,7 @@ export class SubscriberService extends BaseService< @Optional() gateway?: WebsocketGateway, ) { super(repository); - this.gateway = gateway; + if (gateway) this.gateway = gateway; } /** diff --git a/api/src/extensions/channels/web/base-web-channel.ts b/api/src/extensions/channels/web/base-web-channel.ts index 273a3429..2d4123f8 100644 --- a/api/src/extensions/channels/web/base-web-channel.ts +++ b/api/src/extensions/channels/web/base-web-channel.ts @@ -1,12 +1,12 @@ /* - * Copyright © 2025 Hexastack. All rights reserved. + * Copyright © 2024 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. * 2. All derivative works must include clear attribution to the original creator and software, Hexastack and Hexabot, in a prominent location (e.g., in the software's "About" section, documentation, and README file). */ -import { Injectable } from '@nestjs/common'; +import { BadRequestException, Injectable } from '@nestjs/common'; import { EventEmitter2, OnEvent } from '@nestjs/event-emitter'; import { Request, Response } from 'express'; import multer, { diskStorage, memoryStorage } from 'multer'; @@ -231,7 +231,7 @@ export default abstract class BaseWebChannelHandler< ...message, author: 'chatbot', read: true, // Temporary fix as read is false in the bd - mid: anyMessage.mid, + mid: anyMessage.mid || 'DEFAULT_MID', handover: !!anyMessage.handover, createdAt: anyMessage.createdAt, }); @@ -519,6 +519,9 @@ export default abstract class BaseWebChannelHandler< const fetchMessages = async (req: Request, res: Response, retrials = 1) => { try { + if (!req.query.since) + throw new BadRequestException(`QueryParam 'since' is missing`); + const since = new Date(req.query.since.toString()); const messages = await this.pollMessages(req, since); if (messages.length === 0 && retrials <= 5) { @@ -630,10 +633,12 @@ export default abstract class BaseWebChannelHandler< size: Buffer.byteLength(data.file), type: data.type, }); - next(null, { - type: Attachment.getTypeByMime(attachment.type), - url: Attachment.getAttachmentUrl(attachment.id, attachment.name), - }); + + if (attachment) + next(null, { + type: Attachment.getTypeByMime(attachment.type), + url: Attachment.getAttachmentUrl(attachment.id, attachment?.name), + }); } catch (err) { this.logger.error( 'Web Channel Handler : Unable to write uploaded file', @@ -677,6 +682,12 @@ export default abstract class BaseWebChannelHandler< size: file.size, type: file.mimetype, }); + if (!attachment) { + this.logger.debug( + 'Web Channel Handler : failed to store attachment', + ); + return next(null); + } next(null, { type: Attachment.getTypeByMime(attachment.type), url: Attachment.getAttachmentUrl(attachment.id, attachment.name), @@ -721,7 +732,7 @@ export default abstract class BaseWebChannelHandler< return { isSocket: this.isSocketRequest(req), ipAddress: this.getIpAddress(req), - agent: req.headers['user-agent'], + agent: req.headers['user-agent'] || 'browser', }; } @@ -965,11 +976,17 @@ export default abstract class BaseWebChannelHandler< type: Web.OutgoingMessageType.file, data: { type: message.attachment.type, - url: message.attachment.payload.url, + url: message.attachment.payload.url!, }, }; if (message.quickReplies && message.quickReplies.length > 0) { - payload.data.quick_replies = message.quickReplies; + return { + ...payload, + data: { + ...payload.data, + quick_replies: message.quickReplies, + } as Web.OutgoingFileMessageData, + }; } return payload; } diff --git a/api/src/nlp/services/nlp.service.ts b/api/src/nlp/services/nlp.service.ts index e8fe92ad..4b4cae0a 100644 --- a/api/src/nlp/services/nlp.service.ts +++ b/api/src/nlp/services/nlp.service.ts @@ -6,7 +6,7 @@ * 2. All derivative works must include clear attribution to the original creator and software, Hexastack and Hexabot, in a prominent location (e.g., in the software's "About" section, documentation, and README file). */ -import { Injectable } from '@nestjs/common'; +import { Injectable, NotFoundException } from '@nestjs/common'; import { OnEvent } from '@nestjs/event-emitter'; import { HelperService } from '@/helper/helper.service'; @@ -77,9 +77,14 @@ export class NlpService { async handleEntityDelete(entity: NlpEntity) { // Synchonize new entity with NLP provider try { - const helper = await this.helperService.getDefaultNluHelper(); - await helper.deleteEntity(entity.foreign_id); - this.logger.debug('Deleted entity successfully synced!', entity); + if (entity.foreign_id) { + const helper = await this.helperService.getDefaultNluHelper(); + await helper.deleteEntity(entity.foreign_id); + this.logger.debug('Deleted entity successfully synced!', entity); + } else { + this.logger.error(`Entity ${entity} is missing foreign_id`); + throw new NotFoundException(`Entity ${entity} is missing foreign_id`); + } } catch (err) { this.logger.error('Unable to sync deleted entity', err); } @@ -138,8 +143,10 @@ export class NlpService { const populatedValue = await this.nlpValueService.findOneAndPopulate( value.id, ); - await helper.deleteValue(populatedValue); - this.logger.debug('Deleted value successfully synced!', value); + if (populatedValue) { + await helper.deleteValue(populatedValue); + this.logger.debug('Deleted value successfully synced!', value); + } } catch (err) { this.logger.error('Unable to sync deleted value', err); } diff --git a/api/src/utils/test/fixtures/block.ts b/api/src/utils/test/fixtures/block.ts index b66611ce..ec9ea23f 100644 --- a/api/src/utils/test/fixtures/block.ts +++ b/api/src/utils/test/fixtures/block.ts @@ -9,7 +9,7 @@ import mongoose from 'mongoose'; import { BlockCreateDto } from '@/chat/dto/block.dto'; -import { BlockModel, Block } from '@/chat/schemas/block.schema'; +import { Block, BlockModel } from '@/chat/schemas/block.schema'; import { CategoryModel } from '@/chat/schemas/category.schema'; import { FileType } from '@/chat/schemas/types/attachment'; import { ButtonType } from '@/chat/schemas/types/button'; @@ -171,7 +171,6 @@ export const blockDefaultValues: TFixturesDefaultValues = { assign_labels: [], trigger_labels: [], starts_conversation: false, - attachedToBlock: null, }; export const blockFixtures = getFixturesWithDefaultValues({ diff --git a/api/src/utils/test/mocks/block.ts b/api/src/utils/test/mocks/block.ts index 1c8d3c58..6a7c2c6e 100644 --- a/api/src/utils/test/mocks/block.ts +++ b/api/src/utils/test/mocks/block.ts @@ -98,22 +98,22 @@ export const baseBlockInstance = { ...modelInstance, }; -export const blockEmpty: BlockFull = { +export const blockEmpty = { ...baseBlockInstance, name: 'Empty', patterns: [], message: [''], -}; +} as unknown as BlockFull; // Translation Data export const textResult = ['Hi back !']; -export const textBlock: BlockFull = { +export const textBlock = { name: 'message', patterns: ['Hi'], message: textResult, ...baseBlockInstance, -}; +} as unknown as BlockFull; export const quickRepliesResult = [ "What's your favorite color?", @@ -122,7 +122,7 @@ export const quickRepliesResult = [ 'Red', ]; -export const quickRepliesBlock: BlockFull = { +export const quickRepliesBlock = { name: 'message', patterns: ['colors'], message: { @@ -146,7 +146,7 @@ export const quickRepliesBlock: BlockFull = { ], }, ...baseBlockInstance, -}; +} as unknown as BlockFull; export const buttonsResult = [ 'What would you like to know about us?', @@ -155,7 +155,7 @@ export const buttonsResult = [ 'Approach', ]; -export const buttonsBlock: BlockFull = { +export const buttonsBlock = { name: 'message', patterns: ['about'], message: { @@ -179,9 +179,9 @@ export const buttonsBlock: BlockFull = { ], }, ...baseBlockInstance, -}; +} as unknown as BlockFull; -export const attachmentBlock: BlockFull = { +export const attachmentBlock = { name: 'message', patterns: ['image'], message: { @@ -195,7 +195,7 @@ export const attachmentBlock: BlockFull = { quickReplies: [], }, ...baseBlockInstance, -}; +} as unknown as BlockFull; export const allBlocksStringsResult = [ 'Hi back !', @@ -214,7 +214,7 @@ export const allBlocksStringsResult = [ ///////// -export const blockGetStarted: BlockFull = { +export const blockGetStarted = { ...baseBlockInstance, name: 'Get Started', patterns: [ @@ -245,7 +245,7 @@ export const blockGetStarted: BlockFull = { ], trigger_labels: customerLabelsMock, message: ['Welcome! How are you ? '], -}; +} as unknown as BlockFull; const patternsProduct: Pattern[] = [ 'produit', @@ -262,7 +262,7 @@ const patternsProduct: Pattern[] = [ ], ]; -export const blockProductListMock: BlockFull = { +export const blockProductListMock = { ...baseBlockInstance, name: 'test_list', patterns: patternsProduct, @@ -278,11 +278,11 @@ export const blockProductListMock: BlockFull = { limit: 0, }, }, -}; +} as unknown as BlockFull; -export const blockCarouselMock: BlockFull = { +export const blockCarouselMock = { ...blockProductListMock, options: blockCarouselOptions, -}; +} as unknown as BlockFull; export const blocks: BlockFull[] = [blockGetStarted, blockEmpty]; diff --git a/api/src/utils/test/sort.ts b/api/src/utils/test/sort.ts index ddffd00f..6cd91662 100644 --- a/api/src/utils/test/sort.ts +++ b/api/src/utils/test/sort.ts @@ -13,14 +13,16 @@ type TSortProps = { order?: 'desc' | 'asc'; }; -const sort = ({ +type TCreateAt = { createdAt?: string | Date }; + +const sort = ({ row1, row2, field = 'createdAt', order = 'desc', }: TSortProps) => (order === 'asc' && row1[field] > row2[field] ? 1 : -1); -export const sortRowsBy = ( +export const sortRowsBy = ( row1: T, row2: T, field?: keyof T, diff --git a/api/src/utils/types/filter.types.ts b/api/src/utils/types/filter.types.ts index 764d5542..e859d3a7 100644 --- a/api/src/utils/types/filter.types.ts +++ b/api/src/utils/types/filter.types.ts @@ -65,12 +65,16 @@ type TAllowedKeys = { >]: TValue; }; +type TVirtualFields = Pick>; + export type TValidateProps = { dto: | Partial> | Partial>; - allowedIds: TAllowedKeys & - TAllowedKeys; + allowedIds: Omit< + TAllowedKeys, + keyof TVirtualFields + >; }; //populate types diff --git a/api/types/event-emitter.d.ts b/api/types/event-emitter.d.ts index a39d15cf..fadb4d6e 100644 --- a/api/types/event-emitter.d.ts +++ b/api/types/event-emitter.d.ts @@ -93,7 +93,7 @@ declare module '@nestjs/event-emitter' { object, { block: BlockFull; - passation: Subscriber; + passation: Subscriber | null; 'fallback-local': BlockFull; 'fallback-global': EventWrapper; intervention: Subscriber; From 2e269d390e63f52a3f95aebb37426d22a8f24729 Mon Sep 17 00:00:00 2001 From: yassinedorbozgithub Date: Mon, 13 Jan 2025 15:37:55 +0100 Subject: [PATCH 02/12] fix: update Chat module unit tests --- api/src/chat/dto/block.dto.ts | 2 +- api/src/chat/schemas/block.schema.ts | 12 +++++++----- api/src/chat/services/bot.service.ts | 2 +- api/src/utils/test/fixtures/block.ts | 1 - api/src/utils/test/mocks/block.ts | 2 +- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/api/src/chat/dto/block.dto.ts b/api/src/chat/dto/block.dto.ts index 68793f24..b7564714 100644 --- a/api/src/chat/dto/block.dto.ts +++ b/api/src/chat/dto/block.dto.ts @@ -84,7 +84,7 @@ export class BlockCreateDto { @IsObjectId({ message: 'Attached block must be a valid objectId', }) - attachedBlock?: string; + attachedBlock?: string | null; @ApiProperty({ description: 'Block category', type: String }) @IsNotEmpty() diff --git a/api/src/chat/schemas/block.schema.ts b/api/src/chat/schemas/block.schema.ts index e7482124..9f155d0e 100644 --- a/api/src/chat/schemas/block.schema.ts +++ b/api/src/chat/schemas/block.schema.ts @@ -139,10 +139,12 @@ export class Block extends BlockStub { @Transform(({ obj }) => obj.nextBlocks.map((elem) => elem.toString())) nextBlocks: string[]; - @Transform(({ obj }) => obj.attachedBlock?.toString() || null) - attachedBlock: string; + @Transform(({ obj }) => + obj.attachedBlock ? obj.attachedBlock.toString() : null, + ) + attachedBlock: string | null; - @Transform(({ obj }) => obj.category.toString()) + @Transform(({ obj }) => (obj.category ? obj.category.toString() : null)) category: string | null; @Exclude() @@ -164,10 +166,10 @@ export class BlockFull extends BlockStub { nextBlocks: Block[]; @Type(() => Block) - attachedBlock: Block; + attachedBlock: Block | null; @Type(() => Category) - category: Category; + category: Category | null; @Type(() => Block) previousBlocks?: Block[]; diff --git a/api/src/chat/services/bot.service.ts b/api/src/chat/services/bot.service.ts index 6926b2ad..e04ad08a 100644 --- a/api/src/chat/services/bot.service.ts +++ b/api/src/chat/services/bot.service.ts @@ -251,7 +251,7 @@ export class BotService { // If there's labels, they should be already have been assigned assign_labels: [], trigger_labels: [], - attachedBlock: undefined, + attachedBlock: null, category: undefined as any, previousBlocks: [], }; diff --git a/api/src/utils/test/fixtures/block.ts b/api/src/utils/test/fixtures/block.ts index 100aaae8..ed7c3e29 100644 --- a/api/src/utils/test/fixtures/block.ts +++ b/api/src/utils/test/fixtures/block.ts @@ -29,7 +29,6 @@ export const blockDefaultValues: TBlockFixtures['defaultValues'] = { trigger_channels: [], builtin: false, starts_conversation: false, - attachedBlock: null, }; export const blocks: TBlockFixtures['values'][] = [ diff --git a/api/src/utils/test/mocks/block.ts b/api/src/utils/test/mocks/block.ts index 864a39ee..392455e6 100644 --- a/api/src/utils/test/mocks/block.ts +++ b/api/src/utils/test/mocks/block.ts @@ -105,7 +105,7 @@ export const blockEmpty = { patterns: [], message: [''], nextBlocks: [], -}; +} as unknown as BlockFull; // Translation Data export const textResult = ['Hi back !']; From 817785ad8ce23b9d80d09747e1010648992786d7 Mon Sep 17 00:00:00 2001 From: yassinedorbozgithub Date: Mon, 13 Jan 2025 15:40:08 +0100 Subject: [PATCH 03/12] fix: update bot service currenBlock category --- api/src/chat/services/bot.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/chat/services/bot.service.ts b/api/src/chat/services/bot.service.ts index e04ad08a..7b1a71a4 100644 --- a/api/src/chat/services/bot.service.ts +++ b/api/src/chat/services/bot.service.ts @@ -252,7 +252,7 @@ export class BotService { assign_labels: [], trigger_labels: [], attachedBlock: null, - category: undefined as any, + category: null, previousBlocks: [], }; convo.context.attempt++; From 11ef58d048dede97fb1defa818cfd84ad5d6cffd Mon Sep 17 00:00:00 2001 From: yassinedorbozgithub Date: Mon, 13 Jan 2025 19:14:41 +0100 Subject: [PATCH 04/12] fix: adapt chat Module to strict null checks --- api/src/chat/schemas/types/channel.ts | 20 +++++++++----------- api/src/chat/services/chat.service.ts | 10 ++++++++-- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/api/src/chat/schemas/types/channel.ts b/api/src/chat/schemas/types/channel.ts index d53a3c60..06359493 100644 --- a/api/src/chat/schemas/types/channel.ts +++ b/api/src/chat/schemas/types/channel.ts @@ -8,14 +8,12 @@ import { ChannelName } from '@/channel/types'; -export type SubscriberChannelData< - C extends ChannelName = null, - // K extends keyof SubscriberChannelDict[C] = keyof SubscriberChannelDict[C], -> = C extends null - ? { name: ChannelName } - : { - name: C; - } & { - // Channel's specific attributes - [P in keyof SubscriberChannelDict[C]]: SubscriberChannelDict[C][P]; - }; +export type SubscriberChannelData = + C extends 'unknown-channel' + ? { name: ChannelName } + : { + name: C; + } & { + // Channel's specific attributes + [P in keyof SubscriberChannelDict[C]]: SubscriberChannelDict[C][P]; + }; diff --git a/api/src/chat/services/chat.service.ts b/api/src/chat/services/chat.service.ts index 89db5fd2..e13f4506 100644 --- a/api/src/chat/services/chat.service.ts +++ b/api/src/chat/services/chat.service.ts @@ -117,6 +117,12 @@ export class ChatService { try { const msg = await this.messageService.create(received); const populatedMsg = await this.messageService.findOneAndPopulate(msg.id); + + if (!populatedMsg) { + this.logger.warn('Unable to find populated message.', event); + throw new Error(`Unable to find Message by ID ${msg.id} not found`); + } + this.websocketGateway.broadcastMessageReceived(populatedMsg, subscriber); this.eventEmitter.emit('hook:stats:entry', 'incoming', 'Incoming'); this.eventEmitter.emit( @@ -288,7 +294,7 @@ export class ChatService { @OnEvent('hook:subscriber:postCreate') async onSubscriberCreate({ _id }: SubscriberDocument) { const subscriber = await this.subscriberService.findOne(_id); - this.websocketGateway.broadcastSubscriberNew(subscriber); + if (subscriber) this.websocketGateway.broadcastSubscriberNew(subscriber); } /** @@ -299,6 +305,6 @@ export class ChatService { @OnEvent('hook:subscriber:postUpdate') async onSubscriberUpdate({ _id }: SubscriberDocument) { const subscriber = await this.subscriberService.findOne(_id); - this.websocketGateway.broadcastSubscriberUpdate(subscriber); + if (subscriber) this.websocketGateway.broadcastSubscriberUpdate(subscriber); } } From 60677c0435b946e39c6d7fcbd2189500e855d629 Mon Sep 17 00:00:00 2001 From: yassinedorbozgithub Date: Tue, 14 Jan 2025 20:32:49 +0100 Subject: [PATCH 05/12] fix: apply feedback updates --- .../channel/lib/__test__/subscriber.mock.ts | 2 ++ api/src/chat/schemas/subscriber.schema.ts | 16 ++++----- api/src/chat/services/bot.service.spec.ts | 32 ++++++++--------- .../channels/web/base-web-channel.ts | 35 +++++++++++++------ api/src/extensions/channels/web/wrapper.ts | 15 +++++--- api/src/utils/test/mocks/subscriber.ts | 2 ++ api/src/utils/test/sort.ts | 10 ++++-- api/types/event-emitter.d.ts | 2 +- 8 files changed, 70 insertions(+), 44 deletions(-) diff --git a/api/src/channel/lib/__test__/subscriber.mock.ts b/api/src/channel/lib/__test__/subscriber.mock.ts index 5f2bda7e..ea144104 100644 --- a/api/src/channel/lib/__test__/subscriber.mock.ts +++ b/api/src/channel/lib/__test__/subscriber.mock.ts @@ -28,6 +28,8 @@ export const subscriberInstance: Subscriber = { name: 'web-channel', }, labels: [], + avatar: null, + context: {}, ...modelInstance, }; diff --git a/api/src/chat/schemas/subscriber.schema.ts b/api/src/chat/schemas/subscriber.schema.ts index f2a8e1cf..ba3ea80a 100644 --- a/api/src/chat/schemas/subscriber.schema.ts +++ b/api/src/chat/schemas/subscriber.schema.ts @@ -80,13 +80,13 @@ export class SubscriberStub extends BaseSchema { ref: 'User', default: null, }) - assignedTo?: unknown; + assignedTo: unknown; @Prop({ type: Date, default: null, }) - assignedAt?: Date | null; + assignedAt: Date | null; @Prop({ type: Date, @@ -110,13 +110,13 @@ export class SubscriberStub extends BaseSchema { ref: 'Attachment', default: null, }) - avatar?: unknown; + avatar: unknown; @Prop({ type: Object, default: { vars: {} }, }) - context?: SubscriberContext; + context: SubscriberContext; static getChannelData< C extends ChannelName, @@ -131,11 +131,11 @@ export class Subscriber extends SubscriberStub { @Transform(({ obj }) => obj.labels.map((label) => label.toString())) labels: string[]; - @Transform(({ obj }) => (obj.assignedTo ? obj.assignedTo.toString() : null)) - assignedTo?: string | null; + @Transform(({ obj }) => obj.assignedTo?.toString() || null) + assignedTo: string | null; @Transform(({ obj }) => obj.avatar?.toString() || null) - avatar?: string | null; + avatar: string | null; } @Schema({ timestamps: true }) @@ -144,7 +144,7 @@ export class SubscriberFull extends SubscriberStub { labels: Label[]; @Type(() => User) - assignedTo?: User | null; + assignedTo: User | null; @Type(() => Attachment) avatar: Attachment | null; diff --git a/api/src/chat/services/bot.service.spec.ts b/api/src/chat/services/bot.service.spec.ts index dcebfa35..c4bf517a 100644 --- a/api/src/chat/services/bot.service.spec.ts +++ b/api/src/chat/services/bot.service.spec.ts @@ -193,11 +193,11 @@ describe('BlockService', () => { }); const [block] = await blockService.findAndPopulate({ patterns: ['Hi'] }); - const webSubscriber = await subscriberService.findOne({ + const webSubscriber = (await subscriberService.findOne({ foreign_id: 'foreign-id-web-1', - }); + }))!; - event.setSender(webSubscriber!); + event.setSender(webSubscriber); let hasBotSpoken = false; const clearMock = jest @@ -210,15 +210,15 @@ describe('BlockService', () => { isFallback: boolean, ) => { expect(actualConversation).toEqualPayload({ - sender: webSubscriber!.id, + sender: webSubscriber.id, active: true, next: [], context: { user: { - first_name: webSubscriber!.first_name, - last_name: webSubscriber!.last_name, + first_name: webSubscriber.first_name, + last_name: webSubscriber.last_name, language: 'en', - id: webSubscriber!.id, + id: webSubscriber.id, }, user_location: { lat: 0, @@ -260,10 +260,10 @@ describe('BlockService', () => { ipAddress: '1.1.1.1', agent: 'Chromium', }); - const webSubscriber = await subscriberService.findOne({ + const webSubscriber = (await subscriberService.findOne({ foreign_id: 'foreign-id-web-1', - }); - event.setSender(webSubscriber!); + }))!; + event.setSender(webSubscriber); const clearMock = jest .spyOn(botService, 'handleIncomingMessage') @@ -278,10 +278,10 @@ describe('BlockService', () => { active: true, context: { user: { - first_name: webSubscriber!.first_name, - last_name: webSubscriber!.last_name, + first_name: webSubscriber.first_name, + last_name: webSubscriber.last_name, language: 'en', - id: webSubscriber!.id, + id: webSubscriber.id, }, user_location: { lat: 0, lon: 0 }, vars: {}, @@ -314,10 +314,10 @@ describe('BlockService', () => { ipAddress: '1.1.1.1', agent: 'Chromium', }); - const webSubscriber = await subscriberService.findOne({ + const webSubscriber = (await subscriberService.findOne({ foreign_id: 'foreign-id-web-2', - }); - event.setSender(webSubscriber!); + }))!; + event.setSender(webSubscriber); const captured = await botService.processConversationMessage(event); expect(captured).toBe(false); diff --git a/api/src/extensions/channels/web/base-web-channel.ts b/api/src/extensions/channels/web/base-web-channel.ts index 28c49bff..99d2992f 100644 --- a/api/src/extensions/channels/web/base-web-channel.ts +++ b/api/src/extensions/channels/web/base-web-channel.ts @@ -236,7 +236,7 @@ export default abstract class BaseWebChannelHandler< ...message, author: 'chatbot', read: true, // Temporary fix as read is false in the bd - mid: anyMessage.mid || 'DEFAULT_MID', + mid: anyMessage.mid || this.generateId(), handover: !!anyMessage.handover, createdAt: anyMessage.createdAt, }); @@ -621,7 +621,12 @@ export default abstract class BaseWebChannelHandler< size: Buffer.byteLength(data.file), type: data.type, }); - return attachment; + + if (attachment) { + return attachment; + } else { + throw new Error('Unable to retrieve stored attachment'); + } } catch (err) { this.logger.error( 'Web Channel Handler : Unable to store uploaded file', @@ -639,7 +644,7 @@ export default abstract class BaseWebChannelHandler< async handleWebUpload( req: Request, res: Response, - ): Promise { + ): Promise { try { const upload = multer({ limits: { @@ -665,7 +670,9 @@ export default abstract class BaseWebChannelHandler< reject(new Error('Unable to upload file!')); } - resolve(req.file); + if (req.file) { + resolve(req.file); + } }); }, ); @@ -678,12 +685,18 @@ export default abstract class BaseWebChannelHandler< return null; } - const attachment = await this.attachmentService.store(file, { - name: file.originalname, - size: file.size, - type: file.mimetype, - }); - return attachment; + if (file) { + const attachment = await this.attachmentService.store(file, { + name: file.originalname, + size: file.size, + type: file.mimetype, + }); + if (attachment) { + return attachment; + } + + throw new Error('Unable to store uploaded file'); + } } catch (err) { this.logger.error( 'Web Channel Handler : Unable to store uploaded file', @@ -703,7 +716,7 @@ export default abstract class BaseWebChannelHandler< async handleUpload( req: Request | SocketRequest, res: Response | SocketResponse, - ): Promise { + ): Promise { // Check if any file is provided if (!req.session.web) { this.logger.debug('Web Channel Handler : No session provided'); diff --git a/api/src/extensions/channels/web/wrapper.ts b/api/src/extensions/channels/web/wrapper.ts index b26cf7ac..094ee5ae 100644 --- a/api/src/extensions/channels/web/wrapper.ts +++ b/api/src/extensions/channels/web/wrapper.ts @@ -27,26 +27,31 @@ type WebEventAdapter = eventType: StdEventType.unknown; messageType: never; raw: Web.Event; + attachment: never; } | { eventType: StdEventType.read; messageType: never; raw: Web.StatusReadEvent; + attachment: never; } | { eventType: StdEventType.delivery; messageType: never; raw: Web.StatusDeliveryEvent; + attachment: never; } | { eventType: StdEventType.typing; messageType: never; raw: Web.StatusTypingEvent; + attachment: never; } | { eventType: StdEventType.message; messageType: IncomingMessageType.message; raw: Web.IncomingMessage; + attachment: never; } | { eventType: StdEventType.message; @@ -54,11 +59,13 @@ type WebEventAdapter = | IncomingMessageType.postback | IncomingMessageType.quick_reply; raw: Web.IncomingMessage; + attachment: never; } | { eventType: StdEventType.message; messageType: IncomingMessageType.location; raw: Web.IncomingMessage; + attachment: never; } | { eventType: StdEventType.message; @@ -68,11 +75,9 @@ type WebEventAdapter = }; // eslint-disable-next-line prettier/prettier -export default class WebEventWrapper extends EventWrapper< - WebEventAdapter, - Web.Event, - N -> { +export default class WebEventWrapper< + N extends ChannelName, +> extends EventWrapper { /** * Constructor : channel's event wrapper * diff --git a/api/src/utils/test/mocks/subscriber.ts b/api/src/utils/test/mocks/subscriber.ts index 8ec8ee83..490a6bd5 100644 --- a/api/src/utils/test/mocks/subscriber.ts +++ b/api/src/utils/test/mocks/subscriber.ts @@ -28,6 +28,8 @@ export const subscriberInstance: Subscriber = { name: 'web-channel', }, labels: [], + avatar: null, + context: {}, ...modelInstance, }; diff --git a/api/src/utils/test/sort.ts b/api/src/utils/test/sort.ts index 6cd91662..28215170 100644 --- a/api/src/utils/test/sort.ts +++ b/api/src/utils/test/sort.ts @@ -13,16 +13,20 @@ type TSortProps = { order?: 'desc' | 'asc'; }; -type TCreateAt = { createdAt?: string | Date }; +type TCreatedAt = { createdAt?: string | Date }; -const sort = ({ +const sort = ({ row1, row2, field = 'createdAt', order = 'desc', }: TSortProps) => (order === 'asc' && row1[field] > row2[field] ? 1 : -1); -export const sortRowsBy = ( +export const sortRowsBy = < + R extends TCreatedAt, + S, + T extends TCreatedAt = R & S, +>( row1: T, row2: T, field?: keyof T, diff --git a/api/types/event-emitter.d.ts b/api/types/event-emitter.d.ts index fadb4d6e..a39d15cf 100644 --- a/api/types/event-emitter.d.ts +++ b/api/types/event-emitter.d.ts @@ -93,7 +93,7 @@ declare module '@nestjs/event-emitter' { object, { block: BlockFull; - passation: Subscriber | null; + passation: Subscriber; 'fallback-local': BlockFull; 'fallback-global': EventWrapper; intervention: Subscriber; From 227630d6c6637c8ac93b84dd1a52127a478042da Mon Sep 17 00:00:00 2001 From: yassinedorbozgithub Date: Wed, 15 Jan 2025 08:25:14 +0100 Subject: [PATCH 06/12] fix: update conversation fixtures --- api/src/utils/test/fixtures/conversation.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/api/src/utils/test/fixtures/conversation.ts b/api/src/utils/test/fixtures/conversation.ts index fca24ce1..a58c66c3 100644 --- a/api/src/utils/test/fixtures/conversation.ts +++ b/api/src/utils/test/fixtures/conversation.ts @@ -58,6 +58,9 @@ const conversations: ConversationCreateDto[] = [ labels: [], assignedTo: null, channel: { name: 'messenger-channel' }, + avatar: null, + context: {}, + assignedAt: new Date(), }, skip: {}, attempt: 0, @@ -104,6 +107,9 @@ const conversations: ConversationCreateDto[] = [ labels: [], assignedTo: null, channel: { name: 'web-channel' }, + avatar: null, + context: {}, + assignedAt: new Date(), }, skip: {}, attempt: 0, From 7f6b4819885955fff60e351217664d2ecf673fb2 Mon Sep 17 00:00:00 2001 From: yassinedorbozgithub Date: Wed, 15 Jan 2025 08:28:22 +0100 Subject: [PATCH 07/12] fix: update chat.service onSubscriberCreate, onSubscriberUpdate methods --- api/src/chat/services/chat.service.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/api/src/chat/services/chat.service.ts b/api/src/chat/services/chat.service.ts index ddf71e4d..5fa9fcc2 100644 --- a/api/src/chat/services/chat.service.ts +++ b/api/src/chat/services/chat.service.ts @@ -296,7 +296,9 @@ export class ChatService { @OnEvent('hook:subscriber:postCreate') async onSubscriberCreate({ _id }: SubscriberDocument) { const subscriber = await this.subscriberService.findOne(_id); - if (subscriber) this.websocketGateway.broadcastSubscriberNew(subscriber); + if (subscriber) { + this.websocketGateway.broadcastSubscriberNew(subscriber); + } } /** @@ -307,6 +309,8 @@ export class ChatService { @OnEvent('hook:subscriber:postUpdate') async onSubscriberUpdate({ _id }: SubscriberDocument) { const subscriber = await this.subscriberService.findOne(_id); - if (subscriber) this.websocketGateway.broadcastSubscriberUpdate(subscriber); + if (subscriber) { + this.websocketGateway.broadcastSubscriberUpdate(subscriber); + } } } From b574295d8c4b128eb4a17cb3dce80e8d1fa24388 Mon Sep 17 00:00:00 2001 From: yassinedorbozgithub Date: Wed, 15 Jan 2025 08:30:26 +0100 Subject: [PATCH 08/12] fix: update context-var.service getContextVarsByBlock method --- api/src/chat/services/context-var.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/chat/services/context-var.service.ts b/api/src/chat/services/context-var.service.ts index 420833dc..d93f96d5 100644 --- a/api/src/chat/services/context-var.service.ts +++ b/api/src/chat/services/context-var.service.ts @@ -36,7 +36,7 @@ export class ContextVarService extends BaseService< block: Block | BlockFull, ): Promise> { const vars = await this.find({ - name: { $in: block.capture_vars?.map(({ context_var }) => context_var) }, + name: { $in: block.capture_vars.map(({ context_var }) => context_var) }, }); return vars.reduce((acc, cv) => { acc[cv.name] = cv; From 2a48974336beb4cb49d9a6ffc2e07a20195fb395 Mon Sep 17 00:00:00 2001 From: yassinedorbozgithub Date: Wed, 15 Jan 2025 08:34:59 +0100 Subject: [PATCH 09/12] fix: update conversation fixtures + service --- api/src/chat/services/conversation.service.ts | 2 -- api/src/utils/test/fixtures/conversation.ts | 6 ++++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/src/chat/services/conversation.service.ts b/api/src/chat/services/conversation.service.ts index 593ca1e6..118f367f 100644 --- a/api/src/chat/services/conversation.service.ts +++ b/api/src/chat/services/conversation.service.ts @@ -71,8 +71,6 @@ export class ConversationService extends BaseService< const msgType = event.getMessageType(); const profile = event.getSender(); - if (!convo.context) throw new Error('Missing conversation context'); - // Capture channel specific context data convo.context.channel = event.getHandler().getName(); convo.context.text = event.getText(); diff --git a/api/src/utils/test/fixtures/conversation.ts b/api/src/utils/test/fixtures/conversation.ts index a58c66c3..ba469949 100644 --- a/api/src/utils/test/fixtures/conversation.ts +++ b/api/src/utils/test/fixtures/conversation.ts @@ -142,8 +142,10 @@ export const installConversationTypeFixtures = async () => { conversationFixtures.map((conversationFixture) => ({ ...conversationFixture, sender: subscribers[parseInt(conversationFixture.sender)].id, - current: blocks[parseInt(conversationFixture.current)].id, - next: conversationFixture.next.map((n) => blocks[parseInt(n)].id), + current: conversationFixture?.current + ? blocks[parseInt(conversationFixture.current)]?.id + : undefined, + next: conversationFixture.next?.map((n) => blocks[parseInt(n)].id), })), ); }; From a11ee925a61dae5461b6b9b4674db63da0859660 Mon Sep 17 00:00:00 2001 From: yassinedorbozgithub Date: Wed, 15 Jan 2025 09:15:45 +0100 Subject: [PATCH 10/12] fix: update bot service processMessage method --- api/src/chat/services/block.service.ts | 13 +++++++++---- api/src/chat/services/bot.service.ts | 9 +++------ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/api/src/chat/services/block.service.ts b/api/src/chat/services/block.service.ts index 736c5595..2b6f6346 100644 --- a/api/src/chat/services/block.service.ts +++ b/api/src/chat/services/block.service.ts @@ -440,7 +440,7 @@ export class BlockService extends BaseService< subscriberContext: SubscriberContext, fallback = false, conversationId?: string, - ) { + ): Promise { const settings = await this.settingService.getSettings(); const blockMessage: BlockMessage = fallback && block.options?.fallback @@ -592,13 +592,18 @@ export class BlockService extends BaseService< ); // Process custom plugin block try { - return await plugin?.process(block, context, conversationId); + const envelope = await plugin?.process(block, context, conversationId); + + if (!envelope) { + throw new Error('Unable to find envelope'); + } + + return envelope; } catch (e) { this.logger.error('Plugin was unable to load/process ', e); throw new Error(`Unknown plugin - ${JSON.stringify(blockMessage)}`); } - } else { - throw new Error('Invalid message format.'); } + throw new Error('Invalid message format.'); } } diff --git a/api/src/chat/services/bot.service.ts b/api/src/chat/services/bot.service.ts index 7b1a71a4..4152f34e 100644 --- a/api/src/chat/services/bot.service.ts +++ b/api/src/chat/services/bot.service.ts @@ -21,10 +21,7 @@ import { getDefaultConversationContext, } from '../schemas/conversation.schema'; import { Context } from '../schemas/types/context'; -import { - IncomingMessageType, - StdOutgoingEnvelope, -} from '../schemas/types/message'; +import { IncomingMessageType } from '../schemas/types/message'; import { SubscriberContext } from '../schemas/types/subscriberContext'; import { BlockService } from './block.service'; @@ -71,13 +68,13 @@ export class BotService { ); // Process message : Replace tokens with context data and then send the message const recipient = event.getSender(); - const envelope = (await this.blockService.processMessage( + const envelope = await this.blockService.processMessage( block, context, recipient?.context as SubscriberContext, fallback, conservationId, - )) as StdOutgoingEnvelope; + ); // Send message through the right channel const response = await event From 904397d3fec0ef8521a76e81b4bc00a62a9dadc2 Mon Sep 17 00:00:00 2001 From: yassinedorbozgithub Date: Wed, 15 Jan 2025 09:17:26 +0100 Subject: [PATCH 11/12] fix: update conversation context condition --- api/src/chat/services/bot.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/chat/services/bot.service.ts b/api/src/chat/services/bot.service.ts index 4152f34e..8a9ab5ed 100644 --- a/api/src/chat/services/bot.service.ts +++ b/api/src/chat/services/bot.service.ts @@ -255,7 +255,7 @@ export class BotService { convo.context.attempt++; fallback = true; } else { - if (convo.context) convo.context.attempt = 0; + convo.context.attempt = 0; fallbackBlock = undefined; } From 91e61b2e5ae350f4660613c4c8871eb806254494 Mon Sep 17 00:00:00 2001 From: yassinedorbozgithub Date: Wed, 15 Jan 2025 10:29:06 +0100 Subject: [PATCH 12/12] fix: upadte chat module unit tests --- .../chat/controllers/block.controller.spec.ts | 75 +++++++++---------- .../context-var.controller.spec.ts | 48 ++++++------ .../controllers/message.controller.spec.ts | 37 ++++----- .../controllers/subscriber.controller.spec.ts | 30 ++++---- .../repositories/block.repository.spec.ts | 24 +++--- .../repositories/message.repository.spec.ts | 15 ++-- .../subscriber.repository.spec.ts | 15 ++-- api/src/chat/services/block.service.spec.ts | 60 ++++++++------- api/src/chat/services/message.service.spec.ts | 24 +++--- .../chat/services/subscriber.service.spec.ts | 16 ++-- 10 files changed, 171 insertions(+), 173 deletions(-) diff --git a/api/src/chat/controllers/block.controller.spec.ts b/api/src/chat/controllers/block.controller.spec.ts index a68dc7d9..767e69b4 100644 --- a/api/src/chat/controllers/block.controller.spec.ts +++ b/api/src/chat/controllers/block.controller.spec.ts @@ -61,11 +61,11 @@ describe('BlockController', () => { let blockController: BlockController; let blockService: BlockService; let categoryService: CategoryService; - let category: Category | null; - let block: Block | null; - let blockToDelete: Block | null; - let hasNextBlocks: Block | null; - let hasPreviousBlocks: Block | null; + let category: Category; + let block: Block; + let blockToDelete: Block; + let hasNextBlocks: Block; + let hasPreviousBlocks: Block; const FIELDS_TO_POPULATE = [ 'trigger_labels', 'assign_labels', @@ -142,18 +142,19 @@ describe('BlockController', () => { blockController = module.get(BlockController); blockService = module.get(BlockService); categoryService = module.get(CategoryService); - category = await categoryService.findOne({ label: 'default' }); - block = await blockService.findOne({ name: 'first' }); - blockToDelete = await blockService.findOne({ name: 'buttons' }); - hasNextBlocks = await blockService.findOne({ + category = (await categoryService.findOne({ label: 'default' }))!; + block = (await blockService.findOne({ name: 'first' }))!; + blockToDelete = (await blockService.findOne({ name: 'buttons' }))!; + hasNextBlocks = (await blockService.findOne({ name: 'hasNextBlocks', - }); - hasPreviousBlocks = await blockService.findOne({ + }))!; + hasPreviousBlocks = (await blockService.findOne({ name: 'hasPreviousBlocks', - }); + }))!; }); afterEach(jest.clearAllMocks); + afterAll(closeInMongodConnection); describe('find', () => { @@ -162,9 +163,9 @@ describe('BlockController', () => { const result = await blockController.find([], {}); const blocksWithCategory = blockFixtures.map((blockFixture) => ({ ...blockFixture, - category: category!.id, + category: category.id, nextBlocks: - blockFixture.name === 'hasNextBlocks' ? [hasPreviousBlocks!.id] : [], + blockFixture.name === 'hasNextBlocks' ? [hasPreviousBlocks.id] : [], })); expect(blockService.find).toHaveBeenCalledWith({}, undefined); @@ -196,13 +197,13 @@ describe('BlockController', () => { describe('findOne', () => { it('should find one block by id', async () => { jest.spyOn(blockService, 'findOne'); - const result = await blockController.findOne(hasNextBlocks!.id, []); - expect(blockService.findOne).toHaveBeenCalledWith(hasNextBlocks!.id); + const result = await blockController.findOne(hasNextBlocks.id, []); + expect(blockService.findOne).toHaveBeenCalledWith(hasNextBlocks.id); expect(result).toEqualPayload( { - ...blockFixtures.find(({ name }) => name === hasNextBlocks!.name), - category: category!.id, - nextBlocks: [hasPreviousBlocks!.id], + ...blockFixtures.find(({ name }) => name === hasNextBlocks.name), + category: category.id, + nextBlocks: [hasPreviousBlocks.id], }, [...IGNORED_TEST_FIELDS, 'attachedToBlock'], ); @@ -211,11 +212,11 @@ describe('BlockController', () => { it('should find one block by id, and populate its category and previousBlocks', async () => { jest.spyOn(blockService, 'findOneAndPopulate'); const result = await blockController.findOne( - hasPreviousBlocks!.id, + hasPreviousBlocks.id, FIELDS_TO_POPULATE, ); expect(blockService.findOneAndPopulate).toHaveBeenCalledWith( - hasPreviousBlocks!.id, + hasPreviousBlocks.id, ); expect(result).toEqualPayload({ ...blockFixtures.find(({ name }) => name === 'hasPreviousBlocks'), @@ -227,12 +228,12 @@ describe('BlockController', () => { it('should find one block by id, and populate its category and an empty previousBlocks', async () => { jest.spyOn(blockService, 'findOneAndPopulate'); - block = await blockService.findOne({ name: 'attachment' }); + block = (await blockService.findOne({ name: 'attachment' }))!; const result = await blockController.findOne( - block!.id, + block.id, FIELDS_TO_POPULATE, ); - expect(blockService.findOneAndPopulate).toHaveBeenCalledWith(block!.id); + expect(blockService.findOneAndPopulate).toHaveBeenCalledWith(block.id); expect(result).toEqualPayload({ ...blockFixtures.find(({ name }) => name === 'attachment'), category, @@ -247,12 +248,12 @@ describe('BlockController', () => { jest.spyOn(blockService, 'create'); const mockedBlockCreateDto: BlockCreateDto = { name: 'block with nextBlocks', - nextBlocks: [hasNextBlocks!.id], + nextBlocks: [hasNextBlocks.id], patterns: ['Hi'], trigger_labels: [], assign_labels: [], trigger_channels: [], - category: category!.id, + category: category.id, options: { typing: 0, fallback: { @@ -284,17 +285,15 @@ describe('BlockController', () => { describe('deleteOne', () => { it('should delete block', async () => { jest.spyOn(blockService, 'deleteOne'); - const result = await blockController.deleteOne(blockToDelete!.id); + const result = await blockController.deleteOne(blockToDelete.id); - expect(blockService.deleteOne).toHaveBeenCalledWith(blockToDelete!.id); + expect(blockService.deleteOne).toHaveBeenCalledWith(blockToDelete.id); expect(result).toEqual({ acknowledged: true, deletedCount: 1 }); }); it('should throw NotFoundException when attempting to delete a block by id', async () => { - await expect( - blockController.deleteOne(blockToDelete!.id), - ).rejects.toThrow( - new NotFoundException(`Block with ID ${blockToDelete!.id} not found`), + await expect(blockController.deleteOne(blockToDelete.id)).rejects.toThrow( + new NotFoundException(`Block with ID ${blockToDelete.id} not found`), ); }); }); @@ -305,16 +304,16 @@ describe('BlockController', () => { const updateBlock: BlockUpdateDto = { name: 'modified block name', }; - const result = await blockController.updateOne(block!.id, updateBlock); + const result = await blockController.updateOne(block.id, updateBlock); expect(blockService.updateOne).toHaveBeenCalledWith( - block!.id, + block.id, updateBlock, ); expect(result).toEqualPayload( { - ...blockFixtures.find(({ name }) => name === block!.name), - category: category!.id, + ...blockFixtures.find(({ name }) => name === block.name), + category: category.id, ...updateBlock, }, [...IGNORED_TEST_FIELDS, 'attachedToBlock'], @@ -327,9 +326,9 @@ describe('BlockController', () => { }; await expect( - blockController.updateOne(blockToDelete!.id, updateBlock), + blockController.updateOne(blockToDelete.id, updateBlock), ).rejects.toThrow( - new NotFoundException(`Block with ID ${blockToDelete!.id} not found`), + new NotFoundException(`Block with ID ${blockToDelete.id} not found`), ); }); }); diff --git a/api/src/chat/controllers/context-var.controller.spec.ts b/api/src/chat/controllers/context-var.controller.spec.ts index 11a88eb8..d14a7dd6 100644 --- a/api/src/chat/controllers/context-var.controller.spec.ts +++ b/api/src/chat/controllers/context-var.controller.spec.ts @@ -36,8 +36,8 @@ import { ContextVarController } from './context-var.controller'; describe('ContextVarController', () => { let contextVarController: ContextVarController; let contextVarService: ContextVarService; - let contextVar: ContextVar | null; - let contextVarToDelete: ContextVar | null; + let contextVar: ContextVar; + let contextVarToDelete: ContextVar; beforeAll(async () => { const module = await Test.createTestingModule({ @@ -56,15 +56,16 @@ describe('ContextVarController', () => { contextVarController = module.get(ContextVarController); contextVarService = module.get(ContextVarService); - contextVar = await contextVarService.findOne({ + contextVar = (await contextVarService.findOne({ label: 'test context var 1', - }); - contextVarToDelete = await contextVarService.findOne({ + }))!; + contextVarToDelete = (await contextVarService.findOne({ label: 'test context var 2', - }); + }))!; }); afterEach(jest.clearAllMocks); + afterAll(closeInMongodConnection); describe('count', () => { @@ -91,11 +92,11 @@ describe('ContextVarController', () => { describe('findOne', () => { it('should return the existing contextVar', async () => { jest.spyOn(contextVarService, 'findOne'); - const result = await contextVarController.findOne(contextVar!.id); + const result = await contextVarController.findOne(contextVar.id); - expect(contextVarService.findOne).toHaveBeenCalledWith(contextVar!.id); + expect(contextVarService.findOne).toHaveBeenCalledWith(contextVar.id); expect(result).toEqualPayload( - contextVarFixtures.find(({ label }) => label === contextVar!.label)!, + contextVarFixtures.find(({ label }) => label === contextVar.label)!, ); }); }); @@ -121,11 +122,11 @@ describe('ContextVarController', () => { it('should delete a contextVar by id', async () => { jest.spyOn(contextVarService, 'deleteOne'); const result = await contextVarController.deleteOne( - contextVarToDelete!.id, + contextVarToDelete.id, ); expect(contextVarService.deleteOne).toHaveBeenCalledWith( - contextVarToDelete!.id, + contextVarToDelete.id, ); expect(result).toEqual({ acknowledged: true, @@ -135,10 +136,10 @@ describe('ContextVarController', () => { it('should throw a NotFoundException when attempting to delete a contextVar by id', async () => { await expect( - contextVarController.deleteOne(contextVarToDelete!.id), + contextVarController.deleteOne(contextVarToDelete.id), ).rejects.toThrow( new NotFoundException( - `Context var with ID ${contextVarToDelete!.id} not found.`, + `Context var with ID ${contextVarToDelete.id} not found.`, ), ); }); @@ -152,12 +153,12 @@ describe('ContextVarController', () => { .spyOn(contextVarService, 'deleteMany') .mockResolvedValue(deleteResult); const result = await contextVarController.deleteMany([ - contextVarToDelete!.id, - contextVar!.id, + contextVarToDelete.id, + contextVar.id, ]); expect(contextVarService.deleteMany).toHaveBeenCalledWith({ - _id: { $in: [contextVarToDelete!.id, contextVar!.id] }, + _id: { $in: [contextVarToDelete.id, contextVar.id] }, }); expect(result).toEqual(deleteResult); }); @@ -175,10 +176,7 @@ describe('ContextVarController', () => { }); await expect( - contextVarController.deleteMany([ - contextVarToDelete!.id, - contextVar!.id, - ]), + contextVarController.deleteMany([contextVarToDelete.id, contextVar.id]), ).rejects.toThrow( new NotFoundException('Context vars with provided IDs not found'), ); @@ -192,16 +190,16 @@ describe('ContextVarController', () => { it('should return updated contextVar', async () => { jest.spyOn(contextVarService, 'updateOne'); const result = await contextVarController.updateOne( - contextVar!.id, + contextVar.id, contextVarUpdatedDto, ); expect(contextVarService.updateOne).toHaveBeenCalledWith( - contextVar!.id, + contextVar.id, contextVarUpdatedDto, ); expect(result).toEqualPayload({ - ...contextVarFixtures.find(({ label }) => label === contextVar!.label), + ...contextVarFixtures.find(({ label }) => label === contextVar.label), ...contextVarUpdatedDto, }); }); @@ -209,12 +207,12 @@ describe('ContextVarController', () => { it('should throw a NotFoundException when attempting to update an non existing contextVar by id', async () => { await expect( contextVarController.updateOne( - contextVarToDelete!.id, + contextVarToDelete.id, contextVarUpdatedDto, ), ).rejects.toThrow( new NotFoundException( - `ContextVar with ID ${contextVarToDelete!.id} not found`, + `ContextVar with ID ${contextVarToDelete.id} not found`, ), ); }); diff --git a/api/src/chat/controllers/message.controller.spec.ts b/api/src/chat/controllers/message.controller.spec.ts index b7dad706..d2e9c2d3 100644 --- a/api/src/chat/controllers/message.controller.spec.ts +++ b/api/src/chat/controllers/message.controller.spec.ts @@ -53,10 +53,10 @@ describe('MessageController', () => { let messageService: MessageService; let subscriberService: SubscriberService; let userService: UserService; - let sender: Subscriber | null; - let recipient: Subscriber | null; - let user: User | null; - let message: Message | null; + let sender: Subscriber; + let recipient: Subscriber; + let user: User; + let message: Message; let allMessages: Message[]; let allUsers: User[]; let allSubscribers: Subscriber[]; @@ -128,16 +128,17 @@ describe('MessageController', () => { userService = module.get(UserService); subscriberService = module.get(SubscriberService); messageController = module.get(MessageController); - message = await messageService.findOne({ mid: 'mid-1' }); - sender = await subscriberService.findOne(message!.sender!); - recipient = await subscriberService.findOne(message!.recipient!); - user = await userService.findOne(message!.sentBy!); + message = (await messageService.findOne({ mid: 'mid-1' }))!; + sender = (await subscriberService.findOne(message.sender!))!; + recipient = (await subscriberService.findOne(message.recipient!))!; + user = (await userService.findOne(message.sentBy!))!; allSubscribers = await subscriberService.findAll(); allUsers = await userService.findAll(); allMessages = await messageService.findAll(); }); afterEach(jest.clearAllMocks); + afterAll(closeInMongodConnection); describe('count', () => { @@ -153,31 +154,31 @@ describe('MessageController', () => { describe('findOne', () => { it('should find message by id, and populate its corresponding sender and recipient', async () => { jest.spyOn(messageService, 'findOneAndPopulate'); - const result = await messageController.findOne(message!.id, [ + const result = await messageController.findOne(message.id, [ 'sender', 'recipient', ]); expect(messageService.findOneAndPopulate).toHaveBeenCalledWith( - message!.id, + message.id, ); expect(result).toEqualPayload({ - ...messageFixtures.find(({ mid }) => mid === message!.mid), + ...messageFixtures.find(({ mid }) => mid === message.mid), sender, recipient, - sentBy: user!.id, + sentBy: user.id, }); }); it('should find message by id', async () => { jest.spyOn(messageService, 'findOne'); - const result = await messageController.findOne(message!.id, []); + const result = await messageController.findOne(message.id, []); - expect(messageService.findOne).toHaveBeenCalledWith(message!.id); + expect(messageService.findOne).toHaveBeenCalledWith(message.id); expect(result).toEqualPayload({ - ...messageFixtures.find(({ mid }) => mid === message!.mid), - sender: sender!.id, - recipient: recipient!.id, - sentBy: user!.id, + ...messageFixtures.find(({ mid }) => mid === message.mid), + sender: sender.id, + recipient: recipient.id, + sentBy: user.id, }); }); }); diff --git a/api/src/chat/controllers/subscriber.controller.spec.ts b/api/src/chat/controllers/subscriber.controller.spec.ts index af810f15..6e280f99 100644 --- a/api/src/chat/controllers/subscriber.controller.spec.ts +++ b/api/src/chat/controllers/subscriber.controller.spec.ts @@ -48,7 +48,7 @@ describe('SubscriberController', () => { let subscriberService: SubscriberService; let labelService: LabelService; let userService: UserService; - let subscriber: Subscriber | null; + let subscriber: Subscriber; let allLabels: Label[]; let allSubscribers: Subscriber[]; let allUsers: User[]; @@ -90,15 +90,16 @@ describe('SubscriberController', () => { subscriberController = module.get(SubscriberController); - subscriber = await subscriberService.findOne({ + subscriber = (await subscriberService.findOne({ first_name: 'Jhon', - }); + }))!; allLabels = await labelService.findAll(); allSubscribers = await subscriberService.findAll(); allUsers = await userService.findAll(); }); afterEach(jest.clearAllMocks); + afterAll(closeInMongodConnection); describe('count', () => { @@ -114,39 +115,38 @@ describe('SubscriberController', () => { describe('findOne', () => { it('should find one subscriber by id', async () => { jest.spyOn(subscriberService, 'findOne'); - const result = await subscriberService.findOne(subscriber!.id); + const result = await subscriberService.findOne(subscriber.id); const labelIDs = allLabels - .filter((label) => subscriber!.labels.includes(label.id)) + .filter((label) => subscriber.labels.includes(label.id)) .map(({ id }) => id); - expect(subscriberService.findOne).toHaveBeenCalledWith(subscriber!.id); + expect(subscriberService.findOne).toHaveBeenCalledWith(subscriber.id); expect(result).toEqualPayload({ ...subscriberFixtures.find( - ({ first_name }) => first_name === subscriber!.first_name, + ({ first_name }) => first_name === subscriber.first_name, ), labels: labelIDs, - assignedTo: allUsers.find(({ id }) => subscriber!.assignedTo === id) - ?.id, + assignedTo: allUsers.find(({ id }) => subscriber.assignedTo === id)?.id, }); }); it('should find one subscriber by id, and populate its corresponding labels', async () => { jest.spyOn(subscriberService, 'findOneAndPopulate'); - const result = await subscriberController.findOne(subscriber!.id, [ + const result = await subscriberController.findOne(subscriber.id, [ 'labels', ]); expect(subscriberService.findOneAndPopulate).toHaveBeenCalledWith( - subscriber!.id, + subscriber.id, ); expect(result).toEqualPayload({ ...subscriberFixtures.find( - ({ first_name }) => first_name === subscriber!.first_name, + ({ first_name }) => first_name === subscriber.first_name, ), labels: allLabels.filter((label) => - subscriber!.labels.includes(label.id), + subscriber.labels.includes(label.id), ), - assignedTo: allUsers.find(({ id }) => subscriber!.assignedTo === id), + assignedTo: allUsers.find(({ id }) => subscriber.assignedTo === id), }); }); }); @@ -178,7 +178,7 @@ describe('SubscriberController', () => { ({ labels, ...rest }) => ({ ...rest, labels: allLabels.filter((label) => labels.includes(label.id)), - assignedTo: allUsers.find(({ id }) => subscriber!.assignedTo === id), + assignedTo: allUsers.find(({ id }) => subscriber.assignedTo === id), }), ); diff --git a/api/src/chat/repositories/block.repository.spec.ts b/api/src/chat/repositories/block.repository.spec.ts index 0948e427..8556266c 100644 --- a/api/src/chat/repositories/block.repository.spec.ts +++ b/api/src/chat/repositories/block.repository.spec.ts @@ -31,9 +31,9 @@ describe('BlockRepository', () => { let blockRepository: BlockRepository; let categoryRepository: CategoryRepository; let blockModel: Model; - let category: Category | null; - let hasPreviousBlocks: Block | null; - let hasNextBlocks: Block | null; + let category: Category; + let hasPreviousBlocks: Block; + let hasNextBlocks: Block; let validIds: string[]; let validCategory: string; @@ -51,13 +51,13 @@ describe('BlockRepository', () => { validIds = ['64abc1234def567890fedcba', '64abc1234def567890fedcbc']; validCategory = '64def5678abc123490fedcba'; - category = await categoryRepository.findOne({ label: 'default' }); - hasPreviousBlocks = await blockRepository.findOne({ + category = (await categoryRepository.findOne({ label: 'default' }))!; + hasPreviousBlocks = (await blockRepository.findOne({ name: 'hasPreviousBlocks', - }); - hasNextBlocks = await blockRepository.findOne({ + }))!; + hasNextBlocks = (await blockRepository.findOne({ name: 'hasNextBlocks', - }); + }))!; }); afterEach(jest.clearAllMocks); @@ -67,15 +67,13 @@ describe('BlockRepository', () => { it('should find one block by id, and populate its trigger_labels, assign_labels, nextBlocks, attachedBlock, category,previousBlocks', async () => { jest.spyOn(blockModel, 'findById'); - const result = await blockRepository.findOneAndPopulate( - hasNextBlocks!.id, - ); + const result = await blockRepository.findOneAndPopulate(hasNextBlocks.id); expect(blockModel.findById).toHaveBeenCalledWith( - hasNextBlocks!.id, + hasNextBlocks.id, undefined, ); expect(result).toEqualPayload({ - ...blockFixtures.find(({ name }) => name === hasNextBlocks!.name), + ...blockFixtures.find(({ name }) => name === hasNextBlocks.name), category, nextBlocks: [hasPreviousBlocks], previousBlocks: [], diff --git a/api/src/chat/repositories/message.repository.spec.ts b/api/src/chat/repositories/message.repository.spec.ts index 9793f2e3..f817f2a9 100644 --- a/api/src/chat/repositories/message.repository.spec.ts +++ b/api/src/chat/repositories/message.repository.spec.ts @@ -62,23 +62,20 @@ describe('MessageRepository', () => { describe('findOneAndPopulate', () => { it('should find one message by id, and populate its sender and recipient', async () => { jest.spyOn(messageModel, 'findById'); - const message = await messageRepository.findOne({ mid: 'mid-1' }); + const message = (await messageRepository.findOne({ mid: 'mid-1' }))!; const sender = await subscriberRepository.findOne(message!['sender']); const recipient = await subscriberRepository.findOne( message!['recipient'], ); - const user = await userRepository.findOne(message!['sentBy']); - const result = await messageRepository.findOneAndPopulate(message!.id); + const user = (await userRepository.findOne(message!['sentBy']))!; + const result = await messageRepository.findOneAndPopulate(message.id); - expect(messageModel.findById).toHaveBeenCalledWith( - message!.id, - undefined, - ); + expect(messageModel.findById).toHaveBeenCalledWith(message.id, undefined); expect(result).toEqualPayload({ - ...messageFixtures.find(({ mid }) => mid === message!.mid), + ...messageFixtures.find(({ mid }) => mid === message.mid), sender, recipient, - sentBy: user!.id, + sentBy: user.id, }); }); }); diff --git a/api/src/chat/repositories/subscriber.repository.spec.ts b/api/src/chat/repositories/subscriber.repository.spec.ts index 2b54b03d..fd553e5f 100644 --- a/api/src/chat/repositories/subscriber.repository.spec.ts +++ b/api/src/chat/repositories/subscriber.repository.spec.ts @@ -97,30 +97,31 @@ describe('SubscriberRepository', () => { }); afterEach(jest.clearAllMocks); + afterAll(closeInMongodConnection); describe('findOneAndPopulate', () => { it('should find one subscriber by id,and populate its labels', async () => { jest.spyOn(subscriberModel, 'findById'); - const subscriber = await subscriberRepository.findOne({ + const subscriber = (await subscriberRepository.findOne({ first_name: 'Jhon', - }); + }))!; const allLabels = await labelRepository.findAll(); const result = await subscriberRepository.findOneAndPopulate( - subscriber!.id, + subscriber.id, ); const subscriberWithLabels = { ...subscriberFixtures.find( - ({ first_name }) => first_name === subscriber!.first_name, + ({ first_name }) => first_name === subscriber.first_name, ), labels: allLabels.filter((label) => - subscriber!.labels.includes(label.id), + subscriber.labels.includes(label.id), ), - assignedTo: allUsers.find(({ id }) => subscriber!.assignedTo === id), + assignedTo: allUsers.find(({ id }) => subscriber.assignedTo === id), }; expect(subscriberModel.findById).toHaveBeenCalledWith( - subscriber!.id, + subscriber.id, undefined, ); expect(result).toEqualPayload(subscriberWithLabels); diff --git a/api/src/chat/services/block.service.spec.ts b/api/src/chat/services/block.service.spec.ts index f4d21374..cf436248 100644 --- a/api/src/chat/services/block.service.spec.ts +++ b/api/src/chat/services/block.service.spec.ts @@ -74,10 +74,10 @@ import { CategoryService } from './category.service'; describe('BlockService', () => { let blockRepository: BlockRepository; let categoryRepository: CategoryRepository; - let category: Category | null; - let block: Block | null; + let category: Category; + let block: Block; let blockService: BlockService; - let hasPreviousBlocks: Block | null; + let hasPreviousBlocks: Block; let contentService: ContentService; let contentTypeService: ContentTypeService; let settingService: SettingService; @@ -154,11 +154,11 @@ describe('BlockService', () => { contentTypeService = module.get(ContentTypeService); categoryRepository = module.get(CategoryRepository); blockRepository = module.get(BlockRepository); - category = await categoryRepository.findOne({ label: 'default' }); - hasPreviousBlocks = await blockRepository.findOne({ + category = (await categoryRepository.findOne({ label: 'default' }))!; + hasPreviousBlocks = (await blockRepository.findOne({ name: 'hasPreviousBlocks', - }); - block = await blockRepository.findOne({ name: 'hasNextBlocks' }); + }))!; + block = (await blockRepository.findOne({ name: 'hasNextBlocks' }))!; settings = await settingService.getSettings(); }); @@ -168,10 +168,10 @@ describe('BlockService', () => { describe('findOneAndPopulate', () => { it('should find one block by id, and populate its trigger_labels, assign_labels,attachedBlock,category,nextBlocks', async () => { jest.spyOn(blockRepository, 'findOneAndPopulate'); - const result = await blockService.findOneAndPopulate(block!.id); + const result = await blockService.findOneAndPopulate(block.id); expect(blockRepository.findOneAndPopulate).toHaveBeenCalledWith( - block!.id, + block.id, undefined, ); expect(result).toEqualPayload({ @@ -441,8 +441,10 @@ describe('BlockService', () => { describe('processMessage', () => { it('should process list message (with limit = 2 and skip = 0)', async () => { - const contentType = await contentTypeService.findOne({ name: 'Product' }); - blockProductListMock.options!.content!.entity = contentType!.id; + const contentType = (await contentTypeService.findOne({ + name: 'Product', + }))!; + blockProductListMock.options.content!.entity = contentType.id; const result = await blockService.processMessage( blockProductListMock, { @@ -454,27 +456,29 @@ describe('BlockService', () => { 'conv_id', ); const elements = await contentService.findPage( - { status: true, entity: contentType!.id }, + { status: true, entity: contentType.id }, { skip: 0, limit: 2, sort: ['createdAt', 'desc'] }, ); const flattenedElements = elements.map(Content.toElement); - expect(result!.format).toEqualPayload( - blockProductListMock.options!.content!.display, + expect(result.format).toEqualPayload( + blockProductListMock.options.content!.display, ); expect( - (result!.message as StdOutgoingListMessage).elements, + (result.message as StdOutgoingListMessage).elements, ).toEqualPayload(flattenedElements); + expect((result.message as StdOutgoingListMessage).options).toEqualPayload( + blockProductListMock.options.content!, + ); expect( - (result!.message as StdOutgoingListMessage).options, - ).toEqualPayload(blockProductListMock.options!.content!); - expect( - (result!.message as StdOutgoingListMessage).pagination, + (result.message as StdOutgoingListMessage).pagination, ).toEqualPayload({ total: 4, skip: 0, limit: 2 }); }); it('should process list message (with limit = 2 and skip = 2)', async () => { - const contentType = await contentTypeService.findOne({ name: 'Product' }); - blockProductListMock.options!.content!.entity = contentType!.id; + const contentType = (await contentTypeService.findOne({ + name: 'Product', + }))!; + blockProductListMock.options.content!.entity = contentType.id; const result = await blockService.processMessage( blockProductListMock, { @@ -486,20 +490,20 @@ describe('BlockService', () => { 'conv_id', ); const elements = await contentService.findPage( - { status: true, entity: contentType!.id }, + { status: true, entity: contentType.id }, { skip: 2, limit: 2, sort: ['createdAt', 'desc'] }, ); const flattenedElements = elements.map(Content.toElement); - expect(result!.format).toEqual( - blockProductListMock.options!.content?.display, + expect(result.format).toEqual( + blockProductListMock.options.content?.display, ); - expect((result!.message as StdOutgoingListMessage).elements).toEqual( + expect((result.message as StdOutgoingListMessage).elements).toEqual( flattenedElements, ); - expect((result!.message as StdOutgoingListMessage).options).toEqual( - blockProductListMock.options!.content, + expect((result.message as StdOutgoingListMessage).options).toEqual( + blockProductListMock.options.content, ); - expect((result!.message as StdOutgoingListMessage).pagination).toEqual({ + expect((result.message as StdOutgoingListMessage).pagination).toEqual({ total: 4, skip: 2, limit: 2, diff --git a/api/src/chat/services/message.service.spec.ts b/api/src/chat/services/message.service.spec.ts index 07f2b8e9..1e7899b8 100644 --- a/api/src/chat/services/message.service.spec.ts +++ b/api/src/chat/services/message.service.spec.ts @@ -48,11 +48,11 @@ describe('MessageService', () => { let allMessages: Message[]; let allSubscribers: Subscriber[]; let allUsers: User[]; - let message: Message | null; - let sender: Subscriber | null; - let recipient: Subscriber | null; + let message: Message; + let sender: Subscriber; + let recipient: Subscriber; let messagesWithSenderAndRecipient: Message[]; - let user: User | null; + let user: User; beforeAll(async () => { const module = await Test.createTestingModule({ @@ -90,10 +90,10 @@ describe('MessageService', () => { allSubscribers = await subscriberRepository.findAll(); allUsers = await userRepository.findAll(); allMessages = await messageRepository.findAll(); - message = await messageRepository.findOne({ mid: 'mid-1' }); - sender = await subscriberRepository.findOne(message!.sender!); - recipient = await subscriberRepository.findOne(message!.recipient!); - user = await userRepository.findOne(message!.sentBy!); + message = (await messageRepository.findOne({ mid: 'mid-1' }))!; + sender = (await subscriberRepository.findOne(message.sender!))!; + recipient = (await subscriberRepository.findOne(message.recipient!))!; + user = (await userRepository.findOne(message.sentBy!))!; messagesWithSenderAndRecipient = allMessages.map((message) => ({ ...message, sender: allSubscribers.find(({ id }) => id === message.sender)?.id, @@ -108,17 +108,17 @@ describe('MessageService', () => { describe('findOneAndPopulate', () => { it('should find message by id, and populate its corresponding sender and recipient', async () => { jest.spyOn(messageRepository, 'findOneAndPopulate'); - const result = await messageService.findOneAndPopulate(message!.id); + const result = await messageService.findOneAndPopulate(message.id); expect(messageRepository.findOneAndPopulate).toHaveBeenCalledWith( - message!.id, + message.id, undefined, ); expect(result).toEqualPayload({ - ...messageFixtures.find(({ mid }) => mid === message!.mid), + ...messageFixtures.find(({ mid }) => mid === message.mid), sender, recipient, - sentBy: user!.id, + sentBy: user.id, }); }); }); diff --git a/api/src/chat/services/subscriber.service.spec.ts b/api/src/chat/services/subscriber.service.spec.ts index ce93c156..9eec453c 100644 --- a/api/src/chat/services/subscriber.service.spec.ts +++ b/api/src/chat/services/subscriber.service.spec.ts @@ -90,20 +90,20 @@ describe('SubscriberService', () => { describe('findOneAndPopulate', () => { it('should find subscribers, and foreach subscriber populate its corresponding labels', async () => { jest.spyOn(subscriberService, 'findOneAndPopulate'); - const subscriber = await subscriberRepository.findOne({ + const subscriber = (await subscriberRepository.findOne({ first_name: 'Jhon', - }); - const result = await subscriberService.findOneAndPopulate(subscriber!.id); + }))!; + const result = await subscriberService.findOneAndPopulate(subscriber.id); expect(subscriberService.findOneAndPopulate).toHaveBeenCalledWith( - subscriber!.id, + subscriber.id, ); expect(result).toEqualPayload({ ...subscriber, labels: allLabels.filter((label) => - subscriber!.labels.includes(label.id), + subscriber.labels.includes(label.id), ), - assignedTo: allUsers.find(({ id }) => subscriber!.assignedTo === id), + assignedTo: allUsers.find(({ id }) => subscriber.assignedTo === id), }); }); }); @@ -133,13 +133,13 @@ describe('SubscriberService', () => { await subscriberService.findOneByForeignId('foreign-id-dimelo'); const subscriber = allSubscribers.find( ({ foreign_id }) => foreign_id === 'foreign-id-dimelo', - ); + )!; expect(subscriberRepository.findOneByForeignId).toHaveBeenCalled(); expect(result).toEqualPayload({ ...subscriber, labels: allLabels - .filter((label) => subscriber!.labels.includes(label.id)) + .filter((label) => subscriber.labels.includes(label.id)) .map((label) => label.id), }); });