Skip to content

Commit

Permalink
fix api tets and change sorting
Browse files Browse the repository at this point in the history
  • Loading branch information
@virgilchiriac
virgilchiriac committed Jan 20, 2025
1 parent a364574 commit ee088b5
Show file tree
Hide file tree
Showing 9 changed files with 120 additions and 79 deletions.
74 changes: 48 additions & 26 deletions apps/server/src/modules/deletion/api/controller/api-test/deletion-request-create.api.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
});
});
});
});
});
7 changes: 1 addition & 6 deletions apps/server/src/modules/deletion/api/controller/dto/deletion-request.body.props.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -31,6 +25,7 @@ describe(DeletionRequestBodyProps.name, () => {
const customdeleteAfterMinutes = 60;
deletionRequestBodyProps.deleteAfterMinutes = customdeleteAfterMinutes;
expect(deletionRequestBodyProps.deleteAfterMinutes).toBe(customdeleteAfterMinutes);
expect(typeof deletionRequestBodyProps.deleteAfterMinutes).toBe('number');
});
});
});
Expand Down
6 changes: 3 additions & 3 deletions apps/server/src/modules/deletion/api/uc/deletion-request.uc.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export class DeletionRequestUc implements IEventHandler<DataDeletedEvent> {
'course',
'dashboard',
'file',
//'fileRecords',
'fileRecords',
'lessons',
'news',
'pseudonyms',
Expand Down Expand Up @@ -65,10 +65,10 @@ export class DeletionRequestUc implements IEventHandler<DataDeletedEvent> {

async createDeletionRequest(deletionRequest: DeletionRequestBodyProps): Promise<DeletionRequestResponse> {
this.logger.debug({ action: 'createDeletionRequest', deletionRequest });
const hours =
const minutes =
deletionRequest.deleteAfterMinutes ?? this.configService.get<number>('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,
Expand Down
4 changes: 2 additions & 2 deletions apps/server/src/modules/deletion/domain/service/deletion-request.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ export class DeletionRequestService {

async findAllItemsToExecute(limit: number): Promise<DeletionRequest[]> {
const deletionRequests = await this.deletionRequestRepo.findAllItemsToExecution(
limit,
this.olderThan,
this.newerThan,
limit
this.newerThan
);

return deletionRequests;
Expand Down
85 changes: 55 additions & 30 deletions apps/server/src/modules/deletion/repo/deletion-request.repo.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,25 +105,35 @@ 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([]);
});
});

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),
Expand All @@ -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([
Expand Down Expand Up @@ -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);
Expand All @@ -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);

Expand All @@ -234,15 +235,39 @@ 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);
});
});

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,
{
Expand All @@ -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);
});
Expand Down
9 changes: 4 additions & 5 deletions apps/server/src/modules/deletion/repo/deletion-request.repo.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,20 @@ export class DeletionRequestRepo {
await this.em.flush();
}

async findAllItemsToExecution(olderThan: Date, newerThan: Date, limit: number): Promise<DeletionRequest[]> {
async findAllItemsToExecution(limit: number, olderThan: Date, newerThan: Date): Promise<DeletionRequest[]> {
if (olderThan < newerThan) {
throw new Error('olderThan must be greater than newerThan');
}
const scope = new DeletionRequestScope();
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,
Expand All @@ -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<number> {
Expand Down
2 changes: 1 addition & 1 deletion apps/server/src/modules/deletion/repo/entity/deletion-log.entity.ts
View file
Original file line number Diff line number Diff line change
@@ -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';

Expand Down
8 changes: 4 additions & 4 deletions apps/server/src/modules/deletion/repo/scope/deletion-request-scope.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -55,15 +55,15 @@ 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);
});
});
});

describe('byStatusPending', () => {
describe('byStatusAndDate for Pending', () => {
const setup = () => {
const expectedQuery = { status: [StatusModel.PENDING] };

Expand All @@ -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);
Expand Down
Loading

0 comments on commit ee088b5

Please sign in to comment.