diff --git a/apps/server/src/modules/deletion/api/controller/api-test/deletion-request-create.api.spec.ts b/apps/server/src/modules/deletion/api/controller/api-test/deletion-request-create.api.spec.ts index 894baab6f3e..06de3203e64 100644 --- a/apps/server/src/modules/deletion/api/controller/api-test/deletion-request-create.api.spec.ts +++ b/apps/server/src/modules/deletion/api/controller/api-test/deletion-request-create.api.spec.ts @@ -117,34 +117,30 @@ describe(`deletionRequest create (api)`, () => { expect(result.requestId).toBeDefined(); }); - describe('when the "delete in minutes" param has not been provided', () => { - it( - 'should set the "deletion planned at" date to the date after the default "delete in minutes" value ' + - '(43200 minutes which is 30 days), with some operational time tolerance', - async () => { - const { deletionRequestToCreate, defaultdeleteAfterMinutes, operationalTimeToleranceInSeconds } = setup(); - - const response = await testApiClient.post('', deletionRequestToCreate); - - const result = response.body as DeletionRequestResponse; - const createdDeletionRequestId = result.requestId; - - const createdItem = await em.findOneOrFail(DeletionRequestEntity, createdDeletionRequestId); - - const isDeletionPlannedAtDateCorrect = isDeletionPlannedWithinAcceptableRange( - createdItem.createdAt, - createdItem.deleteAfter, - defaultdeleteAfterMinutes, - operationalTimeToleranceInSeconds - ); - - expect(isDeletionPlannedAtDateCorrect).toEqual(true); - } - ); + describe('when the "delete after minutes" param has not been provided', () => { + it('should set the "deleteAfter" date to the date after the default DELETION_DELETE_AFTER_MINUTES ', async () => { + const { deletionRequestToCreate, defaultdeleteAfterMinutes, operationalTimeToleranceInSeconds } = setup(); + + const response = await testApiClient.post('', deletionRequestToCreate); + + const result = response.body as DeletionRequestResponse; + const createdDeletionRequestId = result.requestId; + + const createdItem = await em.findOneOrFail(DeletionRequestEntity, createdDeletionRequestId); + + const isDeletionPlannedAtDateCorrect = isDeletionPlannedWithinAcceptableRange( + createdItem.createdAt, + createdItem.deleteAfter, + defaultdeleteAfterMinutes, + operationalTimeToleranceInSeconds + ); + + expect(isDeletionPlannedAtDateCorrect).toEqual(true); + }); }); - describe('when the "delete in minutes" param has been set to 0', () => { - it('should set the "deletion planned at" date to now, with some operational time tolerance', async () => { + describe('when the "delete after minutes" param has been set to 0', () => { + it('should set the "deleteAfter" date to now', async () => { const { deletionRequestToImmediateRemoval, operationalTimeToleranceInSeconds } = setup(); const response = await testApiClient.post('', deletionRequestToImmediateRemoval); @@ -164,6 +160,32 @@ describe(`deletionRequest create (api)`, () => { expect(isDeletionPlannedAtDateCorrect).toEqual(true); }); }); + + describe('when the "delete after minutes" param has been set to > 0', () => { + it('should set the "deleteAfter" date to now plus "delete after minutes" ', async () => { + const { deletionRequestToCreate, operationalTimeToleranceInSeconds } = setup(); + + const deleteAfterMinutes = 120; + + deletionRequestToCreate.deleteAfterMinutes = deleteAfterMinutes; + + const response = await testApiClient.post('', deletionRequestToCreate); + + const result = response.body as DeletionRequestResponse; + const createdDeletionRequestId = result.requestId; + + const createdItem = await em.findOneOrFail(DeletionRequestEntity, createdDeletionRequestId); + + const isDeletionPlannedAtDateCorrect = isDeletionPlannedWithinAcceptableRange( + createdItem.createdAt, + createdItem.deleteAfter, + deleteAfterMinutes, + operationalTimeToleranceInSeconds + ); + + expect(isDeletionPlannedAtDateCorrect).toEqual(true); + }); + }); }); }); }); diff --git a/apps/server/src/modules/deletion/api/controller/dto/deletion-request.body.props.spec.ts b/apps/server/src/modules/deletion/api/controller/dto/deletion-request.body.props.spec.ts index 7714714e194..418fe5400a6 100644 --- a/apps/server/src/modules/deletion/api/controller/dto/deletion-request.body.props.spec.ts +++ b/apps/server/src/modules/deletion/api/controller/dto/deletion-request.body.props.spec.ts @@ -14,12 +14,6 @@ describe(DeletionRequestBodyProps.name, () => { expect(deletionRequestBodyProps).toBeDefined(); }); - it('deleteAfterMinutes should be a number', () => { - const { deletionRequestBodyProps } = setup(); - expect(deletionRequestBodyProps.deleteAfterMinutes).toBeDefined(); - expect(typeof deletionRequestBodyProps.deleteAfterMinutes).toBe('number'); - }); - it('deleteAfterMinutes should be optional', () => { const { deletionRequestBodyProps } = setup(); deletionRequestBodyProps.deleteAfterMinutes = undefined; @@ -31,6 +25,7 @@ describe(DeletionRequestBodyProps.name, () => { const customdeleteAfterMinutes = 60; deletionRequestBodyProps.deleteAfterMinutes = customdeleteAfterMinutes; expect(deletionRequestBodyProps.deleteAfterMinutes).toBe(customdeleteAfterMinutes); + expect(typeof deletionRequestBodyProps.deleteAfterMinutes).toBe('number'); }); }); }); diff --git a/apps/server/src/modules/deletion/api/uc/deletion-request.uc.ts b/apps/server/src/modules/deletion/api/uc/deletion-request.uc.ts index 12b33ff1908..ebf6c51b795 100644 --- a/apps/server/src/modules/deletion/api/uc/deletion-request.uc.ts +++ b/apps/server/src/modules/deletion/api/uc/deletion-request.uc.ts @@ -36,7 +36,7 @@ export class DeletionRequestUc implements IEventHandler { 'course', 'dashboard', 'file', - //'fileRecords', + 'fileRecords', 'lessons', 'news', 'pseudonyms', @@ -65,10 +65,10 @@ export class DeletionRequestUc implements IEventHandler { async createDeletionRequest(deletionRequest: DeletionRequestBodyProps): Promise { this.logger.debug({ action: 'createDeletionRequest', deletionRequest }); - const hours = + const minutes = deletionRequest.deleteAfterMinutes ?? this.configService.get('ADMIN_API__DELETION_DELETE_AFTER_MINUTES'); const deleteAfter = new Date(); - deleteAfter.setMinutes(deleteAfter.getHours() + hours); + deleteAfter.setMinutes(deleteAfter.getMinutes() + minutes); const result = await this.deletionRequestService.createDeletionRequest( deletionRequest.targetRef.id, deletionRequest.targetRef.domain, diff --git a/apps/server/src/modules/deletion/domain/service/deletion-request.service.ts b/apps/server/src/modules/deletion/domain/service/deletion-request.service.ts index 3635363db98..d73d5c379f3 100644 --- a/apps/server/src/modules/deletion/domain/service/deletion-request.service.ts +++ b/apps/server/src/modules/deletion/domain/service/deletion-request.service.ts @@ -50,9 +50,9 @@ export class DeletionRequestService { async findAllItemsToExecute(limit: number): Promise { const deletionRequests = await this.deletionRequestRepo.findAllItemsToExecution( + limit, this.olderThan, - this.newerThan, - limit + this.newerThan ); return deletionRequests; diff --git a/apps/server/src/modules/deletion/repo/deletion-request.repo.spec.ts b/apps/server/src/modules/deletion/repo/deletion-request.repo.spec.ts index aa6b7755532..1e6a692238c 100644 --- a/apps/server/src/modules/deletion/repo/deletion-request.repo.spec.ts +++ b/apps/server/src/modules/deletion/repo/deletion-request.repo.spec.ts @@ -105,15 +105,19 @@ describe(DeletionRequestRepo.name, () => { describe('findAllItemsToExecution', () => { describe('when there is no deletionRequest for execution', () => { const setup = () => { - const threshold = 1000; + const limit = 1000; + const olderThan = new Date(); + const newerThan = new Date(); return { - threshold, + limit, + olderThan, + newerThan, }; }; it('should return empty array', async () => { - const { threshold } = setup(); - const result = await repo.findAllItemsToExecution(threshold); + const { limit, olderThan, newerThan } = setup(); + const result = await repo.findAllItemsToExecution(limit, olderThan, newerThan); expect(result).toEqual([]); }); @@ -121,9 +125,15 @@ describe(DeletionRequestRepo.name, () => { describe('when there are deletionRequests for execution', () => { const setup = async () => { - const threshold = 1000; + const limit = 1000; + const olderThan = new Date(); + olderThan.setDate(olderThan.getDate() + 1); + const newerThan = new Date(); + newerThan.setDate(newerThan.getDate() - 1); + const dateInFuture = new Date(); dateInFuture.setDate(dateInFuture.getDate() + 30); + const deletionRequestEntity1: DeletionRequestEntity = deletionRequestEntityFactory.build({ createdAt: new Date(2023, 7, 1), updatedAt: new Date(2023, 8, 2), @@ -140,14 +150,17 @@ describe(DeletionRequestRepo.name, () => { createdAt: new Date(2023, 8, 1), updatedAt: new Date(2023, 8, 1), deleteAfter: new Date(2023, 9, 1), + status: StatusModel.REGISTERED, }); const deletionRequestEntity4: DeletionRequestEntity = deletionRequestEntityFactory.build({ createdAt: new Date(2023, 9, 1), updatedAt: new Date(2023, 9, 1), deleteAfter: new Date(2023, 10, 1), + status: StatusModel.REGISTERED, }); const deletionRequestEntity5: DeletionRequestEntity = deletionRequestEntityFactory.build({ deleteAfter: dateInFuture, + status: StatusModel.REGISTERED, }); await em.persistAndFlush([ @@ -178,34 +191,22 @@ describe(DeletionRequestRepo.name, () => { createdAt: deletionRequestEntity3.createdAt, updatedAt: deletionRequestEntity3.updatedAt, }, - { - id: deletionRequestEntity2.id, - targetRefDomain: deletionRequestEntity2.targetRefDomain, - deleteAfter: deletionRequestEntity2.deleteAfter, - targetRefId: deletionRequestEntity2.targetRefId, - status: deletionRequestEntity2.status, - createdAt: deletionRequestEntity2.createdAt, - updatedAt: deletionRequestEntity2.updatedAt, - }, ]; - return { deletionRequestEntity1, deletionRequestEntity5, expectedArray, threshold }; + return { deletionRequestEntity1, deletionRequestEntity5, expectedArray, limit, olderThan, newerThan }; }; - it('should find deletionRequests with deleteAfter smaller then today and status with value registered or failed', async () => { - const { deletionRequestEntity1, deletionRequestEntity5, expectedArray, threshold } = await setup(); + it('should find deletionRequests with deleteAfter smaller then today and status with value registered', async () => { + const { deletionRequestEntity1, deletionRequestEntity5, expectedArray, limit, olderThan, newerThan } = + await setup(); - const results = await repo.findAllItemsToExecution(threshold); + const results = await repo.findAllItemsToExecution(limit, olderThan, newerThan); - expect(results.length).toEqual(3); + expect(results.length).toEqual(2); // Verify explicit fields. expect(results).toEqual( - expect.arrayContaining([ - expect.objectContaining(expectedArray[0]), - expect.objectContaining(expectedArray[1]), - expect.objectContaining(expectedArray[2]), - ]) + expect.arrayContaining([expect.objectContaining(expectedArray[0]), expect.objectContaining(expectedArray[1])]) ); const result1: DeletionRequest = await repo.findById(deletionRequestEntity1.id); @@ -218,9 +219,9 @@ describe(DeletionRequestRepo.name, () => { }); it('should find deletionRequests to execute with limit = 2', async () => { - const { expectedArray, threshold } = await setup(); + const { expectedArray, olderThan, newerThan } = await setup(); - const results = await repo.findAllItemsToExecution(threshold, 2); + const results = await repo.findAllItemsToExecution(2, olderThan, newerThan); expect(results.length).toEqual(2); @@ -234,8 +235,27 @@ describe(DeletionRequestRepo.name, () => { describe('countPendingDeletionRequests', () => { describe('when there is no deletionRequest with status pending', () => { + const setup = async () => { + const olderThan = new Date(); + olderThan.setDate(olderThan.getDate() + 1); + const newerThan = new Date(); + newerThan.setDate(newerThan.getDate() - 1); + const deletionRequestWithStatusRegistered: DeletionRequestEntity[] = + deletionRequestEntityFactory.buildListWithId(5, { + status: StatusModel.REGISTERED, + }); + + await em.persistAndFlush(deletionRequestWithStatusRegistered); + em.clear(); + + return { + olderThan, + newerThan, + }; + }; it('should return zero', async () => { - const result = await repo.countPendingDeletionRequests(); + const { olderThan, newerThan } = await setup(); + const result = await repo.countPendingDeletionRequests(olderThan, newerThan); expect(result).toEqual(0); }); @@ -243,6 +263,11 @@ describe(DeletionRequestRepo.name, () => { describe('when there are deletionRequests with status pending', () => { const setup = async () => { + const olderThan = new Date(); + olderThan.setDate(olderThan.getDate() + 1); + const newerThan = new Date(); + newerThan.setDate(newerThan.getDate() - 1); + const deletionRequestWithStatusPending: DeletionRequestEntity[] = deletionRequestEntityFactory.buildListWithId( 5, { @@ -260,13 +285,13 @@ describe(DeletionRequestRepo.name, () => { await em.persistAndFlush([...deletionRequestWithStatusPending, ...deletionRequestWithStatusRegistered]); em.clear(); - return { expectedNumberDeletionRequestWithStatusPending }; + return { expectedNumberDeletionRequestWithStatusPending, olderThan, newerThan }; }; it('should count deletionRequests with status pending and return proper number', async () => { - const { expectedNumberDeletionRequestWithStatusPending } = await setup(); + const { expectedNumberDeletionRequestWithStatusPending, olderThan, newerThan } = await setup(); - const result = await repo.countPendingDeletionRequests(); + const result = await repo.countPendingDeletionRequests(olderThan, newerThan); expect(result).toEqual(expectedNumberDeletionRequestWithStatusPending); }); diff --git a/apps/server/src/modules/deletion/repo/deletion-request.repo.ts b/apps/server/src/modules/deletion/repo/deletion-request.repo.ts index 653534d9e12..68c6b96f97d 100644 --- a/apps/server/src/modules/deletion/repo/deletion-request.repo.ts +++ b/apps/server/src/modules/deletion/repo/deletion-request.repo.ts @@ -32,7 +32,7 @@ export class DeletionRequestRepo { await this.em.flush(); } - async findAllItemsToExecution(olderThan: Date, newerThan: Date, limit: number): Promise { + async findAllItemsToExecution(limit: number, olderThan: Date, newerThan: Date): Promise { if (olderThan < newerThan) { throw new Error('olderThan must be greater than newerThan'); } @@ -40,12 +40,12 @@ export class DeletionRequestRepo { scope.byDeleteAfter(new Date()); const statusScope = new DeletionRequestScope('$or'); - statusScope.byStatusAndDate(StatusModel.REGISTERED); + statusScope.byStatusAndDate([StatusModel.REGISTERED]); this.addScopeForFailedRequests(statusScope, olderThan, newerThan); scope.addQuery(statusScope.query); - const order = { createdAt: SortOrder.desc }; // TODO why decending order? Should it not be ascending? + const order = { createdAt: SortOrder.asc }; const [deletionRequestEntities] = await this.em.findAndCount(DeletionRequestEntity, scope.query, { limit, @@ -58,8 +58,7 @@ export class DeletionRequestRepo { } private addScopeForFailedRequests(scope: DeletionRequestScope, olderThan: Date, newerThan: Date): void { - scope.byStatusAndDate(StatusModel.FAILED, olderThan, newerThan); - scope.byStatusAndDate(StatusModel.PENDING, olderThan, newerThan); + scope.byStatusAndDate([StatusModel.FAILED, StatusModel.PENDING], olderThan, newerThan); } async countPendingDeletionRequests(olderThan: Date, newerThan: Date): Promise { diff --git a/apps/server/src/modules/deletion/repo/entity/deletion-log.entity.ts b/apps/server/src/modules/deletion/repo/entity/deletion-log.entity.ts index 9abb397e99b..27801cbbe89 100644 --- a/apps/server/src/modules/deletion/repo/entity/deletion-log.entity.ts +++ b/apps/server/src/modules/deletion/repo/entity/deletion-log.entity.ts @@ -1,7 +1,7 @@ import { BaseEntityWithTimestamps } from '@shared/domain/entity/base.entity'; import { EntityId } from '@shared/domain/types'; import { ObjectId } from 'bson'; -import {Entity, Property, Index, Unique} from '@mikro-orm/core'; +import { Entity, Property, Index, Unique } from '@mikro-orm/core'; import { DomainOperationReport, DomainDeletionReport } from '../../domain/interface'; import { DomainName } from '../../domain/types'; diff --git a/apps/server/src/modules/deletion/repo/scope/deletion-request-scope.spec.ts b/apps/server/src/modules/deletion/repo/scope/deletion-request-scope.spec.ts index ae043621731..9b779e6a299 100644 --- a/apps/server/src/modules/deletion/repo/scope/deletion-request-scope.spec.ts +++ b/apps/server/src/modules/deletion/repo/scope/deletion-request-scope.spec.ts @@ -32,7 +32,7 @@ describe(DeletionRequestScope.name, () => { }); }); - describe('byStatus', () => { + describe('byStatusAndDate', () => { const setup = () => { const fifteenMinutesAgo = new Date(Date.now() - 15 * 60 * 1000); // 15 minutes ago const expectedQuery = { @@ -55,7 +55,7 @@ describe(DeletionRequestScope.name, () => { it('should add query', () => { const { expectedQuery, fifteenMinutesAgo } = setup(); - const result = scope.byStatus(fifteenMinutesAgo); + const result = scope.byStatusAndDate(StatusModel.FAILED, fifteenMinutesAgo); expect(result).toBeInstanceOf(DeletionRequestScope); expect(scope.query).toEqual(expectedQuery); @@ -63,7 +63,7 @@ describe(DeletionRequestScope.name, () => { }); }); - describe('byStatusPending', () => { + describe('byStatusAndDate for Pending', () => { const setup = () => { const expectedQuery = { status: [StatusModel.PENDING] }; @@ -74,7 +74,7 @@ describe(DeletionRequestScope.name, () => { it('should add query', () => { const { expectedQuery } = setup(); - const result = scope.byStatusPending(); + const result = scope.byStatusAndDate(StatusModel.PENDING); expect(result).toBeInstanceOf(DeletionRequestScope); expect(scope.query).toEqual(expectedQuery); diff --git a/apps/server/src/modules/deletion/repo/scope/deletion-request-scope.ts b/apps/server/src/modules/deletion/repo/scope/deletion-request-scope.ts index a55594879ea..116c8aca393 100644 --- a/apps/server/src/modules/deletion/repo/scope/deletion-request-scope.ts +++ b/apps/server/src/modules/deletion/repo/scope/deletion-request-scope.ts @@ -9,8 +9,8 @@ export class DeletionRequestScope extends Scope { return this; } - byStatusAndDate(status: StatusModel, olderThan?: Date, newerThan?: Date): this { - let query = { status }; + byStatusAndDate(status: StatusModel[], olderThan?: Date, newerThan?: Date): this { + let query = { status: { $in: status } }; if (olderThan) { const olderThanQuery = { updatedAt: { $lt: olderThan } }; query = { ...query, ...olderThanQuery };