From 6c75e6df4af0f5c733975faa5d4e701bf2713893 Mon Sep 17 00:00:00 2001 From: yassinedorbozgithub Date: Thu, 16 Jan 2025 18:47:25 +0100 Subject: [PATCH 1/2] refactor(api): Refactor updateOne logic --- .../analytics/services/bot-stats.service.ts | 2 +- .../chat/controllers/block.controller.spec.ts | 5 +- api/src/chat/controllers/block.controller.ts | 7 +-- .../chat/controllers/category.controller.ts | 7 +-- .../context-var.controller.spec.ts | 5 +- .../controllers/context-var.controller.ts | 7 +-- .../chat/controllers/label.controller.spec.ts | 7 ++- api/src/chat/controllers/label.controller.ts | 7 +-- .../chat/controllers/subscriber.controller.ts | 7 +-- .../repositories/block.repository.spec.ts | 48 +++++++++++-------- .../repositories/conversation.repository.ts | 2 +- .../repositories/subscriber.repository.ts | 8 ++-- api/src/chat/services/conversation.service.ts | 5 -- .../content-type.controller.spec.ts | 3 +- .../controllers/content-type.controller.ts | 13 +---- .../controllers/content.controller.spec.ts | 3 +- api/src/cms/controllers/content.controller.ts | 9 +--- api/src/cms/controllers/menu.controller.ts | 5 +- .../controllers/language.controller.spec.ts | 5 +- .../i18n/controllers/language.controller.ts | 7 +-- .../translation.controller.spec.ts | 4 +- .../controllers/translation.controller.ts | 10 +--- api/src/migration/migration.service.ts | 2 +- .../nlp/controllers/nlp-entity.controller.ts | 12 +---- .../controllers/nlp-sample.controller.spec.ts | 3 +- .../nlp/controllers/nlp-sample.controller.ts | 5 -- .../controllers/nlp-value.controller.spec.ts | 3 +- .../nlp/controllers/nlp-value.controller.ts | 7 +-- api/src/nlp/services/nlp-value.service.ts | 2 +- .../setting/controllers/setting.controller.ts | 8 +--- api/src/user/controllers/role.controller.ts | 7 +-- api/src/user/controllers/user.controller.ts | 8 +--- api/src/utils/generics/base-repository.ts | 11 ++++- api/src/utils/generics/base-service.ts | 2 +- api/src/utils/test/errors/messages.ts | 10 ++++ 35 files changed, 92 insertions(+), 164 deletions(-) create mode 100644 api/src/utils/test/errors/messages.ts diff --git a/api/src/analytics/services/bot-stats.service.ts b/api/src/analytics/services/bot-stats.service.ts index ad12c982..b2a5894a 100644 --- a/api/src/analytics/services/bot-stats.service.ts +++ b/api/src/analytics/services/bot-stats.service.ts @@ -110,7 +110,7 @@ export class BotStatsService extends BaseService { * @param name - The name or identifier of the statistics entry (e.g., a specific feature or component being tracked). */ @OnEvent('hook:stats:entry') - async handleStatEntry(type: BotStatsType, name: string) { + async handleStatEntry(type: BotStatsType, name: string): Promise { const day = new Date(); day.setMilliseconds(0); day.setSeconds(0); diff --git a/api/src/chat/controllers/block.controller.spec.ts b/api/src/chat/controllers/block.controller.spec.ts index 767e69b4..6680c9fb 100644 --- a/api/src/chat/controllers/block.controller.spec.ts +++ b/api/src/chat/controllers/block.controller.spec.ts @@ -35,6 +35,7 @@ import { PermissionService } from '@/user/services/permission.service'; import { RoleService } from '@/user/services/role.service'; import { UserService } from '@/user/services/user.service'; import { IGNORED_TEST_FIELDS } from '@/utils/test/constants'; +import { getUpdateOneError } from '@/utils/test/errors/messages'; import { blockFixtures, installBlockFixtures, @@ -327,9 +328,7 @@ describe('BlockController', () => { await expect( blockController.updateOne(blockToDelete.id, updateBlock), - ).rejects.toThrow( - new NotFoundException(`Block with ID ${blockToDelete.id} not found`), - ); + ).rejects.toThrow(getUpdateOneError(Block.name, blockToDelete.id)); }); }); }); diff --git a/api/src/chat/controllers/block.controller.ts b/api/src/chat/controllers/block.controller.ts index 949410d3..d6a87d67 100644 --- a/api/src/chat/controllers/block.controller.ts +++ b/api/src/chat/controllers/block.controller.ts @@ -296,12 +296,7 @@ export class BlockController extends BaseController< @Param('id') id: string, @Body() blockUpdate: BlockUpdateDto, ): Promise { - const result = await this.blockService.updateOne(id, blockUpdate); - if (!result) { - this.logger.warn(`Unable to update Block by id ${id}`); - throw new NotFoundException(`Block with ID ${id} not found`); - } - return result; + return await this.blockService.updateOne(id, blockUpdate); } /** diff --git a/api/src/chat/controllers/category.controller.ts b/api/src/chat/controllers/category.controller.ts index 16a54473..c071b108 100644 --- a/api/src/chat/controllers/category.controller.ts +++ b/api/src/chat/controllers/category.controller.ts @@ -114,12 +114,7 @@ export class CategoryController extends BaseController { @Param('id') id: string, @Body() categoryUpdate: CategoryUpdateDto, ): Promise { - const result = await this.categoryService.updateOne(id, categoryUpdate); - if (!result) { - this.logger.warn(`Unable to update Category by id ${id}`); - throw new NotFoundException(`Category with ID ${id} not found`); - } - return result; + return await this.categoryService.updateOne(id, categoryUpdate); } /** diff --git a/api/src/chat/controllers/context-var.controller.spec.ts b/api/src/chat/controllers/context-var.controller.spec.ts index d14a7dd6..0f07f985 100644 --- a/api/src/chat/controllers/context-var.controller.spec.ts +++ b/api/src/chat/controllers/context-var.controller.spec.ts @@ -12,6 +12,7 @@ import { MongooseModule } from '@nestjs/mongoose'; import { Test } from '@nestjs/testing'; import { LoggerService } from '@/logger/logger.service'; +import { getUpdateOneError } from '@/utils/test/errors/messages'; import { contextVarFixtures, installContextVarFixtures, @@ -211,9 +212,7 @@ describe('ContextVarController', () => { contextVarUpdatedDto, ), ).rejects.toThrow( - new NotFoundException( - `ContextVar with ID ${contextVarToDelete.id} not found`, - ), + getUpdateOneError(ContextVar.name, contextVarToDelete.id), ); }); }); diff --git a/api/src/chat/controllers/context-var.controller.ts b/api/src/chat/controllers/context-var.controller.ts index c9234d92..d07fe8c2 100644 --- a/api/src/chat/controllers/context-var.controller.ts +++ b/api/src/chat/controllers/context-var.controller.ts @@ -120,12 +120,7 @@ export class ContextVarController extends BaseController { @Param('id') id: string, @Body() contextVarUpdate: ContextVarUpdateDto, ): Promise { - const result = await this.contextVarService.updateOne(id, contextVarUpdate); - if (!result) { - this.logger.warn(`Unable to update ContextVar by id ${id}`); - throw new NotFoundException(`ContextVar with ID ${id} not found`); - } - return result; + return await this.contextVarService.updateOne(id, contextVarUpdate); } /** diff --git a/api/src/chat/controllers/label.controller.spec.ts b/api/src/chat/controllers/label.controller.spec.ts index b81c34f4..0c5fae41 100644 --- a/api/src/chat/controllers/label.controller.spec.ts +++ b/api/src/chat/controllers/label.controller.spec.ts @@ -23,6 +23,7 @@ import { UserModel } from '@/user/schemas/user.schema'; import { RoleService } from '@/user/services/role.service'; import { UserService } from '@/user/services/user.service'; import { IGNORED_TEST_FIELDS } from '@/utils/test/constants'; +import { getUpdateOneError } from '@/utils/test/errors/messages'; import { labelFixtures } from '@/utils/test/fixtures/label'; import { installSubscriberFixtures } from '@/utils/test/fixtures/subscriber'; import { getPageQuery } from '@/utils/test/pagination'; @@ -223,12 +224,10 @@ describe('LabelController', () => { ); }); - it('should throw a NotFoundException when attempting to update a non existing label by id', async () => { + it('should throw a NotFoundException when attempting to update a non existing label by id', async () => { await expect( labelController.updateOne(labelToDelete.id, labelUpdateDto), - ).rejects.toThrow( - new NotFoundException(`Label with ID ${labelToDelete.id} not found`), - ); + ).rejects.toThrow(getUpdateOneError(Label.name, labelToDelete.id)); }); }); }); diff --git a/api/src/chat/controllers/label.controller.ts b/api/src/chat/controllers/label.controller.ts index 65b6f13e..c3e61106 100644 --- a/api/src/chat/controllers/label.controller.ts +++ b/api/src/chat/controllers/label.controller.ts @@ -111,12 +111,7 @@ export class LabelController extends BaseController< @Param('id') id: string, @Body() labelUpdate: LabelUpdateDto, ) { - const result = await this.labelService.updateOne(id, labelUpdate); - if (!result) { - this.logger.warn(`Unable to update Label by id ${id}`); - throw new NotFoundException(`Label with ID ${id} not found`); - } - return result; + return await this.labelService.updateOne(id, labelUpdate); } @CsrfCheck(true) diff --git a/api/src/chat/controllers/subscriber.controller.ts b/api/src/chat/controllers/subscriber.controller.ts index d0688df0..a8d91132 100644 --- a/api/src/chat/controllers/subscriber.controller.ts +++ b/api/src/chat/controllers/subscriber.controller.ts @@ -178,11 +178,6 @@ export class SubscriberController extends BaseController< @Param('id') id: string, @Body() subscriberUpdate: SubscriberUpdateDto, ) { - const result = await this.subscriberService.updateOne(id, subscriberUpdate); - if (!result) { - this.logger.warn(`Unable to update Subscriber by id ${id}`); - throw new NotFoundException(`Subscriber with ID ${id} not found`); - } - return result; + return await this.subscriberService.updateOne(id, subscriberUpdate); } } diff --git a/api/src/chat/repositories/block.repository.spec.ts b/api/src/chat/repositories/block.repository.spec.ts index 8556266c..7dc3c5cb 100644 --- a/api/src/chat/repositories/block.repository.spec.ts +++ b/api/src/chat/repositories/block.repository.spec.ts @@ -36,6 +36,7 @@ describe('BlockRepository', () => { let hasNextBlocks: Block; let validIds: string[]; let validCategory: string; + let blockValidIds: string[]; beforeAll(async () => { const module = await Test.createTestingModule({ @@ -58,6 +59,7 @@ describe('BlockRepository', () => { hasNextBlocks = (await blockRepository.findOne({ name: 'hasNextBlocks', }))!; + blockValidIds = (await blockRepository.findAll()).map(({ id }) => id); }); afterEach(jest.clearAllMocks); @@ -167,24 +169,25 @@ describe('BlockRepository', () => { }); describe('prepareBlocksInCategoryUpdateScope', () => { + /******/ it('should update blocks within the scope based on category and ids', async () => { jest.spyOn(blockRepository, 'findOne').mockResolvedValue({ - id: validIds[0], + id: blockValidIds[0], category: 'oldCategory', - nextBlocks: [validIds[1]], - attachedBlock: validIds[1], + nextBlocks: [blockValidIds[1]], + attachedBlock: blockValidIds[1], } as Block); const mockUpdateOne = jest.spyOn(blockRepository, 'updateOne'); await blockRepository.prepareBlocksInCategoryUpdateScope( validCategory, - validIds, + blockValidIds, ); - expect(mockUpdateOne).toHaveBeenCalledWith(validIds[0], { - nextBlocks: [validIds[1]], - attachedBlock: validIds[1], + expect(mockUpdateOne).toHaveBeenCalledWith(blockValidIds[0], { + nextBlocks: [blockValidIds[1]], + attachedBlock: blockValidIds[1], }); }); @@ -208,12 +211,13 @@ describe('BlockRepository', () => { }); describe('prepareBlocksOutOfCategoryUpdateScope', () => { + /******/ it('should update blocks outside the scope by removing references from attachedBlock', async () => { const otherBlocks = [ { - id: '64abc1234def567890fedcab', - attachedBlock: validIds[0], - nextBlocks: [validIds[0]], + id: blockValidIds[1], + attachedBlock: blockValidIds[0], + nextBlocks: [blockValidIds[0]], }, ] as Block[]; @@ -221,40 +225,42 @@ describe('BlockRepository', () => { await blockRepository.prepareBlocksOutOfCategoryUpdateScope( otherBlocks, - validIds, + blockValidIds, ); - expect(mockUpdateOne).toHaveBeenCalledWith('64abc1234def567890fedcab', { + expect(mockUpdateOne).toHaveBeenCalledWith(blockValidIds[1], { attachedBlock: null, }); }); + /******/ it('should update blocks outside the scope by removing references from nextBlocks', async () => { const otherBlocks = [ { - id: '64abc1234def567890fedcab', + id: blockValidIds[1], attachedBlock: null, - nextBlocks: [validIds[0], validIds[1]], + nextBlocks: [blockValidIds[0], blockValidIds[1]], }, ] as unknown as Block[]; const mockUpdateOne = jest.spyOn(blockRepository, 'updateOne'); await blockRepository.prepareBlocksOutOfCategoryUpdateScope(otherBlocks, [ - validIds[0], + blockValidIds[0], ]); - expect(mockUpdateOne).toHaveBeenCalledWith('64abc1234def567890fedcab', { - nextBlocks: [validIds[1]], + expect(mockUpdateOne).toHaveBeenCalledWith(blockValidIds[1], { + nextBlocks: [blockValidIds[1]], }); }); }); describe('preUpdateMany', () => { + /******/ it('should update blocks in and out of the scope', async () => { const mockFind = jest.spyOn(blockRepository, 'find').mockResolvedValue([ { - id: '64abc1234def567890fedcab', + id: blockValidIds[1], attachedBlock: validIds[0], nextBlocks: [validIds[0]], }, @@ -278,17 +284,17 @@ describe('BlockRepository', () => { expect(mockFind).toHaveBeenCalled(); expect(prepareBlocksInCategoryUpdateScope).toHaveBeenCalledWith( validCategory, - ['64abc1234def567890fedcab'], + [blockValidIds[1]], ); expect(prepareBlocksOutOfCategoryUpdateScope).toHaveBeenCalledWith( [ { - id: '64abc1234def567890fedcab', + id: blockValidIds[1], attachedBlock: validIds[0], nextBlocks: [validIds[0]], }, ], - ['64abc1234def567890fedcab'], + [blockValidIds[1]], ); }); diff --git a/api/src/chat/repositories/conversation.repository.ts b/api/src/chat/repositories/conversation.repository.ts index 52995fb7..c17c9c62 100644 --- a/api/src/chat/repositories/conversation.repository.ts +++ b/api/src/chat/repositories/conversation.repository.ts @@ -48,7 +48,7 @@ export class ConversationRepository extends BaseRepository< * * @returns A promise resolving to the result of the update operation. */ - async end(convo: Conversation | ConversationFull) { + async end(convo: Conversation | ConversationFull): Promise { return await this.updateOne(convo.id, { active: false }); } } diff --git a/api/src/chat/repositories/subscriber.repository.ts b/api/src/chat/repositories/subscriber.repository.ts index 2d690766..cdbb8cfb 100644 --- a/api/src/chat/repositories/subscriber.repository.ts +++ b/api/src/chat/repositories/subscriber.repository.ts @@ -150,7 +150,7 @@ export class SubscriberRepository extends BaseRepository< async updateOneByForeignIdQuery( id: string, updates: SubscriberUpdateDto, - ): Promise { + ): Promise { return await this.updateOne({ foreign_id: id }, updates); } @@ -161,9 +161,7 @@ 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, @@ -186,7 +184,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/services/conversation.service.ts b/api/src/chat/services/conversation.service.ts index 118f367f..ca650703 100644 --- a/api/src/chat/services/conversation.service.ts +++ b/api/src/chat/services/conversation.service.ts @@ -184,11 +184,6 @@ export class ConversationService extends BaseService< const updatedConversation = await this.updateOne(convo.id, { context: convo.context, }); - if (!updatedConversation) { - throw new Error( - 'Conversation Model : No conversation has been updated', - ); - } //TODO: add check if nothing changed don't update const criteria = diff --git a/api/src/cms/controllers/content-type.controller.spec.ts b/api/src/cms/controllers/content-type.controller.spec.ts index 29d7646c..88dd5e74 100644 --- a/api/src/cms/controllers/content-type.controller.spec.ts +++ b/api/src/cms/controllers/content-type.controller.spec.ts @@ -17,6 +17,7 @@ import { AttachmentService } from '@/attachment/services/attachment.service'; import { BlockService } from '@/chat/services/block.service'; import { LoggerService } from '@/logger/logger.service'; import { NOT_FOUND_ID } from '@/utils/constants/mock'; +import { getUpdateOneError } from '@/utils/test/errors/messages'; import { installContentFixtures } from '@/utils/test/fixtures/content'; import { contentTypeFixtures } from '@/utils/test/fixtures/contenttype'; import { getPageQuery } from '@/utils/test/pagination'; @@ -174,7 +175,7 @@ describe('ContentTypeController', () => { jest.spyOn(contentTypeService, 'updateOne'); await expect( contentTypeController.updateOne(updatedContent, NOT_FOUND_ID), - ).rejects.toThrow(NotFoundException); + ).rejects.toThrow(getUpdateOneError(ContentType.name, NOT_FOUND_ID)); expect(contentTypeService.updateOne).toHaveBeenCalledWith( NOT_FOUND_ID, updatedContent, diff --git a/api/src/cms/controllers/content-type.controller.ts b/api/src/cms/controllers/content-type.controller.ts index b7204fb0..9477196b 100644 --- a/api/src/cms/controllers/content-type.controller.ts +++ b/api/src/cms/controllers/content-type.controller.ts @@ -148,17 +148,6 @@ export class ContentTypeController extends BaseController { @Body() contentTypeDto: ContentTypeUpdateDto, @Param('id') id: string, ) { - const updatedContentType = await this.contentTypeService.updateOne( - id, - contentTypeDto, - ); - - if (!updatedContentType) { - this.logger.warn( - `Failed to update content type with id ${id}. Content type not found.`, - ); - throw new NotFoundException(`Content type with id ${id} not found`); - } - return updatedContentType; + return await this.contentTypeService.updateOne(id, contentTypeDto); } } diff --git a/api/src/cms/controllers/content.controller.spec.ts b/api/src/cms/controllers/content.controller.spec.ts index c6958345..e18df455 100644 --- a/api/src/cms/controllers/content.controller.spec.ts +++ b/api/src/cms/controllers/content.controller.spec.ts @@ -23,6 +23,7 @@ import { LoggerService } from '@/logger/logger.service'; import { NOT_FOUND_ID } from '@/utils/constants/mock'; import { PageQueryDto } from '@/utils/pagination/pagination-query.dto'; import { IGNORED_TEST_FIELDS } from '@/utils/test/constants'; +import { getUpdateOneError } from '@/utils/test/errors/messages'; import { contentFixtures, installContentFixtures, @@ -194,7 +195,7 @@ describe('ContentController', () => { it('should throw NotFoundException if the content is not found', async () => { await expect( contentController.updateOne(updatedContent, NOT_FOUND_ID), - ).rejects.toThrow(NotFoundException); + ).rejects.toThrow(getUpdateOneError(Content.name, NOT_FOUND_ID)); }); }); diff --git a/api/src/cms/controllers/content.controller.ts b/api/src/cms/controllers/content.controller.ts index a4903bec..8913720c 100644 --- a/api/src/cms/controllers/content.controller.ts +++ b/api/src/cms/controllers/content.controller.ts @@ -300,13 +300,6 @@ export class ContentController extends BaseController< @Body() contentDto: ContentUpdateDto, @Param('id') id: string, ): Promise { - const updatedContent = await this.contentService.updateOne(id, contentDto); - if (!updatedContent) { - this.logger.warn( - `Failed to update content with id ${id}. Content not found.`, - ); - throw new NotFoundException(`Content of id ${id} not found`); - } - return updatedContent; + return await this.contentService.updateOne(id, contentDto); } } diff --git a/api/src/cms/controllers/menu.controller.ts b/api/src/cms/controllers/menu.controller.ts index cf9cc164..28f70947 100644 --- a/api/src/cms/controllers/menu.controller.ts +++ b/api/src/cms/controllers/menu.controller.ts @@ -165,7 +165,10 @@ export class MenuController extends BaseController< */ @CsrfCheck(true) @Patch(':id') - async updateOne(@Body() body: MenuCreateDto, @Param('id') id: string) { + async updateOne( + @Body() body: MenuCreateDto, + @Param('id') id: string, + ): Promise { if (!id) return await this.create(body); return await this.menuService.updateOne(id, body); } diff --git a/api/src/i18n/controllers/language.controller.spec.ts b/api/src/i18n/controllers/language.controller.spec.ts index d260a502..e8aaf8df 100644 --- a/api/src/i18n/controllers/language.controller.spec.ts +++ b/api/src/i18n/controllers/language.controller.spec.ts @@ -7,7 +7,7 @@ */ import { CACHE_MANAGER } from '@nestjs/cache-manager'; -import { BadRequestException, NotFoundException } from '@nestjs/common'; +import { BadRequestException } from '@nestjs/common'; import { EventEmitter2 } from '@nestjs/event-emitter'; import { MongooseModule } from '@nestjs/mongoose'; import { Test } from '@nestjs/testing'; @@ -15,6 +15,7 @@ import { Test } from '@nestjs/testing'; import { I18nService } from '@/i18n/services/i18n.service'; import { LoggerService } from '@/logger/logger.service'; import { NOT_FOUND_ID } from '@/utils/constants/mock'; +import { getUpdateOneError } from '@/utils/test/errors/messages'; import { installLanguageFixtures, languageFixtures, @@ -169,7 +170,7 @@ describe('LanguageController', () => { jest.spyOn(languageService, 'updateOne'); await expect( languageController.updateOne(NOT_FOUND_ID, translationUpdateDto), - ).rejects.toThrow(NotFoundException); + ).rejects.toThrow(getUpdateOneError(Language.name, NOT_FOUND_ID)); }); }); diff --git a/api/src/i18n/controllers/language.controller.ts b/api/src/i18n/controllers/language.controller.ts index c8a769f1..cfbaffea 100644 --- a/api/src/i18n/controllers/language.controller.ts +++ b/api/src/i18n/controllers/language.controller.ts @@ -123,12 +123,7 @@ export class LanguageController extends BaseController { } } - const result = await this.languageService.updateOne(id, languageUpdate); - if (!result) { - this.logger.warn(`Unable to update Language by id ${id}`); - throw new NotFoundException(`Language with ID ${id} not found`); - } - return result; + return await this.languageService.updateOne(id, languageUpdate); } /** diff --git a/api/src/i18n/controllers/translation.controller.spec.ts b/api/src/i18n/controllers/translation.controller.spec.ts index 484c90a2..70fbdcf0 100644 --- a/api/src/i18n/controllers/translation.controller.spec.ts +++ b/api/src/i18n/controllers/translation.controller.spec.ts @@ -7,7 +7,6 @@ */ import { CACHE_MANAGER } from '@nestjs/cache-manager'; -import { NotFoundException } from '@nestjs/common'; import { EventEmitter2 } from '@nestjs/event-emitter'; import { MongooseModule } from '@nestjs/mongoose'; import { Test } from '@nestjs/testing'; @@ -38,6 +37,7 @@ import { NlpService } from '@/nlp/services/nlp.service'; import { PluginService } from '@/plugins/plugins.service'; import { SettingService } from '@/setting/services/setting.service'; import { NOT_FOUND_ID } from '@/utils/constants/mock'; +import { getUpdateOneError } from '@/utils/test/errors/messages'; import { installTranslationFixtures, translationFixtures, @@ -209,7 +209,7 @@ describe('TranslationController', () => { jest.spyOn(translationService, 'updateOne'); await expect( translationController.updateOne(NOT_FOUND_ID, translationUpdateDto), - ).rejects.toThrow(NotFoundException); + ).rejects.toThrow(getUpdateOneError(Translation.name, NOT_FOUND_ID)); }); }); }); diff --git a/api/src/i18n/controllers/translation.controller.ts b/api/src/i18n/controllers/translation.controller.ts index 9e571103..50b09717 100644 --- a/api/src/i18n/controllers/translation.controller.ts +++ b/api/src/i18n/controllers/translation.controller.ts @@ -90,15 +90,7 @@ export class TranslationController extends BaseController { @Param('id') id: string, @Body() translationUpdate: TranslationUpdateDto, ) { - const result = await this.translationService.updateOne( - id, - translationUpdate, - ); - if (!result) { - this.logger.warn(`Unable to update Translation by id ${id}`); - throw new NotFoundException(`Translation with ID ${id} not found`); - } - return result; + return await this.translationService.updateOne(id, translationUpdate); } /** diff --git a/api/src/migration/migration.service.ts b/api/src/migration/migration.service.ts index 0459e395..c0fec5c9 100644 --- a/api/src/migration/migration.service.ts +++ b/api/src/migration/migration.service.ts @@ -522,7 +522,7 @@ module.exports = { version, action, migrationDocument, - }: MigrationSuccessCallback) { + }: MigrationSuccessCallback): Promise { await this.updateStatus({ version, action, migrationDocument }); const migrationDisplayName = `${version} [${action}]`; this.logger.log(`"${migrationDisplayName}" migration done`); diff --git a/api/src/nlp/controllers/nlp-entity.controller.ts b/api/src/nlp/controllers/nlp-entity.controller.ts index ec38fdd6..fbb4889f 100644 --- a/api/src/nlp/controllers/nlp-entity.controller.ts +++ b/api/src/nlp/controllers/nlp-entity.controller.ts @@ -167,17 +167,7 @@ export class NlpEntityController extends BaseController< ); } - const result = await this.nlpEntityService.updateOne( - id, - updateNlpEntityDto, - ); - if (!result) { - this.logger.warn(`Failed to update NLP Entity by id ${id}`); - throw new InternalServerErrorException( - `Failed to update NLP Entity with ID ${id}`, - ); - } - return result; + return await this.nlpEntityService.updateOne(id, updateNlpEntityDto); } /** diff --git a/api/src/nlp/controllers/nlp-sample.controller.spec.ts b/api/src/nlp/controllers/nlp-sample.controller.spec.ts index e3285931..832e21da 100644 --- a/api/src/nlp/controllers/nlp-sample.controller.spec.ts +++ b/api/src/nlp/controllers/nlp-sample.controller.spec.ts @@ -22,6 +22,7 @@ import { SettingRepository } from '@/setting/repositories/setting.repository'; import { SettingModel } from '@/setting/schemas/setting.schema'; import { SettingSeeder } from '@/setting/seeds/setting.seed'; import { SettingService } from '@/setting/services/setting.service'; +import { getUpdateOneError } from '@/utils/test/errors/messages'; import { installAttachmentFixtures } from '@/utils/test/fixtures/attachment'; import { nlpSampleFixtures } from '@/utils/test/fixtures/nlpsample'; import { installNlpSampleEntityFixtures } from '@/utils/test/fixtures/nlpsampleentity'; @@ -310,7 +311,7 @@ describe('NlpSampleController', () => { type: NlpSampleState.test, language: 'fr', }), - ).rejects.toThrow(NotFoundException); + ).rejects.toThrow(getUpdateOneError(NlpSample.name, byeJhonSampleId!)); }); }); diff --git a/api/src/nlp/controllers/nlp-sample.controller.ts b/api/src/nlp/controllers/nlp-sample.controller.ts index cc49d41a..22dfa263 100644 --- a/api/src/nlp/controllers/nlp-sample.controller.ts +++ b/api/src/nlp/controllers/nlp-sample.controller.ts @@ -305,11 +305,6 @@ export class NlpSampleController extends BaseController< trained: false, }); - if (!sample) { - this.logger.warn(`Unable to update NLP Sample by id ${id}`); - throw new NotFoundException(`NLP Sample with ID ${id} not found`); - } - await this.nlpSampleEntityService.deleteMany({ sample: id }); const updatedSampleEntities = diff --git a/api/src/nlp/controllers/nlp-value.controller.spec.ts b/api/src/nlp/controllers/nlp-value.controller.spec.ts index cb535137..d78b336f 100644 --- a/api/src/nlp/controllers/nlp-value.controller.spec.ts +++ b/api/src/nlp/controllers/nlp-value.controller.spec.ts @@ -12,6 +12,7 @@ import { MongooseModule } from '@nestjs/mongoose'; import { Test, TestingModule } from '@nestjs/testing'; import { LoggerService } from '@/logger/logger.service'; +import { getUpdateOneError } from '@/utils/test/errors/messages'; import { nlpEntityFixtures } from '@/utils/test/fixtures/nlpentity'; import { installNlpValueFixtures, @@ -241,7 +242,7 @@ describe('NlpValueController', () => { expressions: [], builtin: true, }), - ).rejects.toThrow(NotFoundException); + ).rejects.toThrow(getUpdateOneError(NlpValue.name, jhonNlpValue!.id)); }); }); describe('deleteMany', () => { diff --git a/api/src/nlp/controllers/nlp-value.controller.ts b/api/src/nlp/controllers/nlp-value.controller.ts index b85caab3..083bc46f 100644 --- a/api/src/nlp/controllers/nlp-value.controller.ts +++ b/api/src/nlp/controllers/nlp-value.controller.ts @@ -168,12 +168,7 @@ export class NlpValueController extends BaseController< @Param('id') id: string, @Body() updateNlpValueDto: NlpValueUpdateDto, ): Promise { - const result = await this.nlpValueService.updateOne(id, updateNlpValueDto); - if (!result) { - this.logger.warn(`Unable to update NLP Value by id ${id}`); - throw new NotFoundException(`NLP Value with ID ${id} not found`); - } - return result; + return await this.nlpValueService.updateOne(id, updateNlpValueDto); } /** diff --git a/api/src/nlp/services/nlp-value.service.ts b/api/src/nlp/services/nlp-value.service.ts index d721efd3..f246ad61 100644 --- a/api/src/nlp/services/nlp-value.service.ts +++ b/api/src/nlp/services/nlp-value.service.ts @@ -63,7 +63,7 @@ export class NlpValueService extends BaseService< sampleText: string, sampleEntities: NlpSampleEntityValue[], storedEntities: NlpEntity[], - ) { + ): Promise { const eMap: Record = storedEntities.reduce( (acc, curr) => { if (curr.name) acc[curr?.name] = curr; diff --git a/api/src/setting/controllers/setting.controller.ts b/api/src/setting/controllers/setting.controller.ts index 75a159ca..60108a2a 100644 --- a/api/src/setting/controllers/setting.controller.ts +++ b/api/src/setting/controllers/setting.controller.ts @@ -10,7 +10,6 @@ import { Body, Controller, Get, - NotFoundException, Param, Patch, Query, @@ -71,11 +70,6 @@ export class SettingController { @Param('id') id: string, @Body() settingUpdateDto: { value: any }, ): Promise { - const result = await this.settingService.updateOne(id, settingUpdateDto); - if (!result) { - this.logger.warn(`Unable to update setting by id ${id}`); - throw new NotFoundException(`Setting with ID ${id} not found`); - } - return result; + return await this.settingService.updateOne(id, settingUpdateDto); } } diff --git a/api/src/user/controllers/role.controller.ts b/api/src/user/controllers/role.controller.ts index a427c5b9..e494aca1 100644 --- a/api/src/user/controllers/role.controller.ts +++ b/api/src/user/controllers/role.controller.ts @@ -134,12 +134,7 @@ export class RoleController extends BaseController< @CsrfCheck(true) @Patch(':id') async updateOne(@Param('id') id: string, @Body() roleUpdate: RoleUpdateDto) { - const result = await this.roleService.updateOne(id, roleUpdate); - if (!result) { - this.logger.warn(`Unable to update Role by id ${id}`); - throw new NotFoundException(`Role with ID ${id} not found`); - } - return result; + return await this.roleService.updateOne(id, roleUpdate); } /** diff --git a/api/src/user/controllers/user.controller.ts b/api/src/user/controllers/user.controller.ts index 5f8ff25d..3546ee0a 100644 --- a/api/src/user/controllers/user.controller.ts +++ b/api/src/user/controllers/user.controller.ts @@ -304,7 +304,7 @@ export class ReadWriteUserController extends ReadOnlyUserController { ) : undefined; - const result = await this.userService.updateOne( + return await this.userService.updateOne( req.user.id, avatar ? { @@ -313,12 +313,6 @@ export class ReadWriteUserController extends ReadOnlyUserController { } : userUpdate, ); - - if (!result) { - this.logger.warn(`Unable to update User by id ${id}`); - throw new NotFoundException(`User with ID ${id} not found`); - } - return result; } /** diff --git a/api/src/utils/generics/base-repository.ts b/api/src/utils/generics/base-repository.ts index e0c4fd65..550b6c90 100644 --- a/api/src/utils/generics/base-repository.ts +++ b/api/src/utils/generics/base-repository.ts @@ -482,7 +482,7 @@ export abstract class BaseRepository< options: QueryOptions | null = { new: true, }, - ): Promise { + ): Promise { const query = this.model.findOneAndUpdate( { ...(typeof criteria === 'string' ? { _id: criteria } : criteria), @@ -512,7 +512,14 @@ export abstract class BaseRepository< filterCriteria, queryUpdates, ); - return await this.executeOne(query, this.cls); + const result = await this.executeOne(query, this.cls); + + if (!result) { + const errorMessage = `Unable to update ${this.cls.name}${typeof criteria === 'string' ? ` with ID ${criteria}` : ''}`; + throw new Error(errorMessage); + } + + return result; } async updateMany>( diff --git a/api/src/utils/generics/base-service.ts b/api/src/utils/generics/base-service.ts index bbedbcea..a7b04c55 100644 --- a/api/src/utils/generics/base-service.ts +++ b/api/src/utils/generics/base-service.ts @@ -179,7 +179,7 @@ export abstract class BaseService< criteria: string | TFilterQuery, dto: DtoInfer>, options?: QueryOptions> | null, - ): Promise { + ): Promise { return await this.repository.updateOne(criteria, dto, options); } diff --git a/api/src/utils/test/errors/messages.ts b/api/src/utils/test/errors/messages.ts new file mode 100644 index 00000000..150f1c61 --- /dev/null +++ b/api/src/utils/test/errors/messages.ts @@ -0,0 +1,10 @@ +/* + * Copyright © 2025 Hexastack. All rights reserved. + * + * Licensed under the GNU Affero General Public License v3.0 (AGPLv3) with the following additional terms: + * 1. The name "Hexabot" is a trademark of Hexastack. You may not use this name in derivative works without express written permission. + * 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). + */ + +export const getUpdateOneError = (entity: string, id?: string) => + new Error(`Unable to update ${entity}${id ? ` with ID ${id}` : ''}`); From 6eb29e9a96c6a3b250931e39321f7c5f4800439e Mon Sep 17 00:00:00 2001 From: yassinedorbozgithub Date: Fri, 17 Jan 2025 11:56:59 +0100 Subject: [PATCH 2/2] fix: handle object error messages --- api/src/chat/repositories/block.repository.spec.ts | 4 ---- api/src/utils/generics/base-repository.ts | 2 +- api/src/utils/test/errors/messages.ts | 2 +- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/api/src/chat/repositories/block.repository.spec.ts b/api/src/chat/repositories/block.repository.spec.ts index 7dc3c5cb..e37d7d62 100644 --- a/api/src/chat/repositories/block.repository.spec.ts +++ b/api/src/chat/repositories/block.repository.spec.ts @@ -169,7 +169,6 @@ describe('BlockRepository', () => { }); describe('prepareBlocksInCategoryUpdateScope', () => { - /******/ it('should update blocks within the scope based on category and ids', async () => { jest.spyOn(blockRepository, 'findOne').mockResolvedValue({ id: blockValidIds[0], @@ -211,7 +210,6 @@ describe('BlockRepository', () => { }); describe('prepareBlocksOutOfCategoryUpdateScope', () => { - /******/ it('should update blocks outside the scope by removing references from attachedBlock', async () => { const otherBlocks = [ { @@ -233,7 +231,6 @@ describe('BlockRepository', () => { }); }); - /******/ it('should update blocks outside the scope by removing references from nextBlocks', async () => { const otherBlocks = [ { @@ -256,7 +253,6 @@ describe('BlockRepository', () => { }); describe('preUpdateMany', () => { - /******/ it('should update blocks in and out of the scope', async () => { const mockFind = jest.spyOn(blockRepository, 'find').mockResolvedValue([ { diff --git a/api/src/utils/generics/base-repository.ts b/api/src/utils/generics/base-repository.ts index 550b6c90..523a42b9 100644 --- a/api/src/utils/generics/base-repository.ts +++ b/api/src/utils/generics/base-repository.ts @@ -515,7 +515,7 @@ export abstract class BaseRepository< const result = await this.executeOne(query, this.cls); if (!result) { - const errorMessage = `Unable to update ${this.cls.name}${typeof criteria === 'string' ? ` with ID ${criteria}` : ''}`; + const errorMessage = `Unable to update ${this.cls.name} with ${typeof criteria === 'string' ? 'ID' : 'criteria'} ${JSON.stringify(criteria)}`; throw new Error(errorMessage); } diff --git a/api/src/utils/test/errors/messages.ts b/api/src/utils/test/errors/messages.ts index 150f1c61..2ecc3aff 100644 --- a/api/src/utils/test/errors/messages.ts +++ b/api/src/utils/test/errors/messages.ts @@ -7,4 +7,4 @@ */ export const getUpdateOneError = (entity: string, id?: string) => - new Error(`Unable to update ${entity}${id ? ` with ID ${id}` : ''}`); + new Error(`Unable to update ${entity}${id ? ` with ID \"${id}\"` : ''}`);