Skip to content

Commit

Permalink
Merge pull request #293 from internxt/fix/stop-sharing-items-when-tra…
Browse files Browse the repository at this point in the history
…shed

[PB-1920]: fix/stop sharing items when they are trashed
  • Loading branch information
sg-gs authored Apr 5, 2024
2 parents 223cc83 + 7c14004 commit 20ec460
Show file tree
Hide file tree
Showing 11 changed files with 156 additions and 25 deletions.
20 changes: 20 additions & 0 deletions src/modules/file/file.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ export interface FileRepository {
user: User,
fileUuids: File['uuid'][],
): Promise<void>;
findByFileIds(
userId: User['id'],
fileIds: FileAttributes['fileId'][],
): Promise<File[]>;
}

@Injectable()
Expand All @@ -88,6 +92,22 @@ export class SequelizeFileRepository implements FileRepository {
});
}

async findByFileIds(
userId: User['id'],
fileIds: FileAttributes['fileId'][],
): Promise<File[]> {
const files = await this.fileModel.findAll({
where: {
userId: userId,
fileId: {
[Op.in]: fileIds,
},
},
});

return files.map(this.toDomain.bind(this));
}

async findById(
fileUuid: string,
where: FindOptions<FileAttributes> = {},
Expand Down
40 changes: 23 additions & 17 deletions src/modules/file/file.usecase.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
} from '@nestjs/common';
import { File, FileAttributes, FileStatus } from './file.domain';
import { User } from '../user/user.domain';
import { ShareUseCases } from '../share/share.usecase';
import { BridgeModule } from '../../externals/bridge/bridge.module';
import { BridgeService } from '../../externals/bridge/bridge.service';
import { CryptoService } from '../../externals/crypto/crypto.service';
Expand All @@ -19,6 +18,8 @@ import {
import { newFile, newFolder } from '../../../test/fixtures';
import { FolderUseCases } from '../folder/folder.usecase';
import { v4 } from 'uuid';
import { SharingService } from '../sharing/sharing.service';
import { SharingItemType } from '../sharing/sharing.domain';

const fileId = '6295c99a241bb000083f1c6a';
const userId = 1;
Expand All @@ -28,7 +29,7 @@ describe('FileUseCases', () => {
let folderUseCases: FolderUseCases;
let fileRepository: FileRepository;
let folderRepository: FolderRepository;
let shareUseCases: ShareUseCases;
let sharingService: SharingService;
let bridgeService: BridgeService;
let cryptoService: CryptoService;

Expand Down Expand Up @@ -65,7 +66,7 @@ describe('FileUseCases', () => {
beforeEach(async () => {
const module: TestingModule = await Test.createTestingModule({
imports: [BridgeModule],
providers: [FileUseCases, FolderUseCases],
providers: [FileUseCases, FolderUseCases, SharingService],
})
.useMocker(() => createMock())
.compile();
Expand All @@ -74,9 +75,9 @@ describe('FileUseCases', () => {
fileRepository = module.get<FileRepository>(SequelizeFileRepository);
folderRepository = module.get<FolderRepository>(SequelizeFolderRepository);
folderUseCases = module.get<FolderUseCases>(FolderUseCases);
shareUseCases = module.get<ShareUseCases>(ShareUseCases);
bridgeService = module.get<BridgeService>(BridgeService);
cryptoService = module.get<CryptoService>(CryptoService);
sharingService = module.get<SharingService>(SharingService);
});

afterEach(() => {
Expand Down Expand Up @@ -128,6 +129,23 @@ describe('FileUseCases', () => {
fileRepository.updateFilesStatusToTrashedByUuid,
).toHaveBeenCalledWith(userMocked, fileUuids);
});

it('When you try to trash files, then it stops sharing those files', async () => {
const files = [newFile(), newFile(), newFile()];
const fileUuids = ['656a3abb-36ab-47ee-8303-6e4198f2a32a'];
const fileIds = [fileId];

jest.spyOn(sharingService, 'bulkRemoveSharings');
jest.spyOn(fileRepository, 'findByFileIds').mockResolvedValueOnce(files);

await service.moveFilesToTrash(userMocked, fileIds, fileUuids);

expect(sharingService.bulkRemoveSharings).toHaveBeenCalledWith(
userMocked,
[...fileUuids, ...files.map((file) => file.uuid)],
SharingItemType.File,
);
});
});

describe('get folder by folderId and User Id', () => {
Expand Down Expand Up @@ -240,18 +258,13 @@ describe('FileUseCases', () => {
.spyOn(fileRepository, 'deleteByFileId')
.mockImplementationOnce(() => Promise.resolve());

jest
.spyOn(shareUseCases, 'deleteFileShare')
.mockImplementationOnce(() => Promise.resolve());

jest
.spyOn(bridgeService, 'deleteFile')
.mockImplementationOnce(() => Promise.resolve());

await service.deleteFilePermanently(file, userMock);

expect(fileRepository.deleteByFileId).toHaveBeenCalledWith(fileId);
expect(shareUseCases.deleteFileShare).toHaveBeenCalledTimes(1);
});

it.skip('should fail when the folder trying to delete has not been trashed', async () => {
Expand Down Expand Up @@ -295,14 +308,12 @@ describe('FileUseCases', () => {
deleted: true,
} as File;

jest.spyOn(shareUseCases, 'deleteFileShare');
jest.spyOn(bridgeService, 'deleteFile');
jest.spyOn(fileRepository, 'deleteByFileId');

expect(service.deleteFilePermanently(file, userMock)).rejects.toThrow(
new ForbiddenException(`You are not owner of this share`),
);
expect(shareUseCases.deleteFileShare).not.toHaveBeenCalled();
expect(bridgeService.deleteFile).not.toHaveBeenCalled();
expect(fileRepository.deleteByFileId).not.toHaveBeenCalled();
});
Expand Down Expand Up @@ -334,9 +345,6 @@ describe('FileUseCases', () => {

const errorReason = new Error('reason');

jest
.spyOn(shareUseCases, 'deleteFileShare')
.mockImplementationOnce(() => Promise.reject(errorReason));
jest
.spyOn(fileRepository, 'deleteByFileId')
.mockImplementationOnce(() => Promise.resolve());
Expand Down Expand Up @@ -384,9 +392,7 @@ describe('FileUseCases', () => {
jest
.spyOn(fileRepository, 'deleteByFileId')
.mockImplementationOnce(() => Promise.resolve());
jest
.spyOn(shareUseCases, 'deleteFileShare')
.mockImplementationOnce(() => Promise.resolve());

jest
.spyOn(bridgeService, 'deleteFile')
.mockImplementationOnce(() => Promise.reject(errorReason));
Expand Down
21 changes: 15 additions & 6 deletions src/modules/file/file.usecase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { CryptoService } from '../../externals/crypto/crypto.service';
import { BridgeService } from '../../externals/bridge/bridge.service';
import { FolderAttributes } from '../folder/folder.attributes';
import { Share } from '../share/share.domain';
import { ShareUseCases } from '../share/share.usecase';
import { User } from '../user/user.domain';
import { UserAttributes } from '../user/user.attributes';
import {
Expand All @@ -27,17 +26,19 @@ import { SequelizeFileRepository } from './file.repository';
import { FolderUseCases } from '../folder/folder.usecase';
import { ReplaceFileDto } from './dto/replace-file.dto';
import { FileDto } from './dto/file.dto';
import { SharingService } from '../sharing/sharing.service';
import { SharingItemType } from '../sharing/sharing.domain';

type SortParams = Array<[SortableFileAttributes, 'ASC' | 'DESC']>;

@Injectable()
export class FileUseCases {
constructor(
private fileRepository: SequelizeFileRepository,
@Inject(forwardRef(() => ShareUseCases))
private shareUseCases: ShareUseCases,
@Inject(forwardRef(() => FolderUseCases))
private folderUsecases: FolderUseCases,
@Inject(forwardRef(() => SharingService))
private sharingUsecases: SharingService,
private network: BridgeService,
private cryptoService: CryptoService,
) {}
Expand Down Expand Up @@ -245,14 +246,22 @@ export class FileUseCases {
return files.map((file) => file.toJSON());
}

moveFilesToTrash(
async moveFilesToTrash(
user: User,
fileIds: FileAttributes['fileId'][],
fileUuids: FileAttributes['uuid'][] = [],
): Promise<[void, void]> {
return Promise.all([
): Promise<void> {
const files = await this.fileRepository.findByFileIds(user.id, fileIds);
const allFileUuids = [...fileUuids, ...files.map((file) => file.uuid)];

await Promise.all([
this.fileRepository.updateFilesStatusToTrashed(user, fileIds),
this.fileRepository.updateFilesStatusToTrashedByUuid(user, fileUuids),
this.sharingUsecases.bulkRemoveSharings(
user,
allFileUuids,
SharingItemType.File,
),
]);
}

Expand Down
2 changes: 2 additions & 0 deletions src/modules/folder/folder.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ import { CryptoService } from '../../externals/crypto/crypto.service';
import { FolderController } from './folder.controller';
import { UserModel } from '../user/user.model';
import { UserModule } from '../user/user.module';
import { SharingModule } from '../sharing/sharing.module';

@Module({
imports: [
SequelizeModule.forFeature([FolderModel, UserModel]),
forwardRef(() => FileModule),
forwardRef(() => UserModule),
CryptoModule,
forwardRef(() => SharingModule),
],
controllers: [FolderController],
providers: [SequelizeFolderRepository, CryptoService, FolderUseCases],
Expand Down
10 changes: 9 additions & 1 deletion src/modules/folder/folder.usecase.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { CryptoService } from '../../externals/crypto/crypto.service';
import { User } from '../user/user.domain';
import { newFolder, newUser } from '../../../test/fixtures';
import { CalculateFolderSizeTimeoutException } from './exception/calculate-folder-size-timeout.exception';
import { SharingService } from '../sharing/sharing.service';

const folderId = 4;
const user = newUser();
Expand All @@ -25,6 +26,7 @@ describe('FolderUseCases', () => {
let service: FolderUseCases;
let folderRepository: FolderRepository;
let cryptoService: CryptoService;
let sharingService: SharingService;

const userMocked = User.build({
id: 1,
Expand Down Expand Up @@ -67,6 +69,7 @@ describe('FolderUseCases', () => {
service = module.get<FolderUseCases>(FolderUseCases);
folderRepository = module.get<FolderRepository>(SequelizeFolderRepository);
cryptoService = module.get<CryptoService>(CryptoService);
sharingService = module.get<SharingService>(SharingService);
});

it('should be defined', () => {
Expand Down Expand Up @@ -142,7 +145,7 @@ describe('FolderUseCases', () => {
deletedAt: new Date(),
createdAt: new Date(),
updatedAt: new Date(),
uuid: '',
uuid: '656a3abb-36ab-47ee-8303-6e4198f2a32a',
plainName: '',
removed: false,
removedAt: null,
Expand Down Expand Up @@ -182,6 +185,11 @@ describe('FolderUseCases', () => {
removedAt: expect.any(Date),
},
);
expect(sharingService.bulkRemoveSharings).toHaveBeenCalledWith(
user,
[mockBackupFolder.uuid, mockFolder.uuid],
'folder',
);
});

it('When only ids are passed, then only folders by id should be searched', async () => {
Expand Down
11 changes: 11 additions & 0 deletions src/modules/folder/folder.usecase.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import {
ConflictException,
ForbiddenException,
Inject,
Injectable,
Logger,
NotFoundException,
UnprocessableEntityException,
forwardRef,
} from '@nestjs/common';
import { CryptoService } from '../../externals/crypto/crypto.service';
import { User } from '../user/user.domain';
Expand All @@ -17,6 +19,8 @@ import {
} from './folder.domain';
import { FolderAttributes } from './folder.attributes';
import { SequelizeFolderRepository } from './folder.repository';
import { SharingService } from '../sharing/sharing.service';
import { SharingItemType } from '../sharing/sharing.domain';

const invalidName = /[\\/]|^\s*$/;

Expand All @@ -27,6 +31,8 @@ export class FolderUseCases {
constructor(
private folderRepository: SequelizeFolderRepository,
private userRepository: SequelizeUserRepository,
@Inject(forwardRef(() => SharingService))
private sharingUsecases: SharingService,
private readonly cryptoService: CryptoService,
) {}

Expand Down Expand Up @@ -329,6 +335,11 @@ export class FolderUseCases {
},
)
: Promise.resolve(),
this.sharingUsecases.bulkRemoveSharings(
user,
folders.map((folder) => folder.uuid),
SharingItemType.Folder,
),
]);
}

Expand Down
5 changes: 5 additions & 0 deletions src/modules/sharing/sharing.domain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ export enum SharingType {
Private = 'private',
}

export enum SharingItemType {
File = 'file',
Folder = 'folder',
}

export interface SharingAttributes {
id: string;
itemId: ItemId;
Expand Down
30 changes: 30 additions & 0 deletions src/modules/sharing/sharing.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,36 @@ export class SequelizeSharingRepository implements SharingRepository {
});
}

async bulkDeleteInvites(
itemIds: SharingInvite['itemId'][],
type: SharingInvite['itemType'],
): Promise<void> {
await this.sharingInvites.destroy({
where: {
itemId: {
[Op.in]: itemIds,
},
itemType: type,
},
});
}

async bulkDeleteSharings(
userUuid: User['uuid'],
itemIds: SharingInvite['itemId'][],
type: SharingInvite['itemType'],
): Promise<void> {
await this.sharings.destroy({
where: {
itemId: {
[Op.in]: itemIds,
},
itemType: type,
ownerId: userUuid,
},
});
}

async deleteInvite(invite: SharingInvite): Promise<void> {
await this.sharingInvites.destroy({
where: {
Expand Down
20 changes: 20 additions & 0 deletions src/modules/sharing/sharing.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,26 @@ describe('Sharing Use Cases', () => {
});
});

describe('Bulk remove sharings', () => {
it('When function is called, then invitations and sharings are removed ', async () => {
const owner = newUser();
const itemIds = ['uuid1', 'uuid2'];
const itemType = 'file';

await sharingService.bulkRemoveSharings(owner, itemIds, itemType);

expect(sharingRepository.bulkDeleteInvites).toHaveBeenCalledWith(
itemIds,
itemType,
);
expect(sharingRepository.bulkDeleteSharings).toHaveBeenCalledWith(
owner.uuid,
itemIds,
itemType,
);
});
});

describe('Access to public shared item info', () => {
const owner = newUser();
const otherUser = publicUser();
Expand Down
Loading

0 comments on commit 20ec460

Please sign in to comment.