From f4587465a67f140a02df0d71cb54206a8e2a9b44 Mon Sep 17 00:00:00 2001 From: Max <53796487+dyedwiper@users.noreply.github.com> Date: Fri, 3 Nov 2023 11:45:17 +0100 Subject: [PATCH 1/7] BC-5712 Make test workflow manually runnable (#4519) --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cbf52acce55..2b087f1bc51 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -5,6 +5,7 @@ on: branches: [ main ] pull_request: branches: [ main ] + workflow_dispatch: permissions: contents: read From b78c1bb8f7269825bd3689749c4e297e02812458 Mon Sep 17 00:00:00 2001 From: virgilchiriac <17074330+virgilchiriac@users.noreply.github.com> Date: Fri, 3 Nov 2023 12:51:44 +0100 Subject: [PATCH 2/7] BC-4453 - submission item file (#4484) * add endpoint for creating element inside submission item * add file element and rich text as submission item children nodes * refactor board uc: ** split column and columnBoard uc ** introduce an abstract base.uc for common methods such as permission check ** move some methods from "parent" uc, such as deleteElement (now part of element.uc instead of card.uc) --- .../src/modules/board/board-api.module.ts | 4 +- .../content-element-create.api.spec.ts | 406 ++++++++++++++---- .../submission-item-create.api.spec.ts | 2 +- .../submission-item-lookup.api.spec.ts | 121 +++++- .../controller/board-submission.controller.ts | 49 ++- .../board/controller/card.controller.ts | 12 +- .../board/controller/column.controller.ts | 10 +- .../element/any-content-element.response.ts | 6 + .../update-element-content.body.params.ts | 5 +- .../submission-item.response.ts | 15 +- .../board/controller/element.controller.ts | 4 +- .../content-element-response.factory.ts | 22 +- .../mapper/submission-item-response.mapper.ts | 9 +- .../board/repo/board-do.builder-impl.ts | 8 +- .../board/repo/recursive-save.visitor.ts | 41 +- .../service/content-element.service.spec.ts | 37 ++ .../board/service/content-element.service.ts | 12 +- apps/server/src/modules/board/uc/base.uc.ts | 50 +++ .../src/modules/board/uc/board.uc.spec.ts | 191 +------- apps/server/src/modules/board/uc/board.uc.ts | 97 +---- .../src/modules/board/uc/card.uc.spec.ts | 176 ++++++-- apps/server/src/modules/board/uc/card.uc.ts | 55 ++- .../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 | 105 ++++- .../server/src/modules/board/uc/element.uc.ts | 70 +-- apps/server/src/modules/board/uc/index.ts | 4 + .../board/uc/submission-item.uc.spec.ts | 107 ++++- .../modules/board/uc/submission-item.uc.ts | 77 ++-- .../learnroom/service/board-copy.service.ts | 2 +- .../domainobject/board/submission-item.do.ts | 11 +- .../boardnode/column-board-node.entity.ts | 6 +- .../boardnode/column-board-node.factory.ts | 3 +- .../board/column-board.do.factory.ts | 3 +- 34 files changed, 1404 insertions(+), 560 deletions(-) create mode 100644 apps/server/src/modules/board/uc/base.uc.ts 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/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..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 @@ -8,6 +8,8 @@ import { columnBoardNodeFactory, columnNodeFactory, courseFactory, + submissionContainerElementNodeFactory, + submissionItemNodeFactory, TestApiClient, UserAndAccountTestFactory, } from '@shared/testing'; @@ -15,11 +17,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,169 +34,411 @@ describe(`content element create (api)`, () => { await app.init(); em = module.get(EntityManager); testApiClient = new TestApiClient(app, baseRouteName); + testApiClientSubmission = new TestApiClient(app, submissionRouteName); }); afterAll(async () => { await app.close(); }); - describe('with valid user', () => { - const setup = async () => { - await cleanupCollections(em); + describe('when the parent of the element is a card node', () => { + describe('with valid user', () => { + 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 columnNode = columnNodeFactory.buildWithId({ parent: columnBoardNode }); - const cardNode = cardNodeFactory.buildWithId({ parent: columnNode }); + 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(); + await em.persistAndFlush([columnBoardNode, columnNode, cardNode]); + em.clear(); - const loggedInClient = await testApiClient.login(teacherAccount); + const loggedInClient = await testApiClient.login(teacherAccount); - return { loggedInClient, columnBoardNode, columnNode, cardNode }; - }; + return { loggedInClient, columnBoardNode, columnNode, cardNode }; + }; - it('should return status 201', async () => { - const { loggedInClient, cardNode } = await setup(); + it('should return status 201', 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.RICH_TEXT }); - expect(response.statusCode).toEqual(201); - }); + expect(response.statusCode).toEqual(201); + }); - 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 RICH_TEXT', 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.RICH_TEXT }); - expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.RICH_TEXT); - }); + expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.RICH_TEXT); + }); - 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 FILE', 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.FILE }); - expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.FILE); - }); + expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.FILE); + }); - it('should return the created content element of type EXTERNAL_TOOL', 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.EXTERNAL_TOOL }); + const response = await loggedInClient.post(`${cardNode.id}/elements`, { + type: ContentElementType.EXTERNAL_TOOL, + }); - expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.EXTERNAL_TOOL); - }); + expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.EXTERNAL_TOOL); + }); - it('should return the created content element of type SUBMISSION_CONTAINER with dueDate set to null', 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.SUBMISSION_CONTAINER, + }); - const response = await loggedInClient.post(`${cardNode.id}/elements`, { - type: ContentElementType.SUBMISSION_CONTAINER, + expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.SUBMISSION_CONTAINER); + expect((response.body as SubmissionContainerElementResponse).content.dueDate).toBeNull(); }); - expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.SUBMISSION_CONTAINER); - expect((response.body as SubmissionContainerElementResponse).content.dueDate).toBeNull(); - }); + 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 }); - 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 elementId = (response.body as AnyContentElementResponse).id; - const elementId = (response.body as AnyContentElementResponse).id; + const result = await em.findOneOrFail(RichTextElementNode, elementId); + expect(result.id).toEqual(elementId); + }); - 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 throw an error if toPosition param is not a number', 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.RICH_TEXT, - toPosition: 'not a number', + expect(response.statusCode).toEqual(400); }); - expect(response.statusCode).toEqual(400); + 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); + }); }); + describe('with invalid user', () => { + describe('with teacher not belonging to course', () => { + const setup = async () => { + await cleanupCollections(em); + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); - it('should throw an error if toPosition param is a negative number', async () => { - const { loggedInClient, cardNode } = await setup(); + const course = courseFactory.build({}); + await em.persistAndFlush([teacherAccount, teacherUser, course]); - const response = await loggedInClient.post(`${cardNode.id}/elements`, { - type: ContentElementType.RICH_TEXT, - toPosition: -1, + 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); + }); }); - expect(response.statusCode).toEqual(400); + 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('with invalid user', () => { - describe('with teacher not belonging to course', () => { + 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); + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); + const { studentAccount, studentUser } = UserAndAccountTestFactory.buildStudent(); - const course = courseFactory.build({}); - 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 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(teacherAccount); + const loggedInClient = await testApiClientSubmission.login(studentAccount); - return { loggedInClient, columnBoardNode, columnNode, cardNode }; + return { loggedInClient, cardNode, submissionItemNode }; }; + it('should return status 201', async () => { + const { loggedInClient, submissionItemNode } = await setup(); + + const response = await loggedInClient.post(`${submissionItemNode.id}/elements`, { + type: ContentElementType.RICH_TEXT, + }); + + expect(response.statusCode).toEqual(201); + }); + + it('should return the created content element of type RICH_TEXT', async () => { + const { loggedInClient, submissionItemNode } = await setup(); + + const response = await loggedInClient.post(`${submissionItemNode.id}/elements`, { + type: ContentElementType.RICH_TEXT, + }); + + expect((response.body as AnyContentElementResponse).type).toEqual(ContentElementType.RICH_TEXT); + }); + + 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); + }); + + 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); + }); + + 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(); + + 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 { 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 user not a student', () => { + const setup = async () => { + await cleanupCollections(em); + + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); + + const course = courseFactory.build({ teachers: [teacherUser] }); + + 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 }); + 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(teacherAccount); - describe('with student belonging to course', () => { + 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 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/api-test/submission-item-create.api.spec.ts b/apps/server/src/modules/board/controller/api-test/submission-item-create.api.spec.ts index 0fb70869e18..9155e1faf16 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/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 cffdcd64467..4e81d342849 100644 --- a/apps/server/src/modules/board/controller/board-submission.controller.ts +++ b/apps/server/src/modules/board/controller/board-submission.controller.ts @@ -1,13 +1,30 @@ -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, Authenticate, CurrentUser } from '@modules/authentication'; import { SubmissionsResponse } from './dto/submission-item/submissions.response'; import { CardUc } from '../uc'; import { ElementUc } from '../uc/element.uc'; import { SubmissionItemUc } from '../uc/submission-item.uc'; -import { SubmissionContainerUrlParams, SubmissionItemUrlParams, UpdateSubmissionItemBodyParams } from './dto'; -import { SubmissionItemResponseMapper } from './mapper'; +import { + CreateContentElementBodyParams, + FileElementResponse, + RichTextElementResponse, + SubmissionContainerUrlParams, + SubmissionItemUrlParams, + UpdateSubmissionItemBodyParams, +} from './dto'; +import { ContentElementResponseFactory, SubmissionItemResponseMapper } from './mapper'; @ApiTags('Board Submission') @Authenticate('jwt') @@ -56,4 +73,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.mapSubmissionContentToResponse(element); + + return response; + } } 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/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; 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 d9b709d8d67..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 @@ -1,15 +1,16 @@ import { ApiProperty, ApiPropertyOptional, getSchemaPath } from '@nestjs/swagger'; -import { ContentElementType, InputFormat } from '@shared/domain'; +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'; 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 5b2fd522476..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 @@ -1,12 +1,15 @@ -import { ApiProperty } from '@nestjs/swagger'; +import { ApiExtraModels, ApiProperty, getSchemaPath } from '@nestjs/swagger'; import { TimestampsResponse } from '../timestamps.response'; +import { FileElementResponse, RichTextElementResponse } from '../element'; +@ApiExtraModels(FileElementResponse, RichTextElementResponse) export class SubmissionItemResponse { - constructor({ id, timestamps, completed, userId }: SubmissionItemResponse) { + constructor({ id, timestamps, completed, userId, elements }: SubmissionItemResponse) { this.id = id; this.timestamps = timestamps; this.completed = completed; this.userId = userId; + this.elements = elements; } @ApiProperty({ pattern: '[a-f0-9]{24}' }) @@ -20,4 +23,12 @@ export class SubmissionItemResponse { @ApiProperty({ pattern: '[a-f0-9]{24}' }) userId: string; + + @ApiProperty({ + type: 'array', + items: { + oneOf: [{ $ref: getSchemaPath(FileElementResponse) }, { $ref: getSchemaPath(RichTextElementResponse) }], + }, + }) + 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 229d2d6f2e1..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.' }) @@ -133,7 +133,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/content-element-response.factory.ts b/apps/server/src/modules/board/controller/mapper/content-element-response.factory.ts index bda46e4b73f..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,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, 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 53efb37a482..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,5 +1,6 @@ -import { SubmissionItem, UserBoardRoles } from '@shared/domain'; +import { FileElement, isSubmissionItemContent, RichTextElement, SubmissionItem, UserBoardRoles } from '@shared/domain'; import { SubmissionItemResponse, SubmissionsResponse, TimestampsResponse, UserDataResponse } from '../dto'; +import { ContentElementResponseFactory } from './content-element-response.factory'; export class SubmissionItemResponseMapper { private static instance: SubmissionItemResponseMapper; @@ -14,7 +15,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)); @@ -23,7 +24,8 @@ export class SubmissionItemResponseMapper { return response; } - public mapSubmissionsToResponse(submissionItem: SubmissionItem): SubmissionItemResponse { + public mapSubmissionItemToResponse(submissionItem: SubmissionItem): SubmissionItemResponse { + const children: (FileElement | RichTextElement)[] = submissionItem.children.filter(isSubmissionItemContent); const result = new SubmissionItemResponse({ completed: submissionItem.completed, id: submissionItem.id, @@ -32,6 +34,7 @@ export class SubmissionItemResponseMapper { createdAt: submissionItem.createdAt, }), userId: submissionItem.userId, + elements: children.map((element) => ContentElementResponseFactory.mapSubmissionContentToResponse(element)), }); 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 18b0583daa1..6e2b375991e 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 @@ -155,7 +155,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, @@ -163,7 +167,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 5e8249f1fee..e379cd5c788 100644 --- a/apps/server/src/modules/board/repo/recursive-save.visitor.ts +++ b/apps/server/src/modules/board/repo/recursive-save.visitor.ts @@ -62,8 +62,7 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { context: columnBoard.context, }); - this.createOrUpdateBoardNode(boardNode); - this.visitChildren(columnBoard, boardNode); + this.saveRecursive(boardNode, columnBoard); } visitColumn(column: Column): void { @@ -76,8 +75,7 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { position: parentData?.position, }); - this.createOrUpdateBoardNode(boardNode); - this.visitChildren(column, boardNode); + this.saveRecursive(boardNode, column); } visitCard(card: Card): void { @@ -91,8 +89,7 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { position: parentData?.position, }); - this.createOrUpdateBoardNode(boardNode); - this.visitChildren(card, boardNode); + this.saveRecursive(boardNode, card); } visitFileElement(fileElement: FileElement): void { @@ -106,8 +103,7 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { position: parentData?.position, }); - this.createOrUpdateBoardNode(boardNode); - this.visitChildren(fileElement, boardNode); + this.saveRecursive(boardNode, fileElement); } visitLinkElement(linkElement: LinkElement): void { @@ -137,8 +133,7 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { position: parentData?.position, }); - this.createOrUpdateBoardNode(boardNode); - this.visitChildren(richTextElement, boardNode); + this.saveRecursive(boardNode, richTextElement); } visitSubmissionContainerElement(submissionContainerElement: SubmissionContainerElement): void { @@ -151,22 +146,20 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { dueDate: submissionContainerElement.dueDate, }); - 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); } visitExternalToolElement(externalToolElement: ExternalToolElement): void { @@ -185,14 +178,14 @@ 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); }); } - 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`); @@ -200,6 +193,12 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor { this.parentsMap.set(child.id, { boardNode: parentNode, position }); } + private saveRecursive(boardNode: BoardNode, anyBoardDo: AnyBoardDo): void { + this.createOrUpdateBoardNode(boardNode); + this.visitChildren(anyBoardDo, boardNode); + } + + // 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/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/service/content-element.service.ts b/apps/server/src/modules/board/service/content-element.service.ts index a7c957173f3..4404f51fc3e 100644 --- a/apps/server/src/modules/board/service/content-element.service.ts +++ b/apps/server/src/modules/board/service/content-element.service.ts @@ -1,11 +1,13 @@ import { Injectable, NotFoundException } from '@nestjs/common'; import { + AnyBoardDo, AnyContentElementDo, Card, ContentElementFactory, ContentElementType, EntityId, isAnyContentElement, + SubmissionItem, } from '@shared/domain'; import { AnyElementContentBody } from '../controller/dto'; import { BoardDoRepo } from '../repo'; @@ -32,7 +34,15 @@ export class ContentElementService { return element; } - async create(parent: Card, type: ContentElementType): Promise { + 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 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/base.uc.ts b/apps/server/src/modules/board/uc/base.uc.ts new file mode 100644 index 00000000000..e5d88e7b13d --- /dev/null +++ b/apps/server/src/modules/board/uc/base.uc.ts @@ -0,0 +1,50 @@ +import { AnyBoardDo, EntityId, SubmissionItem, UserRoleEnum } from '@shared/domain'; +import { ForbiddenException } from '@nestjs/common'; +import { AuthorizationService, Action } from '@modules/authorization'; +import { BoardDoAuthorizableService } from '../service'; + +export abstract class BaseUc { + constructor( + protected readonly authorizationService: AuthorizationService, + protected readonly boardDoAuthorizableService: BoardDoAuthorizableService + ) {} + + protected async checkPermission( + userId: EntityId, + anyBoardDo: AnyBoardDo, + action: Action, + requiredUserRole?: UserRoleEnum + ): Promise { + const user = await this.authorizationService.getUserWithPermissions(userId); + const boardDoAuthorizable = await this.boardDoAuthorizableService.getBoardAuthorizable(anyBoardDo); + 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 checkSubmissionItemWritePermission(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.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 36d45dcd5fc..62559bd966f 100644 --- a/apps/server/src/modules/board/uc/board.uc.ts +++ b/apps/server/src/modules/board/uc/board.uc.ts @@ -1,29 +1,24 @@ -import { Injectable } from '@nestjs/common'; -import { - AnyBoardDo, - 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'; 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, + @Inject(forwardRef(() => AuthorizationService)) + 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); } @@ -73,15 +68,6 @@ export class BoardUc { 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, @@ -98,71 +84,4 @@ export class BoardUc { 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); - } - - 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.spec.ts b/apps/server/src/modules/board/uc/card.uc.spec.ts index fce595085c7..7df87747b9c 100644 --- a/apps/server/src/modules/board/uc/card.uc.spec.ts +++ b/apps/server/src/modules/board/uc/card.uc.spec.ts @@ -1,13 +1,12 @@ 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'; 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,6 +94,144 @@ describe(CardUc.name, () => { }); }); + 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); + + return { user, board, boardId, column, card, createCardBodyParams }; + }; + + 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', () => { + 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(); + 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', () => { + 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(); + + 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 +289,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 488f93fd4d8..97289984922 100644 --- a/apps/server/src/modules/board/uc/card.uc.ts +++ b/apps/server/src/modules/board/uc/card.uc.ts @@ -1,18 +1,21 @@ -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'; 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, + @Inject(forwardRef(() => AuthorizationService)) + 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); } @@ -25,6 +28,33 @@ export class CardUc { 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( @@ -46,15 +76,6 @@ export class CardUc { 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, @@ -72,14 +93,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/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..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,10 +10,10 @@ 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 { BoardDoAuthorizableService, ContentElementService } from '../service'; -import { SubmissionItemService } from '../service/submission-item.service'; +import { ForbiddenException } from '@nestjs/common'; +import { BoardDoAuthorizableService, ContentElementService, SubmissionItemService } from '../service'; import { ElementUc } from './element.uc'; describe(ElementUc.name, () => { @@ -123,15 +123,102 @@ 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(); + 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 = () => { 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 () => { @@ -150,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 () => { @@ -171,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/element.uc.ts b/apps/server/src/modules/board/uc/element.uc.ts index 6f71f202e66..d094e758fd5 100644 --- a/apps/server/src/modules/board/uc/element.uc.ts +++ b/apps/server/src/modules/board/uc/element.uc.ts @@ -1,6 +1,7 @@ -import { forwardRef, HttpException, HttpStatus, Inject, Injectable } from '@nestjs/common'; +import { ForbiddenException, forwardRef, Inject, Injectable, UnprocessableEntityException } from '@nestjs/common'; import { AnyBoardDo, + AnyContentElementDo, EntityId, isSubmissionContainerElement, isSubmissionItem, @@ -12,26 +13,50 @@ import { AuthorizationService, Action } from '@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); } - async updateElementContent(userId: EntityId, elementId: EntityId, content: AnyElementContentBody) { - let element = await this.elementService.findById(elementId); + async updateElementContent( + userId: EntityId, + elementId: EntityId, + content: AnyElementContentBody + ): Promise { + const element = await this.getElementWithWritePermission(userId, 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); + } - await this.checkPermission(userId, element, Action.write); + private async getElementWithWritePermission(userId: EntityId, elementId: EntityId): Promise { + const element = await this.elementService.findById(elementId); + + const parent: AnyBoardDo = await this.elementService.findParentOfId(elementId); + + if (isSubmissionItem(parent)) { + await this.checkSubmissionItemWritePermission(userId, parent); + } else { + await this.checkPermission(userId, element, Action.write); + } - element = await this.elementService.update(element, content); return element; } @@ -43,16 +68,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 +81,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' ); } @@ -72,18 +92,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); - } } 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.spec.ts b/apps/server/src/modules/board/uc/submission-item.uc.spec.ts index 8e9b0d052b7..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 @@ -1,15 +1,22 @@ 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 { Action, AuthorizationService } from '@modules/authorization'; +import { + BadRequestException, + ForbiddenException, + NotFoundException, + UnprocessableEntityException, +} from '@nestjs/common'; import { BoardDoAuthorizableService, ContentElementService, SubmissionItemService } from '../service'; import { SubmissionItemUc } from './submission-item.uc'; @@ -193,7 +200,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') + ); }); }); }); @@ -232,16 +241,102 @@ 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); }); }); + + describe('createElement', () => { + describe('when the user is a student', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const submissionItem = submissionItemFactory.build({ + userId: user.id, + }); + + submissionItemService.findById.mockResolvedValue(submissionItem); + + const element = richTextElementFactory.build(); + + boardDoAuthorizableService.getBoardAuthorizable.mockResolvedValue( + new BoardDoAuthorizable({ + users: [{ userId: user.id, roles: [BoardRoles.READER], userRoleEnum: UserRoleEnum.STUDENT }], + id: submissionItem.id, + }) + ); + + return { element, submissionItem, user }; + }; + + 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); + }); + + it('should authorize', async () => { + const { element, submissionItem, user } = setup(); + elementService.create.mockResolvedValueOnce(element); + + 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 { 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 otherElement = submissionContainerElementFactory.build(); + elementService.create.mockResolvedValueOnce(otherElement); + await expect(uc.createElement(user.id, submissionItem.id, ContentElementType.RICH_TEXT)).rejects.toThrow( + new UnprocessableEntityException() + ); + }); + }); + }); }); 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 4748b64d84e..3fb87f2b5dc 100644 --- a/apps/server/src/modules/board/uc/submission-item.uc.ts +++ b/apps/server/src/modules/board/uc/submission-item.uc.ts @@ -1,28 +1,38 @@ -import { ForbiddenException, forwardRef, HttpException, HttpStatus, Inject, Injectable } from '@nestjs/common'; import { - AnyBoardDo, + BadRequestException, + forwardRef, + Inject, + Injectable, + NotFoundException, + UnprocessableEntityException, +} from '@nestjs/common'; +import { + ContentElementType, EntityId, + FileElement, + isFileElement, + isRichTextElement, isSubmissionContainerElement, isSubmissionItem, + RichTextElement, SubmissionItem, UserBoardRoles, UserRoleEnum, } from '@shared/domain'; -import { Logger } from '@src/core/logger'; import { AuthorizationService, Action } from '@modules/authorization'; 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( @@ -32,7 +42,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 NotFoundException('Could not find a submission container with this id'); } await this.checkPermission(userId, submissionContainerElement, Action.read); @@ -57,46 +67,31 @@ 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.checkSubmissionItemWritePermission(userId, submissionItem); await this.submissionItemService.update(submissionItem, completed); return submissionItem; } - 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'); + async createElement( + userId: EntityId, + submissionItemId: EntityId, + type: ContentElementType + ): Promise { + if (type !== ContentElementType.RICH_TEXT && type !== ContentElementType.FILE) { + throw new BadRequestException(); } - // TODO do this with permission instead of role and using authorizable rules - if (userRoleEnum === UserRoleEnum.STUDENT) { - return true; - } + const submissionItem = await this.submissionItemService.findById(submissionItemId); - return false; - } + await this.checkSubmissionItemWritePermission(userId, 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 element = await this.elementService.create(submissionItem, type); + + 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/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'; 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 cb072f37455..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,4 +1,4 @@ -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'; @@ -19,10 +19,10 @@ 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 - return false; + const allowed = isFileElement(child) || isRichTextElement(child); + + return allowed; } accept(visitor: BoardCompositeVisitor): void { @@ -42,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/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/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 9652b635b2580c9241dfac1b2e7f8f375960e618 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= <103562092+MarvinOehlerkingCap@users.noreply.github.com> Date: Fri, 3 Nov 2023 13:17:41 +0100 Subject: [PATCH 3/7] N21-1128 Add automated test for Brandenburg Central LDAP Login (#4510) --- .../controllers/api-test/login.api.spec.ts | 143 ++++++++++++++---- 1 file changed, 113 insertions(+), 30 deletions(-) diff --git a/apps/server/src/modules/authentication/controllers/api-test/login.api.spec.ts b/apps/server/src/modules/authentication/controllers/api-test/login.api.spec.ts index 253d692055d..7da3c21eab9 100644 --- a/apps/server/src/modules/authentication/controllers/api-test/login.api.spec.ts +++ b/apps/server/src/modules/authentication/controllers/api-test/login.api.spec.ts @@ -1,16 +1,17 @@ import { EntityManager } from '@mikro-orm/core'; +import { SSOErrorCode } from '@modules/oauth/loggable'; +import { OauthTokenResponse } from '@modules/oauth/service/dto'; +import { ServerTestModule } from '@modules/server/server.module'; import { HttpStatus, INestApplication } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { Account, RoleName, SchoolEntity, SystemEntity, User } from '@shared/domain'; import { accountFactory, roleFactory, schoolFactory, systemFactory, userFactory } from '@shared/testing'; -import { SSOErrorCode } from '@modules/oauth/loggable'; -import { OauthTokenResponse } from '@modules/oauth/service/dto'; -import { ServerTestModule } from '@modules/server/server.module'; import axios from 'axios'; import MockAdapter from 'axios-mock-adapter'; import crypto, { KeyPairKeyObjectResult } from 'crypto'; import jwt from 'jsonwebtoken'; import request, { Response } from 'supertest'; +import { ICurrentUser } from '../../interface'; import { LdapAuthorizationBodyParams, LocalAuthorizationBodyParams, OauthLoginResponse } from '../dto'; const ldapAccountUserName = 'ldapAccountUserName'; @@ -145,41 +146,38 @@ describe('Login Controller (api)', () => { }); describe('loginLdap', () => { - let account: Account; - let user: User; - let school: SchoolEntity; - let system: SystemEntity; - - beforeAll(async () => { - const schoolExternalId = 'mockSchoolExternalId'; - system = systemFactory.withLdapConfig().buildWithId({}); - school = schoolFactory.buildWithId({ systems: [system], externalId: schoolExternalId }); - const studentRoles = roleFactory.build({ name: RoleName.STUDENT, permissions: [] }); + describe('when user login succeeds', () => { + const setup = async () => { + const schoolExternalId = 'mockSchoolExternalId'; + const system: SystemEntity = systemFactory.withLdapConfig().buildWithId({}); + const school: SchoolEntity = schoolFactory.buildWithId({ systems: [system], externalId: schoolExternalId }); + const studentRoles = roleFactory.build({ name: RoleName.STUDENT, permissions: [] }); - user = userFactory.buildWithId({ school, roles: [studentRoles], ldapDn: mockUserLdapDN }); + const user: User = userFactory.buildWithId({ school, roles: [studentRoles], ldapDn: mockUserLdapDN }); - account = accountFactory.buildWithId({ - userId: user.id, - username: `${schoolExternalId}/${ldapAccountUserName}`.toLowerCase(), - systemId: system.id, - }); + const account: Account = accountFactory.buildWithId({ + userId: user.id, + username: `${schoolExternalId}/${ldapAccountUserName}`.toLowerCase(), + systemId: system.id, + }); - em.persist(system); - em.persist(school); - em.persist(studentRoles); - em.persist(user); - em.persist(account); - await em.flush(); - }); + await em.persistAndFlush([system, school, studentRoles, user, account]); - describe('when user login succeeds', () => { - it('should return jwt', async () => { const params: LdapAuthorizationBodyParams = { username: ldapAccountUserName, password: defaultPassword, schoolId: school.id, systemId: system.id, }; + + return { + params, + }; + }; + + it('should return jwt', async () => { + const { params } = await setup(); + const response = await request(app.getHttpServer()).post(`${basePath}/ldap`).send(params); // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access @@ -199,14 +197,99 @@ describe('Login Controller (api)', () => { }); describe('when user login fails', () => { - it('should return error response', async () => { + const setup = async () => { + const schoolExternalId = 'mockSchoolExternalId'; + const system: SystemEntity = systemFactory.withLdapConfig().buildWithId({}); + const school: SchoolEntity = schoolFactory.buildWithId({ systems: [system], externalId: schoolExternalId }); + const studentRoles = roleFactory.build({ name: RoleName.STUDENT, permissions: [] }); + + const user: User = userFactory.buildWithId({ school, roles: [studentRoles], ldapDn: mockUserLdapDN }); + + const account: Account = accountFactory.buildWithId({ + userId: user.id, + username: `${schoolExternalId}/${ldapAccountUserName}`.toLowerCase(), + systemId: system.id, + }); + + await em.persistAndFlush([system, school, studentRoles, user, account]); + const params: LdapAuthorizationBodyParams = { username: 'nonExistentUser', password: 'wrongPassword', schoolId: school.id, systemId: system.id, }; - await request(app.getHttpServer()).post(`${basePath}/ldap`).send(params).expect(401); + + return { + params, + }; + }; + + it('should return error response', async () => { + const { params } = await setup(); + + const response = await request(app.getHttpServer()).post(`${basePath}/ldap`).send(params); + + expect(response.status).toEqual(HttpStatus.UNAUTHORIZED); + }); + }); + + describe('when logging in as a user of the Central LDAP of Brandenburg', () => { + const setup = async () => { + const officialSchoolNumber = '01234'; + const system: SystemEntity = systemFactory.withLdapConfig().buildWithId({}); + const school: SchoolEntity = schoolFactory.buildWithId({ + systems: [system], + externalId: officialSchoolNumber, + officialSchoolNumber, + }); + const studentRole = roleFactory.build({ name: RoleName.STUDENT, permissions: [] }); + + const user: User = userFactory.buildWithId({ school, roles: [studentRole], ldapDn: mockUserLdapDN }); + + const account: Account = accountFactory.buildWithId({ + userId: user.id, + username: `${officialSchoolNumber}/${ldapAccountUserName}`.toLowerCase(), + systemId: system.id, + }); + + await em.persistAndFlush([system, school, studentRole, user, account]); + + const params: LdapAuthorizationBodyParams = { + username: ldapAccountUserName, + password: defaultPassword, + schoolId: school.id, + systemId: system.id, + }; + + return { + params, + user, + account, + school, + system, + studentRole, + }; + }; + + it('should return a jwt', async () => { + const { params, user, account, school, system, studentRole } = await setup(); + + const response = await request(app.getHttpServer()).post(`${basePath}/ldap`).send(params); + + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + expect(response.body.accessToken).toBeDefined(); + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-argument + const decodedToken = jwt.decode(response.body.accessToken); + expect(decodedToken).toMatchObject({ + userId: user.id, + systemId: system.id, + roles: [studentRole.id], + schoolId: school.id, + accountId: account.id, + isExternalUser: false, + }); + expect(decodedToken).not.toHaveProperty('externalIdToken'); }); }); }); From 8da2a40248dfc4a425c12ca7a3e76c3be78c5758 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= <103562092+MarvinOehlerkingCap@users.noreply.github.com> Date: Fri, 3 Nov 2023 13:58:57 +0100 Subject: [PATCH 4/7] N21-1310 Refactor user login migration api spec (#4520) --- apps/server/src/modules/provisioning/index.ts | 2 +- .../modules/provisioning/strategy/index.ts | 2 +- .../provisioning/strategy/sanis/index.ts | 3 + .../api-test/user-login-migration.api.spec.ts | 234 ++++++++++++------ .../response/user-login-migration.response.ts | 4 + .../user-login-migration.controller.ts | 5 +- .../mapper/user-login-migration.mapper.ts | 3 +- .../src/shared/testing/factory/index.ts | 1 + .../shared/testing/user-role-permissions.ts | 1 + 9 files changed, 170 insertions(+), 85 deletions(-) create mode 100644 apps/server/src/modules/provisioning/strategy/sanis/index.ts diff --git a/apps/server/src/modules/provisioning/index.ts b/apps/server/src/modules/provisioning/index.ts index b9814818220..0e0bc64d04a 100644 --- a/apps/server/src/modules/provisioning/index.ts +++ b/apps/server/src/modules/provisioning/index.ts @@ -1,4 +1,4 @@ export * from './provisioning.module'; export * from './dto/provisioning.dto'; export * from './service/provisioning.service'; -export * from './strategy/index'; +export * from './strategy'; diff --git a/apps/server/src/modules/provisioning/strategy/index.ts b/apps/server/src/modules/provisioning/strategy/index.ts index 369357cc351..e0eeeae1776 100644 --- a/apps/server/src/modules/provisioning/strategy/index.ts +++ b/apps/server/src/modules/provisioning/strategy/index.ts @@ -2,4 +2,4 @@ export * from './base.strategy'; export * from './iserv/iserv.strategy'; export * from './oidc/oidc.strategy'; export * from './oidc-mock/oidc-mock.strategy'; -export * from './sanis/sanis.strategy'; +export * from './sanis'; diff --git a/apps/server/src/modules/provisioning/strategy/sanis/index.ts b/apps/server/src/modules/provisioning/strategy/sanis/index.ts new file mode 100644 index 00000000000..4f98cbd73e9 --- /dev/null +++ b/apps/server/src/modules/provisioning/strategy/sanis/index.ts @@ -0,0 +1,3 @@ +export * from './response'; +export * from './sanis.strategy'; +export * from './sanis-response.mapper'; diff --git a/apps/server/src/modules/user-login-migration/controller/api-test/user-login-migration.api.spec.ts b/apps/server/src/modules/user-login-migration/controller/api-test/user-login-migration.api.spec.ts index b2de2ac9fc0..154148c1fa0 100644 --- a/apps/server/src/modules/user-login-migration/controller/api-test/user-login-migration.api.spec.ts +++ b/apps/server/src/modules/user-login-migration/controller/api-test/user-login-migration.api.spec.ts @@ -1,12 +1,16 @@ import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { EntityManager, ObjectId } from '@mikro-orm/mongodb'; +import { OauthTokenResponse } from '@modules/oauth/service/dto'; +import { SanisResponse, SanisRole } from '@modules/provisioning'; +import { ServerTestModule } from '@modules/server'; import { HttpStatus, INestApplication } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; -import { Permission, SchoolEntity, SystemEntity, User } from '@shared/domain'; +import { SchoolEntity, SystemEntity, User } from '@shared/domain'; import { UserLoginMigrationEntity } from '@shared/domain/entity/user-login-migration.entity'; import { SystemProvisioningStrategy } from '@shared/domain/interface/system-provisioning.strategy'; import { cleanupCollections, + JwtTestFactory, schoolFactory, systemFactory, TestApiClient, @@ -14,14 +18,11 @@ import { userFactory, userLoginMigrationFactory, } from '@shared/testing'; -import { JwtTestFactory } from '@shared/testing/factory/jwt.test.factory'; -import { OauthTokenResponse } from '@modules/oauth/service/dto'; -import { ServerTestModule } from '@modules/server'; +import { ErrorResponse } from '@src/core/error/dto'; import axios from 'axios'; import MockAdapter from 'axios-mock-adapter'; import { UUID } from 'bson'; import { Response } from 'supertest'; -import { SanisResponse, SanisRole } from '@modules/provisioning/strategy/sanis/response'; import { UserLoginMigrationResponse } from '../dto'; import { Oauth2MigrationParams } from '../dto/oauth2-migration.params'; @@ -37,6 +38,7 @@ jest.mock('jwks-rsa', () => () => { getSigningKeys: jest.fn(), }; }); + describe('UserLoginMigrationController (API)', () => { let app: INestApplication; let em: EntityManager; @@ -80,8 +82,8 @@ describe('UserLoginMigrationController (API)', () => { sourceSystem, startedAt: date, mandatorySince: date, - closedAt: undefined, - finishedAt: undefined, + closedAt: date, + finishedAt: date, }); const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher({ school }); @@ -107,6 +109,7 @@ describe('UserLoginMigrationController (API)', () => { expect(response.body).toEqual({ data: [ { + id: userLoginMigration.id, sourceSystemId: sourceSystem.id, targetSystemId: targetSystem.id, startedAt: userLoginMigration.startedAt.toISOString(), @@ -130,7 +133,7 @@ describe('UserLoginMigrationController (API)', () => { }); describe('[GET] /user-login-migrations/schools/:schoolId', () => { - describe('when a user login migration is found', () => { + describe('when a user login migration exists', () => { const setup = async () => { const date: Date = new Date(2023, 5, 4); const sourceSystem: SystemEntity = systemFactory.withLdapConfig().buildWithId({ alias: 'SourceSystem' }); @@ -147,9 +150,7 @@ describe('UserLoginMigrationController (API)', () => { closedAt: undefined, finishedAt: undefined, }); - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser, userLoginMigration]); @@ -171,6 +172,7 @@ describe('UserLoginMigrationController (API)', () => { expect(response.status).toEqual(HttpStatus.OK); expect(response.body).toEqual({ + id: userLoginMigration.id, sourceSystemId: sourceSystem.id, targetSystemId: targetSystem.id, startedAt: userLoginMigration.startedAt.toISOString(), @@ -181,12 +183,10 @@ describe('UserLoginMigrationController (API)', () => { }); }); - describe('when no user login migration is found', () => { + describe('when no user login migration exists', () => { const setup = async () => { const school: SchoolEntity = schoolFactory.buildWithId(); - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([school, adminAccount, adminUser]); @@ -198,13 +198,26 @@ describe('UserLoginMigrationController (API)', () => { }; }; - it('should return the users migration', async () => { + it('should have the status "not found"', async () => { const { loggedInClient, school } = await setup(); const response: Response = await loggedInClient.get(`schools/${school.id}`); expect(response.status).toEqual(HttpStatus.NOT_FOUND); }); + + it('should return an error response', async () => { + const { loggedInClient, school } = await setup(); + + const response: Response = await loggedInClient.get(`schools/${school.id}`); + + expect(response.body).toEqual({ + message: 'Not Found', + type: 'NOT_FOUND', + code: 404, + title: 'Not Found', + }); + }); }); describe('when unauthorized', () => { @@ -217,7 +230,7 @@ describe('UserLoginMigrationController (API)', () => { }); describe('[POST] /start', () => { - describe('when current User start the migration successfully', () => { + describe('when current user start the migration successfully', () => { const setup = async () => { const sourceSystem: SystemEntity = systemFactory.withLdapConfig().buildWithId({ alias: 'SourceSystem' }); const targetSystem: SystemEntity = systemFactory.withOauthConfig().buildWithId({ alias: 'SANIS' }); @@ -226,9 +239,7 @@ describe('UserLoginMigrationController (API)', () => { officialSchoolNumber: '12345', }); - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser]); @@ -236,6 +247,8 @@ describe('UserLoginMigrationController (API)', () => { return { loggedInClient, + sourceSystem, + targetSystem, }; }; @@ -246,6 +259,28 @@ describe('UserLoginMigrationController (API)', () => { expect(response.status).toEqual(HttpStatus.CREATED); }); + + it('should return the user login migration', async () => { + const { loggedInClient, sourceSystem, targetSystem } = await setup(); + + const response: Response = await loggedInClient.post(`/start`); + + expect(response.body).toEqual({ + id: expect.any(String), + sourceSystemId: sourceSystem.id, + startedAt: expect.any(String), + targetSystemId: targetSystem.id, + }); + }); + + it('should should change the database correctly', async () => { + const { loggedInClient } = await setup(); + + const response: Response = await loggedInClient.post(`/start`); + + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-assignment + await em.findOneOrFail(UserLoginMigrationEntity, { id: response.body.id }); + }); }); describe('when current User start the migration and is not authorized', () => { @@ -296,9 +331,7 @@ describe('UserLoginMigrationController (API)', () => { mandatorySince: date, }); - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser, userLoginMigration]); @@ -339,9 +372,7 @@ describe('UserLoginMigrationController (API)', () => { finishedAt: date, }); - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser, userLoginMigration]); @@ -370,9 +401,7 @@ describe('UserLoginMigrationController (API)', () => { systems: [sourceSystem], }); - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser]); @@ -466,10 +495,10 @@ describe('UserLoginMigrationController (API)', () => { startedAt: new Date('2022-12-17T03:24:00'), }); - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin( - { school, externalId: 'externalUserId' }, - [Permission.USER_LOGIN_MIGRATION_ADMIN] - ); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ + school, + externalId: 'externalUserId', + }); await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser, userLoginMigration]); em.clear(); @@ -610,9 +639,7 @@ describe('UserLoginMigrationController (API)', () => { }); school.userLoginMigration = userLoginMigration; - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser, userLoginMigration]); em.clear(); @@ -642,16 +669,29 @@ describe('UserLoginMigrationController (API)', () => { expect(response.body).not.toHaveProperty('closedAt'); expect(response.body).not.toHaveProperty('finishedAt'); }); + + it('should should change the database correctly', async () => { + const { loggedInClient } = await setup(); + + const response: Response = await loggedInClient.put(`/restart`); + + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-assignment + const entity = await em.findOneOrFail(UserLoginMigrationEntity, { id: response.body.id }); + + expect(entity.startedAt).toBeDefined(); + expect(entity.closedAt).toBeUndefined(); + expect(entity.finishedAt).toBeUndefined(); + }); }); describe('when invalid User restart the migration', () => { const setup = async () => { - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin(); + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); - await em.persistAndFlush([adminAccount, adminUser]); + await em.persistAndFlush([teacherAccount, teacherUser]); em.clear(); - const loggedInClient = await testApiClient.login(adminAccount); + const loggedInClient = await testApiClient.login(teacherAccount); return { loggedInClient, @@ -692,9 +732,7 @@ describe('UserLoginMigrationController (API)', () => { }); school.userLoginMigration = userLoginMigration; - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser, userLoginMigration]); em.clear(); @@ -734,9 +772,7 @@ describe('UserLoginMigrationController (API)', () => { }); school.userLoginMigration = userLoginMigration; - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser, userLoginMigration]); em.clear(); @@ -778,9 +814,7 @@ describe('UserLoginMigrationController (API)', () => { }); school.userLoginMigration = userLoginMigration; - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser, userLoginMigration]); em.clear(); @@ -801,6 +835,17 @@ describe('UserLoginMigrationController (API)', () => { const responseBody = response.body as UserLoginMigrationResponse; expect(responseBody.mandatorySince).toBeDefined(); }); + + it('should should change the database correctly', async () => { + const { loggedInClient } = await setup(); + + const response: Response = await loggedInClient.put('/mandatory', { mandatory: true }); + + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-assignment + const entity = await em.findOneOrFail(UserLoginMigrationEntity, { id: response.body.id }); + + expect(entity.mandatorySince).toBeDefined(); + }); }); describe('when migration is set from mandatory to optional', () => { @@ -818,13 +863,10 @@ describe('UserLoginMigrationController (API)', () => { sourceSystem, startedAt: new Date(2023, 1, 4), mandatorySince: new Date(2023, 1, 4), - closedAt: new Date(2023, 1, 5), }); school.userLoginMigration = userLoginMigration; - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser, userLoginMigration]); em.clear(); @@ -845,6 +887,17 @@ describe('UserLoginMigrationController (API)', () => { const responseBody = response.body as UserLoginMigrationResponse; expect(responseBody.mandatorySince).toBeUndefined(); }); + + it('should should change the database correctly', async () => { + const { loggedInClient } = await setup(); + + const response: Response = await loggedInClient.put('/mandatory', { mandatory: false }); + + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-assignment + const entity = await em.findOneOrFail(UserLoginMigrationEntity, { id: response.body.id }); + + expect(entity.mandatorySince).toBeUndefined(); + }); }); describe('when migration is not started', () => { @@ -856,9 +909,7 @@ describe('UserLoginMigrationController (API)', () => { officialSchoolNumber: '12345', }); - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser]); em.clear(); @@ -897,9 +948,7 @@ describe('UserLoginMigrationController (API)', () => { }); school.userLoginMigration = userLoginMigration; - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser, userLoginMigration]); em.clear(); @@ -947,12 +996,12 @@ describe('UserLoginMigrationController (API)', () => { }); school.userLoginMigration = userLoginMigration; - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, []); + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher({ school }); - await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser, userLoginMigration]); + await em.persistAndFlush([sourceSystem, targetSystem, school, teacherAccount, teacherUser, userLoginMigration]); em.clear(); - const loggedInClient = await testApiClient.login(adminAccount); + const loggedInClient = await testApiClient.login(teacherAccount); return { loggedInClient, @@ -990,9 +1039,7 @@ describe('UserLoginMigrationController (API)', () => { lastLoginSystemChange: new Date(2023, 1, 5), }); - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([ sourceSystem, @@ -1018,7 +1065,7 @@ describe('UserLoginMigrationController (API)', () => { const response: Response = await loggedInClient.post('/close'); - expect(response.status).toEqual(HttpStatus.CREATED); + expect(response.status).toEqual(HttpStatus.OK); }); it('should return the closed user login migration', async () => { @@ -1027,6 +1074,7 @@ describe('UserLoginMigrationController (API)', () => { const response: Response = await loggedInClient.post('/close'); expect(response.body).toEqual({ + id: expect.any(String), targetSystemId: userLoginMigration.targetSystem.id, sourceSystemId: userLoginMigration.sourceSystem?.id, startedAt: userLoginMigration.startedAt.toISOString(), @@ -1034,6 +1082,18 @@ describe('UserLoginMigrationController (API)', () => { finishedAt: expect.any(String), }); }); + + it('should should change the database correctly', async () => { + const { loggedInClient } = await setup(); + + const response: Response = await loggedInClient.post('/close'); + + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-assignment + const entity = await em.findOneOrFail(UserLoginMigrationEntity, { id: response.body.id }); + + expect(entity.closedAt).toBeDefined(); + expect(entity.finishedAt).toBeDefined(); + }); }); describe('when migration is not started', () => { @@ -1045,9 +1105,7 @@ describe('UserLoginMigrationController (API)', () => { officialSchoolNumber: '12345', }); - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser]); em.clear(); @@ -1066,6 +1124,19 @@ describe('UserLoginMigrationController (API)', () => { expect(response.status).toEqual(HttpStatus.NOT_FOUND); }); + + it('should return an error response', async () => { + const { loggedInClient } = await setup(); + + const response: Response = await loggedInClient.post('/close'); + + expect(response.body).toEqual({ + message: 'Not Found', + type: 'USER_LOGIN_MIGRATION_NOT_FOUND', + code: 404, + title: 'User Login Migration Not Found', + }); + }); }); describe('when the migration is already closed', () => { @@ -1085,9 +1156,7 @@ describe('UserLoginMigrationController (API)', () => { closedAt: new Date(2023, 1, 5), }); - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser, userLoginMigration]); em.clear(); @@ -1100,12 +1169,21 @@ describe('UserLoginMigrationController (API)', () => { }; }; + it('should return status ok', async () => { + const { loggedInClient } = await setup(); + + const response: Response = await loggedInClient.post('/close'); + + expect(response.status).toEqual(HttpStatus.OK); + }); + it('should return the same user login migration', async () => { const { loggedInClient, userLoginMigration } = await setup(); const response: Response = await loggedInClient.post('/close'); expect(response.body).toEqual({ + id: userLoginMigration.id, targetSystemId: userLoginMigration.targetSystem.id, sourceSystemId: userLoginMigration.sourceSystem?.id, startedAt: userLoginMigration.startedAt.toISOString(), @@ -1132,9 +1210,7 @@ describe('UserLoginMigrationController (API)', () => { finishedAt: new Date(2023, 1, 6), }); - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser, userLoginMigration]); em.clear(); @@ -1181,12 +1257,12 @@ describe('UserLoginMigrationController (API)', () => { closedAt: new Date(2023, 1, 5), }); - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, []); + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher({ school }); - await em.persistAndFlush([sourceSystem, targetSystem, school, adminAccount, adminUser, userLoginMigration]); + await em.persistAndFlush([sourceSystem, targetSystem, school, teacherAccount, teacherUser, userLoginMigration]); em.clear(); - const loggedInClient = await testApiClient.login(adminAccount); + const loggedInClient = await testApiClient.login(teacherAccount); return { loggedInClient, @@ -1220,9 +1296,7 @@ describe('UserLoginMigrationController (API)', () => { const user: User = userFactory.buildWithId(); - const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [ - Permission.USER_LOGIN_MIGRATION_ADMIN, - ]); + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }); await em.persistAndFlush([ sourceSystem, diff --git a/apps/server/src/modules/user-login-migration/controller/dto/response/user-login-migration.response.ts b/apps/server/src/modules/user-login-migration/controller/dto/response/user-login-migration.response.ts index 51c02793d09..efa9fd83720 100644 --- a/apps/server/src/modules/user-login-migration/controller/dto/response/user-login-migration.response.ts +++ b/apps/server/src/modules/user-login-migration/controller/dto/response/user-login-migration.response.ts @@ -1,6 +1,9 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; export class UserLoginMigrationResponse { + @ApiProperty() + id: string; + @ApiPropertyOptional({ description: 'Id of the system which is the origin of the migration', }) @@ -32,6 +35,7 @@ export class UserLoginMigrationResponse { finishedAt?: Date; constructor(props: UserLoginMigrationResponse) { + this.id = props.id; this.sourceSystemId = props.sourceSystemId; this.targetSystemId = props.targetSystemId; this.mandatorySince = props.mandatorySince; diff --git a/apps/server/src/modules/user-login-migration/controller/user-login-migration.controller.ts b/apps/server/src/modules/user-login-migration/controller/user-login-migration.controller.ts index 3e788a54725..fc1c9c9f9cf 100644 --- a/apps/server/src/modules/user-login-migration/controller/user-login-migration.controller.ts +++ b/apps/server/src/modules/user-login-migration/controller/user-login-migration.controller.ts @@ -1,4 +1,5 @@ -import { Body, Controller, Get, Param, Post, Put, Query } from '@nestjs/common'; +import { Authenticate, CurrentUser, ICurrentUser, JWT } from '@modules/authentication'; +import { Body, Controller, Get, HttpCode, HttpStatus, Param, Post, Put, Query } from '@nestjs/common'; import { ApiForbiddenResponse, ApiInternalServerErrorResponse, @@ -11,7 +12,6 @@ import { ApiUnprocessableEntityResponse, } from '@nestjs/swagger'; import { Page, UserLoginMigrationDO } from '@shared/domain'; -import { Authenticate, CurrentUser, ICurrentUser, JWT } from '@modules/authentication'; import { SchoolNumberMissingLoggableException, UserLoginMigrationAlreadyClosedLoggableException, @@ -181,6 +181,7 @@ export class UserLoginMigrationController { } @Post('close') + @HttpCode(HttpStatus.OK) @ApiUnprocessableEntityResponse({ description: 'User login migration is already closed and cannot be modified. Restart is possible.', type: UserLoginMigrationAlreadyClosedLoggableException, diff --git a/apps/server/src/modules/user-login-migration/mapper/user-login-migration.mapper.ts b/apps/server/src/modules/user-login-migration/mapper/user-login-migration.mapper.ts index cf11c639133..272e2309392 100644 --- a/apps/server/src/modules/user-login-migration/mapper/user-login-migration.mapper.ts +++ b/apps/server/src/modules/user-login-migration/mapper/user-login-migration.mapper.ts @@ -1,6 +1,6 @@ import { UserLoginMigrationDO } from '@shared/domain'; import { UserLoginMigrationResponse, UserLoginMigrationSearchParams } from '../controller/dto'; -import { UserLoginMigrationQuery } from '../uc/dto/user-login-migration-query'; +import { UserLoginMigrationQuery } from '../uc'; export class UserLoginMigrationMapper { static mapSearchParamsToQuery(searchParams: UserLoginMigrationSearchParams): UserLoginMigrationQuery { @@ -12,6 +12,7 @@ export class UserLoginMigrationMapper { static mapUserLoginMigrationDoToResponse(domainObject: UserLoginMigrationDO): UserLoginMigrationResponse { const response: UserLoginMigrationResponse = new UserLoginMigrationResponse({ + id: domainObject.id as string, sourceSystemId: domainObject.sourceSystemId, targetSystemId: domainObject.targetSystemId, startedAt: domainObject.startedAt, diff --git a/apps/server/src/shared/testing/factory/index.ts b/apps/server/src/shared/testing/factory/index.ts index d981b4ca29c..7d5ec2ab753 100644 --- a/apps/server/src/shared/testing/factory/index.ts +++ b/apps/server/src/shared/testing/factory/index.ts @@ -36,3 +36,4 @@ export * from './user-login-migration.factory'; export * from './user.do.factory'; export * from './user.factory'; export * from './legacy-file-entity-mock.factory'; +export * from './jwt.test.factory'; diff --git a/apps/server/src/shared/testing/user-role-permissions.ts b/apps/server/src/shared/testing/user-role-permissions.ts index 6c38287a37e..a3c82aecc7c 100644 --- a/apps/server/src/shared/testing/user-role-permissions.ts +++ b/apps/server/src/shared/testing/user-role-permissions.ts @@ -140,4 +140,5 @@ export const adminPermissions = [ Permission.IMPORT_USER_VIEW, Permission.SCHOOL_TOOL_ADMIN, Permission.GROUP_FULL_ADMIN, + Permission.USER_LOGIN_MIGRATION_ADMIN, ] as Permission[]; From 0d2718d1b02ca279848311548b643160e38feafe Mon Sep 17 00:00:00 2001 From: agnisa-cap Date: Fri, 3 Nov 2023 14:35:38 +0100 Subject: [PATCH 5/7] N21-1329 adds gzip as encoding for provisioning data request (#4513) --- .../strategy/sanis/sanis.strategy.spec.ts | 16 +++++++++++++++- .../strategy/sanis/sanis.strategy.ts | 5 ++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/apps/server/src/modules/provisioning/strategy/sanis/sanis.strategy.spec.ts b/apps/server/src/modules/provisioning/strategy/sanis/sanis.strategy.spec.ts index f0ea97f89fd..0ef8173c2d2 100644 --- a/apps/server/src/modules/provisioning/strategy/sanis/sanis.strategy.spec.ts +++ b/apps/server/src/modules/provisioning/strategy/sanis/sanis.strategy.spec.ts @@ -188,7 +188,21 @@ describe('SanisStrategy', () => { provisioningUrl, expect.objectContaining({ // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - headers: expect.objectContaining({ Authorization: 'Bearer sanisAccessToken' }), + headers: expect.objectContaining({ Authorization: 'Bearer sanisAccessToken', 'Accept-Encoding': 'gzip' }), + }) + ); + }); + + it('should accept gzip compressed data', async () => { + const { input, provisioningUrl } = setup(); + + await strategy.getData(input); + + expect(httpService.get).toHaveBeenCalledWith( + provisioningUrl, + expect.objectContaining({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + headers: expect.objectContaining({ 'Accept-Encoding': 'gzip' }), }) ); }); diff --git a/apps/server/src/modules/provisioning/strategy/sanis/sanis.strategy.ts b/apps/server/src/modules/provisioning/strategy/sanis/sanis.strategy.ts index a09ae204c69..ad48ae06d93 100644 --- a/apps/server/src/modules/provisioning/strategy/sanis/sanis.strategy.ts +++ b/apps/server/src/modules/provisioning/strategy/sanis/sanis.strategy.ts @@ -39,7 +39,10 @@ export class SanisProvisioningStrategy extends OidcProvisioningStrategy { } const axiosConfig: AxiosRequestConfig = { - headers: { Authorization: `Bearer ${input.accessToken}` }, + headers: { + Authorization: `Bearer ${input.accessToken}`, + 'Accept-Encoding': 'gzip', + }, }; const axiosResponse: AxiosResponse = await firstValueFrom( From a3fd43f86e2de93cc436461704e11ea88f2e64ee Mon Sep 17 00:00:00 2001 From: hoeppner-dataport <106819770+hoeppner-dataport@users.noreply.github.com> Date: Fri, 3 Nov 2023 16:59:26 +0100 Subject: [PATCH 6/7] BC-5434 - meta data endpoint (#4488) * refactor meta data extract functionality to a module and an independent endpoint * if no open graph data can be fetched, then filename is taken from the url and used as title --- apps/server/src/modules/board/board.module.ts | 2 - .../update-element-content.body.params.ts | 20 ++- .../content-element-update.visitor.spec.ts | 105 ++++++++++----- .../service/content-element-update.visitor.ts | 23 ++-- .../service/content-element.service.spec.ts | 9 -- .../board/service/content-element.service.ts | 6 +- .../server/src/modules/board/service/index.ts | 1 - .../service/open-graph-proxy.service.spec.ts | 91 ------------- .../board/service/open-graph-proxy.service.ts | 41 ------ .../meta-tag-extractor-get-data.api.spec.ts | 77 +++++++++++ .../controller/dto/index.ts | 1 + .../dto/meta-tag-extractor.response.spec.ts | 20 +++ .../dto/meta-tag-extractor.response.ts | 28 ++++ .../meta-tag-extractor/controller/index.ts | 2 + .../meta-tag-extractor.controller.ts | 29 +++++ .../controller/post-link-url.body.params.ts | 11 ++ .../src/modules/meta-tag-extractor/index.ts | 4 + .../meta-tag-extractor-api.module.ts | 13 ++ .../meta-tag-extractor.config.ts | 8 ++ .../meta-tag-extractor.module.ts | 24 ++++ .../meta-tag-extractor/service/index.ts | 1 + .../meta-tag-extractor.service.spec.ts | 122 ++++++++++++++++++ .../service/meta-tag-extractor.service.ts | 65 ++++++++++ .../modules/meta-tag-extractor/uc/index.ts | 1 + .../uc/meta-tag-extractor.uc.spec.ts | 91 +++++++++++++ .../uc/meta-tag-extractor.uc.ts | 23 ++++ .../src/modules/server/server.module.ts | 29 +++-- .../board/link-element.do.factory.ts | 2 +- 28 files changed, 642 insertions(+), 207 deletions(-) delete mode 100644 apps/server/src/modules/board/service/open-graph-proxy.service.spec.ts delete mode 100644 apps/server/src/modules/board/service/open-graph-proxy.service.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/controller/api-test/meta-tag-extractor-get-data.api.spec.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/controller/dto/index.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/controller/dto/meta-tag-extractor.response.spec.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/controller/dto/meta-tag-extractor.response.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/controller/index.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/controller/meta-tag-extractor.controller.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/controller/post-link-url.body.params.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/index.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/meta-tag-extractor-api.module.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/meta-tag-extractor.config.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/meta-tag-extractor.module.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/service/index.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/service/meta-tag-extractor.service.spec.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/service/meta-tag-extractor.service.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/uc/index.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/uc/meta-tag-extractor.uc.spec.ts create mode 100644 apps/server/src/modules/meta-tag-extractor/uc/meta-tag-extractor.uc.ts diff --git a/apps/server/src/modules/board/board.module.ts b/apps/server/src/modules/board/board.module.ts index 09d00c46bbd..7722326a21d 100644 --- a/apps/server/src/modules/board/board.module.ts +++ b/apps/server/src/modules/board/board.module.ts @@ -14,7 +14,6 @@ import { ColumnBoardService, ColumnService, ContentElementService, - OpenGraphProxyService, SubmissionItemService, } from './service'; import { BoardDoCopyService, SchoolSpecificFileCopyServiceFactory } from './service/board-do-copy-service'; @@ -38,7 +37,6 @@ import { ColumnBoardCopyService } from './service/column-board-copy.service'; BoardDoCopyService, ColumnBoardCopyService, SchoolSpecificFileCopyServiceFactory, - OpenGraphProxyService, ], exports: [ BoardDoAuthorizableService, 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 36516ae80a6..23ce88b904c 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 @@ -2,7 +2,7 @@ import { ApiProperty, ApiPropertyOptional, getSchemaPath } from '@nestjs/swagger 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'; +import { IsDate, IsEnum, IsMongoId, IsOptional, IsString, IsUrl, ValidateNested } from 'class-validator'; export abstract class ElementContentBody { @IsEnum(ContentElementType) @@ -32,10 +32,26 @@ export class FileElementContentBody extends ElementContentBody { @ApiProperty() content!: FileContentBody; } + export class LinkContentBody { - @IsString() + @IsUrl() @ApiProperty({}) url!: string; + + @IsString() + @IsOptional() + @ApiProperty({}) + title?: string; + + @IsString() + @IsOptional() + @ApiProperty({}) + description?: string; + + @IsString() + @IsOptional() + @ApiProperty({}) + imageUrl?: string; } export class LinkElementContentBody extends ElementContentBody { diff --git a/apps/server/src/modules/board/service/content-element-update.visitor.spec.ts b/apps/server/src/modules/board/service/content-element-update.visitor.spec.ts index 8a8368fce2b..0b55dfdb1de 100644 --- a/apps/server/src/modules/board/service/content-element-update.visitor.spec.ts +++ b/apps/server/src/modules/board/service/content-element-update.visitor.spec.ts @@ -11,9 +11,8 @@ import { submissionContainerElementFactory, submissionItemFactory, } from '@shared/testing'; -import { ExternalToolContentBody, FileContentBody, RichTextContentBody } from '../controller/dto'; +import { ExternalToolContentBody, FileContentBody, LinkContentBody, RichTextContentBody } from '../controller/dto'; import { ContentElementUpdateVisitor } from './content-element-update.visitor'; -import { OpenGraphProxyService } from './open-graph-proxy.service'; describe(ContentElementUpdateVisitor.name, () => { describe('when visiting an unsupported component', () => { @@ -25,8 +24,7 @@ describe(ContentElementUpdateVisitor.name, () => { content.text = 'a text'; content.inputFormat = InputFormat.RICH_TEXT_CK5; const submissionItem = submissionItemFactory.build(); - const openGraphProxyService = new OpenGraphProxyService(); - const updater = new ContentElementUpdateVisitor(content, openGraphProxyService); + const updater = new ContentElementUpdateVisitor(content); return { board, column, card, submissionItem, updater }; }; @@ -66,8 +64,7 @@ describe(ContentElementUpdateVisitor.name, () => { const content = new RichTextContentBody(); content.text = 'a text'; content.inputFormat = InputFormat.RICH_TEXT_CK5; - const openGraphProxyService = new OpenGraphProxyService(); - const updater = new ContentElementUpdateVisitor(content, openGraphProxyService); + const updater = new ContentElementUpdateVisitor(content); return { fileElement, updater }; }; @@ -79,31 +76,12 @@ describe(ContentElementUpdateVisitor.name, () => { }); }); - describe('when visiting a link element using the wrong content', () => { - const setup = () => { - const linkElement = linkElementFactory.build(); - const content = new FileContentBody(); - content.caption = 'a caption'; - const openGraphProxyService = new OpenGraphProxyService(); - const updater = new ContentElementUpdateVisitor(content, openGraphProxyService); - - return { linkElement, updater }; - }; - - it('should throw an error', async () => { - const { linkElement, updater } = setup(); - - await expect(() => updater.visitLinkElementAsync(linkElement)).rejects.toThrow(); - }); - }); - describe('when visiting a rich text element using the wrong content', () => { const setup = () => { const richTextElement = richTextElementFactory.build(); const content = new FileContentBody(); content.caption = 'a caption'; - const openGraphProxyService = new OpenGraphProxyService(); - const updater = new ContentElementUpdateVisitor(content, openGraphProxyService); + const updater = new ContentElementUpdateVisitor(content); return { richTextElement, updater }; }; @@ -121,8 +99,7 @@ describe(ContentElementUpdateVisitor.name, () => { const content = new RichTextContentBody(); content.text = 'a text'; content.inputFormat = InputFormat.RICH_TEXT_CK5; - const openGraphProxyService = new OpenGraphProxyService(); - const updater = new ContentElementUpdateVisitor(content, openGraphProxyService); + const updater = new ContentElementUpdateVisitor(content); return { submissionContainerElement, updater }; }; @@ -134,14 +111,76 @@ describe(ContentElementUpdateVisitor.name, () => { }); }); + describe('when visiting a link element', () => { + describe('when content is valid', () => { + const setup = () => { + const linkElement = linkElementFactory.build(); + const content = new LinkContentBody(); + content.url = 'https://super-example.com/'; + content.title = 'SuperExample - the best examples in the web'; + content.imageUrl = '/preview/image.jpg'; + const updater = new ContentElementUpdateVisitor(content); + + return { linkElement, content, updater }; + }; + + it('should update the content', async () => { + const { linkElement, content, updater } = setup(); + + await updater.visitLinkElementAsync(linkElement); + + expect(linkElement.url).toEqual(content.url); + expect(linkElement.title).toEqual(content.title); + expect(linkElement.imageUrl).toEqual(content.imageUrl); + }); + }); + + describe('when content is not a link element', () => { + const setup = () => { + const linkElement = linkElementFactory.build(); + const content = new FileContentBody(); + content.caption = 'a caption'; + const updater = new ContentElementUpdateVisitor(content); + + return { linkElement, updater }; + }; + + it('should throw an error', async () => { + const { linkElement, updater } = setup(); + + await expect(() => updater.visitLinkElementAsync(linkElement)).rejects.toThrow(); + }); + }); + + describe('when imageUrl for preview image is not a relative url', () => { + const setup = () => { + const linkElement = linkElementFactory.build(); + const content = new LinkContentBody(); + content.url = 'https://super-example.com/'; + content.title = 'SuperExample - the best examples in the web'; + content.imageUrl = 'https://www.external.de/fake-preview-image.jpg'; + const updater = new ContentElementUpdateVisitor(content); + + return { linkElement, content, updater }; + }; + + it('should ignore the image url', async () => { + const { linkElement, updater } = setup(); + + await updater.visitLinkElementAsync(linkElement); + + expect(linkElement.imageUrl).toBe(''); + }); + }); + }); + describe('when visiting a external tool element', () => { describe('when visiting a external tool element with valid content', () => { const setup = () => { const externalToolElement = externalToolElementFactory.build({ contextExternalToolId: undefined }); const content = new ExternalToolContentBody(); content.contextExternalToolId = new ObjectId().toHexString(); - const openGraphProxyService = new OpenGraphProxyService(); - const updater = new ContentElementUpdateVisitor(content, openGraphProxyService); + const updater = new ContentElementUpdateVisitor(content); return { externalToolElement, updater, content }; }; @@ -161,8 +200,7 @@ describe(ContentElementUpdateVisitor.name, () => { const content = new RichTextContentBody(); content.text = 'a text'; content.inputFormat = InputFormat.RICH_TEXT_CK5; - const openGraphProxyService = new OpenGraphProxyService(); - const updater = new ContentElementUpdateVisitor(content, openGraphProxyService); + const updater = new ContentElementUpdateVisitor(content); return { externalToolElement, updater }; }; @@ -178,8 +216,7 @@ describe(ContentElementUpdateVisitor.name, () => { const setup = () => { const externalToolElement = externalToolElementFactory.build(); const content = new ExternalToolContentBody(); - const openGraphProxyService = new OpenGraphProxyService(); - const updater = new ContentElementUpdateVisitor(content, openGraphProxyService); + const updater = new ContentElementUpdateVisitor(content); return { externalToolElement, updater }; }; diff --git a/apps/server/src/modules/board/service/content-element-update.visitor.ts b/apps/server/src/modules/board/service/content-element-update.visitor.ts index 0f75bcaee2d..86e3fb67985 100644 --- a/apps/server/src/modules/board/service/content-element-update.visitor.ts +++ b/apps/server/src/modules/board/service/content-element-update.visitor.ts @@ -22,13 +22,12 @@ import { RichTextContentBody, SubmissionContainerContentBody, } from '../controller/dto'; -import { OpenGraphProxyService } from './open-graph-proxy.service'; @Injectable() export class ContentElementUpdateVisitor implements BoardCompositeVisitorAsync { private readonly content: AnyElementContentBody; - constructor(content: AnyElementContentBody, private readonly openGraphProxyService: OpenGraphProxyService) { + constructor(content: AnyElementContentBody) { this.content = content; } @@ -55,13 +54,19 @@ export class ContentElementUpdateVisitor implements BoardCompositeVisitorAsync { async visitLinkElementAsync(linkElement: LinkElement): Promise { if (this.content instanceof LinkContentBody) { - const urlWithProtocol = /:\/\//.test(this.content.url) ? this.content.url : `https://${this.content.url}`; - linkElement.url = new URL(urlWithProtocol).toString(); - const openGraphData = await this.openGraphProxyService.fetchOpenGraphData(linkElement.url); - linkElement.title = openGraphData.title; - linkElement.description = openGraphData.description; - if (openGraphData.image) { - linkElement.imageUrl = openGraphData.image.url; + linkElement.url = new URL(this.content.url).toString(); + linkElement.title = this.content.title ?? ''; + linkElement.description = this.content.description ?? ''; + if (this.content.imageUrl) { + const isRelativeUrl = (url: string) => { + const fallbackHostname = 'https://www.fallback-url-if-url-is-relative.org'; + const imageUrlObject = new URL(url, fallbackHostname); + return imageUrlObject.origin === fallbackHostname; + }; + + if (isRelativeUrl(this.content.imageUrl)) { + linkElement.imageUrl = this.content.imageUrl; + } } return Promise.resolve(); } 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 90f3a73aa21..d70fe591bbb 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 @@ -26,7 +26,6 @@ import { import { BoardDoRepo } from '../repo'; import { BoardDoService } from './board-do.service'; import { ContentElementService } from './content-element.service'; -import { OpenGraphProxyService } from './open-graph-proxy.service'; describe(ContentElementService.name, () => { let module: TestingModule; @@ -34,7 +33,6 @@ describe(ContentElementService.name, () => { let boardDoRepo: DeepMocked; let boardDoService: DeepMocked; let contentElementFactory: DeepMocked; - let openGraphProxyService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -52,10 +50,6 @@ describe(ContentElementService.name, () => { provide: ContentElementFactory, useValue: createMock(), }, - { - provide: OpenGraphProxyService, - useValue: createMock(), - }, ], }).compile(); @@ -63,7 +57,6 @@ describe(ContentElementService.name, () => { boardDoRepo = module.get(BoardDoRepo); boardDoService = module.get(BoardDoService); contentElementFactory = module.get(ContentElementFactory); - openGraphProxyService = module.get(OpenGraphProxyService); await setupEntities(); }); @@ -302,8 +295,6 @@ describe(ContentElementService.name, () => { image: { url: 'https://my-open-graph-proxy.scvs.de/image/adefcb12ed3a' }, }; - openGraphProxyService.fetchOpenGraphData.mockResolvedValueOnce(imageResponse); - return { linkElement, content, card, imageResponse }; }; 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 4404f51fc3e..20f225e665b 100644 --- a/apps/server/src/modules/board/service/content-element.service.ts +++ b/apps/server/src/modules/board/service/content-element.service.ts @@ -13,15 +13,13 @@ import { AnyElementContentBody } from '../controller/dto'; import { BoardDoRepo } from '../repo'; import { BoardDoService } from './board-do.service'; import { ContentElementUpdateVisitor } from './content-element-update.visitor'; -import { OpenGraphProxyService } from './open-graph-proxy.service'; @Injectable() export class ContentElementService { constructor( private readonly boardDoRepo: BoardDoRepo, private readonly boardDoService: BoardDoService, - private readonly contentElementFactory: ContentElementFactory, - private readonly openGraphProxyService: OpenGraphProxyService + private readonly contentElementFactory: ContentElementFactory ) {} async findById(elementId: EntityId): Promise { @@ -58,7 +56,7 @@ export class ContentElementService { } async update(element: AnyContentElementDo, content: AnyElementContentBody): Promise { - const updater = new ContentElementUpdateVisitor(content, this.openGraphProxyService); + const updater = new ContentElementUpdateVisitor(content); await element.acceptAsync(updater); const parent = await this.boardDoRepo.findParentOfId(element.id); diff --git a/apps/server/src/modules/board/service/index.ts b/apps/server/src/modules/board/service/index.ts index ac9c686d4b4..8ff2787f35d 100644 --- a/apps/server/src/modules/board/service/index.ts +++ b/apps/server/src/modules/board/service/index.ts @@ -4,5 +4,4 @@ export * from './card.service'; export * from './column-board.service'; export * from './column.service'; export * from './content-element.service'; -export * from './open-graph-proxy.service'; export * from './submission-item.service'; diff --git a/apps/server/src/modules/board/service/open-graph-proxy.service.spec.ts b/apps/server/src/modules/board/service/open-graph-proxy.service.spec.ts deleted file mode 100644 index debe76cdeba..00000000000 --- a/apps/server/src/modules/board/service/open-graph-proxy.service.spec.ts +++ /dev/null @@ -1,91 +0,0 @@ -import { Test, TestingModule } from '@nestjs/testing'; -import { setupEntities } from '@shared/testing'; -import { ImageObject } from 'open-graph-scraper/dist/lib/types'; -import { OpenGraphProxyService } from './open-graph-proxy.service'; - -let ogsResponseMock = {}; -jest.mock( - 'open-graph-scraper', - () => () => - Promise.resolve({ - error: false, - html: '', - response: {}, - result: ogsResponseMock, - }) -); - -describe(OpenGraphProxyService.name, () => { - let module: TestingModule; - let service: OpenGraphProxyService; - - beforeAll(async () => { - module = await Test.createTestingModule({ - providers: [OpenGraphProxyService], - }).compile(); - - service = module.get(OpenGraphProxyService); - - await setupEntities(); - }); - - afterAll(async () => { - await module.close(); - }); - - afterEach(() => { - jest.resetAllMocks(); - }); - - describe('create', () => { - it('should return also the original url', async () => { - const url = 'https://de.wikipedia.org'; - - const result = await service.fetchOpenGraphData(url); - - expect(result).toEqual(expect.objectContaining({ url })); - }); - - it('should thrown an error if url is an empty string', async () => { - const url = ''; - - await expect(service.fetchOpenGraphData(url)).rejects.toThrow(); - }); - - it('should return ogTitle as title', async () => { - const ogTitle = 'My Title'; - const url = 'https://de.wikipedia.org'; - ogsResponseMock = { ogTitle }; - - const result = await service.fetchOpenGraphData(url); - - expect(result).toEqual(expect.objectContaining({ title: ogTitle })); - }); - - it('should return ogImage as title', async () => { - const ogImage: ImageObject[] = [ - { - width: 800, - type: 'jpeg', - url: 'big-image.jpg', - }, - { - width: 500, - type: 'jpeg', - url: 'medium-image.jpg', - }, - { - width: 300, - type: 'jpeg', - url: 'small-image.jpg', - }, - ]; - const url = 'https://de.wikipedia.org'; - ogsResponseMock = { ogImage }; - - const result = await service.fetchOpenGraphData(url); - - expect(result).toEqual(expect.objectContaining({ image: ogImage[1] })); - }); - }); -}); diff --git a/apps/server/src/modules/board/service/open-graph-proxy.service.ts b/apps/server/src/modules/board/service/open-graph-proxy.service.ts deleted file mode 100644 index 2b54d75ee82..00000000000 --- a/apps/server/src/modules/board/service/open-graph-proxy.service.ts +++ /dev/null @@ -1,41 +0,0 @@ -import { Injectable } from '@nestjs/common'; -import ogs from 'open-graph-scraper'; -import { ImageObject } from 'open-graph-scraper/dist/lib/types'; - -type OpenGraphData = { - title: string; - description: string; - url: string; - image?: ImageObject; -}; - -@Injectable() -export class OpenGraphProxyService { - async fetchOpenGraphData(url: string): Promise { - if (url.length === 0) { - throw new Error(`OpenGraphProxyService requires a valid URL. Given URL: ${url}`); - } - - const data = await ogs({ url }); - // WIP: add nice debug logging for available openGraphData?!? - - const title = data.result.ogTitle ?? ''; - const description = data.result.ogDescription ?? ''; - const image = data.result.ogImage ? this.pickImage(data.result.ogImage) : undefined; - - return { - title, - description, - image, - url, - }; - } - - private pickImage(images: ImageObject[], minWidth = 400): ImageObject | undefined { - const sortedImages = [...images]; - sortedImages.sort((a, b) => (a.width && b.width ? Number(a.width) - Number(b.width) : 0)); - const smallestBigEnoughImage = sortedImages.find((i) => i.width && i.width >= minWidth); - const fallbackImage = images[0] && images[0].width === undefined ? images[0] : undefined; - return smallestBigEnoughImage ?? fallbackImage; - } -} diff --git a/apps/server/src/modules/meta-tag-extractor/controller/api-test/meta-tag-extractor-get-data.api.spec.ts b/apps/server/src/modules/meta-tag-extractor/controller/api-test/meta-tag-extractor-get-data.api.spec.ts new file mode 100644 index 00000000000..c80d47df66d --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/controller/api-test/meta-tag-extractor-get-data.api.spec.ts @@ -0,0 +1,77 @@ +import { EntityManager } from '@mikro-orm/mongodb'; +import { ServerTestModule } from '@modules/server/server.module'; +import { INestApplication } from '@nestjs/common'; +import { Test, TestingModule } from '@nestjs/testing'; +import { TestApiClient, UserAndAccountTestFactory } from '@shared/testing'; +import { MetaTagExtractorService } from '../../service'; + +const URL = 'https://test.de'; + +const mockedResponse = { + url: URL, + title: 'The greatest Test-Page', + description: 'with great description', +}; + +describe(`get data (api)`, () => { + let app: INestApplication; + let em: EntityManager; + let testApiClient: TestApiClient; + + beforeAll(async () => { + const module: TestingModule = await Test.createTestingModule({ + imports: [ServerTestModule], + }) + .overrideProvider(MetaTagExtractorService) + .useValue({ + fetchMetaData: () => mockedResponse, + }) + .compile(); + + app = module.createNestApplication(); + await app.init(); + em = module.get(EntityManager); + testApiClient = new TestApiClient(app, '/meta-tag-extractor'); + }); + + afterAll(async () => { + await app.close(); + }); + + describe('with valid user', () => { + const setup = async () => { + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); + + await em.persistAndFlush([teacherAccount, teacherUser]); + em.clear(); + + const loggedInClient = await testApiClient.login(teacherAccount); + + return { loggedInClient }; + }; + + it('should return status 201', async () => { + const { loggedInClient } = await setup(); + + const { status } = await loggedInClient.post(undefined, { url: URL }); + + expect(status).toEqual(201); + }); + + it('should return the meta tags of the external page', async () => { + const { loggedInClient } = await setup(); + + const response = await loggedInClient.post(undefined, { url: URL }); + + expect(response?.body).toEqual(mockedResponse); + }); + }); + + describe('with invalid user', () => { + it('should return status 401', async () => { + const { status } = await testApiClient.post(undefined, { url: URL }); + + expect(status).toEqual(401); + }); + }); +}); diff --git a/apps/server/src/modules/meta-tag-extractor/controller/dto/index.ts b/apps/server/src/modules/meta-tag-extractor/controller/dto/index.ts new file mode 100644 index 00000000000..f4f64d6113d --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/controller/dto/index.ts @@ -0,0 +1 @@ +export * from './meta-tag-extractor.response'; diff --git a/apps/server/src/modules/meta-tag-extractor/controller/dto/meta-tag-extractor.response.spec.ts b/apps/server/src/modules/meta-tag-extractor/controller/dto/meta-tag-extractor.response.spec.ts new file mode 100644 index 00000000000..29dfbd94c72 --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/controller/dto/meta-tag-extractor.response.spec.ts @@ -0,0 +1,20 @@ +import { MetaTagExtractorResponse } from './meta-tag-extractor.response'; + +describe(MetaTagExtractorResponse.name, () => { + describe('when creating a error response', () => { + it('should have basic properties defined', () => { + const properties: MetaTagExtractorResponse = { + url: 'https://www.abc.de/my-article', + title: 'Testbild', + description: 'Here we describe what this page is about.', + imageUrl: 'https://www.abc.de/test.png', + }; + + const errorResponse = new MetaTagExtractorResponse(properties); + expect(errorResponse.url).toEqual(properties.url); + expect(errorResponse.title).toEqual(properties.title); + expect(errorResponse.description).toEqual(properties.description); + expect(errorResponse.imageUrl).toEqual(properties.imageUrl); + }); + }); +}); diff --git a/apps/server/src/modules/meta-tag-extractor/controller/dto/meta-tag-extractor.response.ts b/apps/server/src/modules/meta-tag-extractor/controller/dto/meta-tag-extractor.response.ts new file mode 100644 index 00000000000..a2f5acd8465 --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/controller/dto/meta-tag-extractor.response.ts @@ -0,0 +1,28 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { DecodeHtmlEntities } from '@shared/controller'; +import { IsString, IsUrl } from 'class-validator'; + +export class MetaTagExtractorResponse { + constructor({ url, title, description, imageUrl }: MetaTagExtractorResponse) { + this.url = url; + this.title = title; + this.description = description; + this.imageUrl = imageUrl; + } + + @ApiProperty() + @IsUrl() + url!: string; + + @ApiProperty() + @DecodeHtmlEntities() + title?: string; + + @ApiProperty() + @DecodeHtmlEntities() + description?: string; + + @ApiProperty() + @IsString() + imageUrl?: string; +} diff --git a/apps/server/src/modules/meta-tag-extractor/controller/index.ts b/apps/server/src/modules/meta-tag-extractor/controller/index.ts new file mode 100644 index 00000000000..296b343c591 --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/controller/index.ts @@ -0,0 +1,2 @@ +export * from './dto'; +export * from './meta-tag-extractor.controller'; diff --git a/apps/server/src/modules/meta-tag-extractor/controller/meta-tag-extractor.controller.ts b/apps/server/src/modules/meta-tag-extractor/controller/meta-tag-extractor.controller.ts new file mode 100644 index 00000000000..8133c4c0b83 --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/controller/meta-tag-extractor.controller.ts @@ -0,0 +1,29 @@ +import { Body, Controller, InternalServerErrorException, Post, UnauthorizedException } from '@nestjs/common'; +import { ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger'; +import { ICurrentUser } from '@src/modules/authentication'; +import { Authenticate, CurrentUser } from '@src/modules/authentication/decorator/auth.decorator'; +import { MetaTagExtractorUc } from '../uc'; +import { MetaTagExtractorResponse } from './dto'; +import { GetMetaTagDataBody } from './post-link-url.body.params'; + +@ApiTags('Meta Tag Extractor') +@Authenticate('jwt') +@Controller('meta-tag-extractor') +export class MetaTagExtractorController { + constructor(private readonly metaTagExtractorUc: MetaTagExtractorUc) {} + + @ApiOperation({ summary: 'return extract meta tags' }) + @ApiResponse({ status: 201, type: MetaTagExtractorResponse }) + @ApiResponse({ status: 401, type: UnauthorizedException }) + @ApiResponse({ status: 500, type: InternalServerErrorException }) + @Post('') + async getData( + @CurrentUser() currentUser: ICurrentUser, + @Body() bodyParams: GetMetaTagDataBody + ): Promise { + const result = await this.metaTagExtractorUc.fetchMetaData(currentUser.userId, bodyParams.url); + const imageUrl = result.image?.url; + const response = new MetaTagExtractorResponse({ ...result, imageUrl }); + return response; + } +} diff --git a/apps/server/src/modules/meta-tag-extractor/controller/post-link-url.body.params.ts b/apps/server/src/modules/meta-tag-extractor/controller/post-link-url.body.params.ts new file mode 100644 index 00000000000..1e9cd1f7f34 --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/controller/post-link-url.body.params.ts @@ -0,0 +1,11 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { IsUrl } from 'class-validator'; + +export class GetMetaTagDataBody { + @IsUrl() + @ApiProperty({ + required: true, + nullable: false, + }) + url!: string; +} diff --git a/apps/server/src/modules/meta-tag-extractor/index.ts b/apps/server/src/modules/meta-tag-extractor/index.ts new file mode 100644 index 00000000000..fb549460609 --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/index.ts @@ -0,0 +1,4 @@ +export * from './controller'; +export * from './meta-tag-extractor-api.module'; +export * from './meta-tag-extractor.module'; +export * from './uc'; diff --git a/apps/server/src/modules/meta-tag-extractor/meta-tag-extractor-api.module.ts b/apps/server/src/modules/meta-tag-extractor/meta-tag-extractor-api.module.ts new file mode 100644 index 00000000000..d9095315e87 --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/meta-tag-extractor-api.module.ts @@ -0,0 +1,13 @@ +import { forwardRef, Module } from '@nestjs/common'; +import { LoggerModule } from '@src/core/logger'; +import { AuthorizationModule } from '@src/modules/authorization'; +import { MetaTagExtractorController } from './controller'; +import { MetaTagExtractorModule } from './meta-tag-extractor.module'; +import { MetaTagExtractorUc } from './uc'; + +@Module({ + imports: [MetaTagExtractorModule, LoggerModule, forwardRef(() => AuthorizationModule)], + controllers: [MetaTagExtractorController], + providers: [MetaTagExtractorUc], +}) +export class MetaTagExtractorApiModule {} diff --git a/apps/server/src/modules/meta-tag-extractor/meta-tag-extractor.config.ts b/apps/server/src/modules/meta-tag-extractor/meta-tag-extractor.config.ts new file mode 100644 index 00000000000..d82e6811009 --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/meta-tag-extractor.config.ts @@ -0,0 +1,8 @@ +import { Configuration } from '@hpi-schul-cloud/commons'; + +const metaTagExtractorConfig = { + NEST_LOG_LEVEL: Configuration.get('NEST_LOG_LEVEL') as string, + INCOMING_REQUEST_TIMEOUT: Configuration.get('INCOMING_REQUEST_TIMEOUT_API') as number, +}; + +export default () => metaTagExtractorConfig; diff --git a/apps/server/src/modules/meta-tag-extractor/meta-tag-extractor.module.ts b/apps/server/src/modules/meta-tag-extractor/meta-tag-extractor.module.ts new file mode 100644 index 00000000000..817d7257330 --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/meta-tag-extractor.module.ts @@ -0,0 +1,24 @@ +import { HttpModule } from '@nestjs/axios'; +import { Module } from '@nestjs/common'; +import { ConfigModule } from '@nestjs/config'; +import { ConsoleWriterModule } from '@shared/infra/console'; +import { createConfigModuleOptions } from '@src/config'; +import { LoggerModule } from '@src/core/logger'; +import { AuthenticationModule } from '../authentication/authentication.module'; +import { UserModule } from '../user'; +import metaTagExtractorConfig from './meta-tag-extractor.config'; +import { MetaTagExtractorService } from './service'; + +@Module({ + imports: [ + AuthenticationModule, + ConsoleWriterModule, + HttpModule, + LoggerModule, + UserModule, + ConfigModule.forRoot(createConfigModuleOptions(metaTagExtractorConfig)), + ], + providers: [MetaTagExtractorService], + exports: [MetaTagExtractorService], +}) +export class MetaTagExtractorModule {} diff --git a/apps/server/src/modules/meta-tag-extractor/service/index.ts b/apps/server/src/modules/meta-tag-extractor/service/index.ts new file mode 100644 index 00000000000..238c426e837 --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/service/index.ts @@ -0,0 +1 @@ +export * from './meta-tag-extractor.service'; diff --git a/apps/server/src/modules/meta-tag-extractor/service/meta-tag-extractor.service.spec.ts b/apps/server/src/modules/meta-tag-extractor/service/meta-tag-extractor.service.spec.ts new file mode 100644 index 00000000000..af1a256d121 --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/service/meta-tag-extractor.service.spec.ts @@ -0,0 +1,122 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { setupEntities } from '@shared/testing'; +import { ImageObject } from 'open-graph-scraper/dist/lib/types'; +import { MetaTagExtractorService } from './meta-tag-extractor.service'; + +let ogsResponseMock = {}; +let ogsRejectMock: Error | undefined; + +jest.mock('open-graph-scraper', () => () => { + if (ogsRejectMock) { + return Promise.reject(ogsRejectMock); + } + + return Promise.resolve({ + error: false, + html: '', + response: {}, + result: ogsResponseMock, + }); +}); + +describe(MetaTagExtractorService.name, () => { + let module: TestingModule; + let service: MetaTagExtractorService; + + beforeAll(async () => { + module = await Test.createTestingModule({ + providers: [MetaTagExtractorService], + }).compile(); + + service = module.get(MetaTagExtractorService); + + await setupEntities(); + }); + + afterAll(async () => { + await module.close(); + }); + + beforeEach(() => { + ogsResponseMock = {}; + ogsRejectMock = undefined; + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('create', () => { + describe('when url points to webpage', () => { + it('should return also the original url', async () => { + const url = 'https://de.wikipedia.org'; + + const result = await service.fetchMetaData(url); + + expect(result).toEqual(expect.objectContaining({ url })); + }); + + it('should thrown an error if url is an empty string', async () => { + const url = ''; + + await expect(service.fetchMetaData(url)).rejects.toThrow(); + }); + + it('should return ogTitle as title', async () => { + const ogTitle = 'My Title'; + const url = 'https://de.wikipedia.org'; + ogsResponseMock = { ogTitle }; + + const result = await service.fetchMetaData(url); + + expect(result).toEqual(expect.objectContaining({ title: ogTitle })); + }); + + it('should return ogImage as image', async () => { + const ogImage: ImageObject[] = [ + { + width: 800, + type: 'jpeg', + url: 'big-image.jpg', + }, + { + width: 500, + type: 'jpeg', + url: 'medium-image.jpg', + }, + { + width: 300, + type: 'jpeg', + url: 'small-image.jpg', + }, + ]; + const url = 'https://de.wikipedia.org'; + ogsResponseMock = { ogImage }; + + const result = await service.fetchMetaData(url); + + expect(result).toEqual(expect.objectContaining({ image: ogImage[1] })); + }); + }); + + describe('when url points to a file', () => { + it('should return filename as title', async () => { + const url = 'https://de.wikipedia.org/abc.jpg'; + ogsRejectMock = new Error('no open graph data included... probably not a webpage'); + + const result = await service.fetchMetaData(url); + expect(result).toEqual(expect.objectContaining({ title: 'abc.jpg' })); + }); + }); + + describe('when url is invalid', () => { + it('should return url as it is', async () => { + const url = 'not-a-real-domain'; + ogsRejectMock = new Error('no open graph data included... probably not a webpage'); + + const result = await service.fetchMetaData(url); + expect(result).toEqual(expect.objectContaining({ url, title: '', description: '' })); + }); + }); + }); +}); diff --git a/apps/server/src/modules/meta-tag-extractor/service/meta-tag-extractor.service.ts b/apps/server/src/modules/meta-tag-extractor/service/meta-tag-extractor.service.ts new file mode 100644 index 00000000000..46c30c17702 --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/service/meta-tag-extractor.service.ts @@ -0,0 +1,65 @@ +import { Injectable } from '@nestjs/common'; +import ogs from 'open-graph-scraper'; +import { ImageObject } from 'open-graph-scraper/dist/lib/types'; +import { basename } from 'path'; + +export type MetaData = { + title: string; + description: string; + url: string; + image?: ImageObject; +}; + +@Injectable() +export class MetaTagExtractorService { + async fetchMetaData(url: string): Promise { + if (url.length === 0) { + throw new Error(`MetaTagExtractorService requires a valid URL. Given URL: ${url}`); + } + + const metaData = (await this.tryExtractMetaTags(url)) ?? this.tryFilenameAsFallback(url); + + return metaData ?? { url, title: '', description: '' }; + } + + private async tryExtractMetaTags(url: string): Promise { + try { + const data = await ogs({ url, fetchOptions: { headers: { 'User-Agent': 'Open Graph Scraper' } } }); + + const title = data.result?.ogTitle ?? ''; + const description = data.result?.ogDescription ?? ''; + const image = data.result?.ogImage ? this.pickImage(data?.result?.ogImage) : undefined; + + return { + title, + description, + image, + url, + }; + } catch (error) { + return undefined; + } + } + + private tryFilenameAsFallback(url: string): MetaData | undefined { + try { + const urlObject = new URL(url); + const title = basename(urlObject.pathname); + return { + title, + description: '', + url, + }; + } catch (error) { + return undefined; + } + } + + private pickImage(images: ImageObject[], minWidth = 400): ImageObject | undefined { + const sortedImages = [...images]; + sortedImages.sort((a, b) => (a.width && b.width ? Number(a.width) - Number(b.width) : 0)); + const smallestBigEnoughImage = sortedImages.find((i) => i.width && i.width >= minWidth); + const fallbackImage = images[0] && images[0].width === undefined ? images[0] : undefined; + return smallestBigEnoughImage ?? fallbackImage; + } +} diff --git a/apps/server/src/modules/meta-tag-extractor/uc/index.ts b/apps/server/src/modules/meta-tag-extractor/uc/index.ts new file mode 100644 index 00000000000..6621180a92b --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/uc/index.ts @@ -0,0 +1 @@ +export * from './meta-tag-extractor.uc'; diff --git a/apps/server/src/modules/meta-tag-extractor/uc/meta-tag-extractor.uc.spec.ts b/apps/server/src/modules/meta-tag-extractor/uc/meta-tag-extractor.uc.spec.ts new file mode 100644 index 00000000000..118b7d82633 --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/uc/meta-tag-extractor.uc.spec.ts @@ -0,0 +1,91 @@ +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { UnauthorizedException } from '@nestjs/common'; +import { Test, TestingModule } from '@nestjs/testing'; +import { setupEntities, userFactory } from '@shared/testing'; +import { AuthorizationService } from '@src/modules/authorization'; +import { MetaTagExtractorService } from '../service'; +import { MetaTagExtractorUc } from './meta-tag-extractor.uc'; + +describe(MetaTagExtractorUc.name, () => { + let module: TestingModule; + let uc: MetaTagExtractorUc; + let authorizationService: DeepMocked; + let metaTagExtractorService: DeepMocked; + + beforeAll(async () => { + module = await Test.createTestingModule({ + providers: [ + MetaTagExtractorUc, + { + provide: MetaTagExtractorService, + useValue: createMock(), + }, + { + provide: AuthorizationService, + useValue: createMock(), + }, + ], + }).compile(); + + uc = module.get(MetaTagExtractorUc); + authorizationService = module.get(AuthorizationService); + metaTagExtractorService = module.get(MetaTagExtractorService); + + await setupEntities(); + }); + + afterAll(async () => { + await module.close(); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('fetchMetaData', () => { + describe('when user exists', () => { + const setup = () => { + const user = userFactory.build(); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + + return { user }; + }; + + it('should check if the user is a valid user', async () => { + const { user } = setup(); + + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + + const url = 'https://www.example.com/great-example'; + await uc.fetchMetaData(user.id, url); + + expect(authorizationService.getUserWithPermissions).toHaveBeenCalledWith(user.id); + }); + + it('should call meta tag extractor service', async () => { + const { user } = setup(); + + const url = 'https://www.example.com/great-example'; + await uc.fetchMetaData(user.id, url); + + expect(metaTagExtractorService.fetchMetaData).toHaveBeenCalledWith(url); + }); + }); + + describe('when user does not exist', () => { + const setup = () => { + const user = userFactory.build(); + authorizationService.getUserWithPermissions.mockRejectedValue(false); + + return { user }; + }; + + it('should throw an UnauthorizedException', async () => { + const { user } = setup(); + + const url = 'https://www.example.com/great-example'; + await expect(uc.fetchMetaData(user.id, url)).rejects.toThrow(UnauthorizedException); + }); + }); + }); +}); diff --git a/apps/server/src/modules/meta-tag-extractor/uc/meta-tag-extractor.uc.ts b/apps/server/src/modules/meta-tag-extractor/uc/meta-tag-extractor.uc.ts new file mode 100644 index 00000000000..5daca6c962d --- /dev/null +++ b/apps/server/src/modules/meta-tag-extractor/uc/meta-tag-extractor.uc.ts @@ -0,0 +1,23 @@ +import { Injectable, UnauthorizedException } from '@nestjs/common'; +import { EntityId } from '@shared/domain'; +import { AuthorizationService } from '@src/modules/authorization'; +import { MetaData, MetaTagExtractorService } from '../service'; + +@Injectable() +export class MetaTagExtractorUc { + constructor( + private readonly authorizationService: AuthorizationService, + private readonly metaTagExtractorService: MetaTagExtractorService + ) {} + + async fetchMetaData(userId: EntityId, url: string): Promise { + try { + await this.authorizationService.getUserWithPermissions(userId); + } catch (error) { + throw new UnauthorizedException(); + } + + const result = await this.metaTagExtractorService.fetchMetaData(url); + return result; + } +} diff --git a/apps/server/src/modules/server/server.module.ts b/apps/server/src/modules/server/server.module.ts index 084ca72fca3..9454fa06154 100644 --- a/apps/server/src/modules/server/server.module.ts +++ b/apps/server/src/modules/server/server.module.ts @@ -1,16 +1,6 @@ import { Configuration } from '@hpi-schul-cloud/commons'; import { Dictionary, IPrimaryKey } from '@mikro-orm/core'; import { MikroOrmModule, MikroOrmModuleSyncOptions } from '@mikro-orm/nestjs'; -import { DynamicModule, Inject, MiddlewareConsumer, Module, NestModule, NotFoundException } from '@nestjs/common'; -import { ConfigModule } from '@nestjs/config'; -import { ALL_ENTITIES } from '@shared/domain'; -import { MongoDatabaseModuleOptions, MongoMemoryDatabaseModule } from '@shared/infra/database'; -import { MailModule } from '@shared/infra/mail'; -import { RabbitMQWrapperModule, RabbitMQWrapperTestModule } from '@shared/infra/rabbitmq'; -import { REDIS_CLIENT, RedisModule } from '@shared/infra/redis'; -import { createConfigModuleOptions, DB_PASSWORD, DB_URL, DB_USERNAME } from '@src/config'; -import { CoreModule } from '@src/core'; -import { LegacyLogger, LoggerModule } from '@src/core/logger'; import { AccountApiModule } from '@modules/account/account-api.module'; import { AuthenticationApiModule } from '@modules/authentication/authentication-api.module'; import { BoardApiModule } from '@modules/board/board-api.module'; @@ -18,25 +8,36 @@ import { CollaborativeStorageModule } from '@modules/collaborative-storage'; import { FilesStorageClientModule } from '@modules/files-storage-client'; import { GroupApiModule } from '@modules/group/group-api.module'; import { LearnroomApiModule } from '@modules/learnroom/learnroom-api.module'; +import { LegacySchoolApiModule } from '@modules/legacy-school/legacy-school-api.module'; import { LessonApiModule } from '@modules/lesson/lesson-api.module'; +import { MetaTagExtractorApiModule, MetaTagExtractorModule } from '@modules/meta-tag-extractor'; import { NewsModule } from '@modules/news'; import { OauthProviderApiModule } from '@modules/oauth-provider'; import { OauthApiModule } from '@modules/oauth/oauth-api.module'; +import { PseudonymApiModule } from '@modules/pseudonym/pseudonym-api.module'; import { RocketChatModule } from '@modules/rocketchat'; -import { LegacySchoolApiModule } from '@modules/legacy-school/legacy-school-api.module'; import { SharingApiModule } from '@modules/sharing/sharing.module'; import { SystemApiModule } from '@modules/system/system-api.module'; import { TaskApiModule } from '@modules/task/task-api.module'; +import { TeamsApiModule } from '@modules/teams/teams-api.module'; import { ToolApiModule } from '@modules/tool/tool-api.module'; import { ImportUserModule } from '@modules/user-import'; import { UserLoginMigrationApiModule } from '@modules/user-login-migration/user-login-migration-api.module'; import { UserApiModule } from '@modules/user/user-api.module'; import { VideoConferenceApiModule } from '@modules/video-conference/video-conference-api.module'; +import { DynamicModule, Inject, MiddlewareConsumer, Module, NestModule, NotFoundException } from '@nestjs/common'; +import { ConfigModule } from '@nestjs/config'; +import { ALL_ENTITIES } from '@shared/domain'; +import { MongoDatabaseModuleOptions, MongoMemoryDatabaseModule } from '@shared/infra/database'; +import { MailModule } from '@shared/infra/mail'; +import { RabbitMQWrapperModule, RabbitMQWrapperTestModule } from '@shared/infra/rabbitmq'; +import { RedisModule, REDIS_CLIENT } from '@shared/infra/redis'; +import { createConfigModuleOptions, DB_PASSWORD, DB_URL, DB_USERNAME } from '@src/config'; +import { CoreModule } from '@src/core'; +import { LegacyLogger, LoggerModule } from '@src/core/logger'; import connectRedis from 'connect-redis'; import session from 'express-session'; import { RedisClient } from 'redis'; -import { TeamsApiModule } from '@modules/teams/teams-api.module'; -import { PseudonymApiModule } from '@modules/pseudonym/pseudonym-api.module'; import { ServerController } from './controller/server.controller'; import { serverConfig } from './server.config'; @@ -47,6 +48,7 @@ const serverModules = [ AccountApiModule, CollaborativeStorageModule, OauthApiModule, + MetaTagExtractorModule, TaskApiModule, LessonApiModule, NewsModule, @@ -75,6 +77,7 @@ const serverModules = [ BoardApiModule, GroupApiModule, TeamsApiModule, + MetaTagExtractorApiModule, PseudonymApiModule, ]; diff --git a/apps/server/src/shared/testing/factory/domainobject/board/link-element.do.factory.ts b/apps/server/src/shared/testing/factory/domainobject/board/link-element.do.factory.ts index af0e55a1912..415cfeae1dc 100644 --- a/apps/server/src/shared/testing/factory/domainobject/board/link-element.do.factory.ts +++ b/apps/server/src/shared/testing/factory/domainobject/board/link-element.do.factory.ts @@ -7,7 +7,7 @@ export const linkElementFactory = BaseFactory.define Date: Sun, 5 Nov 2023 01:58:54 +0100 Subject: [PATCH 7/7] N21-1318 adjust classInfoResponse for new class page (#4496) * add studentCount * add classFilter --- .../controller/api-test/group.api.spec.ts | 5 +- .../dto/interface/class-sort-by.enum.ts | 4 + .../group/controller/dto/interface/index.ts | 2 + .../interface/school-year-query-type.enum.ts | 5 + .../dto/request/class-filter-params.ts | 10 ++ .../dto/request/class-sort-params.ts | 6 +- .../group/controller/dto/request/index.ts | 1 + .../dto/response/class-info.response.ts | 4 + .../group/controller/group.controller.ts | 4 +- .../mapper/group-response.mapper.ts | 1 + .../src/modules/group/loggable/index.ts | 1 + ...nown-query-type-loggable-exception.spec.ts | 31 ++++ .../unknown-query-type-loggable-exception.ts | 19 ++ .../modules/group/uc/dto/class-info.dto.ts | 3 + .../src/modules/group/uc/group.uc.spec.ts | 165 ++++++++++++++++-- apps/server/src/modules/group/uc/group.uc.ts | 138 ++++++++++++--- .../group/uc/mapper/group-uc.mapper.ts | 3 + 17 files changed, 357 insertions(+), 45 deletions(-) create mode 100644 apps/server/src/modules/group/controller/dto/interface/class-sort-by.enum.ts create mode 100644 apps/server/src/modules/group/controller/dto/interface/index.ts create mode 100644 apps/server/src/modules/group/controller/dto/interface/school-year-query-type.enum.ts create mode 100644 apps/server/src/modules/group/controller/dto/request/class-filter-params.ts create mode 100644 apps/server/src/modules/group/loggable/index.ts create mode 100644 apps/server/src/modules/group/loggable/unknown-query-type-loggable-exception.spec.ts create mode 100644 apps/server/src/modules/group/loggable/unknown-query-type-loggable-exception.ts diff --git a/apps/server/src/modules/group/controller/api-test/group.api.spec.ts b/apps/server/src/modules/group/controller/api-test/group.api.spec.ts index 2d9f4105f80..dc1753a6c7f 100644 --- a/apps/server/src/modules/group/controller/api-test/group.api.spec.ts +++ b/apps/server/src/modules/group/controller/api-test/group.api.spec.ts @@ -18,7 +18,8 @@ import { import { ObjectId } from 'bson'; import { GroupEntity, GroupEntityTypes } from '../../entity'; import { ClassRootType } from '../../uc/dto/class-root-type'; -import { ClassInfoSearchListResponse, ClassSortBy } from '../dto'; +import { ClassInfoSearchListResponse } from '../dto'; +import { ClassSortBy } from '../dto/interface'; const baseRouteName = '/groups'; @@ -120,6 +121,7 @@ describe('Group (API)', () => { name: group.name, externalSourceName: system.displayName, teachers: [adminUser.lastName], + studentCount: 0, }, { id: clazz.id, @@ -128,6 +130,7 @@ describe('Group (API)', () => { teachers: [teacherUser.lastName], schoolYear: schoolYear.name, isUpgradable: false, + studentCount: 0, }, ], skip: 0, diff --git a/apps/server/src/modules/group/controller/dto/interface/class-sort-by.enum.ts b/apps/server/src/modules/group/controller/dto/interface/class-sort-by.enum.ts new file mode 100644 index 00000000000..24dda3c9382 --- /dev/null +++ b/apps/server/src/modules/group/controller/dto/interface/class-sort-by.enum.ts @@ -0,0 +1,4 @@ +export enum ClassSortBy { + NAME = 'name', + EXTERNAL_SOURCE_NAME = 'externalSourceName', +} diff --git a/apps/server/src/modules/group/controller/dto/interface/index.ts b/apps/server/src/modules/group/controller/dto/interface/index.ts new file mode 100644 index 00000000000..fa69bb70b30 --- /dev/null +++ b/apps/server/src/modules/group/controller/dto/interface/index.ts @@ -0,0 +1,2 @@ +export * from './class-sort-by.enum'; +export * from './school-year-query-type.enum'; diff --git a/apps/server/src/modules/group/controller/dto/interface/school-year-query-type.enum.ts b/apps/server/src/modules/group/controller/dto/interface/school-year-query-type.enum.ts new file mode 100644 index 00000000000..ebec4637c46 --- /dev/null +++ b/apps/server/src/modules/group/controller/dto/interface/school-year-query-type.enum.ts @@ -0,0 +1,5 @@ +export enum SchoolYearQueryType { + NEXT_YEAR = 'nextYear', + CURRENT_YEAR = 'currentYear', + PREVIOUS_YEARS = 'previousYears', +} diff --git a/apps/server/src/modules/group/controller/dto/request/class-filter-params.ts b/apps/server/src/modules/group/controller/dto/request/class-filter-params.ts new file mode 100644 index 00000000000..d6a7e3ba62f --- /dev/null +++ b/apps/server/src/modules/group/controller/dto/request/class-filter-params.ts @@ -0,0 +1,10 @@ +import { ApiPropertyOptional } from '@nestjs/swagger'; +import { IsEnum, IsOptional } from 'class-validator'; +import { SchoolYearQueryType } from '../interface'; + +export class ClassFilterParams { + @IsOptional() + @IsEnum(SchoolYearQueryType) + @ApiPropertyOptional({ enum: SchoolYearQueryType, enumName: 'SchoolYearQueryType' }) + type?: SchoolYearQueryType; +} diff --git a/apps/server/src/modules/group/controller/dto/request/class-sort-params.ts b/apps/server/src/modules/group/controller/dto/request/class-sort-params.ts index 094f7efece4..980146c92d4 100644 --- a/apps/server/src/modules/group/controller/dto/request/class-sort-params.ts +++ b/apps/server/src/modules/group/controller/dto/request/class-sort-params.ts @@ -1,11 +1,7 @@ import { ApiPropertyOptional } from '@nestjs/swagger'; import { SortingParams } from '@shared/controller'; import { IsEnum, IsOptional } from 'class-validator'; - -export enum ClassSortBy { - NAME = 'name', - EXTERNAL_SOURCE_NAME = 'externalSourceName', -} +import { ClassSortBy } from '../interface'; export class ClassSortParams extends SortingParams { @IsOptional() diff --git a/apps/server/src/modules/group/controller/dto/request/index.ts b/apps/server/src/modules/group/controller/dto/request/index.ts index 17ecd658b7d..ceef988aa92 100644 --- a/apps/server/src/modules/group/controller/dto/request/index.ts +++ b/apps/server/src/modules/group/controller/dto/request/index.ts @@ -1,2 +1,3 @@ export * from './class-sort-params'; export * from './group-id-params'; +export * from './class-filter-params'; diff --git a/apps/server/src/modules/group/controller/dto/response/class-info.response.ts b/apps/server/src/modules/group/controller/dto/response/class-info.response.ts index a62b8134158..c1e394174a6 100644 --- a/apps/server/src/modules/group/controller/dto/response/class-info.response.ts +++ b/apps/server/src/modules/group/controller/dto/response/class-info.response.ts @@ -23,6 +23,9 @@ export class ClassInfoResponse { @ApiPropertyOptional() isUpgradable?: boolean; + @ApiProperty() + studentCount: number; + constructor(props: ClassInfoResponse) { this.id = props.id; this.type = props.type; @@ -31,5 +34,6 @@ export class ClassInfoResponse { this.teachers = props.teachers; this.schoolYear = props.schoolYear; this.isUpgradable = props.isUpgradable; + this.studentCount = props.studentCount; } } diff --git a/apps/server/src/modules/group/controller/group.controller.ts b/apps/server/src/modules/group/controller/group.controller.ts index a7dc0c77563..c92ee337050 100644 --- a/apps/server/src/modules/group/controller/group.controller.ts +++ b/apps/server/src/modules/group/controller/group.controller.ts @@ -6,7 +6,7 @@ import { Page } from '@shared/domain'; import { ErrorResponse } from '@src/core/error/dto'; import { GroupUc } from '../uc'; import { ClassInfoDto, ResolvedGroupDto } from '../uc/dto'; -import { ClassInfoSearchListResponse, ClassSortParams, GroupIdParams, GroupResponse } from './dto'; +import { ClassInfoSearchListResponse, ClassSortParams, GroupIdParams, GroupResponse, ClassFilterParams } from './dto'; import { GroupResponseMapper } from './mapper'; @ApiTags('Group') @@ -23,11 +23,13 @@ export class GroupController { public async findClasses( @Query() pagination: PaginationParams, @Query() sortingQuery: ClassSortParams, + @Query() filterParams: ClassFilterParams, @CurrentUser() currentUser: ICurrentUser ): Promise { const board: Page = await this.groupUc.findAllClasses( currentUser.userId, currentUser.schoolId, + filterParams.type, pagination.skip, pagination.limit, sortingQuery.sortBy, diff --git a/apps/server/src/modules/group/controller/mapper/group-response.mapper.ts b/apps/server/src/modules/group/controller/mapper/group-response.mapper.ts index 8c990cbd44a..668253de1ec 100644 --- a/apps/server/src/modules/group/controller/mapper/group-response.mapper.ts +++ b/apps/server/src/modules/group/controller/mapper/group-response.mapper.ts @@ -43,6 +43,7 @@ export class GroupResponseMapper { teachers: classInfo.teacherNames, schoolYear: classInfo.schoolYear, isUpgradable: classInfo.isUpgradable, + studentCount: classInfo.studentCount, }); return mapped; diff --git a/apps/server/src/modules/group/loggable/index.ts b/apps/server/src/modules/group/loggable/index.ts new file mode 100644 index 00000000000..0191fcaf981 --- /dev/null +++ b/apps/server/src/modules/group/loggable/index.ts @@ -0,0 +1 @@ +export * from './unknown-query-type-loggable-exception'; diff --git a/apps/server/src/modules/group/loggable/unknown-query-type-loggable-exception.spec.ts b/apps/server/src/modules/group/loggable/unknown-query-type-loggable-exception.spec.ts new file mode 100644 index 00000000000..c4f87e6a21a --- /dev/null +++ b/apps/server/src/modules/group/loggable/unknown-query-type-loggable-exception.spec.ts @@ -0,0 +1,31 @@ +import { UnknownQueryTypeLoggableException } from './unknown-query-type-loggable-exception'; + +describe('UnknownQueryTypeLoggableException', () => { + describe('getLogMessage', () => { + const setup = () => { + const unknownQueryType = 'unknwon'; + + const exception = new UnknownQueryTypeLoggableException(unknownQueryType); + + return { + exception, + unknownQueryType, + }; + }; + + it('should log the correct message', () => { + const { exception, unknownQueryType } = setup(); + + const result = exception.getLogMessage(); + + expect(result).toEqual({ + type: 'INTERNAL_SERVER_ERROR', + stack: expect.any(String), + message: 'Unable to process unknown query type for class years.', + data: { + unknownQueryType, + }, + }); + }); + }); +}); diff --git a/apps/server/src/modules/group/loggable/unknown-query-type-loggable-exception.ts b/apps/server/src/modules/group/loggable/unknown-query-type-loggable-exception.ts new file mode 100644 index 00000000000..758b4c1fb6b --- /dev/null +++ b/apps/server/src/modules/group/loggable/unknown-query-type-loggable-exception.ts @@ -0,0 +1,19 @@ +import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger'; +import { InternalServerErrorException } from '@nestjs/common'; + +export class UnknownQueryTypeLoggableException extends InternalServerErrorException implements Loggable { + constructor(private readonly unknownQueryType: string) { + super(); + } + + getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage { + return { + type: 'INTERNAL_SERVER_ERROR', + stack: this.stack, + message: 'Unable to process unknown query type for class years.', + data: { + unknownQueryType: this.unknownQueryType, + }, + }; + } +} diff --git a/apps/server/src/modules/group/uc/dto/class-info.dto.ts b/apps/server/src/modules/group/uc/dto/class-info.dto.ts index 611275e3bcd..c17689fe0fa 100644 --- a/apps/server/src/modules/group/uc/dto/class-info.dto.ts +++ b/apps/server/src/modules/group/uc/dto/class-info.dto.ts @@ -15,6 +15,8 @@ export class ClassInfoDto { isUpgradable?: boolean; + studentCount: number; + constructor(props: ClassInfoDto) { this.id = props.id; this.type = props.type; @@ -23,5 +25,6 @@ export class ClassInfoDto { this.teacherNames = props.teacherNames; this.schoolYear = props.schoolYear; this.isUpgradable = props.isUpgradable; + this.studentCount = props.studentCount; } } diff --git a/apps/server/src/modules/group/uc/group.uc.spec.ts b/apps/server/src/modules/group/uc/group.uc.spec.ts index d5236826def..51cf6151d45 100644 --- a/apps/server/src/modules/group/uc/group.uc.spec.ts +++ b/apps/server/src/modules/group/uc/group.uc.spec.ts @@ -28,6 +28,8 @@ import { GroupService } from '../service'; import { ClassInfoDto, ResolvedGroupDto } from './dto'; import { ClassRootType } from './dto/class-root-type'; import { GroupUc } from './group.uc'; +import { SchoolYearQueryType } from '../controller/dto/interface'; +import { UnknownQueryTypeLoggableException } from '../loggable'; describe('GroupUc', () => { let module: TestingModule; @@ -155,12 +157,26 @@ describe('GroupUc', () => { roles: [{ id: studentUser.roles[0].id, name: studentUser.roles[0].name }], }); const schoolYear: SchoolYearEntity = schoolYearFactory.buildWithId(); + const nextSchoolYear: SchoolYearEntity = schoolYearFactory.buildWithId({ + startDate: schoolYear.endDate, + }); const clazz: Class = classFactory.build({ name: 'A', teacherIds: [teacherUser.id], source: 'LDAP', year: schoolYear.id, }); + const successorClass: Class = classFactory.build({ + name: 'NEW', + teacherIds: [teacherUser.id], + year: nextSchoolYear.id, + }); + const classWithoutSchoolYear = classFactory.build({ + name: 'NoYear', + teacherIds: [teacherUser.id], + year: undefined, + }); + const system: SystemDto = new SystemDto({ id: new ObjectId().toHexString(), displayName: 'External System', @@ -183,8 +199,10 @@ describe('GroupUc', () => { schoolService.getSchoolById.mockResolvedValueOnce(school); authorizationService.getUserWithPermissions.mockResolvedValueOnce(teacherUser); authorizationService.hasAllPermissions.mockReturnValueOnce(false); - classService.findAllByUserId.mockResolvedValueOnce([clazz]); + classService.findAllByUserId.mockResolvedValueOnce([clazz, successorClass, classWithoutSchoolYear]); groupService.findByUser.mockResolvedValueOnce([group, groupWithSystem]); + classService.findClassesForSchool.mockResolvedValueOnce([clazz, successorClass, classWithoutSchoolYear]); + groupService.findClassesForSchool.mockResolvedValueOnce([group, groupWithSystem]); systemService.findById.mockResolvedValue(system); userService.findById.mockImplementation((userId: string): Promise => { if (userId === teacherUser.id) { @@ -208,23 +226,28 @@ describe('GroupUc', () => { throw new Error(); }); - schoolYearService.findById.mockResolvedValue(schoolYear); + schoolYearService.findById.mockResolvedValueOnce(schoolYear); + schoolYearService.findById.mockResolvedValueOnce(nextSchoolYear); + schoolYearService.getCurrentSchoolYear.mockResolvedValue(schoolYear); return { teacherUser, school, clazz, + successorClass, + classWithoutSchoolYear, group, groupWithSystem, system, schoolYear, + nextSchoolYear, }; }; it('should check the required permissions', async () => { const { teacherUser, school } = setup(); - await uc.findAllClasses(teacherUser.id, teacherUser.school.id); + await uc.findAllClasses(teacherUser.id, teacherUser.school.id, SchoolYearQueryType.CURRENT_YEAR); expect(authorizationService.checkPermission).toHaveBeenCalledWith<[User, LegacySchoolDo, AuthorizationContext]>( teacherUser, @@ -249,9 +272,19 @@ describe('GroupUc', () => { describe('when no pagination is given', () => { it('should return all classes sorted by name', async () => { - const { teacherUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); - - const result: Page = await uc.findAllClasses(teacherUser.id, teacherUser.school.id); + const { + teacherUser, + clazz, + successorClass, + classWithoutSchoolYear, + group, + groupWithSystem, + system, + schoolYear, + nextSchoolYear, + } = setup(); + + const result: Page = await uc.findAllClasses(teacherUser.id, teacherUser.school.id, undefined); expect(result).toEqual>({ data: [ @@ -263,12 +296,37 @@ describe('GroupUc', () => { teacherNames: [teacherUser.lastName], schoolYear: schoolYear.name, isUpgradable: false, + studentCount: 2, + }, + { + id: successorClass.id, + name: successorClass.gradeLevel + ? `${successorClass.gradeLevel}${successorClass.name}` + : successorClass.name, + type: ClassRootType.CLASS, + externalSourceName: successorClass.source, + teacherNames: [teacherUser.lastName], + schoolYear: nextSchoolYear.name, + isUpgradable: false, + studentCount: 2, + }, + { + id: classWithoutSchoolYear.id, + name: classWithoutSchoolYear.gradeLevel + ? `${classWithoutSchoolYear.gradeLevel}${classWithoutSchoolYear.name}` + : classWithoutSchoolYear.name, + type: ClassRootType.CLASS, + externalSourceName: classWithoutSchoolYear.source, + teacherNames: [teacherUser.lastName], + isUpgradable: false, + studentCount: 2, }, { id: group.id, name: group.name, type: ClassRootType.GROUP, teacherNames: [teacherUser.lastName], + studentCount: 0, }, { id: groupWithSystem.id, @@ -276,20 +334,22 @@ describe('GroupUc', () => { type: ClassRootType.GROUP, externalSourceName: system.displayName, teacherNames: [teacherUser.lastName], + studentCount: 1, }, ], - total: 3, + total: 5, }); }); }); describe('when sorting by external source name in descending order', () => { it('should return all classes sorted by external source name in descending order', async () => { - const { teacherUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); + const { teacherUser, clazz, classWithoutSchoolYear, group, groupWithSystem, system, schoolYear } = setup(); const result: Page = await uc.findAllClasses( teacherUser.id, teacherUser.school.id, + SchoolYearQueryType.CURRENT_YEAR, undefined, undefined, 'externalSourceName', @@ -298,6 +358,17 @@ describe('GroupUc', () => { expect(result).toEqual>({ data: [ + { + id: classWithoutSchoolYear.id, + name: classWithoutSchoolYear.gradeLevel + ? `${classWithoutSchoolYear.gradeLevel}${classWithoutSchoolYear.name}` + : classWithoutSchoolYear.name, + type: ClassRootType.CLASS, + externalSourceName: classWithoutSchoolYear.source, + teacherNames: [teacherUser.lastName], + isUpgradable: false, + studentCount: 2, + }, { id: clazz.id, name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, @@ -306,6 +377,7 @@ describe('GroupUc', () => { teacherNames: [teacherUser.lastName], schoolYear: schoolYear.name, isUpgradable: false, + studentCount: 2, }, { id: groupWithSystem.id, @@ -313,15 +385,17 @@ describe('GroupUc', () => { type: ClassRootType.GROUP, externalSourceName: system.displayName, teacherNames: [teacherUser.lastName], + studentCount: 1, }, { id: group.id, name: group.name, type: ClassRootType.GROUP, teacherNames: [teacherUser.lastName], + studentCount: 0, }, ], - total: 3, + total: 4, }); }); }); @@ -333,7 +407,8 @@ describe('GroupUc', () => { const result: Page = await uc.findAllClasses( teacherUser.id, teacherUser.school.id, - 1, + SchoolYearQueryType.CURRENT_YEAR, + 2, 1, 'name', SortOrder.asc @@ -346,12 +421,71 @@ describe('GroupUc', () => { name: group.name, type: ClassRootType.GROUP, teacherNames: [teacherUser.lastName], + studentCount: 0, }, ], - total: 3, + total: 4, }); }); }); + + describe('when querying for classes from next school year', () => { + it('should only return classes from next school year', async () => { + const { teacherUser, successorClass, nextSchoolYear } = setup(); + + const result: Page = await uc.findAllClasses( + teacherUser.id, + teacherUser.school.id, + SchoolYearQueryType.NEXT_YEAR + ); + + expect(result).toEqual>({ + data: [ + { + id: successorClass.id, + name: successorClass.gradeLevel + ? `${successorClass.gradeLevel}${successorClass.name}` + : successorClass.name, + externalSourceName: successorClass.source, + type: ClassRootType.CLASS, + teacherNames: [teacherUser.lastName], + schoolYear: nextSchoolYear.name, + isUpgradable: false, + studentCount: 2, + }, + ], + total: 1, + }); + }); + }); + + describe('when querying for archived classes', () => { + it('should only return classes from previous school years', async () => { + const { teacherUser } = setup(); + + const result: Page = await uc.findAllClasses( + teacherUser.id, + teacherUser.school.id, + SchoolYearQueryType.PREVIOUS_YEARS + ); + + expect(result).toEqual>({ + data: [], + total: 0, + }); + }); + }); + + describe('when querying for not existing type', () => { + it('should throw', async () => { + const { teacherUser } = setup(); + + const func = async () => + uc.findAllClasses(teacherUser.id, teacherUser.school.id, 'notAType' as SchoolYearQueryType); + + await expect(func).rejects.toThrow(UnknownQueryTypeLoggableException); + }); + }); }); describe('when accessing as a user with elevated permission', () => { @@ -498,12 +632,14 @@ describe('GroupUc', () => { teacherNames: [teacherUser.lastName], schoolYear: schoolYear.name, isUpgradable: false, + studentCount: 2, }, { id: group.id, name: group.name, type: ClassRootType.GROUP, teacherNames: [teacherUser.lastName], + studentCount: 0, }, { id: groupWithSystem.id, @@ -511,6 +647,7 @@ describe('GroupUc', () => { type: ClassRootType.GROUP, externalSourceName: system.displayName, teacherNames: [teacherUser.lastName], + studentCount: 1, }, ], total: 3, @@ -527,6 +664,7 @@ describe('GroupUc', () => { adminUser.school.id, undefined, undefined, + undefined, 'externalSourceName', SortOrder.desc ); @@ -541,6 +679,7 @@ describe('GroupUc', () => { teacherNames: [teacherUser.lastName], schoolYear: schoolYear.name, isUpgradable: false, + studentCount: 2, }, { id: groupWithSystem.id, @@ -548,12 +687,14 @@ describe('GroupUc', () => { type: ClassRootType.GROUP, externalSourceName: system.displayName, teacherNames: [teacherUser.lastName], + studentCount: 1, }, { id: group.id, name: group.name, type: ClassRootType.GROUP, teacherNames: [teacherUser.lastName], + studentCount: 0, }, ], total: 3, @@ -568,6 +709,7 @@ describe('GroupUc', () => { const result: Page = await uc.findAllClasses( adminUser.id, adminUser.school.id, + undefined, 1, 1, 'name', @@ -581,6 +723,7 @@ describe('GroupUc', () => { name: group.name, type: ClassRootType.GROUP, teacherNames: [teacherUser.lastName], + studentCount: 0, }, ], total: 3, diff --git a/apps/server/src/modules/group/uc/group.uc.ts b/apps/server/src/modules/group/uc/group.uc.ts index f40750fc852..f7399fa2fc9 100644 --- a/apps/server/src/modules/group/uc/group.uc.ts +++ b/apps/server/src/modules/group/uc/group.uc.ts @@ -13,6 +13,8 @@ import { GroupService } from '../service'; import { SortHelper } from '../util'; import { ClassInfoDto, ResolvedGroupDto, ResolvedGroupUser } from './dto'; import { GroupUcMapper } from './mapper/group-uc.mapper'; +import { SchoolYearQueryType } from '../controller/dto/interface'; +import { UnknownQueryTypeLoggableException } from '../loggable'; @Injectable() export class GroupUc { @@ -30,6 +32,7 @@ export class GroupUc { public async findAllClasses( userId: EntityId, schoolId: EntityId, + schoolYearQueryType?: SchoolYearQueryType, skip = 0, limit?: number, sortBy: keyof ClassInfoDto = 'name', @@ -51,9 +54,9 @@ export class GroupUc { let combinedClassInfo: ClassInfoDto[]; if (canSeeFullList) { - combinedClassInfo = await this.findCombinedClassListForSchool(schoolId); + combinedClassInfo = await this.findCombinedClassListForSchool(schoolId, schoolYearQueryType); } else { - combinedClassInfo = await this.findCombinedClassListForUser(userId); + combinedClassInfo = await this.findCombinedClassListForUser(userId, schoolYearQueryType); } combinedClassInfo.sort((a: ClassInfoDto, b: ClassInfoDto): number => @@ -67,61 +70,142 @@ export class GroupUc { return page; } - private async findCombinedClassListForSchool(schoolId: EntityId): Promise { - const [classInfosFromClasses, classInfosFromGroups] = await Promise.all([ - await this.findClassesForSchool(schoolId), - await this.findGroupsOfTypeClassForSchool(schoolId), - ]); + private async findCombinedClassListForSchool( + schoolId: EntityId, + schoolYearQueryType?: SchoolYearQueryType + ): Promise { + let classInfosFromGroups: ClassInfoDto[] = []; + + const classInfosFromClasses = await this.findClassesForSchool(schoolId, schoolYearQueryType); + + if (!schoolYearQueryType || schoolYearQueryType === SchoolYearQueryType.CURRENT_YEAR) { + classInfosFromGroups = await this.findGroupsOfTypeClassForSchool(schoolId); + } const combinedClassInfo: ClassInfoDto[] = [...classInfosFromClasses, ...classInfosFromGroups]; return combinedClassInfo; } - private async findCombinedClassListForUser(userId: EntityId): Promise { - const [classInfosFromClasses, classInfosFromGroups] = await Promise.all([ - await this.findClassesForUser(userId), - await this.findGroupsOfTypeClassForUser(userId), - ]); + private async findCombinedClassListForUser( + userId: EntityId, + schoolYearQueryType?: SchoolYearQueryType + ): Promise { + let classInfosFromGroups: ClassInfoDto[] = []; + + const classInfosFromClasses = await this.findClassesForUser(userId, schoolYearQueryType); + + if (!schoolYearQueryType || schoolYearQueryType === SchoolYearQueryType.CURRENT_YEAR) { + classInfosFromGroups = await this.findGroupsOfTypeClassForUser(userId); + } const combinedClassInfo: ClassInfoDto[] = [...classInfosFromClasses, ...classInfosFromGroups]; return combinedClassInfo; } - private async findClassesForSchool(schoolId: EntityId): Promise { + private async findClassesForSchool( + schoolId: EntityId, + schoolYearQueryType?: SchoolYearQueryType + ): Promise { const classes: Class[] = await this.classService.findClassesForSchool(schoolId); - const classInfosFromClasses: ClassInfoDto[] = await Promise.all( - classes.map((clazz) => this.getClassInfoFromClass(clazz)) - ); + const classInfosFromClasses: ClassInfoDto[] = await this.getClassInfosFromClasses(classes, schoolYearQueryType); return classInfosFromClasses; } - private async findClassesForUser(userId: EntityId): Promise { + private async findClassesForUser( + userId: EntityId, + schoolYearQueryType?: SchoolYearQueryType + ): Promise { const classes: Class[] = await this.classService.findAllByUserId(userId); - const classInfosFromClasses: ClassInfoDto[] = await Promise.all( - classes.map((clazz) => this.getClassInfoFromClass(clazz)) + const classInfosFromClasses: ClassInfoDto[] = await this.getClassInfosFromClasses(classes, schoolYearQueryType); + + return classInfosFromClasses; + } + + private async getClassInfosFromClasses( + classes: Class[], + schoolYearQueryType?: SchoolYearQueryType + ): Promise { + const currentYear: SchoolYearEntity = await this.schoolYearService.getCurrentSchoolYear(); + + const classesWithSchoolYear: { clazz: Class; schoolYear?: SchoolYearEntity }[] = await this.addSchoolYearsToClasses( + classes + ); + + const filteredClassesForSchoolYear = classesWithSchoolYear.filter((classWithSchoolYear) => + this.isClassOfQueryType(currentYear, classWithSchoolYear.schoolYear, schoolYearQueryType) ); + const classInfosFromClasses = await this.mapClassInfosFromClasses(filteredClassesForSchoolYear); + return classInfosFromClasses; } - private async getClassInfoFromClass(clazz: Class): Promise { - const teachers: UserDO[] = await Promise.all( - clazz.teacherIds.map((teacherId: EntityId) => this.userService.findById(teacherId)) + private async addSchoolYearsToClasses(classes: Class[]): Promise<{ clazz: Class; schoolYear?: SchoolYearEntity }[]> { + const classesWithSchoolYear: { clazz: Class; schoolYear?: SchoolYearEntity }[] = await Promise.all( + classes.map(async (clazz) => { + let schoolYear: SchoolYearEntity | undefined; + if (clazz.year) { + schoolYear = await this.schoolYearService.findById(clazz.year); + } + + return { + clazz, + schoolYear, + }; + }) ); + return classesWithSchoolYear; + } - let schoolYear: SchoolYearEntity | undefined; - if (clazz.year) { - schoolYear = await this.schoolYearService.findById(clazz.year); + private isClassOfQueryType( + currentYear: SchoolYearEntity, + schoolYear?: SchoolYearEntity, + schoolYearQueryType?: SchoolYearQueryType + ): boolean { + if (schoolYearQueryType === undefined) { + return true; } - const mapped: ClassInfoDto = GroupUcMapper.mapClassToClassInfoDto(clazz, teachers, schoolYear); + if (schoolYear === undefined) { + return schoolYearQueryType === SchoolYearQueryType.CURRENT_YEAR; + } - return mapped; + switch (schoolYearQueryType) { + case SchoolYearQueryType.CURRENT_YEAR: + return schoolYear.startDate === currentYear.startDate; + case SchoolYearQueryType.NEXT_YEAR: + return schoolYear.startDate > currentYear.startDate; + case SchoolYearQueryType.PREVIOUS_YEARS: + return schoolYear.startDate < currentYear.startDate; + default: + throw new UnknownQueryTypeLoggableException(schoolYearQueryType); + } + } + + private async mapClassInfosFromClasses( + filteredClassesForSchoolYear: { clazz: Class; schoolYear?: SchoolYearEntity }[] + ): Promise { + const classInfosFromClasses = await Promise.all( + filteredClassesForSchoolYear.map(async (classWithSchoolYear): Promise => { + const teachers: UserDO[] = await Promise.all( + classWithSchoolYear.clazz.teacherIds.map((teacherId: EntityId) => this.userService.findById(teacherId)) + ); + + const mapped: ClassInfoDto = GroupUcMapper.mapClassToClassInfoDto( + classWithSchoolYear.clazz, + teachers, + classWithSchoolYear.schoolYear + ); + + return mapped; + }) + ); + return classInfosFromClasses; } private async findGroupsOfTypeClassForSchool(schoolId: EntityId): Promise { diff --git a/apps/server/src/modules/group/uc/mapper/group-uc.mapper.ts b/apps/server/src/modules/group/uc/mapper/group-uc.mapper.ts index f65e8cca602..52ff160921f 100644 --- a/apps/server/src/modules/group/uc/mapper/group-uc.mapper.ts +++ b/apps/server/src/modules/group/uc/mapper/group-uc.mapper.ts @@ -19,6 +19,8 @@ export class GroupUcMapper { teacherNames: resolvedUsers .filter((groupUser: ResolvedGroupUser) => groupUser.role.name === RoleName.TEACHER) .map((groupUser: ResolvedGroupUser) => groupUser.user.lastName), + studentCount: resolvedUsers.filter((groupUser: ResolvedGroupUser) => groupUser.role.name === RoleName.STUDENT) + .length, }); return mapped; @@ -36,6 +38,7 @@ export class GroupUcMapper { teacherNames: teachers.map((user: UserDO) => user.lastName), schoolYear: schoolYear?.name, isUpgradable, + studentCount: clazz.userIds ? clazz.userIds.length : 0, }); return mapped;