Merge branch 'main' into BC-5836-Add-News-to-KNLDeletion

apps/server/src/config/database.config.ts

@@ -4,10 +4,9 @@ interface GlobalConstants {
 	DB_URL: string;
 	DB_PASSWORD?: string;
 	DB_USERNAME?: string;
-	TLDRAW_DB_URL: string;
 }
 
 const usedGlobals: GlobalConstants = globals;
 
 /** Database URL */
-export const { DB_URL, DB_PASSWORD, DB_USERNAME, TLDRAW_DB_URL } = usedGlobals;
+export const { DB_URL, DB_PASSWORD, DB_USERNAME } = usedGlobals;

apps/server/src/modules/account/repo/account.repo.integration.spec.ts

@@ -196,14 +196,46 @@ describe('account repo', () => {
 	});
 
 	describe('deleteByUserId', () => {
-		it('should delete an account by user id', async () => {
+		const setup = async () => {
 			const user = userFactory.buildWithId();
+			const userWithoutAccount = userFactory.buildWithId();
 			const account = accountFactory.build({ userId: user.id });
+
 			await em.persistAndFlush([user, account]);
 
-			await expect(repo.deleteByUserId(user.id)).resolves.not.toThrow();
+			return {
+				account,
+				user,
+				userWithoutAccount,
+			};
+		};
 
-			await expect(repo.findById(account.id)).rejects.toThrow(NotFoundError);
+		describe('when user has account', () => {
+			it('should delete an account by user id', async () => {
+				const { account, user } = await setup();
+
+				await expect(repo.deleteByUserId(user.id)).resolves.not.toThrow();
+
+				await expect(repo.findById(account.id)).rejects.toThrow(NotFoundError);
+			});
+
+			it('should return accoountId', async () => {
+				const { account, user } = await setup();
+
+				const result = await repo.deleteByUserId(user.id);
+
+				expect(result).toEqual([account.id]);
+			});
+		});
+
+		describe('when user has no account', () => {
+			it('should return null', async () => {
+				const { userWithoutAccount } = await setup();
+
+				const result = await repo.deleteByUserId(userWithoutAccount.id);
+
+				expect(result).toEqual([]);
+			});
 		});
 	});
 

apps/server/src/modules/account/repo/account.repo.ts

@@ -61,11 +61,14 @@ export class AccountRepo extends BaseRepo<Account> {
 		return this.delete(account);
 	}
 
-	async deleteByUserId(userId: EntityId): Promise<void> {
+	async deleteByUserId(userId: EntityId): Promise<EntityId[]> {
 		const account = await this.findByUserId(userId);
-		if (account) {
-			await this._em.removeAndFlush(account);
+		if (account === null) {
+			return [];
 		}
+		await this._em.removeAndFlush(account);
+
+		return [account.id];
 	}
 
 	/**

apps/server/src/modules/account/services/account-db.service.ts

@@ -111,7 +111,7 @@ export class AccountServiceDb extends AbstractAccountService {
 		return this.accountRepo.deleteById(internalId);
 	}
 
-	async deleteByUserId(userId: EntityId): Promise<void> {
+	async deleteByUserId(userId: EntityId): Promise<EntityId[]> {
 		return this.accountRepo.deleteByUserId(userId);
 	}
 

apps/server/src/modules/account/services/account-idm.service.ts

@@ -152,9 +152,11 @@ export class AccountServiceIdm extends AbstractAccountService {
 		await this.identityManager.deleteAccountById(id);
 	}
 
-	async deleteByUserId(userId: EntityId): Promise<void> {
+	async deleteByUserId(userId: EntityId): Promise<EntityId[]> {
 		const idmAccount = await this.identityManager.findAccountByDbcUserId(userId);
-		await this.identityManager.deleteAccountById(idmAccount.id);
+		const deletedAccountId = await this.identityManager.deleteAccountById(idmAccount.id);
+
+		return [deletedAccountId];
 	}
 
 	// eslint-disable-next-line @typescript-eslint/require-await, @typescript-eslint/no-unused-vars

apps/server/src/modules/account/services/account.service.abstract.ts

@@ -26,7 +26,7 @@ export abstract class AbstractAccountService {
 
 	abstract delete(id: EntityId): Promise<void>;
 
-	abstract deleteByUserId(userId: EntityId): Promise<void>;
+	abstract deleteByUserId(userId: EntityId): Promise<EntityId[]>;
 
 	abstract searchByUsernamePartialMatch(userName: string, skip: number, limit: number): Promise<Counted<AccountDto[]>>;
 

apps/server/src/modules/account/services/account.service.spec.ts

@@ -2,6 +2,9 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest';
 import { ServerConfig } from '@modules/server';
 import { ConfigService } from '@nestjs/config';
 import { Test, TestingModule } from '@nestjs/testing';
+import { ObjectId } from 'bson';
+import { DomainOperationBuilder } from '@shared/domain/builder';
+import { DomainName, OperationType } from '@shared/domain/types';
 import { LegacyLogger } from '../../../core/logger';
 import { AccountServiceDb } from './account-db.service';
 import { AccountServiceIdm } from './account-idm.service';
@@ -289,6 +292,37 @@ describe('AccountService', () => {
 		});
 	});
 
+	describe('deleteAccountByUserId', () => {
+		const setup = () => {
+			const userId = new ObjectId().toHexString();
+			const accountId = new ObjectId().toHexString();
+			const spy = jest.spyOn(accountService, 'deleteByUserId');
+
+			const expectedResult = DomainOperationBuilder.build(DomainName.ACCOUNT, OperationType.DELETE, 1, [accountId]);
+
+			return { accountId, expectedResult, spy, userId };
+		};
+
+		it('should call deleteByUserId in accountService', async () => {
+			const { spy, userId } = setup();
+
+			await accountService.deleteAccountByUserId(userId);
+			expect(spy).toHaveBeenCalledWith(userId);
+			spy.mockRestore();
+		});
+
+		it('should call deleteByUserId in accountService', async () => {
+			const { accountId, expectedResult, spy, userId } = setup();
+
+			spy.mockResolvedValueOnce([accountId]);
+
+			const result = await accountService.deleteAccountByUserId(userId);
+			expect(spy).toHaveBeenCalledWith(userId);
+			expect(result).toEqual(expectedResult);
+			spy.mockRestore();
+		});
+	});
+
 	describe('findMany', () => {
 		it('should call findMany in accountServiceDb', async () => {
 			await expect(accountService.findMany()).resolves.not.toThrow();

apps/server/src/modules/account/services/account.service.ts

@@ -2,8 +2,10 @@ import { ObjectId } from '@mikro-orm/mongodb';
 import { Injectable } from '@nestjs/common';
 import { ConfigService } from '@nestjs/config';
 import { ValidationError } from '@shared/common';
-import { Counted } from '@shared/domain/types';
+import { Counted, DomainName, EntityId, OperationType } from '@shared/domain/types';
 import { isEmail, validateOrReject } from 'class-validator';
+import { DomainOperation } from '@shared/domain/interface';
+import { DomainOperationBuilder } from '@shared/domain/builder';
 import { LegacyLogger } from '../../../core/logger';
 import { ServerConfig } from '../../server/server.config';
 import { AccountServiceDb } from './account-db.service';
@@ -157,13 +159,29 @@ export class AccountService extends AbstractAccountService {
 		});
 	}
 
-	async deleteByUserId(userId: string): Promise<void> {
-		await this.accountDb.deleteByUserId(userId);
+	async deleteByUserId(userId: string): Promise<EntityId[]> {
+		const deletedAccounts = await this.accountDb.deleteByUserId(userId);
 		await this.executeIdmMethod(async () => {
 			this.logger.debug(`Deleting account with userId ${userId} ...`);
-			await this.accountIdm.deleteByUserId(userId);
+			const deletedAccountIdm = await this.accountIdm.deleteByUserId(userId);
+			deletedAccounts.push(...deletedAccountIdm);
 			this.logger.debug(`Deleted account with userId ${userId}`);
 		});
+
+		return deletedAccounts;
+	}
+
+	async deleteAccountByUserId(userId: string): Promise<DomainOperation> {
+		const deletedAccounts = await this.deleteByUserId(userId);
+
+		const result = DomainOperationBuilder.build(
+			DomainName.ACCOUNT,
+			OperationType.DELETE,
+			deletedAccounts.length,
+			deletedAccounts
+		);
+
+		return result;
 	}
 
 	/**

apps/server/src/modules/board/repo/recursive-delete.vistor.ts

@@ -65,6 +65,7 @@ export class RecursiveDeleteVisitor implements BoardCompositeVisitorAsync {
 
 	async visitDrawingElementAsync(drawingElement: DrawingElement): Promise<void> {
 		await this.drawingElementAdapterService.deleteDrawingBinData(drawingElement.id);
+		await this.filesStorageClientAdapterService.deleteFilesOfParent(drawingElement.id);
 
 		this.deleteNode(drawingElement);
 		await this.visitChildrenAsync(drawingElement);

apps/server/src/modules/board/service/board-do-authorizable.service.ts

@@ -6,6 +6,7 @@ import {
 	BoardExternalReferenceType,
 	BoardRoles,
 	ColumnBoard,
+	isDrawingElement,
 	UserBoardRoles,
 	UserRoleEnum,
 } from '@shared/domain/domainobject';
@@ -34,10 +35,12 @@ export class BoardDoAuthorizableService implements AuthorizationLoaderService {
 		const ids = [...ancestorIds, boardDo.id];
 		const rootId = ids[0];
 		const rootBoardDo = await this.boardDoRepo.findById(rootId, 1);
+		const isDrawing = isDrawingElement(boardDo);
+
 		if (rootBoardDo instanceof ColumnBoard) {
 			if (rootBoardDo.context?.type === BoardExternalReferenceType.Course) {
 				const course = await this.courseRepo.findById(rootBoardDo.context.id);
-				const users = this.mapCourseUsersToUsergroup(course);
+				const users = this.mapCourseUsersToUsergroup(course, isDrawing);
 				return new BoardDoAuthorizable({ users, id: boardDo.id });
 			}
 		} else {
@@ -47,7 +50,7 @@ export class BoardDoAuthorizableService implements AuthorizationLoaderService {
 		return new BoardDoAuthorizable({ users: [], id: boardDo.id });
 	}
 
-	private mapCourseUsersToUsergroup(course: Course): UserBoardRoles[] {
+	private mapCourseUsersToUsergroup(course: Course, isDrawing: boolean): UserBoardRoles[] {
 		const users = [
 			...course.getTeachersList().map((user) => {
 				return {
@@ -72,7 +75,10 @@ export class BoardDoAuthorizableService implements AuthorizationLoaderService {
 					userId: user.id,
 					firstName: user.firstName,
 					lastName: user.lastName,
-					roles: [BoardRoles.READER],
+					// TODO: fix this temporary hack allowing students to upload files to the DrawingElement
+					// linked with getElementWithWritePermission method in element.uc.ts
+					// this is needed to allow students to upload/delete files to/from the tldraw whiteboard (DrawingElement)
+					roles: isDrawing ? [BoardRoles.EDITOR] : [BoardRoles.READER],
 					userRoleEnum: UserRoleEnum.STUDENT,
 				};
 			}),

apps/server/src/modules/board/uc/element.uc.spec.ts

@@ -189,6 +189,30 @@ describe(ElementUc.name, () => {
 			});
 		});
 
+		describe('when deleting an element which is of DrawingElement type', () => {
+			const setup = () => {
+				const user = userFactory.build();
+				const element = drawingElementFactory.build();
+
+				boardDoAuthorizableService.getBoardAuthorizable.mockResolvedValue(
+					new BoardDoAuthorizable({ users: [], id: new ObjectId().toHexString() })
+				);
+
+				elementService.findById.mockResolvedValueOnce(element);
+				return { element, user };
+			};
+
+			it('should authorize the user to delete the element', async () => {
+				const { element, user } = setup();
+
+				const boardDoAuthorizable = await boardDoAuthorizableService.getBoardAuthorizable(element);
+				const context = { action: Action.write, requiredPermissions: [] };
+				await uc.deleteElement(user.id, element.id);
+
+				expect(authorizationService.checkPermission).toHaveBeenCalledWith(user, boardDoAuthorizable, context);
+			});
+		});
+
 		describe('when deleting a content element', () => {
 			const setup = () => {
 				const user = userFactory.build();

apps/server/src/modules/board/uc/element.uc.ts

@@ -3,6 +3,7 @@ import { ForbiddenException, forwardRef, Inject, Injectable, UnprocessableEntity
 import {
 	AnyBoardDo,
 	AnyContentElementDo,
+	isDrawingElement,
 	isSubmissionContainerElement,
 	isSubmissionItem,
 	SubmissionItem,
@@ -53,6 +54,13 @@ export class ElementUc extends BaseUc {
 
 		if (isSubmissionItem(parent)) {
 			await this.checkSubmissionItemWritePermission(userId, parent);
+		} else if (isDrawingElement(element)) {
+			// TODO: fix this temporary hack preventing students from deleting the DrawingElement
+			// linked with getBoardAuthorizable method in board-do-authorizable.service.ts
+			// the roles are hacked for the DrawingElement to allow students for file upload
+			// so because students have BoardRoles.EDITOR role, they can delete the DrawingElement by calling delete endpoint directly
+			// to prevent this, we add UserRoleEnum.TEACHER to the requiredUserRole
+			await this.checkPermission(userId, element, Action.write, UserRoleEnum.TEACHER);
 		} else {
 			await this.checkPermission(userId, element, Action.write);
 		}

apps/server/src/modules/class/service/class.service.spec.ts

@@ -2,9 +2,10 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest';
 import { ObjectId } from '@mikro-orm/mongodb';
 import { InternalServerErrorException } from '@nestjs/common';
 import { Test, TestingModule } from '@nestjs/testing';
-import { EntityId } from '@shared/domain/types';
+import { DomainName, EntityId, OperationType } from '@shared/domain/types';
 import { setupEntities } from '@shared/testing';
 import { Logger } from '@src/core/logger';
+import { DomainOperationBuilder } from '@shared/domain/builder';
 import { Class } from '../domain';
 import { classFactory } from '../domain/testing';
 import { classEntityFactory } from '../entity/testing';
@@ -134,7 +135,13 @@ describe(ClassService.name, () => {
 
 				classesRepo.findAllByUserId.mockResolvedValue(mappedClasses);
 
+				const expectedResult = DomainOperationBuilder.build(DomainName.CLASS, OperationType.UPDATE, 2, [
+					class1.id,
+					class2.id,
+				]);
+
 				return {
+					expectedResult,
 					userId1,
 				};
 			};
@@ -147,11 +154,11 @@ describe(ClassService.name, () => {
 			});
 
 			it('should update classes without updated user', async () => {
-				const { userId1 } = setup();
+				const { expectedResult, userId1 } = setup();
 
 				const result = await service.deleteUserDataFromClasses(userId1.toHexString());
 
-				expect(result).toEqual(2);
+				expect(result).toEqual(expectedResult);
 			});
 		});
 	});