From 399244fee786da12b63005d37c5806c4a6894449 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= <103562092+MarvinOehlerkingCap@users.noreply.github.com> Date: Wed, 29 Nov 2023 15:57:15 +0100 Subject: [PATCH 1/2] N21-1553 Fix deletion of board elements, when the external tool does not exist anymore (#4591) --- .../repo/recursive-delete.visitor.spec.ts | 94 ++++++++++++++----- .../board/repo/recursive-delete.vistor.ts | 10 +- .../context-external-tool.service.spec.ts | 48 +++++++++- .../service/context-external-tool.service.ts | 8 +- .../service/tool-reference.service.spec.ts | 2 +- .../service/tool-reference.service.ts | 2 +- .../uc/context-external-tool.uc.spec.ts | 14 +-- .../uc/context-external-tool.uc.ts | 6 +- .../uc/tool-reference.uc.spec.ts | 6 +- .../uc/tool-reference.uc.ts | 4 +- .../uc/external-tool-configuration.uc.spec.ts | 8 +- .../uc/external-tool-configuration.uc.ts | 4 +- .../tool-launch/uc/tool-launch.uc.spec.ts | 4 +- .../tool/tool-launch/uc/tool-launch.uc.ts | 4 +- ...ext-external-tool.repo.integration.spec.ts | 55 ++++++++++- .../context-external-tool.repo.ts | 18 ++++ 16 files changed, 225 insertions(+), 62 deletions(-) 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 49604d6fcbf..e8f3aa762a5 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,7 +1,8 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { FileRecordParentType } from '@infra/rabbitmq'; -import { EntityManager } from '@mikro-orm/mongodb'; +import { EntityManager, ObjectId } from '@mikro-orm/mongodb'; import { FileDto, FilesStorageClientAdapterService } from '@modules/files-storage-client'; +import { DrawingElementAdapterService } from '@modules/tldraw-client/service/drawing-element-adapter.service'; import { ContextExternalToolService } from '@modules/tool/context-external-tool/service'; import { Test, TestingModule } from '@nestjs/testing'; import { @@ -16,7 +17,6 @@ import { submissionContainerElementFactory, submissionItemFactory, } from '@shared/testing'; -import { DrawingElementAdapterService } from '@modules/tldraw-client/service/drawing-element-adapter.service'; import { RecursiveDeleteVisitor } from './recursive-delete.vistor'; describe(RecursiveDeleteVisitor.name, () => { @@ -255,40 +255,82 @@ describe(RecursiveDeleteVisitor.name, () => { }); describe('visitExternalToolElementAsync', () => { - const setup = () => { - const contextExternalTool = contextExternalToolFactory.buildWithId(); - const childExternalToolElement = externalToolElementFactory.build(); - const externalToolElement = externalToolElementFactory.build({ - children: [childExternalToolElement], - contextExternalToolId: contextExternalTool.id, - }); + describe('when the linked context external tool exists', () => { + const setup = () => { + const contextExternalTool = contextExternalToolFactory.buildWithId(); + const childExternalToolElement = externalToolElementFactory.build(); + const externalToolElement = externalToolElementFactory.build({ + children: [childExternalToolElement], + contextExternalToolId: contextExternalTool.id, + }); - contextExternalToolService.findById.mockResolvedValue(contextExternalTool); + contextExternalToolService.findById.mockResolvedValue(contextExternalTool); - return { - externalToolElement, - childExternalToolElement, - contextExternalTool, + return { + externalToolElement, + childExternalToolElement, + contextExternalTool, + }; }; - }; - it('should delete the context external tool that is linked to the element', async () => { - const { externalToolElement, contextExternalTool } = setup(); + it('should delete the context external tool that is linked to the element', async () => { + const { externalToolElement, contextExternalTool } = setup(); - await service.visitExternalToolElementAsync(externalToolElement); + await service.visitExternalToolElementAsync(externalToolElement); - expect(contextExternalToolService.deleteContextExternalTool).toHaveBeenCalledWith(contextExternalTool); + expect(contextExternalToolService.deleteContextExternalTool).toHaveBeenCalledWith(contextExternalTool); + }); + + it('should call entity remove', async () => { + const { externalToolElement, childExternalToolElement } = setup(); + + await service.visitExternalToolElementAsync(externalToolElement); + + expect(em.remove).toHaveBeenCalledWith( + em.getReference(externalToolElement.constructor, externalToolElement.id) + ); + expect(em.remove).toHaveBeenCalledWith( + em.getReference(childExternalToolElement.constructor, childExternalToolElement.id) + ); + }); }); - it('should call entity remove', async () => { - const { externalToolElement, childExternalToolElement } = setup(); + describe('when the external tool does not exist anymore', () => { + const setup = () => { + const childExternalToolElement = externalToolElementFactory.build(); + const externalToolElement = externalToolElementFactory.build({ + children: [childExternalToolElement], + contextExternalToolId: new ObjectId().toHexString(), + }); - await service.visitExternalToolElementAsync(externalToolElement); + contextExternalToolService.findById.mockResolvedValue(null); - expect(em.remove).toHaveBeenCalledWith(em.getReference(externalToolElement.constructor, externalToolElement.id)); - expect(em.remove).toHaveBeenCalledWith( - em.getReference(childExternalToolElement.constructor, childExternalToolElement.id) - ); + return { + externalToolElement, + childExternalToolElement, + }; + }; + + it('should not try to delete the context external tool that is linked to the element', async () => { + const { externalToolElement } = setup(); + + await service.visitExternalToolElementAsync(externalToolElement); + + expect(contextExternalToolService.deleteContextExternalTool).not.toHaveBeenCalled(); + }); + + it('should call entity remove', async () => { + const { externalToolElement, childExternalToolElement } = setup(); + + await service.visitExternalToolElementAsync(externalToolElement); + + expect(em.remove).toHaveBeenCalledWith( + em.getReference(externalToolElement.constructor, externalToolElement.id) + ); + expect(em.remove).toHaveBeenCalledWith( + em.getReference(childExternalToolElement.constructor, childExternalToolElement.id) + ); + }); }); }); }); 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 a4ba34425e0..c7acaa1c81d 100644 --- a/apps/server/src/modules/board/repo/recursive-delete.vistor.ts +++ b/apps/server/src/modules/board/repo/recursive-delete.vistor.ts @@ -1,5 +1,6 @@ import { EntityManager } from '@mikro-orm/mongodb'; import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; +import { DrawingElementAdapterService } from '@modules/tldraw-client/service/drawing-element-adapter.service'; import { ContextExternalTool } from '@modules/tool/context-external-tool/domain'; import { ContextExternalToolService } from '@modules/tool/context-external-tool/service'; import { Injectable } from '@nestjs/common'; @@ -16,9 +17,8 @@ import { SubmissionContainerElement, SubmissionItem, } from '@shared/domain'; -import { LinkElement } from '@shared/domain/domainobject/board/link-element.do'; import { DrawingElement } from '@shared/domain/domainobject/board/drawing-element.do'; -import { DrawingElementAdapterService } from '@modules/tldraw-client/service/drawing-element-adapter.service'; +import { LinkElement } from '@shared/domain/domainobject/board/link-element.do'; @Injectable() export class RecursiveDeleteVisitor implements BoardCompositeVisitorAsync { @@ -82,11 +82,13 @@ export class RecursiveDeleteVisitor implements BoardCompositeVisitorAsync { async visitExternalToolElementAsync(externalToolElement: ExternalToolElement): Promise { if (externalToolElement.contextExternalToolId) { - const linkedTool: ContextExternalTool = await this.contextExternalToolService.findById( + const linkedTool: ContextExternalTool | null = await this.contextExternalToolService.findById( externalToolElement.contextExternalToolId ); - await this.contextExternalToolService.deleteContextExternalTool(linkedTool); + if (linkedTool) { + await this.contextExternalToolService.deleteContextExternalTool(linkedTool); + } } this.deleteNode(externalToolElement); diff --git a/apps/server/src/modules/tool/context-external-tool/service/context-external-tool.service.spec.ts b/apps/server/src/modules/tool/context-external-tool/service/context-external-tool.service.spec.ts index 60275c71d0d..ba4c90a6cb0 100644 --- a/apps/server/src/modules/tool/context-external-tool/service/context-external-tool.service.spec.ts +++ b/apps/server/src/modules/tool/context-external-tool/service/context-external-tool.service.spec.ts @@ -1,4 +1,5 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { AuthorizationService } from '@modules/authorization'; import { NotFoundException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { ContextExternalToolRepo } from '@shared/repo'; @@ -7,7 +8,6 @@ import { legacySchoolDoFactory, schoolExternalToolFactory, } from '@shared/testing/factory/domainobject'; -import { AuthorizationService } from '@modules/authorization'; import { ToolContextType } from '../../common/enum'; import { SchoolExternalTool } from '../../school-external-tool/domain'; import { ContextExternalTool, ContextRef } from '../domain'; @@ -151,7 +151,7 @@ describe('ContextExternalToolService', () => { it('should return a contextExternalTool', async () => { const { contextExternalTool } = setup(); - const result: ContextExternalTool = await service.findById(contextExternalTool.id as string); + const result: ContextExternalTool = await service.findByIdOrFail(contextExternalTool.id as string); expect(result).toEqual(contextExternalTool); }); @@ -165,13 +165,55 @@ describe('ContextExternalToolService', () => { it('should throw a not found exception', async () => { setup(); - const func = () => service.findById('unknownContextExternalToolId'); + const func = () => service.findByIdOrFail('unknownContextExternalToolId'); await expect(func()).rejects.toThrow(NotFoundException); }); }); }); + describe('findByIdOrNull', () => { + describe('when contextExternalToolId is given', () => { + const setup = () => { + const schoolId: string = legacySchoolDoFactory.buildWithId().id as string; + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build({ + schoolId, + }); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory + .withSchoolExternalToolRef(schoolExternalTool.id as string, schoolExternalTool.schoolId) + .build(); + + contextExternalToolRepo.findByIdOrNull.mockResolvedValue(contextExternalTool); + + return { + contextExternalTool, + }; + }; + + it('should return a contextExternalTool', async () => { + const { contextExternalTool } = setup(); + + const result: ContextExternalTool | null = await service.findById(contextExternalTool.id as string); + + expect(result).toEqual(contextExternalTool); + }); + }); + + describe('when contextExternalTool could not be found', () => { + const setup = () => { + contextExternalToolRepo.findByIdOrNull.mockResolvedValueOnce(null); + }; + + it('should throw a not found exception', async () => { + setup(); + + const result: ContextExternalTool | null = await service.findById('unknownContextExternalToolId'); + + expect(result).toBeNull(); + }); + }); + }); + describe('deleteContextExternalTool', () => { describe('when contextExternalToolId is given', () => { const setup = () => { diff --git a/apps/server/src/modules/tool/context-external-tool/service/context-external-tool.service.ts b/apps/server/src/modules/tool/context-external-tool/service/context-external-tool.service.ts index 63618191810..b389b4b948e 100644 --- a/apps/server/src/modules/tool/context-external-tool/service/context-external-tool.service.ts +++ b/apps/server/src/modules/tool/context-external-tool/service/context-external-tool.service.ts @@ -14,12 +14,18 @@ export class ContextExternalToolService { return contextExternalTools; } - async findById(contextExternalToolId: EntityId): Promise { + async findByIdOrFail(contextExternalToolId: EntityId): Promise { const tool: ContextExternalTool = await this.contextExternalToolRepo.findById(contextExternalToolId); return tool; } + async findById(contextExternalToolId: EntityId): Promise { + const tool: ContextExternalTool | null = await this.contextExternalToolRepo.findByIdOrNull(contextExternalToolId); + + return tool; + } + async saveContextExternalTool(contextExternalTool: ContextExternalTool): Promise { const savedContextExternalTool: ContextExternalTool = await this.contextExternalToolRepo.save(contextExternalTool); diff --git a/apps/server/src/modules/tool/context-external-tool/service/tool-reference.service.spec.ts b/apps/server/src/modules/tool/context-external-tool/service/tool-reference.service.spec.ts index 1796b2f681d..be90bdd3599 100644 --- a/apps/server/src/modules/tool/context-external-tool/service/tool-reference.service.spec.ts +++ b/apps/server/src/modules/tool/context-external-tool/service/tool-reference.service.spec.ts @@ -76,7 +76,7 @@ describe('ToolReferenceService', () => { .buildWithId(undefined, contextExternalToolId); const logoUrl = 'logoUrl'; - contextExternalToolService.findById.mockResolvedValueOnce(contextExternalTool); + contextExternalToolService.findByIdOrFail.mockResolvedValueOnce(contextExternalTool); schoolExternalToolService.findById.mockResolvedValueOnce(schoolExternalTool); externalToolService.findById.mockResolvedValueOnce(externalTool); toolVersionService.determineToolConfigurationStatus.mockResolvedValue(ToolConfigurationStatus.OUTDATED); diff --git a/apps/server/src/modules/tool/context-external-tool/service/tool-reference.service.ts b/apps/server/src/modules/tool/context-external-tool/service/tool-reference.service.ts index 1f677e3e159..7fc2966de78 100644 --- a/apps/server/src/modules/tool/context-external-tool/service/tool-reference.service.ts +++ b/apps/server/src/modules/tool/context-external-tool/service/tool-reference.service.ts @@ -20,7 +20,7 @@ export class ToolReferenceService { ) {} async getToolReference(contextExternalToolId: EntityId): Promise { - const contextExternalTool: ContextExternalTool = await this.contextExternalToolService.findById( + const contextExternalTool: ContextExternalTool = await this.contextExternalToolService.findByIdOrFail( contextExternalToolId ); const schoolExternalTool: SchoolExternalTool = await this.schoolExternalToolService.findById( diff --git a/apps/server/src/modules/tool/context-external-tool/uc/context-external-tool.uc.spec.ts b/apps/server/src/modules/tool/context-external-tool/uc/context-external-tool.uc.spec.ts index 9239e4db2a9..c3eb4cb0298 100644 --- a/apps/server/src/modules/tool/context-external-tool/uc/context-external-tool.uc.spec.ts +++ b/apps/server/src/modules/tool/context-external-tool/uc/context-external-tool.uc.spec.ts @@ -301,7 +301,7 @@ describe('ContextExternalToolUc', () => { schoolExternalToolService.findById.mockResolvedValueOnce(schoolExternalTool); contextExternalToolService.saveContextExternalTool.mockResolvedValue(contextExternalTool); - contextExternalToolService.findById.mockResolvedValueOnce(contextExternalTool); + contextExternalToolService.findByIdOrFail.mockResolvedValueOnce(contextExternalTool); return { contextExternalTool, @@ -424,7 +424,7 @@ describe('ContextExternalToolUc', () => { const error = new ForbiddenException(); schoolExternalToolService.findById.mockResolvedValueOnce(schoolExternalTool); - contextExternalToolService.findById.mockResolvedValueOnce(contextExternalTool); + contextExternalToolService.findByIdOrFail.mockResolvedValueOnce(contextExternalTool); toolPermissionHelper.ensureContextPermissions.mockRejectedValue(error); return { @@ -470,7 +470,7 @@ describe('ContextExternalToolUc', () => { const error = new UnprocessableEntityException(); schoolExternalToolService.findById.mockResolvedValueOnce(schoolExternalTool); - contextExternalToolService.findById.mockResolvedValueOnce(contextExternalTool); + contextExternalToolService.findByIdOrFail.mockResolvedValueOnce(contextExternalTool); contextExternalToolValidationService.validate.mockRejectedValue(error); return { @@ -501,7 +501,7 @@ describe('ContextExternalToolUc', () => { const contextExternalTool: ContextExternalTool = contextExternalToolFactory.buildWithId(); toolPermissionHelper.ensureContextPermissions.mockResolvedValue(); - contextExternalToolService.findById.mockResolvedValue(contextExternalTool); + contextExternalToolService.findByIdOrFail.mockResolvedValue(contextExternalTool); return { contextExternalTool, @@ -658,7 +658,7 @@ describe('ContextExternalToolUc', () => { }, }); - contextExternalToolService.findById.mockResolvedValue(contextExternalTool); + contextExternalToolService.findByIdOrFail.mockResolvedValue(contextExternalTool); toolPermissionHelper.ensureContextPermissions.mockResolvedValue(Promise.resolve()); return { @@ -686,7 +686,7 @@ describe('ContextExternalToolUc', () => { await uc.getContextExternalTool(userId, contextExternalTool.id as string); - expect(contextExternalToolService.findById).toHaveBeenCalledWith(contextExternalTool.id); + expect(contextExternalToolService.findByIdOrFail).toHaveBeenCalledWith(contextExternalTool.id); }); }); @@ -704,7 +704,7 @@ describe('ContextExternalToolUc', () => { }, }); - contextExternalToolService.findById.mockResolvedValue(contextExternalTool); + contextExternalToolService.findByIdOrFail.mockResolvedValue(contextExternalTool); toolPermissionHelper.ensureContextPermissions.mockRejectedValue( new ForbiddenLoggableException( userId, diff --git a/apps/server/src/modules/tool/context-external-tool/uc/context-external-tool.uc.ts b/apps/server/src/modules/tool/context-external-tool/uc/context-external-tool.uc.ts index 80a85321933..d584be201a8 100644 --- a/apps/server/src/modules/tool/context-external-tool/uc/context-external-tool.uc.ts +++ b/apps/server/src/modules/tool/context-external-tool/uc/context-external-tool.uc.ts @@ -69,7 +69,7 @@ export class ContextExternalToolUc { throw new ForbiddenLoggableException(userId, AuthorizableReferenceType.ContextExternalToolEntity, context); } - let contextExternalTool: ContextExternalTool = await this.contextExternalToolService.findById( + let contextExternalTool: ContextExternalTool = await this.contextExternalToolService.findByIdOrFail( contextExternalToolId ); @@ -91,7 +91,7 @@ export class ContextExternalToolUc { } public async deleteContextExternalTool(userId: EntityId, contextExternalToolId: EntityId): Promise { - const tool: ContextExternalTool = await this.contextExternalToolService.findById(contextExternalToolId); + const tool: ContextExternalTool = await this.contextExternalToolService.findByIdOrFail(contextExternalToolId); const context = AuthorizationContextBuilder.write([Permission.CONTEXT_TOOL_ADMIN]); await this.toolPermissionHelper.ensureContextPermissions(userId, tool, context); @@ -114,7 +114,7 @@ export class ContextExternalToolUc { } async getContextExternalTool(userId: EntityId, contextToolId: EntityId) { - const tool: ContextExternalTool = await this.contextExternalToolService.findById(contextToolId); + const tool: ContextExternalTool = await this.contextExternalToolService.findByIdOrFail(contextToolId); const context: AuthorizationContext = AuthorizationContextBuilder.read([Permission.CONTEXT_TOOL_ADMIN]); await this.toolPermissionHelper.ensureContextPermissions(userId, tool, context); diff --git a/apps/server/src/modules/tool/context-external-tool/uc/tool-reference.uc.spec.ts b/apps/server/src/modules/tool/context-external-tool/uc/tool-reference.uc.spec.ts index 9b18e7ffd3b..e0b39787737 100644 --- a/apps/server/src/modules/tool/context-external-tool/uc/tool-reference.uc.spec.ts +++ b/apps/server/src/modules/tool/context-external-tool/uc/tool-reference.uc.spec.ts @@ -1,9 +1,9 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { AuthorizationContextBuilder } from '@modules/authorization'; import { ForbiddenException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { Permission } from '@shared/domain'; import { contextExternalToolFactory, externalToolFactory } from '@shared/testing'; -import { AuthorizationContextBuilder } from '@modules/authorization'; import { ToolConfigurationStatus, ToolContextType } from '../../common/enum'; import { ToolPermissionHelper } from '../../common/uc/tool-permission-helper'; import { ExternalTool } from '../../external-tool/domain'; @@ -150,7 +150,7 @@ describe('ToolReferenceUc', () => { openInNewTab: externalTool.openNewTab, }); - contextExternalToolService.findById.mockResolvedValueOnce(contextExternalTool); + contextExternalToolService.findByIdOrFail.mockResolvedValueOnce(contextExternalTool); toolPermissionHelper.ensureContextPermissions.mockResolvedValueOnce(); toolReferenceService.getToolReference.mockResolvedValue(toolReference); @@ -195,7 +195,7 @@ describe('ToolReferenceUc', () => { ); const error = new ForbiddenException(); - contextExternalToolService.findById.mockResolvedValueOnce(contextExternalTool); + contextExternalToolService.findByIdOrFail.mockResolvedValueOnce(contextExternalTool); toolPermissionHelper.ensureContextPermissions.mockRejectedValueOnce(error); return { diff --git a/apps/server/src/modules/tool/context-external-tool/uc/tool-reference.uc.ts b/apps/server/src/modules/tool/context-external-tool/uc/tool-reference.uc.ts index ac6ccda016f..dd2e8d1c49d 100644 --- a/apps/server/src/modules/tool/context-external-tool/uc/tool-reference.uc.ts +++ b/apps/server/src/modules/tool/context-external-tool/uc/tool-reference.uc.ts @@ -1,6 +1,6 @@ +import { AuthorizationContext, AuthorizationContextBuilder } from '@modules/authorization'; import { Injectable } from '@nestjs/common'; import { EntityId, Permission } from '@shared/domain'; -import { AuthorizationContext, AuthorizationContextBuilder } from '@modules/authorization'; import { ToolContextType } from '../../common/enum'; import { ToolPermissionHelper } from '../../common/uc/tool-permission-helper'; import { ContextExternalTool, ContextRef, ToolReference } from '../domain'; @@ -55,7 +55,7 @@ export class ToolReferenceUc { } async getToolReference(userId: EntityId, contextExternalToolId: EntityId): Promise { - const contextExternalTool: ContextExternalTool = await this.contextExternalToolService.findById( + const contextExternalTool: ContextExternalTool = await this.contextExternalToolService.findByIdOrFail( contextExternalToolId ); diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool-configuration.uc.spec.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool-configuration.uc.spec.ts index 5327c8d4f80..90d205ad605 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool-configuration.uc.spec.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool-configuration.uc.spec.ts @@ -1,5 +1,6 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; +import { AuthorizationContextBuilder } from '@modules/authorization'; import { ForbiddenException, NotFoundException, UnauthorizedException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { Page, Permission } from '@shared/domain'; @@ -10,7 +11,6 @@ import { schoolExternalToolFactory, setupEntities, } from '@shared/testing'; -import { AuthorizationContextBuilder } from '@modules/authorization'; import { CustomParameterScope, ToolContextType } from '../../common/enum'; import { ToolPermissionHelper } from '../../common/uc/tool-permission-helper'; import { ContextExternalTool } from '../../context-external-tool/domain'; @@ -553,7 +553,7 @@ describe('ExternalToolConfigurationUc', () => { contextExternalToolId ); - contextExternalToolService.findById.mockResolvedValueOnce(contextExternalTool); + contextExternalToolService.findByIdOrFail.mockResolvedValueOnce(contextExternalTool); schoolExternalToolService.findById.mockResolvedValueOnce(schoolExternalTool); externalToolService.findById.mockResolvedValueOnce(externalTool); @@ -593,7 +593,7 @@ describe('ExternalToolConfigurationUc', () => { contextExternalToolId ); - contextExternalToolService.findById.mockResolvedValueOnce(contextExternalTool); + contextExternalToolService.findByIdOrFail.mockResolvedValueOnce(contextExternalTool); toolPermissionHelper.ensureContextPermissions.mockImplementation(() => { throw new UnauthorizedException(); }); @@ -632,7 +632,7 @@ describe('ExternalToolConfigurationUc', () => { contextExternalToolId ); - contextExternalToolService.findById.mockResolvedValueOnce(contextExternalTool); + contextExternalToolService.findByIdOrFail.mockResolvedValueOnce(contextExternalTool); schoolExternalToolService.findById.mockResolvedValueOnce(schoolExternalTool); externalToolService.findById.mockResolvedValueOnce(externalTool); diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool-configuration.uc.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool-configuration.uc.ts index f0c1af811fb..43a1c7b7775 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool-configuration.uc.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool-configuration.uc.ts @@ -1,8 +1,8 @@ +import { AuthorizationContext, AuthorizationContextBuilder } from '@modules/authorization'; import { forwardRef, Inject, Injectable } from '@nestjs/common'; import { NotFoundException } from '@nestjs/common/exceptions/not-found.exception'; import { EntityId, Permission } from '@shared/domain'; import { Page } from '@shared/domain/domainobject/page'; -import { AuthorizationContext, AuthorizationContextBuilder } from '@modules/authorization'; import { CustomParameterScope, ToolContextType } from '../../common/enum'; import { ToolPermissionHelper } from '../../common/uc/tool-permission-helper'; import { ContextExternalTool } from '../../context-external-tool/domain'; @@ -137,7 +137,7 @@ export class ExternalToolConfigurationUc { userId: EntityId, contextExternalToolId: EntityId ): Promise { - const contextExternalTool: ContextExternalTool = await this.contextExternalToolService.findById( + const contextExternalTool: ContextExternalTool = await this.contextExternalToolService.findByIdOrFail( contextExternalToolId ); diff --git a/apps/server/src/modules/tool/tool-launch/uc/tool-launch.uc.spec.ts b/apps/server/src/modules/tool/tool-launch/uc/tool-launch.uc.spec.ts index e9b7311e06a..20ae93b7dd0 100644 --- a/apps/server/src/modules/tool/tool-launch/uc/tool-launch.uc.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/uc/tool-launch.uc.spec.ts @@ -66,7 +66,7 @@ describe('ToolLaunchUc', () => { const userId: string = new ObjectId().toHexString(); toolPermissionHelper.ensureContextPermissions.mockResolvedValueOnce(); - contextExternalToolService.findById.mockResolvedValueOnce(contextExternalTool); + contextExternalToolService.findByIdOrFail.mockResolvedValueOnce(contextExternalTool); toolLaunchService.getLaunchData.mockResolvedValueOnce(toolLaunchData); return { @@ -82,7 +82,7 @@ describe('ToolLaunchUc', () => { await uc.getToolLaunchRequest(userId, contextExternalToolId); - expect(contextExternalToolService.findById).toHaveBeenCalledWith(contextExternalToolId); + expect(contextExternalToolService.findByIdOrFail).toHaveBeenCalledWith(contextExternalToolId); }); it('should call service to get data', async () => { diff --git a/apps/server/src/modules/tool/tool-launch/uc/tool-launch.uc.ts b/apps/server/src/modules/tool/tool-launch/uc/tool-launch.uc.ts index 272d8dfd376..cb38ecb065e 100644 --- a/apps/server/src/modules/tool/tool-launch/uc/tool-launch.uc.ts +++ b/apps/server/src/modules/tool/tool-launch/uc/tool-launch.uc.ts @@ -1,6 +1,6 @@ +import { AuthorizationContext, AuthorizationContextBuilder } from '@modules/authorization'; import { Injectable } from '@nestjs/common'; import { EntityId, Permission } from '@shared/domain'; -import { AuthorizationContext, AuthorizationContextBuilder } from '@modules/authorization'; import { ToolPermissionHelper } from '../../common/uc/tool-permission-helper'; import { ContextExternalTool } from '../../context-external-tool/domain'; import { ContextExternalToolService } from '../../context-external-tool/service'; @@ -16,7 +16,7 @@ export class ToolLaunchUc { ) {} async getToolLaunchRequest(userId: EntityId, contextExternalToolId: EntityId): Promise { - const contextExternalTool: ContextExternalTool = await this.contextExternalToolService.findById( + const contextExternalTool: ContextExternalTool = await this.contextExternalToolService.findByIdOrFail( contextExternalToolId ); const context: AuthorizationContext = AuthorizationContextBuilder.read([Permission.CONTEXT_TOOL_USER]); diff --git a/apps/server/src/shared/repo/contextexternaltool/context-external-tool.repo.integration.spec.ts b/apps/server/src/shared/repo/contextexternaltool/context-external-tool.repo.integration.spec.ts index c6a684a377d..cb320249dad 100644 --- a/apps/server/src/shared/repo/contextexternaltool/context-external-tool.repo.integration.spec.ts +++ b/apps/server/src/shared/repo/contextexternaltool/context-external-tool.repo.integration.spec.ts @@ -367,7 +367,7 @@ describe('ContextExternalToolRepo', () => { }; }; - it('should return correct results', async () => { + it('should return a context external tool', async () => { const { contextExternalTool, schoolExternalTool } = await setup(); const result = await repo.findById(contextExternalTool.id); @@ -395,6 +395,59 @@ describe('ContextExternalToolRepo', () => { }); }); + describe('findByIdOrNull', () => { + describe('when a ContextExternalTool is found', () => { + const setup = async () => { + const schoolExternalTool = schoolExternalToolEntityFactory.buildWithId(); + const contextExternalTool = contextExternalToolEntityFactory.buildWithId({ + contextType: ContextExternalToolType.COURSE, + schoolTool: schoolExternalTool, + }); + + await em.persistAndFlush([schoolExternalTool, contextExternalTool]); + + return { + contextExternalTool, + schoolExternalTool, + }; + }; + + it('should return a context external tool', async () => { + const { contextExternalTool, schoolExternalTool } = await setup(); + + const result = await repo.findByIdOrNull(contextExternalTool.id); + + expect(result).toEqual({ + id: contextExternalTool.id, + contextRef: { + id: contextExternalTool.contextId, + type: ToolContextType.COURSE, + }, + displayName: contextExternalTool.displayName, + parameters: [ + { + name: contextExternalTool.parameters[0].name, + value: contextExternalTool.parameters[0].value, + }, + ], + schoolToolRef: { + schoolToolId: schoolExternalTool.id, + schoolId: schoolExternalTool.school.id, + }, + toolVersion: contextExternalTool.toolVersion, + }); + }); + }); + + describe('when no ContextExternalTool is found', () => { + it('should should return null', async () => { + const result: ContextExternalTool | null = await repo.findByIdOrNull(new ObjectId().toHexString()); + + expect(result).toBeNull(); + }); + }); + }); + describe('countBySchoolToolIdsAndContextType', () => { describe('when a ContextExternalTool is found for course context', () => { const setup = async () => { diff --git a/apps/server/src/shared/repo/contextexternaltool/context-external-tool.repo.ts b/apps/server/src/shared/repo/contextexternaltool/context-external-tool.repo.ts index e51dac73b1c..17e998b8002 100644 --- a/apps/server/src/shared/repo/contextexternaltool/context-external-tool.repo.ts +++ b/apps/server/src/shared/repo/contextexternaltool/context-external-tool.repo.ts @@ -75,6 +75,24 @@ export class ContextExternalToolRepo extends BaseDORepo< return mapped; } + public async findByIdOrNull(id: EntityId): Promise { + const entity: ContextExternalToolEntity | null = await this._em.findOne( + this.entityName, + { id }, + { + populate: ['schoolTool.school'], + } + ); + + if (!entity) { + return null; + } + + const mapped: ContextExternalTool = this.mapEntityToDO(entity); + + return mapped; + } + private buildScope(query: ContextExternalToolQuery): ContextExternalToolScope { const scope: ContextExternalToolScope = new ContextExternalToolScope(); From 1522c3fbf72169c30b07a30872910348044eea31 Mon Sep 17 00:00:00 2001 From: mamutmk5 <3045922+mamutmk5@users.noreply.github.com> Date: Wed, 29 Nov 2023 16:57:06 +0100 Subject: [PATCH 2/2] BC-5801 - Ingress class configurable (#4593) --- .../schulcloud-server-core/templates/api-files-ingress.yml.j2 | 2 +- ansible/roles/schulcloud-server-core/templates/ingress.yml.j2 | 2 +- .../schulcloud-server-core/templates/tldraw-ingress.yml.j2 | 2 +- .../schulcloud-server-h5p/templates/api-h5p-ingress.yml.j2 | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ansible/roles/schulcloud-server-core/templates/api-files-ingress.yml.j2 b/ansible/roles/schulcloud-server-core/templates/api-files-ingress.yml.j2 index a1264b52001..3eabd5d2659 100644 --- a/ansible/roles/schulcloud-server-core/templates/api-files-ingress.yml.j2 +++ b/ansible/roles/schulcloud-server-core/templates/api-files-ingress.yml.j2 @@ -19,7 +19,7 @@ metadata: {% endif %} spec: - ingressClassName: nginx + ingressClassName: {{ INGRESS_CLASS }} {% if CLUSTER_ISSUER is defined or (TLS_ENABELD is defined and TLS_ENABELD|bool) %} tls: - hosts: diff --git a/ansible/roles/schulcloud-server-core/templates/ingress.yml.j2 b/ansible/roles/schulcloud-server-core/templates/ingress.yml.j2 index b2dd208765f..86588e74b47 100644 --- a/ansible/roles/schulcloud-server-core/templates/ingress.yml.j2 +++ b/ansible/roles/schulcloud-server-core/templates/ingress.yml.j2 @@ -19,7 +19,7 @@ metadata: {% endif %} spec: - ingressClassName: nginx + ingressClassName: {{ INGRESS_CLASS }} {% if CLUSTER_ISSUER is defined or (TLS_ENABELD is defined and TLS_ENABELD|bool) %} tls: - hosts: diff --git a/ansible/roles/schulcloud-server-core/templates/tldraw-ingress.yml.j2 b/ansible/roles/schulcloud-server-core/templates/tldraw-ingress.yml.j2 index e80028a5985..37b476e0834 100644 --- a/ansible/roles/schulcloud-server-core/templates/tldraw-ingress.yml.j2 +++ b/ansible/roles/schulcloud-server-core/templates/tldraw-ingress.yml.j2 @@ -20,7 +20,7 @@ metadata: {% endif %} spec: - ingressClassName: nginx + ingressClassName: {{ INGRESS_CLASS }} {% if CLUSTER_ISSUER is defined or (TLS_ENABELD is defined and TLS_ENABELD|bool) %} tls: - hosts: diff --git a/ansible/roles/schulcloud-server-h5p/templates/api-h5p-ingress.yml.j2 b/ansible/roles/schulcloud-server-h5p/templates/api-h5p-ingress.yml.j2 index ec68641bfa2..c3359d63b5f 100644 --- a/ansible/roles/schulcloud-server-h5p/templates/api-h5p-ingress.yml.j2 +++ b/ansible/roles/schulcloud-server-h5p/templates/api-h5p-ingress.yml.j2 @@ -19,7 +19,7 @@ metadata: {% endif %} spec: - ingressClassName: nginx + ingressClassName: {{ INGRESS_CLASS }} {% if CLUSTER_ISSUER is defined or (TLS_ENABELD is defined and TLS_ENABELD|bool) %} tls: - hosts: