refactor(api): Refactor updateOne logic

This commit is contained in:
yassinedorbozgithub 2025-01-16 18:47:25 +01:00
parent 47e8056a15
commit 6c75e6df4a
35 changed files with 92 additions and 164 deletions

View File

@ -110,7 +110,7 @@ export class BotStatsService extends BaseService<BotStats> {
* @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<void> {
const day = new Date();
day.setMilliseconds(0);
day.setSeconds(0);

View File

@ -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));
});
});
});

View File

@ -296,12 +296,7 @@ export class BlockController extends BaseController<
@Param('id') id: string,
@Body() blockUpdate: BlockUpdateDto,
): Promise<Block> {
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);
}
/**

View File

@ -114,12 +114,7 @@ export class CategoryController extends BaseController<Category> {
@Param('id') id: string,
@Body() categoryUpdate: CategoryUpdateDto,
): Promise<Category> {
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);
}
/**

View File

@ -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),
);
});
});

View File

@ -120,12 +120,7 @@ export class ContextVarController extends BaseController<ContextVar> {
@Param('id') id: string,
@Body() contextVarUpdate: ContextVarUpdateDto,
): Promise<ContextVar> {
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);
}
/**

View File

@ -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';
@ -226,9 +227,7 @@ describe('LabelController', () => {
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));
});
});
});

View File

@ -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)

View File

@ -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);
}
}

View File

@ -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]],
);
});

View File

@ -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<Conversation> {
return await this.updateOne(convo.id, { active: false });
}
}

View File

@ -150,7 +150,7 @@ export class SubscriberRepository extends BaseRepository<
async updateOneByForeignIdQuery(
id: string,
updates: SubscriberUpdateDto,
): Promise<Subscriber | null> {
): Promise<Subscriber> {
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<Subscriber | null> {
async handBackByForeignIdQuery(foreignId: string): Promise<Subscriber> {
return await this.updateOne(
{
foreign_id: foreignId,
@ -186,7 +184,7 @@ export class SubscriberRepository extends BaseRepository<
async handOverByForeignIdQuery(
foreignId: string,
userId: string,
): Promise<Subscriber | null> {
): Promise<Subscriber> {
return await this.updateOne(
{
foreign_id: foreignId,

View File

@ -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 =

View File

@ -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,

View File

@ -148,17 +148,6 @@ export class ContentTypeController extends BaseController<ContentType> {
@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);
}
}

View File

@ -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));
});
});

View File

@ -300,13 +300,6 @@ export class ContentController extends BaseController<
@Body() contentDto: ContentUpdateDto,
@Param('id') id: string,
): Promise<Content> {
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);
}
}

View File

@ -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<Menu> {
if (!id) return await this.create(body);
return await this.menuService.updateOne(id, body);
}

View File

@ -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));
});
});

View File

@ -123,12 +123,7 @@ export class LanguageController extends BaseController<Language> {
}
}
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);
}
/**

View File

@ -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));
});
});
});

View File

@ -90,15 +90,7 @@ export class TranslationController extends BaseController<Translation> {
@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);
}
/**

View File

@ -522,7 +522,7 @@ module.exports = {
version,
action,
migrationDocument,
}: MigrationSuccessCallback) {
}: MigrationSuccessCallback): Promise<void> {
await this.updateStatus({ version, action, migrationDocument });
const migrationDisplayName = `${version} [${action}]`;
this.logger.log(`"${migrationDisplayName}" migration done`);

View File

@ -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);
}
/**

View File

@ -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!));
});
});

View File

@ -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 =

View File

@ -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', () => {

View File

@ -168,12 +168,7 @@ export class NlpValueController extends BaseController<
@Param('id') id: string,
@Body() updateNlpValueDto: NlpValueUpdateDto,
): Promise<NlpValue> {
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);
}
/**

View File

@ -63,7 +63,7 @@ export class NlpValueService extends BaseService<
sampleText: string,
sampleEntities: NlpSampleEntityValue[],
storedEntities: NlpEntity[],
) {
): Promise<NlpSampleEntityValue[]> {
const eMap: Record<string, NlpEntity> = storedEntities.reduce(
(acc, curr) => {
if (curr.name) acc[curr?.name] = curr;

View File

@ -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<Setting> {
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);
}
}

View File

@ -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);
}
/**

View File

@ -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;
}
/**

View File

@ -482,7 +482,7 @@ export abstract class BaseRepository<
options: QueryOptions<D> | null = {
new: true,
},
): Promise<T | null> {
): Promise<T> {
const query = this.model.findOneAndUpdate<T>(
{
...(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<D extends Partial<U>>(

View File

@ -179,7 +179,7 @@ export abstract class BaseService<
criteria: string | TFilterQuery<T>,
dto: DtoInfer<DtoAction.Update, Dto, Partial<U>>,
options?: QueryOptions<Partial<U>> | null,
): Promise<T | null> {
): Promise<T> {
return await this.repository.updateOne(criteria, dto, options);
}

View File

@ -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}` : ''}`);