From e26897b0f6d410934a2db063168afa2835fad5ff Mon Sep 17 00:00:00 2001 From: sszafGCA <116172610+sszafGCA@users.noreply.github.com> Date: Thu, 15 Feb 2024 15:07:58 +0100 Subject: [PATCH] BC-5833-Add Submissions Entities Deletion to the Main User Deletion Use Case (#4719) --- .../deletion/uc/deletion-request.uc.spec.ts | 40 ++++++- .../deletion/uc/deletion-request.uc.ts | 29 ++++- .../task/service/submission.service.spec.ts | 108 +++++++++++++++++- .../task/service/submission.service.ts | 99 +++++++++++++++- .../domain/entity/submission.entity.spec.ts | 56 ++++++++- .../shared/domain/entity/submission.entity.ts | 40 ++++++- .../shared/domain/types/domain-name.enum.ts | 1 + .../submission.repo.integration.spec.ts | 2 +- .../shared/repo/submission/submission.repo.ts | 1 + 9 files changed, 363 insertions(+), 13 deletions(-) diff --git a/apps/server/src/modules/deletion/uc/deletion-request.uc.spec.ts b/apps/server/src/modules/deletion/uc/deletion-request.uc.spec.ts index f82364a4e83..f277c0cc6f0 100644 --- a/apps/server/src/modules/deletion/uc/deletion-request.uc.spec.ts +++ b/apps/server/src/modules/deletion/uc/deletion-request.uc.spec.ts @@ -16,7 +16,7 @@ import { ObjectId } from 'bson'; import { RegistrationPinService } from '@modules/registration-pin'; import { FilesStorageClientAdapterService } from '@src/modules/files-storage-client'; import { DomainName, OperationType } from '@shared/domain/types'; -import { TaskService } from '@modules/task'; +import { SubmissionService, TaskService } from '@modules/task'; import { DomainOperationBuilder } from '@shared/domain/builder'; import { NewsService } from '@src/modules/news/service/news.service'; import { DeletionStatusModel } from '../domain/types'; @@ -49,6 +49,7 @@ describe(DeletionRequestUc.name, () => { let dashboardService: DeepMocked; let taskService: DeepMocked; let newsService: DeepMocked; + let submissionService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -127,6 +128,7 @@ describe(DeletionRequestUc.name, () => { useValue: createMock(), }, { provide: NewsService, useValue: createMock() }, + { provide: SubmissionService, useValue: createMock() }, ], }).compile(); @@ -149,6 +151,7 @@ describe(DeletionRequestUc.name, () => { dashboardService = module.get(DashboardService); taskService = module.get(TaskService); newsService = module.get(NewsService); + submissionService = module.get(SubmissionService); await setupEntities(); }); @@ -295,6 +298,13 @@ describe(DeletionRequestUc.name, () => { new ObjectId().toHexString(), ]); + const submissionsDeleted = DomainOperationBuilder.build(DomainName.SUBMISSIONS, OperationType.DELETE, 1, [ + new ObjectId().toHexString(), + ]); + const submissionsUpdated = DomainOperationBuilder.build(DomainName.SUBMISSIONS, OperationType.UPDATE, 1, [ + new ObjectId().toHexString(), + ]); + const user = userDoFactory.buildWithId(); accountService.deleteAccountByUserId.mockResolvedValueOnce(accountDeleted); @@ -316,6 +326,8 @@ describe(DeletionRequestUc.name, () => { taskService.removeUserFromFinished.mockResolvedValueOnce(tasksModifiedByRemoveUserFromFinished); taskService.deleteTasksByOnlyCreator.mockResolvedValueOnce(tasksDeleted); newsService.deleteCreatorOrUpdaterReference.mockResolvedValueOnce(newsUpdated); + submissionService.deleteSingleSubmissionsOwnedByUser.mockResolvedValueOnce(submissionsDeleted); + submissionService.removeUserReferencesFromSubmissions.mockResolvedValueOnce(submissionsUpdated); return { deletionRequestToExecute, @@ -553,6 +565,30 @@ describe(DeletionRequestUc.name, () => { expect(newsService.deleteCreatorOrUpdaterReference).toHaveBeenCalledWith(deletionRequestToExecute.targetRefId); }); + it('should call submissionService.deleteSubmissionsByUserId to delete submissions', async () => { + const { deletionRequestToExecute } = setup(); + + deletionRequestService.findAllItemsToExecute.mockResolvedValueOnce([deletionRequestToExecute]); + + await uc.executeDeletionRequests(); + + expect(submissionService.deleteSingleSubmissionsOwnedByUser).toHaveBeenCalledWith( + deletionRequestToExecute.targetRefId + ); + }); + + it('should call submissionService.updateSubmissionByUserId to update submissions', async () => { + const { deletionRequestToExecute } = setup(); + + deletionRequestService.findAllItemsToExecute.mockResolvedValueOnce([deletionRequestToExecute]); + + await uc.executeDeletionRequests(); + + expect(submissionService.removeUserReferencesFromSubmissions).toHaveBeenCalledWith( + deletionRequestToExecute.targetRefId + ); + }); + it('should call deletionLogService.createDeletionLog to create logs for deletionRequest', async () => { const { deletionRequestToExecute } = setup(); @@ -560,7 +596,7 @@ describe(DeletionRequestUc.name, () => { await uc.executeDeletionRequests(); - expect(deletionLogService.createDeletionLog).toHaveBeenCalledTimes(13); + expect(deletionLogService.createDeletionLog).toHaveBeenCalledTimes(15); }); }); diff --git a/apps/server/src/modules/deletion/uc/deletion-request.uc.ts b/apps/server/src/modules/deletion/uc/deletion-request.uc.ts index 7535e30deaa..a6a9524a617 100644 --- a/apps/server/src/modules/deletion/uc/deletion-request.uc.ts +++ b/apps/server/src/modules/deletion/uc/deletion-request.uc.ts @@ -13,7 +13,7 @@ import { Injectable } from '@nestjs/common'; import { DomainName, EntityId, OperationType } from '@shared/domain/types'; import { LegacyLogger } from '@src/core/logger'; import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; -import { TaskService } from '@modules/task'; +import { SubmissionService, TaskService } from '@modules/task'; import { DomainOperation } from '@shared/domain/interface'; import { DomainOperationBuilder } from '@shared/domain/builder/domain-operation.builder'; import { NewsService } from '@src/modules/news/service/news.service'; @@ -43,6 +43,7 @@ export class DeletionRequestUc { private readonly filesStorageClientAdapterService: FilesStorageClientAdapterService, private readonly dashboardService: DashboardService, private readonly taskService: TaskService, + private readonly submissionService: SubmissionService, private readonly newsService: NewsService ) { this.logger.setContext(DeletionRequestUc.name); @@ -113,6 +114,7 @@ export class DeletionRequestUc { this.removeUserFromRocketChat(deletionRequest), this.removeUsersDashboard(deletionRequest), this.removeUserFromTasks(deletionRequest), + this.removeUserFromSubmissions(deletionRequest), this.removeUsersDataFromNews(deletionRequest), ]); await this.deletionRequestService.markDeletionRequestAsExecuted(deletionRequest.id); @@ -374,6 +376,31 @@ export class DeletionRequestUc { ); } + private async removeUserFromSubmissions(deletionRequest: DeletionRequest) { + this.logger.debug({ action: 'removeUserFromSubmissions', deletionRequest }); + + const [submissionsDeleted, submissionsModified] = await Promise.all([ + this.submissionService.deleteSingleSubmissionsOwnedByUser(deletionRequest.targetRefId), + this.submissionService.removeUserReferencesFromSubmissions(deletionRequest.targetRefId), + ]); + + await this.logDeletion( + deletionRequest, + submissionsDeleted.domain, + submissionsDeleted.operation, + submissionsDeleted.count, + submissionsDeleted.refs + ); + + await this.logDeletion( + deletionRequest, + submissionsModified.domain, + submissionsModified.operation, + submissionsModified.count, + submissionsModified.refs + ); + } + private async removeUsersDataFromNews(deletionRequest: DeletionRequest) { this.logger.debug({ action: 'removeUsersDataFromNews', deletionRequest }); const newsModified = await this.newsService.deleteCreatorOrUpdaterReference(deletionRequest.targetRefId); diff --git a/apps/server/src/modules/task/service/submission.service.spec.ts b/apps/server/src/modules/task/service/submission.service.spec.ts index a75ba5d51ac..f801f11581f 100644 --- a/apps/server/src/modules/task/service/submission.service.spec.ts +++ b/apps/server/src/modules/task/service/submission.service.spec.ts @@ -5,7 +5,9 @@ import { Test, TestingModule } from '@nestjs/testing'; import { Submission } from '@shared/domain/entity'; import { Counted } from '@shared/domain/types'; import { SubmissionRepo } from '@shared/repo'; -import { setupEntities, submissionFactory, taskFactory } from '@shared/testing'; +import { setupEntities, submissionFactory, taskFactory, userFactory } from '@shared/testing'; +import { Logger } from '@src/core/logger'; +import { ObjectId } from 'bson'; import { SubmissionService } from './submission.service'; describe('Submission Service', () => { @@ -29,6 +31,10 @@ describe('Submission Service', () => { provide: FilesStorageClientAdapterService, useValue: createMock(), }, + { + provide: Logger, + useValue: createMock(), + }, ], }).compile(); @@ -211,4 +217,104 @@ describe('Submission Service', () => { }); }); }); + + describe('deleteSingleSubmissionsOwnedByUser', () => { + describe('when submission with specified userId was not found ', () => { + const setup = () => { + const submission = submissionFactory.buildWithId(); + + submissionRepo.findAllByUserId.mockResolvedValueOnce([[], 0]); + + return { submission }; + }; + + it('should return deletedSubmissions number of 0', async () => { + const { submission } = setup(); + + const result = await service.deleteSingleSubmissionsOwnedByUser(new ObjectId().toString()); + + expect(result.count).toEqual(0); + expect(result.refs.length).toEqual(0); + expect(submission).toBeDefined(); + }); + }); + + describe('when submission with specified userId was found ', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const submission = submissionFactory.buildWithId({ student: user, teamMembers: [user] }); + + submissionRepo.findAllByUserId.mockResolvedValueOnce([[submission], 1]); + submissionRepo.delete.mockResolvedValueOnce(); + + return { submission, user }; + }; + + it('should return deletedSubmissions number of 1', async () => { + const { submission, user } = setup(); + + const result = await service.deleteSingleSubmissionsOwnedByUser(user.id); + + expect(result.count).toEqual(1); + expect(result.refs.length).toEqual(1); + expect(submissionRepo.delete).toBeCalledTimes(1); + expect(submissionRepo.delete).toHaveBeenCalledWith([submission]); + }); + }); + }); + + describe('removeUserReferencesFromSubmissions', () => { + describe('when submission with specified userId was not found ', () => { + const setup = () => { + const user1 = userFactory.buildWithId(); + const user2 = userFactory.buildWithId(); + const submission = submissionFactory.buildWithId({ student: user1, teamMembers: [user1, user2] }); + + submissionRepo.findAllByUserId.mockResolvedValueOnce([[], 0]); + + return { submission, user1, user2 }; + }; + + it('should return updated submission number of 0', async () => { + const { submission, user1 } = setup(); + + const result = await service.removeUserReferencesFromSubmissions(new ObjectId().toString()); + + expect(result.count).toEqual(0); + expect(result.refs.length).toEqual(0); + expect(submission.student).toEqual(user1); + expect(submission.teamMembers.length).toEqual(2); + }); + }); + + describe('when submission with specified userId was found ', () => { + const setup = () => { + const user1 = userFactory.buildWithId(); + const user2 = userFactory.buildWithId(); + const submission = submissionFactory.buildWithId({ + student: user1, + teamMembers: [user1, user2], + }); + + submissionRepo.findAllByUserId.mockResolvedValueOnce([[submission], 1]); + submissionRepo.delete.mockResolvedValueOnce(); + + return { submission, user1, user2 }; + }; + + it('should return updated submission number of 1', async () => { + const { submission, user1, user2 } = setup(); + + const result = await service.removeUserReferencesFromSubmissions(user1.id); + + expect(result.count).toEqual(1); + expect(result.refs.length).toEqual(1); + expect(submission.student).toBeUndefined(); + expect(submission.teamMembers.length).toEqual(1); + expect(submission.teamMembers[0]).toEqual(user2); + expect(submissionRepo.save).toBeCalledTimes(1); + expect(submissionRepo.save).toHaveBeenCalledWith([submission]); + }); + }); + }); }); diff --git a/apps/server/src/modules/task/service/submission.service.ts b/apps/server/src/modules/task/service/submission.service.ts index b6a545c51c1..bd835ade5c9 100644 --- a/apps/server/src/modules/task/service/submission.service.ts +++ b/apps/server/src/modules/task/service/submission.service.ts @@ -1,14 +1,19 @@ import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; import { Injectable } from '@nestjs/common'; +import { DataDeletionDomainOperationLoggable } from '@shared/common/loggable'; +import { DomainOperationBuilder } from '@shared/domain/builder'; import { Submission } from '@shared/domain/entity'; -import { Counted, EntityId } from '@shared/domain/types'; +import { DomainOperation } from '@shared/domain/interface'; +import { Counted, DomainName, EntityId, OperationType, StatusModel } from '@shared/domain/types'; import { SubmissionRepo } from '@shared/repo'; +import { Logger } from '@src/core/logger'; @Injectable() export class SubmissionService { constructor( private readonly submissionRepo: SubmissionRepo, - private readonly filesStorageClientAdapterService: FilesStorageClientAdapterService + private readonly filesStorageClientAdapterService: FilesStorageClientAdapterService, + private readonly logger: Logger ) {} async findById(submissionId: EntityId): Promise { @@ -26,4 +31,94 @@ export class SubmissionService { await this.submissionRepo.delete(submission); } + + async deleteSingleSubmissionsOwnedByUser(userId: EntityId): Promise { + this.logger.info( + new DataDeletionDomainOperationLoggable( + 'Deleting single Submissions owned by user', + DomainName.SUBMISSIONS, + userId, + StatusModel.PENDING + ) + ); + let [submissionsEntities, submissionsCount] = await this.submissionRepo.findAllByUserId(userId); + + if (submissionsCount > 0) { + submissionsEntities = submissionsEntities.filter((submission) => submission.isSingleSubmissionOwnedByUser()); + submissionsCount = submissionsEntities.length; + } + + if (submissionsCount > 0) { + await this.submissionRepo.delete(submissionsEntities); + } + + const result = DomainOperationBuilder.build( + DomainName.SUBMISSIONS, + OperationType.DELETE, + submissionsCount, + this.getSubmissionsId(submissionsEntities) + ); + + this.logger.info( + new DataDeletionDomainOperationLoggable( + 'Successfully deleted single Submissions owned by user', + DomainName.SUBMISSIONS, + userId, + StatusModel.FINISHED, + submissionsCount, + 0 + ) + ); + + return result; + } + + async removeUserReferencesFromSubmissions(userId: EntityId): Promise { + this.logger.info( + new DataDeletionDomainOperationLoggable( + 'Deleting user references from Submissions', + DomainName.SUBMISSIONS, + userId, + StatusModel.PENDING + ) + ); + + let [submissionsEntities, submissionsCount] = await this.submissionRepo.findAllByUserId(userId); + + if (submissionsCount > 0) { + submissionsEntities = submissionsEntities.filter((submission) => submission.isGroupSubmission()); + submissionsCount = submissionsEntities.length; + } + + if (submissionsCount > 0) { + submissionsEntities.forEach((submission) => { + submission.removeStudentById(userId); + submission.removeUserFromTeamMembers(userId); + }); + + await this.submissionRepo.save(submissionsEntities); + } + const result = DomainOperationBuilder.build( + DomainName.SUBMISSIONS, + OperationType.UPDATE, + submissionsCount, + this.getSubmissionsId(submissionsEntities) + ); + + this.logger.info( + new DataDeletionDomainOperationLoggable( + 'Successfully deleted references from Submissions collection', + DomainName.SUBMISSIONS, + userId, + StatusModel.FINISHED, + submissionsCount, + 0 + ) + ); + return result; + } + + private getSubmissionsId(submissions: Submission[]): EntityId[] { + return submissions.map((submission) => submission.id); + } } diff --git a/apps/server/src/shared/domain/entity/submission.entity.spec.ts b/apps/server/src/shared/domain/entity/submission.entity.spec.ts index 1a778f612c2..f6320e1fdd9 100644 --- a/apps/server/src/shared/domain/entity/submission.entity.spec.ts +++ b/apps/server/src/shared/domain/entity/submission.entity.spec.ts @@ -490,7 +490,7 @@ describe('Submission entity', () => { const courseGroup = courseGroupFactory.build(); const submission = submissionFactory.studentWithId().buildWithId({ courseGroup }); - const creatorId = submission.student.id; + const creatorId = submission.student?.id; const spy = jest.spyOn(courseGroup, 'getStudentIds').mockReturnValueOnce(studentIds); @@ -591,4 +591,58 @@ describe('Submission entity', () => { }); }); }); + describe('removeStudentById', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const submission = submissionFactory.buildWithId({ student: user }); + + return { submission, user }; + }; + describe('when userId matches studentId', () => { + it('should remove student', () => { + const { user, submission } = setup(); + submission.removeStudentById(user.id); + + expect(submission.student).toBeUndefined(); + }); + }); + describe('when userId not matches studentId', () => { + it('should not remove student', () => { + const { user, submission } = setup(); + + submission.removeStudentById(new ObjectId().toString()); + + expect(submission.student).toEqual(user); + }); + }); + }); + describe('removeUserFromTeamMembers', () => { + const setup = () => { + const user1 = userFactory.buildWithId(); + const user2 = userFactory.buildWithId(); + const submission = submissionFactory.buildWithId({ student: user1, teamMembers: [user1, user2] }); + + return { submission, user1, user2 }; + }; + describe('when userId matches teamMemberId', () => { + it('should remove student', () => { + const { user1, submission, user2 } = setup(); + submission.removeUserFromTeamMembers(user1.id); + + expect(submission.teamMembers.length).toEqual(1); + expect(submission.teamMembers[0]).toEqual(user2); + }); + }); + describe('when userId not matches teamMemberId', () => { + it('should not remove student', () => { + const { user1, submission, user2 } = setup(); + + submission.removeUserFromTeamMembers(new ObjectId().toString()); + + expect(submission.teamMembers.length).toEqual(2); + expect(submission.teamMembers[0]).toEqual(user1); + expect(submission.teamMembers[1]).toEqual(user2); + }); + }); + }); }); diff --git a/apps/server/src/shared/domain/entity/submission.entity.ts b/apps/server/src/shared/domain/entity/submission.entity.ts index 12893ed3bd5..66ad4c5e542 100644 --- a/apps/server/src/shared/domain/entity/submission.entity.ts +++ b/apps/server/src/shared/domain/entity/submission.entity.ts @@ -11,7 +11,7 @@ import type { User } from './user.entity'; export interface SubmissionProperties { school: SchoolEntity; task: Task; - student: User; + student?: User; courseGroup?: CourseGroup; teamMembers?: User[]; comment: string; @@ -33,8 +33,8 @@ export class Submission extends BaseEntityWithTimestamps { @Index() task: Task; - @ManyToOne('User', { fieldName: 'studentId' }) - student: User; + @ManyToOne('User', { fieldName: 'studentId', nullable: true }) + student?: User; @ManyToOne('CourseGroup', { fieldName: 'courseGroupId', nullable: true }) courseGroup?: CourseGroup; @@ -60,6 +60,9 @@ export class Submission extends BaseEntityWithTimestamps { constructor(props: SubmissionProperties) { super(); this.school = props.school; + if (props.student !== undefined) { + this.student = props.student; + } this.student = props.student; this.comment = props.comment; this.task = props.task; @@ -112,10 +115,13 @@ export class Submission extends BaseEntityWithTimestamps { // Bad that the logic is needed to expose the userIds, but is used in task for now. // Check later if it can be replaced and remove all related code. public getSubmitterIds(): EntityId[] { - const creatorId = this.student.id; + const creatorId = this.student?.id; const teamMemberIds = this.getTeamMemberIds(); const courseGroupMemberIds = this.getCourseGroupStudentIds(); - const memberIds = [creatorId, ...teamMemberIds, ...courseGroupMemberIds]; + const memberIds = + creatorId !== undefined + ? [creatorId, ...teamMemberIds, ...courseGroupMemberIds] + : [...teamMemberIds, ...courseGroupMemberIds]; const uniqueMemberIds = [...new Set(memberIds)]; @@ -140,4 +146,28 @@ export class Submission extends BaseEntityWithTimestamps { return isGradedForUser; } + + public isGroupSubmission(): boolean { + return this.hasCourseGroup() || (!this.hasCourseGroup() && this.teamMembers.length > 1); + } + + public isSingleSubmissionOwnedByUser(): boolean { + return !this.hasCourseGroup() && this.teamMembers.length === 1; + } + + private hasCourseGroup(): boolean { + return !!this.courseGroup; + } + + public removeStudentById(userId: EntityId): void { + if (userId === this.student?.id) { + this.student = undefined; + } + } + + public removeUserFromTeamMembers(userId: EntityId): void { + const modifiedArray = this.teamMembers.getItems().filter((user) => user.id !== userId); + + this.teamMembers.set(modifiedArray); + } } diff --git a/apps/server/src/shared/domain/types/domain-name.enum.ts b/apps/server/src/shared/domain/types/domain-name.enum.ts index 320117b5b3c..64aebb7d24a 100644 --- a/apps/server/src/shared/domain/types/domain-name.enum.ts +++ b/apps/server/src/shared/domain/types/domain-name.enum.ts @@ -14,5 +14,6 @@ export const enum DomainName { TASK = 'task', TEAMS = 'teams', USER = 'user', + SUBMISSIONS = 'submissions', NEWS = 'news', } diff --git a/apps/server/src/shared/repo/submission/submission.repo.integration.spec.ts b/apps/server/src/shared/repo/submission/submission.repo.integration.spec.ts index 5ce4aef23cd..9e9c7b9213a 100644 --- a/apps/server/src/shared/repo/submission/submission.repo.integration.spec.ts +++ b/apps/server/src/shared/repo/submission/submission.repo.integration.spec.ts @@ -107,7 +107,7 @@ describe('submission repo', () => { expect(count).toEqual(1); expect(result.length).toEqual(1); - expect(result[0].student.id).toEqual(student.id); + expect(result[0].student?.id).toEqual(student.id); }); it('should return submissions when the user is a team member', async () => { diff --git a/apps/server/src/shared/repo/submission/submission.repo.ts b/apps/server/src/shared/repo/submission/submission.repo.ts index 0508e19e7c8..6107fe350cf 100644 --- a/apps/server/src/shared/repo/submission/submission.repo.ts +++ b/apps/server/src/shared/repo/submission/submission.repo.ts @@ -23,6 +23,7 @@ export class SubmissionRepo extends BaseRepo { const [submissions, count] = await this._em.findAndCount(this.entityName, { task: { $in: taskIds }, }); + await this.populateReferences(submissions); return [submissions, count];