From ad71650a2b80e2b724738c2b2652cb8775fc23e6 Mon Sep 17 00:00:00 2001 From: virgilchiriac Date: Tue, 12 Sep 2023 19:12:02 +0200 Subject: [PATCH 01/16] BC-4983 - board: POC elements as submission item children --- .../controller/board-submission.controller.ts | 46 ++++++++++++++-- .../controller/dto/card/card.response.ts | 1 + .../submission-item.response.ts | 12 ++++- .../mapper/submission-item-response.mapper.ts | 26 ++++++++- .../board/repo/board-do.builder-impl.ts | 8 ++- .../board/repo/recursive-save.visitor.ts | 40 +++++++------- .../board/service/content-element.service.ts | 14 ++++- .../modules/board/uc/submission-item.uc.ts | 54 ++++++++++++++----- .../domainobject/board/submission-item.do.ts | 15 +++++- .../domainobject/board/types/any-board-do.ts | 2 + .../board/types/any-content-element-do.ts | 8 +++ .../entity/boardnode/types/any-board-node.ts | 12 +++++ .../domain/entity/boardnode/types/index.ts | 1 + 13 files changed, 195 insertions(+), 44 deletions(-) create mode 100644 apps/server/src/shared/domain/entity/boardnode/types/any-board-node.ts diff --git a/apps/server/src/modules/board/controller/board-submission.controller.ts b/apps/server/src/modules/board/controller/board-submission.controller.ts index 8f74b05df3f..dbc7c44dfa7 100644 --- a/apps/server/src/modules/board/controller/board-submission.controller.ts +++ b/apps/server/src/modules/board/controller/board-submission.controller.ts @@ -1,5 +1,15 @@ -import { Body, Controller, ForbiddenException, Get, HttpCode, NotFoundException, Param, Patch } from '@nestjs/common'; -import { ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger'; +import { + Body, + Controller, + ForbiddenException, + Get, + HttpCode, + NotFoundException, + Param, + Patch, + Post, +} from '@nestjs/common'; +import { ApiExtraModels, ApiOperation, ApiResponse, ApiTags, getSchemaPath } from '@nestjs/swagger'; import { ApiValidationError } from '@shared/common'; import { ICurrentUser } from '@src/modules/authentication'; import { Authenticate, CurrentUser } from '@src/modules/authentication/decorator/auth.decorator'; @@ -7,12 +17,18 @@ import { CardUc } from '../uc'; import { ElementUc } from '../uc/element.uc'; import { SubmissionItemUc } from '../uc/submission-item.uc'; import { + AnyContentElementResponse, + CardUrlParams, + CreateContentElementBodyParams, + FileElementResponse, + RichTextElementResponse, + SubmissionContainerElementResponse, SubmissionContainerUrlParams, SubmissionItemResponse, SubmissionItemUrlParams, UpdateSubmissionItemBodyParams, } from './dto'; -import { SubmissionItemResponseMapper } from './mapper'; +import { ContentElementResponseFactory, SubmissionItemResponseMapper } from './mapper'; @ApiTags('Board Submission') @Authenticate('jwt') @@ -56,4 +72,28 @@ export class BoardSubmissionController { bodyParams.completed ); } + + @ApiOperation({ summary: 'Create a new element in a submission item.' }) + @ApiExtraModels(RichTextElementResponse, FileElementResponse) + @ApiResponse({ + status: 201, + schema: { + oneOf: [{ $ref: getSchemaPath(RichTextElementResponse) }, { $ref: getSchemaPath(FileElementResponse) }], + }, + }) + @ApiResponse({ status: 400, type: ApiValidationError }) + @ApiResponse({ status: 403, type: ForbiddenException }) + @ApiResponse({ status: 404, type: NotFoundException }) + @Post(':submissionItemId/elements') + async createElement( + @Param() urlParams: SubmissionItemUrlParams, + @Body() bodyParams: CreateContentElementBodyParams, + @CurrentUser() currentUser: ICurrentUser + ): Promise { + const { type } = bodyParams; + const element = await this.submissionItemUc.createElement(currentUser.userId, urlParams.submissionItemId, type); + const response = ContentElementResponseFactory.mapToResponse(element); + + return response; + } } diff --git a/apps/server/src/modules/board/controller/dto/card/card.response.ts b/apps/server/src/modules/board/controller/dto/card/card.response.ts index 8ff034ad093..06c82c81b1f 100644 --- a/apps/server/src/modules/board/controller/dto/card/card.response.ts +++ b/apps/server/src/modules/board/controller/dto/card/card.response.ts @@ -31,6 +31,7 @@ export class CardResponse { @ApiProperty({ type: 'array', items: { + // TODO why only RichText ? oneOf: [{ $ref: getSchemaPath(RichTextElementResponse) }], }, }) diff --git a/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts b/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts index 02c3936d843..d5faa56e36d 100644 --- a/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts +++ b/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts @@ -1,13 +1,15 @@ -import { ApiProperty } from '@nestjs/swagger'; +import { ApiProperty, getSchemaPath } from '@nestjs/swagger'; +import { FileElementResponse, RichTextElementResponse } from '@src/modules/board/controller/dto'; import { TimestampsResponse } from '../timestamps.response'; import { UserDataResponse } from '../user-data.response'; export class SubmissionItemResponse { - constructor({ id, timestamps, completed, userData }: SubmissionItemResponse) { + constructor({ id, timestamps, completed, userData, elements }: SubmissionItemResponse) { this.id = id; this.timestamps = timestamps; this.completed = completed; this.userData = userData; + this.elements = elements; } @ApiProperty({ pattern: '[a-f0-9]{24}' }) @@ -21,4 +23,10 @@ export class SubmissionItemResponse { @ApiProperty() userData: UserDataResponse; + + @ApiProperty({ + type: 'array', + // TODO add types + }) + elements: (RichTextElementResponse | FileElementResponse)[]; } diff --git a/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts b/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts index c2c613da8c6..3c6da1a193f 100644 --- a/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts +++ b/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts @@ -1,4 +1,15 @@ -import { SubmissionItem } from '@shared/domain'; +import { + FileElement, + isContent, + isFileElement, + isRichTextElement, + RichTextElement, + SubmissionItem, +} from '@shared/domain'; +import { ContentElementResponseFactory } from '@src/modules/board/controller/mapper/content-element-response.factory'; +import { FileElementResponseMapper } from '@src/modules/board/controller/mapper/file-element-response.mapper'; +import { RichTextElementResponseMapper } from '@src/modules/board/controller/mapper/rich-text-element-response.mapper'; +import { UnprocessableEntityException } from '@nestjs/common'; import { SubmissionItemResponse, TimestampsResponse, UserDataResponse } from '../dto'; export class SubmissionItemResponseMapper { @@ -13,6 +24,8 @@ export class SubmissionItemResponseMapper { } public mapToResponse(submissionItem: SubmissionItem): SubmissionItemResponse { + const children: (FileElement | RichTextElement)[] = submissionItem.children.filter(isContent); + const result = new SubmissionItemResponse({ completed: submissionItem.completed, id: submissionItem.id, @@ -26,6 +39,17 @@ export class SubmissionItemResponseMapper { lastName: 'Mr Doe', userId: submissionItem.userId, }), + elements: children.map((element) => { + if (isFileElement(element)) { + const mapper = FileElementResponseMapper.getInstance(); + return mapper.mapToResponse(element); + } + if (isRichTextElement(element)) { + const mapper = RichTextElementResponseMapper.getInstance(); + return mapper.mapToResponse(element); + } + throw new UnprocessableEntityException(); + }), }); return result; diff --git a/apps/server/src/modules/board/repo/board-do.builder-impl.ts b/apps/server/src/modules/board/repo/board-do.builder-impl.ts index ec13cf8da0d..70dc0da4f5a 100644 --- a/apps/server/src/modules/board/repo/board-do.builder-impl.ts +++ b/apps/server/src/modules/board/repo/board-do.builder-impl.ts @@ -130,7 +130,11 @@ export class BoardDoBuilderImpl implements BoardDoBuilder { } public buildSubmissionItem(boardNode: SubmissionItemNode): SubmissionItem { - this.ensureLeafNode(boardNode); + this.ensureBoardNodeType(this.getChildren(boardNode), [ + BoardNodeType.FILE_ELEMENT, + BoardNodeType.RICH_TEXT_ELEMENT, + ]); + const elements = this.buildChildren(boardNode); const element = new SubmissionItem({ id: boardNode.id, @@ -138,7 +142,7 @@ export class BoardDoBuilderImpl implements BoardDoBuilder { updatedAt: boardNode.updatedAt, completed: boardNode.completed, userId: boardNode.userId, - children: [], + children: elements, }); return element; } diff --git a/apps/server/src/modules/board/repo/recursive-save.visitor.ts b/apps/server/src/modules/board/repo/recursive-save.visitor.ts index ed47ac2d128..42dd01b79f6 100644 --- a/apps/server/src/modules/board/repo/recursive-save.visitor.ts +++ b/apps/server/src/modules/board/repo/recursive-save.visitor.ts @@ -2,6 +2,7 @@ import { Utils } from '@mikro-orm/core'; import { EntityManager } from '@mikro-orm/mongodb'; import { AnyBoardDo, + AnyBoardNode, BoardCompositeVisitor, BoardNode, Card, @@ -57,8 +58,7 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { context: columnBoard.context, }); - this.createOrUpdateBoardNode(boardNode); - this.visitChildren(columnBoard, boardNode); + this.saveRecursive(boardNode, columnBoard); } visitColumn(column: Column): void { @@ -71,8 +71,7 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { position: parentData?.position, }); - this.createOrUpdateBoardNode(boardNode); - this.visitChildren(column, boardNode); + this.saveRecursive(boardNode, column); } visitCard(card: Card): void { @@ -86,8 +85,7 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { position: parentData?.position, }); - this.createOrUpdateBoardNode(boardNode); - this.visitChildren(card, boardNode); + this.saveRecursive(boardNode, card); } visitFileElement(fileElement: FileElement): void { @@ -100,8 +98,7 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { position: parentData?.position, }); - this.createOrUpdateBoardNode(boardNode); - this.visitChildren(fileElement, boardNode); + this.saveRecursive(boardNode, fileElement); } visitRichTextElement(richTextElement: RichTextElement): void { @@ -115,8 +112,7 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { position: parentData?.position, }); - this.createOrUpdateBoardNode(boardNode); - this.visitChildren(richTextElement, boardNode); + this.saveRecursive(boardNode, richTextElement); } visitSubmissionContainerElement(submissionContainerElement: SubmissionContainerElement): void { @@ -129,22 +125,20 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { position: parentData?.position, }); - this.createOrUpdateBoardNode(boardNode); - this.visitChildren(submissionContainerElement, boardNode); + this.saveRecursive(boardNode, submissionContainerElement); } - visitSubmissionItem(submission: SubmissionItem): void { - const parentData = this.parentsMap.get(submission.id); + visitSubmissionItem(submissionItem: SubmissionItem): void { + const parentData = this.parentsMap.get(submissionItem.id); const boardNode = new SubmissionItemNode({ - id: submission.id, + id: submissionItem.id, parent: parentData?.boardNode, position: parentData?.position, - completed: submission.completed, - userId: submission.userId, + completed: submissionItem.completed, + userId: submissionItem.userId, }); - this.createOrUpdateBoardNode(boardNode); - this.visitChildren(submission, boardNode); + this.saveRecursive(boardNode, submissionItem); } visitChildren(parent: AnyBoardDo, parentNode: BoardNode) { @@ -154,7 +148,7 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { }); } - registerParentData(parent: AnyBoardDo, child: AnyBoardDo, parentNode: BoardNode) { + private registerParentData(parent: AnyBoardDo, child: AnyBoardDo, parentNode: BoardNode) { const position = parent.children.findIndex((obj) => obj.id === child.id); if (position === -1) { throw new Error(`Cannot get child position. Child doesnt belong to parent`); @@ -162,6 +156,12 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { this.parentsMap.set(child.id, { boardNode: parentNode, position }); } + private saveRecursive(anyBoardNode: AnyBoardNode, anyBoardDo: AnyBoardDo): void { + this.createOrUpdateBoardNode(anyBoardNode); + this.visitChildren(anyBoardDo, anyBoardNode); + } + + // TODO make private createOrUpdateBoardNode(boardNode: BoardNode): void { const existing = this.em.getUnitOfWork().getById(BoardNode.name, boardNode.id); if (existing) { diff --git a/apps/server/src/modules/board/service/content-element.service.ts b/apps/server/src/modules/board/service/content-element.service.ts index 1c92968ec15..74ff7fa5f01 100644 --- a/apps/server/src/modules/board/service/content-element.service.ts +++ b/apps/server/src/modules/board/service/content-element.service.ts @@ -6,6 +6,8 @@ import { ContentElementType, EntityId, isAnyContentElement, + SubmissionContainerElement, + SubmissionItem, } from '@shared/domain'; import { FileContentBody, RichTextContentBody, SubmissionContainerContentBody } from '../controller/dto'; import { BoardDoRepo } from '../repo'; @@ -30,7 +32,17 @@ export class ContentElementService { return element; } - async create(parent: Card, type: ContentElementType): Promise { + async findSubmissionContainerElement(elementId: EntityId): Promise { + const element = await this.boardDoRepo.findById(elementId); + + if (!(element instanceof SubmissionContainerElement)) { + throw new NotFoundException(`There is no '${element.constructor.name}' with this id`); + } + + return element; + } + + async create(parent: Card | SubmissionItem, type: ContentElementType): Promise { const element = this.contentElementFactory.build(type); parent.addChild(element); await this.boardDoRepo.save(parent.children, parent); diff --git a/apps/server/src/modules/board/uc/submission-item.uc.ts b/apps/server/src/modules/board/uc/submission-item.uc.ts index ec15b9322e3..77c7db6113b 100644 --- a/apps/server/src/modules/board/uc/submission-item.uc.ts +++ b/apps/server/src/modules/board/uc/submission-item.uc.ts @@ -1,5 +1,23 @@ -import { ForbiddenException, forwardRef, HttpException, HttpStatus, Inject, Injectable } from '@nestjs/common'; -import { AnyBoardDo, EntityId, SubmissionContainerElement, SubmissionItem, UserRoleEnum } from '@shared/domain'; +import { + BadRequestException, + ForbiddenException, + forwardRef, + HttpException, + HttpStatus, + Inject, + Injectable, +} from '@nestjs/common'; +import { + AnyBoardDo, + ContentElementType, + EntityId, + FileElement, + isSumbmissionItem, + RichTextElement, + SubmissionContainerElement, + SubmissionItem, + UserRoleEnum, +} from '@shared/domain'; import { Logger } from '@src/core/logger'; import { AuthorizationService } from '@src/modules/authorization'; import { Action } from '@src/modules/authorization/types/action.enum'; @@ -22,14 +40,7 @@ export class SubmissionItemUc { const submissionContainer = await this.getSubmissionContainer(submissionContainerId); await this.checkPermission(userId, submissionContainer, Action.read); - let submissionItems = submissionContainer.children as SubmissionItem[]; - - if (!submissionItems.every((child) => child instanceof SubmissionItem)) { - throw new HttpException( - 'Children of submission-container-element must be of type submission-item', - HttpStatus.UNPROCESSABLE_ENTITY - ); - } + let submissionItems: SubmissionItem[] = submissionContainer.children.filter(isSumbmissionItem); const isAuthorizedStudent = await this.isAuthorizedStudent(userId, submissionContainer); if (isAuthorizedStudent) { @@ -56,6 +67,25 @@ export class SubmissionItemUc { return submissionItem; } + async createElement( + userId: EntityId, + submissionItemId: EntityId, + type: ContentElementType + ): Promise { + if (type !== ContentElementType.RICH_TEXT && type !== ContentElementType.FILE) { + throw new BadRequestException(); + } + + const submissionItem = await this.submissionItemService.findById(submissionItemId); + + // TODO + // await this.checkPermission(userId, submissionItem, Action.write); + + const element = await this.elementService.create(submissionItem, type); + // TODO + return element as FileElement | RichTextElement; + } + private async isAuthorizedStudent(userId: EntityId, boardDo: AnyBoardDo): Promise { const boardDoAuthorizable = await this.boardDoAuthorizableService.getBoardAuthorizable(boardDo); const userRoleEnum = boardDoAuthorizable.users.find((u) => u.userId === userId)?.userRoleEnum; @@ -73,9 +103,7 @@ export class SubmissionItemUc { } private async getSubmissionContainer(submissionContainerId: EntityId): Promise { - const submissionContainer = (await this.elementService.findById( - submissionContainerId - )) as SubmissionContainerElement; + const submissionContainer = await this.elementService.findSubmissionContainerElement(submissionContainerId); if (!(submissionContainer instanceof SubmissionContainerElement)) { throw new HttpException('Id does not belong to a submission container', HttpStatus.UNPROCESSABLE_ENTITY); diff --git a/apps/server/src/shared/domain/domainobject/board/submission-item.do.ts b/apps/server/src/shared/domain/domainobject/board/submission-item.do.ts index c21c2d60af4..4843a8ad52a 100644 --- a/apps/server/src/shared/domain/domainobject/board/submission-item.do.ts +++ b/apps/server/src/shared/domain/domainobject/board/submission-item.do.ts @@ -1,8 +1,17 @@ -import { EntityId } from '@shared/domain'; +import { EntityId, FileElement, isFileElement, isRichTextElement, RichTextElement } from '@shared/domain'; import { BoardComposite, BoardCompositeProps } from './board-composite.do'; import type { AnyBoardDo, BoardCompositeVisitor, BoardCompositeVisitorAsync } from './types'; export class SubmissionItem extends BoardComposite { + /* get children(): (RichTextElement | FileElement)[] { + const { children } = this.props; + if (!children) return []; + + const filteredChildren = children.filter(isFileElement || isRichTextElement); + + return filteredChildren; + } */ + get completed(): boolean { return this.props.completed; } @@ -22,7 +31,9 @@ export class SubmissionItem extends BoardComposite { // eslint-disable-next-line @typescript-eslint/no-unused-vars isAllowedAsChild(child: AnyBoardDo): boolean { // Currently submission-item rejects any children, will open in the future - return false; + const allowed = child instanceof FileElement || child instanceof RichTextElement; + + return allowed; } accept(visitor: BoardCompositeVisitor): void { diff --git a/apps/server/src/shared/domain/domainobject/board/types/any-board-do.ts b/apps/server/src/shared/domain/domainobject/board/types/any-board-do.ts index c3504070f36..c6634e9aa0f 100644 --- a/apps/server/src/shared/domain/domainobject/board/types/any-board-do.ts +++ b/apps/server/src/shared/domain/domainobject/board/types/any-board-do.ts @@ -5,3 +5,5 @@ import { SubmissionItem } from '../submission-item.do'; import { AnyContentElementDo } from './any-content-element-do'; export type AnyBoardDo = ColumnBoard | Column | Card | AnyContentElementDo | SubmissionItem; + +export const isSumbmissionItem = (element: AnyBoardDo): element is SubmissionItem => element instanceof SubmissionItem; diff --git a/apps/server/src/shared/domain/domainobject/board/types/any-content-element-do.ts b/apps/server/src/shared/domain/domainobject/board/types/any-content-element-do.ts index d0fc5ac15a7..ecb20683181 100644 --- a/apps/server/src/shared/domain/domainobject/board/types/any-content-element-do.ts +++ b/apps/server/src/shared/domain/domainobject/board/types/any-content-element-do.ts @@ -13,3 +13,11 @@ export const isAnyContentElement = (element: AnyBoardDo): element is AnyContentE return result; }; + +export const isFileElement = (element: AnyBoardDo): element is FileElement => element instanceof FileElement; + +export const isRichTextElement = (element: AnyBoardDo): element is RichTextElement => + element instanceof RichTextElement; + +export const isContent = (element: AnyBoardDo): element is RichTextElement | FileElement => + element instanceof RichTextElement || element instanceof FileElement; diff --git a/apps/server/src/shared/domain/entity/boardnode/types/any-board-node.ts b/apps/server/src/shared/domain/entity/boardnode/types/any-board-node.ts new file mode 100644 index 00000000000..1b6a3991475 --- /dev/null +++ b/apps/server/src/shared/domain/entity/boardnode/types/any-board-node.ts @@ -0,0 +1,12 @@ +import { + CardNode, + ColumnBoardNode, + ColumnNode, + FileElementNode, + RichTextElementNode, + SubmissionContainerElementNode, + SubmissionItemNode, +} from '@shared/domain'; + +export type AnyElementNode = FileElementNode | RichTextElementNode | SubmissionContainerElementNode; +export type AnyBoardNode = ColumnBoardNode | ColumnNode | CardNode | AnyElementNode | SubmissionItemNode; diff --git a/apps/server/src/shared/domain/entity/boardnode/types/index.ts b/apps/server/src/shared/domain/entity/boardnode/types/index.ts index e2ead1d7ef0..d1f0e3bfb9a 100644 --- a/apps/server/src/shared/domain/entity/boardnode/types/index.ts +++ b/apps/server/src/shared/domain/entity/boardnode/types/index.ts @@ -1,2 +1,3 @@ export * from './board-node-type'; export * from './board-do.builder'; +export * from './any-board-node'; From c488b6f7ad92625ccf55bb623b88b1a1469f6db2 Mon Sep 17 00:00:00 2001 From: virgilchiriac Date: Wed, 27 Sep 2023 14:48:36 +0200 Subject: [PATCH 02/16] cleanup --- .../domainobject/board/types/any-content-element-do.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/apps/server/src/shared/domain/domainobject/board/types/any-content-element-do.ts b/apps/server/src/shared/domain/domainobject/board/types/any-content-element-do.ts index f92cc622a06..5bd7f3860bf 100644 --- a/apps/server/src/shared/domain/domainobject/board/types/any-content-element-do.ts +++ b/apps/server/src/shared/domain/domainobject/board/types/any-content-element-do.ts @@ -16,10 +16,5 @@ export const isAnyContentElement = (element: AnyBoardDo): element is AnyContentE return result; }; -export const isFileElement = (element: AnyBoardDo): element is FileElement => element instanceof FileElement; - -export const isRichTextElement = (element: AnyBoardDo): element is RichTextElement => - element instanceof RichTextElement; - export const isContent = (element: AnyBoardDo): element is RichTextElement | FileElement => element instanceof RichTextElement || element instanceof FileElement; From 5e99894bdd65da4c120916d279775f41240a1636 Mon Sep 17 00:00:00 2001 From: virgilchiriac Date: Wed, 4 Oct 2023 10:27:51 +0200 Subject: [PATCH 03/16] BC-4983 - update sub element --- .../update-element-content.body.params.ts | 4 +- .../modules/board/controller/mapper/index.ts | 1 + .../board/service/content-element.service.ts | 9 ++++ .../server/src/modules/board/uc/element.uc.ts | 42 ++++++++++++------- .../modules/board/uc/submission-item.uc.ts | 17 ++++---- .../domainobject/board/submission-item.do.ts | 9 +++- 6 files changed, 59 insertions(+), 23 deletions(-) diff --git a/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts b/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts index 05856e9ef5f..d782b50b1e6 100644 --- a/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts +++ b/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts @@ -38,7 +38,9 @@ export class RichTextContentBody { text!: string; @IsEnum(InputFormat) - @ApiProperty() + @ApiProperty({ + enum: InputFormat, + }) inputFormat!: InputFormat; } diff --git a/apps/server/src/modules/board/controller/mapper/index.ts b/apps/server/src/modules/board/controller/mapper/index.ts index 116692df5a4..24b6ccb0943 100644 --- a/apps/server/src/modules/board/controller/mapper/index.ts +++ b/apps/server/src/modules/board/controller/mapper/index.ts @@ -3,6 +3,7 @@ export * from './card-response.mapper'; export * from './column-response.mapper'; export * from './content-element-response.factory'; export * from './rich-text-element-response.mapper'; +export * from './file-element-response.mapper'; export * from './submission-container-element-response.mapper'; export * from './submission-item-response.mapper'; export * from './external-tool-element-response.mapper'; diff --git a/apps/server/src/modules/board/service/content-element.service.ts b/apps/server/src/modules/board/service/content-element.service.ts index 8cc2820bd1a..f9f90f1ca7b 100644 --- a/apps/server/src/modules/board/service/content-element.service.ts +++ b/apps/server/src/modules/board/service/content-element.service.ts @@ -1,5 +1,6 @@ import { Injectable, NotFoundException } from '@nestjs/common'; import { + AnyBoardDo, AnyContentElementDo, Card, ContentElementFactory, @@ -32,6 +33,14 @@ export class ContentElementService { return element; } + async findParentOfId(elementId: EntityId): Promise { + const parent = await this.boardDoRepo.findParentOfId(elementId); + if (!parent) { + throw new NotFoundException('There is no node with this id'); + } + return parent; + } + async findSubmissionContainerElement(elementId: EntityId): Promise { const element = await this.boardDoRepo.findById(elementId); diff --git a/apps/server/src/modules/board/uc/element.uc.ts b/apps/server/src/modules/board/uc/element.uc.ts index 0dafd9eb98f..1674825bc90 100644 --- a/apps/server/src/modules/board/uc/element.uc.ts +++ b/apps/server/src/modules/board/uc/element.uc.ts @@ -1,4 +1,12 @@ -import { forwardRef, HttpException, HttpStatus, Inject, Injectable } from '@nestjs/common'; +import { + ForbiddenException, + forwardRef, + HttpException, + HttpStatus, + Inject, + Injectable, + UnprocessableEntityException, +} from '@nestjs/common'; import { AnyBoardDo, EntityId, @@ -8,8 +16,7 @@ import { UserRoleEnum, } from '@shared/domain'; import { Logger } from '@src/core/logger'; -import { AuthorizationService } from '@src/modules/authorization'; -import { Action } from '@src/modules/authorization/types/action.enum'; +import { Action, AuthorizationService } from '@src/modules/authorization'; import { AnyElementContentBody } from '../controller/dto'; import { BoardDoAuthorizableService, ContentElementService } from '../service'; import { SubmissionItemService } from '../service/submission-item.service'; @@ -30,8 +37,13 @@ export class ElementUc { async updateElementContent(userId: EntityId, elementId: EntityId, content: AnyElementContentBody) { const element = await this.elementService.findById(elementId); - await this.checkPermission(userId, element, Action.write); + const parent = await this.elementService.findParentOfId(elementId); + if (isSubmissionItem(parent)) { + await this.checkSubmissionPermission(userId, element, parent); + } else { + await this.checkPermission(userId, element, Action.write); + } await this.elementService.update(element, content); } @@ -43,16 +55,12 @@ export class ElementUc { const submissionContainerElement = await this.elementService.findById(contentElementId); if (!isSubmissionContainerElement(submissionContainerElement)) { - throw new HttpException( - 'Cannot create submission-item for non submission-container-element', - HttpStatus.UNPROCESSABLE_ENTITY - ); + throw new UnprocessableEntityException('Cannot create submission-item for non submission-container-element'); } if (!submissionContainerElement.children.every((child) => isSubmissionItem(child))) { - throw new HttpException( - 'Children of submission-container-element must be of type submission-item', - HttpStatus.UNPROCESSABLE_ENTITY + throw new UnprocessableEntityException( + 'Children of submission-container-element must be of type submission-item' ); } @@ -60,9 +68,8 @@ export class ElementUc { .filter(isSubmissionItem) .find((item) => item.userId === userId); if (userSubmissionExists) { - throw new HttpException( - 'User is not allowed to have multiple submission-items per submission-container-element', - HttpStatus.NOT_ACCEPTABLE + throw new ForbiddenException( + 'User is not allowed to have multiple submission-items per submission-container-element' ); } @@ -86,4 +93,11 @@ export class ElementUc { return this.authorizationService.checkPermission(user, boardDoAuthorizable, context); } + + private async checkSubmissionPermission(userId: EntityId, element: AnyBoardDo, parent: SubmissionItem) { + if (parent.userId !== userId) { + throw new ForbiddenException(); + } + await this.checkPermission(userId, parent, Action.read, UserRoleEnum.STUDENT); + } } diff --git a/apps/server/src/modules/board/uc/submission-item.uc.ts b/apps/server/src/modules/board/uc/submission-item.uc.ts index 91270449cad..d384da71465 100644 --- a/apps/server/src/modules/board/uc/submission-item.uc.ts +++ b/apps/server/src/modules/board/uc/submission-item.uc.ts @@ -6,6 +6,7 @@ import { HttpStatus, Inject, Injectable, + UnprocessableEntityException, } from '@nestjs/common'; import { AnyBoardDo, @@ -44,7 +45,7 @@ export class SubmissionItemUc { const submissionContainerElement = await this.elementService.findById(submissionContainerId); if (!isSubmissionContainerElement(submissionContainerElement)) { - throw new HttpException('Id is not submission container', HttpStatus.UNPROCESSABLE_ENTITY); + throw new UnprocessableEntityException('Id is not belong to a submission container'); } await this.checkPermission(userId, submissionContainerElement, Action.read); @@ -69,12 +70,7 @@ export class SubmissionItemUc { completed: boolean ): Promise { const submissionItem = await this.submissionItemService.findById(submissionItemId); - - await this.checkPermission(userId, submissionItem, Action.read, UserRoleEnum.STUDENT); - if (submissionItem.userId !== userId) { - throw new ForbiddenException(); - } - + await this.checkSubmissionItemPermission(userId, submissionItem); await this.submissionItemService.update(submissionItem, completed); return submissionItem; @@ -115,6 +111,13 @@ export class SubmissionItemUc { return false; } + private async checkSubmissionItemPermission(userId: EntityId, submissionItem: SubmissionItem) { + await this.checkPermission(userId, submissionItem, Action.read, UserRoleEnum.STUDENT); + if (submissionItem.userId !== userId) { + throw new ForbiddenException(); + } + } + private async checkPermission( userId: EntityId, boardDo: AnyBoardDo, diff --git a/apps/server/src/shared/domain/domainobject/board/submission-item.do.ts b/apps/server/src/shared/domain/domainobject/board/submission-item.do.ts index 00c8e9a74be..ce5f9dcbdcb 100644 --- a/apps/server/src/shared/domain/domainobject/board/submission-item.do.ts +++ b/apps/server/src/shared/domain/domainobject/board/submission-item.do.ts @@ -1,4 +1,11 @@ -import { EntityId, FileElement, isFileElement, isRichTextElement, RichTextElement } from '@shared/domain'; +import { + EntityId, + FileElement, + isFileElement, + isRichTextElement, + isSumbmissionItem, + RichTextElement, +} from '@shared/domain'; import { BoardComposite, BoardCompositeProps } from './board-composite.do'; import type { AnyBoardDo, BoardCompositeVisitor, BoardCompositeVisitorAsync } from './types'; From dbf226773ffa985b497b19dc84c70a686543f5a5 Mon Sep 17 00:00:00 2001 From: virgilchiriac Date: Fri, 13 Oct 2023 13:39:56 +0200 Subject: [PATCH 04/16] refactor UC and cleanup --- .../submission-item-create.api.spec.ts | 2 +- .../submission-item.response.ts | 2 +- .../board/controller/element.controller.ts | 2 +- .../modules/board/controller/mapper/index.ts | 1 - .../mapper/submission-item-response.mapper.ts | 8 +- .../board/repo/recursive-save.visitor.ts | 7 +- .../board/service/content-element.service.ts | 11 --- apps/server/src/modules/board/uc/base.uc.ts | 51 +++++++++++++ apps/server/src/modules/board/uc/board.uc.ts | 26 ++----- apps/server/src/modules/board/uc/card.uc.ts | 16 ++-- .../server/src/modules/board/uc/element.uc.ts | 33 ++------ .../modules/board/uc/submission-item.uc.ts | 75 ++++--------------- .../domainobject/board/submission-item.do.ts | 25 ++----- .../domainobject/board/types/any-board-do.ts | 2 - .../board/types/any-content-element-do.ts | 3 - .../entity/boardnode/types/any-board-node.ts | 10 ++- 16 files changed, 108 insertions(+), 166 deletions(-) create mode 100644 apps/server/src/modules/board/uc/base.uc.ts diff --git a/apps/server/src/modules/board/controller/api-test/submission-item-create.api.spec.ts b/apps/server/src/modules/board/controller/api-test/submission-item-create.api.spec.ts index 49cd3af657c..d594ca235fe 100644 --- a/apps/server/src/modules/board/controller/api-test/submission-item-create.api.spec.ts +++ b/apps/server/src/modules/board/controller/api-test/submission-item-create.api.spec.ts @@ -142,7 +142,7 @@ describe('submission create (api)', () => { expect(response.status).toBe(201); const response2 = await loggedInClient.post(`${submissionContainerNode.id}/submissions`, { completed: false }); - expect(response2.status).toBe(406); + expect(response2.status).toBe(403); }); }); diff --git a/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts b/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts index a85681879b6..1962e9bed28 100644 --- a/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts +++ b/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts @@ -1,6 +1,6 @@ import { ApiProperty } from '@nestjs/swagger'; import { TimestampsResponse } from '../timestamps.response'; -import { FileElementResponse, RichTextElementResponse } from '..'; +import { FileElementResponse, RichTextElementResponse } from '../element'; export class SubmissionItemResponse { constructor({ id, timestamps, completed, userId, elements }: SubmissionItemResponse) { diff --git a/apps/server/src/modules/board/controller/element.controller.ts b/apps/server/src/modules/board/controller/element.controller.ts index 2dacd2cf539..ef5aab6d3d6 100644 --- a/apps/server/src/modules/board/controller/element.controller.ts +++ b/apps/server/src/modules/board/controller/element.controller.ts @@ -134,7 +134,7 @@ export class ElementController { bodyParams.completed ); const mapper = SubmissionItemResponseMapper.getInstance(); - const response = mapper.mapSubmissionsToResponse(submissionItem); + const response = mapper.mapSubmissionItemToResponse(submissionItem); return response; } diff --git a/apps/server/src/modules/board/controller/mapper/index.ts b/apps/server/src/modules/board/controller/mapper/index.ts index 84fbbfe1da8..a24a905ae3f 100644 --- a/apps/server/src/modules/board/controller/mapper/index.ts +++ b/apps/server/src/modules/board/controller/mapper/index.ts @@ -6,6 +6,5 @@ export * from './external-tool-element-response.mapper'; export * from './file-element-response.mapper'; export * from './link-element-response.mapper'; export * from './rich-text-element-response.mapper'; -export * from './file-element-response.mapper'; export * from './submission-container-element-response.mapper'; export * from './submission-item-response.mapper'; diff --git a/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts b/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts index b9ef34b5338..a556d1b32f2 100644 --- a/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts +++ b/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts @@ -1,8 +1,8 @@ import { FileElement, - isContent, isFileElement, isRichTextElement, + isSubmissionItemContent, RichTextElement, SubmissionItem, UserBoardRoles, @@ -26,7 +26,7 @@ export class SubmissionItemResponseMapper { public mapToResponse(submissionItems: SubmissionItem[], users: UserBoardRoles[]): SubmissionsResponse { const submissionItemsResponse: SubmissionItemResponse[] = submissionItems.map((item) => - this.mapSubmissionsToResponse(item) + this.mapSubmissionItemToResponse(item) ); const usersResponse: UserDataResponse[] = users.map((user) => this.mapUsersToResponse(user)); @@ -35,8 +35,8 @@ export class SubmissionItemResponseMapper { return response; } - public mapSubmissionsToResponse(submissionItem: SubmissionItem): SubmissionItemResponse { - const children: (FileElement | RichTextElement)[] = submissionItem.children.filter(isContent); + public mapSubmissionItemToResponse(submissionItem: SubmissionItem): SubmissionItemResponse { + const children: (FileElement | RichTextElement)[] = submissionItem.children.filter(isSubmissionItemContent); const result = new SubmissionItemResponse({ completed: submissionItem.completed, id: submissionItem.id, diff --git a/apps/server/src/modules/board/repo/recursive-save.visitor.ts b/apps/server/src/modules/board/repo/recursive-save.visitor.ts index ea7f1aa0ba8..699ea6c3958 100644 --- a/apps/server/src/modules/board/repo/recursive-save.visitor.ts +++ b/apps/server/src/modules/board/repo/recursive-save.visitor.ts @@ -2,7 +2,6 @@ import { Utils } from '@mikro-orm/core'; import { EntityManager } from '@mikro-orm/mongodb'; import { AnyBoardDo, - AnyBoardNode, BoardCompositeVisitor, BoardNode, Card, @@ -194,9 +193,9 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { this.parentsMap.set(child.id, { boardNode: parentNode, position }); } - private saveRecursive(anyBoardNode: AnyBoardNode, anyBoardDo: AnyBoardDo): void { - this.createOrUpdateBoardNode(anyBoardNode); - this.visitChildren(anyBoardDo, anyBoardNode); + private saveRecursive(boardNode: BoardNode, anyBoardDo: AnyBoardDo): void { + this.createOrUpdateBoardNode(boardNode); + this.visitChildren(anyBoardDo, boardNode); } // TODO make private diff --git a/apps/server/src/modules/board/service/content-element.service.ts b/apps/server/src/modules/board/service/content-element.service.ts index 10679e416a1..4404f51fc3e 100644 --- a/apps/server/src/modules/board/service/content-element.service.ts +++ b/apps/server/src/modules/board/service/content-element.service.ts @@ -7,7 +7,6 @@ import { ContentElementType, EntityId, isAnyContentElement, - SubmissionContainerElement, SubmissionItem, } from '@shared/domain'; import { AnyElementContentBody } from '../controller/dto'; @@ -43,16 +42,6 @@ export class ContentElementService { return parent; } - async findSubmissionContainerElement(elementId: EntityId): Promise { - const element = await this.boardDoRepo.findById(elementId); - - if (!(element instanceof SubmissionContainerElement)) { - throw new NotFoundException(`There is no '${element.constructor.name}' with this id`); - } - - return element; - } - async create(parent: Card | SubmissionItem, type: ContentElementType): Promise { const element = this.contentElementFactory.build(type); parent.addChild(element); diff --git a/apps/server/src/modules/board/uc/base.uc.ts b/apps/server/src/modules/board/uc/base.uc.ts new file mode 100644 index 00000000000..f7b77d9107e --- /dev/null +++ b/apps/server/src/modules/board/uc/base.uc.ts @@ -0,0 +1,51 @@ +import { AnyBoardDo, EntityId, SubmissionItem, UserRoleEnum } from '@shared/domain'; +import { ForbiddenException, forwardRef, Inject } from '@nestjs/common'; +import { Action, AuthorizationService } from '../../authorization'; +import { BoardDoAuthorizableService } from '../service'; + +export abstract class BaseUc { + constructor( + @Inject(forwardRef(() => AuthorizationService)) + protected readonly authorizationService: AuthorizationService, + protected readonly boardDoAuthorizableService: BoardDoAuthorizableService + ) {} + + protected async checkPermission( + userId: EntityId, + boardDo: AnyBoardDo, + action: Action, + requiredUserRole?: UserRoleEnum + ): Promise { + const user = await this.authorizationService.getUserWithPermissions(userId); + const boardDoAuthorizable = await this.boardDoAuthorizableService.getBoardAuthorizable(boardDo); + if (requiredUserRole) { + boardDoAuthorizable.requiredUserRole = requiredUserRole; + } + const context = { action, requiredPermissions: [] }; + + return this.authorizationService.checkPermission(user, boardDoAuthorizable, context); + } + + protected async isAuthorizedStudent(userId: EntityId, boardDo: AnyBoardDo): Promise { + const boardDoAuthorizable = await this.boardDoAuthorizableService.getBoardAuthorizable(boardDo); + const userRoleEnum = boardDoAuthorizable.users.find((u) => u.userId === userId)?.userRoleEnum; + + if (!userRoleEnum) { + throw new ForbiddenException('User not part of this board'); + } + + // TODO do this with permission instead of role and using authorizable rules + if (userRoleEnum === UserRoleEnum.STUDENT) { + return true; + } + + return false; + } + + protected async checkSubmissionItemEditPermission(userId: EntityId, submissionItem: SubmissionItem) { + if (submissionItem.userId !== userId) { + throw new ForbiddenException(); + } + await this.checkPermission(userId, submissionItem, Action.read, UserRoleEnum.STUDENT); + } +} diff --git a/apps/server/src/modules/board/uc/board.uc.ts b/apps/server/src/modules/board/uc/board.uc.ts index 7c3194916ac..bf14283221e 100644 --- a/apps/server/src/modules/board/uc/board.uc.ts +++ b/apps/server/src/modules/board/uc/board.uc.ts @@ -1,29 +1,23 @@ import { Injectable } from '@nestjs/common'; -import { - AnyBoardDo, - BoardExternalReference, - Card, - Column, - ColumnBoard, - ContentElementType, - EntityId, -} from '@shared/domain'; +import { BoardExternalReference, Card, Column, ColumnBoard, ContentElementType, EntityId } from '@shared/domain'; import { LegacyLogger } from '@src/core/logger'; import { AuthorizationService } from '@src/modules/authorization/authorization.service'; import { Action } from '@src/modules/authorization/types/action.enum'; import { CardService, ColumnBoardService, ColumnService } from '../service'; import { BoardDoAuthorizableService } from '../service/board-do-authorizable.service'; +import { BaseUc } from './base.uc'; @Injectable() -export class BoardUc { +export class BoardUc extends BaseUc { constructor( - private readonly authorizationService: AuthorizationService, - private readonly boardDoAuthorizableService: BoardDoAuthorizableService, + protected readonly authorizationService: AuthorizationService, + protected readonly boardDoAuthorizableService: BoardDoAuthorizableService, private readonly cardService: CardService, private readonly columnBoardService: ColumnBoardService, private readonly columnService: ColumnService, private readonly logger: LegacyLogger ) { + super(authorizationService, boardDoAuthorizableService); this.logger.setContext(BoardUc.name); } @@ -157,12 +151,4 @@ export class BoardUc { await this.cardService.delete(card); } - - private async checkPermission(userId: EntityId, boardDo: AnyBoardDo, action: Action): Promise { - const user = await this.authorizationService.getUserWithPermissions(userId); - const boardDoAuthorizable = await this.boardDoAuthorizableService.getBoardAuthorizable(boardDo); - const context = { action, requiredPermissions: [] }; - - return this.authorizationService.checkPermission(user, boardDoAuthorizable, context); - } } diff --git a/apps/server/src/modules/board/uc/card.uc.ts b/apps/server/src/modules/board/uc/card.uc.ts index 577f3a8b963..b159840ccd6 100644 --- a/apps/server/src/modules/board/uc/card.uc.ts +++ b/apps/server/src/modules/board/uc/card.uc.ts @@ -4,16 +4,18 @@ import { LegacyLogger } from '@src/core/logger'; import { AuthorizationService } from '@src/modules/authorization/authorization.service'; import { Action } from '@src/modules/authorization/types/action.enum'; import { BoardDoAuthorizableService, CardService, ContentElementService } from '../service'; +import { BaseUc } from './base.uc'; @Injectable() -export class CardUc { +export class CardUc extends BaseUc { constructor( - private readonly authorizationService: AuthorizationService, - private readonly boardDoAuthorizableService: BoardDoAuthorizableService, + protected readonly authorizationService: AuthorizationService, + protected readonly boardDoAuthorizableService: BoardDoAuthorizableService, private readonly cardService: CardService, private readonly elementService: ContentElementService, private readonly logger: LegacyLogger ) { + super(authorizationService, boardDoAuthorizableService); this.logger.setContext(CardUc.name); } @@ -73,14 +75,6 @@ export class CardUc { await this.elementService.move(element, targetCard, targetPosition); } - private async checkPermission(userId: EntityId, boardDo: AnyBoardDo, action: Action): Promise { - const user = await this.authorizationService.getUserWithPermissions(userId); - const boardDoAuthorizable = await this.boardDoAuthorizableService.getBoardAuthorizable(boardDo); - const context = { action, requiredPermissions: [] }; - - return this.authorizationService.checkPermission(user, boardDoAuthorizable, context); - } - private async filterAllowed(userId: EntityId, boardDos: T[], action: Action): Promise { const user = await this.authorizationService.getUserWithPermissions(userId); diff --git a/apps/server/src/modules/board/uc/element.uc.ts b/apps/server/src/modules/board/uc/element.uc.ts index b7cdce5abcc..372f3d8f1a2 100644 --- a/apps/server/src/modules/board/uc/element.uc.ts +++ b/apps/server/src/modules/board/uc/element.uc.ts @@ -1,6 +1,5 @@ import { ForbiddenException, forwardRef, Inject, Injectable, UnprocessableEntityException } from '@nestjs/common'; import { - AnyBoardDo, AnyContentElementDo, EntityId, isSubmissionContainerElement, @@ -13,17 +12,19 @@ import { Action, AuthorizationService } from '@src/modules/authorization'; import { AnyElementContentBody } from '../controller/dto'; import { BoardDoAuthorizableService, ContentElementService } from '../service'; import { SubmissionItemService } from '../service/submission-item.service'; +import { BaseUc } from './base.uc'; @Injectable() -export class ElementUc { +export class ElementUc extends BaseUc { constructor( @Inject(forwardRef(() => AuthorizationService)) - private readonly authorizationService: AuthorizationService, - private readonly boardDoAuthorizableService: BoardDoAuthorizableService, + protected readonly authorizationService: AuthorizationService, + protected readonly boardDoAuthorizableService: BoardDoAuthorizableService, private readonly elementService: ContentElementService, private readonly submissionItemService: SubmissionItemService, private readonly logger: Logger ) { + super(authorizationService, boardDoAuthorizableService); this.logger.setContext(ElementUc.name); } @@ -36,8 +37,9 @@ export class ElementUc { const parent = await this.elementService.findParentOfId(elementId); + // TODO refactor if (isSubmissionItem(parent)) { - await this.checkSubmissionPermission(userId, element, parent); + await this.checkSubmissionItemEditPermission(userId, parent); } else { await this.checkPermission(userId, element, Action.write); } @@ -78,25 +80,4 @@ export class ElementUc { return submissionItem; } - - private async checkPermission( - userId: EntityId, - boardDo: AnyBoardDo, - action: Action, - requiredUserRole?: UserRoleEnum - ): Promise { - const user = await this.authorizationService.getUserWithPermissions(userId); - const boardDoAuthorizable = await this.boardDoAuthorizableService.getBoardAuthorizable(boardDo); - if (requiredUserRole) boardDoAuthorizable.requiredUserRole = requiredUserRole; - const context = { action, requiredPermissions: [] }; - - return this.authorizationService.checkPermission(user, boardDoAuthorizable, context); - } - - private async checkSubmissionPermission(userId: EntityId, element: AnyBoardDo, parent: SubmissionItem) { - if (parent.userId !== userId) { - throw new ForbiddenException(); - } - await this.checkPermission(userId, parent, Action.read, UserRoleEnum.STUDENT); - } } diff --git a/apps/server/src/modules/board/uc/submission-item.uc.ts b/apps/server/src/modules/board/uc/submission-item.uc.ts index d384da71465..d2bc5954c1a 100644 --- a/apps/server/src/modules/board/uc/submission-item.uc.ts +++ b/apps/server/src/modules/board/uc/submission-item.uc.ts @@ -1,18 +1,10 @@ +import { BadRequestException, forwardRef, Inject, Injectable, UnprocessableEntityException } from '@nestjs/common'; import { - BadRequestException, - ForbiddenException, - forwardRef, - HttpException, - HttpStatus, - Inject, - Injectable, - UnprocessableEntityException, -} from '@nestjs/common'; -import { - AnyBoardDo, ContentElementType, EntityId, FileElement, + isFileElement, + isRichTextElement, isSubmissionContainerElement, isSubmissionItem, RichTextElement, @@ -20,22 +12,21 @@ import { UserBoardRoles, UserRoleEnum, } from '@shared/domain'; -import { Logger } from '@src/core/logger'; import { AuthorizationService } from '@src/modules/authorization'; import { Action } from '@src/modules/authorization/types/action.enum'; import { BoardDoAuthorizableService, ContentElementService, SubmissionItemService } from '../service'; +import { BaseUc } from './base.uc'; @Injectable() -export class SubmissionItemUc { +export class SubmissionItemUc extends BaseUc { constructor( @Inject(forwardRef(() => AuthorizationService)) - private readonly authorizationService: AuthorizationService, - private readonly boardDoAuthorizableService: BoardDoAuthorizableService, - private readonly elementService: ContentElementService, - private readonly submissionItemService: SubmissionItemService, - private readonly logger: Logger + protected readonly authorizationService: AuthorizationService, + protected readonly boardDoAuthorizableService: BoardDoAuthorizableService, + protected readonly elementService: ContentElementService, + protected readonly submissionItemService: SubmissionItemService ) { - this.logger.setContext(SubmissionItemUc.name); + super(authorizationService, boardDoAuthorizableService); } async findSubmissionItems( @@ -70,7 +61,7 @@ export class SubmissionItemUc { completed: boolean ): Promise { const submissionItem = await this.submissionItemService.findById(submissionItemId); - await this.checkSubmissionItemPermission(userId, submissionItem); + await this.checkSubmissionItemEditPermission(userId, submissionItem); await this.submissionItemService.update(submissionItem, completed); return submissionItem; @@ -87,50 +78,14 @@ export class SubmissionItemUc { const submissionItem = await this.submissionItemService.findById(submissionItemId); - // TODO - // await this.checkPermission(userId, submissionItem, Action.write); + await this.checkSubmissionItemEditPermission(userId, submissionItem); const element = await this.elementService.create(submissionItem, type); - // TODO - return element as FileElement | RichTextElement; - } - - private async isAuthorizedStudent(userId: EntityId, boardDo: AnyBoardDo): Promise { - const boardDoAuthorizable = await this.boardDoAuthorizableService.getBoardAuthorizable(boardDo); - const userRoleEnum = boardDoAuthorizable.users.find((u) => u.userId === userId)?.userRoleEnum; - - if (!userRoleEnum) { - throw new ForbiddenException('User not part of this board'); - } - - // TODO do this with permission instead of role and using authorizable rules - if (userRoleEnum === UserRoleEnum.STUDENT) { - return true; - } - return false; - } - - private async checkSubmissionItemPermission(userId: EntityId, submissionItem: SubmissionItem) { - await this.checkPermission(userId, submissionItem, Action.read, UserRoleEnum.STUDENT); - if (submissionItem.userId !== userId) { - throw new ForbiddenException(); - } - } - - private async checkPermission( - userId: EntityId, - boardDo: AnyBoardDo, - action: Action, - requiredUserRole?: UserRoleEnum - ): Promise { - const user = await this.authorizationService.getUserWithPermissions(userId); - const boardDoAuthorizable = await this.boardDoAuthorizableService.getBoardAuthorizable(boardDo); - if (requiredUserRole) { - boardDoAuthorizable.requiredUserRole = requiredUserRole; + if (!isFileElement(element) && !isRichTextElement(element)) { + throw new UnprocessableEntityException(); } - const context = { action, requiredPermissions: [] }; - return this.authorizationService.checkPermission(user, boardDoAuthorizable, context); + return element; } } diff --git a/apps/server/src/shared/domain/domainobject/board/submission-item.do.ts b/apps/server/src/shared/domain/domainobject/board/submission-item.do.ts index ce5f9dcbdcb..4a96e562bb4 100644 --- a/apps/server/src/shared/domain/domainobject/board/submission-item.do.ts +++ b/apps/server/src/shared/domain/domainobject/board/submission-item.do.ts @@ -1,24 +1,8 @@ -import { - EntityId, - FileElement, - isFileElement, - isRichTextElement, - isSumbmissionItem, - RichTextElement, -} from '@shared/domain'; +import { EntityId, FileElement, isFileElement, isRichTextElement, RichTextElement } from '@shared/domain'; import { BoardComposite, BoardCompositeProps } from './board-composite.do'; import type { AnyBoardDo, BoardCompositeVisitor, BoardCompositeVisitorAsync } from './types'; export class SubmissionItem extends BoardComposite { - /* get children(): (RichTextElement | FileElement)[] { - const { children } = this.props; - if (!children) return []; - - const filteredChildren = children.filter(isFileElement || isRichTextElement); - - return filteredChildren; - } */ - get completed(): boolean { return this.props.completed; } @@ -35,10 +19,8 @@ export class SubmissionItem extends BoardComposite { this.props.userId = value; } - // eslint-disable-next-line @typescript-eslint/no-unused-vars isAllowedAsChild(child: AnyBoardDo): boolean { - // Currently submission-item rejects any children, will open in the future - const allowed = child instanceof FileElement || child instanceof RichTextElement; + const allowed = isFileElement(child) || isRichTextElement(child); return allowed; } @@ -60,3 +42,6 @@ export interface SubmissionItemProps extends BoardCompositeProps { export function isSubmissionItem(reference: unknown): reference is SubmissionItem { return reference instanceof SubmissionItem; } + +export const isSubmissionItemContent = (element: AnyBoardDo): element is RichTextElement | FileElement => + isRichTextElement(element) || isFileElement(element); diff --git a/apps/server/src/shared/domain/domainobject/board/types/any-board-do.ts b/apps/server/src/shared/domain/domainobject/board/types/any-board-do.ts index c6634e9aa0f..c3504070f36 100644 --- a/apps/server/src/shared/domain/domainobject/board/types/any-board-do.ts +++ b/apps/server/src/shared/domain/domainobject/board/types/any-board-do.ts @@ -5,5 +5,3 @@ import { SubmissionItem } from '../submission-item.do'; import { AnyContentElementDo } from './any-content-element-do'; export type AnyBoardDo = ColumnBoard | Column | Card | AnyContentElementDo | SubmissionItem; - -export const isSumbmissionItem = (element: AnyBoardDo): element is SubmissionItem => element instanceof SubmissionItem; diff --git a/apps/server/src/shared/domain/domainobject/board/types/any-content-element-do.ts b/apps/server/src/shared/domain/domainobject/board/types/any-content-element-do.ts index f47b46b40f6..614071e658c 100644 --- a/apps/server/src/shared/domain/domainobject/board/types/any-content-element-do.ts +++ b/apps/server/src/shared/domain/domainobject/board/types/any-content-element-do.ts @@ -22,6 +22,3 @@ export const isAnyContentElement = (element: AnyBoardDo): element is AnyContentE return result; }; - -export const isContent = (element: AnyBoardDo): element is RichTextElement | FileElement => - element instanceof RichTextElement || element instanceof FileElement; diff --git a/apps/server/src/shared/domain/entity/boardnode/types/any-board-node.ts b/apps/server/src/shared/domain/entity/boardnode/types/any-board-node.ts index 1b6a3991475..68110bb0fa7 100644 --- a/apps/server/src/shared/domain/entity/boardnode/types/any-board-node.ts +++ b/apps/server/src/shared/domain/entity/boardnode/types/any-board-node.ts @@ -2,11 +2,19 @@ import { CardNode, ColumnBoardNode, ColumnNode, + ExternalToolElement, FileElementNode, + LinkElementNode, RichTextElementNode, SubmissionContainerElementNode, SubmissionItemNode, } from '@shared/domain'; -export type AnyElementNode = FileElementNode | RichTextElementNode | SubmissionContainerElementNode; +export type AnyElementNode = + | FileElementNode + | RichTextElementNode + | SubmissionContainerElementNode + | ExternalToolElement + | LinkElementNode; + export type AnyBoardNode = ColumnBoardNode | ColumnNode | CardNode | AnyElementNode | SubmissionItemNode; From 4acf2052e4de9d39471a893772339a6107cde464 Mon Sep 17 00:00:00 2001 From: virgilchiriac Date: Wed, 18 Oct 2023 21:41:07 +0200 Subject: [PATCH 05/16] BC-4453 - more cleanup --- .../submission-item.response.ts | 11 +++++++--- .../mapper/submission-item-response.mapper.ts | 14 ++++++------- .../board/repo/recursive-save.visitor.ts | 4 ++-- .../board/uc/submission-item.uc.spec.ts | 5 ++++- .../modules/board/uc/submission-item.uc.ts | 11 ++++++++-- .../board/types/board-external-reference.ts | 2 +- .../boardnode/column-board-node.entity.ts | 6 +++++- .../entity/boardnode/types/any-board-node.ts | 20 ------------------- .../domain/entity/boardnode/types/index.ts | 1 - .../boardnode/column-board-node.factory.ts | 3 ++- .../board/column-board.do.factory.ts | 3 ++- 11 files changed, 40 insertions(+), 40 deletions(-) delete mode 100644 apps/server/src/shared/domain/entity/boardnode/types/any-board-node.ts diff --git a/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts b/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts index 1962e9bed28..d813ed6db11 100644 --- a/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts +++ b/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts @@ -1,7 +1,10 @@ -import { ApiProperty } from '@nestjs/swagger'; +import { ApiExtraModels, ApiProperty, getSchemaPath } from '@nestjs/swagger'; import { TimestampsResponse } from '../timestamps.response'; import { FileElementResponse, RichTextElementResponse } from '../element'; +export type SubmissionContentElementResponse = RichTextElementResponse | FileElementResponse; + +@ApiExtraModels(FileElementResponse, RichTextElementResponse) export class SubmissionItemResponse { constructor({ id, timestamps, completed, userId, elements }: SubmissionItemResponse) { this.id = id; @@ -25,7 +28,9 @@ export class SubmissionItemResponse { @ApiProperty({ type: 'array', - // TODO add types + items: { + oneOf: [{ $ref: getSchemaPath(FileElementResponse) }, { $ref: getSchemaPath(RichTextElementResponse) }], + }, }) - elements: (RichTextElementResponse | FileElementResponse)[]; + elements: SubmissionContentElementResponse[]; } diff --git a/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts b/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts index 2217606d475..c04d6d50d52 100644 --- a/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts +++ b/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts @@ -1,11 +1,11 @@ import { - FileElement, - isFileElement, - isRichTextElement, - isSubmissionItemContent, - RichTextElement, - SubmissionItem, - UserBoardRoles, + FileElement, + isFileElement, + isRichTextElement, + isSubmissionItemContent, + RichTextElement, + SubmissionItem, + UserBoardRoles, } from '@shared/domain'; import { UnprocessableEntityException } from '@nestjs/common'; import { FileElementResponseMapper } from './file-element-response.mapper'; diff --git a/apps/server/src/modules/board/repo/recursive-save.visitor.ts b/apps/server/src/modules/board/repo/recursive-save.visitor.ts index 699ea6c3958..b57eb729ef4 100644 --- a/apps/server/src/modules/board/repo/recursive-save.visitor.ts +++ b/apps/server/src/modules/board/repo/recursive-save.visitor.ts @@ -178,7 +178,7 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { this.visitChildren(externalToolElement, boardNode); } - visitChildren(parent: AnyBoardDo, parentNode: BoardNode) { + private visitChildren(parent: AnyBoardDo, parentNode: BoardNode) { parent.children.forEach((child) => { this.registerParentData(parent, child, parentNode); child.accept(this); @@ -198,7 +198,7 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { this.visitChildren(anyBoardDo, boardNode); } - // TODO make private + // TODO make private (change tests) createOrUpdateBoardNode(boardNode: BoardNode): void { const existing = this.em.getUnitOfWork().getById(BoardNode.name, boardNode.id); if (existing) { diff --git a/apps/server/src/modules/board/uc/submission-item.uc.spec.ts b/apps/server/src/modules/board/uc/submission-item.uc.spec.ts index 33bc8468fc9..acb7fa75286 100644 --- a/apps/server/src/modules/board/uc/submission-item.uc.spec.ts +++ b/apps/server/src/modules/board/uc/submission-item.uc.spec.ts @@ -11,6 +11,7 @@ import { import { Logger } from '@src/core/logger'; import { AuthorizationService } from '@src/modules/authorization'; import { Action } from '@src/modules/authorization/types/action.enum'; +import { NotFoundException } from '@nestjs/common'; import { BoardDoAuthorizableService, ContentElementService, SubmissionItemService } from '../service'; import { SubmissionItemUc } from './submission-item.uc'; @@ -194,7 +195,9 @@ describe(SubmissionItemUc.name, () => { it('should throw HttpException', async () => { const { teacher, fileEl } = setup(); - await expect(uc.findSubmissionItems(teacher.id, fileEl.id)).rejects.toThrow('Id is not submission container'); + await expect(uc.findSubmissionItems(teacher.id, fileEl.id)).rejects.toThrow( + new NotFoundException('Could not find a submission container with this id') + ); }); }); }); diff --git a/apps/server/src/modules/board/uc/submission-item.uc.ts b/apps/server/src/modules/board/uc/submission-item.uc.ts index d2bc5954c1a..74c702a1ce1 100644 --- a/apps/server/src/modules/board/uc/submission-item.uc.ts +++ b/apps/server/src/modules/board/uc/submission-item.uc.ts @@ -1,4 +1,11 @@ -import { BadRequestException, forwardRef, Inject, Injectable, UnprocessableEntityException } from '@nestjs/common'; +import { + BadRequestException, + forwardRef, + Inject, + Injectable, + NotFoundException, + UnprocessableEntityException, +} from '@nestjs/common'; import { ContentElementType, EntityId, @@ -36,7 +43,7 @@ export class SubmissionItemUc extends BaseUc { const submissionContainerElement = await this.elementService.findById(submissionContainerId); if (!isSubmissionContainerElement(submissionContainerElement)) { - throw new UnprocessableEntityException('Id is not belong to a submission container'); + throw new NotFoundException('Could not find a submission container with this id'); } await this.checkPermission(userId, submissionContainerElement, Action.read); diff --git a/apps/server/src/shared/domain/domainobject/board/types/board-external-reference.ts b/apps/server/src/shared/domain/domainobject/board/types/board-external-reference.ts index 5d16a6853d9..002804ccb30 100644 --- a/apps/server/src/shared/domain/domainobject/board/types/board-external-reference.ts +++ b/apps/server/src/shared/domain/domainobject/board/types/board-external-reference.ts @@ -1,7 +1,7 @@ import { EntityId } from '@shared/domain/types'; export enum BoardExternalReferenceType { - 'Course' = 'course', + Course = 'course', } export interface BoardExternalReference { diff --git a/apps/server/src/shared/domain/entity/boardnode/column-board-node.entity.ts b/apps/server/src/shared/domain/entity/boardnode/column-board-node.entity.ts index 26f49bf7e2c..11764f10ae1 100644 --- a/apps/server/src/shared/domain/entity/boardnode/column-board-node.entity.ts +++ b/apps/server/src/shared/domain/entity/boardnode/column-board-node.entity.ts @@ -1,5 +1,9 @@ import { Entity, Property } from '@mikro-orm/core'; -import { AnyBoardDo, BoardExternalReference, BoardExternalReferenceType } from '@shared/domain/domainobject'; +import { + AnyBoardDo, + BoardExternalReference, + BoardExternalReferenceType, +} from '@shared/domain/domainobject/board/types'; import { ObjectId } from 'bson'; import { BoardNode, BoardNodeProps } from './boardnode.entity'; import { BoardDoBuilder } from './types'; diff --git a/apps/server/src/shared/domain/entity/boardnode/types/any-board-node.ts b/apps/server/src/shared/domain/entity/boardnode/types/any-board-node.ts deleted file mode 100644 index 68110bb0fa7..00000000000 --- a/apps/server/src/shared/domain/entity/boardnode/types/any-board-node.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { - CardNode, - ColumnBoardNode, - ColumnNode, - ExternalToolElement, - FileElementNode, - LinkElementNode, - RichTextElementNode, - SubmissionContainerElementNode, - SubmissionItemNode, -} from '@shared/domain'; - -export type AnyElementNode = - | FileElementNode - | RichTextElementNode - | SubmissionContainerElementNode - | ExternalToolElement - | LinkElementNode; - -export type AnyBoardNode = ColumnBoardNode | ColumnNode | CardNode | AnyElementNode | SubmissionItemNode; diff --git a/apps/server/src/shared/domain/entity/boardnode/types/index.ts b/apps/server/src/shared/domain/entity/boardnode/types/index.ts index d1f0e3bfb9a..e2ead1d7ef0 100644 --- a/apps/server/src/shared/domain/entity/boardnode/types/index.ts +++ b/apps/server/src/shared/domain/entity/boardnode/types/index.ts @@ -1,3 +1,2 @@ export * from './board-node-type'; export * from './board-do.builder'; -export * from './any-board-node'; diff --git a/apps/server/src/shared/testing/factory/boardnode/column-board-node.factory.ts b/apps/server/src/shared/testing/factory/boardnode/column-board-node.factory.ts index 3293d1dfa7e..392e1724352 100644 --- a/apps/server/src/shared/testing/factory/boardnode/column-board-node.factory.ts +++ b/apps/server/src/shared/testing/factory/boardnode/column-board-node.factory.ts @@ -1,5 +1,6 @@ /* istanbul ignore file */ -import { BoardExternalReferenceType, ColumnBoardNode, ColumnBoardNodeProps } from '@shared/domain'; +import { ColumnBoardNode, ColumnBoardNodeProps } from '@shared/domain'; +import { BoardExternalReferenceType } from '@shared/domain/domainobject/board/types'; import { ObjectId } from 'bson'; import { BaseFactory } from '../base.factory'; diff --git a/apps/server/src/shared/testing/factory/domainobject/board/column-board.do.factory.ts b/apps/server/src/shared/testing/factory/domainobject/board/column-board.do.factory.ts index a0583ce1e8b..84561f2ab75 100644 --- a/apps/server/src/shared/testing/factory/domainobject/board/column-board.do.factory.ts +++ b/apps/server/src/shared/testing/factory/domainobject/board/column-board.do.factory.ts @@ -1,5 +1,6 @@ /* istanbul ignore file */ -import { BoardExternalReferenceType, ColumnBoard, ColumnBoardProps } from '@shared/domain'; +import { ColumnBoard, ColumnBoardProps } from '@shared/domain'; +import { BoardExternalReferenceType } from '@shared/domain/domainobject/board/types'; import { ObjectId } from 'bson'; import { BaseFactory } from '../../base.factory'; From 4db00a2c8b69742e6dc01eb87d94250eb202f995 Mon Sep 17 00:00:00 2001 From: virgilchiriac Date: Mon, 23 Oct 2023 07:43:43 +0200 Subject: [PATCH 06/16] more refactoring - adds a column uc --- .../src/modules/board/board-api.module.ts | 4 +- .../board/controller/card.controller.ts | 12 +- .../board/controller/column.controller.ts | 10 +- .../create-content-element.body.params.ts | 2 +- .../update-element-content.body.params.ts | 5 +- .../submission-item.response.ts | 4 +- .../board/controller/element.controller.ts | 2 +- apps/server/src/modules/board/uc/base.uc.ts | 8 +- .../src/modules/board/uc/board.uc.spec.ts | 191 +----------------- apps/server/src/modules/board/uc/board.uc.ts | 73 +------ .../src/modules/board/uc/card.uc.spec.ts | 132 ++++++++---- apps/server/src/modules/board/uc/card.uc.ts | 39 +++- .../src/modules/board/uc/column.uc.spec.ts | 182 +++++++++++++++++ apps/server/src/modules/board/uc/column.uc.ts | 62 ++++++ .../src/modules/board/uc/element.uc.spec.ts | 33 +++ .../server/src/modules/board/uc/element.uc.ts | 22 +- apps/server/src/modules/board/uc/index.ts | 4 + .../modules/board/uc/submission-item.uc.ts | 4 +- .../board/types/board-external-reference.ts | 2 +- 19 files changed, 465 insertions(+), 326 deletions(-) create mode 100644 apps/server/src/modules/board/uc/column.uc.spec.ts create mode 100644 apps/server/src/modules/board/uc/column.uc.ts diff --git a/apps/server/src/modules/board/board-api.module.ts b/apps/server/src/modules/board/board-api.module.ts index 10b868e67b3..6eb9ad678dc 100644 --- a/apps/server/src/modules/board/board-api.module.ts +++ b/apps/server/src/modules/board/board-api.module.ts @@ -9,13 +9,13 @@ import { ColumnController, ElementController, } from './controller'; -import { BoardUc, CardUc } from './uc'; +import { BoardUc, CardUc, ColumnUc } from './uc'; import { ElementUc } from './uc/element.uc'; import { SubmissionItemUc } from './uc/submission-item.uc'; @Module({ imports: [BoardModule, LoggerModule, forwardRef(() => AuthorizationModule)], controllers: [BoardController, ColumnController, CardController, ElementController, BoardSubmissionController], - providers: [BoardUc, CardUc, ElementUc, SubmissionItemUc], + providers: [BoardUc, ColumnUc, CardUc, ElementUc, SubmissionItemUc], }) export class BoardApiModule {} diff --git a/apps/server/src/modules/board/controller/card.controller.ts b/apps/server/src/modules/board/controller/card.controller.ts index 62afa262439..e75d7afc7a5 100644 --- a/apps/server/src/modules/board/controller/card.controller.ts +++ b/apps/server/src/modules/board/controller/card.controller.ts @@ -15,7 +15,7 @@ import { import { ApiExtraModels, ApiOperation, ApiResponse, ApiTags, getSchemaPath } from '@nestjs/swagger'; import { ApiValidationError } from '@shared/common'; import { ICurrentUser, Authenticate, CurrentUser } from '@modules/authentication'; -import { BoardUc, CardUc } from '../uc'; +import { CardUc, ColumnUc } from '../uc'; import { AnyContentElementResponse, CardIdsParams, @@ -37,7 +37,7 @@ import { CardResponseMapper, ContentElementResponseFactory } from './mapper'; @Authenticate('jwt') @Controller('cards') export class CardController { - constructor(private readonly boardUc: BoardUc, private readonly cardUc: CardUc) {} + constructor(private readonly columnUc: ColumnUc, private readonly cardUc: CardUc) {} @ApiOperation({ summary: 'Get a list of cards by their ids.' }) @ApiResponse({ status: 200, type: CardListResponse }) @@ -70,7 +70,7 @@ export class CardController { @Body() bodyParams: MoveCardBodyParams, @CurrentUser() currentUser: ICurrentUser ): Promise { - await this.boardUc.moveCard(currentUser.userId, urlParams.cardId, bodyParams.toColumnId, bodyParams.toPosition); + await this.columnUc.moveCard(currentUser.userId, urlParams.cardId, bodyParams.toColumnId, bodyParams.toPosition); } @ApiOperation({ summary: 'Update the height of a single card.' }) @@ -85,7 +85,7 @@ export class CardController { @Body() bodyParams: SetHeightBodyParams, @CurrentUser() currentUser: ICurrentUser ): Promise { - await this.boardUc.updateCardHeight(currentUser.userId, urlParams.cardId, bodyParams.height); + await this.cardUc.updateCardHeight(currentUser.userId, urlParams.cardId, bodyParams.height); } @ApiOperation({ summary: 'Update the title of a single card.' }) @@ -100,7 +100,7 @@ export class CardController { @Body() bodyParams: RenameBodyParams, @CurrentUser() currentUser: ICurrentUser ): Promise { - await this.boardUc.updateCardTitle(currentUser.userId, urlParams.cardId, bodyParams.title); + await this.cardUc.updateCardTitle(currentUser.userId, urlParams.cardId, bodyParams.title); } @ApiOperation({ summary: 'Delete a single card.' }) @@ -111,7 +111,7 @@ export class CardController { @HttpCode(204) @Delete(':cardId') async deleteCard(@Param() urlParams: CardUrlParams, @CurrentUser() currentUser: ICurrentUser): Promise { - await this.boardUc.deleteCard(currentUser.userId, urlParams.cardId); + await this.cardUc.deleteCard(currentUser.userId, urlParams.cardId); } @ApiOperation({ summary: 'Create a new element on a card.' }) diff --git a/apps/server/src/modules/board/controller/column.controller.ts b/apps/server/src/modules/board/controller/column.controller.ts index 9862ef23a74..870bcc5dc06 100644 --- a/apps/server/src/modules/board/controller/column.controller.ts +++ b/apps/server/src/modules/board/controller/column.controller.ts @@ -13,7 +13,7 @@ import { import { ApiBody, ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger'; import { ApiValidationError } from '@shared/common'; import { ICurrentUser, Authenticate, CurrentUser } from '@modules/authentication'; -import { BoardUc } from '../uc'; +import { BoardUc, ColumnUc } from '../uc'; import { CardResponse, ColumnUrlParams, MoveColumnBodyParams, RenameBodyParams } from './dto'; import { CardResponseMapper } from './mapper'; import { CreateCardBodyParams } from './dto/card/create-card.body.params'; @@ -22,7 +22,7 @@ import { CreateCardBodyParams } from './dto/card/create-card.body.params'; @Authenticate('jwt') @Controller('columns') export class ColumnController { - constructor(private readonly boardUc: BoardUc) {} + constructor(private readonly boardUc: BoardUc, private readonly columnUc: ColumnUc) {} @ApiOperation({ summary: 'Move a single column.' }) @ApiResponse({ status: 204 }) @@ -51,7 +51,7 @@ export class ColumnController { @Body() bodyParams: RenameBodyParams, @CurrentUser() currentUser: ICurrentUser ): Promise { - await this.boardUc.updateColumnTitle(currentUser.userId, urlParams.columnId, bodyParams.title); + await this.columnUc.updateColumnTitle(currentUser.userId, urlParams.columnId, bodyParams.title); } @ApiOperation({ summary: 'Delete a single column.' }) @@ -62,7 +62,7 @@ export class ColumnController { @HttpCode(204) @Delete(':columnId') async deleteColumn(@Param() urlParams: ColumnUrlParams, @CurrentUser() currentUser: ICurrentUser): Promise { - await this.boardUc.deleteColumn(currentUser.userId, urlParams.columnId); + await this.columnUc.deleteColumn(currentUser.userId, urlParams.columnId); } @ApiOperation({ summary: 'Create a new card on a column.' }) @@ -78,7 +78,7 @@ export class ColumnController { @Body() createCardBodyParams?: CreateCardBodyParams ): Promise { const { requiredEmptyElements } = createCardBodyParams || {}; - const card = await this.boardUc.createCard(currentUser.userId, urlParams.columnId, requiredEmptyElements); + const card = await this.columnUc.createCard(currentUser.userId, urlParams.columnId, requiredEmptyElements); const response = CardResponseMapper.mapToResponse(card); diff --git a/apps/server/src/modules/board/controller/dto/element/create-content-element.body.params.ts b/apps/server/src/modules/board/controller/dto/element/create-content-element.body.params.ts index 4eaf32d2a6d..d38a5c744c7 100644 --- a/apps/server/src/modules/board/controller/dto/element/create-content-element.body.params.ts +++ b/apps/server/src/modules/board/controller/dto/element/create-content-element.body.params.ts @@ -1,5 +1,5 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; -import { ContentElementType } from '@shared/domain'; +import { ContentElementType } from '@shared/domain/domainobject/board/types'; import { IsEnum, IsInt, IsOptional, Min } from 'class-validator'; export class CreateContentElementBodyParams { diff --git a/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts b/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts index 13f025eae0f..38a4163df3a 100644 --- a/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts +++ b/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts @@ -1,15 +1,16 @@ import { ApiProperty, ApiPropertyOptional, getSchemaPath } from '@nestjs/swagger'; -import { ContentElementType, InputFormat } from '@shared/domain'; +import { ContentElementType } from '@shared/domain/domainobject/board/types'; +import { InputFormat } from '@shared/domain/types'; import { Type } from 'class-transformer'; import { IsDate, IsEnum, IsMongoId, IsOptional, IsString, ValidateNested } from 'class-validator'; export abstract class ElementContentBody { + @IsEnum(ContentElementType) @ApiProperty({ enum: ContentElementType, description: 'the type of the updated element', enumName: 'ContentElementType', }) - @IsEnum(ContentElementType) type!: ContentElementType; } diff --git a/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts b/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts index d813ed6db11..77218bbc3f2 100644 --- a/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts +++ b/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts @@ -2,7 +2,7 @@ import { ApiExtraModels, ApiProperty, getSchemaPath } from '@nestjs/swagger'; import { TimestampsResponse } from '../timestamps.response'; import { FileElementResponse, RichTextElementResponse } from '../element'; -export type SubmissionContentElementResponse = RichTextElementResponse | FileElementResponse; +// export SubmissionContentElementResponse = RichTextElementResponse | FileElementResponse; @ApiExtraModels(FileElementResponse, RichTextElementResponse) export class SubmissionItemResponse { @@ -32,5 +32,5 @@ export class SubmissionItemResponse { oneOf: [{ $ref: getSchemaPath(FileElementResponse) }, { $ref: getSchemaPath(RichTextElementResponse) }], }, }) - elements: SubmissionContentElementResponse[]; + elements: (RichTextElementResponse | FileElementResponse)[]; } diff --git a/apps/server/src/modules/board/controller/element.controller.ts b/apps/server/src/modules/board/controller/element.controller.ts index 8cad0a911c1..2bed9006a0f 100644 --- a/apps/server/src/modules/board/controller/element.controller.ts +++ b/apps/server/src/modules/board/controller/element.controller.ts @@ -111,7 +111,7 @@ export class ElementController { @Param() urlParams: ContentElementUrlParams, @CurrentUser() currentUser: ICurrentUser ): Promise { - await this.cardUc.deleteElement(currentUser.userId, urlParams.contentElementId); + await this.elementUc.deleteElement(currentUser.userId, urlParams.contentElementId); } @ApiOperation({ summary: 'Create a new submission item having parent a submission container element.' }) diff --git a/apps/server/src/modules/board/uc/base.uc.ts b/apps/server/src/modules/board/uc/base.uc.ts index f7b77d9107e..e5feab2ef9f 100644 --- a/apps/server/src/modules/board/uc/base.uc.ts +++ b/apps/server/src/modules/board/uc/base.uc.ts @@ -1,6 +1,6 @@ import { AnyBoardDo, EntityId, SubmissionItem, UserRoleEnum } from '@shared/domain'; import { ForbiddenException, forwardRef, Inject } from '@nestjs/common'; -import { Action, AuthorizationService } from '../../authorization'; +import { AuthorizationService, Action } from '@modules/authorization'; import { BoardDoAuthorizableService } from '../service'; export abstract class BaseUc { @@ -12,12 +12,12 @@ export abstract class BaseUc { protected async checkPermission( userId: EntityId, - boardDo: AnyBoardDo, + anyBoardDo: AnyBoardDo, action: Action, requiredUserRole?: UserRoleEnum ): Promise { const user = await this.authorizationService.getUserWithPermissions(userId); - const boardDoAuthorizable = await this.boardDoAuthorizableService.getBoardAuthorizable(boardDo); + const boardDoAuthorizable = await this.boardDoAuthorizableService.getBoardAuthorizable(anyBoardDo); if (requiredUserRole) { boardDoAuthorizable.requiredUserRole = requiredUserRole; } @@ -42,7 +42,7 @@ export abstract class BaseUc { return false; } - protected async checkSubmissionItemEditPermission(userId: EntityId, submissionItem: SubmissionItem) { + protected async checkSubmissionItemWritePermission(userId: EntityId, submissionItem: SubmissionItem) { if (submissionItem.userId !== userId) { throw new ForbiddenException(); } diff --git a/apps/server/src/modules/board/uc/board.uc.spec.ts b/apps/server/src/modules/board/uc/board.uc.spec.ts index c644bb120da..947370bbf75 100644 --- a/apps/server/src/modules/board/uc/board.uc.spec.ts +++ b/apps/server/src/modules/board/uc/board.uc.spec.ts @@ -1,17 +1,19 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ForbiddenException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; -import { BoardDoAuthorizable, BoardRoles, ContentElementType, UserRoleEnum } from '@shared/domain/domainobject/board'; +import { BoardDoAuthorizable, BoardRoles, ContentElementType, UserRoleEnum } from '@shared/domain'; import { setupEntities, userFactory } from '@shared/testing'; -import { cardFactory, columnBoardFactory, columnFactory } from '@shared/testing/factory/domainobject'; +import { columnBoardFactory, columnFactory } from '@shared/testing/factory/domainobject'; import { LegacyLogger } from '@src/core/logger'; import { AuthorizationService } from '@modules/authorization'; import { ObjectId } from 'bson'; -import { ContentElementService } from '../service'; -import { BoardDoAuthorizableService } from '../service/board-do-authorizable.service'; -import { CardService } from '../service/card.service'; -import { ColumnBoardService } from '../service/column-board.service'; -import { ColumnService } from '../service/column.service'; +import { + BoardDoAuthorizableService, + ContentElementService, + CardService, + ColumnBoardService, + ColumnService, +} from '../service'; import { BoardUc } from './board.uc'; describe(BoardUc.name, () => { @@ -21,7 +23,6 @@ describe(BoardUc.name, () => { let boardDoAuthorizableService: DeepMocked; let columnBoardService: DeepMocked; let columnService: DeepMocked; - let cardService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -63,7 +64,6 @@ describe(BoardUc.name, () => { boardDoAuthorizableService = module.get(BoardDoAuthorizableService); columnBoardService = module.get(ColumnBoardService); columnService = module.get(ColumnService); - cardService = module.get(CardService); await setupEntities(); }); @@ -81,7 +81,6 @@ describe(BoardUc.name, () => { const board = columnBoardFactory.build(); const boardId = board.id; const column = columnFactory.build(); - const card = cardFactory.build(); authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); const authorizableMock: BoardDoAuthorizable = new BoardDoAuthorizable({ @@ -94,7 +93,7 @@ describe(BoardUc.name, () => { boardDoAuthorizableService.findById.mockResolvedValueOnce(authorizableMock); - return { user, board, boardId, column, card, createCardBodyParams }; + return { user, board, boardId, column, createCardBodyParams }; }; describe('findBoard', () => { @@ -203,27 +202,6 @@ describe(BoardUc.name, () => { }); }); - describe('deleteColumn', () => { - describe('when deleting a column', () => { - it('should call the service to find the column', async () => { - const { user, column } = setup(); - - await uc.deleteColumn(user.id, column.id); - - expect(columnService.findById).toHaveBeenCalledWith(column.id); - }); - - it('should call the service to delete the column', async () => { - const { user, column } = setup(); - columnService.findById.mockResolvedValueOnce(column); - - await uc.deleteColumn(user.id, column.id); - - expect(columnService.delete).toHaveBeenCalledWith(column); - }); - }); - }); - describe('moveColumn', () => { describe('when moving a column', () => { it('should call the service to find the column', async () => { @@ -253,153 +231,4 @@ describe(BoardUc.name, () => { }); }); }); - - describe('updateColumnTitle', () => { - describe('when updating a column title', () => { - it('should call the service to find the column', async () => { - const { user, column } = setup(); - - await uc.updateColumnTitle(user.id, column.id, 'new title'); - - expect(columnService.findById).toHaveBeenCalledWith(column.id); - }); - - it('should call the service to update the column title', async () => { - const { user, column } = setup(); - columnService.findById.mockResolvedValueOnce(column); - const newTitle = 'new title'; - - await uc.updateColumnTitle(user.id, column.id, newTitle); - - expect(columnService.updateTitle).toHaveBeenCalledWith(column, newTitle); - }); - }); - }); - - describe('createCard', () => { - describe('when creating a card', () => { - it('should call the service to create the card', async () => { - const { user, column, createCardBodyParams } = setup(); - const { requiredEmptyElements } = createCardBodyParams; - - await uc.createCard(user.id, column.id, requiredEmptyElements); - - expect(cardService.create).toHaveBeenCalledWith(column, requiredEmptyElements); - }); - - it('should return the card object', async () => { - const { user, column, card } = setup(); - cardService.create.mockResolvedValueOnce(card); - - const result = await uc.createCard(user.id, column.id); - - expect(result).toEqual(card); - }); - }); - }); - - describe('deleteCard', () => { - describe('when deleting a card', () => { - it('should call the service to find the card', async () => { - const { user, card } = setup(); - - await uc.deleteCard(user.id, card.id); - - expect(cardService.findById).toHaveBeenCalledWith(card.id); - }); - - it('should call the service to delete the card', async () => { - const { user, card } = setup(); - cardService.findById.mockResolvedValueOnce(card); - - await uc.deleteCard(user.id, card.id); - - expect(cardService.delete).toHaveBeenCalledWith(card); - }); - }); - }); - - describe('moveCard', () => { - describe('when moving a card', () => { - it('should call the service to find the card', async () => { - const { user, column, card } = setup(); - - await uc.moveCard(user.id, card.id, column.id, 5); - - expect(cardService.findById).toHaveBeenCalledWith(card.id); - }); - - it('should call the service to find the target column', async () => { - const { user, column, card } = setup(); - - await uc.moveCard(user.id, card.id, column.id, 5); - - expect(columnService.findById).toHaveBeenCalledWith(column.id); - }); - - it('should call the service to move the card', async () => { - const { user, column, card } = setup(); - cardService.findById.mockResolvedValueOnce(card); - columnService.findById.mockResolvedValueOnce(column); - - await uc.moveCard(user.id, card.id, column.id, 5); - - expect(cardService.move).toHaveBeenCalledWith(card, column, 5); - }); - }); - }); - - describe('updateCardHeight', () => { - describe('when updating a card height', () => { - it('should call the service to find the card', async () => { - const { user, card } = setup(); - const cardHeight = 200; - - await uc.updateCardHeight(user.id, card.id, cardHeight); - - expect(cardService.findById).toHaveBeenCalledWith(card.id); - }); - - it('should check the permission', async () => { - const { user, card } = setup(); - const cardHeight = 200; - - await uc.updateCardHeight(user.id, card.id, cardHeight); - - expect(authorizationService.checkPermission).toHaveBeenCalled(); - }); - - it('should call the service to update the card height', async () => { - const { user, card } = setup(); - columnService.findById.mockResolvedValueOnce(card); - const newHeight = 250; - - await uc.updateCardHeight(user.id, card.id, newHeight); - - expect(cardService.updateHeight).toHaveBeenCalledWith(card, newHeight); - }); - }); - }); - - describe('updateCardTitle', () => { - describe('when updating a card title', () => { - it('should call the service to find the card', async () => { - const { user, card } = setup(); - - await uc.updateCardTitle(user.id, card.id, 'new title'); - - expect(cardService.findById).toHaveBeenCalledWith(card.id); - }); - - it('should call the service to update the card title', async () => { - const { user, card } = setup(); - columnService.findById.mockResolvedValueOnce(card); - const newTitle = 'new title'; - - await uc.updateCardTitle(user.id, card.id, newTitle); - - expect(cardService.updateTitle).toHaveBeenCalledWith(card, newTitle); - }); - }); - }); }); diff --git a/apps/server/src/modules/board/uc/board.uc.ts b/apps/server/src/modules/board/uc/board.uc.ts index 13d90c551c0..62559bd966f 100644 --- a/apps/server/src/modules/board/uc/board.uc.ts +++ b/apps/server/src/modules/board/uc/board.uc.ts @@ -1,5 +1,5 @@ -import { Injectable } from '@nestjs/common'; -import { BoardExternalReference, Card, Column, ColumnBoard, ContentElementType, EntityId } from '@shared/domain'; +import { forwardRef, Inject, Injectable } from '@nestjs/common'; +import { BoardExternalReference, Column, ColumnBoard, EntityId } from '@shared/domain'; import { LegacyLogger } from '@src/core/logger'; import { AuthorizationService } from '@modules/authorization/domain'; import { Action } from '@modules/authorization'; @@ -10,6 +10,7 @@ import { BaseUc } from './base.uc'; @Injectable() export class BoardUc extends BaseUc { constructor( + @Inject(forwardRef(() => AuthorizationService)) protected readonly authorizationService: AuthorizationService, protected readonly boardDoAuthorizableService: BoardDoAuthorizableService, private readonly cardService: CardService, @@ -67,15 +68,6 @@ export class BoardUc extends BaseUc { return column; } - async deleteColumn(userId: EntityId, columnId: EntityId): Promise { - this.logger.debug({ action: 'deleteColumn', userId, columnId }); - - const column = await this.columnService.findById(columnId); - await this.checkPermission(userId, column, Action.write); - - await this.columnService.delete(column); - } - async moveColumn( userId: EntityId, columnId: EntityId, @@ -92,63 +84,4 @@ export class BoardUc extends BaseUc { await this.columnService.move(column, targetBoard, targetPosition); } - - async updateColumnTitle(userId: EntityId, columnId: EntityId, title: string): Promise { - this.logger.debug({ action: 'updateColumnTitle', userId, columnId, title }); - - const column = await this.columnService.findById(columnId); - await this.checkPermission(userId, column, Action.write); - - await this.columnService.updateTitle(column, title); - } - - async createCard(userId: EntityId, columnId: EntityId, requiredEmptyElements?: ContentElementType[]): Promise { - this.logger.debug({ action: 'createCard', userId, columnId }); - - const column = await this.columnService.findById(columnId); - await this.checkPermission(userId, column, Action.read); - - const card = await this.cardService.create(column, requiredEmptyElements); - - return card; - } - - async moveCard(userId: EntityId, cardId: EntityId, targetColumnId: EntityId, targetPosition: number): Promise { - this.logger.debug({ action: 'moveCard', userId, cardId, targetColumnId, toPosition: targetPosition }); - - const card = await this.cardService.findById(cardId); - const targetColumn = await this.columnService.findById(targetColumnId); - - await this.checkPermission(userId, card, Action.write); - await this.checkPermission(userId, targetColumn, Action.write); - - await this.cardService.move(card, targetColumn, targetPosition); - } - - async updateCardHeight(userId: EntityId, cardId: EntityId, height: number): Promise { - this.logger.debug({ action: 'updateCardHeight', userId, cardId, height }); - - const card = await this.cardService.findById(cardId); - await this.checkPermission(userId, card, Action.write); - - await this.cardService.updateHeight(card, height); - } - - async updateCardTitle(userId: EntityId, cardId: EntityId, title: string): Promise { - this.logger.debug({ action: 'updateCardTitle', userId, cardId, title }); - - const card = await this.cardService.findById(cardId); - await this.checkPermission(userId, card, Action.write); - - await this.cardService.updateTitle(card, title); - } - - async deleteCard(userId: EntityId, cardId: EntityId): Promise { - this.logger.debug({ action: 'deleteCard', userId, cardId }); - - const card = await this.cardService.findById(cardId); - await this.checkPermission(userId, card, Action.write); - - await this.cardService.delete(card); - } } diff --git a/apps/server/src/modules/board/uc/card.uc.spec.ts b/apps/server/src/modules/board/uc/card.uc.spec.ts index fce595085c7..7ed23a12ac4 100644 --- a/apps/server/src/modules/board/uc/card.uc.spec.ts +++ b/apps/server/src/modules/board/uc/card.uc.spec.ts @@ -1,7 +1,7 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { Test, TestingModule } from '@nestjs/testing'; import { BoardDoAuthorizable, BoardRoles, ContentElementType, UserRoleEnum } from '@shared/domain'; -import { setupEntities, userFactory } from '@shared/testing'; +import { columnBoardFactory, columnFactory, setupEntities, userFactory } from '@shared/testing'; import { cardFactory, richTextElementFactory } from '@shared/testing/factory/domainobject'; import { LegacyLogger } from '@src/core/logger'; import { AuthorizationService } from '@modules/authorization'; @@ -95,6 +95,103 @@ describe(CardUc.name, () => { }); }); + const setup = () => { + jest.clearAllMocks(); + const user = userFactory.buildWithId(); + const board = columnBoardFactory.build(); + const boardId = board.id; + const column = columnFactory.build(); + const card = cardFactory.build(); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + + const authorizableMock: BoardDoAuthorizable = new BoardDoAuthorizable({ + users: [{ userId: user.id, roles: [BoardRoles.EDITOR], userRoleEnum: UserRoleEnum.TEACHER }], + id: board.id, + }); + const createCardBodyParams = { + requiredEmptyElements: [ContentElementType.FILE, ContentElementType.RICH_TEXT], + }; + + boardDoAuthorizableService.findById.mockResolvedValueOnce(authorizableMock); + + return { user, board, boardId, column, card, createCardBodyParams }; + }; + + describe('deleteCard', () => { + describe('when deleting a card', () => { + it('should call the service to find the card', async () => { + const { user, card } = setup(); + + await uc.deleteCard(user.id, card.id); + + expect(cardService.findById).toHaveBeenCalledWith(card.id); + }); + + it('should call the service to delete the card', async () => { + const { user, card } = setup(); + cardService.findById.mockResolvedValueOnce(card); + + await uc.deleteCard(user.id, card.id); + + expect(cardService.delete).toHaveBeenCalledWith(card); + }); + }); + }); + + describe('updateCardHeight', () => { + describe('when updating a card height', () => { + it('should call the service to find the card', async () => { + const { user, card } = setup(); + const cardHeight = 200; + + await uc.updateCardHeight(user.id, card.id, cardHeight); + + expect(cardService.findById).toHaveBeenCalledWith(card.id); + }); + + it('should check the permission', async () => { + const { user, card } = setup(); + const cardHeight = 200; + + await uc.updateCardHeight(user.id, card.id, cardHeight); + + expect(authorizationService.checkPermission).toHaveBeenCalled(); + }); + + it('should call the service to update the card height', async () => { + const { user, card } = setup(); + cardService.findById.mockResolvedValueOnce(card); + const newHeight = 250; + + await uc.updateCardHeight(user.id, card.id, newHeight); + + expect(cardService.updateHeight).toHaveBeenCalledWith(card, newHeight); + }); + }); + }); + + describe('updateCardTitle', () => { + describe('when updating a card title', () => { + it('should call the service to find the card', async () => { + const { user, card } = setup(); + + await uc.updateCardTitle(user.id, card.id, 'new title'); + + expect(cardService.findById).toHaveBeenCalledWith(card.id); + }); + + it('should call the service to update the card title', async () => { + const { user, card } = setup(); + cardService.findById.mockResolvedValueOnce(card); + const newTitle = 'new title'; + + await uc.updateCardTitle(user.id, card.id, newTitle); + + expect(cardService.updateTitle).toHaveBeenCalledWith(card, newTitle); + }); + }); + }); + describe('createElement', () => { describe('when creating a content element', () => { const setup = () => { @@ -152,39 +249,6 @@ describe(CardUc.name, () => { }); }); - describe('deleteElement', () => { - describe('when deleting a content element', () => { - const setup = () => { - const user = userFactory.build(); - const element = richTextElementFactory.build(); - const card = cardFactory.build(); - - boardDoAuthorizableService.getBoardAuthorizable.mockResolvedValue( - new BoardDoAuthorizable({ users: [], id: new ObjectId().toHexString() }) - ); - - return { user, card, element }; - }; - - it('should call the service to find the element', async () => { - const { user, element } = setup(); - - await uc.deleteElement(user.id, element.id); - - expect(elementService.findById).toHaveBeenCalledWith(element.id); - }); - - it('should call the service to delete the element', async () => { - const { user, element } = setup(); - elementService.findById.mockResolvedValueOnce(element); - - await uc.deleteElement(user.id, element.id); - - expect(elementService.delete).toHaveBeenCalledWith(element); - }); - }); - }); - describe('moveElement', () => { describe('when moving an element', () => { const setup = () => { diff --git a/apps/server/src/modules/board/uc/card.uc.ts b/apps/server/src/modules/board/uc/card.uc.ts index df92728ecb9..97289984922 100644 --- a/apps/server/src/modules/board/uc/card.uc.ts +++ b/apps/server/src/modules/board/uc/card.uc.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@nestjs/common'; +import { forwardRef, Inject, Injectable } from '@nestjs/common'; import { AnyBoardDo, AnyContentElementDo, Card, ContentElementType, EntityId } from '@shared/domain'; import { LegacyLogger } from '@src/core/logger'; import { AuthorizationService, Action } from '@modules/authorization'; @@ -8,6 +8,7 @@ import { BaseUc } from './base.uc'; @Injectable() export class CardUc extends BaseUc { constructor( + @Inject(forwardRef(() => AuthorizationService)) protected readonly authorizationService: AuthorizationService, protected readonly boardDoAuthorizableService: BoardDoAuthorizableService, private readonly cardService: CardService, @@ -27,6 +28,33 @@ export class CardUc extends BaseUc { return allowedCards; } + async updateCardHeight(userId: EntityId, cardId: EntityId, height: number): Promise { + this.logger.debug({ action: 'updateCardHeight', userId, cardId, height }); + + const card = await this.cardService.findById(cardId); + await this.checkPermission(userId, card, Action.write); + + await this.cardService.updateHeight(card, height); + } + + async updateCardTitle(userId: EntityId, cardId: EntityId, title: string): Promise { + this.logger.debug({ action: 'updateCardTitle', userId, cardId, title }); + + const card = await this.cardService.findById(cardId); + await this.checkPermission(userId, card, Action.write); + + await this.cardService.updateTitle(card, title); + } + + async deleteCard(userId: EntityId, cardId: EntityId): Promise { + this.logger.debug({ action: 'deleteCard', userId, cardId }); + + const card = await this.cardService.findById(cardId); + await this.checkPermission(userId, card, Action.write); + + await this.cardService.delete(card); + } + // --- elements --- async createElement( @@ -48,15 +76,6 @@ export class CardUc extends BaseUc { return element; } - async deleteElement(userId: EntityId, elementId: EntityId): Promise { - this.logger.debug({ action: 'deleteElement', userId, elementId }); - - const element = await this.elementService.findById(elementId); - await this.checkPermission(userId, element, Action.write); - - await this.elementService.delete(element); - } - async moveElement( userId: EntityId, elementId: EntityId, diff --git a/apps/server/src/modules/board/uc/column.uc.spec.ts b/apps/server/src/modules/board/uc/column.uc.spec.ts new file mode 100644 index 00000000000..4708a52f793 --- /dev/null +++ b/apps/server/src/modules/board/uc/column.uc.spec.ts @@ -0,0 +1,182 @@ +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { Test, TestingModule } from '@nestjs/testing'; +import { BoardDoAuthorizable, BoardRoles, ContentElementType, UserRoleEnum } from '@shared/domain'; +import { setupEntities, userFactory } from '@shared/testing'; +import { cardFactory, columnBoardFactory, columnFactory } from '@shared/testing/factory/domainobject'; +import { LegacyLogger } from '@src/core/logger'; +import { AuthorizationService } from '@modules/authorization'; +import { ContentElementService, BoardDoAuthorizableService, CardService, ColumnService } from '../service'; +import { ColumnUc } from './column.uc'; + +describe(ColumnUc.name, () => { + let module: TestingModule; + let uc: ColumnUc; + let authorizationService: DeepMocked; + let boardDoAuthorizableService: DeepMocked; + let columnService: DeepMocked; + let cardService: DeepMocked; + + beforeAll(async () => { + module = await Test.createTestingModule({ + providers: [ + ColumnUc, + { + provide: AuthorizationService, + useValue: createMock(), + }, + { + provide: BoardDoAuthorizableService, + useValue: createMock(), + }, + { + provide: CardService, + useValue: createMock(), + }, + { + provide: ColumnService, + useValue: createMock(), + }, + { + provide: LegacyLogger, + useValue: createMock(), + }, + { + provide: ContentElementService, + useValue: createMock(), + }, + ], + }).compile(); + + uc = module.get(ColumnUc); + authorizationService = module.get(AuthorizationService); + boardDoAuthorizableService = module.get(BoardDoAuthorizableService); + columnService = module.get(ColumnService); + cardService = module.get(CardService); + await setupEntities(); + }); + + afterAll(async () => { + await module.close(); + }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + const setup = () => { + jest.clearAllMocks(); + const user = userFactory.buildWithId(); + const board = columnBoardFactory.build(); + const boardId = board.id; + const column = columnFactory.build(); + const card = cardFactory.build(); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + + const authorizableMock: BoardDoAuthorizable = new BoardDoAuthorizable({ + users: [{ userId: user.id, roles: [BoardRoles.EDITOR], userRoleEnum: UserRoleEnum.TEACHER }], + id: board.id, + }); + const createCardBodyParams = { + requiredEmptyElements: [ContentElementType.FILE, ContentElementType.RICH_TEXT], + }; + + boardDoAuthorizableService.findById.mockResolvedValueOnce(authorizableMock); + + return { user, board, boardId, column, card, createCardBodyParams }; + }; + + describe('deleteColumn', () => { + describe('when deleting a column', () => { + it('should call the service to find the column', async () => { + const { user, column } = setup(); + + await uc.deleteColumn(user.id, column.id); + + expect(columnService.findById).toHaveBeenCalledWith(column.id); + }); + + it('should call the service to delete the column', async () => { + const { user, column } = setup(); + columnService.findById.mockResolvedValueOnce(column); + + await uc.deleteColumn(user.id, column.id); + + expect(columnService.delete).toHaveBeenCalledWith(column); + }); + }); + }); + + describe('updateColumnTitle', () => { + describe('when updating a column title', () => { + it('should call the service to find the column', async () => { + const { user, column } = setup(); + + await uc.updateColumnTitle(user.id, column.id, 'new title'); + + expect(columnService.findById).toHaveBeenCalledWith(column.id); + }); + + it('should call the service to update the column title', async () => { + const { user, column } = setup(); + columnService.findById.mockResolvedValueOnce(column); + const newTitle = 'new title'; + + await uc.updateColumnTitle(user.id, column.id, newTitle); + + expect(columnService.updateTitle).toHaveBeenCalledWith(column, newTitle); + }); + }); + }); + + describe('createCard', () => { + describe('when creating a card', () => { + it('should call the service to create the card', async () => { + const { user, column, createCardBodyParams } = setup(); + const { requiredEmptyElements } = createCardBodyParams; + + await uc.createCard(user.id, column.id, requiredEmptyElements); + + expect(cardService.create).toHaveBeenCalledWith(column, requiredEmptyElements); + }); + + it('should return the card object', async () => { + const { user, column, card } = setup(); + cardService.create.mockResolvedValueOnce(card); + + const result = await uc.createCard(user.id, column.id); + + expect(result).toEqual(card); + }); + }); + }); + + describe('moveCard', () => { + describe('when moving a card', () => { + it('should call the service to find the card', async () => { + const { user, column, card } = setup(); + + await uc.moveCard(user.id, card.id, column.id, 5); + + expect(cardService.findById).toHaveBeenCalledWith(card.id); + }); + + it('should call the service to find the target column', async () => { + const { user, column, card } = setup(); + + await uc.moveCard(user.id, card.id, column.id, 5); + + expect(columnService.findById).toHaveBeenCalledWith(column.id); + }); + + it('should call the service to move the card', async () => { + const { user, column, card } = setup(); + cardService.findById.mockResolvedValueOnce(card); + columnService.findById.mockResolvedValueOnce(column); + + await uc.moveCard(user.id, card.id, column.id, 5); + + expect(cardService.move).toHaveBeenCalledWith(card, column, 5); + }); + }); + }); +}); diff --git a/apps/server/src/modules/board/uc/column.uc.ts b/apps/server/src/modules/board/uc/column.uc.ts new file mode 100644 index 00000000000..da8eceedbe3 --- /dev/null +++ b/apps/server/src/modules/board/uc/column.uc.ts @@ -0,0 +1,62 @@ +import { forwardRef, Inject, Injectable } from '@nestjs/common'; +import { Card, ContentElementType, EntityId } from '@shared/domain'; +import { LegacyLogger } from '@src/core/logger'; +import { AuthorizationService, Action } from '@modules/authorization'; +import { CardService, ColumnService, BoardDoAuthorizableService } from '../service'; +import { BaseUc } from './base.uc'; + +@Injectable() +export class ColumnUc extends BaseUc { + constructor( + @Inject(forwardRef(() => AuthorizationService)) + protected readonly authorizationService: AuthorizationService, + protected readonly boardDoAuthorizableService: BoardDoAuthorizableService, + private readonly cardService: CardService, + private readonly columnService: ColumnService, + private readonly logger: LegacyLogger + ) { + super(authorizationService, boardDoAuthorizableService); + this.logger.setContext(ColumnUc.name); + } + + async deleteColumn(userId: EntityId, columnId: EntityId): Promise { + this.logger.debug({ action: 'deleteColumn', userId, columnId }); + + const column = await this.columnService.findById(columnId); + await this.checkPermission(userId, column, Action.write); + + await this.columnService.delete(column); + } + + async updateColumnTitle(userId: EntityId, columnId: EntityId, title: string): Promise { + this.logger.debug({ action: 'updateColumnTitle', userId, columnId, title }); + + const column = await this.columnService.findById(columnId); + await this.checkPermission(userId, column, Action.write); + + await this.columnService.updateTitle(column, title); + } + + async createCard(userId: EntityId, columnId: EntityId, requiredEmptyElements?: ContentElementType[]): Promise { + this.logger.debug({ action: 'createCard', userId, columnId }); + + const column = await this.columnService.findById(columnId); + await this.checkPermission(userId, column, Action.read); + + const card = await this.cardService.create(column, requiredEmptyElements); + + return card; + } + + async moveCard(userId: EntityId, cardId: EntityId, targetColumnId: EntityId, targetPosition: number): Promise { + this.logger.debug({ action: 'moveCard', userId, cardId, targetColumnId, toPosition: targetPosition }); + + const card = await this.cardService.findById(cardId); + const targetColumn = await this.columnService.findById(targetColumnId); + + await this.checkPermission(userId, card, Action.write); + await this.checkPermission(userId, targetColumn, Action.write); + + await this.cardService.move(card, targetColumn, targetPosition); + } +} diff --git a/apps/server/src/modules/board/uc/element.uc.spec.ts b/apps/server/src/modules/board/uc/element.uc.spec.ts index 03124305dfa..4664148a0ed 100644 --- a/apps/server/src/modules/board/uc/element.uc.spec.ts +++ b/apps/server/src/modules/board/uc/element.uc.spec.ts @@ -2,6 +2,7 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { Test, TestingModule } from '@nestjs/testing'; import { BoardDoAuthorizable, InputFormat } from '@shared/domain'; import { + cardFactory, fileElementFactory, richTextElementFactory, setupEntities, @@ -123,6 +124,38 @@ describe(ElementUc.name, () => { }); }); + describe('deleteElement', () => { + describe('when deleting a content element', () => { + const setup = () => { + const user = userFactory.build(); + const element = richTextElementFactory.build(); + + boardDoAuthorizableService.getBoardAuthorizable.mockResolvedValue( + new BoardDoAuthorizable({ users: [], id: new ObjectId().toHexString() }) + ); + + return { user, element }; + }; + + it('should call the service to find the element', async () => { + const { user, element } = setup(); + + await uc.deleteElement(user.id, element.id); + + expect(elementService.findById).toHaveBeenCalledWith(element.id); + }); + + it('should call the service to delete the element', async () => { + const { user, element } = setup(); + elementService.findById.mockResolvedValueOnce(element); + + await uc.deleteElement(user.id, element.id); + + expect(elementService.delete).toHaveBeenCalledWith(element); + }); + }); + }); + describe('createSubmissionItem', () => { describe('with non SubmissionContainerElement parent', () => { const setup = () => { diff --git a/apps/server/src/modules/board/uc/element.uc.ts b/apps/server/src/modules/board/uc/element.uc.ts index e2e01d4f07b..d094e758fd5 100644 --- a/apps/server/src/modules/board/uc/element.uc.ts +++ b/apps/server/src/modules/board/uc/element.uc.ts @@ -1,5 +1,6 @@ import { ForbiddenException, forwardRef, Inject, Injectable, UnprocessableEntityException } from '@nestjs/common'; import { + AnyBoardDo, AnyContentElementDo, EntityId, isSubmissionContainerElement, @@ -33,18 +34,29 @@ export class ElementUc extends BaseUc { elementId: EntityId, content: AnyElementContentBody ): Promise { - let element = await this.elementService.findById(elementId); + const element = await this.getElementWithWritePermission(userId, elementId); - const parent = await this.elementService.findParentOfId(elementId); + await this.elementService.update(element, content); + return element; + } + + async deleteElement(userId: EntityId, elementId: EntityId): Promise { + const element = await this.getElementWithWritePermission(userId, elementId); + + await this.elementService.delete(element); + } + + private async getElementWithWritePermission(userId: EntityId, elementId: EntityId): Promise { + const element = await this.elementService.findById(elementId); + + const parent: AnyBoardDo = await this.elementService.findParentOfId(elementId); - // TODO refactor if (isSubmissionItem(parent)) { - await this.checkSubmissionItemEditPermission(userId, parent); + await this.checkSubmissionItemWritePermission(userId, parent); } else { await this.checkPermission(userId, element, Action.write); } - element = await this.elementService.update(element, content); return element; } diff --git a/apps/server/src/modules/board/uc/index.ts b/apps/server/src/modules/board/uc/index.ts index 931629fcfed..b818c6753c0 100644 --- a/apps/server/src/modules/board/uc/index.ts +++ b/apps/server/src/modules/board/uc/index.ts @@ -1,2 +1,6 @@ +export * from './base.uc'; export * from './board.uc'; export * from './card.uc'; +export * from './column.uc'; +export * from './element.uc'; +export * from './submission-item.uc'; diff --git a/apps/server/src/modules/board/uc/submission-item.uc.ts b/apps/server/src/modules/board/uc/submission-item.uc.ts index 6d1154a099a..3fb87f2b5dc 100644 --- a/apps/server/src/modules/board/uc/submission-item.uc.ts +++ b/apps/server/src/modules/board/uc/submission-item.uc.ts @@ -67,7 +67,7 @@ export class SubmissionItemUc extends BaseUc { completed: boolean ): Promise { const submissionItem = await this.submissionItemService.findById(submissionItemId); - await this.checkSubmissionItemEditPermission(userId, submissionItem); + await this.checkSubmissionItemWritePermission(userId, submissionItem); await this.submissionItemService.update(submissionItem, completed); return submissionItem; @@ -84,7 +84,7 @@ export class SubmissionItemUc extends BaseUc { const submissionItem = await this.submissionItemService.findById(submissionItemId); - await this.checkSubmissionItemEditPermission(userId, submissionItem); + await this.checkSubmissionItemWritePermission(userId, submissionItem); const element = await this.elementService.create(submissionItem, type); diff --git a/apps/server/src/shared/domain/domainobject/board/types/board-external-reference.ts b/apps/server/src/shared/domain/domainobject/board/types/board-external-reference.ts index 002804ccb30..5d16a6853d9 100644 --- a/apps/server/src/shared/domain/domainobject/board/types/board-external-reference.ts +++ b/apps/server/src/shared/domain/domainobject/board/types/board-external-reference.ts @@ -1,7 +1,7 @@ import { EntityId } from '@shared/domain/types'; export enum BoardExternalReferenceType { - Course = 'course', + 'Course' = 'course', } export interface BoardExternalReference { From 04bca1418cc7cd04c002ed7c5ad76b76352ad626 Mon Sep 17 00:00:00 2001 From: virgilchiriac Date: Mon, 23 Oct 2023 09:47:37 +0200 Subject: [PATCH 07/16] fix linter --- .../src/modules/board/uc/card.uc.spec.ts | 84 ++++++++++++++----- .../src/modules/board/uc/element.uc.spec.ts | 4 +- 2 files changed, 63 insertions(+), 25 deletions(-) diff --git a/apps/server/src/modules/board/uc/card.uc.spec.ts b/apps/server/src/modules/board/uc/card.uc.spec.ts index 7ed23a12ac4..7df87747b9c 100644 --- a/apps/server/src/modules/board/uc/card.uc.spec.ts +++ b/apps/server/src/modules/board/uc/card.uc.spec.ts @@ -6,8 +6,7 @@ import { cardFactory, richTextElementFactory } from '@shared/testing/factory/dom import { LegacyLogger } from '@src/core/logger'; import { AuthorizationService } from '@modules/authorization'; import { ObjectId } from 'bson'; -import { BoardDoAuthorizableService, ContentElementService } from '../service'; -import { CardService } from '../service/card.service'; +import { BoardDoAuthorizableService, ContentElementService, CardService } from '../service'; import { CardUc } from './card.uc'; describe(CardUc.name, () => { @@ -95,29 +94,28 @@ describe(CardUc.name, () => { }); }); - const setup = () => { - jest.clearAllMocks(); - const user = userFactory.buildWithId(); - const board = columnBoardFactory.build(); - const boardId = board.id; - const column = columnFactory.build(); - const card = cardFactory.build(); - authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); - - const authorizableMock: BoardDoAuthorizable = new BoardDoAuthorizable({ - users: [{ userId: user.id, roles: [BoardRoles.EDITOR], userRoleEnum: UserRoleEnum.TEACHER }], - id: board.id, - }); - const createCardBodyParams = { - requiredEmptyElements: [ContentElementType.FILE, ContentElementType.RICH_TEXT], - }; + describe('deleteCard', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const board = columnBoardFactory.build(); + const boardId = board.id; + const column = columnFactory.build(); + const card = cardFactory.build(); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + + const authorizableMock: BoardDoAuthorizable = new BoardDoAuthorizable({ + users: [{ userId: user.id, roles: [BoardRoles.EDITOR], userRoleEnum: UserRoleEnum.TEACHER }], + id: board.id, + }); + const createCardBodyParams = { + requiredEmptyElements: [ContentElementType.FILE, ContentElementType.RICH_TEXT], + }; - boardDoAuthorizableService.findById.mockResolvedValueOnce(authorizableMock); + boardDoAuthorizableService.findById.mockResolvedValueOnce(authorizableMock); - return { user, board, boardId, column, card, createCardBodyParams }; - }; + return { user, board, boardId, column, card, createCardBodyParams }; + }; - describe('deleteCard', () => { describe('when deleting a card', () => { it('should call the service to find the card', async () => { const { user, card } = setup(); @@ -139,6 +137,27 @@ describe(CardUc.name, () => { }); describe('updateCardHeight', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const board = columnBoardFactory.build(); + const boardId = board.id; + const column = columnFactory.build(); + const card = cardFactory.build(); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + + const authorizableMock: BoardDoAuthorizable = new BoardDoAuthorizable({ + users: [{ userId: user.id, roles: [BoardRoles.EDITOR], userRoleEnum: UserRoleEnum.TEACHER }], + id: board.id, + }); + const createCardBodyParams = { + requiredEmptyElements: [ContentElementType.FILE, ContentElementType.RICH_TEXT], + }; + + boardDoAuthorizableService.findById.mockResolvedValueOnce(authorizableMock); + + return { user, board, boardId, column, card, createCardBodyParams }; + }; + describe('when updating a card height', () => { it('should call the service to find the card', async () => { const { user, card } = setup(); @@ -171,6 +190,27 @@ describe(CardUc.name, () => { }); describe('updateCardTitle', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const board = columnBoardFactory.build(); + const boardId = board.id; + const column = columnFactory.build(); + const card = cardFactory.build(); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + + const authorizableMock: BoardDoAuthorizable = new BoardDoAuthorizable({ + users: [{ userId: user.id, roles: [BoardRoles.EDITOR], userRoleEnum: UserRoleEnum.TEACHER }], + id: board.id, + }); + const createCardBodyParams = { + requiredEmptyElements: [ContentElementType.FILE, ContentElementType.RICH_TEXT], + }; + + boardDoAuthorizableService.findById.mockResolvedValueOnce(authorizableMock); + + return { user, board, boardId, column, card, createCardBodyParams }; + }; + describe('when updating a card title', () => { it('should call the service to find the card', async () => { const { user, card } = setup(); diff --git a/apps/server/src/modules/board/uc/element.uc.spec.ts b/apps/server/src/modules/board/uc/element.uc.spec.ts index 4664148a0ed..8945b9612e8 100644 --- a/apps/server/src/modules/board/uc/element.uc.spec.ts +++ b/apps/server/src/modules/board/uc/element.uc.spec.ts @@ -2,7 +2,6 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { Test, TestingModule } from '@nestjs/testing'; import { BoardDoAuthorizable, InputFormat } from '@shared/domain'; import { - cardFactory, fileElementFactory, richTextElementFactory, setupEntities, @@ -13,8 +12,7 @@ import { import { Logger } from '@src/core/logger'; import { AuthorizationService } from '@modules/authorization'; import { ObjectId } from 'bson'; -import { BoardDoAuthorizableService, ContentElementService } from '../service'; -import { SubmissionItemService } from '../service/submission-item.service'; +import { BoardDoAuthorizableService, ContentElementService, SubmissionItemService } from '../service'; import { ElementUc } from './element.uc'; describe(ElementUc.name, () => { From 2b8cfd8050f7137616949a14d534caeca41b719e Mon Sep 17 00:00:00 2001 From: virgilchiriac Date: Mon, 23 Oct 2023 17:04:49 +0200 Subject: [PATCH 08/16] fix weird path making tests fail --- apps/server/src/modules/learnroom/service/board-copy.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/server/src/modules/learnroom/service/board-copy.service.ts b/apps/server/src/modules/learnroom/service/board-copy.service.ts index f695dfd2c05..e9c1734f574 100644 --- a/apps/server/src/modules/learnroom/service/board-copy.service.ts +++ b/apps/server/src/modules/learnroom/service/board-copy.service.ts @@ -3,7 +3,6 @@ import { Board, BoardElement, BoardElementType, - BoardExternalReferenceType, ColumnBoard, ColumnboardBoardElement, ColumnBoardTarget, @@ -17,6 +16,7 @@ import { TaskBoardElement, User, } from '@shared/domain'; +import { BoardExternalReferenceType } from '@shared/domain/domainobject/board/types'; import { BoardRepo } from '@shared/repo'; import { LegacyLogger } from '@src/core/logger'; import { ColumnBoardCopyService } from '@modules/board/service/column-board-copy.service'; From e2ec1f33b7e0f11dffae173027e0c014af4dea6b Mon Sep 17 00:00:00 2001 From: virgilchiriac Date: Mon, 23 Oct 2023 17:49:55 +0200 Subject: [PATCH 09/16] restrict returned response type --- .../board/controller/board-submission.controller.ts | 9 +++++++-- .../dto/element/any-content-element.response.ts | 6 ++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/apps/server/src/modules/board/controller/board-submission.controller.ts b/apps/server/src/modules/board/controller/board-submission.controller.ts index 1d68ef54d43..1dd46440f21 100644 --- a/apps/server/src/modules/board/controller/board-submission.controller.ts +++ b/apps/server/src/modules/board/controller/board-submission.controller.ts @@ -8,6 +8,7 @@ import { Param, Patch, Post, + UnprocessableEntityException, } from '@nestjs/common'; import { ApiExtraModels, ApiOperation, ApiResponse, ApiTags, getSchemaPath } from '@nestjs/swagger'; import { ApiValidationError } from '@shared/common'; @@ -17,9 +18,10 @@ import { CardUc } from '../uc'; import { ElementUc } from '../uc/element.uc'; import { SubmissionItemUc } from '../uc/submission-item.uc'; import { - AnyContentElementResponse, CreateContentElementBodyParams, FileElementResponse, + isFileElementResponse, + isRichTextElementResponse, RichTextElementResponse, SubmissionContainerUrlParams, SubmissionItemUrlParams, @@ -91,10 +93,13 @@ export class BoardSubmissionController { @Param() urlParams: SubmissionItemUrlParams, @Body() bodyParams: CreateContentElementBodyParams, @CurrentUser() currentUser: ICurrentUser - ): Promise { + ): Promise { const { type } = bodyParams; const element = await this.submissionItemUc.createElement(currentUser.userId, urlParams.submissionItemId, type); const response = ContentElementResponseFactory.mapToResponse(element); + if (!isFileElementResponse(response) && !isRichTextElementResponse(response)) { + throw new UnprocessableEntityException(); + } return response; } diff --git a/apps/server/src/modules/board/controller/dto/element/any-content-element.response.ts b/apps/server/src/modules/board/controller/dto/element/any-content-element.response.ts index 18415d172fa..84681de7691 100644 --- a/apps/server/src/modules/board/controller/dto/element/any-content-element.response.ts +++ b/apps/server/src/modules/board/controller/dto/element/any-content-element.response.ts @@ -10,3 +10,9 @@ export type AnyContentElementResponse = | RichTextElementResponse | SubmissionContainerElementResponse | ExternalToolElementResponse; + +export const isFileElementResponse = (element: AnyContentElementResponse): element is FileElementResponse => + element instanceof FileElementResponse; + +export const isRichTextElementResponse = (element: AnyContentElementResponse): element is RichTextElementResponse => + element instanceof RichTextElementResponse; From 9f135421e753aacc0c1a041c7a72573d7d4c9fde Mon Sep 17 00:00:00 2001 From: virgilchiriac Date: Mon, 23 Oct 2023 18:52:49 +0200 Subject: [PATCH 10/16] add uc unit tests --- .../board/uc/submission-item.uc.spec.ts | 84 +++++++++++++++++-- 1 file changed, 78 insertions(+), 6 deletions(-) diff --git a/apps/server/src/modules/board/uc/submission-item.uc.spec.ts b/apps/server/src/modules/board/uc/submission-item.uc.spec.ts index e1b3aa1e536..122caae2bf6 100644 --- a/apps/server/src/modules/board/uc/submission-item.uc.spec.ts +++ b/apps/server/src/modules/board/uc/submission-item.uc.spec.ts @@ -1,18 +1,20 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { Test, TestingModule } from '@nestjs/testing'; -import { BoardDoAuthorizable, BoardRoles, UserRoleEnum } from '@shared/domain'; +import { BoardDoAuthorizable, BoardRoles, ContentElementType, UserRoleEnum } from '@shared/domain'; import { fileElementFactory, + richTextElementFactory, setupEntities, submissionContainerElementFactory, submissionItemFactory, userFactory, } from '@shared/testing'; import { Logger } from '@src/core/logger'; -import { AuthorizationService, Action } from '@modules/authorization'; -import { NotFoundException } from '@nestjs/common'; +import { Action, AuthorizationService } from '@modules/authorization'; +import { BadRequestException, ForbiddenException, NotFoundException } from '@nestjs/common'; import { BoardDoAuthorizableService, ContentElementService, SubmissionItemService } from '../service'; import { SubmissionItemUc } from './submission-item.uc'; +import { isFileElementResponse, isRichTextElementResponse } from '../controller/dto'; describe(SubmissionItemUc.name, () => { let module: TestingModule; @@ -235,16 +237,86 @@ describe(SubmissionItemUc.name, () => { const context = { action: Action.read, requiredPermissions: [] }; expect(authorizationService.checkPermission).toBeCalledWith(user, boardDoAuthorizable, context); }); - it('should throw if user is not creator of submission', async () => { + it('should throw if user is not creator of submission item', async () => { const user2 = userFactory.buildWithId(); const { submissionItem } = setup(); - await expect(uc.updateSubmissionItem(user2.id, submissionItem.id, false)).rejects.toThrow(); + await expect(uc.updateSubmissionItem(user2.id, submissionItem.id, false)).rejects.toThrow( + new ForbiddenException() + ); }); - it('should call service to update element', async () => { + it('should call service to update submission item', async () => { const { submissionItem, user } = setup(); await uc.updateSubmissionItem(user.id, submissionItem.id, false); expect(submissionItemService.update).toHaveBeenCalledWith(submissionItem, false); }); }); + + // write tests + describe('createElement', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const submissionItem = submissionItemFactory.build({ + userId: user.id, + }); + + submissionItemService.findById.mockResolvedValue(submissionItem); + + const element = richTextElementFactory.build(); + elementService.create.mockResolvedValueOnce(element); + + boardDoAuthorizableService.getBoardAuthorizable.mockResolvedValue( + new BoardDoAuthorizable({ + users: [{ userId: user.id, roles: [BoardRoles.READER], userRoleEnum: UserRoleEnum.STUDENT }], + id: submissionItem.id, + }) + ); + + return { submissionItem, user }; + }; + + it('should call service to find the submission item ', async () => { + const { submissionItem, user } = setup(); + await uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT); + expect(submissionItemService.findById).toHaveBeenCalledWith(submissionItem.id); + }); + + it('should authorize', async () => { + const { submissionItem, user } = setup(); + + const boardDoAuthorizable = await boardDoAuthorizableService.getBoardAuthorizable(submissionItem); + + await uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT); + const context = { action: Action.read, requiredPermissions: [] }; + expect(authorizationService.checkPermission).toBeCalledWith(user, boardDoAuthorizable, context); + }); + + it('should throw if user is not creator of submission item', async () => { + const user2 = userFactory.buildWithId(); + const { submissionItem } = setup(); + + await expect(uc.createElement(user2.id, submissionItem.id, ContentElementType.RICH_TEXT)).rejects.toThrow( + new ForbiddenException() + ); + }); + + it('should throw if type is not file or rich text', async () => { + const { submissionItem, user } = setup(); + await expect(uc.createElement(user.id, submissionItem.id, ContentElementType.LINK)).rejects.toThrow( + new BadRequestException() + ); + }); + + it('should call service to create element', async () => { + const { submissionItem, user } = setup(); + await uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT); + expect(elementService.create).toHaveBeenCalledWith(submissionItem, ContentElementType.RICH_TEXT); + }); + + it('should return element', async () => { + const { submissionItem, user } = setup(); + const element = await uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT); + expect(element).toBeDefined(); + }); + }); }); From 73595f420c442eaa5469b3603aeeb91d0b007881 Mon Sep 17 00:00:00 2001 From: virgilchiriac Date: Tue, 24 Oct 2023 02:06:26 +0200 Subject: [PATCH 11/16] add tests --- .../content-element-create.api.spec.ts | 180 ++++++++++++------ .../service/content-element.service.spec.ts | 37 ++++ .../src/modules/board/uc/element.uc.spec.ts | 70 ++++++- .../board/uc/submission-item.uc.spec.ts | 37 +++- 4 files changed, 248 insertions(+), 76 deletions(-) diff --git a/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts b/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts index 57ef692ace1..a021fcc477f 100644 --- a/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts +++ b/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts @@ -37,101 +37,161 @@ describe(`content element create (api)`, () => { }); describe('with valid user', () => { - const setup = async () => { - await cleanupCollections(em); + describe('when parent is a card', () => { + const setup = async () => { + await cleanupCollections(em); - const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); - const course = courseFactory.build({ teachers: [teacherUser] }); - await em.persistAndFlush([teacherAccount, teacherUser, course]); + const course = courseFactory.build({ teachers: [teacherUser] }); + await em.persistAndFlush([teacherAccount, teacherUser, course]); - const columnBoardNode = columnBoardNodeFactory.buildWithId({ - context: { id: course.id, type: BoardExternalReferenceType.Course }, + const columnBoardNode = columnBoardNodeFactory.buildWithId({ + context: { id: course.id, type: BoardExternalReferenceType.Course }, + }); + const columnNode = columnNodeFactory.buildWithId({ parent: columnBoardNode }); + const cardNode = cardNodeFactory.buildWithId({ parent: columnNode }); + + await em.persistAndFlush([columnBoardNode, columnNode, cardNode]); + em.clear(); + + const loggedInClient = await testApiClient.login(teacherAccount); + + return { loggedInClient, columnBoardNode, columnNode, cardNode }; + }; + + it('should return status 201', async () => { + const { loggedInClient, cardNode } = await setup(); + + const response = await loggedInClient.post(`${cardNode.id}/elements`, { type: ContentElementType.RICH_TEXT }); + + expect(response.statusCode).toEqual(201); }); - const columnNode = columnNodeFactory.buildWithId({ parent: columnBoardNode }); - const cardNode = cardNodeFactory.buildWithId({ parent: columnNode }); - await em.persistAndFlush([columnBoardNode, columnNode, cardNode]); - em.clear(); + it('should return the created content element of type RICH_TEXT', async () => { + const { loggedInClient, cardNode } = await setup(); - const loggedInClient = await testApiClient.login(teacherAccount); + const response = await loggedInClient.post(`${cardNode.id}/elements`, { type: ContentElementType.RICH_TEXT }); - return { loggedInClient, columnBoardNode, columnNode, cardNode }; - }; + expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.RICH_TEXT); + }); - it('should return status 201', async () => { - const { loggedInClient, cardNode } = await setup(); + it('should return the created content element of type FILE', async () => { + const { loggedInClient, cardNode } = await setup(); - const response = await loggedInClient.post(`${cardNode.id}/elements`, { type: ContentElementType.RICH_TEXT }); + const response = await loggedInClient.post(`${cardNode.id}/elements`, { type: ContentElementType.FILE }); - expect(response.statusCode).toEqual(201); - }); + expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.FILE); + }); - it('should return the created content element of type RICH_TEXT', async () => { - const { loggedInClient, cardNode } = await setup(); + it('should return the created content element of type EXTERNAL_TOOL', async () => { + const { loggedInClient, cardNode } = await setup(); - const response = await loggedInClient.post(`${cardNode.id}/elements`, { type: ContentElementType.RICH_TEXT }); + const response = await loggedInClient.post(`${cardNode.id}/elements`, { + type: ContentElementType.EXTERNAL_TOOL, + }); - expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.RICH_TEXT); - }); + expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.EXTERNAL_TOOL); + }); - it('should return the created content element of type FILE', async () => { - const { loggedInClient, cardNode } = await setup(); + it('should return the created content element of type SUBMISSION_CONTAINER with dueDate set to null', async () => { + const { loggedInClient, cardNode } = await setup(); - const response = await loggedInClient.post(`${cardNode.id}/elements`, { type: ContentElementType.FILE }); + const response = await loggedInClient.post(`${cardNode.id}/elements`, { + type: ContentElementType.SUBMISSION_CONTAINER, + }); - expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.FILE); - }); + expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.SUBMISSION_CONTAINER); + expect((response.body as SubmissionContainerElementResponse).content.dueDate).toBeNull(); + }); - it('should return the created content element of type EXTERNAL_TOOL', async () => { - const { loggedInClient, cardNode } = await setup(); + it('should actually create the content element', async () => { + const { loggedInClient, cardNode } = await setup(); + const response = await loggedInClient.post(`${cardNode.id}/elements`, { type: ContentElementType.RICH_TEXT }); - const response = await loggedInClient.post(`${cardNode.id}/elements`, { type: ContentElementType.EXTERNAL_TOOL }); + const elementId = (response.body as AnyContentElementResponse).id; - expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.EXTERNAL_TOOL); - }); + const result = await em.findOneOrFail(RichTextElementNode, elementId); + expect(result.id).toEqual(elementId); + }); + + it('should throw an error if toPosition param is not a number', async () => { + const { loggedInClient, cardNode } = await setup(); - it('should return the created content element of type SUBMISSION_CONTAINER with dueDate set to null', async () => { - const { loggedInClient, cardNode } = await setup(); + const response = await loggedInClient.post(`${cardNode.id}/elements`, { + type: ContentElementType.RICH_TEXT, + toPosition: 'not a number', + }); - const response = await loggedInClient.post(`${cardNode.id}/elements`, { - type: ContentElementType.SUBMISSION_CONTAINER, + expect(response.statusCode).toEqual(400); }); - expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.SUBMISSION_CONTAINER); - expect((response.body as SubmissionContainerElementResponse).content.dueDate).toBeNull(); + it('should throw an error if toPosition param is a negative number', async () => { + const { loggedInClient, cardNode } = await setup(); + + const response = await loggedInClient.post(`${cardNode.id}/elements`, { + type: ContentElementType.RICH_TEXT, + toPosition: -1, + }); + + expect(response.statusCode).toEqual(400); + }); }); - it('should actually create the content element', async () => { - const { loggedInClient, cardNode } = await setup(); - const response = await loggedInClient.post(`${cardNode.id}/elements`, { type: ContentElementType.RICH_TEXT }); + describe('when parent is a submission item', () => { + const setup = async () => { + await cleanupCollections(em); + + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); - const elementId = (response.body as AnyContentElementResponse).id; + const course = courseFactory.build({ teachers: [teacherUser] }); + await em.persistAndFlush([teacherAccount, teacherUser, course]); - const result = await em.findOneOrFail(RichTextElementNode, elementId); - expect(result.id).toEqual(elementId); - }); + const columnBoardNode = columnBoardNodeFactory.buildWithId({ + context: { id: course.id, type: BoardExternalReferenceType.Course }, + }); + const columnNode = columnNodeFactory.buildWithId({ parent: columnBoardNode }); + const cardNode = cardNodeFactory.buildWithId({ parent: columnNode }); + const submissionContainerNode = columnNodeFactory.buildWithId({ parent: cardNode }); + const submissionItemNode = cardNodeFactory.buildWithId({ parent: submissionContainerNode }); + + await em.persistAndFlush([columnBoardNode, columnNode, cardNode, submissionContainerNode, submissionItemNode]); + em.clear(); + + const loggedInClient = await testApiClient.login(teacherAccount); + + return { loggedInClient, cardNode, submissionItemNode }; + }; - it('should throw an error if toPosition param is not a number', async () => { - const { loggedInClient, cardNode } = await setup(); + it('should return status 201', async () => { + const { loggedInClient, submissionItemNode } = await setup(); - const response = await loggedInClient.post(`${cardNode.id}/elements`, { - type: ContentElementType.RICH_TEXT, - toPosition: 'not a number', + const response = await loggedInClient.post(`${submissionItemNode.id}/elements`, { + type: ContentElementType.RICH_TEXT, + }); + + expect(response.statusCode).toEqual(201); }); - expect(response.statusCode).toEqual(400); - }); + it('should return the created content element of type RICH_TEXT', async () => { + const { loggedInClient, submissionItemNode } = await setup(); - it('should throw an error if toPosition param is a negative number', async () => { - const { loggedInClient, cardNode } = await setup(); + const response = await loggedInClient.post(`${submissionItemNode.id}/elements`, { + type: ContentElementType.RICH_TEXT, + }); - const response = await loggedInClient.post(`${cardNode.id}/elements`, { - type: ContentElementType.RICH_TEXT, - toPosition: -1, + expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.RICH_TEXT); }); - expect(response.statusCode).toEqual(400); + it('should return the created content element of type FILE', async () => { + const { loggedInClient, submissionItemNode } = await setup(); + + const response = await loggedInClient.post(`${submissionItemNode.id}/elements`, { + type: ContentElementType.FILE, + }); + + expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.FILE); + }); }); }); diff --git a/apps/server/src/modules/board/service/content-element.service.spec.ts b/apps/server/src/modules/board/service/content-element.service.spec.ts index b1326450089..90f3a73aa21 100644 --- a/apps/server/src/modules/board/service/content-element.service.spec.ts +++ b/apps/server/src/modules/board/service/content-element.service.spec.ts @@ -123,6 +123,43 @@ describe(ContentElementService.name, () => { }); }); + describe('findParentOfId', () => { + describe('when parent is a vaid node', () => { + const setup = () => { + const card = cardFactory.build(); + const element = richTextElementFactory.build(); + + return { element, card }; + }; + + it('should call the repo', async () => { + const { element, card } = setup(); + boardDoRepo.findParentOfId.mockResolvedValueOnce(card); + + await service.findParentOfId(element.id); + + expect(boardDoRepo.findParentOfId).toHaveBeenCalledWith(element.id); + }); + + it('should throw NotFoundException', async () => { + const { element } = setup(); + + boardDoRepo.findParentOfId.mockResolvedValue(undefined); + + await expect(service.findParentOfId(element.id)).rejects.toThrowError(NotFoundException); + }); + + it('should return the parent', async () => { + const { element, card } = setup(); + boardDoRepo.findParentOfId.mockResolvedValueOnce(card); + + const result = await service.findParentOfId(element.id); + + expect(result).toEqual(card); + }); + }); + }); + describe('create', () => { describe('when creating a content element of type', () => { const setup = () => { diff --git a/apps/server/src/modules/board/uc/element.uc.spec.ts b/apps/server/src/modules/board/uc/element.uc.spec.ts index 8945b9612e8..e17c20bb067 100644 --- a/apps/server/src/modules/board/uc/element.uc.spec.ts +++ b/apps/server/src/modules/board/uc/element.uc.spec.ts @@ -10,8 +10,9 @@ import { userFactory, } from '@shared/testing'; import { Logger } from '@src/core/logger'; -import { AuthorizationService } from '@modules/authorization'; +import { AuthorizationService, Action } from '@modules/authorization'; import { ObjectId } from 'bson'; +import { ForbiddenException } from '@nestjs/common'; import { BoardDoAuthorizableService, ContentElementService, SubmissionItemService } from '../service'; import { ElementUc } from './element.uc'; @@ -123,6 +124,61 @@ describe(ElementUc.name, () => { }); describe('deleteElement', () => { + describe('when deleting an element which has a submission item parent', () => { + const setup = () => { + const user = userFactory.build(); + const element = richTextElementFactory.build(); + const submissionItem = submissionItemFactory.build({ userId: user.id }); + + boardDoAuthorizableService.getBoardAuthorizable.mockResolvedValue( + new BoardDoAuthorizable({ users: [], id: new ObjectId().toHexString() }) + ); + + elementService.findById.mockResolvedValueOnce(element); + return { element, user, submissionItem }; + }; + + it('should call the service to find the element', async () => { + const { element, user } = setup(); + await uc.deleteElement(user.id, element.id); + + expect(elementService.findById).toHaveBeenCalledWith(element.id); + }); + + it('should call the service to find the parent of the element', async () => { + const { element, user } = setup(); + await uc.deleteElement(user.id, element.id); + + expect(elementService.findParentOfId).toHaveBeenCalledWith(element.id); + }); + + it('should throw if the user is not the owner of the submission item', async () => { + const { element, user } = setup(); + const otherSubmissionItem = submissionItemFactory.build({ userId: new ObjectId().toHexString() }); + elementService.findParentOfId.mockResolvedValueOnce(otherSubmissionItem); + + await expect(uc.deleteElement(user.id, element.id)).rejects.toThrow(new ForbiddenException()); + }); + + it('should authorize the user to delete the element', async () => { + const { element, user, submissionItem } = setup(); + elementService.findParentOfId.mockResolvedValueOnce(submissionItem); + const boardDoAuthorizable = await boardDoAuthorizableService.getBoardAuthorizable(submissionItem); + const context = { action: Action.read, requiredPermissions: [] }; + await uc.deleteElement(user.id, element.id); + + expect(authorizationService.checkPermission).toHaveBeenCalledWith(user, boardDoAuthorizable, context); + }); + + it('should call the service to delete the element', async () => { + const { user, element, submissionItem } = setup(); + elementService.findParentOfId.mockResolvedValueOnce(submissionItem); + + await uc.deleteElement(user.id, element.id); + + expect(elementService.delete).toHaveBeenCalledWith(element); + }); + }); describe('when deleting a content element', () => { const setup = () => { const user = userFactory.build(); @@ -160,9 +216,9 @@ describe(ElementUc.name, () => { const user = userFactory.build(); const fileElement = fileElementFactory.build(); - const elementSpy = elementService.findById.mockResolvedValue(fileElement); + elementService.findById.mockResolvedValue(fileElement); - return { fileElement, user, elementSpy }; + return { fileElement, user }; }; it('should throw', async () => { @@ -181,9 +237,9 @@ describe(ElementUc.name, () => { const submissionContainer = submissionContainerElementFactory.build({ children: [fileElement] }); - const elementSpy = elementService.findById.mockResolvedValue(submissionContainer); + elementService.findById.mockResolvedValue(submissionContainer); - return { submissionContainer, fileElement, user, elementSpy }; + return { submissionContainer, fileElement, user }; }; it('should throw', async () => { @@ -202,9 +258,9 @@ describe(ElementUc.name, () => { const submissionItem = submissionItemFactory.build({ userId: user.id }); const submissionContainer = submissionContainerElementFactory.build({ children: [submissionItem] }); - const elementSpy = elementService.findById.mockResolvedValue(submissionContainer); + elementService.findById.mockResolvedValue(submissionContainer); - return { submissionContainer, submissionItem, user, elementSpy }; + return { submissionContainer, submissionItem, user }; }; it('should throw', async () => { diff --git a/apps/server/src/modules/board/uc/submission-item.uc.spec.ts b/apps/server/src/modules/board/uc/submission-item.uc.spec.ts index 122caae2bf6..93fa11fd48b 100644 --- a/apps/server/src/modules/board/uc/submission-item.uc.spec.ts +++ b/apps/server/src/modules/board/uc/submission-item.uc.spec.ts @@ -11,10 +11,14 @@ import { } from '@shared/testing'; import { Logger } from '@src/core/logger'; import { Action, AuthorizationService } from '@modules/authorization'; -import { BadRequestException, ForbiddenException, NotFoundException } from '@nestjs/common'; +import { + BadRequestException, + ForbiddenException, + NotFoundException, + UnprocessableEntityException, +} from '@nestjs/common'; import { BoardDoAuthorizableService, ContentElementService, SubmissionItemService } from '../service'; import { SubmissionItemUc } from './submission-item.uc'; -import { isFileElementResponse, isRichTextElementResponse } from '../controller/dto'; describe(SubmissionItemUc.name, () => { let module: TestingModule; @@ -263,7 +267,6 @@ describe(SubmissionItemUc.name, () => { submissionItemService.findById.mockResolvedValue(submissionItem); const element = richTextElementFactory.build(); - elementService.create.mockResolvedValueOnce(element); boardDoAuthorizableService.getBoardAuthorizable.mockResolvedValue( new BoardDoAuthorizable({ @@ -272,17 +275,20 @@ describe(SubmissionItemUc.name, () => { }) ); - return { submissionItem, user }; + return { element, submissionItem, user }; }; it('should call service to find the submission item ', async () => { - const { submissionItem, user } = setup(); + const { element, submissionItem, user } = setup(); + elementService.create.mockResolvedValueOnce(element); + await uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT); expect(submissionItemService.findById).toHaveBeenCalledWith(submissionItem.id); }); it('should authorize', async () => { - const { submissionItem, user } = setup(); + const { element, submissionItem, user } = setup(); + elementService.create.mockResolvedValueOnce(element); const boardDoAuthorizable = await boardDoAuthorizableService.getBoardAuthorizable(submissionItem); @@ -308,15 +314,28 @@ describe(SubmissionItemUc.name, () => { }); it('should call service to create element', async () => { - const { submissionItem, user } = setup(); + const { element, submissionItem, user } = setup(); + elementService.create.mockResolvedValueOnce(element); + await uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT); expect(elementService.create).toHaveBeenCalledWith(submissionItem, ContentElementType.RICH_TEXT); }); it('should return element', async () => { + const { element, submissionItem, user } = setup(); + elementService.create.mockResolvedValueOnce(element); + + const returnedElement = await uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT); + expect(returnedElement).toEqual(element); + }); + + it('should throw if element is not file or rich text', async () => { const { submissionItem, user } = setup(); - const element = await uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT); - expect(element).toBeDefined(); + const otherElement = submissionContainerElementFactory.build(); + elementService.create.mockResolvedValueOnce(otherElement); + await expect(uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT)).rejects.toThrow( + new UnprocessableEntityException() + ); }); }); }); From ace6d5c03e69f75a7b903ea8786e1f9ff478838b Mon Sep 17 00:00:00 2001 From: virgilchiriac Date: Tue, 24 Oct 2023 17:19:22 +0200 Subject: [PATCH 12/16] fix api test --- .../content-element-create.api.spec.ts | 40 ++++++++++++++++--- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts b/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts index a021fcc477f..d53f5e4b694 100644 --- a/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts +++ b/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts @@ -8,6 +8,10 @@ import { columnBoardNodeFactory, columnNodeFactory, courseFactory, + submissionContainerElementFactory, + submissionContainerElementNodeFactory, + submissionItemFactory, + submissionItemNodeFactory, TestApiClient, UserAndAccountTestFactory, } from '@shared/testing'; @@ -15,11 +19,13 @@ import { ServerTestModule } from '@modules/server/server.module'; import { AnyContentElementResponse, SubmissionContainerElementResponse } from '../dto'; const baseRouteName = '/cards'; +const submissionRouteName = '/board-submissions'; describe(`content element create (api)`, () => { let app: INestApplication; let em: EntityManager; let testApiClient: TestApiClient; + let testApiClientSubmission: TestApiClient; beforeAll(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -30,6 +36,7 @@ describe(`content element create (api)`, () => { await app.init(); em = module.get(EntityManager); testApiClient = new TestApiClient(app, baseRouteName); + testApiClientSubmission = new TestApiClient(app, submissionRouteName); }); afterAll(async () => { @@ -143,22 +150,33 @@ describe(`content element create (api)`, () => { await cleanupCollections(em); const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); + const { studentAccount, studentUser } = UserAndAccountTestFactory.buildStudent(); - const course = courseFactory.build({ teachers: [teacherUser] }); - await em.persistAndFlush([teacherAccount, teacherUser, course]); + const course = courseFactory.build({ teachers: [teacherUser], students: [studentUser] }); + + await em.persistAndFlush([teacherAccount, teacherUser, studentAccount, studentUser, course]); const columnBoardNode = columnBoardNodeFactory.buildWithId({ context: { id: course.id, type: BoardExternalReferenceType.Course }, }); const columnNode = columnNodeFactory.buildWithId({ parent: columnBoardNode }); const cardNode = cardNodeFactory.buildWithId({ parent: columnNode }); - const submissionContainerNode = columnNodeFactory.buildWithId({ parent: cardNode }); - const submissionItemNode = cardNodeFactory.buildWithId({ parent: submissionContainerNode }); + const submissionElementContainerNode = submissionContainerElementNodeFactory.buildWithId({ parent: cardNode }); + const submissionItemNode = submissionItemNodeFactory.buildWithId({ + parent: submissionElementContainerNode, + userId: studentUser.id, + }); - await em.persistAndFlush([columnBoardNode, columnNode, cardNode, submissionContainerNode, submissionItemNode]); + await em.persistAndFlush([ + columnBoardNode, + columnNode, + cardNode, + submissionElementContainerNode, + submissionItemNode, + ]); em.clear(); - const loggedInClient = await testApiClient.login(teacherAccount); + const loggedInClient = await testApiClientSubmission.login(studentAccount); return { loggedInClient, cardNode, submissionItemNode }; }; @@ -192,6 +210,16 @@ describe(`content element create (api)`, () => { expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.FILE); }); + + it('should throw if element is not RICH_TEXT or FILE', async () => { + const { loggedInClient, submissionItemNode } = await setup(); + + const response = await loggedInClient.post(`${submissionItemNode.id}/elements`, { + type: ContentElementType.EXTERNAL_TOOL, + }); + + expect(response.statusCode).toEqual(400); + }); }); }); From 7878a955dcd0703abb03bc579044b3ed488afa7c Mon Sep 17 00:00:00 2001 From: virgilchiriac Date: Tue, 24 Oct 2023 20:54:36 +0200 Subject: [PATCH 13/16] more tests and some refactoring --- .../content-element-create.api.spec.ts | 2 - .../submission-item-lookup.api.spec.ts | 121 +++++++++++++++++- .../controller/board-submission.controller.ts | 5 +- .../content-element-response.factory.ts | 22 +++- .../mapper/submission-item-response.mapper.ts | 13 +- apps/server/src/modules/board/uc/base.uc.ts | 3 +- 6 files changed, 141 insertions(+), 25 deletions(-) diff --git a/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts b/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts index d53f5e4b694..a593f6d79b4 100644 --- a/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts +++ b/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts @@ -8,9 +8,7 @@ import { columnBoardNodeFactory, columnNodeFactory, courseFactory, - submissionContainerElementFactory, submissionContainerElementNodeFactory, - submissionItemFactory, submissionItemNodeFactory, TestApiClient, UserAndAccountTestFactory, diff --git a/apps/server/src/modules/board/controller/api-test/submission-item-lookup.api.spec.ts b/apps/server/src/modules/board/controller/api-test/submission-item-lookup.api.spec.ts index c119ace1d3c..e297b164d9e 100644 --- a/apps/server/src/modules/board/controller/api-test/submission-item-lookup.api.spec.ts +++ b/apps/server/src/modules/board/controller/api-test/submission-item-lookup.api.spec.ts @@ -1,17 +1,19 @@ import { EntityManager } from '@mikro-orm/mongodb'; import { INestApplication } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; -import { BoardExternalReferenceType } from '@shared/domain'; +import { BoardExternalReferenceType, ContentElementType } from '@shared/domain'; import { - TestApiClient, - UserAndAccountTestFactory, cardNodeFactory, cleanupCollections, columnBoardNodeFactory, columnNodeFactory, courseFactory, + fileElementNodeFactory, + richTextElementNodeFactory, submissionContainerElementNodeFactory, submissionItemNodeFactory, + TestApiClient, + UserAndAccountTestFactory, userFactory, } from '@shared/testing'; import { ServerTestModule } from '@modules/server'; @@ -257,4 +259,117 @@ describe('submission item lookup (api)', () => { expect(response.status).toEqual(403); }); }); + + describe('when submission item has child elements', () => { + describe('when submission item has a RICH_TEXT child element', () => { + const setup = async () => { + await cleanupCollections(em); + + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); + const { studentAccount, studentUser } = UserAndAccountTestFactory.buildStudent(); + const course = courseFactory.build({ teachers: [teacherUser], students: [studentUser] }); + await em.persistAndFlush([studentAccount, studentUser, teacherAccount, teacherUser, course]); + + const columnBoardNode = columnBoardNodeFactory.buildWithId({ + context: { id: course.id, type: BoardExternalReferenceType.Course }, + }); + + const columnNode = columnNodeFactory.buildWithId({ parent: columnBoardNode }); + + const cardNode = cardNodeFactory.buildWithId({ parent: columnNode }); + + const submissionContainer = submissionContainerElementNodeFactory.buildWithId({ parent: cardNode }); + const submissionItem = submissionItemNodeFactory.buildWithId({ + parent: submissionContainer, + userId: studentUser.id, + }); + const richTextElement = richTextElementNodeFactory.buildWithId({ parent: submissionItem }); + + await em.persistAndFlush([ + columnBoardNode, + columnNode, + cardNode, + submissionContainer, + submissionItem, + richTextElement, + ]); + + const loggedInClient = await testApiClient.login(studentAccount); + + return { + loggedInClient, + submissionContainer, + submissionItem, + richTextElement, + }; + }; + + it('should return all RICH_TEXT child elements', async () => { + const { loggedInClient, submissionContainer, richTextElement } = await setup(); + + const response = await loggedInClient.get(`${submissionContainer.id}`); + const submissionItemResponse = (response.body as SubmissionsResponse).submissionItemsResponse[0]; + const richTextElementResponse = submissionItemResponse.elements.filter( + (element) => element.type === ContentElementType.RICH_TEXT + ); + + expect(richTextElementResponse[0].id).toEqual(richTextElement.id); + }); + }); + + describe('when submission item has a FILE child element', () => { + const setup = async () => { + await cleanupCollections(em); + + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); + const { studentAccount, studentUser } = UserAndAccountTestFactory.buildStudent(); + const course = courseFactory.build({ teachers: [teacherUser], students: [studentUser] }); + await em.persistAndFlush([studentAccount, studentUser, teacherAccount, teacherUser, course]); + + const columnBoardNode = columnBoardNodeFactory.buildWithId({ + context: { id: course.id, type: BoardExternalReferenceType.Course }, + }); + + const columnNode = columnNodeFactory.buildWithId({ parent: columnBoardNode }); + + const cardNode = cardNodeFactory.buildWithId({ parent: columnNode }); + + const submissionContainer = submissionContainerElementNodeFactory.buildWithId({ parent: cardNode }); + const submissionItem = submissionItemNodeFactory.buildWithId({ + parent: submissionContainer, + userId: studentUser.id, + }); + const fileElement = fileElementNodeFactory.buildWithId({ parent: submissionItem }); + + await em.persistAndFlush([ + columnBoardNode, + columnNode, + cardNode, + submissionContainer, + submissionItem, + fileElement, + ]); + + const loggedInClient = await testApiClient.login(studentAccount); + + return { + loggedInClient, + submissionContainer, + submissionItem, + fileElement, + }; + }; + it('should return all FILE child elements', async () => { + const { loggedInClient, submissionContainer, fileElement } = await setup(); + + const response = await loggedInClient.get(`${submissionContainer.id}`); + const submissionItemResponse = (response.body as SubmissionsResponse).submissionItemsResponse[0]; + const fileElementResponse = submissionItemResponse.elements.filter( + (element) => element.type === ContentElementType.FILE + ); + + expect(fileElementResponse[0].id).toEqual(fileElement.id); + }); + }); + }); }); diff --git a/apps/server/src/modules/board/controller/board-submission.controller.ts b/apps/server/src/modules/board/controller/board-submission.controller.ts index 1dd46440f21..ad852ed5cfb 100644 --- a/apps/server/src/modules/board/controller/board-submission.controller.ts +++ b/apps/server/src/modules/board/controller/board-submission.controller.ts @@ -96,10 +96,7 @@ export class BoardSubmissionController { ): Promise { const { type } = bodyParams; const element = await this.submissionItemUc.createElement(currentUser.userId, urlParams.submissionItemId, type); - const response = ContentElementResponseFactory.mapToResponse(element); - if (!isFileElementResponse(response) && !isRichTextElementResponse(response)) { - throw new UnprocessableEntityException(); - } + const response = ContentElementResponseFactory.mapSubmissionContentToResponse(element); return response; } diff --git a/apps/server/src/modules/board/controller/mapper/content-element-response.factory.ts b/apps/server/src/modules/board/controller/mapper/content-element-response.factory.ts index bda46e4b73f..286d1b4d5c2 100644 --- a/apps/server/src/modules/board/controller/mapper/content-element-response.factory.ts +++ b/apps/server/src/modules/board/controller/mapper/content-element-response.factory.ts @@ -1,6 +1,12 @@ -import { NotImplementedException } from '@nestjs/common'; -import { AnyBoardDo } from '@shared/domain'; -import { AnyContentElementResponse } from '../dto'; +import { NotImplementedException, UnprocessableEntityException } from '@nestjs/common'; +import { AnyBoardDo, FileElement, isSubmissionItemContent, RichTextElement } from '@shared/domain'; +import { + AnyContentElementResponse, + FileElementResponse, + RichTextElementResponse, + isFileElementResponse, + isRichTextElementResponse, +} from '../dto'; import { BaseResponseMapper } from './base-mapper.interface'; import { ExternalToolElementResponseMapper } from './external-tool-element-response.mapper'; import { FileElementResponseMapper } from './file-element-response.mapper'; @@ -28,4 +34,14 @@ export class ContentElementResponseFactory { return result; } + + static mapSubmissionContentToResponse( + element: RichTextElement | FileElement + ): FileElementResponse | RichTextElementResponse { + const result = this.mapToResponse(element); + if (!isFileElementResponse(result) && !isRichTextElementResponse(result)) { + throw new UnprocessableEntityException(); + } + return result; + } } diff --git a/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts b/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts index c04d6d50d52..70023b36020 100644 --- a/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts +++ b/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts @@ -11,6 +11,7 @@ import { UnprocessableEntityException } from '@nestjs/common'; import { FileElementResponseMapper } from './file-element-response.mapper'; import { RichTextElementResponseMapper } from './rich-text-element-response.mapper'; import { SubmissionItemResponse, SubmissionsResponse, TimestampsResponse, UserDataResponse } from '../dto'; +import { ContentElementResponseFactory } from './content-element-response.factory'; export class SubmissionItemResponseMapper { private static instance: SubmissionItemResponseMapper; @@ -44,17 +45,7 @@ export class SubmissionItemResponseMapper { createdAt: submissionItem.createdAt, }), userId: submissionItem.userId, - elements: children.map((element) => { - if (isFileElement(element)) { - const mapper = FileElementResponseMapper.getInstance(); - return mapper.mapToResponse(element); - } - if (isRichTextElement(element)) { - const mapper = RichTextElementResponseMapper.getInstance(); - return mapper.mapToResponse(element); - } - throw new UnprocessableEntityException(); - }), + elements: children.map((element) => ContentElementResponseFactory.mapSubmissionContentToResponse(element)), }); return result; diff --git a/apps/server/src/modules/board/uc/base.uc.ts b/apps/server/src/modules/board/uc/base.uc.ts index e5feab2ef9f..e5d88e7b13d 100644 --- a/apps/server/src/modules/board/uc/base.uc.ts +++ b/apps/server/src/modules/board/uc/base.uc.ts @@ -1,11 +1,10 @@ import { AnyBoardDo, EntityId, SubmissionItem, UserRoleEnum } from '@shared/domain'; -import { ForbiddenException, forwardRef, Inject } from '@nestjs/common'; +import { ForbiddenException } from '@nestjs/common'; import { AuthorizationService, Action } from '@modules/authorization'; import { BoardDoAuthorizableService } from '../service'; export abstract class BaseUc { constructor( - @Inject(forwardRef(() => AuthorizationService)) protected readonly authorizationService: AuthorizationService, protected readonly boardDoAuthorizableService: BoardDoAuthorizableService ) {} From ad7e79c478a6515abc8967ad82aa9eb26cffd680 Mon Sep 17 00:00:00 2001 From: virgilchiriac Date: Tue, 24 Oct 2023 21:05:33 +0200 Subject: [PATCH 14/16] fix lint --- .../board/controller/board-submission.controller.ts | 3 --- .../mapper/content-element-response.factory.ts | 2 +- .../mapper/submission-item-response.mapper.ts | 13 +------------ 3 files changed, 2 insertions(+), 16 deletions(-) diff --git a/apps/server/src/modules/board/controller/board-submission.controller.ts b/apps/server/src/modules/board/controller/board-submission.controller.ts index ad852ed5cfb..4e81d342849 100644 --- a/apps/server/src/modules/board/controller/board-submission.controller.ts +++ b/apps/server/src/modules/board/controller/board-submission.controller.ts @@ -8,7 +8,6 @@ import { Param, Patch, Post, - UnprocessableEntityException, } from '@nestjs/common'; import { ApiExtraModels, ApiOperation, ApiResponse, ApiTags, getSchemaPath } from '@nestjs/swagger'; import { ApiValidationError } from '@shared/common'; @@ -20,8 +19,6 @@ import { SubmissionItemUc } from '../uc/submission-item.uc'; import { CreateContentElementBodyParams, FileElementResponse, - isFileElementResponse, - isRichTextElementResponse, RichTextElementResponse, SubmissionContainerUrlParams, SubmissionItemUrlParams, diff --git a/apps/server/src/modules/board/controller/mapper/content-element-response.factory.ts b/apps/server/src/modules/board/controller/mapper/content-element-response.factory.ts index 286d1b4d5c2..8431b630be9 100644 --- a/apps/server/src/modules/board/controller/mapper/content-element-response.factory.ts +++ b/apps/server/src/modules/board/controller/mapper/content-element-response.factory.ts @@ -1,5 +1,5 @@ import { NotImplementedException, UnprocessableEntityException } from '@nestjs/common'; -import { AnyBoardDo, FileElement, isSubmissionItemContent, RichTextElement } from '@shared/domain'; +import { AnyBoardDo, FileElement, RichTextElement } from '@shared/domain'; import { AnyContentElementResponse, FileElementResponse, diff --git a/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts b/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts index 70023b36020..6cbf0348109 100644 --- a/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts +++ b/apps/server/src/modules/board/controller/mapper/submission-item-response.mapper.ts @@ -1,15 +1,4 @@ -import { - FileElement, - isFileElement, - isRichTextElement, - isSubmissionItemContent, - RichTextElement, - SubmissionItem, - UserBoardRoles, -} from '@shared/domain'; -import { UnprocessableEntityException } from '@nestjs/common'; -import { FileElementResponseMapper } from './file-element-response.mapper'; -import { RichTextElementResponseMapper } from './rich-text-element-response.mapper'; +import { FileElement, isSubmissionItemContent, RichTextElement, SubmissionItem, UserBoardRoles } from '@shared/domain'; import { SubmissionItemResponse, SubmissionsResponse, TimestampsResponse, UserDataResponse } from '../dto'; import { ContentElementResponseFactory } from './content-element-response.factory'; From 34592eb8dfc495f23badf3000a12877ea8db5f9d Mon Sep 17 00:00:00 2001 From: virgilchiriac Date: Thu, 26 Oct 2023 16:33:59 +0200 Subject: [PATCH 15/16] cleanup and adding missing api tests add one more test --- .../content-element-create.api.spec.ts | 208 ++++++++++++++++-- .../create-content-element.body.params.ts | 2 +- .../update-element-content.body.params.ts | 2 +- .../submission-item.response.ts | 2 - .../board/uc/submission-item.uc.spec.ts | 125 +++++------ 5 files changed, 249 insertions(+), 90 deletions(-) diff --git a/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts b/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts index a593f6d79b4..50dec80777d 100644 --- a/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts +++ b/apps/server/src/modules/board/controller/api-test/content-element-create.api.spec.ts @@ -41,8 +41,8 @@ describe(`content element create (api)`, () => { await app.close(); }); - describe('with valid user', () => { - describe('when parent is a card', () => { + describe('when the parent of the element is a card node', () => { + describe('with valid user', () => { const setup = async () => { await cleanupCollections(em); @@ -142,8 +142,78 @@ describe(`content element create (api)`, () => { expect(response.statusCode).toEqual(400); }); }); + describe('with invalid user', () => { + describe('with teacher not belonging to course', () => { + const setup = async () => { + await cleanupCollections(em); + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); - describe('when parent is a submission item', () => { + const course = courseFactory.build({}); + await em.persistAndFlush([teacherAccount, teacherUser, course]); + + const columnBoardNode = columnBoardNodeFactory.buildWithId({ + context: { id: course.id, type: BoardExternalReferenceType.Course }, + }); + const columnNode = columnNodeFactory.buildWithId({ parent: columnBoardNode }); + const cardNode = cardNodeFactory.buildWithId({ parent: columnNode }); + + await em.persistAndFlush([columnBoardNode, columnNode, cardNode]); + em.clear(); + + const loggedInClient = await testApiClient.login(teacherAccount); + + return { loggedInClient, columnBoardNode, columnNode, cardNode }; + }; + + it('should return status 403', async () => { + const { cardNode, loggedInClient } = await setup(); + + const response = await loggedInClient.post(`${cardNode.id}/elements`, { type: ContentElementType.RICH_TEXT }); + + expect(response.statusCode).toEqual(403); + }); + }); + + describe('with student belonging to course', () => { + describe('when the parent of the element is a card node', () => { + const setup = async () => { + await cleanupCollections(em); + const { studentAccount, studentUser } = UserAndAccountTestFactory.buildStudent(); + + const course = courseFactory.build({ students: [studentUser] }); + await em.persistAndFlush([studentAccount, studentUser, course]); + + const columnBoardNode = columnBoardNodeFactory.buildWithId({ + context: { id: course.id, type: BoardExternalReferenceType.Course }, + }); + const columnNode = columnNodeFactory.buildWithId({ parent: columnBoardNode }); + const cardNode = cardNodeFactory.buildWithId({ parent: columnNode }); + + await em.persistAndFlush([columnBoardNode, columnNode, cardNode]); + em.clear(); + + const loggedInClient = await testApiClient.login(studentAccount); + + return { loggedInClient, columnBoardNode, columnNode, cardNode }; + }; + + it('should return status 403', async () => { + const { cardNode, loggedInClient } = await setup(); + + const response = await loggedInClient.post(`${cardNode.id}/elements`, { + type: ContentElementType.RICH_TEXT, + }); + + expect(response.statusCode).toEqual(403); + }); + }); + describe('when the parent of the element is a submission item', () => {}); + }); + }); + }); + + describe('when the parent of the element is a submission item', () => { + describe('with user being the owner of the parent submission item', () => { const setup = async () => { await cleanupCollections(em); @@ -159,7 +229,9 @@ describe(`content element create (api)`, () => { }); const columnNode = columnNodeFactory.buildWithId({ parent: columnBoardNode }); const cardNode = cardNodeFactory.buildWithId({ parent: columnNode }); - const submissionElementContainerNode = submissionContainerElementNodeFactory.buildWithId({ parent: cardNode }); + const submissionElementContainerNode = submissionContainerElementNodeFactory.buildWithId({ + parent: cardNode, + }); const submissionItemNode = submissionItemNodeFactory.buildWithId({ parent: submissionElementContainerNode, userId: studentUser.id, @@ -218,16 +290,74 @@ describe(`content element create (api)`, () => { expect(response.statusCode).toEqual(400); }); + + it('should actually create the content element', async () => { + const { loggedInClient, submissionItemNode } = await setup(); + const response = await loggedInClient.post(`${submissionItemNode.id}/elements`, { + type: ContentElementType.RICH_TEXT, + }); + + const elementId = (response.body as AnyContentElementResponse).id; + + const result = await em.findOneOrFail(RichTextElementNode, elementId); + expect(result.id).toEqual(elementId); + }); }); - }); + describe('with user not being the owner of the parent submission item', () => { + const setup = async () => { + await cleanupCollections(em); + + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); + const { studentAccount, studentUser } = UserAndAccountTestFactory.buildStudent(); - describe('with invalid user', () => { - describe('with teacher not belonging to course', () => { + const course = courseFactory.build({ teachers: [teacherUser], students: [studentUser] }); + + await em.persistAndFlush([teacherAccount, teacherUser, studentAccount, studentUser, course]); + + const columnBoardNode = columnBoardNodeFactory.buildWithId({ + context: { id: course.id, type: BoardExternalReferenceType.Course }, + }); + const columnNode = columnNodeFactory.buildWithId({ parent: columnBoardNode }); + const cardNode = cardNodeFactory.buildWithId({ parent: columnNode }); + const submissionElementContainerNode = submissionContainerElementNodeFactory.buildWithId({ + parent: cardNode, + }); + const submissionItemNode = submissionItemNodeFactory.buildWithId({ + parent: submissionElementContainerNode, + userId: teacherUser.id, + }); + + await em.persistAndFlush([ + columnBoardNode, + columnNode, + cardNode, + submissionElementContainerNode, + submissionItemNode, + ]); + em.clear(); + + const loggedInClient = await testApiClientSubmission.login(studentAccount); + + return { loggedInClient, cardNode, submissionItemNode }; + }; + it('should return status 403', async () => { + const { loggedInClient, submissionItemNode } = await setup(); + + const response = await loggedInClient.post(`${submissionItemNode.id}/elements`, { + type: ContentElementType.RICH_TEXT, + }); + + expect(response.statusCode).toEqual(403); + }); + }); + describe('with user not a student', () => { const setup = async () => { await cleanupCollections(em); + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); - const course = courseFactory.build({}); + const course = courseFactory.build({ teachers: [teacherUser] }); + await em.persistAndFlush([teacherAccount, teacherUser, course]); const columnBoardNode = columnBoardNodeFactory.buildWithId({ @@ -235,50 +365,80 @@ describe(`content element create (api)`, () => { }); const columnNode = columnNodeFactory.buildWithId({ parent: columnBoardNode }); const cardNode = cardNodeFactory.buildWithId({ parent: columnNode }); + const submissionElementContainerNode = submissionContainerElementNodeFactory.buildWithId({ + parent: cardNode, + }); + const submissionItemNode = submissionItemNodeFactory.buildWithId({ + parent: submissionElementContainerNode, + userId: teacherUser.id, + }); - await em.persistAndFlush([columnBoardNode, columnNode, cardNode]); + await em.persistAndFlush([ + columnBoardNode, + columnNode, + cardNode, + submissionElementContainerNode, + submissionItemNode, + ]); em.clear(); - const loggedInClient = await testApiClient.login(teacherAccount); + const loggedInClient = await testApiClientSubmission.login(teacherAccount); - return { loggedInClient, columnBoardNode, columnNode, cardNode }; + return { loggedInClient, cardNode, submissionItemNode }; }; - it('should return status 403', async () => { - const { cardNode, loggedInClient } = await setup(); + const { loggedInClient, submissionItemNode } = await setup(); - const response = await loggedInClient.post(`${cardNode.id}/elements`, { type: ContentElementType.RICH_TEXT }); + const response = await loggedInClient.post(`${submissionItemNode.id}/elements`, { + type: ContentElementType.RICH_TEXT, + }); expect(response.statusCode).toEqual(403); }); }); - - describe('with student belonging to course', () => { + describe('with user not belonging to course', () => { const setup = async () => { await cleanupCollections(em); + + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); const { studentAccount, studentUser } = UserAndAccountTestFactory.buildStudent(); - const course = courseFactory.build({ students: [studentUser] }); - await em.persistAndFlush([studentAccount, studentUser, course]); + const course = courseFactory.build({ teachers: [teacherUser] }); + + await em.persistAndFlush([teacherAccount, teacherUser, studentAccount, studentUser, course]); const columnBoardNode = columnBoardNodeFactory.buildWithId({ context: { id: course.id, type: BoardExternalReferenceType.Course }, }); const columnNode = columnNodeFactory.buildWithId({ parent: columnBoardNode }); const cardNode = cardNodeFactory.buildWithId({ parent: columnNode }); + const submissionElementContainerNode = submissionContainerElementNodeFactory.buildWithId({ + parent: cardNode, + }); + const submissionItemNode = submissionItemNodeFactory.buildWithId({ + parent: submissionElementContainerNode, + userId: studentUser.id, + }); - await em.persistAndFlush([columnBoardNode, columnNode, cardNode]); + await em.persistAndFlush([ + columnBoardNode, + columnNode, + cardNode, + submissionElementContainerNode, + submissionItemNode, + ]); em.clear(); - const loggedInClient = await testApiClient.login(studentAccount); + const loggedInClient = await testApiClientSubmission.login(studentAccount); - return { loggedInClient, columnBoardNode, columnNode, cardNode }; + return { loggedInClient, cardNode, submissionItemNode }; }; - it('should return status 403', async () => { - const { cardNode, loggedInClient } = await setup(); + const { loggedInClient, submissionItemNode } = await setup(); - const response = await loggedInClient.post(`${cardNode.id}/elements`, { type: ContentElementType.RICH_TEXT }); + const response = await loggedInClient.post(`${submissionItemNode.id}/elements`, { + type: ContentElementType.RICH_TEXT, + }); expect(response.statusCode).toEqual(403); }); diff --git a/apps/server/src/modules/board/controller/dto/element/create-content-element.body.params.ts b/apps/server/src/modules/board/controller/dto/element/create-content-element.body.params.ts index d38a5c744c7..4eaf32d2a6d 100644 --- a/apps/server/src/modules/board/controller/dto/element/create-content-element.body.params.ts +++ b/apps/server/src/modules/board/controller/dto/element/create-content-element.body.params.ts @@ -1,5 +1,5 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; -import { ContentElementType } from '@shared/domain/domainobject/board/types'; +import { ContentElementType } from '@shared/domain'; import { IsEnum, IsInt, IsOptional, Min } from 'class-validator'; export class CreateContentElementBodyParams { diff --git a/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts b/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts index 38a4163df3a..8ce973cc110 100644 --- a/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts +++ b/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts @@ -1,5 +1,5 @@ import { ApiProperty, ApiPropertyOptional, getSchemaPath } from '@nestjs/swagger'; -import { ContentElementType } from '@shared/domain/domainobject/board/types'; +import { ContentElementType } from '@shared/domain'; import { InputFormat } from '@shared/domain/types'; import { Type } from 'class-transformer'; import { IsDate, IsEnum, IsMongoId, IsOptional, IsString, ValidateNested } from 'class-validator'; diff --git a/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts b/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts index 77218bbc3f2..14b1da73b75 100644 --- a/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts +++ b/apps/server/src/modules/board/controller/dto/submission-item/submission-item.response.ts @@ -2,8 +2,6 @@ import { ApiExtraModels, ApiProperty, getSchemaPath } from '@nestjs/swagger'; import { TimestampsResponse } from '../timestamps.response'; import { FileElementResponse, RichTextElementResponse } from '../element'; -// export SubmissionContentElementResponse = RichTextElementResponse | FileElementResponse; - @ApiExtraModels(FileElementResponse, RichTextElementResponse) export class SubmissionItemResponse { constructor({ id, timestamps, completed, userId, elements }: SubmissionItemResponse) { diff --git a/apps/server/src/modules/board/uc/submission-item.uc.spec.ts b/apps/server/src/modules/board/uc/submission-item.uc.spec.ts index 93fa11fd48b..e6b96d83394 100644 --- a/apps/server/src/modules/board/uc/submission-item.uc.spec.ts +++ b/apps/server/src/modules/board/uc/submission-item.uc.spec.ts @@ -256,86 +256,87 @@ describe(SubmissionItemUc.name, () => { }); }); - // write tests describe('createElement', () => { - const setup = () => { - const user = userFactory.buildWithId(); - const submissionItem = submissionItemFactory.build({ - userId: user.id, - }); + describe('when the user is a student', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const submissionItem = submissionItemFactory.build({ + userId: user.id, + }); - submissionItemService.findById.mockResolvedValue(submissionItem); + submissionItemService.findById.mockResolvedValue(submissionItem); - const element = richTextElementFactory.build(); + const element = richTextElementFactory.build(); - boardDoAuthorizableService.getBoardAuthorizable.mockResolvedValue( - new BoardDoAuthorizable({ - users: [{ userId: user.id, roles: [BoardRoles.READER], userRoleEnum: UserRoleEnum.STUDENT }], - id: submissionItem.id, - }) - ); + boardDoAuthorizableService.getBoardAuthorizable.mockResolvedValue( + new BoardDoAuthorizable({ + users: [{ userId: user.id, roles: [BoardRoles.READER], userRoleEnum: UserRoleEnum.STUDENT }], + id: submissionItem.id, + }) + ); - return { element, submissionItem, user }; - }; + return { element, submissionItem, user }; + }; - it('should call service to find the submission item ', async () => { - const { element, submissionItem, user } = setup(); - elementService.create.mockResolvedValueOnce(element); + it('should call service to find the submission item ', async () => { + const { element, submissionItem, user } = setup(); + elementService.create.mockResolvedValueOnce(element); - await uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT); - expect(submissionItemService.findById).toHaveBeenCalledWith(submissionItem.id); - }); + await uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT); + expect(submissionItemService.findById).toHaveBeenCalledWith(submissionItem.id); + }); - it('should authorize', async () => { - const { element, submissionItem, user } = setup(); - elementService.create.mockResolvedValueOnce(element); + it('should authorize', async () => { + const { element, submissionItem, user } = setup(); + elementService.create.mockResolvedValueOnce(element); - const boardDoAuthorizable = await boardDoAuthorizableService.getBoardAuthorizable(submissionItem); + const boardDoAuthorizable = await boardDoAuthorizableService.getBoardAuthorizable(submissionItem); - await uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT); - const context = { action: Action.read, requiredPermissions: [] }; - expect(authorizationService.checkPermission).toBeCalledWith(user, boardDoAuthorizable, context); - }); + await uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT); + const context = { action: Action.read, requiredPermissions: [] }; + expect(authorizationService.checkPermission).toBeCalledWith(user, boardDoAuthorizable, context); + }); - it('should throw if user is not creator of submission item', async () => { - const user2 = userFactory.buildWithId(); - const { submissionItem } = setup(); + it('should throw if user is not creator of submission item', async () => { + const user2 = userFactory.buildWithId(); + const { submissionItem } = setup(); - await expect(uc.createElement(user2.id, submissionItem.id, ContentElementType.RICH_TEXT)).rejects.toThrow( - new ForbiddenException() - ); - }); + await expect(uc.createElement(user2.id, submissionItem.id, ContentElementType.RICH_TEXT)).rejects.toThrow( + new ForbiddenException() + ); + }); - it('should throw if type is not file or rich text', async () => { - const { submissionItem, user } = setup(); - await expect(uc.createElement(user.id, submissionItem.id, ContentElementType.LINK)).rejects.toThrow( - new BadRequestException() - ); - }); + it('should throw if type is not file or rich text', async () => { + const { submissionItem, user } = setup(); + await expect(uc.createElement(user.id, submissionItem.id, ContentElementType.LINK)).rejects.toThrow( + new BadRequestException() + ); + }); - it('should call service to create element', async () => { - const { element, submissionItem, user } = setup(); - elementService.create.mockResolvedValueOnce(element); + it('should call service to create element', async () => { + const { element, submissionItem, user } = setup(); + elementService.create.mockResolvedValueOnce(element); - await uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT); - expect(elementService.create).toHaveBeenCalledWith(submissionItem, ContentElementType.RICH_TEXT); - }); + await uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT); + expect(elementService.create).toHaveBeenCalledWith(submissionItem, ContentElementType.RICH_TEXT); + }); - it('should return element', async () => { - const { element, submissionItem, user } = setup(); - elementService.create.mockResolvedValueOnce(element); + it('should return element', async () => { + const { element, submissionItem, user } = setup(); + elementService.create.mockResolvedValueOnce(element); - const returnedElement = await uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT); - expect(returnedElement).toEqual(element); - }); + const returnedElement = await uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT); + expect(returnedElement).toEqual(element); + }); - it('should throw if element is not file or rich text', async () => { - const { submissionItem, user } = setup(); - const otherElement = submissionContainerElementFactory.build(); - elementService.create.mockResolvedValueOnce(otherElement); - await expect(uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT)).rejects.toThrow( - new UnprocessableEntityException() - ); + it('should throw if element is not file or rich text', async () => { + const { submissionItem, user } = setup(); + const otherElement = submissionContainerElementFactory.build(); + elementService.create.mockResolvedValueOnce(otherElement); + await expect(uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT)).rejects.toThrow( + new UnprocessableEntityException() + ); + }); }); }); }); From 9e592d18818fc2824a45ec02f10056b10c83406f Mon Sep 17 00:00:00 2001 From: virgilchiriac Date: Fri, 3 Nov 2023 10:54:58 +0100 Subject: [PATCH 16/16] undo api prop which was breaking client tests --- .../dto/element/update-element-content.body.params.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts b/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts index 8ce973cc110..36516ae80a6 100644 --- a/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts +++ b/apps/server/src/modules/board/controller/dto/element/update-element-content.body.params.ts @@ -53,9 +53,7 @@ export class RichTextContentBody { text!: string; @IsEnum(InputFormat) - @ApiProperty({ - enum: InputFormat, - }) + @ApiProperty() inputFormat!: InputFormat; }