From c8fbf7037e51757d659be5ea868bcf4199257d0b Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Wed, 31 Jul 2024 14:54:30 +0200 Subject: [PATCH 01/13] added new type & enum for auto param for moin.schule group uuid --- .../src/modules/tool/common/enum/custom-parameter-type.enum.ts | 2 ++ .../common/enum/request-response/custom-parameter-type.enum.ts | 1 + .../service/validation/tool-parameter-type-validation.util.ts | 1 + .../tool/external-tool/mapper/external-tool-request.mapper.ts | 1 + .../tool/external-tool/mapper/external-tool-response.mapper.ts | 1 + 5 files changed, 6 insertions(+) diff --git a/apps/server/src/modules/tool/common/enum/custom-parameter-type.enum.ts b/apps/server/src/modules/tool/common/enum/custom-parameter-type.enum.ts index 7165a6a5750..8e81a113266 100644 --- a/apps/server/src/modules/tool/common/enum/custom-parameter-type.enum.ts +++ b/apps/server/src/modules/tool/common/enum/custom-parameter-type.enum.ts @@ -7,6 +7,7 @@ export enum CustomParameterType { AUTO_SCHOOLID = 'auto_schoolid', AUTO_SCHOOLNUMBER = 'auto_schoolnumber', AUTO_MEDIUMID = 'auto_mediumid', + AUTO_MOINSCHULE_GROUPUUID = 'auto_moinschule_groupuuid', } export const autoParameters: CustomParameterType[] = [ @@ -15,4 +16,5 @@ export const autoParameters: CustomParameterType[] = [ CustomParameterType.AUTO_SCHOOLID, CustomParameterType.AUTO_SCHOOLNUMBER, CustomParameterType.AUTO_MEDIUMID, + CustomParameterType.AUTO_MOINSCHULE_GROUPUUID, ]; diff --git a/apps/server/src/modules/tool/common/enum/request-response/custom-parameter-type.enum.ts b/apps/server/src/modules/tool/common/enum/request-response/custom-parameter-type.enum.ts index 7771941ee9a..0daa5c9582e 100644 --- a/apps/server/src/modules/tool/common/enum/request-response/custom-parameter-type.enum.ts +++ b/apps/server/src/modules/tool/common/enum/request-response/custom-parameter-type.enum.ts @@ -7,4 +7,5 @@ export enum CustomParameterTypeParams { AUTO_SCHOOLID = 'auto_schoolid', AUTO_SCHOOLNUMBER = 'auto_schoolnumber', AUTO_MEDIUMID = 'auto_mediumid', + AUTO_MOINSCHULE_GROUPUUID = 'auto_moinschule_groupuuid', } diff --git a/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.ts b/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.ts index 7c2b09cac5b..09269f9ccf9 100644 --- a/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.ts +++ b/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.ts @@ -11,6 +11,7 @@ export class ToolParameterTypeValidationUtil { [CustomParameterType.AUTO_SCHOOLID]: () => false, [CustomParameterType.AUTO_SCHOOLNUMBER]: () => false, [CustomParameterType.AUTO_MEDIUMID]: () => false, + [CustomParameterType.AUTO_MOINSCHULE_GROUPUUID]: () => false, }; public static isValueValidForType(type: CustomParameterType, val: string): boolean { diff --git a/apps/server/src/modules/tool/external-tool/mapper/external-tool-request.mapper.ts b/apps/server/src/modules/tool/external-tool/mapper/external-tool-request.mapper.ts index 96e0ef3dff7..5c35aaef005 100644 --- a/apps/server/src/modules/tool/external-tool/mapper/external-tool-request.mapper.ts +++ b/apps/server/src/modules/tool/external-tool/mapper/external-tool-request.mapper.ts @@ -57,6 +57,7 @@ const typeMapping: Record = { [CustomParameterTypeParams.AUTO_SCHOOLID]: CustomParameterType.AUTO_SCHOOLID, [CustomParameterTypeParams.AUTO_SCHOOLNUMBER]: CustomParameterType.AUTO_SCHOOLNUMBER, [CustomParameterTypeParams.AUTO_MEDIUMID]: CustomParameterType.AUTO_MEDIUMID, + [CustomParameterTypeParams.AUTO_MOINSCHULE_GROUPUUID]: CustomParameterType.AUTO_MOINSCHULE_GROUPUUID, }; @Injectable() diff --git a/apps/server/src/modules/tool/external-tool/mapper/external-tool-response.mapper.ts b/apps/server/src/modules/tool/external-tool/mapper/external-tool-response.mapper.ts index 7a91f127cd8..876fd0d33b5 100644 --- a/apps/server/src/modules/tool/external-tool/mapper/external-tool-response.mapper.ts +++ b/apps/server/src/modules/tool/external-tool/mapper/external-tool-response.mapper.ts @@ -42,6 +42,7 @@ const typeMapping: Record = { [CustomParameterType.AUTO_SCHOOLID]: CustomParameterTypeParams.AUTO_SCHOOLID, [CustomParameterType.AUTO_SCHOOLNUMBER]: CustomParameterTypeParams.AUTO_SCHOOLNUMBER, [CustomParameterType.AUTO_MEDIUMID]: CustomParameterTypeParams.AUTO_MEDIUMID, + [CustomParameterType.AUTO_MOINSCHULE_GROUPUUID]: CustomParameterTypeParams.AUTO_MOINSCHULE_GROUPUUID, }; @Injectable() From 53d4461c665d0247a01e4adb7743f83e195a33fc Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Wed, 31 Jul 2024 17:08:09 +0200 Subject: [PATCH 02/13] added new auto param strategy --- .../auto-moin-schule-group-uuid.strategy.ts | 38 +++++++++++++++++++ .../service/auto-parameter-strategy/index.ts | 1 + .../abstract-launch.strategy.ts | 5 ++- .../lti11-tool-launch.strategy.ts | 7 +++- .../tool/tool-launch/tool-launch.module.ts | 2 + 5 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-moin-schule-group-uuid.strategy.ts diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-moin-schule-group-uuid.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-moin-schule-group-uuid.strategy.ts new file mode 100644 index 00000000000..f9fe98e81d9 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-moin-schule-group-uuid.strategy.ts @@ -0,0 +1,38 @@ +import { Injectable } from '@nestjs/common'; +import { CourseService } from '@modules/learnroom'; +import { ToolContextType } from '../../../common/enum'; +import { MissingToolParameterValueLoggableException } from '../../error'; +import { ContextExternalToolLaunchable } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { AutoParameterStrategy } from './auto-parameter.strategy'; + +@Injectable() +export class AutoMoinSchuleGroupUuidStrategy implements AutoParameterStrategy { + constructor(private readonly courseService: CourseService) {} + + async getValue( + _schoolExternalTool: SchoolExternalTool, + contextExternalTool: ContextExternalToolLaunchable + ): Promise { + if (contextExternalTool.contextRef.type !== ToolContextType.COURSE) { + return undefined; + } + + const courseId = contextExternalTool.contextRef.id; + const courseEntity = await this.courseService.findById(courseId); + const syncedGroup = courseEntity.syncedWithGroup; + + if (!syncedGroup) { + return undefined; + } + + const uuid = syncedGroup.externalSource?.externalId; + + if (!uuid) { + // TODO: think if a new special error is needed for this case + throw new MissingToolParameterValueLoggableException(contextExternalTool, []); + } + + return uuid; + } +} 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 index 30f058547a1..780f72eab94 100644 --- 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 @@ -3,4 +3,5 @@ export * from './auto-school-id.strategy'; export * from './auto-context-id.strategy'; export * from './auto-context-name.strategy'; export * from './auto-school-number.strategy'; +export * from './auto-moin-schule-group-uuid.strategy'; export { AutoMediumIdStrategy } from './auto-medium-id.strategy'; diff --git a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts index 9abfb4cf78c..42c4ea95249 100644 --- a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts @@ -16,6 +16,7 @@ import { AutoParameterStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, + AutoMoinSchuleGroupUuidStrategy, } from '../auto-parameter-strategy'; import { ToolLaunchParams } from './tool-launch-params.interface'; import { ToolLaunchStrategy } from './tool-launch-strategy.interface'; @@ -29,7 +30,8 @@ export abstract class AbstractLaunchStrategy implements ToolLaunchStrategy { autoSchoolNumberStrategy: AutoSchoolNumberStrategy, autoContextIdStrategy: AutoContextIdStrategy, autoContextNameStrategy: AutoContextNameStrategy, - autoMediumIdStrategy: AutoMediumIdStrategy + autoMediumIdStrategy: AutoMediumIdStrategy, + autoMoinSchuleGroupUuidStrategy: AutoMoinSchuleGroupUuidStrategy ) { this.autoParameterStrategyMap = new Map([ [CustomParameterType.AUTO_SCHOOLID, autoSchoolIdStrategy], @@ -37,6 +39,7 @@ export abstract class AbstractLaunchStrategy implements ToolLaunchStrategy { [CustomParameterType.AUTO_CONTEXTID, autoContextIdStrategy], [CustomParameterType.AUTO_CONTEXTNAME, autoContextNameStrategy], [CustomParameterType.AUTO_MEDIUMID, autoMediumIdStrategy], + [CustomParameterType.AUTO_MOINSCHULE_GROUPUUID, autoMoinSchuleGroupUuidStrategy], ]); } diff --git a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts index 30a601f6025..b054e00a600 100644 --- a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts @@ -16,6 +16,7 @@ import { AutoMediumIdStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, + AutoMoinSchuleGroupUuidStrategy, } from '../auto-parameter-strategy'; import { Lti11EncryptionService } from '../lti11-encryption.service'; import { AbstractLaunchStrategy } from './abstract-launch.strategy'; @@ -31,14 +32,16 @@ export class Lti11ToolLaunchStrategy extends AbstractLaunchStrategy { autoSchoolNumberStrategy: AutoSchoolNumberStrategy, autoContextIdStrategy: AutoContextIdStrategy, autoContextNameStrategy: AutoContextNameStrategy, - autoMediumIdStrategy: AutoMediumIdStrategy + autoMediumIdStrategy: AutoMediumIdStrategy, + autoMoinSchuleGroupUuidStrategy: AutoMoinSchuleGroupUuidStrategy ) { super( autoSchoolIdStrategy, autoSchoolNumberStrategy, autoContextIdStrategy, autoContextNameStrategy, - autoMediumIdStrategy + autoMediumIdStrategy, + autoMoinSchuleGroupUuidStrategy ); } 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 9c1504321a9..c029e50457e 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 @@ -15,6 +15,7 @@ import { AutoMediumIdStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, + AutoMoinSchuleGroupUuidStrategy, } from './service/auto-parameter-strategy'; import { BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrategy } from './service/launch-strategy'; @@ -41,6 +42,7 @@ import { BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrat AutoSchoolIdStrategy, AutoSchoolNumberStrategy, AutoMediumIdStrategy, + AutoMoinSchuleGroupUuidStrategy, ], exports: [ToolLaunchService], }) From d94dc06a3e69741d6c055fbf4b24d99de402d63c Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Thu, 1 Aug 2024 12:15:19 +0200 Subject: [PATCH 03/13] ensure moinschule_group_uuid auto param is always optional --- .../external-tool-parameter-validation.service.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.ts b/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.ts index b7ed1709209..ae6c04c640e 100644 --- a/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.ts +++ b/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.ts @@ -45,6 +45,12 @@ export class ExternalToolParameterValidationService { ); } + if (!this.isAutoParameterMoinSchuleGroupUuidValid(param)) { + throw new ValidationError( + `tool_param_auto_moin_schule_group_uuid: The custom parameter "${param.name}" with type "${param.type} must be optional."` + ); + } + if (!this.isRegexCommentMandatoryAndFilled(param)) { throw new ValidationError( `tool_param_regexComment: The custom parameter "${param.name}" parameter is missing a regex comment.` @@ -165,4 +171,12 @@ export class ExternalToolParameterValidationService { return true; } + + private isAutoParameterMoinSchuleGroupUuidValid(customParameter: CustomParameter) { + if (customParameter.type === CustomParameterType.AUTO_MOINSCHULE_GROUPUUID && !customParameter.isOptional) { + return false; + } + + return true; + } } From f4cc5ef5631b3547a7284bacc2bb2ed2acaeef26 Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Thu, 1 Aug 2024 15:45:46 +0200 Subject: [PATCH 04/13] properly fetch syncedGroup from group service --- .../auto-moin-schule-group-uuid.strategy.ts | 19 +++++++++++++++---- .../tool/tool-launch/tool-launch.module.ts | 2 ++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-moin-schule-group-uuid.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-moin-schule-group-uuid.strategy.ts index f9fe98e81d9..faa81e6b2a7 100644 --- a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-moin-schule-group-uuid.strategy.ts +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-moin-schule-group-uuid.strategy.ts @@ -1,5 +1,7 @@ import { Injectable } from '@nestjs/common'; import { CourseService } from '@modules/learnroom'; +import { Group, GroupService } from '@modules/group'; +import { Course } from '@shared/domain/entity'; import { ToolContextType } from '../../../common/enum'; import { MissingToolParameterValueLoggableException } from '../../error'; import { ContextExternalToolLaunchable } from '../../../context-external-tool/domain'; @@ -8,7 +10,7 @@ import { AutoParameterStrategy } from './auto-parameter.strategy'; @Injectable() export class AutoMoinSchuleGroupUuidStrategy implements AutoParameterStrategy { - constructor(private readonly courseService: CourseService) {} + constructor(private readonly courseService: CourseService, private readonly groupService: GroupService) {} async getValue( _schoolExternalTool: SchoolExternalTool, @@ -19,15 +21,14 @@ export class AutoMoinSchuleGroupUuidStrategy implements AutoParameterStrategy { } const courseId = contextExternalTool.contextRef.id; - const courseEntity = await this.courseService.findById(courseId); - const syncedGroup = courseEntity.syncedWithGroup; + const course = await this.courseService.findById(courseId); + const syncedGroup = await this.getSyncedGroup(course); if (!syncedGroup) { return undefined; } const uuid = syncedGroup.externalSource?.externalId; - if (!uuid) { // TODO: think if a new special error is needed for this case throw new MissingToolParameterValueLoggableException(contextExternalTool, []); @@ -35,4 +36,14 @@ export class AutoMoinSchuleGroupUuidStrategy implements AutoParameterStrategy { return uuid; } + + private async getSyncedGroup(course: Course): Promise { + const syncedGroupId = course.syncedWithGroup?.id; + if (!syncedGroupId) { + return undefined; + } + + const syncedGroup = await this.groupService.findById(syncedGroupId); + return syncedGroup; + } } 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 c029e50457e..3fe571378f9 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 @@ -3,6 +3,7 @@ import { LearnroomModule } from '@modules/learnroom'; import { LegacySchoolModule } from '@modules/legacy-school'; import { PseudonymModule } from '@modules/pseudonym'; import { UserModule } from '@modules/user'; +import { GroupModule } from '@modules/group'; import { forwardRef, Module } from '@nestjs/common'; import { CommonToolModule } from '../common'; import { ContextExternalToolModule } from '../context-external-tool'; @@ -30,6 +31,7 @@ import { BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrat forwardRef(() => PseudonymModule), // i do not like this solution, the root problem is on other place but not detectable for me LearnroomModule, BoardModule, + GroupModule, ], providers: [ ToolLaunchService, From 154755128bdea56102108553808d82de4964a12a Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Thu, 1 Aug 2024 15:55:18 +0200 Subject: [PATCH 05/13] fix fetch external id --- .../auto-moin-schule-group-uuid.strategy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-moin-schule-group-uuid.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-moin-schule-group-uuid.strategy.ts index faa81e6b2a7..63498b0979e 100644 --- a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-moin-schule-group-uuid.strategy.ts +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-moin-schule-group-uuid.strategy.ts @@ -28,7 +28,7 @@ export class AutoMoinSchuleGroupUuidStrategy implements AutoParameterStrategy { return undefined; } - const uuid = syncedGroup.externalSource?.externalId; + const uuid = syncedGroup.getProps().externalSource?.externalId; if (!uuid) { // TODO: think if a new special error is needed for this case throw new MissingToolParameterValueLoggableException(contextExternalTool, []); From a7db9ef3dddeed0a011e2873a8d1cca30e555115 Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Fri, 2 Aug 2024 14:03:25 +0200 Subject: [PATCH 06/13] renamed auto param type to only AUTO_GROUP_UUID --- .../tool/common/enum/custom-parameter-type.enum.ts | 4 ++-- .../enum/request-response/custom-parameter-type.enum.ts | 2 +- .../validation/tool-parameter-type-validation.util.ts | 2 +- .../external-tool/mapper/external-tool-request.mapper.ts | 2 +- .../external-tool/mapper/external-tool-response.mapper.ts | 2 +- .../service/external-tool-parameter-validation.service.ts | 8 ++++---- ...group-uuid.strategy.ts => auto-group-uuid.strategy.ts} | 8 ++++---- .../tool-launch/service/auto-parameter-strategy/index.ts | 2 +- .../service/launch-strategy/abstract-launch.strategy.ts | 6 +++--- .../service/launch-strategy/lti11-tool-launch.strategy.ts | 6 +++--- .../src/modules/tool/tool-launch/tool-launch.module.ts | 4 ++-- 11 files changed, 23 insertions(+), 23 deletions(-) rename apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/{auto-moin-schule-group-uuid.strategy.ts => auto-group-uuid.strategy.ts} (89%) diff --git a/apps/server/src/modules/tool/common/enum/custom-parameter-type.enum.ts b/apps/server/src/modules/tool/common/enum/custom-parameter-type.enum.ts index 8e81a113266..a8c0d7b1e26 100644 --- a/apps/server/src/modules/tool/common/enum/custom-parameter-type.enum.ts +++ b/apps/server/src/modules/tool/common/enum/custom-parameter-type.enum.ts @@ -7,7 +7,7 @@ export enum CustomParameterType { AUTO_SCHOOLID = 'auto_schoolid', AUTO_SCHOOLNUMBER = 'auto_schoolnumber', AUTO_MEDIUMID = 'auto_mediumid', - AUTO_MOINSCHULE_GROUPUUID = 'auto_moinschule_groupuuid', + AUTO_GROUPUUID = 'auto_groupuuid', } export const autoParameters: CustomParameterType[] = [ @@ -16,5 +16,5 @@ export const autoParameters: CustomParameterType[] = [ CustomParameterType.AUTO_SCHOOLID, CustomParameterType.AUTO_SCHOOLNUMBER, CustomParameterType.AUTO_MEDIUMID, - CustomParameterType.AUTO_MOINSCHULE_GROUPUUID, + CustomParameterType.AUTO_GROUPUUID, ]; diff --git a/apps/server/src/modules/tool/common/enum/request-response/custom-parameter-type.enum.ts b/apps/server/src/modules/tool/common/enum/request-response/custom-parameter-type.enum.ts index 0daa5c9582e..346b74169ed 100644 --- a/apps/server/src/modules/tool/common/enum/request-response/custom-parameter-type.enum.ts +++ b/apps/server/src/modules/tool/common/enum/request-response/custom-parameter-type.enum.ts @@ -7,5 +7,5 @@ export enum CustomParameterTypeParams { AUTO_SCHOOLID = 'auto_schoolid', AUTO_SCHOOLNUMBER = 'auto_schoolnumber', AUTO_MEDIUMID = 'auto_mediumid', - AUTO_MOINSCHULE_GROUPUUID = 'auto_moinschule_groupuuid', + AUTO_GROUPUUID = 'auto_groupuuid', } diff --git a/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.ts b/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.ts index 09269f9ccf9..f0cd88263aa 100644 --- a/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.ts +++ b/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.ts @@ -11,7 +11,7 @@ export class ToolParameterTypeValidationUtil { [CustomParameterType.AUTO_SCHOOLID]: () => false, [CustomParameterType.AUTO_SCHOOLNUMBER]: () => false, [CustomParameterType.AUTO_MEDIUMID]: () => false, - [CustomParameterType.AUTO_MOINSCHULE_GROUPUUID]: () => false, + [CustomParameterType.AUTO_GROUPUUID]: () => false, }; public static isValueValidForType(type: CustomParameterType, val: string): boolean { diff --git a/apps/server/src/modules/tool/external-tool/mapper/external-tool-request.mapper.ts b/apps/server/src/modules/tool/external-tool/mapper/external-tool-request.mapper.ts index 5c35aaef005..b2c2be7bbeb 100644 --- a/apps/server/src/modules/tool/external-tool/mapper/external-tool-request.mapper.ts +++ b/apps/server/src/modules/tool/external-tool/mapper/external-tool-request.mapper.ts @@ -57,7 +57,7 @@ const typeMapping: Record = { [CustomParameterTypeParams.AUTO_SCHOOLID]: CustomParameterType.AUTO_SCHOOLID, [CustomParameterTypeParams.AUTO_SCHOOLNUMBER]: CustomParameterType.AUTO_SCHOOLNUMBER, [CustomParameterTypeParams.AUTO_MEDIUMID]: CustomParameterType.AUTO_MEDIUMID, - [CustomParameterTypeParams.AUTO_MOINSCHULE_GROUPUUID]: CustomParameterType.AUTO_MOINSCHULE_GROUPUUID, + [CustomParameterTypeParams.AUTO_GROUPUUID]: CustomParameterType.AUTO_GROUPUUID, }; @Injectable() diff --git a/apps/server/src/modules/tool/external-tool/mapper/external-tool-response.mapper.ts b/apps/server/src/modules/tool/external-tool/mapper/external-tool-response.mapper.ts index 876fd0d33b5..e3c82d1d375 100644 --- a/apps/server/src/modules/tool/external-tool/mapper/external-tool-response.mapper.ts +++ b/apps/server/src/modules/tool/external-tool/mapper/external-tool-response.mapper.ts @@ -42,7 +42,7 @@ const typeMapping: Record = { [CustomParameterType.AUTO_SCHOOLID]: CustomParameterTypeParams.AUTO_SCHOOLID, [CustomParameterType.AUTO_SCHOOLNUMBER]: CustomParameterTypeParams.AUTO_SCHOOLNUMBER, [CustomParameterType.AUTO_MEDIUMID]: CustomParameterTypeParams.AUTO_MEDIUMID, - [CustomParameterType.AUTO_MOINSCHULE_GROUPUUID]: CustomParameterTypeParams.AUTO_MOINSCHULE_GROUPUUID, + [CustomParameterType.AUTO_GROUPUUID]: CustomParameterTypeParams.AUTO_GROUPUUID, }; @Injectable() diff --git a/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.ts b/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.ts index ae6c04c640e..f515c594013 100644 --- a/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.ts +++ b/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.ts @@ -45,9 +45,9 @@ export class ExternalToolParameterValidationService { ); } - if (!this.isAutoParameterMoinSchuleGroupUuidValid(param)) { + if (!this.isAutoParameterGroupUuidValid(param)) { throw new ValidationError( - `tool_param_auto_moin_schule_group_uuid: The custom parameter "${param.name}" with type "${param.type} must be optional."` + `tool_param_auto_group_uuid: The custom parameter "${param.name}" with type "${param.type} must be optional."` ); } @@ -172,8 +172,8 @@ export class ExternalToolParameterValidationService { return true; } - private isAutoParameterMoinSchuleGroupUuidValid(customParameter: CustomParameter) { - if (customParameter.type === CustomParameterType.AUTO_MOINSCHULE_GROUPUUID && !customParameter.isOptional) { + private isAutoParameterGroupUuidValid(customParameter: CustomParameter) { + if (customParameter.type === CustomParameterType.AUTO_GROUPUUID && !customParameter.isOptional) { return false; } diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-moin-schule-group-uuid.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.ts similarity index 89% rename from apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-moin-schule-group-uuid.strategy.ts rename to apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.ts index 63498b0979e..182d7fbe21b 100644 --- a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-moin-schule-group-uuid.strategy.ts +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.ts @@ -9,7 +9,7 @@ import { SchoolExternalTool } from '../../../school-external-tool/domain'; import { AutoParameterStrategy } from './auto-parameter.strategy'; @Injectable() -export class AutoMoinSchuleGroupUuidStrategy implements AutoParameterStrategy { +export class AutoGroupUuidStrategy implements AutoParameterStrategy { constructor(private readonly courseService: CourseService, private readonly groupService: GroupService) {} async getValue( @@ -28,13 +28,13 @@ export class AutoMoinSchuleGroupUuidStrategy implements AutoParameterStrategy { return undefined; } - const uuid = syncedGroup.getProps().externalSource?.externalId; - if (!uuid) { + const groupUuid = syncedGroup.externalSource?.externalId; + if (!groupUuid) { // TODO: think if a new special error is needed for this case throw new MissingToolParameterValueLoggableException(contextExternalTool, []); } - return uuid; + return groupUuid; } private async getSyncedGroup(course: Course): Promise { 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 index 780f72eab94..271693e3e1f 100644 --- 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 @@ -3,5 +3,5 @@ export * from './auto-school-id.strategy'; export * from './auto-context-id.strategy'; export * from './auto-context-name.strategy'; export * from './auto-school-number.strategy'; -export * from './auto-moin-schule-group-uuid.strategy'; +export * from './auto-group-uuid.strategy'; export { AutoMediumIdStrategy } from './auto-medium-id.strategy'; diff --git a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts index 42c4ea95249..0382d93ce1e 100644 --- a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts @@ -16,7 +16,7 @@ import { AutoParameterStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, - AutoMoinSchuleGroupUuidStrategy, + AutoGroupUuidStrategy, } from '../auto-parameter-strategy'; import { ToolLaunchParams } from './tool-launch-params.interface'; import { ToolLaunchStrategy } from './tool-launch-strategy.interface'; @@ -31,7 +31,7 @@ export abstract class AbstractLaunchStrategy implements ToolLaunchStrategy { autoContextIdStrategy: AutoContextIdStrategy, autoContextNameStrategy: AutoContextNameStrategy, autoMediumIdStrategy: AutoMediumIdStrategy, - autoMoinSchuleGroupUuidStrategy: AutoMoinSchuleGroupUuidStrategy + autoGroupUuidStrategy: AutoGroupUuidStrategy ) { this.autoParameterStrategyMap = new Map([ [CustomParameterType.AUTO_SCHOOLID, autoSchoolIdStrategy], @@ -39,7 +39,7 @@ export abstract class AbstractLaunchStrategy implements ToolLaunchStrategy { [CustomParameterType.AUTO_CONTEXTID, autoContextIdStrategy], [CustomParameterType.AUTO_CONTEXTNAME, autoContextNameStrategy], [CustomParameterType.AUTO_MEDIUMID, autoMediumIdStrategy], - [CustomParameterType.AUTO_MOINSCHULE_GROUPUUID, autoMoinSchuleGroupUuidStrategy], + [CustomParameterType.AUTO_GROUPUUID, autoGroupUuidStrategy], ]); } diff --git a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts index b054e00a600..94ccf4d8df9 100644 --- a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts @@ -16,7 +16,7 @@ import { AutoMediumIdStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, - AutoMoinSchuleGroupUuidStrategy, + AutoGroupUuidStrategy, } from '../auto-parameter-strategy'; import { Lti11EncryptionService } from '../lti11-encryption.service'; import { AbstractLaunchStrategy } from './abstract-launch.strategy'; @@ -33,7 +33,7 @@ export class Lti11ToolLaunchStrategy extends AbstractLaunchStrategy { autoContextIdStrategy: AutoContextIdStrategy, autoContextNameStrategy: AutoContextNameStrategy, autoMediumIdStrategy: AutoMediumIdStrategy, - autoMoinSchuleGroupUuidStrategy: AutoMoinSchuleGroupUuidStrategy + autoGroupUuidStrategy: AutoGroupUuidStrategy ) { super( autoSchoolIdStrategy, @@ -41,7 +41,7 @@ export class Lti11ToolLaunchStrategy extends AbstractLaunchStrategy { autoContextIdStrategy, autoContextNameStrategy, autoMediumIdStrategy, - autoMoinSchuleGroupUuidStrategy + autoGroupUuidStrategy ); } 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 3fe571378f9..adca4eb69b0 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 @@ -16,7 +16,7 @@ import { AutoMediumIdStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, - AutoMoinSchuleGroupUuidStrategy, + AutoGroupUuidStrategy, } from './service/auto-parameter-strategy'; import { BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrategy } from './service/launch-strategy'; @@ -44,7 +44,7 @@ import { BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrat AutoSchoolIdStrategy, AutoSchoolNumberStrategy, AutoMediumIdStrategy, - AutoMoinSchuleGroupUuidStrategy, + AutoGroupUuidStrategy, ], exports: [ToolLaunchService], }) From af8add5a424e03589781797f04d1961c32107da4 Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Fri, 2 Aug 2024 14:42:32 +0200 Subject: [PATCH 07/13] added new tests & adapted existing ones --- .../auto-group-uuid.strategy.spec.ts | 224 ++++++++++++++++++ .../abstract-launch.strategy.spec.ts | 26 +- .../basic-tool-launch.strategy.spec.ts | 5 + .../lti11-tool-launch.strategy.spec.ts | 5 + .../oauth2-tool-launch.strategy.spec.ts | 5 + 5 files changed, 263 insertions(+), 2 deletions(-) create mode 100644 apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts new file mode 100644 index 00000000000..50ef285d968 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts @@ -0,0 +1,224 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { CourseService } from '@modules/learnroom'; +import { Group, GroupService } from '@modules/group'; +import { courseFactory, groupEntityFactory, groupFactory, setupEntities } from '@shared/testing'; +import { ObjectId } from '@mikro-orm/mongodb'; +import { Course } from '@shared/domain/entity'; +import { GroupEntity } from '@modules/group/entity'; +import { ToolContextType } from '../../../common/enum'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { contextExternalToolFactory } from '../../../context-external-tool/testing'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { schoolExternalToolFactory } from '../../../school-external-tool/testing'; +import { AutoGroupUuidStrategy } from './auto-group-uuid.strategy'; + +describe(AutoGroupUuidStrategy.name, () => { + let module: TestingModule; + let strategy: AutoGroupUuidStrategy; + + let courseService: DeepMocked; + let groupService: DeepMocked; + + beforeAll(async () => { + await setupEntities(); + + module = await Test.createTestingModule({ + providers: [ + AutoGroupUuidStrategy, + { + provide: CourseService, + useValue: createMock(), + }, + { + provide: GroupService, + useValue: createMock(), + }, + ], + }).compile(); + + strategy = module.get(AutoGroupUuidStrategy); + courseService = module.get(CourseService); + groupService = module.get(GroupService); + }); + + afterAll(async () => { + await module.close(); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('getValue', () => { + describe('when the context is type course', () => { + const setupExternalTools = () => { + const courseId = new ObjectId().toHexString(); + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build(); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({ + contextRef: { + id: courseId, + type: ToolContextType.COURSE, + }, + }); + return { courseId, schoolExternalTool, contextExternalTool }; + }; + + describe('when the course is synced with a group that has an external ID', () => { + const setup = () => { + const { courseId, schoolExternalTool, contextExternalTool } = setupExternalTools(); + const groupEntity: GroupEntity = groupEntityFactory.buildWithId(); + const course: Course = courseFactory.buildWithId( + { + name: 'Synced Course', + syncedWithGroup: groupEntity, + }, + courseId + ); + + const groupDo: Group = groupFactory.build({ + id: groupEntity.id, + externalSource: { + externalId: groupEntity.externalSource?.externalId, + }, + }); + + courseService.findById.mockResolvedValue(course); + groupService.findById.mockResolvedValue(groupDo); + + return { + group: groupDo, + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should return the external ID from the synced group', async () => { + const { group, schoolExternalTool, contextExternalTool } = setup(); + + const result = await strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toEqual(group.externalSource?.externalId); + }); + }); + + describe('when the course is synced with a group that has no external ID', () => { + const setup = () => { + const { courseId, schoolExternalTool, contextExternalTool } = setupExternalTools(); + + const groupEntity: GroupEntity = groupEntityFactory.buildWithId(); + const course: Course = courseFactory.buildWithId( + { + name: 'Synced Course', + syncedWithGroup: groupEntity, + }, + courseId + ); + + const groupDo: Group = groupFactory.build({ + id: groupEntity.id, + externalSource: { + externalId: undefined, + }, + }); + + courseService.findById.mockResolvedValue(course); + groupService.findById.mockResolvedValue(groupDo); + + return { + group: groupDo, + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should throw an error', async () => { + const { schoolExternalTool, contextExternalTool } = setup(); + + await expect(strategy.getValue(schoolExternalTool, contextExternalTool)).rejects.toThrow(); + }); + }); + + describe('when the course is not synced with any group', () => { + const setup = () => { + const { courseId, schoolExternalTool, contextExternalTool } = setupExternalTools(); + + const course: Course = courseFactory.buildWithId( + { + name: 'Synced Course', + syncedWithGroup: undefined, + }, + courseId + ); + + courseService.findById.mockResolvedValue(course); + + return { + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should return undefined', async () => { + const { schoolExternalTool, contextExternalTool } = setup(); + + const result = await strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toBeUndefined(); + }); + }); + }); + + describe('when the context is type board element', () => { + const setup = () => { + const courseId = new ObjectId().toHexString(); + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build(); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({ + contextRef: { + id: courseId, + type: ToolContextType.BOARD_ELEMENT, + }, + }); + + return { + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should return undefined', async () => { + const { schoolExternalTool, contextExternalTool } = setup(); + + const result = await strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toBeUndefined(); + }); + }); + + describe('when the context is type media board', () => { + const setup = () => { + const courseId = new ObjectId().toHexString(); + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build(); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({ + contextRef: { + id: courseId, + type: ToolContextType.MEDIA_BOARD, + }, + }); + + return { + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should return undefined', async () => { + const { schoolExternalTool, contextExternalTool } = setup(); + + const result = await strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toBeUndefined(); + }); + }); + }); +}); diff --git a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.spec.ts index 843f11a511a..791ed1517ca 100644 --- a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.spec.ts @@ -26,6 +26,7 @@ import { AutoMediumIdStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, + AutoGroupUuidStrategy, } from '../auto-parameter-strategy'; import { AbstractLaunchStrategy } from './abstract-launch.strategy'; import { ToolLaunchParams } from './tool-launch-params.interface'; @@ -72,6 +73,7 @@ describe(AbstractLaunchStrategy.name, () => { let autoContextIdStrategy: DeepMocked; let autoContextNameStrategy: DeepMocked; let autoMediumIdStrategy: DeepMocked; + let autoGroupUuidStrategy: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -97,6 +99,10 @@ describe(AbstractLaunchStrategy.name, () => { provide: AutoMediumIdStrategy, useValue: createMock(), }, + { + provide: AutoGroupUuidStrategy, + useValue: createMock(), + }, ], }).compile(); @@ -107,6 +113,7 @@ describe(AbstractLaunchStrategy.name, () => { autoContextIdStrategy = module.get(AutoContextIdStrategy); autoContextNameStrategy = module.get(AutoContextNameStrategy); autoMediumIdStrategy = module.get(AutoMediumIdStrategy); + autoGroupUuidStrategy = module.get(AutoGroupUuidStrategy); }); afterAll(async () => { @@ -154,13 +161,13 @@ describe(AbstractLaunchStrategy.name, () => { const autoContextIdCustomParameter = customParameterFactory.build({ scope: CustomParameterScope.GLOBAL, location: CustomParameterLocation.BODY, - name: 'autoSchoolNumberParam', + name: 'autoContextIdParam', type: CustomParameterType.AUTO_CONTEXTID, }); const autoContextNameCustomParameter = customParameterFactory.build({ scope: CustomParameterScope.GLOBAL, location: CustomParameterLocation.BODY, - name: 'autoSchoolNumberParam', + name: 'autoContextNameParam', type: CustomParameterType.AUTO_CONTEXTNAME, }); const autoMediumIdCustomParameter = customParameterFactory.build({ @@ -169,6 +176,12 @@ describe(AbstractLaunchStrategy.name, () => { name: 'autoMediumIdParam', type: CustomParameterType.AUTO_MEDIUMID, }); + const autoGroupUuidCustomParameter = customParameterFactory.build({ + scope: CustomParameterScope.GLOBAL, + location: CustomParameterLocation.QUERY, + name: 'autoGroupUuidParam', + type: CustomParameterType.AUTO_GROUPUUID, + }); const externalTool: ExternalTool = externalToolFactory.build({ parameters: [ @@ -180,6 +193,7 @@ describe(AbstractLaunchStrategy.name, () => { autoContextIdCustomParameter, autoContextNameCustomParameter, autoMediumIdCustomParameter, + autoGroupUuidCustomParameter, ], }); @@ -217,6 +231,7 @@ describe(AbstractLaunchStrategy.name, () => { autoContextIdStrategy.getValue.mockReturnValueOnce(mockedAutoValue); autoContextNameStrategy.getValue.mockResolvedValueOnce(mockedAutoValue); autoMediumIdStrategy.getValue.mockResolvedValueOnce(mockedAutoValue); + autoGroupUuidStrategy.getValue.mockResolvedValueOnce(mockedAutoValue); return { globalCustomParameter, @@ -226,6 +241,7 @@ describe(AbstractLaunchStrategy.name, () => { autoContextIdCustomParameter, autoContextNameCustomParameter, autoMediumIdCustomParameter, + autoGroupUuidCustomParameter, schoolParameterEntry, contextParameterEntry, externalTool, @@ -246,6 +262,7 @@ describe(AbstractLaunchStrategy.name, () => { autoContextIdCustomParameter, autoContextNameCustomParameter, autoMediumIdCustomParameter, + autoGroupUuidCustomParameter, schoolParameterEntry, externalTool, schoolExternalTool, @@ -306,6 +323,11 @@ describe(AbstractLaunchStrategy.name, () => { value: mockedAutoValue, location: PropertyLocation.QUERY, }, + { + name: autoGroupUuidCustomParameter.name, + value: mockedAutoValue, + location: PropertyLocation.QUERY, + }, { name: concreteConfigParameter.name, value: concreteConfigParameter.value, diff --git a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.spec.ts index e6d3780b6a9..18f99218220 100644 --- a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.spec.ts @@ -13,6 +13,7 @@ import { AutoMediumIdStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, + AutoGroupUuidStrategy, } from '../auto-parameter-strategy'; import { BasicToolLaunchStrategy } from './basic-tool-launch.strategy'; import { ToolLaunchParams } from './tool-launch-params.interface'; @@ -45,6 +46,10 @@ describe('BasicToolLaunchStrategy', () => { provide: AutoMediumIdStrategy, useValue: createMock(), }, + { + provide: AutoGroupUuidStrategy, + useValue: createMock(), + }, ], }).compile(); diff --git a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.spec.ts index 39464dbafd3..76befd90204 100644 --- a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.spec.ts @@ -23,6 +23,7 @@ import { AutoMediumIdStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, + AutoGroupUuidStrategy, } from '../auto-parameter-strategy'; import { Lti11EncryptionService } from '../lti11-encryption.service'; import { Lti11ToolLaunchStrategy } from './lti11-tool-launch.strategy'; @@ -72,6 +73,10 @@ describe('Lti11ToolLaunchStrategy', () => { provide: AutoMediumIdStrategy, useValue: createMock(), }, + { + provide: AutoGroupUuidStrategy, + useValue: createMock(), + }, ], }).compile(); diff --git a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.spec.ts index 608ef0597ab..553ca287949 100644 --- a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.spec.ts @@ -13,6 +13,7 @@ import { AutoMediumIdStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, + AutoGroupUuidStrategy, } from '../auto-parameter-strategy'; import { OAuth2ToolLaunchStrategy } from './oauth2-tool-launch.strategy'; import { ToolLaunchParams } from './tool-launch-params.interface'; @@ -45,6 +46,10 @@ describe('OAuth2ToolLaunchStrategy', () => { provide: AutoMediumIdStrategy, useValue: createMock(), }, + { + provide: AutoGroupUuidStrategy, + useValue: createMock(), + }, ], }).compile(); From cdf72314d98a8aea28ae572023cd87b8694d23cb Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Fri, 2 Aug 2024 15:36:27 +0200 Subject: [PATCH 08/13] N21-1825 added new tests for create external tool validator --- ...ool-parameter-type-validation.util.spec.ts | 11 ++++ ...-tool-parameter-validation.service.spec.ts | 64 +++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.spec.ts b/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.spec.ts index 79cba0dd459..e44867be4fb 100644 --- a/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.spec.ts +++ b/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.spec.ts @@ -106,5 +106,16 @@ describe(ToolParameterTypeValidationUtil.name, () => { expect(result).toEqual(false); }); }); + + describe('when the type is AUTO_GROUPUUID', () => { + it('should return false', () => { + const result: boolean = ToolParameterTypeValidationUtil.isValueValidForType( + CustomParameterType.AUTO_GROUPUUID, + 'any value' + ); + + expect(result).toEqual(false); + }); + }); }); }); diff --git a/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.spec.ts b/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.spec.ts index a5f6c50f4cd..4885e84f333 100644 --- a/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.spec.ts +++ b/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.spec.ts @@ -415,5 +415,69 @@ describe('ExternalToolParameterValidationService', () => { }); }); }); + + describe('when auto parameter is auto group uuid', () => { + describe('when the optional flag is not set', () => { + const setup = () => { + const parameter = customParameterFactory.buildWithId({ + type: CustomParameterType.AUTO_GROUPUUID, + scope: CustomParameterScope.GLOBAL, + isOptional: false, + }); + + const externalTool: ExternalTool = externalToolFactory.withMedium().buildWithId({ + parameters: [parameter], + }); + + externalToolService.findExternalToolByName.mockResolvedValue(externalTool); + + return { + externalTool, + parameter, + }; + }; + + it('should throw an exception', async () => { + const { externalTool, parameter } = setup(); + + const result: Promise = service.validateCommon(externalTool); + + await expect(result).rejects.toThrow( + new ValidationError( + `tool_param_auto_group_uuid: The custom parameter "${parameter.name}" with type "${parameter.type} must be optional."` + ) + ); + }); + }); + + describe('when the optional flag is set', () => { + const setup = () => { + const parameter = customParameterFactory.buildWithId({ + type: CustomParameterType.AUTO_GROUPUUID, + scope: CustomParameterScope.GLOBAL, + isOptional: true, + }); + + const externalTool: ExternalTool = externalToolFactory.withMedium().buildWithId({ + parameters: [parameter], + }); + + externalToolService.findExternalToolByName.mockResolvedValue(externalTool); + + return { + externalTool, + parameter, + }; + }; + + it('should not throw an exception', async () => { + const { externalTool } = setup(); + + const result: Promise = service.validateCommon(externalTool); + + await expect(result).resolves.not.toThrow(); + }); + }); + }); }); }); From 6bcd135165b1b7ce75c52bc820a51cad852c1241 Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Fri, 2 Aug 2024 16:27:21 +0200 Subject: [PATCH 09/13] N21-1825 added special error for when auto param value cannot be filled --- .../modules/tool/tool-launch/error/index.ts | 1 + ...parameter-value.loggable-exception.spec.ts | 40 +++++++++++++++++++ ...auto-parameter-value.loggable-exception.ts | 38 ++++++++++++++++++ .../auto-group-uuid.strategy.spec.ts | 7 +++- .../auto-group-uuid.strategy.ts | 11 +++-- 5 files changed, 89 insertions(+), 8 deletions(-) create mode 100644 apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.spec.ts create mode 100644 apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.ts diff --git a/apps/server/src/modules/tool/tool-launch/error/index.ts b/apps/server/src/modules/tool/tool-launch/error/index.ts index f57f583e432..b19465a77b8 100644 --- a/apps/server/src/modules/tool/tool-launch/error/index.ts +++ b/apps/server/src/modules/tool/tool-launch/error/index.ts @@ -3,3 +3,4 @@ export * from './missing-tool-parameter-value.loggable-exception'; export * from './parameter-type-not-implemented.loggable-exception'; export { MissingMediaLicenseLoggableException } from './missing-licence.loggable-exception'; export { LaunchContextUnavailableLoggableException } from './launch-context-unavailable.loggable-exception'; +export { MissingAutoParameterValueLoggableException } from './missing-auto-parameter-value.loggable-exception'; diff --git a/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.spec.ts b/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.spec.ts new file mode 100644 index 00000000000..0634ca8797d --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.spec.ts @@ -0,0 +1,40 @@ +import { contextExternalToolFactory } from '../../context-external-tool/testing'; +import { ContextExternalTool } from '../../context-external-tool/domain'; +import { CustomParameterType } from '../../common/enum'; +import { MissingAutoParameterValueLoggableException } from './missing-auto-parameter-value.loggable-exception'; + +describe(MissingAutoParameterValueLoggableException.name, () => { + describe('getLogMessage', () => { + const setup = () => { + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build(); + + const parameterType: CustomParameterType = CustomParameterType.AUTO_GROUPUUID; + + const exception = new MissingAutoParameterValueLoggableException(contextExternalTool, parameterType); + + return { + contextExternalTool, + parameterType, + exception, + }; + }; + + it('should give the correct log message', () => { + const { contextExternalTool, parameterType, exception } = setup(); + + const result = exception.getLogMessage(); + + expect(result).toEqual({ + type: 'MISSING_AUTO_PARAMETER_VALUE', + message: + 'The external tool was attempted to launch, but the value to fill an auto parameter was not found ' + + 'or could not be retrieved successfully', + stack: expect.any(String), + data: { + contextExternalToolId: contextExternalTool.id, + parameterType: parameterType, + }, + }); + }); + }); +}); diff --git a/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.ts b/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.ts new file mode 100644 index 00000000000..f3ab33ea52e --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.ts @@ -0,0 +1,38 @@ +import { HttpStatus } from '@nestjs/common'; +import { BusinessError } from '@shared/common'; +import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger'; +import { ContextExternalToolLaunchable } from '../../context-external-tool/domain'; +import { CustomParameterType } from '../../common/enum'; + +export class MissingAutoParameterValueLoggableException extends BusinessError implements Loggable { + constructor( + private readonly contextExternalTool: ContextExternalToolLaunchable, + private readonly parameterType: CustomParameterType + ) { + super( + { + type: 'MISSING_AUTO_PARAMETER_VALUE', + title: 'Missing auto parameter value', + defaultMessage: + 'The external tool was attempted to launch, but the value to fill an auto parameter was not found ' + + 'or could not be retrieved successfully', + }, + HttpStatus.UNPROCESSABLE_ENTITY, + { + parameterType: parameterType, + } + ); + } + + getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage { + return { + type: this.type, + message: this.message, + stack: this.stack, + data: { + contextExternalToolId: this.contextExternalTool.id, + parameterType: this.parameterType, + }, + }; + } +} diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts index 50ef285d968..012da95f8cf 100644 --- a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts @@ -6,11 +6,12 @@ import { courseFactory, groupEntityFactory, groupFactory, setupEntities } from ' import { ObjectId } from '@mikro-orm/mongodb'; import { Course } from '@shared/domain/entity'; import { GroupEntity } from '@modules/group/entity'; -import { ToolContextType } from '../../../common/enum'; +import { CustomParameterType, ToolContextType } from '../../../common/enum'; import { ContextExternalTool } from '../../../context-external-tool/domain'; import { contextExternalToolFactory } from '../../../context-external-tool/testing'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; import { schoolExternalToolFactory } from '../../../school-external-tool/testing'; +import { MissingAutoParameterValueLoggableException } from '../../error'; import { AutoGroupUuidStrategy } from './auto-group-uuid.strategy'; describe(AutoGroupUuidStrategy.name, () => { @@ -135,7 +136,9 @@ describe(AutoGroupUuidStrategy.name, () => { it('should throw an error', async () => { const { schoolExternalTool, contextExternalTool } = setup(); - await expect(strategy.getValue(schoolExternalTool, contextExternalTool)).rejects.toThrow(); + await expect(strategy.getValue(schoolExternalTool, contextExternalTool)).rejects.toThrow( + new MissingAutoParameterValueLoggableException(contextExternalTool, CustomParameterType.AUTO_GROUPUUID) + ); }); }); diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.ts index 182d7fbe21b..9b28e115093 100644 --- a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.ts +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.ts @@ -2,8 +2,8 @@ import { Injectable } from '@nestjs/common'; import { CourseService } from '@modules/learnroom'; import { Group, GroupService } from '@modules/group'; import { Course } from '@shared/domain/entity'; -import { ToolContextType } from '../../../common/enum'; -import { MissingToolParameterValueLoggableException } from '../../error'; +import { CustomParameterType, ToolContextType } from '../../../common/enum'; +import { MissingAutoParameterValueLoggableException } from '../../error'; import { ContextExternalToolLaunchable } from '../../../context-external-tool/domain'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; import { AutoParameterStrategy } from './auto-parameter.strategy'; @@ -21,17 +21,16 @@ export class AutoGroupUuidStrategy implements AutoParameterStrategy { } const courseId = contextExternalTool.contextRef.id; - const course = await this.courseService.findById(courseId); + const course: Course = await this.courseService.findById(courseId); - const syncedGroup = await this.getSyncedGroup(course); + const syncedGroup: Group | undefined = await this.getSyncedGroup(course); if (!syncedGroup) { return undefined; } const groupUuid = syncedGroup.externalSource?.externalId; if (!groupUuid) { - // TODO: think if a new special error is needed for this case - throw new MissingToolParameterValueLoggableException(contextExternalTool, []); + throw new MissingAutoParameterValueLoggableException(contextExternalTool, CustomParameterType.AUTO_GROUPUUID); } return groupUuid; From 8ef509b117f7a295f1fb54a35f6e7983ebcd2661 Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Mon, 5 Aug 2024 09:12:39 +0200 Subject: [PATCH 10/13] N21-1825 reorder imports, fixed eslint errors --- .../missing-auto-parameter-value.loggable-exception.spec.ts | 2 +- .../missing-auto-parameter-value.loggable-exception.ts | 2 +- .../auto-group-uuid.strategy.spec.ts | 6 +++--- .../auto-parameter-strategy/auto-group-uuid.strategy.ts | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.spec.ts b/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.spec.ts index 0634ca8797d..6b2e2efc861 100644 --- a/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.spec.ts @@ -32,7 +32,7 @@ describe(MissingAutoParameterValueLoggableException.name, () => { stack: expect.any(String), data: { contextExternalToolId: contextExternalTool.id, - parameterType: parameterType, + parameterType, }, }); }); diff --git a/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.ts b/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.ts index f3ab33ea52e..d144643e2f2 100644 --- a/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.ts +++ b/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.ts @@ -19,7 +19,7 @@ export class MissingAutoParameterValueLoggableException extends BusinessError im }, HttpStatus.UNPROCESSABLE_ENTITY, { - parameterType: parameterType, + parameterType, } ); } diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts index 012da95f8cf..29807aa5090 100644 --- a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts @@ -1,11 +1,11 @@ import { Test, TestingModule } from '@nestjs/testing'; import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { CourseService } from '@modules/learnroom'; -import { Group, GroupService } from '@modules/group'; -import { courseFactory, groupEntityFactory, groupFactory, setupEntities } from '@shared/testing'; import { ObjectId } from '@mikro-orm/mongodb'; +import { CourseService } from '@modules/learnroom'; import { Course } from '@shared/domain/entity'; +import { Group, GroupService } from '@modules/group'; import { GroupEntity } from '@modules/group/entity'; +import { courseFactory, groupEntityFactory, groupFactory, setupEntities } from '@shared/testing'; import { CustomParameterType, ToolContextType } from '../../../common/enum'; import { ContextExternalTool } from '../../../context-external-tool/domain'; import { contextExternalToolFactory } from '../../../context-external-tool/testing'; diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.ts index 9b28e115093..102681a96fe 100644 --- a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.ts +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.ts @@ -3,9 +3,9 @@ import { CourseService } from '@modules/learnroom'; import { Group, GroupService } from '@modules/group'; import { Course } from '@shared/domain/entity'; import { CustomParameterType, ToolContextType } from '../../../common/enum'; -import { MissingAutoParameterValueLoggableException } from '../../error'; import { ContextExternalToolLaunchable } from '../../../context-external-tool/domain'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { MissingAutoParameterValueLoggableException } from '../../error'; import { AutoParameterStrategy } from './auto-parameter.strategy'; @Injectable() From 199b7406ad9145283104f400f6141cd73ac9b28a Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Mon, 5 Aug 2024 12:08:19 +0200 Subject: [PATCH 11/13] N21-1825 re-allow non-optional auto group uuid tool --- ...-tool-parameter-validation.service.spec.ts | 64 ------------------- ...ernal-tool-parameter-validation.service.ts | 14 ---- 2 files changed, 78 deletions(-) diff --git a/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.spec.ts b/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.spec.ts index 4885e84f333..a5f6c50f4cd 100644 --- a/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.spec.ts +++ b/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.spec.ts @@ -415,69 +415,5 @@ describe('ExternalToolParameterValidationService', () => { }); }); }); - - describe('when auto parameter is auto group uuid', () => { - describe('when the optional flag is not set', () => { - const setup = () => { - const parameter = customParameterFactory.buildWithId({ - type: CustomParameterType.AUTO_GROUPUUID, - scope: CustomParameterScope.GLOBAL, - isOptional: false, - }); - - const externalTool: ExternalTool = externalToolFactory.withMedium().buildWithId({ - parameters: [parameter], - }); - - externalToolService.findExternalToolByName.mockResolvedValue(externalTool); - - return { - externalTool, - parameter, - }; - }; - - it('should throw an exception', async () => { - const { externalTool, parameter } = setup(); - - const result: Promise = service.validateCommon(externalTool); - - await expect(result).rejects.toThrow( - new ValidationError( - `tool_param_auto_group_uuid: The custom parameter "${parameter.name}" with type "${parameter.type} must be optional."` - ) - ); - }); - }); - - describe('when the optional flag is set', () => { - const setup = () => { - const parameter = customParameterFactory.buildWithId({ - type: CustomParameterType.AUTO_GROUPUUID, - scope: CustomParameterScope.GLOBAL, - isOptional: true, - }); - - const externalTool: ExternalTool = externalToolFactory.withMedium().buildWithId({ - parameters: [parameter], - }); - - externalToolService.findExternalToolByName.mockResolvedValue(externalTool); - - return { - externalTool, - parameter, - }; - }; - - it('should not throw an exception', async () => { - const { externalTool } = setup(); - - const result: Promise = service.validateCommon(externalTool); - - await expect(result).resolves.not.toThrow(); - }); - }); - }); }); }); diff --git a/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.ts b/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.ts index f515c594013..b7ed1709209 100644 --- a/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.ts +++ b/apps/server/src/modules/tool/external-tool/service/external-tool-parameter-validation.service.ts @@ -45,12 +45,6 @@ export class ExternalToolParameterValidationService { ); } - if (!this.isAutoParameterGroupUuidValid(param)) { - throw new ValidationError( - `tool_param_auto_group_uuid: The custom parameter "${param.name}" with type "${param.type} must be optional."` - ); - } - if (!this.isRegexCommentMandatoryAndFilled(param)) { throw new ValidationError( `tool_param_regexComment: The custom parameter "${param.name}" parameter is missing a regex comment.` @@ -171,12 +165,4 @@ export class ExternalToolParameterValidationService { return true; } - - private isAutoParameterGroupUuidValid(customParameter: CustomParameter) { - if (customParameter.type === CustomParameterType.AUTO_GROUPUUID && !customParameter.isOptional) { - return false; - } - - return true; - } } From b8c0a42cb58933bc46756a086c7dab28ef1b1497 Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Mon, 5 Aug 2024 12:40:10 +0200 Subject: [PATCH 12/13] N21-1825 renamed auto_groupuuid -> auto_group_externaluuid --- .../common/enum/custom-parameter-type.enum.ts | 4 +-- .../custom-parameter-type.enum.ts | 2 +- ...ool-parameter-type-validation.util.spec.ts | 2 +- .../tool-parameter-type-validation.util.ts | 2 +- .../mapper/external-tool-request.mapper.ts | 2 +- .../mapper/external-tool-response.mapper.ts | 2 +- ...parameter-value.loggable-exception.spec.ts | 2 +- ...o-group-external-uuid-strategy.service.ts} | 7 +++-- .../auto-group-uuid.strategy.spec.ts | 15 ++++++----- .../service/auto-parameter-strategy/index.ts | 2 +- .../abstract-launch.strategy.spec.ts | 26 +++++++++---------- .../abstract-launch.strategy.ts | 6 ++--- .../basic-tool-launch.strategy.spec.ts | 6 ++--- .../lti11-tool-launch.strategy.spec.ts | 6 ++--- .../lti11-tool-launch.strategy.ts | 6 ++--- .../oauth2-tool-launch.strategy.spec.ts | 6 ++--- .../tool/tool-launch/tool-launch.module.ts | 4 +-- 17 files changed, 53 insertions(+), 47 deletions(-) rename apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/{auto-group-uuid.strategy.ts => auto-group-external-uuid-strategy.service.ts} (88%) diff --git a/apps/server/src/modules/tool/common/enum/custom-parameter-type.enum.ts b/apps/server/src/modules/tool/common/enum/custom-parameter-type.enum.ts index a8c0d7b1e26..d3aa29c8258 100644 --- a/apps/server/src/modules/tool/common/enum/custom-parameter-type.enum.ts +++ b/apps/server/src/modules/tool/common/enum/custom-parameter-type.enum.ts @@ -7,7 +7,7 @@ export enum CustomParameterType { AUTO_SCHOOLID = 'auto_schoolid', AUTO_SCHOOLNUMBER = 'auto_schoolnumber', AUTO_MEDIUMID = 'auto_mediumid', - AUTO_GROUPUUID = 'auto_groupuuid', + AUTO_GROUP_EXTERNALUUID = 'auto_group_externaluuid', } export const autoParameters: CustomParameterType[] = [ @@ -16,5 +16,5 @@ export const autoParameters: CustomParameterType[] = [ CustomParameterType.AUTO_SCHOOLID, CustomParameterType.AUTO_SCHOOLNUMBER, CustomParameterType.AUTO_MEDIUMID, - CustomParameterType.AUTO_GROUPUUID, + CustomParameterType.AUTO_GROUP_EXTERNALUUID, ]; diff --git a/apps/server/src/modules/tool/common/enum/request-response/custom-parameter-type.enum.ts b/apps/server/src/modules/tool/common/enum/request-response/custom-parameter-type.enum.ts index 346b74169ed..2f166134562 100644 --- a/apps/server/src/modules/tool/common/enum/request-response/custom-parameter-type.enum.ts +++ b/apps/server/src/modules/tool/common/enum/request-response/custom-parameter-type.enum.ts @@ -7,5 +7,5 @@ export enum CustomParameterTypeParams { AUTO_SCHOOLID = 'auto_schoolid', AUTO_SCHOOLNUMBER = 'auto_schoolnumber', AUTO_MEDIUMID = 'auto_mediumid', - AUTO_GROUPUUID = 'auto_groupuuid', + AUTO_GROUP_EXTERNALUUID = 'auto_group_externaluuid', } diff --git a/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.spec.ts b/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.spec.ts index e44867be4fb..6cea79213fe 100644 --- a/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.spec.ts +++ b/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.spec.ts @@ -110,7 +110,7 @@ describe(ToolParameterTypeValidationUtil.name, () => { describe('when the type is AUTO_GROUPUUID', () => { it('should return false', () => { const result: boolean = ToolParameterTypeValidationUtil.isValueValidForType( - CustomParameterType.AUTO_GROUPUUID, + CustomParameterType.AUTO_GROUP_EXTERNALUUID, 'any value' ); diff --git a/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.ts b/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.ts index f0cd88263aa..ad046f0ba3c 100644 --- a/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.ts +++ b/apps/server/src/modules/tool/common/service/validation/tool-parameter-type-validation.util.ts @@ -11,7 +11,7 @@ export class ToolParameterTypeValidationUtil { [CustomParameterType.AUTO_SCHOOLID]: () => false, [CustomParameterType.AUTO_SCHOOLNUMBER]: () => false, [CustomParameterType.AUTO_MEDIUMID]: () => false, - [CustomParameterType.AUTO_GROUPUUID]: () => false, + [CustomParameterType.AUTO_GROUP_EXTERNALUUID]: () => false, }; public static isValueValidForType(type: CustomParameterType, val: string): boolean { diff --git a/apps/server/src/modules/tool/external-tool/mapper/external-tool-request.mapper.ts b/apps/server/src/modules/tool/external-tool/mapper/external-tool-request.mapper.ts index b2c2be7bbeb..5805ee736e6 100644 --- a/apps/server/src/modules/tool/external-tool/mapper/external-tool-request.mapper.ts +++ b/apps/server/src/modules/tool/external-tool/mapper/external-tool-request.mapper.ts @@ -57,7 +57,7 @@ const typeMapping: Record = { [CustomParameterTypeParams.AUTO_SCHOOLID]: CustomParameterType.AUTO_SCHOOLID, [CustomParameterTypeParams.AUTO_SCHOOLNUMBER]: CustomParameterType.AUTO_SCHOOLNUMBER, [CustomParameterTypeParams.AUTO_MEDIUMID]: CustomParameterType.AUTO_MEDIUMID, - [CustomParameterTypeParams.AUTO_GROUPUUID]: CustomParameterType.AUTO_GROUPUUID, + [CustomParameterTypeParams.AUTO_GROUP_EXTERNALUUID]: CustomParameterType.AUTO_GROUP_EXTERNALUUID, }; @Injectable() diff --git a/apps/server/src/modules/tool/external-tool/mapper/external-tool-response.mapper.ts b/apps/server/src/modules/tool/external-tool/mapper/external-tool-response.mapper.ts index e3c82d1d375..13a68d0e834 100644 --- a/apps/server/src/modules/tool/external-tool/mapper/external-tool-response.mapper.ts +++ b/apps/server/src/modules/tool/external-tool/mapper/external-tool-response.mapper.ts @@ -42,7 +42,7 @@ const typeMapping: Record = { [CustomParameterType.AUTO_SCHOOLID]: CustomParameterTypeParams.AUTO_SCHOOLID, [CustomParameterType.AUTO_SCHOOLNUMBER]: CustomParameterTypeParams.AUTO_SCHOOLNUMBER, [CustomParameterType.AUTO_MEDIUMID]: CustomParameterTypeParams.AUTO_MEDIUMID, - [CustomParameterType.AUTO_GROUPUUID]: CustomParameterTypeParams.AUTO_GROUPUUID, + [CustomParameterType.AUTO_GROUP_EXTERNALUUID]: CustomParameterTypeParams.AUTO_GROUP_EXTERNALUUID, }; @Injectable() diff --git a/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.spec.ts b/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.spec.ts index 6b2e2efc861..9141d7a7a09 100644 --- a/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.spec.ts @@ -8,7 +8,7 @@ describe(MissingAutoParameterValueLoggableException.name, () => { const setup = () => { const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build(); - const parameterType: CustomParameterType = CustomParameterType.AUTO_GROUPUUID; + const parameterType: CustomParameterType = CustomParameterType.AUTO_GROUP_EXTERNALUUID; const exception = new MissingAutoParameterValueLoggableException(contextExternalTool, parameterType); diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-external-uuid-strategy.service.ts similarity index 88% rename from apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.ts rename to apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-external-uuid-strategy.service.ts index 102681a96fe..1c45f52ac83 100644 --- a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.ts +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-external-uuid-strategy.service.ts @@ -9,7 +9,7 @@ import { MissingAutoParameterValueLoggableException } from '../../error'; import { AutoParameterStrategy } from './auto-parameter.strategy'; @Injectable() -export class AutoGroupUuidStrategy implements AutoParameterStrategy { +export class AutoGroupExternalUuidStrategy implements AutoParameterStrategy { constructor(private readonly courseService: CourseService, private readonly groupService: GroupService) {} async getValue( @@ -30,7 +30,10 @@ export class AutoGroupUuidStrategy implements AutoParameterStrategy { const groupUuid = syncedGroup.externalSource?.externalId; if (!groupUuid) { - throw new MissingAutoParameterValueLoggableException(contextExternalTool, CustomParameterType.AUTO_GROUPUUID); + throw new MissingAutoParameterValueLoggableException( + contextExternalTool, + CustomParameterType.AUTO_GROUP_EXTERNALUUID + ); } return groupUuid; diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts index 29807aa5090..2c37368f0d1 100644 --- a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts @@ -12,11 +12,11 @@ import { contextExternalToolFactory } from '../../../context-external-tool/testi import { SchoolExternalTool } from '../../../school-external-tool/domain'; import { schoolExternalToolFactory } from '../../../school-external-tool/testing'; import { MissingAutoParameterValueLoggableException } from '../../error'; -import { AutoGroupUuidStrategy } from './auto-group-uuid.strategy'; +import { AutoGroupExternalUuidStrategy } from './auto-group-external-uuid-strategy.service'; -describe(AutoGroupUuidStrategy.name, () => { +describe(AutoGroupExternalUuidStrategy.name, () => { let module: TestingModule; - let strategy: AutoGroupUuidStrategy; + let strategy: AutoGroupExternalUuidStrategy; let courseService: DeepMocked; let groupService: DeepMocked; @@ -26,7 +26,7 @@ describe(AutoGroupUuidStrategy.name, () => { module = await Test.createTestingModule({ providers: [ - AutoGroupUuidStrategy, + AutoGroupExternalUuidStrategy, { provide: CourseService, useValue: createMock(), @@ -38,7 +38,7 @@ describe(AutoGroupUuidStrategy.name, () => { ], }).compile(); - strategy = module.get(AutoGroupUuidStrategy); + strategy = module.get(AutoGroupExternalUuidStrategy); courseService = module.get(CourseService); groupService = module.get(GroupService); }); @@ -137,7 +137,10 @@ describe(AutoGroupUuidStrategy.name, () => { const { schoolExternalTool, contextExternalTool } = setup(); await expect(strategy.getValue(schoolExternalTool, contextExternalTool)).rejects.toThrow( - new MissingAutoParameterValueLoggableException(contextExternalTool, CustomParameterType.AUTO_GROUPUUID) + new MissingAutoParameterValueLoggableException( + contextExternalTool, + CustomParameterType.AUTO_GROUP_EXTERNALUUID + ) ); }); }); 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 index 271693e3e1f..d647726b1a7 100644 --- 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 @@ -3,5 +3,5 @@ export * from './auto-school-id.strategy'; export * from './auto-context-id.strategy'; export * from './auto-context-name.strategy'; export * from './auto-school-number.strategy'; -export * from './auto-group-uuid.strategy'; +export * from './auto-group-external-uuid-strategy.service'; export { AutoMediumIdStrategy } from './auto-medium-id.strategy'; diff --git a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.spec.ts index 791ed1517ca..2bb4a05d7b0 100644 --- a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.spec.ts @@ -26,7 +26,7 @@ import { AutoMediumIdStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, - AutoGroupUuidStrategy, + AutoGroupExternalUuidStrategy, } from '../auto-parameter-strategy'; import { AbstractLaunchStrategy } from './abstract-launch.strategy'; import { ToolLaunchParams } from './tool-launch-params.interface'; @@ -73,7 +73,7 @@ describe(AbstractLaunchStrategy.name, () => { let autoContextIdStrategy: DeepMocked; let autoContextNameStrategy: DeepMocked; let autoMediumIdStrategy: DeepMocked; - let autoGroupUuidStrategy: DeepMocked; + let autoGroupExternalUuidStrategy: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -100,8 +100,8 @@ describe(AbstractLaunchStrategy.name, () => { useValue: createMock(), }, { - provide: AutoGroupUuidStrategy, - useValue: createMock(), + provide: AutoGroupExternalUuidStrategy, + useValue: createMock(), }, ], }).compile(); @@ -113,7 +113,7 @@ describe(AbstractLaunchStrategy.name, () => { autoContextIdStrategy = module.get(AutoContextIdStrategy); autoContextNameStrategy = module.get(AutoContextNameStrategy); autoMediumIdStrategy = module.get(AutoMediumIdStrategy); - autoGroupUuidStrategy = module.get(AutoGroupUuidStrategy); + autoGroupExternalUuidStrategy = module.get(AutoGroupExternalUuidStrategy); }); afterAll(async () => { @@ -176,11 +176,11 @@ describe(AbstractLaunchStrategy.name, () => { name: 'autoMediumIdParam', type: CustomParameterType.AUTO_MEDIUMID, }); - const autoGroupUuidCustomParameter = customParameterFactory.build({ + const autoGroupExternalUuidCustomParameter = customParameterFactory.build({ scope: CustomParameterScope.GLOBAL, location: CustomParameterLocation.QUERY, - name: 'autoGroupUuidParam', - type: CustomParameterType.AUTO_GROUPUUID, + name: 'autoGroupExternalUuidParam', + type: CustomParameterType.AUTO_GROUP_EXTERNALUUID, }); const externalTool: ExternalTool = externalToolFactory.build({ @@ -193,7 +193,7 @@ describe(AbstractLaunchStrategy.name, () => { autoContextIdCustomParameter, autoContextNameCustomParameter, autoMediumIdCustomParameter, - autoGroupUuidCustomParameter, + autoGroupExternalUuidCustomParameter, ], }); @@ -231,7 +231,7 @@ describe(AbstractLaunchStrategy.name, () => { autoContextIdStrategy.getValue.mockReturnValueOnce(mockedAutoValue); autoContextNameStrategy.getValue.mockResolvedValueOnce(mockedAutoValue); autoMediumIdStrategy.getValue.mockResolvedValueOnce(mockedAutoValue); - autoGroupUuidStrategy.getValue.mockResolvedValueOnce(mockedAutoValue); + autoGroupExternalUuidStrategy.getValue.mockResolvedValueOnce(mockedAutoValue); return { globalCustomParameter, @@ -241,7 +241,7 @@ describe(AbstractLaunchStrategy.name, () => { autoContextIdCustomParameter, autoContextNameCustomParameter, autoMediumIdCustomParameter, - autoGroupUuidCustomParameter, + autoGroupExternalUuidCustomParameter, schoolParameterEntry, contextParameterEntry, externalTool, @@ -262,7 +262,7 @@ describe(AbstractLaunchStrategy.name, () => { autoContextIdCustomParameter, autoContextNameCustomParameter, autoMediumIdCustomParameter, - autoGroupUuidCustomParameter, + autoGroupExternalUuidCustomParameter, schoolParameterEntry, externalTool, schoolExternalTool, @@ -324,7 +324,7 @@ describe(AbstractLaunchStrategy.name, () => { location: PropertyLocation.QUERY, }, { - name: autoGroupUuidCustomParameter.name, + name: autoGroupExternalUuidCustomParameter.name, value: mockedAutoValue, location: PropertyLocation.QUERY, }, diff --git a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts index 0382d93ce1e..ca5f5535571 100644 --- a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts @@ -16,7 +16,7 @@ import { AutoParameterStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, - AutoGroupUuidStrategy, + AutoGroupExternalUuidStrategy, } from '../auto-parameter-strategy'; import { ToolLaunchParams } from './tool-launch-params.interface'; import { ToolLaunchStrategy } from './tool-launch-strategy.interface'; @@ -31,7 +31,7 @@ export abstract class AbstractLaunchStrategy implements ToolLaunchStrategy { autoContextIdStrategy: AutoContextIdStrategy, autoContextNameStrategy: AutoContextNameStrategy, autoMediumIdStrategy: AutoMediumIdStrategy, - autoGroupUuidStrategy: AutoGroupUuidStrategy + autoGroupExternalUuidStrategy: AutoGroupExternalUuidStrategy ) { this.autoParameterStrategyMap = new Map([ [CustomParameterType.AUTO_SCHOOLID, autoSchoolIdStrategy], @@ -39,7 +39,7 @@ export abstract class AbstractLaunchStrategy implements ToolLaunchStrategy { [CustomParameterType.AUTO_CONTEXTID, autoContextIdStrategy], [CustomParameterType.AUTO_CONTEXTNAME, autoContextNameStrategy], [CustomParameterType.AUTO_MEDIUMID, autoMediumIdStrategy], - [CustomParameterType.AUTO_GROUPUUID, autoGroupUuidStrategy], + [CustomParameterType.AUTO_GROUP_EXTERNALUUID, autoGroupExternalUuidStrategy], ]); } diff --git a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.spec.ts index 18f99218220..db7bda486d0 100644 --- a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.spec.ts @@ -13,7 +13,7 @@ import { AutoMediumIdStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, - AutoGroupUuidStrategy, + AutoGroupExternalUuidStrategy, } from '../auto-parameter-strategy'; import { BasicToolLaunchStrategy } from './basic-tool-launch.strategy'; import { ToolLaunchParams } from './tool-launch-params.interface'; @@ -47,8 +47,8 @@ describe('BasicToolLaunchStrategy', () => { useValue: createMock(), }, { - provide: AutoGroupUuidStrategy, - useValue: createMock(), + provide: AutoGroupExternalUuidStrategy, + useValue: createMock(), }, ], }).compile(); diff --git a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.spec.ts index 76befd90204..05fecd75d56 100644 --- a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.spec.ts @@ -23,7 +23,7 @@ import { AutoMediumIdStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, - AutoGroupUuidStrategy, + AutoGroupExternalUuidStrategy, } from '../auto-parameter-strategy'; import { Lti11EncryptionService } from '../lti11-encryption.service'; import { Lti11ToolLaunchStrategy } from './lti11-tool-launch.strategy'; @@ -74,8 +74,8 @@ describe('Lti11ToolLaunchStrategy', () => { useValue: createMock(), }, { - provide: AutoGroupUuidStrategy, - useValue: createMock(), + provide: AutoGroupExternalUuidStrategy, + useValue: createMock(), }, ], }).compile(); diff --git a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts index 94ccf4d8df9..948df83c295 100644 --- a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts @@ -16,7 +16,7 @@ import { AutoMediumIdStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, - AutoGroupUuidStrategy, + AutoGroupExternalUuidStrategy, } from '../auto-parameter-strategy'; import { Lti11EncryptionService } from '../lti11-encryption.service'; import { AbstractLaunchStrategy } from './abstract-launch.strategy'; @@ -33,7 +33,7 @@ export class Lti11ToolLaunchStrategy extends AbstractLaunchStrategy { autoContextIdStrategy: AutoContextIdStrategy, autoContextNameStrategy: AutoContextNameStrategy, autoMediumIdStrategy: AutoMediumIdStrategy, - autoGroupUuidStrategy: AutoGroupUuidStrategy + autoGroupExternalUuidStrategy: AutoGroupExternalUuidStrategy ) { super( autoSchoolIdStrategy, @@ -41,7 +41,7 @@ export class Lti11ToolLaunchStrategy extends AbstractLaunchStrategy { autoContextIdStrategy, autoContextNameStrategy, autoMediumIdStrategy, - autoGroupUuidStrategy + autoGroupExternalUuidStrategy ); } diff --git a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.spec.ts index 553ca287949..dcda4d88a86 100644 --- a/apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.spec.ts @@ -13,7 +13,7 @@ import { AutoMediumIdStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, - AutoGroupUuidStrategy, + AutoGroupExternalUuidStrategy, } from '../auto-parameter-strategy'; import { OAuth2ToolLaunchStrategy } from './oauth2-tool-launch.strategy'; import { ToolLaunchParams } from './tool-launch-params.interface'; @@ -47,8 +47,8 @@ describe('OAuth2ToolLaunchStrategy', () => { useValue: createMock(), }, { - provide: AutoGroupUuidStrategy, - useValue: createMock(), + provide: AutoGroupExternalUuidStrategy, + useValue: createMock(), }, ], }).compile(); 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 adca4eb69b0..15344278bcd 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 @@ -16,7 +16,7 @@ import { AutoMediumIdStrategy, AutoSchoolIdStrategy, AutoSchoolNumberStrategy, - AutoGroupUuidStrategy, + AutoGroupExternalUuidStrategy, } from './service/auto-parameter-strategy'; import { BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrategy } from './service/launch-strategy'; @@ -44,7 +44,7 @@ import { BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrat AutoSchoolIdStrategy, AutoSchoolNumberStrategy, AutoMediumIdStrategy, - AutoGroupUuidStrategy, + AutoGroupExternalUuidStrategy, ], exports: [ToolLaunchService], }) From 566b93f2154767b2bcb401dd0682ade32d927c18 Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Tue, 6 Aug 2024 13:56:04 +0200 Subject: [PATCH 13/13] N21-1825 return undefined when no external id found --- .../modules/tool/tool-launch/error/index.ts | 1 - ...parameter-value.loggable-exception.spec.ts | 40 ------------------- ...auto-parameter-value.loggable-exception.ts | 38 ------------------ ...to-group-external-uuid-strategy.service.ts | 8 +--- .../auto-group-uuid.strategy.spec.ts | 14 +++---- 5 files changed, 7 insertions(+), 94 deletions(-) delete mode 100644 apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.spec.ts delete mode 100644 apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.ts diff --git a/apps/server/src/modules/tool/tool-launch/error/index.ts b/apps/server/src/modules/tool/tool-launch/error/index.ts index b19465a77b8..f57f583e432 100644 --- a/apps/server/src/modules/tool/tool-launch/error/index.ts +++ b/apps/server/src/modules/tool/tool-launch/error/index.ts @@ -3,4 +3,3 @@ export * from './missing-tool-parameter-value.loggable-exception'; export * from './parameter-type-not-implemented.loggable-exception'; export { MissingMediaLicenseLoggableException } from './missing-licence.loggable-exception'; export { LaunchContextUnavailableLoggableException } from './launch-context-unavailable.loggable-exception'; -export { MissingAutoParameterValueLoggableException } from './missing-auto-parameter-value.loggable-exception'; diff --git a/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.spec.ts b/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.spec.ts deleted file mode 100644 index 9141d7a7a09..00000000000 --- a/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.spec.ts +++ /dev/null @@ -1,40 +0,0 @@ -import { contextExternalToolFactory } from '../../context-external-tool/testing'; -import { ContextExternalTool } from '../../context-external-tool/domain'; -import { CustomParameterType } from '../../common/enum'; -import { MissingAutoParameterValueLoggableException } from './missing-auto-parameter-value.loggable-exception'; - -describe(MissingAutoParameterValueLoggableException.name, () => { - describe('getLogMessage', () => { - const setup = () => { - const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build(); - - const parameterType: CustomParameterType = CustomParameterType.AUTO_GROUP_EXTERNALUUID; - - const exception = new MissingAutoParameterValueLoggableException(contextExternalTool, parameterType); - - return { - contextExternalTool, - parameterType, - exception, - }; - }; - - it('should give the correct log message', () => { - const { contextExternalTool, parameterType, exception } = setup(); - - const result = exception.getLogMessage(); - - expect(result).toEqual({ - type: 'MISSING_AUTO_PARAMETER_VALUE', - message: - 'The external tool was attempted to launch, but the value to fill an auto parameter was not found ' + - 'or could not be retrieved successfully', - stack: expect.any(String), - data: { - contextExternalToolId: contextExternalTool.id, - parameterType, - }, - }); - }); - }); -}); diff --git a/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.ts b/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.ts deleted file mode 100644 index d144643e2f2..00000000000 --- a/apps/server/src/modules/tool/tool-launch/error/missing-auto-parameter-value.loggable-exception.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { HttpStatus } from '@nestjs/common'; -import { BusinessError } from '@shared/common'; -import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger'; -import { ContextExternalToolLaunchable } from '../../context-external-tool/domain'; -import { CustomParameterType } from '../../common/enum'; - -export class MissingAutoParameterValueLoggableException extends BusinessError implements Loggable { - constructor( - private readonly contextExternalTool: ContextExternalToolLaunchable, - private readonly parameterType: CustomParameterType - ) { - super( - { - type: 'MISSING_AUTO_PARAMETER_VALUE', - title: 'Missing auto parameter value', - defaultMessage: - 'The external tool was attempted to launch, but the value to fill an auto parameter was not found ' + - 'or could not be retrieved successfully', - }, - HttpStatus.UNPROCESSABLE_ENTITY, - { - parameterType, - } - ); - } - - getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage { - return { - type: this.type, - message: this.message, - stack: this.stack, - data: { - contextExternalToolId: this.contextExternalTool.id, - parameterType: this.parameterType, - }, - }; - } -} diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-external-uuid-strategy.service.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-external-uuid-strategy.service.ts index 1c45f52ac83..6d3de082343 100644 --- a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-external-uuid-strategy.service.ts +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-external-uuid-strategy.service.ts @@ -2,10 +2,9 @@ import { Injectable } from '@nestjs/common'; import { CourseService } from '@modules/learnroom'; import { Group, GroupService } from '@modules/group'; import { Course } from '@shared/domain/entity'; -import { CustomParameterType, ToolContextType } from '../../../common/enum'; +import { ToolContextType } from '../../../common/enum'; import { ContextExternalToolLaunchable } from '../../../context-external-tool/domain'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; -import { MissingAutoParameterValueLoggableException } from '../../error'; import { AutoParameterStrategy } from './auto-parameter.strategy'; @Injectable() @@ -30,10 +29,7 @@ export class AutoGroupExternalUuidStrategy implements AutoParameterStrategy { const groupUuid = syncedGroup.externalSource?.externalId; if (!groupUuid) { - throw new MissingAutoParameterValueLoggableException( - contextExternalTool, - CustomParameterType.AUTO_GROUP_EXTERNALUUID - ); + return undefined; } return groupUuid; diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts index 2c37368f0d1..ffae6e12e7a 100644 --- a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-group-uuid.strategy.spec.ts @@ -6,12 +6,11 @@ import { Course } from '@shared/domain/entity'; import { Group, GroupService } from '@modules/group'; import { GroupEntity } from '@modules/group/entity'; import { courseFactory, groupEntityFactory, groupFactory, setupEntities } from '@shared/testing'; -import { CustomParameterType, ToolContextType } from '../../../common/enum'; +import { ToolContextType } from '../../../common/enum'; import { ContextExternalTool } from '../../../context-external-tool/domain'; import { contextExternalToolFactory } from '../../../context-external-tool/testing'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; import { schoolExternalToolFactory } from '../../../school-external-tool/testing'; -import { MissingAutoParameterValueLoggableException } from '../../error'; import { AutoGroupExternalUuidStrategy } from './auto-group-external-uuid-strategy.service'; describe(AutoGroupExternalUuidStrategy.name, () => { @@ -133,15 +132,12 @@ describe(AutoGroupExternalUuidStrategy.name, () => { }; }; - it('should throw an error', async () => { + it('should return undefined', async () => { const { schoolExternalTool, contextExternalTool } = setup(); - await expect(strategy.getValue(schoolExternalTool, contextExternalTool)).rejects.toThrow( - new MissingAutoParameterValueLoggableException( - contextExternalTool, - CustomParameterType.AUTO_GROUP_EXTERNALUUID - ) - ); + const result = await strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toBeUndefined(); }); });