diff --git a/api/src/chat/repositories/block.repository.spec.ts b/api/src/chat/repositories/block.repository.spec.ts index 37dae25f..2f7cf226 100644 --- a/api/src/chat/repositories/block.repository.spec.ts +++ b/api/src/chat/repositories/block.repository.spec.ts @@ -9,7 +9,7 @@ import { EventEmitter2 } from '@nestjs/event-emitter'; import { MongooseModule, getModelToken } from '@nestjs/mongoose'; import { Test } from '@nestjs/testing'; -import mongoose, { Model } from 'mongoose'; +import { Model } from 'mongoose'; import { blockFixtures, @@ -36,7 +36,6 @@ describe('BlockRepository', () => { let hasNextBlocks: Block; let validIds: string[]; let validCategory: string; - let objCategory: mongoose.Types.ObjectId; beforeAll(async () => { const module = await Test.createTestingModule({ @@ -51,7 +50,6 @@ describe('BlockRepository', () => { blockModel = module.get>(getModelToken('Block')); validIds = ['64abc1234def567890fedcba', '64abc1234def567890fedcbc']; validCategory = '64def5678abc123490fedcba'; - objCategory = new mongoose.Types.ObjectId('64def5678abc123490fedcba'); category = await categoryRepository.findOne({ label: 'default' }); hasPreviousBlocks = await blockRepository.findOne({ @@ -117,269 +115,161 @@ describe('BlockRepository', () => { }); describe('preUpdate', () => { - it('should update blocks referencing the moved block when category is updated with valid criteria', async () => { - const mockUpdateMany = jest.spyOn(blockModel, 'updateMany'); + it('should remove references to a moved block when updating category', async () => { + const mockUpdateMany = jest.spyOn(blockRepository, 'updateMany'); + const criteria = { _id: validIds[0] }; + const updates = { $set: { category: validCategory } }; - const criteria = { _id: hasPreviousBlocks.id }; - const updates = { $set: { category: 'newCategory' } }; - const mockQuery = {} as any; + const mockFindOne = jest + .spyOn(blockRepository, 'findOne') + .mockResolvedValue({ + id: validIds[0], + category: 'oldCategory', + } as Block); - await blockRepository.preUpdate(mockQuery, criteria, updates); + await blockRepository.preUpdate({} as any, criteria, updates); + expect(mockFindOne).toHaveBeenCalledWith(criteria); + expect(mockUpdateMany).toHaveBeenCalledTimes(2); expect(mockUpdateMany).toHaveBeenNthCalledWith( 1, - { nextBlocks: hasPreviousBlocks.id }, - { $pull: { nextBlocks: hasPreviousBlocks.id } }, + { nextBlocks: validIds[0] }, + { $pull: { nextBlocks: validIds[0] } }, ); - expect(mockUpdateMany).toHaveBeenNthCalledWith( 2, - { attachedBlock: hasPreviousBlocks.id }, + { attachedBlock: validIds[0] }, { $set: { attachedBlock: null } }, ); }); - it('should throw an error if category is updated without a valid _id in criteria', async () => { - const updates = { $set: { category: 'newCategory' } }; - const mockQuery = {} as any; + it('should do nothing if no block is found for the criteria', async () => { + jest.spyOn(blockRepository, 'findOne').mockResolvedValue(null); + const mockUpdateMany = jest.spyOn(blockRepository, 'updateMany'); - await expect( - blockRepository.preUpdate(mockQuery, {}, updates), - ).rejects.toThrowError( - 'Criteria must include a valid id to update category.', - ); - }); - - it('should call checkDeprecatedAttachmentUrl with the correct update object', async () => { - const mockCheckDeprecatedAttachmentUrl = jest.spyOn( - blockRepository, - 'checkDeprecatedAttachmentUrl', - ); - - const criteria = { _id: hasPreviousBlocks.id }; - const updates = { - $set: { category: 'newCategory', attachedBlock: 'someUrl' }, - }; - const mockQuery = {} as any; - - await blockRepository.preUpdate(mockQuery, criteria, updates); - - expect(mockCheckDeprecatedAttachmentUrl).toHaveBeenCalledWith( - updates.$set, - ); - }); - - it('should not call updateMany if no category update is provided', async () => { - const mockUpdateMany = jest.spyOn(blockModel, 'updateMany'); - - const criteria = { _id: hasPreviousBlocks.id }; - const updates = { $set: { name: 'newName' } }; - const mockQuery = {} as any; - - await blockRepository.preUpdate(mockQuery, criteria, updates); + await blockRepository.preUpdate({} as any, { _id: 'nonexistent' }, {}); expect(mockUpdateMany).not.toHaveBeenCalled(); }); }); - describe('mapIdsAndCategory', () => { - it('should map string IDs and category to Mongoose ObjectIDs', async () => { - const result = blockRepository.mapIdsAndCategory(validIds, validCategory); - - expect(result.objIds).toHaveLength(validIds.length); - validIds.forEach((id, index) => { - expect(result.objIds[index].toHexString()).toBe(id); - }); - expect(result.objCategory.toHexString()).toBe(validCategory); - }); - - it('should throw an error if invalid IDs or category are provided', () => { - const ids = ['invalidId', '64xyz6789abc1234567defca']; - const category = 'invalidCategory'; - - expect(() => - blockRepository.mapIdsAndCategory(ids, category), - ).toThrowError( - 'input must be a 24 character hex string, 12 byte Uint8Array, or an integer', - ); - }); - }); - describe('updateBlocksInScope', () => { - it('should update blocks within the scope', async () => { - const mockFindOne = jest.spyOn(blockModel, 'findOne').mockResolvedValue({ - _id: validIds[0], - category: new mongoose.Types.ObjectId('64abc1234def567890fedcbc'), - nextBlocks: [new mongoose.Types.ObjectId(validIds[1])], - attachedBlock: new mongoose.Types.ObjectId(validIds[1]), + it('should update blocks within the scope based on category and ids', async () => { + jest.spyOn(blockRepository, 'findOne').mockResolvedValue({ + id: validIds[0], + category: 'oldCategory', + nextBlocks: [validIds[1]], + attachedBlock: validIds[1], + } as Block); + + const mockUpdateOne = jest.spyOn(blockRepository, 'updateOne'); + + await blockRepository.updateBlocksInScope(validCategory, validIds); + + expect(mockUpdateOne).toHaveBeenCalledWith(validIds[0], { + nextBlocks: [validIds[1]], + attachedBlock: validIds[1], }); - - const mockUpdateOne = jest.spyOn(blockModel, 'updateOne'); - - await blockRepository.updateBlocksInScope(objCategory, validIds); - - expect(mockFindOne).toHaveBeenCalledWith({ - _id: new mongoose.Types.ObjectId(validIds[0]), - }); - expect(mockUpdateOne).toHaveBeenCalledWith( - { _id: new mongoose.Types.ObjectId(validIds[0]) }, - { - nextBlocks: [new mongoose.Types.ObjectId(validIds[1])], - attachedBlock: new mongoose.Types.ObjectId(validIds[1]), - }, - ); }); - it('should not update blocks if category matches', async () => { - jest.spyOn(blockModel, 'findOne').mockResolvedValue({ - _id: validIds[0], - category: objCategory, + it('should not update blocks if the category already matches', async () => { + jest.spyOn(blockRepository, 'findOne').mockResolvedValue({ + id: validIds[0], + category: validCategory, nextBlocks: [], attachedBlock: null, - }); + } as Block); - const mockUpdateOne = jest.spyOn(blockModel, 'updateOne'); + const mockUpdateOne = jest.spyOn(blockRepository, 'updateOne'); - await blockRepository.updateBlocksInScope(objCategory, validIds); + await blockRepository.updateBlocksInScope(validCategory, validIds); expect(mockUpdateOne).not.toHaveBeenCalled(); }); }); describe('updateExternalBlocks', () => { - let validIds: string[]; - - beforeAll(() => { - validIds = ['64abc1234def567890fedcba', '64def5678abc123490fedcbc']; - }); - - it('should update external blocks with attachedBlock or nextBlocks', async () => { + it('should update blocks outside the scope by removing references from attachedBlock', async () => { const otherBlocks = [ { - id: new mongoose.Types.ObjectId('64abc1234def567890fedcba'), - attachedBlock: new mongoose.Types.ObjectId(validIds[0]), - nextBlocks: [new mongoose.Types.ObjectId(validIds[0])], + id: '64abc1234def567890fedcab', + attachedBlock: validIds[0], + nextBlocks: [validIds[0]], }, - ]; - const mockUpdateOne = jest.spyOn(blockModel, 'updateOne'); + ] as Block[]; - await blockRepository.updateExternalBlocks(otherBlocks, [ - new mongoose.Types.ObjectId(validIds[0]), - ]); + const mockUpdateOne = jest.spyOn(blockRepository, 'updateOne'); - expect(mockUpdateOne).toHaveBeenCalledWith( - { _id: otherBlocks[0].id }, - { attachedBlock: null }, - ); + await blockRepository.updateExternalBlocks(otherBlocks, validIds); - expect(mockUpdateOne).toHaveBeenCalledWith( - { _id: otherBlocks[0].id }, - { nextBlocks: [] }, - ); + expect(mockUpdateOne).toHaveBeenCalledWith('64abc1234def567890fedcab', { + attachedBlock: null, + }); }); - it('should not update if no changes are necessary', async () => { + it('should update blocks outside the scope by removing references from nextBlocks', async () => { const otherBlocks = [ { - id: new mongoose.Types.ObjectId('64abc1234def567890fedcba'), + id: '64abc1234def567890fedcab', attachedBlock: null, - nextBlocks: [], + nextBlocks: [validIds[0], validIds[1]], }, - ]; - const mockUpdateOne = jest.spyOn(blockModel, 'updateOne'); + ] as Block[]; - await blockRepository.updateExternalBlocks(otherBlocks, [ - new mongoose.Types.ObjectId(validIds[0]), - ]); + const mockUpdateOne = jest.spyOn(blockRepository, 'updateOne'); - expect(mockUpdateOne).not.toHaveBeenCalled(); + await blockRepository.updateExternalBlocks(otherBlocks, [validIds[0]]); + + expect(mockUpdateOne).toHaveBeenCalledWith('64abc1234def567890fedcab', { + $pull: { nextBlocks: [validIds[1]] }, + }); }); }); describe('preUpdateMany', () => { - let validIds: string[]; - let objCategory: mongoose.Types.ObjectId; - - beforeAll(() => { - validIds = ['64abc1234def567890fedcba', '64def5678abc123490fedcbc']; - objCategory = new mongoose.Types.ObjectId('64def5678abc123490fedcbc'); - }); - - it('should map IDs, find other blocks, and update blocks in scope and external blocks', async () => { - const mockMapIdsAndCategory = jest - .spyOn(blockRepository, 'mapIdsAndCategory') - .mockReturnValue({ - objIds: validIds.map((id) => new mongoose.Types.ObjectId(id)), - objCategory, - }); - - const mockFind = jest.spyOn(blockModel, 'find').mockResolvedValue([ + it('should update blocks in and out of the scope', async () => { + const mockFind = jest.spyOn(blockRepository, 'find').mockResolvedValue([ { - id: new mongoose.Types.ObjectId('64abc1234def567890fedcba'), - attachedBlock: new mongoose.Types.ObjectId(validIds[0]), - nextBlocks: [new mongoose.Types.ObjectId(validIds[0])], + id: '64abc1234def567890fedcab', + attachedBlock: validIds[0], + nextBlocks: [validIds[0]], }, - ]); + ] as Block[]); - const mockUpdateBlocksInScope = jest - .spyOn(blockRepository, 'updateBlocksInScope') - .mockResolvedValue(undefined); - - const mockUpdateExternalBlocks = jest - .spyOn(blockRepository, 'updateExternalBlocks') - .mockResolvedValue(undefined); + const mockUpdateBlocksInScope = jest.spyOn( + blockRepository, + 'updateBlocksInScope', + ); + const mockUpdateExternalBlocks = jest.spyOn( + blockRepository, + 'updateExternalBlocks', + ); await blockRepository.preUpdateMany( {} as any, { _id: { $in: validIds } }, - { $set: { category: objCategory.toHexString() } }, + { $set: { category: validCategory } }, ); - expect(mockMapIdsAndCategory).toHaveBeenCalledWith( - validIds, - objCategory.toHexString(), - ); - - expect(mockFind).toHaveBeenCalledWith({ - _id: { $nin: validIds.map((id) => new mongoose.Types.ObjectId(id)) }, - category: { $ne: objCategory }, - $or: [ - { - attachedBlock: { - $in: validIds.map((id) => new mongoose.Types.ObjectId(id)), - }, - }, - { - nextBlocks: { - $in: validIds.map((id) => new mongoose.Types.ObjectId(id)), - }, - }, - ], - }); - + expect(mockFind).toHaveBeenCalled(); expect(mockUpdateBlocksInScope).toHaveBeenCalledWith( - objCategory, + validCategory, validIds, ); - expect(mockUpdateExternalBlocks).toHaveBeenCalledWith( [ { - id: new mongoose.Types.ObjectId('64abc1234def567890fedcba'), - attachedBlock: new mongoose.Types.ObjectId(validIds[0]), - nextBlocks: [new mongoose.Types.ObjectId(validIds[0])], + id: '64abc1234def567890fedcab', + attachedBlock: validIds[0], + nextBlocks: [validIds[0]], }, ], - validIds.map((id) => new mongoose.Types.ObjectId(id)), + validIds, ); }); - it('should not perform updates if criteria or updates are missing', async () => { - const mockMapIdsAndCategory = jest.spyOn( - blockRepository, - 'mapIdsAndCategory', - ); - const mockFind = jest.spyOn(blockModel, 'find'); + it('should not perform updates if no category is provided', async () => { + const mockFind = jest.spyOn(blockRepository, 'find'); const mockUpdateBlocksInScope = jest.spyOn( blockRepository, 'updateBlocksInScope', @@ -391,7 +281,6 @@ describe('BlockRepository', () => { await blockRepository.preUpdateMany({} as any, {}, {}); - expect(mockMapIdsAndCategory).not.toHaveBeenCalled(); expect(mockFind).not.toHaveBeenCalled(); expect(mockUpdateBlocksInScope).not.toHaveBeenCalled(); expect(mockUpdateExternalBlocks).not.toHaveBeenCalled(); diff --git a/api/src/chat/repositories/block.repository.ts b/api/src/chat/repositories/block.repository.ts index 2931ad6d..8b610bed 100644 --- a/api/src/chat/repositories/block.repository.ts +++ b/api/src/chat/repositories/block.repository.ts @@ -94,22 +94,24 @@ export class BlockRepository extends BaseRepository< | UpdateWithAggregationPipeline | UpdateQuery>, ): Promise { + const movedBlock = await this.findOne(criteria); + if (!movedBlock) { + return; + } const update: BlockUpdateDto = updates?.['$set']; - if (update?.category && criteria._id) { + if (update?.category) { const movedBlockId = criteria._id; // Find and update blocks that reference the moved block - await this.model.updateMany( + await this.updateMany( { nextBlocks: movedBlockId }, { $pull: { nextBlocks: movedBlockId } }, ); - await this.model.updateMany( + await this.updateMany( { attachedBlock: movedBlockId }, { $set: { attachedBlock: null } }, ); - } else if (update?.category && !criteria._id) { - throw new Error('Criteria must include a valid id to update category.'); } this.checkDeprecatedAttachmentUrl(update); @@ -139,10 +141,11 @@ export class BlockRepository extends BaseRepository< const category: string = updates.$set.category; // Step 1: Map IDs and Category - const { objIds, objCategory } = this.mapIdsAndCategory(ids, category); + const objIds = ids.map((id) => new mongoose.Types.ObjectId(id)); + const objCategory = new mongoose.Types.ObjectId(category); // Step 2: Find other blocks - const otherBlocks = await this.model.find({ + const otherBlocks = await this.find({ _id: { $nin: objIds }, category: { $ne: objCategory }, $or: [ @@ -152,76 +155,66 @@ export class BlockRepository extends BaseRepository< }); // Step 3: Update blocks in the provided scope - await this.updateBlocksInScope(objCategory, ids); + await this.updateBlocksInScope(category, ids); // Step 4: Update external blocks - await this.updateExternalBlocks(otherBlocks, objIds); + await this.updateExternalBlocks(otherBlocks, ids); } } - mapIdsAndCategory( - ids: string[], - category: string, - ): { - objIds: mongoose.Types.ObjectId[]; - objCategory: mongoose.Types.ObjectId; - } { - const objIds = ids.map((id) => new mongoose.Types.ObjectId(id)); - const objCategory = new mongoose.Types.ObjectId(category); - return { objIds, objCategory }; - } - - async updateBlocksInScope( - objCategory: mongoose.Types.ObjectId, - ids: string[], - ): Promise { + /** + * Updates blocks within a specified category scope. + * Ensures nextBlocks and attachedBlock are consistent with the provided IDs and category. + * + * @param category - The category + * @param ids - IDs representing the blocks to update. + * @returns A promise that resolves once all updates within the scope are complete. + */ + async updateBlocksInScope(category: string, ids: string[]): Promise { for (const id of ids) { - const oldState = await this.model.findOne({ - _id: new mongoose.Types.ObjectId(id), - }); - if (oldState.category.toString() !== objCategory.toString()) { + const oldState = await this.findOne(id); + if (oldState.category !== category) { const updatedNextBlocks = oldState.nextBlocks.filter((nextBlock) => - ids.includes(nextBlock.toString()), + ids.includes(nextBlock), ); - const updatedAttachedBlock = ids.includes( - oldState.attachedBlock?.toString() || '', - ) + const updatedAttachedBlock = ids.includes(oldState.attachedBlock || '') ? oldState.attachedBlock : null; - await this.model.updateOne( - { _id: new mongoose.Types.ObjectId(id) }, - { - nextBlocks: updatedNextBlocks, - attachedBlock: updatedAttachedBlock, - }, - ); + await this.updateOne(id, { + nextBlocks: updatedNextBlocks, + attachedBlock: updatedAttachedBlock, + }); } } } + /** + * Updates blocks outside the specified category scope by removing references to the provided IDs. + * Handles updates to both attachedBlock and nextBlocks. + * + * @param otherBlocks - An array of blocks outside the provided category scope. + * @param ids - An array of the Ids to disassociate. + * @returns A promise that resolves once all external block updates are complete. + */ async updateExternalBlocks( - otherBlocks, - objIds: Types.ObjectId[], + otherBlocks: Block[], + ids: string[], ): Promise { for (const block of otherBlocks) { - if ( - objIds.some((id) => id.toString() === block.attachedBlock?.toString()) - ) { - await this.model.updateOne({ _id: block.id }, { attachedBlock: null }); + if (ids.includes(block.attachedBlock)) { + await this.updateOne(block.id, { attachedBlock: null }); } const updatedNextBlocks = block.nextBlocks.filter( - (nextBlock) => - !objIds.some((id) => id.toString() === nextBlock.toString()), + (nextBlock) => !ids.includes(nextBlock), ); - if (updatedNextBlocks.length !== block.nextBlocks.length) { - await this.model.updateOne( - { _id: block.id }, - { nextBlocks: updatedNextBlocks }, - ); + if (updatedNextBlocks.length > 0) { + await this.updateOne(block.id, { + $pull: { nextBlocks: updatedNextBlocks }, + }); } } } diff --git a/api/src/utils/generics/base-repository.ts b/api/src/utils/generics/base-repository.ts index 78750c13..00f205ce 100644 --- a/api/src/utils/generics/base-repository.ts +++ b/api/src/utils/generics/base-repository.ts @@ -343,7 +343,7 @@ export abstract class BaseRepository< async updateOne>( criteria: string | TFilterQuery, - dto: D, + dto: UpdateQuery, ): Promise { const query = this.model.findOneAndUpdate( { @@ -359,7 +359,10 @@ export abstract class BaseRepository< return await this.executeOne(query, this.cls); } - async updateMany>(filter: TFilterQuery, dto: D) { + async updateMany>( + filter: TFilterQuery, + dto: UpdateQuery, + ) { return await this.model.updateMany(filter, { $set: dto, });