From 95b2ccea914c112afe88210f4f2bf5799986e062 Mon Sep 17 00:00:00 2001 From: Andres Pinto Date: Fri, 1 Mar 2024 01:12:36 -0400 Subject: [PATCH 1/7] feat: add items to trash by uuid --- src/modules/file/file.repository.ts | 27 +++++++++++++++++++ src/modules/file/file.usecase.ts | 8 ++++-- src/modules/folder/folder.repository.ts | 7 +++++ src/modules/folder/folder.usecase.ts | 6 ++++- .../controllers/move-items-to-trash.dto.ts | 23 +++++++++++++--- src/modules/trash/trash.controller.ts | 19 ++++++++++--- 6 files changed, 80 insertions(+), 10 deletions(-) diff --git a/src/modules/file/file.repository.ts b/src/modules/file/file.repository.ts index eec47a77d..3fbcb5a4e 100644 --- a/src/modules/file/file.repository.ts +++ b/src/modules/file/file.repository.ts @@ -416,6 +416,33 @@ export class SequelizeFileRepository implements FileRepository { ); } + async updateFilesStatusToTrashedByUuid( + user: User, + fileUuids: File['uuid'][], + ): Promise { + await this.fileModel.update( + { + // Remove this after status is the main field + deleted: true, + deletedAt: new Date(), + // + status: FileStatus.TRASHED, + updatedAt: new Date(), + }, + { + where: { + userId: user.id, + uuid: { + [Op.in]: fileUuids, + }, + status: { + [Op.not]: FileStatus.DELETED, + }, + }, + }, + ); + } + async deleteFilesByUser(user: User, files: File[]): Promise { await this.fileModel.update( { diff --git a/src/modules/file/file.usecase.ts b/src/modules/file/file.usecase.ts index 517ad4630..f1bb24c42 100644 --- a/src/modules/file/file.usecase.ts +++ b/src/modules/file/file.usecase.ts @@ -248,8 +248,12 @@ export class FileUseCases { moveFilesToTrash( user: User, fileIds: FileAttributes['fileId'][], - ): Promise { - return this.fileRepository.updateFilesStatusToTrashed(user, fileIds); + fileUuids: FileAttributes['uuid'][] = [], + ): Promise<[void, void]> { + return Promise.all([ + this.fileRepository.updateFilesStatusToTrashed(user, fileIds), + this.fileRepository.updateFilesStatusToTrashedByUuid(user, fileUuids), + ]); } async getEncryptionKeyFileFromShare( diff --git a/src/modules/folder/folder.repository.ts b/src/modules/folder/folder.repository.ts index e97608772..3867759a1 100644 --- a/src/modules/folder/folder.repository.ts +++ b/src/modules/folder/folder.repository.ts @@ -143,6 +143,13 @@ export class SequelizeFolderRepository implements FolderRepository { return folders.map((folder) => this.toDomain(folder)); } + async findUserFoldersByUuid(user: User, uuids: FolderAttributes['uuid'][]) { + const folders = await this.folderModel.findAll({ + where: { uuid: { [Op.in]: uuids }, userId: user.id }, + }); + return folders.map((folder) => this.toDomain(folder)); + } + async findAllByParentIdCursor( where: Partial, limit: number, diff --git a/src/modules/folder/folder.usecase.ts b/src/modules/folder/folder.usecase.ts index a2f7359e2..8f9182775 100644 --- a/src/modules/folder/folder.usecase.ts +++ b/src/modules/folder/folder.usecase.ts @@ -293,12 +293,16 @@ export class FolderUseCases { async moveFoldersToTrash( user: User, folderIds: FolderAttributes['id'][], + folderUuids: FolderAttributes['uuid'][] = [], ): Promise { - const [folders, driveRootFolder] = await Promise.all([ + const [foldersById, driveRootFolder, foldersByUuid] = await Promise.all([ this.getFoldersByIds(user, folderIds), this.getFolder(user.rootFolderId), + this.folderRepository.findUserFoldersByUuid(user, folderUuids), ]); + const folders = foldersById.concat(foldersByUuid); + const backups = folders.filter((f) => f.isBackup(driveRootFolder)); const driveFolders = folders.filter((f) => !f.isBackup(driveRootFolder)); diff --git a/src/modules/trash/dto/controllers/move-items-to-trash.dto.ts b/src/modules/trash/dto/controllers/move-items-to-trash.dto.ts index c4106e350..f7afc9cb6 100644 --- a/src/modules/trash/dto/controllers/move-items-to-trash.dto.ts +++ b/src/modules/trash/dto/controllers/move-items-to-trash.dto.ts @@ -1,5 +1,12 @@ import { Type } from 'class-transformer'; -import { ArrayMaxSize, IsEnum, IsNotEmpty } from 'class-validator'; +import { + ArrayMaxSize, + IsDefined, + IsEnum, + IsNotEmpty, + ValidateIf, + ValidateNested, +} from 'class-validator'; import { ApiProperty } from '@nestjs/swagger'; export enum ItemType { @@ -8,12 +15,21 @@ export enum ItemType { } export class ItemToTrash { - @IsNotEmpty() @ApiProperty({ example: '4', description: 'Id of file or folder', }) - id: string; + id?: string; + + @ApiProperty({ + example: '4', + description: 'Uuid of file or folder', + }) + uuid?: string; + + @ValidateIf((item) => (!item.id && !item.uuid) || (item.id && item.uuid)) + @IsDefined({ message: 'Provide either item id or uuid, and not both' }) + protected readonly combinedCheck: undefined; @IsEnum(ItemType) @ApiProperty({ @@ -29,6 +45,7 @@ export class MoveItemsToTrashDto { description: 'Array of items with files and folders ids', }) @ArrayMaxSize(50) + @ValidateNested() @Type(() => ItemToTrash) items: ItemToTrash[]; } diff --git a/src/modules/trash/trash.controller.ts b/src/modules/trash/trash.controller.ts index 484e9feab..2df87c103 100644 --- a/src/modules/trash/trash.controller.ts +++ b/src/modules/trash/trash.controller.ts @@ -137,19 +137,30 @@ export class TrashController { try { const fileIds: string[] = []; + const fileUuids: string[] = []; const folderIds: number[] = []; + const folderUuids: string[] = []; + for (const item of moveItemsDto.items) { if (item.type === 'file') { - fileIds.push(item.id); + if (item.id) { + fileIds.push(item.id); + } else { + fileUuids.push(item.uuid); + } } else if (item.type === 'folder') { - folderIds.push(parseInt(item.id)); + if (item.id) { + folderIds.push(parseInt(item.id)); + } else { + folderUuids.push(item.uuid); + } } else { throw new BadRequestException(`type ${item.type} invalid`); } } await Promise.all([ - this.fileUseCases.moveFilesToTrash(user, fileIds), - this.folderUseCases.moveFoldersToTrash(user, folderIds), + this.fileUseCases.moveFilesToTrash(user, fileIds, fileUuids), + this.folderUseCases.moveFoldersToTrash(user, folderIds, folderUuids), ]); this.userUseCases From d976cfa3bbe50f619ae2db49746cd43b2869d1e5 Mon Sep 17 00:00:00 2001 From: Andres Pinto Date: Mon, 11 Mar 2024 12:43:19 -0400 Subject: [PATCH 2/7] chore: add unit tests --- src/modules/file/file.repository.ts | 10 ++- src/modules/file/file.usecase.spec.ts | 84 +++++++++-------------- src/modules/folder/folder.repository.ts | 4 ++ src/modules/folder/folder.usecase.spec.ts | 74 +++++++++++++++++++- src/modules/folder/folder.usecase.ts | 4 +- 5 files changed, 122 insertions(+), 54 deletions(-) diff --git a/src/modules/file/file.repository.ts b/src/modules/file/file.repository.ts index 3fbcb5a4e..e41216754 100644 --- a/src/modules/file/file.repository.ts +++ b/src/modules/file/file.repository.ts @@ -56,6 +56,14 @@ export interface FileRepository { ): Promise; getFilesWhoseFolderIdDoesNotExist(userId: File['userId']): Promise; getFilesCountWhere(where: Partial): Promise; + updateFilesStatusToTrashed( + user: User, + fileIds: File['fileId'][], + ): Promise; + updateFilesStatusToTrashedByUuid( + user: User, + fileUuids: File['uuid'][], + ): Promise; } @Injectable() @@ -422,10 +430,8 @@ export class SequelizeFileRepository implements FileRepository { ): Promise { await this.fileModel.update( { - // Remove this after status is the main field deleted: true, deletedAt: new Date(), - // status: FileStatus.TRASHED, updatedAt: new Date(), }, diff --git a/src/modules/file/file.usecase.spec.ts b/src/modules/file/file.usecase.spec.ts index e2b71057f..336ee8f34 100644 --- a/src/modules/file/file.usecase.spec.ts +++ b/src/modules/file/file.usecase.spec.ts @@ -89,61 +89,45 @@ describe('FileUseCases', () => { }); describe('move file to trash', () => { - it.skip('calls moveFilesToTrash and return file', async () => { - const mockFile = File.build({ - id: 1, - fileId: '', - name: '', - type: '', - size: null, - bucket: '', - folderId: 4, - encryptVersion: '', - deleted: false, - deletedAt: new Date(), - userId: 1, - modificationTime: new Date(), - createdAt: new Date(), - updatedAt: new Date(), - uuid: '', - folderUuid: '', - removed: false, - removedAt: undefined, - plainName: '', - status: FileStatus.EXISTS, - }); - jest - .spyOn(fileRepository, 'updateByFieldIdAndUserId') - .mockResolvedValue(mockFile); - const result = await service.moveFilesToTrash(userMocked, [fileId]); - expect(result).toEqual(mockFile); - }); - - it.skip('throws an error if the file is not found', async () => { - jest - .spyOn(fileRepository, 'updateByFieldIdAndUserId') - .mockRejectedValue(new NotFoundException()); - expect(service.moveFilesToTrash(userMocked, [fileId])).rejects.toThrow( - NotFoundException, - ); + const mockFile = File.build({ + id: 1, + fileId: '', + name: '', + type: '', + size: null, + bucket: '', + folderId: 4, + encryptVersion: '', + deleted: false, + deletedAt: new Date(), + userId: 1, + modificationTime: new Date(), + createdAt: new Date(), + updatedAt: new Date(), + uuid: '723274e5-ca2a-4e61-bf17-d9fba3b8d430', + folderUuid: '', + removed: false, + removedAt: undefined, + plainName: '', + status: FileStatus.EXISTS, }); - }); - describe('move multiple files to trash', () => { - it.skip('calls moveFilesToTrash', async () => { + it('When you try to trash files with id and uuid, then functions are called with respective values', async () => { const fileIds = [fileId]; - jest - .spyOn(fileRepository, 'updateManyByFieldIdAndUserId') - .mockImplementation(() => { - return new Promise((resolve) => { - resolve(); - }); - }); - const result = await service.moveFilesToTrash(userMocked, fileIds); - expect(result).toEqual(undefined); - expect(fileRepository.updateManyByFieldIdAndUserId).toHaveBeenCalledTimes( + const fileUuids = [mockFile.uuid]; + jest.spyOn(fileRepository, 'updateFilesStatusToTrashed'); + jest.spyOn(fileRepository, 'updateFilesStatusToTrashedByUuid'); + await service.moveFilesToTrash(userMocked, fileIds, fileUuids); + expect(fileRepository.updateFilesStatusToTrashed).toHaveBeenCalledTimes( 1, ); + expect(fileRepository.updateFilesStatusToTrashed).toHaveBeenCalledWith( + userMocked, + fileIds, + ); + expect( + fileRepository.updateFilesStatusToTrashedByUuid, + ).toHaveBeenCalledWith(userMocked, fileUuids); }); }); diff --git a/src/modules/folder/folder.repository.ts b/src/modules/folder/folder.repository.ts index 3867759a1..d4cb16a02 100644 --- a/src/modules/folder/folder.repository.ts +++ b/src/modules/folder/folder.repository.ts @@ -65,6 +65,10 @@ export interface FolderRepository { deleteById(folderId: FolderAttributes['id']): Promise; clearOrphansFolders(userId: FolderAttributes['userId']): Promise; calculateFolderSize(folderUuid: string): Promise; + findUserFoldersByUuid( + user: User, + uuids: FolderAttributes['uuid'][], + ): Promise; } @Injectable() diff --git a/src/modules/folder/folder.usecase.spec.ts b/src/modules/folder/folder.usecase.spec.ts index 9f0416adb..6a1869ec8 100644 --- a/src/modules/folder/folder.usecase.spec.ts +++ b/src/modules/folder/folder.usecase.spec.ts @@ -15,10 +15,12 @@ import { BridgeModule } from '../../externals/bridge/bridge.module'; import { CryptoModule } from '../../externals/crypto/crypto.module'; import { CryptoService } from '../../externals/crypto/crypto.service'; import { User } from '../user/user.domain'; -import { newFolder } from '../../../test/fixtures'; +import { newFolder, newUser } from '../../../test/fixtures'; import { CalculateFolderSizeTimeoutException } from './exception/calculate-folder-size-timeout.exception'; const folderId = 4; +const user = newUser(); + describe('FolderUseCases', () => { let service: FolderUseCases; let folderRepository: FolderRepository; @@ -107,6 +109,76 @@ describe('FolderUseCases', () => { }); }); + describe('move multiple folders to trash', () => { + const rootFolderBucket = 'bucketRoot'; + const mockFolder = Folder.build({ + id: 1, + parentId: null, + parentUuid: null, + name: 'name', + bucket: rootFolderBucket, + userId: 1, + encryptVersion: '03-aes', + deleted: true, + deletedAt: new Date(), + createdAt: new Date(), + updatedAt: new Date(), + uuid: '', + plainName: '', + removed: false, + removedAt: null, + }); + + it('When uuid and id are passed and there is a backup and drive folder, then should backups and drive folders should be updated', async () => { + const mockBackupFolder = Folder.build({ + id: 1, + parentId: null, + parentUuid: null, + name: 'name', + bucket: 'bucketIdforBackup', + userId: 1, + encryptVersion: '03-aes', + deleted: true, + deletedAt: new Date(), + createdAt: new Date(), + updatedAt: new Date(), + uuid: '', + plainName: '', + removed: false, + removedAt: null, + }); + + jest + .spyOn(service, 'getFoldersByIds') + .mockResolvedValue([mockBackupFolder]); + jest + .spyOn(folderRepository, 'findUserFoldersByUuid') + .mockResolvedValue([mockFolder]); + jest.spyOn(service, 'getFolder').mockResolvedValue({ + bucket: rootFolderBucket, + } as Folder); + jest.spyOn(folderRepository, 'updateManyByFolderId'); + + await service.moveFoldersToTrash( + user, + [mockBackupFolder.id], + [mockFolder.uuid], + ); + expect(folderRepository.updateManyByFolderId).toHaveBeenCalledTimes(2); + }); + + it('When only ids are passed, then only folders by id should be searched', async () => { + jest.spyOn(service, 'getFoldersByIds').mockResolvedValue([mockFolder]); + jest.spyOn(service, 'getFolder').mockResolvedValue({ + bucket: rootFolderBucket, + } as Folder); + jest.spyOn(folderRepository, 'findUserFoldersByUuid'); + + await service.moveFoldersToTrash(user, [mockFolder.id]); + expect(folderRepository.findUserFoldersByUuid).not.toHaveBeenCalled(); + }); + }); + describe('get folder use case', () => { it('calls getFolder and return folder', async () => { const mockFolder = Folder.build({ diff --git a/src/modules/folder/folder.usecase.ts b/src/modules/folder/folder.usecase.ts index 8f9182775..6e1508d31 100644 --- a/src/modules/folder/folder.usecase.ts +++ b/src/modules/folder/folder.usecase.ts @@ -298,7 +298,9 @@ export class FolderUseCases { const [foldersById, driveRootFolder, foldersByUuid] = await Promise.all([ this.getFoldersByIds(user, folderIds), this.getFolder(user.rootFolderId), - this.folderRepository.findUserFoldersByUuid(user, folderUuids), + folderUuids.length > 0 + ? this.folderRepository.findUserFoldersByUuid(user, folderUuids) + : Promise.resolve([]), ]); const folders = foldersById.concat(foldersByUuid); From a8bf07830ee49de840bc23c761faa6f2e2a8522d Mon Sep 17 00:00:00 2001 From: Andres Pinto Date: Mon, 11 Mar 2024 13:47:18 -0400 Subject: [PATCH 3/7] chore: added tests to dto --- .../move-items-to-trash.dto.spec.ts | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 src/modules/trash/dto/controllers/move-items-to-trash.dto.spec.ts diff --git a/src/modules/trash/dto/controllers/move-items-to-trash.dto.spec.ts b/src/modules/trash/dto/controllers/move-items-to-trash.dto.spec.ts new file mode 100644 index 000000000..a71083816 --- /dev/null +++ b/src/modules/trash/dto/controllers/move-items-to-trash.dto.spec.ts @@ -0,0 +1,65 @@ +import { validate } from 'class-validator'; +import { plainToInstance } from 'class-transformer'; +import { + ItemToTrash, + ItemType, + MoveItemsToTrashDto, +} from './move-items-to-trash.dto'; + +describe('MoveItemsToTrashDto', () => { + it('When valid data is passed, then no errors should be returned', async () => { + const dto = plainToInstance(MoveItemsToTrashDto, { + items: [ + { id: '1', type: ItemType.FILE }, + { uuid: 'uuid-2', type: ItemType.FOLDER }, + ], + }); + + const errors = await validate(dto); + expect(errors.length).toBe(0); + }); + + it('When items array exceeds max size, then should fail', async () => { + const items = Array.from({ length: 51 }, (_, i) => ({ + id: `${i + 1}`, + type: ItemType.FILE, + })); + const dto = plainToInstance(MoveItemsToTrashDto, { items }); + + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + expect(errors[0].constraints).toBeDefined(); + }); + + describe('ItemToTrash', () => { + it('When both id and uuid are provided in one item, then should fail', async () => { + const item = plainToInstance(ItemToTrash, { + id: '1', + uuid: 'uuid-1', + type: ItemType.FILE, + }); + const errors = await validate(item); + expect(errors.length).toBeGreaterThan(0); + }); + + it('When neither id nor uuid are provided in one item, then should fail', async () => { + const item = plainToInstance(ItemToTrash, { type: ItemType.FILE }); + const errors = await validate(item); + expect(errors.length).toBeGreaterThan(0); + }); + + it('when either id or uuid are provided, then should validate successfuly ', async () => { + const onlyIdErrors = await validate( + plainToInstance(ItemToTrash, { id: '1', type: ItemType.FILE }), + ); + const onlyUuidErrors = await validate( + plainToInstance(ItemToTrash, { + uuid: 'uuid-1', + type: ItemType.FILE, + }), + ); + expect(onlyIdErrors.length).toBe(0); + expect(onlyUuidErrors.length).toBe(0); + }); + }); +}); From 1f479141b7814c37eb9f27d3523d30f4fe0a4947 Mon Sep 17 00:00:00 2001 From: Andres Pinto Date: Mon, 11 Mar 2024 15:24:22 -0400 Subject: [PATCH 4/7] chore: added test for trash controller --- src/modules/file/file.usecase.spec.ts | 1 - .../move-items-to-trash.dto.spec.ts | 6 +- .../controllers/move-items-to-trash.dto.ts | 2 +- src/modules/trash/trash.controller.spec.ts | 113 ++++++++++++++++++ src/modules/trash/trash.controller.ts | 45 ++++--- 5 files changed, 144 insertions(+), 23 deletions(-) create mode 100644 src/modules/trash/trash.controller.spec.ts diff --git a/src/modules/file/file.usecase.spec.ts b/src/modules/file/file.usecase.spec.ts index 336ee8f34..c1cb9b1be 100644 --- a/src/modules/file/file.usecase.spec.ts +++ b/src/modules/file/file.usecase.spec.ts @@ -4,7 +4,6 @@ import { FileUseCases } from './file.usecase'; import { SequelizeFileRepository, FileRepository } from './file.repository'; import { ForbiddenException, - NotFoundException, UnprocessableEntityException, } from '@nestjs/common'; import { File, FileAttributes, FileStatus } from './file.domain'; diff --git a/src/modules/trash/dto/controllers/move-items-to-trash.dto.spec.ts b/src/modules/trash/dto/controllers/move-items-to-trash.dto.spec.ts index a71083816..9ddb26ac5 100644 --- a/src/modules/trash/dto/controllers/move-items-to-trash.dto.spec.ts +++ b/src/modules/trash/dto/controllers/move-items-to-trash.dto.spec.ts @@ -11,7 +11,7 @@ describe('MoveItemsToTrashDto', () => { const dto = plainToInstance(MoveItemsToTrashDto, { items: [ { id: '1', type: ItemType.FILE }, - { uuid: 'uuid-2', type: ItemType.FOLDER }, + { uuid: '5bf9dca1-fd68-4864-9a16-ef36b77d063b', type: ItemType.FOLDER }, ], }); @@ -35,7 +35,7 @@ describe('MoveItemsToTrashDto', () => { it('When both id and uuid are provided in one item, then should fail', async () => { const item = plainToInstance(ItemToTrash, { id: '1', - uuid: 'uuid-1', + uuid: '5bf9dca1-fd68-4864-9a16-ef36b77d063b', type: ItemType.FILE, }); const errors = await validate(item); @@ -54,7 +54,7 @@ describe('MoveItemsToTrashDto', () => { ); const onlyUuidErrors = await validate( plainToInstance(ItemToTrash, { - uuid: 'uuid-1', + uuid: '5bf9dca1-fd68-4864-9a16-ef36b77d063b', type: ItemType.FILE, }), ); diff --git a/src/modules/trash/dto/controllers/move-items-to-trash.dto.ts b/src/modules/trash/dto/controllers/move-items-to-trash.dto.ts index f7afc9cb6..ea3624da2 100644 --- a/src/modules/trash/dto/controllers/move-items-to-trash.dto.ts +++ b/src/modules/trash/dto/controllers/move-items-to-trash.dto.ts @@ -29,7 +29,7 @@ export class ItemToTrash { @ValidateIf((item) => (!item.id && !item.uuid) || (item.id && item.uuid)) @IsDefined({ message: 'Provide either item id or uuid, and not both' }) - protected readonly combinedCheck: undefined; + readonly AreUuidAndIdDefined?: boolean; @IsEnum(ItemType) @ApiProperty({ diff --git a/src/modules/trash/trash.controller.spec.ts b/src/modules/trash/trash.controller.spec.ts new file mode 100644 index 000000000..ede9834a9 --- /dev/null +++ b/src/modules/trash/trash.controller.spec.ts @@ -0,0 +1,113 @@ +import { createMock } from '@golevelup/ts-jest'; +import { TrashController } from './trash.controller'; +import { FileUseCases } from '../file/file.usecase'; +import { FolderUseCases } from '../folder/folder.usecase'; +import { UserUseCases } from '../user/user.usecase'; +import { TrashUseCases } from './trash.usecase'; +import { NotificationService } from '../../externals/notifications/notification.service'; +import { newUser } from '../../../test/fixtures'; +import { BadRequestException } from '@nestjs/common'; +import { ItemType } from './dto/controllers/move-items-to-trash.dto'; + +const user = newUser(); + +describe('TrashController', () => { + let controller: TrashController; + let folderUseCases: FolderUseCases; + let fileUseCases: FileUseCases; + let userUseCases: UserUseCases; + let trashUseCases: TrashUseCases; + let notificationService: NotificationService; + + beforeEach(async () => { + folderUseCases = createMock(); + fileUseCases = createMock(); + userUseCases = createMock(); + trashUseCases = createMock(); + notificationService = createMock(); + controller = new TrashController( + fileUseCases, + folderUseCases, + userUseCases, + notificationService, + trashUseCases, + ); + }); + + it('should be defined', () => { + expect(controller).toBeDefined(); + }); + + it('When item type is invalid, then it should throw', async () => { + await expect( + controller.moveItemsToTrash( + { + items: [ + { + uuid: '5bf9dca1-fd68-4864-9a16-ef36b77d063b', + type: 'test' as ItemType, + }, + ], + }, + user, + 'anyid', + ), + ).rejects.toThrow(BadRequestException); + }); + + it('When array is empty, then it should not call anything', async () => { + const body = { items: [] }; + jest.spyOn(fileUseCases, 'moveFilesToTrash'); + + await controller.moveItemsToTrash(body, user, ''); + expect(fileUseCases.moveFilesToTrash).not.toHaveBeenCalled(); + }); + + it('When items are passed, then items should be deleted with their respective uuid or id', async () => { + const fileItems = [ + { + uuid: '5bf9dca1-fd68-4864-9a16-ef36b77d063b', + type: ItemType.FILE, + }, + { + id: '2', + type: ItemType.FILE, + }, + ]; + const folderItems = [ + { + id: '1', + type: ItemType.FOLDER, + }, + { + uuid: '9af7dca1-fd68-4864-9b60-ef36b77d0903', + type: ItemType.FOLDER, + }, + ]; + + jest.spyOn(fileUseCases, 'moveFilesToTrash'); + jest.spyOn(folderUseCases, 'moveFoldersToTrash'); + jest + .spyOn(userUseCases, 'getWorkspaceMembersByBrigeUser') + .mockResolvedValue([]); + + await controller.moveItemsToTrash( + { + items: [...fileItems, ...folderItems], + }, + user, + '', + ); + + expect(fileUseCases.moveFilesToTrash).toHaveBeenCalledWith( + user, + [fileItems[1].id], + [fileItems[0].uuid], + ); + expect(folderUseCases.moveFoldersToTrash).toHaveBeenCalledWith( + user, + [parseInt(folderItems[0].id)], + [folderItems[1].uuid], + ); + }); +}); diff --git a/src/modules/trash/trash.controller.ts b/src/modules/trash/trash.controller.ts index 2df87c103..3e7bd3d49 100644 --- a/src/modules/trash/trash.controller.ts +++ b/src/modules/trash/trash.controller.ts @@ -21,7 +21,10 @@ import { ApiOperation, ApiTags, } from '@nestjs/swagger'; -import { MoveItemsToTrashDto } from './dto/controllers/move-items-to-trash.dto'; +import { + ItemType, + MoveItemsToTrashDto, +} from './dto/controllers/move-items-to-trash.dto'; import { User as UserDecorator } from '../auth/decorators/user.decorator'; import { Client } from '../auth/decorators/client.decorator'; import { FileUseCases } from '../file/file.usecase'; @@ -124,7 +127,6 @@ export class TrashController { @Body() moveItemsDto: MoveItemsToTrashDto, @UserDecorator() user: User, @Client() clientId: string, - @Res({ passthrough: true }) res: Response, ) { if (moveItemsDto.items.length === 0) { logger('error', { @@ -142,20 +144,23 @@ export class TrashController { const folderUuids: string[] = []; for (const item of moveItemsDto.items) { - if (item.type === 'file') { - if (item.id) { - fileIds.push(item.id); - } else { - fileUuids.push(item.uuid); - } - } else if (item.type === 'folder') { - if (item.id) { - folderIds.push(parseInt(item.id)); - } else { - folderUuids.push(item.uuid); - } - } else { - throw new BadRequestException(`type ${item.type} invalid`); + switch (item.type) { + case ItemType.FILE: + if (item.id) { + fileIds.push(item.id); + } else { + fileUuids.push(item.uuid); + } + break; + case ItemType.FOLDER: + if (item.id) { + folderIds.push(parseInt(item.id, 10)); + } else { + folderUuids.push(item.uuid); + } + break; + default: + throw new BadRequestException(`type ${item.type} invalid`); } } await Promise.all([ @@ -190,9 +195,13 @@ export class TrashController { user: { email, uuid }, })}, STACK: ${(err as Error).stack}`, ); - res.status(HttpStatus.INTERNAL_SERVER_ERROR); + if (err instanceof BadRequestException) { + throw err; + } - return { error: 'Internal Server Error' }; + throw new InternalServerErrorException({ + error: 'Internal Server Error', + }); } } From 5b984796f51a067db9021151245830afe2e54d18 Mon Sep 17 00:00:00 2001 From: Andres Pinto Date: Fri, 22 Mar 2024 07:37:37 -0400 Subject: [PATCH 5/7] fix: prevent trashed files from being updated to trashed and non existent folders from being fetch --- src/modules/file/file.repository.ts | 2 ++ src/modules/folder/folder.repository.ts | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/modules/file/file.repository.ts b/src/modules/file/file.repository.ts index e41216754..3ae333da6 100644 --- a/src/modules/file/file.repository.ts +++ b/src/modules/file/file.repository.ts @@ -417,6 +417,7 @@ export class SequelizeFileRepository implements FileRepository { [Op.in]: fileIds, }, status: { + [Op.eq]: FileStatus.EXISTS, [Op.not]: FileStatus.DELETED, }, }, @@ -442,6 +443,7 @@ export class SequelizeFileRepository implements FileRepository { [Op.in]: fileUuids, }, status: { + [Op.eq]: FileStatus.EXISTS, [Op.not]: FileStatus.DELETED, }, }, diff --git a/src/modules/folder/folder.repository.ts b/src/modules/folder/folder.repository.ts index d4cb16a02..7068ba465 100644 --- a/src/modules/folder/folder.repository.ts +++ b/src/modules/folder/folder.repository.ts @@ -149,7 +149,12 @@ export class SequelizeFolderRepository implements FolderRepository { async findUserFoldersByUuid(user: User, uuids: FolderAttributes['uuid'][]) { const folders = await this.folderModel.findAll({ - where: { uuid: { [Op.in]: uuids }, userId: user.id }, + where: { + uuid: { [Op.in]: uuids }, + userId: user.id, + deleted: false, + removed: false, + }, }); return folders.map((folder) => this.toDomain(folder)); } From f45285b750436454410cab77caa1fc8e1c8ad766 Mon Sep 17 00:00:00 2001 From: Andres Pinto Date: Fri, 22 Mar 2024 09:43:19 -0400 Subject: [PATCH 6/7] chore: added some extra expects to test cases --- src/modules/file/file.repository.ts | 1 - src/modules/folder/folder.usecase.spec.ts | 25 +++++++++++++++++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/modules/file/file.repository.ts b/src/modules/file/file.repository.ts index 3ae333da6..42521dc31 100644 --- a/src/modules/file/file.repository.ts +++ b/src/modules/file/file.repository.ts @@ -418,7 +418,6 @@ export class SequelizeFileRepository implements FileRepository { }, status: { [Op.eq]: FileStatus.EXISTS, - [Op.not]: FileStatus.DELETED, }, }, }, diff --git a/src/modules/folder/folder.usecase.spec.ts b/src/modules/folder/folder.usecase.spec.ts index 6a1869ec8..34243fb2c 100644 --- a/src/modules/folder/folder.usecase.spec.ts +++ b/src/modules/folder/folder.usecase.spec.ts @@ -123,13 +123,13 @@ describe('FolderUseCases', () => { deletedAt: new Date(), createdAt: new Date(), updatedAt: new Date(), - uuid: '', + uuid: '2545feaf-4d6b-40d8-9bf8-550285268bd3', plainName: '', removed: false, removedAt: null, }); - it('When uuid and id are passed and there is a backup and drive folder, then should backups and drive folders should be updated', async () => { + it('When uuid and id are passed and there is a backup and drive folder, then backups and drive folders should be updated', async () => { const mockBackupFolder = Folder.build({ id: 1, parentId: null, @@ -164,7 +164,24 @@ describe('FolderUseCases', () => { [mockBackupFolder.id], [mockFolder.uuid], ); + expect(folderRepository.updateManyByFolderId).toHaveBeenCalledTimes(2); + expect(folderRepository.updateManyByFolderId).toHaveBeenCalledWith( + [mockFolder.id], + { + deleted: true, + deletedAt: expect.any(Date), + }, + ); + expect(folderRepository.updateManyByFolderId).toHaveBeenCalledWith( + [mockBackupFolder.id], + { + deleted: true, + deletedAt: expect.any(Date), + removed: true, + removedAt: expect.any(Date), + }, + ); }); it('When only ids are passed, then only folders by id should be searched', async () => { @@ -173,9 +190,13 @@ describe('FolderUseCases', () => { bucket: rootFolderBucket, } as Folder); jest.spyOn(folderRepository, 'findUserFoldersByUuid'); + jest.spyOn(service, 'getFoldersByIds'); await service.moveFoldersToTrash(user, [mockFolder.id]); expect(folderRepository.findUserFoldersByUuid).not.toHaveBeenCalled(); + expect(service.getFoldersByIds).toHaveBeenCalledWith(user, [ + mockFolder.id, + ]); }); }); From e291d451e304ca6aaf76f34c19c3909c4785e04d Mon Sep 17 00:00:00 2001 From: Andres Pinto Date: Fri, 22 Mar 2024 09:50:23 -0400 Subject: [PATCH 7/7] mend --- src/modules/file/file.repository.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/modules/file/file.repository.ts b/src/modules/file/file.repository.ts index 42521dc31..c9a7b288b 100644 --- a/src/modules/file/file.repository.ts +++ b/src/modules/file/file.repository.ts @@ -443,7 +443,6 @@ export class SequelizeFileRepository implements FileRepository { }, status: { [Op.eq]: FileStatus.EXISTS, - [Op.not]: FileStatus.DELETED, }, }, },