diff --git a/apps/server/src/modules/board/board.module.ts b/apps/server/src/modules/board/board.module.ts index fb04364b6c3..09d00c46bbd 100644 --- a/apps/server/src/modules/board/board.module.ts +++ b/apps/server/src/modules/board/board.module.ts @@ -1,12 +1,12 @@ +import { FilesStorageClientModule } from '@modules/files-storage-client'; +import { ContextExternalToolModule } from '@modules/tool/context-external-tool'; +import { UserModule } from '@modules/user'; import { Module } from '@nestjs/common'; import { ContentElementFactory } from '@shared/domain'; import { ConsoleWriterModule } from '@shared/infra/console'; import { CourseRepo } from '@shared/repo'; import { LoggerModule } from '@src/core/logger'; -import { FilesStorageClientModule } from '../files-storage-client'; -import { UserModule } from '../user'; -import { BoardDoRepo, BoardNodeRepo } from './repo'; -import { RecursiveDeleteVisitor } from './repo/recursive-delete.vistor'; +import { BoardDoRepo, BoardNodeRepo, RecursiveDeleteVisitor } from './repo'; import { BoardDoAuthorizableService, BoardDoService, @@ -21,7 +21,7 @@ import { BoardDoCopyService, SchoolSpecificFileCopyServiceFactory } from './serv import { ColumnBoardCopyService } from './service/column-board-copy.service'; @Module({ - imports: [ConsoleWriterModule, FilesStorageClientModule, LoggerModule, UserModule], + imports: [ConsoleWriterModule, FilesStorageClientModule, LoggerModule, UserModule, ContextExternalToolModule], providers: [ BoardDoAuthorizableService, BoardDoRepo, diff --git a/apps/server/src/modules/board/repo/board-do.repo.spec.ts b/apps/server/src/modules/board/repo/board-do.repo.spec.ts index aa1c49224fe..3874e9301ba 100644 --- a/apps/server/src/modules/board/repo/board-do.repo.spec.ts +++ b/apps/server/src/modules/board/repo/board-do.repo.spec.ts @@ -1,6 +1,8 @@ import { createMock } from '@golevelup/ts-jest'; import { NotFoundError } from '@mikro-orm/core'; import { EntityManager } from '@mikro-orm/mongodb'; +import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; +import { ContextExternalToolService } from '@modules/tool/context-external-tool/service'; import { NotFoundException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { @@ -26,7 +28,6 @@ import { richTextElementFactory, richTextElementNodeFactory, } from '@shared/testing'; -import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; import { BoardDoRepo } from './board-do.repo'; import { BoardNodeRepo } from './board-node.repo'; import { RecursiveDeleteVisitor } from './recursive-delete.vistor'; @@ -46,6 +47,7 @@ describe(BoardDoRepo.name, () => { BoardNodeRepo, RecursiveDeleteVisitor, { provide: FilesStorageClientAdapterService, useValue: createMock() }, + { provide: ContextExternalToolService, useValue: createMock() }, ], }).compile(); repo = module.get(BoardDoRepo); diff --git a/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts b/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts index 9142cb33553..6236d5de8bb 100644 --- a/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts +++ b/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts @@ -1,10 +1,13 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { EntityManager } from '@mikro-orm/mongodb'; +import { FileDto, FilesStorageClientAdapterService } from '@modules/files-storage-client'; +import { ContextExternalToolService } from '@modules/tool/context-external-tool/service'; import { Test, TestingModule } from '@nestjs/testing'; import { FileRecordParentType } from '@shared/infra/rabbitmq'; import { columnBoardFactory, columnFactory, + contextExternalToolFactory, externalToolElementFactory, fileElementFactory, linkElementFactory, @@ -12,14 +15,15 @@ import { submissionContainerElementFactory, submissionItemFactory, } from '@shared/testing'; -import { FileDto, FilesStorageClientAdapterService } from '@modules/files-storage-client'; import { RecursiveDeleteVisitor } from './recursive-delete.vistor'; describe(RecursiveDeleteVisitor.name, () => { let module: TestingModule; + let service: RecursiveDeleteVisitor; + let em: DeepMocked; let filesStorageClientAdapterService: DeepMocked; - let service: RecursiveDeleteVisitor; + let contextExternalToolService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -27,12 +31,15 @@ describe(RecursiveDeleteVisitor.name, () => { RecursiveDeleteVisitor, { provide: EntityManager, useValue: createMock() }, { provide: FilesStorageClientAdapterService, useValue: createMock() }, + { provide: ContextExternalToolService, useValue: createMock() }, ], }).compile(); + service = module.get(RecursiveDeleteVisitor); em = module.get(EntityManager); filesStorageClientAdapterService = module.get(FilesStorageClientAdapterService); - service = module.get(RecursiveDeleteVisitor); + contextExternalToolService = module.get(ContextExternalToolService); + await setupEntities(); }); @@ -212,14 +219,30 @@ describe(RecursiveDeleteVisitor.name, () => { describe('visitExternalToolElementAsync', () => { const setup = () => { + const contextExternalTool = contextExternalToolFactory.buildWithId(); const childExternalToolElement = externalToolElementFactory.build(); const externalToolElement = externalToolElementFactory.build({ children: [childExternalToolElement], + contextExternalToolId: contextExternalTool.id, }); - return { externalToolElement, childExternalToolElement }; + contextExternalToolService.findById.mockResolvedValue(contextExternalTool); + + return { + externalToolElement, + childExternalToolElement, + contextExternalTool, + }; }; + it('should delete the context external tool that is linked to the element', async () => { + const { externalToolElement, contextExternalTool } = setup(); + + await service.visitExternalToolElementAsync(externalToolElement); + + expect(contextExternalToolService.deleteContextExternalTool).toHaveBeenCalledWith(contextExternalTool); + }); + it('should call entity remove', async () => { const { externalToolElement, childExternalToolElement } = setup(); diff --git a/apps/server/src/modules/board/repo/recursive-delete.vistor.ts b/apps/server/src/modules/board/repo/recursive-delete.vistor.ts index 1c407391da4..0a1b08e663a 100644 --- a/apps/server/src/modules/board/repo/recursive-delete.vistor.ts +++ b/apps/server/src/modules/board/repo/recursive-delete.vistor.ts @@ -1,4 +1,7 @@ import { EntityManager } from '@mikro-orm/mongodb'; +import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; +import { ContextExternalTool } from '@modules/tool/context-external-tool/domain'; +import { ContextExternalToolService } from '@modules/tool/context-external-tool/service'; import { Injectable } from '@nestjs/common'; import { AnyBoardDo, @@ -14,13 +17,13 @@ import { SubmissionItem, } from '@shared/domain'; import { LinkElement } from '@shared/domain/domainobject/board/link-element.do'; -import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; @Injectable() export class RecursiveDeleteVisitor implements BoardCompositeVisitorAsync { constructor( private readonly em: EntityManager, - private readonly filesStorageClientAdapterService: FilesStorageClientAdapterService + private readonly filesStorageClientAdapterService: FilesStorageClientAdapterService, + private readonly contextExternalToolService: ContextExternalToolService ) {} async visitColumnBoardAsync(columnBoard: ColumnBoard): Promise { @@ -67,7 +70,14 @@ export class RecursiveDeleteVisitor implements BoardCompositeVisitorAsync { } async visitExternalToolElementAsync(externalToolElement: ExternalToolElement): Promise { - // TODO N21-1296: Delete linked ContextExternalTool + if (externalToolElement.contextExternalToolId) { + const linkedTool: ContextExternalTool = await this.contextExternalToolService.findById( + externalToolElement.contextExternalToolId + ); + + await this.contextExternalToolService.deleteContextExternalTool(linkedTool); + } + this.deleteNode(externalToolElement); await this.visitChildrenAsync(externalToolElement); diff --git a/apps/server/src/modules/board/service/column-board.service.spec.ts b/apps/server/src/modules/board/service/column-board.service.spec.ts index 6ed6bb6f0a4..97b4a3d2578 100644 --- a/apps/server/src/modules/board/service/column-board.service.spec.ts +++ b/apps/server/src/modules/board/service/column-board.service.spec.ts @@ -2,6 +2,7 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { IConfig } from '@hpi-schul-cloud/commons/lib/interfaces/IConfig'; import { Test, TestingModule } from '@nestjs/testing'; +import { NotFoundLoggableException } from '@shared/common/loggable-exception'; import { BoardExternalReference, BoardExternalReferenceType, @@ -106,6 +107,87 @@ describe(ColumnBoardService.name, () => { }); }); + describe('findByDescendant', () => { + describe('when searching a board for an element', () => { + const setup2 = () => { + const element = richTextElementFactory.build(); + const board: ColumnBoard = columnBoardFactory.build({ children: [element] }); + + boardDoRepo.getAncestorIds.mockResolvedValue([board.id]); + boardDoRepo.findById.mockResolvedValue(board); + + return { + element, + board, + }; + }; + + it('should search by the root id', async () => { + const { element, board } = setup2(); + + await service.findByDescendant(element); + + expect(boardDoRepo.findById).toHaveBeenCalledWith(board.id, 1); + }); + + it('should return the board', async () => { + const { element, board } = setup2(); + + const result = await service.findByDescendant(element); + + expect(result).toEqual(board); + }); + }); + + describe('when searching a board by itself', () => { + const setup2 = () => { + const board: ColumnBoard = columnBoardFactory.build({ children: [] }); + + boardDoRepo.getAncestorIds.mockResolvedValue([]); + boardDoRepo.findById.mockResolvedValue(board); + + return { + board, + }; + }; + + it('should search by the root id', async () => { + const { board } = setup2(); + + await service.findByDescendant(board); + + expect(boardDoRepo.findById).toHaveBeenCalledWith(board.id, 1); + }); + + it('should return the board', async () => { + const { board } = setup2(); + + const result = await service.findByDescendant(board); + + expect(result).toEqual(board); + }); + }); + + describe('when the root node is not a board', () => { + const setup2 = () => { + const element = richTextElementFactory.build(); + + boardDoRepo.getAncestorIds.mockResolvedValue([]); + boardDoRepo.findById.mockResolvedValue(element); + + return { + element, + }; + }; + + it('should throw a NotFoundLoggableException', async () => { + const { element } = setup2(); + + await expect(service.findByDescendant(element)).rejects.toThrow(NotFoundLoggableException); + }); + }); + }); + describe('getBoardObjectTitlesById', () => { describe('when asking for a list of boardObject-ids', () => { const setupBoards = () => { diff --git a/apps/server/src/modules/board/service/column-board.service.ts b/apps/server/src/modules/board/service/column-board.service.ts index f53f4f5f051..7a455e0fcfa 100644 --- a/apps/server/src/modules/board/service/column-board.service.ts +++ b/apps/server/src/modules/board/service/column-board.service.ts @@ -1,6 +1,8 @@ import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { Injectable } from '@nestjs/common'; +import { NotFoundLoggableException } from '@shared/common/loggable-exception'; import { + AnyBoardDo, BoardExternalReference, Card, Column, @@ -34,6 +36,19 @@ export class ColumnBoardService { return ids; } + async findByDescendant(boardDo: AnyBoardDo): Promise { + const ancestorIds: EntityId[] = await this.boardDoRepo.getAncestorIds(boardDo); + const idHierarchy: EntityId[] = [...ancestorIds, boardDo.id]; + const rootId: EntityId = idHierarchy[0]; + const rootBoardDo: AnyBoardDo = await this.boardDoRepo.findById(rootId, 1); + + if (rootBoardDo instanceof ColumnBoard) { + return rootBoardDo; + } + + throw new NotFoundLoggableException(ColumnBoard.name, 'id', rootId); + } + async getBoardObjectTitlesById(boardIds: EntityId[]): Promise> { const titleMap = this.boardDoRepo.getTitlesByIds(boardIds); return titleMap; diff --git a/apps/server/src/modules/class/domain/testing/index.ts b/apps/server/src/modules/class/domain/testing/index.ts new file mode 100644 index 00000000000..3c5809ece1b --- /dev/null +++ b/apps/server/src/modules/class/domain/testing/index.ts @@ -0,0 +1 @@ +export * from './factory/class.factory'; diff --git a/apps/server/src/modules/class/entity/testing/index.ts b/apps/server/src/modules/class/entity/testing/index.ts new file mode 100644 index 00000000000..45893909755 --- /dev/null +++ b/apps/server/src/modules/class/entity/testing/index.ts @@ -0,0 +1 @@ +export * from './factory/class.entity.factory'; diff --git a/apps/server/src/modules/class/repo/classes.repo.spec.ts b/apps/server/src/modules/class/repo/classes.repo.spec.ts index 302a8f2de8e..7801045aff0 100644 --- a/apps/server/src/modules/class/repo/classes.repo.spec.ts +++ b/apps/server/src/modules/class/repo/classes.repo.spec.ts @@ -1,10 +1,11 @@ import { EntityManager, ObjectId } from '@mikro-orm/mongodb'; +import { classEntityFactory } from '@modules/class/entity/testing/factory/class.entity.factory'; import { Test } from '@nestjs/testing'; import { TestingModule } from '@nestjs/testing/testing-module'; +import { NotFoundLoggableException } from '@shared/common/loggable-exception'; import { SchoolEntity } from '@shared/domain'; import { MongoMemoryDatabaseModule } from '@shared/infra/database'; import { cleanupCollections, schoolFactory } from '@shared/testing'; -import { classEntityFactory } from '@modules/class/entity/testing/factory/class.entity.factory'; import { Class } from '../domain'; import { ClassEntity } from '../entity'; import { ClassesRepo } from './classes.repo'; @@ -48,6 +49,7 @@ describe(ClassesRepo.name, () => { const classes: ClassEntity[] = classEntityFactory.buildListWithId(3, { schoolId: school.id }); await em.persistAndFlush(classes); + em.clear(); return { school, @@ -78,9 +80,13 @@ describe(ClassesRepo.name, () => { const setup = async () => { const testUser = new ObjectId(); const class1: ClassEntity = classEntityFactory.withUserIds([testUser, new ObjectId()]).buildWithId(); - const class2: ClassEntity = classEntityFactory.withUserIds([testUser, new ObjectId()]).buildWithId(); + const class2: ClassEntity = classEntityFactory + .withUserIds([new ObjectId()]) + .buildWithId({ teacherIds: [testUser] }); const class3: ClassEntity = classEntityFactory.withUserIds([new ObjectId(), new ObjectId()]).buildWithId(); + await em.persistAndFlush([class1, class2, class3]); + em.clear(); return { class1, @@ -104,7 +110,7 @@ describe(ClassesRepo.name, () => { }); describe('updateMany', () => { - describe('When deleting user data from classes', () => { + describe('when deleting user data from classes', () => { const setup = async () => { const testUser1 = new ObjectId(); const testUser2 = new ObjectId(); @@ -112,7 +118,9 @@ describe(ClassesRepo.name, () => { const class1: ClassEntity = classEntityFactory.withUserIds([testUser1, testUser2]).buildWithId(); const class2: ClassEntity = classEntityFactory.withUserIds([testUser1, testUser3]).buildWithId(); const class3: ClassEntity = classEntityFactory.withUserIds([testUser2, testUser3]).buildWithId(); + await em.persistAndFlush([class1, class2, class3]); + em.clear(); return { class1, @@ -144,5 +152,29 @@ describe(ClassesRepo.name, () => { expect(result3).toHaveLength(2); }); }); + + describe('when updating a class that does not exist', () => { + const setup = async () => { + const class1: ClassEntity = classEntityFactory.buildWithId(); + const class2: ClassEntity = classEntityFactory.buildWithId(); + + await em.persistAndFlush([class1]); + em.clear(); + + return { + class1, + class2, + }; + }; + + it('should throw an error', async () => { + const { class1, class2 } = await setup(); + + const updatedArray: ClassEntity[] = [class1, class2]; + const domainObjectsArray: Class[] = ClassMapper.mapToDOs(updatedArray); + + await expect(repo.updateMany(domainObjectsArray)).rejects.toThrow(NotFoundLoggableException); + }); + }); }); }); diff --git a/apps/server/src/modules/class/repo/classes.repo.ts b/apps/server/src/modules/class/repo/classes.repo.ts index 378b3de9716..24300bbe673 100644 --- a/apps/server/src/modules/class/repo/classes.repo.ts +++ b/apps/server/src/modules/class/repo/classes.repo.ts @@ -1,5 +1,6 @@ import { EntityManager, ObjectId } from '@mikro-orm/mongodb'; import { Injectable } from '@nestjs/common'; +import { NotFoundLoggableException } from '@shared/common/loggable-exception'; import { EntityId } from '@shared/domain'; import { Class } from '../domain'; import { ClassEntity } from '../entity'; @@ -18,7 +19,9 @@ export class ClassesRepo { } async findAllByUserId(userId: EntityId): Promise { - const classes: ClassEntity[] = await this.em.find(ClassEntity, { userIds: new ObjectId(userId) }); + const classes: ClassEntity[] = await this.em.find(ClassEntity, { + $or: [{ userIds: new ObjectId(userId) }, { teacherIds: new ObjectId(userId) }], + }); const mapped: Class[] = ClassMapper.mapToDOs(classes); @@ -26,9 +29,30 @@ export class ClassesRepo { } async updateMany(classes: Class[]): Promise { - const classesEntities = ClassMapper.mapToEntities(classes); - const referencedEntities = classesEntities.map((classEntity) => this.em.getReference(ClassEntity, classEntity.id)); + const classMap: Map = new Map( + classes.map((clazz: Class): [string, Class] => [clazz.id, clazz]) + ); - await this.em.persistAndFlush(referencedEntities); + const existingEntities: ClassEntity[] = await this.em.find(ClassEntity, { + id: { $in: Array.from(classMap.keys()) }, + }); + + if (existingEntities.length < classes.length) { + const missingEntityIds: string[] = Array.from(classMap.keys()).filter( + (classId) => !existingEntities.find((entity) => entity.id === classId) + ); + + throw new NotFoundLoggableException(Class.name, 'id', missingEntityIds.toString()); + } + + existingEntities.forEach((entity) => { + const updatedDomainObject: Class | undefined = classMap.get(entity.id); + + const updatedEntity: ClassEntity = ClassMapper.mapToEntity(updatedDomainObject as Class); + + this.em.assign(entity, updatedEntity); + }); + + await this.em.persistAndFlush(existingEntities); } } diff --git a/apps/server/src/modules/class/repo/mapper/class.mapper.spec.ts b/apps/server/src/modules/class/repo/mapper/class.mapper.spec.ts index 53d0ecd4360..f1e71da5c4d 100644 --- a/apps/server/src/modules/class/repo/mapper/class.mapper.spec.ts +++ b/apps/server/src/modules/class/repo/mapper/class.mapper.spec.ts @@ -3,7 +3,7 @@ import { Class } from '../../domain'; import { ClassSourceOptions } from '../../domain/class-source-options.do'; import { classFactory } from '../../domain/testing/factory/class.factory'; import { ClassEntity } from '../../entity'; -import { classEntityFactory } from '../../entity/testing/factory/class.entity.factory'; +import { classEntityFactory } from '../../entity/testing'; import { ClassMapper } from './class.mapper'; describe(ClassMapper.name, () => { diff --git a/apps/server/src/modules/class/repo/mapper/class.mapper.ts b/apps/server/src/modules/class/repo/mapper/class.mapper.ts index 6340ffce7b0..8ae5e3b79b9 100644 --- a/apps/server/src/modules/class/repo/mapper/class.mapper.ts +++ b/apps/server/src/modules/class/repo/mapper/class.mapper.ts @@ -23,7 +23,7 @@ export class ClassMapper { }); } - private static mapToEntity(domainObject: Class): ClassEntity { + static mapToEntity(domainObject: Class): ClassEntity { return new ClassEntity({ id: domainObject.id, name: domainObject.name, diff --git a/apps/server/src/modules/class/service/class.service.spec.ts b/apps/server/src/modules/class/service/class.service.spec.ts index 850eaf655a6..5ba4b367b59 100644 --- a/apps/server/src/modules/class/service/class.service.spec.ts +++ b/apps/server/src/modules/class/service/class.service.spec.ts @@ -4,9 +4,9 @@ import { InternalServerErrorException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { EntityId } from '@shared/domain'; import { setupEntities } from '@shared/testing'; -import { classEntityFactory } from '@modules/class/entity/testing/factory/class.entity.factory'; import { Class } from '../domain'; -import { classFactory } from '../domain/testing/factory/class.factory'; +import { classFactory } from '../domain/testing'; +import { classEntityFactory } from '../entity/testing'; import { ClassesRepo } from '../repo'; import { ClassMapper } from '../repo/mapper'; import { ClassService } from './class.service'; @@ -74,6 +74,31 @@ describe(ClassService.name, () => { }); }); + describe('findAllByUserId', () => { + describe('when the user has classes', () => { + const setup = () => { + const userId: string = new ObjectId().toHexString(); + + const classes: Class[] = classFactory.buildList(3); + + classesRepo.findAllByUserId.mockResolvedValueOnce(classes); + + return { + userId, + classes, + }; + }; + + it('should return the classes', async () => { + const { userId, classes } = setup(); + + const result: Class[] = await service.findAllByUserId(userId); + + expect(result).toEqual(classes); + }); + }); + }); + describe('deleteUserDataFromClasses', () => { describe('when user is missing', () => { const setup = () => { diff --git a/apps/server/src/modules/class/service/class.service.ts b/apps/server/src/modules/class/service/class.service.ts index 9671c456912..772b9f0c4d4 100644 --- a/apps/server/src/modules/class/service/class.service.ts +++ b/apps/server/src/modules/class/service/class.service.ts @@ -13,6 +13,13 @@ export class ClassService { return classes; } + public async findAllByUserId(userId: EntityId): Promise { + const classes: Class[] = await this.classesRepo.findAllByUserId(userId); + + return classes; + } + + // FIXME There is no usage of this method public async deleteUserDataFromClasses(userId: EntityId): Promise { if (!userId) { throw new InternalServerErrorException('User id is missing'); 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 34a49c03a35..2d9f4105f80 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 @@ -1,4 +1,7 @@ import { EntityManager } from '@mikro-orm/mongodb'; +import { ClassEntity } from '@modules/class/entity'; +import { classEntityFactory } from '@modules/class/entity/testing'; +import { ServerTestModule } from '@modules/server'; import { HttpStatus, INestApplication } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { Role, RoleName, SchoolEntity, SchoolYearEntity, SortOrder, SystemEntity, User } from '@shared/domain'; @@ -12,9 +15,6 @@ import { UserAndAccountTestFactory, userFactory, } from '@shared/testing'; -import { ClassEntity } from '@modules/class/entity'; -import { classEntityFactory } from '@modules/class/entity/testing/factory/class.entity.factory'; -import { ServerTestModule } from '@modules/server'; import { ObjectId } from 'bson'; import { GroupEntity, GroupEntityTypes } from '../../entity'; import { ClassRootType } from '../../uc/dto/class-root-type'; @@ -135,29 +135,6 @@ describe('Group (API)', () => { }); }); }); - - describe('when an invalid user requests a list of classes', () => { - const setup = async () => { - const { studentAccount, studentUser } = UserAndAccountTestFactory.buildStudent(); - - await em.persistAndFlush([studentAccount, studentUser]); - em.clear(); - - const studentClient = await testApiClient.login(studentAccount); - - return { - studentClient, - }; - }; - - it('should return forbidden', async () => { - const { studentClient } = await setup(); - - const response = await studentClient.get(`/class`); - - expect(response.status).toEqual(HttpStatus.FORBIDDEN); - }); - }); }); describe('[GET] /groups/:groupId', () => { diff --git a/apps/server/src/modules/group/controller/group.controller.ts b/apps/server/src/modules/group/controller/group.controller.ts index 9e5f4b3b51a..a7dc0c77563 100644 --- a/apps/server/src/modules/group/controller/group.controller.ts +++ b/apps/server/src/modules/group/controller/group.controller.ts @@ -1,9 +1,9 @@ +import { Authenticate, CurrentUser, ICurrentUser } from '@modules/authentication'; import { Controller, Get, HttpStatus, Param, Query } from '@nestjs/common'; import { ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger'; import { PaginationParams } from '@shared/controller'; import { Page } from '@shared/domain'; import { ErrorResponse } from '@src/core/error/dto'; -import { ICurrentUser, Authenticate, CurrentUser } from '@modules/authentication'; import { GroupUc } from '../uc'; import { ClassInfoDto, ResolvedGroupDto } from '../uc/dto'; import { ClassInfoSearchListResponse, ClassSortParams, GroupIdParams, GroupResponse } from './dto'; @@ -15,17 +15,17 @@ import { GroupResponseMapper } from './mapper'; export class GroupController { constructor(private readonly groupUc: GroupUc) {} - @ApiOperation({ summary: 'Get a list of classes and groups of type class for the current users school.' }) + @ApiOperation({ summary: 'Get a list of classes and groups of type class for the current user.' }) @ApiResponse({ status: HttpStatus.OK, type: ClassInfoSearchListResponse }) @ApiResponse({ status: '4XX', type: ErrorResponse }) @ApiResponse({ status: '5XX', type: ErrorResponse }) @Get('/class') - public async findClassesForSchool( + public async findClasses( @Query() pagination: PaginationParams, @Query() sortingQuery: ClassSortParams, @CurrentUser() currentUser: ICurrentUser ): Promise { - const board: Page = await this.groupUc.findAllClassesForSchool( + const board: Page = await this.groupUc.findAllClasses( currentUser.userId, currentUser.schoolId, pagination.skip, 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 6efd02d899d..8c990cbd44a 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 @@ -40,7 +40,7 @@ export class GroupResponseMapper { type: classInfo.type, name: classInfo.name, externalSourceName: classInfo.externalSourceName, - teachers: classInfo.teachers, + teachers: classInfo.teacherNames, schoolYear: classInfo.schoolYear, isUpgradable: classInfo.isUpgradable, }); 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 8c564d9e106..611275e3bcd 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 @@ -9,7 +9,7 @@ export class ClassInfoDto { externalSourceName?: string; - teachers: string[]; + teacherNames: string[]; schoolYear?: string; @@ -20,7 +20,7 @@ export class ClassInfoDto { this.type = props.type; this.name = props.name; this.externalSourceName = props.externalSourceName; - this.teachers = props.teachers; + this.teacherNames = props.teacherNames; this.schoolYear = props.schoolYear; this.isUpgradable = props.isUpgradable; } 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 34cb55a1354..d5236826def 100644 --- a/apps/server/src/modules/group/uc/group.uc.spec.ts +++ b/apps/server/src/modules/group/uc/group.uc.spec.ts @@ -1,5 +1,14 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; +import { Action, AuthorizationContext, AuthorizationService } from '@modules/authorization'; +import { ClassService } from '@modules/class'; +import { Class } from '@modules/class/domain'; +import { classFactory } from '@modules/class/domain/testing/factory/class.factory'; +import { LegacySchoolService, SchoolYearService } from '@modules/legacy-school'; +import { RoleService } from '@modules/role'; +import { RoleDto } from '@modules/role/service/dto/role.dto'; +import { SystemDto, SystemService } from '@modules/system'; +import { UserService } from '@modules/user'; import { ForbiddenException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { NotFoundLoggableException } from '@shared/common/loggable-exception'; @@ -14,15 +23,6 @@ import { userDoFactory, userFactory, } from '@shared/testing'; -import { Action, AuthorizationContext, AuthorizationService } from '@modules/authorization'; -import { ClassService } from '@modules/class'; -import { Class } from '@modules/class/domain'; -import { classFactory } from '@modules/class/domain/testing/factory/class.factory'; -import { LegacySchoolService, SchoolYearService } from '@modules/legacy-school'; -import { RoleService } from '@modules/role'; -import { RoleDto } from '@modules/role/service/dto/role.dto'; -import { SystemDto, SystemService } from '@modules/system'; -import { UserService } from '@modules/user'; import { Group, GroupTypes } from '../domain'; import { GroupService } from '../service'; import { ClassInfoDto, ResolvedGroupDto } from './dto'; @@ -102,7 +102,7 @@ describe('GroupUc', () => { jest.resetAllMocks(); }); - describe('findClassesForSchool', () => { + describe('findAllClasses', () => { describe('when the user has no permission', () => { const setup = () => { const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId(); @@ -114,6 +114,7 @@ describe('GroupUc', () => { authorizationService.checkPermission.mockImplementation(() => { throw error; }); + authorizationService.hasAllPermissions.mockReturnValueOnce(false); return { user, @@ -124,13 +125,13 @@ describe('GroupUc', () => { it('should throw forbidden', async () => { const { user, error } = setup(); - const func = () => uc.findAllClassesForSchool(user.id, user.school.id); + const func = () => uc.findAllClasses(user.id, user.school.id); await expect(func).rejects.toThrow(error); }); }); - describe('when the school has classes', () => { + describe('when accessing as a normal user', () => { const setup = () => { const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId(); const { studentUser } = UserAndAccountTestFactory.buildStudent(); @@ -181,8 +182,9 @@ describe('GroupUc', () => { schoolService.getSchoolById.mockResolvedValueOnce(school); authorizationService.getUserWithPermissions.mockResolvedValueOnce(teacherUser); - classService.findClassesForSchool.mockResolvedValueOnce([clazz]); - groupService.findClassesForSchool.mockResolvedValueOnce([group, groupWithSystem]); + authorizationService.hasAllPermissions.mockReturnValueOnce(false); + classService.findAllByUserId.mockResolvedValueOnce([clazz]); + groupService.findByUser.mockResolvedValueOnce([group, groupWithSystem]); systemService.findById.mockResolvedValue(system); userService.findById.mockImplementation((userId: string): Promise => { if (userId === teacherUser.id) { @@ -222,23 +224,34 @@ describe('GroupUc', () => { it('should check the required permissions', async () => { const { teacherUser, school } = setup(); - await uc.findAllClassesForSchool(teacherUser.id, teacherUser.school.id); + await uc.findAllClasses(teacherUser.id, teacherUser.school.id); expect(authorizationService.checkPermission).toHaveBeenCalledWith<[User, LegacySchoolDo, AuthorizationContext]>( teacherUser, school, { action: Action.read, - requiredPermissions: [Permission.CLASS_LIST, Permission.GROUP_LIST], + requiredPermissions: [Permission.CLASS_VIEW, Permission.GROUP_VIEW], } ); }); + it('should check the access to the full list', async () => { + const { teacherUser } = setup(); + + await uc.findAllClasses(teacherUser.id, teacherUser.school.id); + + expect(authorizationService.hasAllPermissions).toHaveBeenCalledWith<[User, string[]]>(teacherUser, [ + Permission.CLASS_FULL_ADMIN, + Permission.GROUP_FULL_ADMIN, + ]); + }); + 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.findAllClassesForSchool(teacherUser.id, teacherUser.school.id); + const result: Page = await uc.findAllClasses(teacherUser.id, teacherUser.school.id); expect(result).toEqual>({ data: [ @@ -247,7 +260,7 @@ describe('GroupUc', () => { name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, type: ClassRootType.CLASS, externalSourceName: clazz.source, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], schoolYear: schoolYear.name, isUpgradable: false, }, @@ -255,14 +268,14 @@ describe('GroupUc', () => { id: group.id, name: group.name, type: ClassRootType.GROUP, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], }, { id: groupWithSystem.id, name: groupWithSystem.name, type: ClassRootType.GROUP, externalSourceName: system.displayName, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], }, ], total: 3, @@ -274,7 +287,7 @@ describe('GroupUc', () => { it('should return all classes sorted by external source name in descending order', async () => { const { teacherUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); - const result: Page = await uc.findAllClassesForSchool( + const result: Page = await uc.findAllClasses( teacherUser.id, teacherUser.school.id, undefined, @@ -290,7 +303,7 @@ describe('GroupUc', () => { name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, type: ClassRootType.CLASS, externalSourceName: clazz.source, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], schoolYear: schoolYear.name, isUpgradable: false, }, @@ -299,13 +312,13 @@ describe('GroupUc', () => { name: groupWithSystem.name, type: ClassRootType.GROUP, externalSourceName: system.displayName, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], }, { id: group.id, name: group.name, type: ClassRootType.GROUP, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], }, ], total: 3, @@ -317,7 +330,7 @@ describe('GroupUc', () => { it('should return the selected page', async () => { const { teacherUser, group } = setup(); - const result: Page = await uc.findAllClassesForSchool( + const result: Page = await uc.findAllClasses( teacherUser.id, teacherUser.school.id, 1, @@ -332,7 +345,242 @@ describe('GroupUc', () => { id: group.id, name: group.name, type: ClassRootType.GROUP, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], + }, + ], + total: 3, + }); + }); + }); + }); + + describe('when accessing as a user with elevated permission', () => { + const setup = () => { + const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId(); + const { studentUser } = UserAndAccountTestFactory.buildStudent(); + const { teacherUser } = UserAndAccountTestFactory.buildTeacher(); + const { adminUser } = UserAndAccountTestFactory.buildAdmin(); + const teacherRole: RoleDto = roleDtoFactory.buildWithId({ + id: teacherUser.roles[0].id, + name: teacherUser.roles[0].name, + }); + const studentRole: RoleDto = roleDtoFactory.buildWithId({ + id: studentUser.roles[0].id, + name: studentUser.roles[0].name, + }); + const adminUserDo: UserDO = userDoFactory.buildWithId({ + id: adminUser.id, + lastName: adminUser.lastName, + roles: [{ id: adminUser.roles[0].id, name: adminUser.roles[0].name }], + }); + const teacherUserDo: UserDO = userDoFactory.buildWithId({ + id: teacherUser.id, + lastName: teacherUser.lastName, + roles: [{ id: teacherUser.roles[0].id, name: teacherUser.roles[0].name }], + }); + const studentUserDo: UserDO = userDoFactory.buildWithId({ + id: studentUser.id, + lastName: studentUser.lastName, + roles: [{ id: studentUser.roles[0].id, name: studentUser.roles[0].name }], + }); + const schoolYear: SchoolYearEntity = schoolYearFactory.buildWithId(); + const clazz: Class = classFactory.build({ + name: 'A', + teacherIds: [teacherUser.id], + source: 'LDAP', + year: schoolYear.id, + }); + const system: SystemDto = new SystemDto({ + id: new ObjectId().toHexString(), + displayName: 'External System', + type: 'oauth2', + }); + const group: Group = groupFactory.build({ + name: 'B', + users: [{ userId: teacherUser.id, roleId: teacherUser.roles[0].id }], + externalSource: undefined, + }); + const groupWithSystem: Group = groupFactory.build({ + name: 'C', + externalSource: { externalId: 'externalId', systemId: system.id }, + users: [ + { userId: teacherUser.id, roleId: teacherUser.roles[0].id }, + { userId: studentUser.id, roleId: studentUser.roles[0].id }, + ], + }); + + schoolService.getSchoolById.mockResolvedValueOnce(school); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(adminUser); + authorizationService.hasAllPermissions.mockReturnValueOnce(true); + classService.findClassesForSchool.mockResolvedValueOnce([clazz]); + groupService.findClassesForSchool.mockResolvedValueOnce([group, groupWithSystem]); + systemService.findById.mockResolvedValue(system); + + userService.findById.mockImplementation((userId: string): Promise => { + if (userId === teacherUser.id) { + return Promise.resolve(teacherUserDo); + } + + if (userId === studentUser.id) { + return Promise.resolve(studentUserDo); + } + + if (userId === adminUser.id) { + return Promise.resolve(adminUserDo); + } + + throw new Error(); + }); + roleService.findById.mockImplementation((roleId: string): Promise => { + if (roleId === teacherUser.roles[0].id) { + return Promise.resolve(teacherRole); + } + + if (roleId === studentUser.roles[0].id) { + return Promise.resolve(studentRole); + } + + throw new Error(); + }); + schoolYearService.findById.mockResolvedValue(schoolYear); + + return { + adminUser, + teacherUser, + school, + clazz, + group, + groupWithSystem, + system, + schoolYear, + }; + }; + + it('should check the required permissions', async () => { + const { adminUser, school } = setup(); + + await uc.findAllClasses(adminUser.id, adminUser.school.id); + + expect(authorizationService.checkPermission).toHaveBeenCalledWith<[User, LegacySchoolDo, AuthorizationContext]>( + adminUser, + school, + { + action: Action.read, + requiredPermissions: [Permission.CLASS_VIEW, Permission.GROUP_VIEW], + } + ); + }); + + it('should check the access to the full list', async () => { + const { adminUser } = setup(); + + await uc.findAllClasses(adminUser.id, adminUser.school.id); + + expect(authorizationService.hasAllPermissions).toHaveBeenCalledWith<[User, string[]]>(adminUser, [ + Permission.CLASS_FULL_ADMIN, + Permission.GROUP_FULL_ADMIN, + ]); + }); + + describe('when no pagination is given', () => { + it('should return all classes sorted by name', async () => { + const { adminUser, teacherUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); + + const result: Page = await uc.findAllClasses(adminUser.id, adminUser.school.id); + + expect(result).toEqual>({ + data: [ + { + id: clazz.id, + name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, + type: ClassRootType.CLASS, + externalSourceName: clazz.source, + teacherNames: [teacherUser.lastName], + schoolYear: schoolYear.name, + isUpgradable: false, + }, + { + id: group.id, + name: group.name, + type: ClassRootType.GROUP, + teacherNames: [teacherUser.lastName], + }, + { + id: groupWithSystem.id, + name: groupWithSystem.name, + type: ClassRootType.GROUP, + externalSourceName: system.displayName, + teacherNames: [teacherUser.lastName], + }, + ], + total: 3, + }); + }); + }); + + 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 { adminUser, teacherUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); + + const result: Page = await uc.findAllClasses( + adminUser.id, + adminUser.school.id, + undefined, + undefined, + 'externalSourceName', + SortOrder.desc + ); + + expect(result).toEqual>({ + data: [ + { + id: clazz.id, + name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, + type: ClassRootType.CLASS, + externalSourceName: clazz.source, + teacherNames: [teacherUser.lastName], + schoolYear: schoolYear.name, + isUpgradable: false, + }, + { + id: groupWithSystem.id, + name: groupWithSystem.name, + type: ClassRootType.GROUP, + externalSourceName: system.displayName, + teacherNames: [teacherUser.lastName], + }, + { + id: group.id, + name: group.name, + type: ClassRootType.GROUP, + teacherNames: [teacherUser.lastName], + }, + ], + total: 3, + }); + }); + }); + + describe('when using pagination', () => { + it('should return the selected page', async () => { + const { adminUser, teacherUser, group } = setup(); + + const result: Page = await uc.findAllClasses( + adminUser.id, + adminUser.school.id, + 1, + 1, + 'name', + SortOrder.asc + ); + + expect(result).toEqual>({ + data: [ + { + id: group.id, + name: group.name, + type: ClassRootType.GROUP, + teacherNames: [teacherUser.lastName], }, ], 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 2421e444e73..f40750fc852 100644 --- a/apps/server/src/modules/group/uc/group.uc.ts +++ b/apps/server/src/modules/group/uc/group.uc.ts @@ -1,5 +1,3 @@ -import { Injectable } from '@nestjs/common'; -import { EntityId, LegacySchoolDo, Page, Permission, SchoolYearEntity, SortOrder, User, UserDO } from '@shared/domain'; import { AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization'; import { ClassService } from '@modules/class'; import { Class } from '@modules/class/domain'; @@ -8,6 +6,8 @@ import { RoleService } from '@modules/role'; import { RoleDto } from '@modules/role/service/dto/role.dto'; import { SystemDto, SystemService } from '@modules/system'; import { UserService } from '@modules/user'; +import { Injectable } from '@nestjs/common'; +import { EntityId, LegacySchoolDo, Page, Permission, SchoolYearEntity, SortOrder, User, UserDO } from '@shared/domain'; import { Group, GroupUser } from '../domain'; import { GroupService } from '../service'; import { SortHelper } from '../util'; @@ -27,7 +27,7 @@ export class GroupUc { private readonly schoolYearService: SchoolYearService ) {} - public async findAllClassesForSchool( + public async findAllClasses( userId: EntityId, schoolId: EntityId, skip = 0, @@ -41,10 +41,20 @@ export class GroupUc { this.authorizationService.checkPermission( user, school, - AuthorizationContextBuilder.read([Permission.CLASS_LIST, Permission.GROUP_LIST]) + AuthorizationContextBuilder.read([Permission.CLASS_VIEW, Permission.GROUP_VIEW]) ); - const combinedClassInfo: ClassInfoDto[] = await this.findCombinedClassListForSchool(schoolId); + const canSeeFullList: boolean = this.authorizationService.hasAllPermissions(user, [ + Permission.CLASS_FULL_ADMIN, + Permission.GROUP_FULL_ADMIN, + ]); + + let combinedClassInfo: ClassInfoDto[]; + if (canSeeFullList) { + combinedClassInfo = await this.findCombinedClassListForSchool(schoolId); + } else { + combinedClassInfo = await this.findCombinedClassListForUser(userId); + } combinedClassInfo.sort((a: ClassInfoDto, b: ClassInfoDto): number => SortHelper.genericSortFunction(a[sortBy], b[sortBy], sortOrder) @@ -57,7 +67,7 @@ export class GroupUc { return page; } - private async findCombinedClassListForSchool(schoolId: string): Promise { + private async findCombinedClassListForSchool(schoolId: EntityId): Promise { const [classInfosFromClasses, classInfosFromGroups] = await Promise.all([ await this.findClassesForSchool(schoolId), await this.findGroupsOfTypeClassForSchool(schoolId), @@ -68,52 +78,91 @@ export class GroupUc { return combinedClassInfo; } + private async findCombinedClassListForUser(userId: EntityId): Promise { + const [classInfosFromClasses, classInfosFromGroups] = await Promise.all([ + await this.findClassesForUser(userId), + await this.findGroupsOfTypeClassForUser(userId), + ]); + + const combinedClassInfo: ClassInfoDto[] = [...classInfosFromClasses, ...classInfosFromGroups]; + + return combinedClassInfo; + } + private async findClassesForSchool(schoolId: EntityId): Promise { const classes: Class[] = await this.classService.findClassesForSchool(schoolId); const classInfosFromClasses: ClassInfoDto[] = await Promise.all( - classes.map(async (clazz: Class): Promise => { - const teachers: UserDO[] = await Promise.all( - clazz.teacherIds.map((teacherId: EntityId) => this.userService.findById(teacherId)) - ); + classes.map((clazz) => this.getClassInfoFromClass(clazz)) + ); - let schoolYear: SchoolYearEntity | undefined; - if (clazz.year) { - schoolYear = await this.schoolYearService.findById(clazz.year); - } + return classInfosFromClasses; + } - const mapped: ClassInfoDto = GroupUcMapper.mapClassToClassInfoDto(clazz, teachers, schoolYear); + private async findClassesForUser(userId: EntityId): Promise { + const classes: Class[] = await this.classService.findAllByUserId(userId); - return mapped; - }) + const classInfosFromClasses: ClassInfoDto[] = await Promise.all( + classes.map((clazz) => this.getClassInfoFromClass(clazz)) ); return classInfosFromClasses; } + private async getClassInfoFromClass(clazz: Class): Promise { + const teachers: UserDO[] = await Promise.all( + clazz.teacherIds.map((teacherId: EntityId) => this.userService.findById(teacherId)) + ); + + let schoolYear: SchoolYearEntity | undefined; + if (clazz.year) { + schoolYear = await this.schoolYearService.findById(clazz.year); + } + + const mapped: ClassInfoDto = GroupUcMapper.mapClassToClassInfoDto(clazz, teachers, schoolYear); + + return mapped; + } + private async findGroupsOfTypeClassForSchool(schoolId: EntityId): Promise { const groupsOfTypeClass: Group[] = await this.groupService.findClassesForSchool(schoolId); const systemMap: Map = await this.findSystemNamesForGroups(groupsOfTypeClass); const classInfosFromGroups: ClassInfoDto[] = await Promise.all( - groupsOfTypeClass.map(async (group: Group): Promise => { - let system: SystemDto | undefined; - if (group.externalSource) { - system = systemMap.get(group.externalSource.systemId); - } + groupsOfTypeClass.map(async (group: Group): Promise => this.getClassInfoFromGroup(group, systemMap)) + ); - const resolvedUsers: ResolvedGroupUser[] = await this.findUsersForGroup(group); + return classInfosFromGroups; + } - const mapped: ClassInfoDto = GroupUcMapper.mapGroupToClassInfoDto(group, resolvedUsers, system); + private async findGroupsOfTypeClassForUser(userId: EntityId): Promise { + const user: UserDO = await this.userService.findById(userId); - return mapped; - }) + const groupsOfTypeClass: Group[] = await this.groupService.findByUser(user); + + const systemMap: Map = await this.findSystemNamesForGroups(groupsOfTypeClass); + + const classInfosFromGroups: ClassInfoDto[] = await Promise.all( + groupsOfTypeClass.map(async (group: Group): Promise => this.getClassInfoFromGroup(group, systemMap)) ); return classInfosFromGroups; } + private async getClassInfoFromGroup(group: Group, systemMap: Map): Promise { + let system: SystemDto | undefined; + if (group.externalSource) { + system = systemMap.get(group.externalSource.systemId); + } + + const resolvedUsers: ResolvedGroupUser[] = await this.findUsersForGroup(group); + + const mapped: ClassInfoDto = GroupUcMapper.mapGroupToClassInfoDto(group, resolvedUsers, system); + + return mapped; + } + private async findSystemNamesForGroups(groups: Group[]): Promise> { const systemIds: EntityId[] = groups .map((group: Group): string | undefined => group.externalSource?.systemId) 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 5ac11f0e0b6..f65e8cca602 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 @@ -1,6 +1,6 @@ -import { RoleName, SchoolYearEntity, UserDO } from '@shared/domain'; import { Class } from '@modules/class/domain'; import { SystemDto } from '@modules/system'; +import { RoleName, SchoolYearEntity, UserDO } from '@shared/domain'; import { Group } from '../../domain'; import { ClassInfoDto, ResolvedGroupDto, ResolvedGroupUser } from '../dto'; import { ClassRootType } from '../dto/class-root-type'; @@ -16,7 +16,7 @@ export class GroupUcMapper { type: ClassRootType.GROUP, name: group.name, externalSourceName: system?.displayName, - teachers: resolvedUsers + teacherNames: resolvedUsers .filter((groupUser: ResolvedGroupUser) => groupUser.role.name === RoleName.TEACHER) .map((groupUser: ResolvedGroupUser) => groupUser.user.lastName), }); @@ -33,7 +33,7 @@ export class GroupUcMapper { type: ClassRootType.CLASS, name, externalSourceName: clazz.source, - teachers: teachers.map((user: UserDO) => user.lastName), + teacherNames: teachers.map((user: UserDO) => user.lastName), schoolYear: schoolYear?.name, isUpgradable, }); diff --git a/apps/server/src/modules/provisioning/strategy/oidc/oidc.strategy.spec.ts b/apps/server/src/modules/provisioning/strategy/oidc/oidc.strategy.spec.ts index 2b838159524..e4f50429d0f 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/oidc.strategy.spec.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/oidc.strategy.spec.ts @@ -269,5 +269,46 @@ describe('OidcStrategy', () => { expect(oidcProvisioningService.provisionExternalGroup).not.toHaveBeenCalled(); }); }); + + describe('when group data is not provided', () => { + const setup = () => { + Configuration.set('FEATURE_SANIS_GROUP_PROVISIONING_ENABLED', true); + + const externalUserId = 'externalUserId'; + const oauthData: OauthDataDto = new OauthDataDto({ + system: new ProvisioningSystemDto({ + systemId: 'systemId', + provisioningStrategy: SystemProvisioningStrategy.OIDC, + }), + externalUser: new ExternalUserDto({ + externalId: externalUserId, + }), + externalGroups: undefined, + }); + + const user: UserDO = userDoFactory.withRoles([{ id: 'roleId', name: RoleName.USER }]).build({ + externalId: externalUserId, + }); + + oidcProvisioningService.provisionExternalUser.mockResolvedValue(user); + + return { + externalUserId, + oauthData, + }; + }; + + it('should remove external groups and affiliation', async () => { + const { externalUserId, oauthData } = setup(); + + await strategy.apply(oauthData); + + expect(oidcProvisioningService.removeExternalGroupsAndAffiliation).toHaveBeenCalledWith( + externalUserId, + [], + oauthData.system.systemId + ); + }); + }); }); }); diff --git a/apps/server/src/modules/provisioning/strategy/oidc/oidc.strategy.ts b/apps/server/src/modules/provisioning/strategy/oidc/oidc.strategy.ts index 7804f2190f9..4cb3e920da6 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/oidc.strategy.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/oidc.strategy.ts @@ -23,18 +23,20 @@ export abstract class OidcProvisioningStrategy extends ProvisioningStrategy { school?.id ); - if (Configuration.get('FEATURE_SANIS_GROUP_PROVISIONING_ENABLED') && data.externalGroups) { + if (Configuration.get('FEATURE_SANIS_GROUP_PROVISIONING_ENABLED')) { await this.oidcProvisioningService.removeExternalGroupsAndAffiliation( data.externalUser.externalId, - data.externalGroups, + data.externalGroups ?? [], data.system.systemId ); - await Promise.all( - data.externalGroups.map((externalGroup) => - this.oidcProvisioningService.provisionExternalGroup(externalGroup, data.system.systemId) - ) - ); + if (data.externalGroups) { + await Promise.all( + data.externalGroups.map((externalGroup) => + this.oidcProvisioningService.provisionExternalGroup(externalGroup, data.system.systemId) + ) + ); + } } return new ProvisioningDto({ externalUserId: user.externalId || data.externalUser.externalId }); diff --git a/apps/server/src/modules/pseudonym/pseudonym.module.ts b/apps/server/src/modules/pseudonym/pseudonym.module.ts index d282c5dd9fe..3a8bcdacbd1 100644 --- a/apps/server/src/modules/pseudonym/pseudonym.module.ts +++ b/apps/server/src/modules/pseudonym/pseudonym.module.ts @@ -1,14 +1,13 @@ -import { forwardRef, Module } from '@nestjs/common'; -import { LegacyLogger } from '@src/core/logger'; import { LearnroomModule } from '@modules/learnroom'; -import { UserModule } from '@modules/user'; import { ToolModule } from '@modules/tool'; -import { AuthorizationModule } from '@modules/authorization'; +import { UserModule } from '@modules/user'; +import { forwardRef, Module } from '@nestjs/common'; +import { LegacyLogger } from '@src/core/logger'; import { ExternalToolPseudonymRepo, PseudonymsRepo } from './repo'; import { FeathersRosterService, PseudonymService } from './service'; @Module({ - imports: [UserModule, LearnroomModule, forwardRef(() => ToolModule), forwardRef(() => AuthorizationModule)], + imports: [UserModule, LearnroomModule, forwardRef(() => ToolModule)], providers: [PseudonymService, PseudonymsRepo, ExternalToolPseudonymRepo, LegacyLogger, FeathersRosterService], exports: [PseudonymService, FeathersRosterService], }) diff --git a/apps/server/src/modules/task/task.module.ts b/apps/server/src/modules/task/task.module.ts index 696d608d0a3..45a0fdb720a 100644 --- a/apps/server/src/modules/task/task.module.ts +++ b/apps/server/src/modules/task/task.module.ts @@ -1,12 +1,11 @@ -import { forwardRef, Module } from '@nestjs/common'; -import { CourseRepo, LessonRepo, SubmissionRepo, TaskRepo } from '@shared/repo'; -import { AuthorizationModule } from '@modules/authorization'; import { CopyHelperModule } from '@modules/copy-helper'; import { FilesStorageClientModule } from '@modules/files-storage-client'; +import { Module } from '@nestjs/common'; +import { CourseRepo, LessonRepo, SubmissionRepo, TaskRepo } from '@shared/repo'; import { SubmissionService, TaskCopyService, TaskService } from './service'; @Module({ - imports: [forwardRef(() => AuthorizationModule), FilesStorageClientModule, CopyHelperModule], + imports: [FilesStorageClientModule, CopyHelperModule], providers: [TaskService, TaskCopyService, SubmissionService, TaskRepo, LessonRepo, CourseRepo, SubmissionRepo], exports: [TaskService, TaskCopyService, SubmissionService], }) diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-id.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-id.strategy.spec.ts new file mode 100644 index 00000000000..d1865d231c2 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-id.strategy.spec.ts @@ -0,0 +1,56 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { contextExternalToolFactory, schoolExternalToolFactory } from '@shared/testing'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { AutoContextIdStrategy } from './auto-context-id.strategy'; + +describe(AutoContextIdStrategy.name, () => { + let module: TestingModule; + let strategy: AutoContextIdStrategy; + + beforeAll(async () => { + module = await Test.createTestingModule({ + providers: [AutoContextIdStrategy], + }).compile(); + + strategy = module.get(AutoContextIdStrategy); + }); + + afterAll(async () => { + await module.close(); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('getValue', () => { + const setup = () => { + const contextId = 'contextId'; + + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.buildWithId(); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.buildWithId({ + schoolToolRef: { + schoolToolId: schoolExternalTool.id as string, + }, + contextRef: { + id: contextId, + }, + }); + + return { + contextId, + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should return the context id', () => { + const { contextId, schoolExternalTool, contextExternalTool } = setup(); + + const result: string | undefined = strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toEqual(contextId); + }); + }); +}); diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-id.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-id.strategy.ts new file mode 100644 index 00000000000..a732d038522 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-id.strategy.ts @@ -0,0 +1,11 @@ +import { Injectable } from '@nestjs/common'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { AutoParameterStrategy } from './auto-parameter.strategy'; + +@Injectable() +export class AutoContextIdStrategy implements AutoParameterStrategy { + getValue(schoolExternalTool: SchoolExternalTool, contextExternalTool: ContextExternalTool): string | undefined { + return contextExternalTool.contextRef.id; + } +} diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-name.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-name.strategy.spec.ts new file mode 100644 index 00000000000..caa02d1c69b --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-name.strategy.spec.ts @@ -0,0 +1,207 @@ +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { ObjectId } from '@mikro-orm/mongodb'; +import { ColumnBoardService, ContentElementService } from '@modules/board'; +import { CourseService } from '@modules/learnroom'; +import { Test, TestingModule } from '@nestjs/testing'; +import { BoardExternalReferenceType, ColumnBoard, Course, ExternalToolElement } from '@shared/domain'; +import { + columnBoardFactory, + contextExternalToolFactory, + courseFactory, + externalToolElementFactory, + schoolExternalToolFactory, + setupEntities, +} from '@shared/testing'; +import { ToolContextType } from '../../../common/enum'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { ParameterTypeNotImplementedLoggableException } from '../../error'; +import { AutoContextNameStrategy } from './auto-context-name.strategy'; + +describe(AutoContextNameStrategy.name, () => { + let module: TestingModule; + let strategy: AutoContextNameStrategy; + + let courseService: DeepMocked; + let contentElementService: DeepMocked; + let columnBoardService: DeepMocked; + + beforeAll(async () => { + await setupEntities(); + + module = await Test.createTestingModule({ + providers: [ + AutoContextNameStrategy, + { + provide: CourseService, + useValue: createMock(), + }, + { + provide: ContentElementService, + useValue: createMock(), + }, + { + provide: ColumnBoardService, + useValue: createMock(), + }, + ], + }).compile(); + + strategy = module.get(AutoContextNameStrategy); + courseService = module.get(CourseService); + contentElementService = module.get(ContentElementService); + columnBoardService = module.get(ColumnBoardService); + }); + + afterAll(async () => { + await module.close(); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('getValue', () => { + describe('when the tool context is "course"', () => { + const setup = () => { + const courseId = new ObjectId().toHexString(); + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build(); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({ + contextRef: { + id: courseId, + type: ToolContextType.COURSE, + }, + }); + + const course: Course = courseFactory.buildWithId( + { + name: 'testName', + }, + courseId + ); + + courseService.findById.mockResolvedValue(course); + + return { + schoolExternalTool, + contextExternalTool, + course, + }; + }; + + it('should return the course name', async () => { + const { schoolExternalTool, contextExternalTool, course } = setup(); + + const result: string | undefined = await strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toEqual(course.name); + }); + }); + + describe('when the tool context is "board element" and the board context is "course"', () => { + const setup = () => { + const boardElementId = new ObjectId().toHexString(); + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build(); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({ + contextRef: { + id: boardElementId, + type: ToolContextType.BOARD_ELEMENT, + }, + }); + + const course: Course = courseFactory.buildWithId({ + name: 'testName', + }); + + const externalToolElement: ExternalToolElement = externalToolElementFactory.build(); + + const columnBoard: ColumnBoard = columnBoardFactory.build({ + context: { + id: course.id, + type: BoardExternalReferenceType.Course, + }, + }); + + courseService.findById.mockResolvedValue(course); + contentElementService.findById.mockResolvedValue(externalToolElement); + columnBoardService.findByDescendant.mockResolvedValue(columnBoard); + + return { + schoolExternalTool, + contextExternalTool, + course, + }; + }; + + it('should return the course name', async () => { + const { schoolExternalTool, contextExternalTool, course } = setup(); + + const result: string | undefined = await strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toEqual(course.name); + }); + }); + + describe('when the tool context is "board element" and the board context is unknown', () => { + const setup = () => { + const boardElementId = new ObjectId().toHexString(); + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build(); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({ + contextRef: { + id: boardElementId, + type: ToolContextType.BOARD_ELEMENT, + }, + }); + + const externalToolElement: ExternalToolElement = externalToolElementFactory.build(); + + const columnBoard: ColumnBoard = columnBoardFactory.build({ + context: { + id: new ObjectId().toHexString(), + type: 'unknown' as unknown as BoardExternalReferenceType, + }, + }); + + contentElementService.findById.mockResolvedValue(externalToolElement); + columnBoardService.findByDescendant.mockResolvedValue(columnBoard); + + return { + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should return undefined', async () => { + const { schoolExternalTool, contextExternalTool } = setup(); + + const result: string | undefined = await strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toBeUndefined(); + }); + }); + + describe('when a lookup for a context name is not implemented', () => { + const setup = () => { + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build(); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({ + contextRef: { + type: 'unknownContext' as unknown as ToolContextType, + }, + }); + + return { + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should throw a ParameterNotImplementedLoggableException', async () => { + const { schoolExternalTool, contextExternalTool } = setup(); + + await expect(strategy.getValue(schoolExternalTool, contextExternalTool)).rejects.toThrow( + ParameterTypeNotImplementedLoggableException + ); + }); + }); + }); +}); diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-name.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-name.strategy.ts new file mode 100644 index 00000000000..14d296d8b60 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-name.strategy.ts @@ -0,0 +1,61 @@ +import { ColumnBoardService, ContentElementService } from '@modules/board'; +import { CourseService } from '@modules/learnroom'; +import { Injectable } from '@nestjs/common'; +import { AnyContentElementDo, BoardExternalReferenceType, ColumnBoard, Course, EntityId } from '@shared/domain'; +import { CustomParameterType, ToolContextType } from '../../../common/enum'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { ParameterTypeNotImplementedLoggableException } from '../../error'; +import { AutoParameterStrategy } from './auto-parameter.strategy'; + +@Injectable() +export class AutoContextNameStrategy implements AutoParameterStrategy { + constructor( + private readonly courseService: CourseService, + private readonly contentElementService: ContentElementService, + private readonly columnBoardService: ColumnBoardService + ) {} + + async getValue( + schoolExternalTool: SchoolExternalTool, + contextExternalTool: ContextExternalTool + ): Promise { + switch (contextExternalTool.contextRef.type) { + case ToolContextType.COURSE: { + const courseValue: string = await this.getCourseValue(contextExternalTool.contextRef.id); + + return courseValue; + } + case ToolContextType.BOARD_ELEMENT: { + const boardValue: string | undefined = await this.getBoardValue(contextExternalTool.contextRef.id); + + return boardValue; + } + default: { + throw new ParameterTypeNotImplementedLoggableException( + `${CustomParameterType.AUTO_CONTEXTNAME}/${contextExternalTool.contextRef.type as string}` + ); + } + } + } + + private async getCourseValue(courseId: EntityId): Promise { + const course: Course = await this.courseService.findById(courseId); + + return course.name; + } + + private async getBoardValue(elementId: EntityId): Promise { + const element: AnyContentElementDo = await this.contentElementService.findById(elementId); + + const board: ColumnBoard = await this.columnBoardService.findByDescendant(element); + + if (board.context.type === BoardExternalReferenceType.Course) { + const courseName: string = await this.getCourseValue(board.context.id); + + return courseName; + } + + return undefined; + } +} diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-parameter.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-parameter.strategy.ts new file mode 100644 index 00000000000..5c5efbcc2b1 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-parameter.strategy.ts @@ -0,0 +1,9 @@ +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; + +export interface AutoParameterStrategy { + getValue( + schoolExternalTool: SchoolExternalTool, + contextExternalTool: ContextExternalTool + ): string | Promise | undefined; +} diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-id.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-id.strategy.spec.ts new file mode 100644 index 00000000000..2b184c51140 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-id.strategy.spec.ts @@ -0,0 +1,56 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { contextExternalToolFactory, schoolExternalToolFactory } from '@shared/testing'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { AutoSchoolIdStrategy } from './auto-school-id.strategy'; + +describe(AutoSchoolIdStrategy.name, () => { + let module: TestingModule; + let strategy: AutoSchoolIdStrategy; + + beforeAll(async () => { + module = await Test.createTestingModule({ + providers: [AutoSchoolIdStrategy], + }).compile(); + + strategy = module.get(AutoSchoolIdStrategy); + }); + + afterAll(async () => { + await module.close(); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('getValue', () => { + const setup = () => { + const schoolId = 'schoolId'; + + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.buildWithId({ + schoolId, + }); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.buildWithId({ + schoolToolRef: { + schoolToolId: schoolExternalTool.id as string, + schoolId, + }, + }); + + return { + schoolId, + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should return the context id', () => { + const { schoolId, schoolExternalTool, contextExternalTool } = setup(); + + const result: string | undefined = strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toEqual(schoolId); + }); + }); +}); diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-id.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-id.strategy.ts new file mode 100644 index 00000000000..faad4be07d9 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-id.strategy.ts @@ -0,0 +1,15 @@ +import { Injectable } from '@nestjs/common'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { AutoParameterStrategy } from './auto-parameter.strategy'; + +@Injectable() +export class AutoSchoolIdStrategy implements AutoParameterStrategy { + getValue( + schoolExternalTool: SchoolExternalTool, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + contextExternalTool: ContextExternalTool + ): string | undefined { + return schoolExternalTool.schoolId; + } +} diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-number.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-number.strategy.spec.ts new file mode 100644 index 00000000000..91a01bbdd39 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-number.strategy.spec.ts @@ -0,0 +1,107 @@ +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { LegacySchoolService } from '@modules/legacy-school'; +import { Test, TestingModule } from '@nestjs/testing'; +import { LegacySchoolDo } from '@shared/domain'; +import { contextExternalToolFactory, legacySchoolDoFactory, schoolExternalToolFactory } from '@shared/testing'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { AutoSchoolNumberStrategy } from './auto-school-number.strategy'; + +describe(AutoSchoolNumberStrategy.name, () => { + let module: TestingModule; + let strategy: AutoSchoolNumberStrategy; + + let schoolService: DeepMocked; + + beforeAll(async () => { + module = await Test.createTestingModule({ + providers: [ + AutoSchoolNumberStrategy, + { + provide: LegacySchoolService, + useValue: createMock(), + }, + ], + }).compile(); + + strategy = module.get(AutoSchoolNumberStrategy); + schoolService = module.get(LegacySchoolService); + }); + + afterAll(async () => { + await module.close(); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('getValue', () => { + describe('when the school has a school number', () => { + const setup = () => { + const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId({ + officialSchoolNumber: 'officialSchoolNumber', + }); + + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.buildWithId({ + schoolId: school.id as string, + }); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.buildWithId({ + schoolToolRef: { + schoolToolId: schoolExternalTool.id as string, + schoolId: school.id, + }, + }); + + schoolService.getSchoolById.mockResolvedValue(school); + + return { + school, + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should return the school number', async () => { + const { school, schoolExternalTool, contextExternalTool } = setup(); + + const result: string | undefined = await strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toEqual(school.officialSchoolNumber); + }); + }); + + describe('when the school does not have a school number', () => { + const setup = () => { + const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId({ + officialSchoolNumber: undefined, + }); + + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.buildWithId({ + schoolId: school.id as string, + }); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.buildWithId({ + schoolToolRef: { + schoolToolId: schoolExternalTool.id as string, + schoolId: school.id, + }, + }); + + schoolService.getSchoolById.mockResolvedValue(school); + + return { + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should return undefined', async () => { + const { schoolExternalTool, contextExternalTool } = setup(); + + const result: string | undefined = await strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toBeUndefined(); + }); + }); + }); +}); diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-number.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-number.strategy.ts new file mode 100644 index 00000000000..7d9ad9dad65 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-number.strategy.ts @@ -0,0 +1,21 @@ +import { LegacySchoolService } from '@modules/legacy-school'; +import { Injectable } from '@nestjs/common'; +import { LegacySchoolDo } from '@shared/domain'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { AutoParameterStrategy } from './auto-parameter.strategy'; + +@Injectable() +export class AutoSchoolNumberStrategy implements AutoParameterStrategy { + constructor(private readonly schoolService: LegacySchoolService) {} + + async getValue( + schoolExternalTool: SchoolExternalTool, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + contextExternalTool: ContextExternalTool + ): Promise { + const school: LegacySchoolDo = await this.schoolService.getSchoolById(schoolExternalTool.schoolId); + + return school.officialSchoolNumber; + } +} diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/index.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/index.ts new file mode 100644 index 00000000000..619a0a6296c --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/index.ts @@ -0,0 +1,5 @@ +export * from './auto-parameter.strategy'; +export * from './auto-school-id.strategy'; +export * from './auto-context-id.strategy'; +export * from './auto-context-name.strategy'; +export * from './auto-school-number.strategy'; diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/abstract-launch.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.spec.ts similarity index 67% rename from apps/server/src/modules/tool/tool-launch/service/strategy/abstract-launch.strategy.spec.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.spec.ts index 7ee237b3e93..04868663671 100644 --- a/apps/server/src/modules/tool/tool-launch/service/strategy/abstract-launch.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.spec.ts @@ -2,25 +2,15 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; import { Injectable } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; -import { Course, EntityId, LegacySchoolDo } from '@shared/domain'; +import { EntityId } from '@shared/domain'; import { contextExternalToolFactory, - courseFactory, customParameterFactory, externalToolFactory, - legacySchoolDoFactory, schoolExternalToolFactory, - setupEntities, } from '@shared/testing'; -import { CourseService } from '@modules/learnroom/service'; -import { LegacySchoolService } from '@modules/legacy-school'; import { CustomParameterEntry } from '../../../common/domain'; -import { - CustomParameterLocation, - CustomParameterScope, - CustomParameterType, - ToolContextType, -} from '../../../common/enum'; +import { CustomParameterLocation, CustomParameterScope, CustomParameterType } from '../../../common/enum'; import { ContextExternalTool } from '../../../context-external-tool/domain'; import { ExternalTool } from '../../../external-tool/domain'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; @@ -33,6 +23,12 @@ import { ToolLaunchDataType, ToolLaunchRequest, } from '../../types'; +import { + AutoContextIdStrategy, + AutoContextNameStrategy, + AutoSchoolIdStrategy, + AutoSchoolNumberStrategy, +} from '../auto-parameter-strategy'; import { AbstractLaunchStrategy } from './abstract-launch.strategy'; import { IToolLaunchParams } from './tool-launch-params.interface'; @@ -69,33 +65,44 @@ class TestLaunchStrategy extends AbstractLaunchStrategy { } } -describe('AbstractLaunchStrategy', () => { +describe(AbstractLaunchStrategy.name, () => { let module: TestingModule; - let launchStrategy: TestLaunchStrategy; + let strategy: TestLaunchStrategy; - let schoolService: DeepMocked; - let courseService: DeepMocked; + let autoSchoolIdStrategy: DeepMocked; + let autoSchoolNumberStrategy: DeepMocked; + let autoContextIdStrategy: DeepMocked; + let autoContextNameStrategy: DeepMocked; beforeAll(async () => { - await setupEntities(); - module = await Test.createTestingModule({ providers: [ TestLaunchStrategy, { - provide: LegacySchoolService, - useValue: createMock(), + provide: AutoSchoolIdStrategy, + useValue: createMock(), + }, + { + provide: AutoSchoolNumberStrategy, + useValue: createMock(), }, { - provide: CourseService, - useValue: createMock(), + provide: AutoContextIdStrategy, + useValue: createMock(), + }, + { + provide: AutoContextNameStrategy, + useValue: createMock(), }, ], }).compile(); - launchStrategy = module.get(TestLaunchStrategy); - schoolService = module.get(LegacySchoolService); - courseService = module.get(CourseService); + strategy = module.get(TestLaunchStrategy); + + autoSchoolIdStrategy = module.get(AutoSchoolIdStrategy); + autoSchoolNumberStrategy = module.get(AutoSchoolNumberStrategy); + autoContextIdStrategy = module.get(AutoContextIdStrategy); + autoContextNameStrategy = module.get(AutoContextNameStrategy); }); afterAll(async () => { @@ -106,6 +113,7 @@ describe('AbstractLaunchStrategy', () => { describe('when parameters of every type are defined', () => { const setup = () => { const schoolId: string = new ObjectId().toHexString(); + const mockedAutoValue = 'mockedAutoValue'; // External Tool const globalCustomParameter = customParameterFactory.build({ @@ -139,6 +147,18 @@ describe('AbstractLaunchStrategy', () => { name: 'autoSchoolNumberParam', type: CustomParameterType.AUTO_SCHOOLNUMBER, }); + const autoContextIdCustomParameter = customParameterFactory.build({ + scope: CustomParameterScope.GLOBAL, + location: CustomParameterLocation.BODY, + name: 'autoSchoolNumberParam', + type: CustomParameterType.AUTO_CONTEXTID, + }); + const autoContextNameCustomParameter = customParameterFactory.build({ + scope: CustomParameterScope.GLOBAL, + location: CustomParameterLocation.BODY, + name: 'autoSchoolNumberParam', + type: CustomParameterType.AUTO_CONTEXTNAME, + }); const externalTool: ExternalTool = externalToolFactory.build({ parameters: [ @@ -147,6 +167,8 @@ describe('AbstractLaunchStrategy', () => { contextCustomParameter, autoSchoolIdCustomParameter, autoSchoolNumberCustomParameter, + autoContextIdCustomParameter, + autoContextNameCustomParameter, ], }); @@ -169,16 +191,6 @@ describe('AbstractLaunchStrategy', () => { parameters: [contextParameterEntry], }); - // Other - const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId( - { - officialSchoolNumber: '1234', - }, - schoolId - ); - - schoolService.getSchoolById.mockResolvedValue(school); - const sortFn = (a: PropertyData, b: PropertyData) => { if (a.name < b.name) { return -1; @@ -189,17 +201,24 @@ describe('AbstractLaunchStrategy', () => { return 0; }; + autoSchoolIdStrategy.getValue.mockReturnValueOnce(mockedAutoValue); + autoSchoolNumberStrategy.getValue.mockResolvedValueOnce(mockedAutoValue); + autoContextIdStrategy.getValue.mockReturnValueOnce(mockedAutoValue); + autoContextNameStrategy.getValue.mockResolvedValueOnce(mockedAutoValue); + return { globalCustomParameter, schoolCustomParameter, autoSchoolIdCustomParameter, autoSchoolNumberCustomParameter, + autoContextIdCustomParameter, + autoContextNameCustomParameter, schoolParameterEntry, contextParameterEntry, externalTool, schoolExternalTool, contextExternalTool, - school, + mockedAutoValue, sortFn, }; }; @@ -211,15 +230,17 @@ describe('AbstractLaunchStrategy', () => { contextParameterEntry, autoSchoolIdCustomParameter, autoSchoolNumberCustomParameter, + autoContextIdCustomParameter, + autoContextNameCustomParameter, schoolParameterEntry, externalTool, schoolExternalTool, contextExternalTool, - school, + mockedAutoValue, sortFn, } = setup(); - const result: ToolLaunchData = await launchStrategy.createLaunchData('userId', { + const result: ToolLaunchData = await strategy.createLaunchData('userId', { externalTool, schoolExternalTool, contextExternalTool, @@ -248,135 +269,22 @@ describe('AbstractLaunchStrategy', () => { }, { name: autoSchoolIdCustomParameter.name, - value: school.id as string, + value: mockedAutoValue, location: PropertyLocation.BODY, }, { name: autoSchoolNumberCustomParameter.name, - value: school.officialSchoolNumber as string, + value: mockedAutoValue, location: PropertyLocation.BODY, }, { - name: concreteConfigParameter.name, - value: concreteConfigParameter.value, - location: concreteConfigParameter.location, - }, - ].sort(sortFn), - }); - }); - }); - - describe('when launching with context name parameter for the context "course"', () => { - const setup = () => { - const autoCourseNameCustomParameter = customParameterFactory.build({ - scope: CustomParameterScope.GLOBAL, - location: CustomParameterLocation.BODY, - name: 'autoCourseNameParam', - type: CustomParameterType.AUTO_CONTEXTNAME, - }); - - const externalTool: ExternalTool = externalToolFactory.build({ - parameters: [autoCourseNameCustomParameter], - }); - - const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build(); - - const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({ - contextRef: { - type: ToolContextType.COURSE, - }, - }); - - const course: Course = courseFactory.buildWithId( - { - name: 'testName', - }, - contextExternalTool.contextRef.id - ); - - courseService.findById.mockResolvedValue(course); - - return { - autoCourseNameCustomParameter, - externalTool, - schoolExternalTool, - contextExternalTool, - course, - }; - }; - - it('should return ToolLaunchData with the course name as parameter value', async () => { - const { externalTool, schoolExternalTool, contextExternalTool, autoCourseNameCustomParameter, course } = - setup(); - - const result: ToolLaunchData = await launchStrategy.createLaunchData('userId', { - externalTool, - schoolExternalTool, - contextExternalTool, - }); - - expect(result).toEqual({ - baseUrl: externalTool.config.baseUrl, - type: ToolLaunchDataType.BASIC, - openNewTab: false, - properties: [ - { - name: autoCourseNameCustomParameter.name, - value: course.name, + name: autoContextIdCustomParameter.name, + value: mockedAutoValue, location: PropertyLocation.BODY, }, { - name: concreteConfigParameter.name, - value: concreteConfigParameter.value, - location: concreteConfigParameter.location, - }, - ], - }); - }); - }); - - describe('when launching with context id parameter', () => { - const setup = () => { - const autoContextIdCustomParameter = customParameterFactory.build({ - scope: CustomParameterScope.GLOBAL, - location: CustomParameterLocation.BODY, - name: 'autoContextIdParam', - type: CustomParameterType.AUTO_CONTEXTID, - }); - - const externalTool: ExternalTool = externalToolFactory.build({ - parameters: [autoContextIdCustomParameter], - }); - - const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build(); - - const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build(); - - return { - autoContextIdCustomParameter, - externalTool, - schoolExternalTool, - contextExternalTool, - }; - }; - - it('should return ToolLaunchData with the context id as parameter value', async () => { - const { externalTool, schoolExternalTool, contextExternalTool, autoContextIdCustomParameter } = setup(); - - const result: ToolLaunchData = await launchStrategy.createLaunchData('userId', { - externalTool, - schoolExternalTool, - contextExternalTool, - }); - - expect(result).toEqual({ - baseUrl: externalTool.config.baseUrl, - type: ToolLaunchDataType.BASIC, - openNewTab: false, - properties: [ - { - name: autoContextIdCustomParameter.name, - value: contextExternalTool.contextRef.id, + name: autoContextNameCustomParameter.name, + value: mockedAutoValue, location: PropertyLocation.BODY, }, { @@ -384,7 +292,7 @@ describe('AbstractLaunchStrategy', () => { value: concreteConfigParameter.value, location: concreteConfigParameter.location, }, - ], + ].sort(sortFn), }); }); }); @@ -413,7 +321,7 @@ describe('AbstractLaunchStrategy', () => { it('should return a ToolLaunchData with no custom parameters', async () => { const { externalTool, schoolExternalTool, contextExternalTool } = setup(); - const result: ToolLaunchData = await launchStrategy.createLaunchData('userId', { + const result: ToolLaunchData = await strategy.createLaunchData('userId', { externalTool, schoolExternalTool, contextExternalTool, @@ -454,11 +362,7 @@ describe('AbstractLaunchStrategy', () => { parameters: [], }); - const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId({ - officialSchoolNumber: undefined, - }); - - schoolService.getSchoolById.mockResolvedValue(school); + autoSchoolNumberStrategy.getValue.mockResolvedValue(undefined); return { externalTool, @@ -471,7 +375,7 @@ describe('AbstractLaunchStrategy', () => { const { externalTool, schoolExternalTool, contextExternalTool } = setup(); const func = async () => - launchStrategy.createLaunchData('userId', { + strategy.createLaunchData('userId', { externalTool, schoolExternalTool, contextExternalTool, @@ -512,52 +416,7 @@ describe('AbstractLaunchStrategy', () => { const { externalTool, schoolExternalTool, contextExternalTool } = setup(); const func = async () => - launchStrategy.createLaunchData('userId', { - externalTool, - schoolExternalTool, - contextExternalTool, - }); - - await expect(func).rejects.toThrow(ParameterTypeNotImplementedLoggableException); - }); - }); - - describe('when a lookup for a context name is not implemented', () => { - const setup = () => { - const customParameterWithUnknownType = customParameterFactory.build({ - scope: CustomParameterScope.GLOBAL, - location: CustomParameterLocation.BODY, - name: 'autoContextNameParam', - type: CustomParameterType.AUTO_CONTEXTNAME, - }); - const externalTool: ExternalTool = externalToolFactory.build({ - parameters: [customParameterWithUnknownType], - }); - - const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build({ - parameters: [], - }); - - const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({ - contextRef: { - id: new ObjectId().toHexString(), - type: 'unknownContext' as unknown as ToolContextType, - }, - parameters: [], - }); - - return { - externalTool, - schoolExternalTool, - contextExternalTool, - }; - }; - - it('should throw a ParameterNotImplementedLoggableException', async () => { - const { externalTool, schoolExternalTool, contextExternalTool } = setup(); - - const func = async () => - launchStrategy.createLaunchData('userId', { + strategy.createLaunchData('userId', { externalTool, schoolExternalTool, contextExternalTool, @@ -597,7 +456,7 @@ describe('AbstractLaunchStrategy', () => { }); toolLaunchDataDO.properties = [propertyData1, propertyData2]; - const result: ToolLaunchRequest = launchStrategy.createLaunchRequest(toolLaunchDataDO); + const result: ToolLaunchRequest = strategy.createLaunchRequest(toolLaunchDataDO); expect(result).toEqual({ method: LaunchRequestMethod.GET, @@ -622,7 +481,7 @@ describe('AbstractLaunchStrategy', () => { }); toolLaunchDataDO.properties = [bodyProperty1, bodyProperty2]; - const result: ToolLaunchRequest = launchStrategy.createLaunchRequest(toolLaunchDataDO); + const result: ToolLaunchRequest = strategy.createLaunchRequest(toolLaunchDataDO); expect(result.payload).toEqual(expectedPayload); }); @@ -642,7 +501,7 @@ describe('AbstractLaunchStrategy', () => { }); toolLaunchDataDO.properties = [pathProperty, queryProperty]; - const result: ToolLaunchRequest = launchStrategy.createLaunchRequest(toolLaunchDataDO); + const result: ToolLaunchRequest = strategy.createLaunchRequest(toolLaunchDataDO); expect(result.url).toEqual( `https://www.basic-baseurl.com/pre/${pathProperty.value}/post?${queryProperty.name}=${queryProperty.value}` diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/abstract-launch.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts similarity index 81% rename from apps/server/src/modules/tool/tool-launch/service/strategy/abstract-launch.strategy.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts index 63ba0680734..ce0f07c5731 100644 --- a/apps/server/src/modules/tool/tool-launch/service/strategy/abstract-launch.strategy.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts @@ -1,27 +1,41 @@ import { Injectable } from '@nestjs/common'; -import { Course, EntityId, LegacySchoolDo } from '@shared/domain'; -import { CourseService } from '@modules/learnroom/service'; -import { LegacySchoolService } from '@modules/legacy-school'; +import { EntityId } from '@shared/domain'; import { URLSearchParams } from 'url'; import { CustomParameter, CustomParameterEntry } from '../../../common/domain'; -import { - CustomParameterLocation, - CustomParameterScope, - CustomParameterType, - ToolContextType, -} from '../../../common/enum'; +import { CustomParameterLocation, CustomParameterScope, CustomParameterType } from '../../../common/enum'; import { ContextExternalTool } from '../../../context-external-tool/domain'; import { ExternalTool } from '../../../external-tool/domain'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; import { MissingToolParameterValueLoggableException, ParameterTypeNotImplementedLoggableException } from '../../error'; import { ToolLaunchMapper } from '../../mapper'; import { LaunchRequestMethod, PropertyData, PropertyLocation, ToolLaunchData, ToolLaunchRequest } from '../../types'; +import { + AutoContextIdStrategy, + AutoContextNameStrategy, + AutoParameterStrategy, + AutoSchoolIdStrategy, + AutoSchoolNumberStrategy, +} from '../auto-parameter-strategy'; import { IToolLaunchParams } from './tool-launch-params.interface'; import { IToolLaunchStrategy } from './tool-launch-strategy.interface'; @Injectable() export abstract class AbstractLaunchStrategy implements IToolLaunchStrategy { - constructor(private readonly schoolService: LegacySchoolService, private readonly courseService: CourseService) {} + private readonly autoParameterStrategyMap: Map; + + constructor( + autoSchoolIdStrategy: AutoSchoolIdStrategy, + autoSchoolNumberStrategy: AutoSchoolNumberStrategy, + autoContextIdStrategy: AutoContextIdStrategy, + autoContextNameStrategy: AutoContextNameStrategy + ) { + this.autoParameterStrategyMap = new Map([ + [CustomParameterType.AUTO_SCHOOLID, autoSchoolIdStrategy], + [CustomParameterType.AUTO_SCHOOLNUMBER, autoSchoolNumberStrategy], + [CustomParameterType.AUTO_CONTEXTID, autoContextIdStrategy], + [CustomParameterType.AUTO_CONTEXTNAME, autoContextNameStrategy], + ]); + } public async createLaunchData(userId: EntityId, data: IToolLaunchParams): Promise { const launchData: ToolLaunchData = this.buildToolLaunchDataFromExternalTool(data.externalTool); @@ -207,43 +221,29 @@ export abstract class AbstractLaunchStrategy implements IToolLaunchStrategy { schoolExternalTool: SchoolExternalTool, contextExternalTool: ContextExternalTool ): Promise { - switch (customParameter.type) { - case CustomParameterType.AUTO_SCHOOLID: { - return schoolExternalTool.schoolId; - } - case CustomParameterType.AUTO_CONTEXTID: { - return contextExternalTool.contextRef.id; - } - case CustomParameterType.AUTO_CONTEXTNAME: { - switch (contextExternalTool.contextRef.type) { - case ToolContextType.COURSE: { - const course: Course = await this.courseService.findById(contextExternalTool.contextRef.id); - - return course.name; - } - default: { - throw new ParameterTypeNotImplementedLoggableException( - `${customParameter.type}/${contextExternalTool.contextRef.type as string}` - ); - } - } - } - case CustomParameterType.AUTO_SCHOOLNUMBER: { - const school: LegacySchoolDo = await this.schoolService.getSchoolById(schoolExternalTool.schoolId); + if ( + customParameter.type === CustomParameterType.BOOLEAN || + customParameter.type === CustomParameterType.NUMBER || + customParameter.type === CustomParameterType.STRING + ) { + return customParameter.scope === CustomParameterScope.GLOBAL + ? customParameter.default + : matchingParameterEntry?.value; + } - return school.officialSchoolNumber; - } - case CustomParameterType.BOOLEAN: - case CustomParameterType.NUMBER: - case CustomParameterType.STRING: { - return customParameter.scope === CustomParameterScope.GLOBAL - ? customParameter.default - : matchingParameterEntry?.value; - } - default: { - throw new ParameterTypeNotImplementedLoggableException(customParameter.type); - } + const autoParameterStrategy: AutoParameterStrategy | undefined = this.autoParameterStrategyMap.get( + customParameter.type + ); + if (autoParameterStrategy) { + const autoValue: string | undefined = await autoParameterStrategy.getValue( + schoolExternalTool, + contextExternalTool + ); + + return autoValue; } + + throw new ParameterTypeNotImplementedLoggableException(customParameter.type); } private addProperty( diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/basic-tool-launch.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.spec.ts similarity index 90% rename from apps/server/src/modules/tool/tool-launch/service/strategy/basic-tool-launch.strategy.spec.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.spec.ts index 3bb95b97755..db80f78498f 100644 --- a/apps/server/src/modules/tool/tool-launch/service/strategy/basic-tool-launch.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.spec.ts @@ -1,12 +1,16 @@ import { createMock } from '@golevelup/ts-jest'; import { Test, TestingModule } from '@nestjs/testing'; import { contextExternalToolFactory, externalToolFactory, schoolExternalToolFactory } from '@shared/testing'; -import { CourseService } from '@modules/learnroom/service'; -import { LegacySchoolService } from '@modules/legacy-school'; import { ContextExternalTool } from '../../../context-external-tool/domain'; import { ExternalTool } from '../../../external-tool/domain'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; import { LaunchRequestMethod, PropertyData, PropertyLocation } from '../../types'; +import { + AutoContextIdStrategy, + AutoContextNameStrategy, + AutoSchoolIdStrategy, + AutoSchoolNumberStrategy, +} from '../auto-parameter-strategy'; import { BasicToolLaunchStrategy } from './basic-tool-launch.strategy'; import { IToolLaunchParams } from './tool-launch-params.interface'; @@ -19,12 +23,20 @@ describe('BasicToolLaunchStrategy', () => { providers: [ BasicToolLaunchStrategy, { - provide: LegacySchoolService, - useValue: createMock(), + provide: AutoSchoolIdStrategy, + useValue: createMock(), }, { - provide: CourseService, - useValue: createMock(), + provide: AutoSchoolNumberStrategy, + useValue: createMock(), + }, + { + provide: AutoContextIdStrategy, + useValue: createMock(), + }, + { + provide: AutoContextNameStrategy, + useValue: createMock(), }, ], }).compile(); diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/basic-tool-launch.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.ts similarity index 100% rename from apps/server/src/modules/tool/tool-launch/service/strategy/basic-tool-launch.strategy.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.ts diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/index.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/index.ts similarity index 100% rename from apps/server/src/modules/tool/tool-launch/service/strategy/index.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/index.ts diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/lti11-tool-launch.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.spec.ts similarity index 96% rename from apps/server/src/modules/tool/tool-launch/service/strategy/lti11-tool-launch.strategy.spec.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.spec.ts index 5113ff3cc76..ee2932cd535 100644 --- a/apps/server/src/modules/tool/tool-launch/service/strategy/lti11-tool-launch.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.spec.ts @@ -1,4 +1,6 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { PseudonymService } from '@modules/pseudonym/service'; +import { UserService } from '@modules/user'; import { InternalServerErrorException, UnprocessableEntityException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { Pseudonym, RoleName, UserDO } from '@shared/domain'; @@ -9,10 +11,6 @@ import { userDoFactory, } from '@shared/testing'; import { pseudonymFactory } from '@shared/testing/factory/domainobject/pseudonym.factory'; -import { CourseService } from '@modules/learnroom/service'; -import { LegacySchoolService } from '@modules/legacy-school'; -import { PseudonymService } from '@modules/pseudonym/service'; -import { UserService } from '@modules/user'; import { ObjectId } from 'bson'; import { Authorization } from 'oauth-1.0a'; import { LtiMessageType, LtiPrivacyPermission, LtiRole, ToolContextType } from '../../../common/enum'; @@ -20,6 +18,12 @@ import { ContextExternalTool } from '../../../context-external-tool/domain'; import { ExternalTool } from '../../../external-tool/domain'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; import { LaunchRequestMethod, PropertyData, PropertyLocation } from '../../types'; +import { + AutoContextIdStrategy, + AutoContextNameStrategy, + AutoSchoolIdStrategy, + AutoSchoolNumberStrategy, +} from '../auto-parameter-strategy'; import { Lti11EncryptionService } from '../lti11-encryption.service'; import { Lti11ToolLaunchStrategy } from './lti11-tool-launch.strategy'; import { IToolLaunchParams } from './tool-launch-params.interface'; @@ -32,7 +36,7 @@ describe('Lti11ToolLaunchStrategy', () => { let pseudonymService: DeepMocked; let lti11EncryptionService: DeepMocked; - beforeEach(async () => { + beforeAll(async () => { module = await Test.createTestingModule({ providers: [ Lti11ToolLaunchStrategy, @@ -49,12 +53,20 @@ describe('Lti11ToolLaunchStrategy', () => { useValue: createMock(), }, { - provide: LegacySchoolService, - useValue: createMock(), + provide: AutoSchoolIdStrategy, + useValue: createMock(), + }, + { + provide: AutoSchoolNumberStrategy, + useValue: createMock(), + }, + { + provide: AutoContextIdStrategy, + useValue: createMock(), }, { - provide: CourseService, - useValue: createMock(), + provide: AutoContextNameStrategy, + useValue: createMock(), }, ], }).compile(); diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/lti11-tool-launch.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts similarity index 92% rename from apps/server/src/modules/tool/tool-launch/service/strategy/lti11-tool-launch.strategy.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts index 09d04e388f3..09516395234 100644 --- a/apps/server/src/modules/tool/tool-launch/service/strategy/lti11-tool-launch.strategy.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts @@ -1,15 +1,19 @@ +import { PseudonymService } from '@modules/pseudonym/service'; +import { UserService } from '@modules/user'; import { Injectable, InternalServerErrorException, UnprocessableEntityException } from '@nestjs/common'; import { EntityId, LtiPrivacyPermission, Pseudonym, RoleName, UserDO } from '@shared/domain'; import { RoleReference } from '@shared/domain/domainobject'; -import { CourseService } from '@modules/learnroom/service'; -import { LegacySchoolService } from '@modules/legacy-school'; -import { PseudonymService } from '@modules/pseudonym/service'; -import { UserService } from '@modules/user'; import { Authorization } from 'oauth-1.0a'; import { LtiRole } from '../../../common/enum'; import { ExternalTool } from '../../../external-tool/domain'; import { LtiRoleMapper } from '../../mapper'; import { AuthenticationValues, LaunchRequestMethod, PropertyData, PropertyLocation } from '../../types'; +import { + AutoContextIdStrategy, + AutoContextNameStrategy, + AutoSchoolIdStrategy, + AutoSchoolNumberStrategy, +} from '../auto-parameter-strategy'; import { Lti11EncryptionService } from '../lti11-encryption.service'; import { AbstractLaunchStrategy } from './abstract-launch.strategy'; import { IToolLaunchParams } from './tool-launch-params.interface'; @@ -20,10 +24,12 @@ export class Lti11ToolLaunchStrategy extends AbstractLaunchStrategy { private readonly userService: UserService, private readonly pseudonymService: PseudonymService, private readonly lti11EncryptionService: Lti11EncryptionService, - schoolService: LegacySchoolService, - courseService: CourseService + autoSchoolIdStrategy: AutoSchoolIdStrategy, + autoSchoolNumberStrategy: AutoSchoolNumberStrategy, + autoContextIdStrategy: AutoContextIdStrategy, + autoContextNameStrategy: AutoContextNameStrategy ) { - super(schoolService, courseService); + super(autoSchoolIdStrategy, autoSchoolNumberStrategy, autoContextIdStrategy, autoContextNameStrategy); } // eslint-disable-next-line @typescript-eslint/no-unused-vars diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/oauth2-tool-launch.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.spec.ts similarity index 80% rename from apps/server/src/modules/tool/tool-launch/service/strategy/oauth2-tool-launch.strategy.spec.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.spec.ts index bd97fafde71..8f81a1d0eb2 100644 --- a/apps/server/src/modules/tool/tool-launch/service/strategy/oauth2-tool-launch.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.spec.ts @@ -1,12 +1,16 @@ import { createMock } from '@golevelup/ts-jest'; import { Test, TestingModule } from '@nestjs/testing'; import { contextExternalToolFactory, externalToolFactory, schoolExternalToolFactory } from '@shared/testing'; -import { CourseService } from '@modules/learnroom/service'; -import { LegacySchoolService } from '@modules/legacy-school'; import { ContextExternalTool } from '../../../context-external-tool/domain'; import { ExternalTool } from '../../../external-tool/domain'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; import { LaunchRequestMethod, PropertyData } from '../../types'; +import { + AutoContextIdStrategy, + AutoContextNameStrategy, + AutoSchoolIdStrategy, + AutoSchoolNumberStrategy, +} from '../auto-parameter-strategy'; import { OAuth2ToolLaunchStrategy } from './oauth2-tool-launch.strategy'; import { IToolLaunchParams } from './tool-launch-params.interface'; @@ -14,17 +18,25 @@ describe('OAuth2ToolLaunchStrategy', () => { let module: TestingModule; let strategy: OAuth2ToolLaunchStrategy; - beforeEach(async () => { + beforeAll(async () => { module = await Test.createTestingModule({ providers: [ OAuth2ToolLaunchStrategy, { - provide: LegacySchoolService, - useValue: createMock(), + provide: AutoSchoolIdStrategy, + useValue: createMock(), }, { - provide: CourseService, - useValue: createMock(), + provide: AutoSchoolNumberStrategy, + useValue: createMock(), + }, + { + provide: AutoContextIdStrategy, + useValue: createMock(), + }, + { + provide: AutoContextNameStrategy, + useValue: createMock(), }, ], }).compile(); diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/oauth2-tool-launch.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.ts similarity index 100% rename from apps/server/src/modules/tool/tool-launch/service/strategy/oauth2-tool-launch.strategy.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.ts diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/tool-launch-params.interface.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/tool-launch-params.interface.ts similarity index 100% rename from apps/server/src/modules/tool/tool-launch/service/strategy/tool-launch-params.interface.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/tool-launch-params.interface.ts index a6d1b75d9cf..24e368476f5 100644 --- a/apps/server/src/modules/tool/tool-launch/service/strategy/tool-launch-params.interface.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/tool-launch-params.interface.ts @@ -1,6 +1,6 @@ +import { ContextExternalTool } from '../../../context-external-tool/domain'; import { ExternalTool } from '../../../external-tool/domain'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; -import { ContextExternalTool } from '../../../context-external-tool/domain'; export interface IToolLaunchParams { externalTool: ExternalTool; diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/tool-launch-strategy.interface.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/tool-launch-strategy.interface.ts similarity index 100% rename from apps/server/src/modules/tool/tool-launch/service/strategy/tool-launch-strategy.interface.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/tool-launch-strategy.interface.ts diff --git a/apps/server/src/modules/tool/tool-launch/service/tool-launch.service.spec.ts b/apps/server/src/modules/tool/tool-launch/service/tool-launch.service.spec.ts index 3330b0c9f0e..e4f9eaa6113 100644 --- a/apps/server/src/modules/tool/tool-launch/service/tool-launch.service.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/tool-launch.service.spec.ts @@ -21,7 +21,7 @@ import { IToolLaunchParams, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrategy, -} from './strategy'; +} from './launch-strategy'; import { ToolLaunchService } from './tool-launch.service'; describe('ToolLaunchService', () => { diff --git a/apps/server/src/modules/tool/tool-launch/service/tool-launch.service.ts b/apps/server/src/modules/tool/tool-launch/service/tool-launch.service.ts index 46d2efdeb70..abb5598796f 100644 --- a/apps/server/src/modules/tool/tool-launch/service/tool-launch.service.ts +++ b/apps/server/src/modules/tool/tool-launch/service/tool-launch.service.ts @@ -15,7 +15,7 @@ import { IToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrategy, -} from './strategy'; +} from './launch-strategy'; @Injectable() export class ToolLaunchService { diff --git a/apps/server/src/modules/tool/tool-launch/tool-launch.module.ts b/apps/server/src/modules/tool/tool-launch/tool-launch.module.ts index 4ae6a3a38a5..b8ff7623586 100644 --- a/apps/server/src/modules/tool/tool-launch/tool-launch.module.ts +++ b/apps/server/src/modules/tool/tool-launch/tool-launch.module.ts @@ -1,14 +1,21 @@ -import { Module, forwardRef } from '@nestjs/common'; +import { BoardModule } from '@modules/board'; import { LearnroomModule } from '@modules/learnroom'; import { LegacySchoolModule } from '@modules/legacy-school'; import { PseudonymModule } from '@modules/pseudonym'; import { UserModule } from '@modules/user'; +import { forwardRef, Module } from '@nestjs/common'; import { CommonToolModule } from '../common'; import { ContextExternalToolModule } from '../context-external-tool'; import { ExternalToolModule } from '../external-tool'; import { SchoolExternalToolModule } from '../school-external-tool'; import { Lti11EncryptionService, ToolLaunchService } from './service'; -import { BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrategy } from './service/strategy'; +import { + AutoContextIdStrategy, + AutoContextNameStrategy, + AutoSchoolIdStrategy, + AutoSchoolNumberStrategy, +} from './service/auto-parameter-strategy'; +import { BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrategy } from './service/launch-strategy'; @Module({ imports: [ @@ -20,13 +27,18 @@ import { BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrat UserModule, forwardRef(() => PseudonymModule), // i do not like this solution, the root problem is on other place but not detectable for me LearnroomModule, + BoardModule, ], providers: [ ToolLaunchService, + Lti11EncryptionService, BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrategy, - Lti11EncryptionService, + AutoContextIdStrategy, + AutoContextNameStrategy, + AutoSchoolIdStrategy, + AutoSchoolNumberStrategy, ], exports: [ToolLaunchService], }) diff --git a/apps/server/src/shared/domain/interface/permission.enum.ts b/apps/server/src/shared/domain/interface/permission.enum.ts index 4512b95de7f..c3f880101b5 100644 --- a/apps/server/src/shared/domain/interface/permission.enum.ts +++ b/apps/server/src/shared/domain/interface/permission.enum.ts @@ -55,6 +55,7 @@ export enum Permission { FOLDER_CREATE = 'FOLDER_CREATE', FOLDER_DELETE = 'FOLDER_DELETE', GROUP_LIST = 'GROUP_LIST', + GROUP_FULL_ADMIN = 'GROUP_FULL_ADMIN', GROUP_VIEW = 'GROUP_VIEW', HELPDESK_CREATE = 'HELPDESK_CREATE', HELPDESK_EDIT = 'HELPDESK_EDIT', diff --git a/apps/server/src/shared/testing/user-role-permissions.ts b/apps/server/src/shared/testing/user-role-permissions.ts index cfd38cea3a3..6c38287a37e 100644 --- a/apps/server/src/shared/testing/user-role-permissions.ts +++ b/apps/server/src/shared/testing/user-role-permissions.ts @@ -139,4 +139,5 @@ export const adminPermissions = [ Permission.IMPORT_USER_UPDATE, Permission.IMPORT_USER_VIEW, Permission.SCHOOL_TOOL_ADMIN, + Permission.GROUP_FULL_ADMIN, ] as Permission[]; diff --git a/backup/setup/external_tools.json b/backup/setup/external_tools.json index 22a582f9a09..2a3fd937a37 100644 --- a/backup/setup/external_tools.json +++ b/backup/setup/external_tools.json @@ -76,9 +76,9 @@ } }, "name": "LTI Test Tool", - "url": "https://www.tsugi.org/lti-test/tool.php", + "url": "https://saltire.lti.app", "config_type": "lti11", - "config_baseUrl": "https://www.tsugi.org/lti-test/tool.php", + "config_baseUrl": "https://saltire.lti.app/tool", "config_key": "12345", "config_secret": "secret", "config_lti_message_type": "basic-lti-launch-request", diff --git a/backup/setup/migrations.json b/backup/setup/migrations.json index 8292d8fc62f..90fcee0baa3 100644 --- a/backup/setup/migrations.json +++ b/backup/setup/migrations.json @@ -328,5 +328,16 @@ "$date": "2023-10-17T14:38:44.886Z" }, "__v": 0 + }, + { + "_id": { + "$oid": "653a645338f94b0ea8e3173d" + }, + "state": "up", + "name": "add-group-full-admin-permission", + "createdAt": { + "$date": "2023-10-26T13:06:27.322Z" + }, + "__v": 0 } ] diff --git a/backup/setup/roles.json b/backup/setup/roles.json index 0ad460fc526..9ceaa532f1d 100644 --- a/backup/setup/roles.json +++ b/backup/setup/roles.json @@ -134,7 +134,8 @@ "USER_LOGIN_MIGRATION_ADMIN", "START_MEETING", "JOIN_MEETING", - "GROUP_LIST" + "GROUP_LIST", + "GROUP_FULL_ADMIN" ], "__v": 2 }, @@ -191,7 +192,8 @@ "TOOL_CREATE", "TOOL_EDIT", "YEARS_EDIT", - "GROUP_LIST" + "GROUP_LIST", + "GROUP_FULL_ADMIN" ], "__v": 2 }, diff --git a/migrations/1698325587322-add-group-full-admin-permission.js b/migrations/1698325587322-add-group-full-admin-permission.js new file mode 100644 index 00000000000..cb7a10a642a --- /dev/null +++ b/migrations/1698325587322-add-group-full-admin-permission.js @@ -0,0 +1,74 @@ +const mongoose = require('mongoose'); +// eslint-disable-next-line no-unused-vars +const { info } = require('winston'); +const { alert } = require('../src/logger'); + +const { connect, close } = require('../src/utils/database'); + +const Roles = mongoose.model( + 'roles202310261524', + new mongoose.Schema( + { + name: { type: String, required: true }, + permissions: [{ type: String }], + }, + { + timestamps: true, + } + ), + 'roles' +); + +module.exports = { + up: async function up() { + // eslint-disable-next-line no-process-env + if (process.env.SC_THEME !== 'n21') { + info('Permission GROUP_FULL_ADMIN will not be added for this instance.'); + return; + } + + await connect(); + + const adminAndSuperheroRole = await Roles.updateMany( + { name: { $in: ['administrator', 'superhero'] } }, + { + $addToSet: { + permissions: { + $each: ['GROUP_FULL_ADMIN'], + }, + }, + } + ).exec(); + + if (adminAndSuperheroRole) { + alert('Permission GROUP_FULL_ADMIN added to role superhero and administrator'); + } + + await close(); + }, + + down: async function down() { + // eslint-disable-next-line no-process-env + if (process.env.SC_THEME !== 'n21') { + info('Permission GROUP_FULL_ADMIN will not be removed for this instance.'); + return; + } + + await connect(); + + const adminAndSuperheroRole = await Roles.updateMany( + { name: { $in: ['administrator', 'superhero'] } }, + { + $pull: { + permissions: 'GROUP_FULL_ADMIN', + }, + } + ).exec(); + + if (adminAndSuperheroRole) { + alert('Rollback: Removed permission GROUP_FULL_ADMIN from roles superhero and administrator'); + } + + await close(); + }, +};