N21-1803 Refactorings from Code Review 6

apps/server/src/modules/board/service/board-do-copy-service/recursive-copy.visitor.ts

@@ -247,39 +247,40 @@ export class RecursiveCopyVisitor implements BoardCompositeVisitorAsync {
 	}
 
 	async visitExternalToolElementAsync(original: ExternalToolElement): Promise<void> {
-		let status: CopyStatusEnum = CopyStatusEnum.SUCCESS;
-
-		const copy = new ExternalToolElement({
+		const boardElementCopy: ExternalToolElement = new ExternalToolElement({
 			id: new ObjectId().toHexString(),
 			contextExternalToolId: undefined,
 			children: [],
 			createdAt: new Date(),
 			updatedAt: new Date(),
 		});
 
+		let status: CopyStatusEnum;
 		if (this.toolFeatures.ctlToolsCopyEnabled && original.contextExternalToolId) {
-			const tool: ContextExternalTool | null = await this.contextExternalToolService.findById(
+			const linkedTool: ContextExternalTool | null = await this.contextExternalToolService.findById(
 				original.contextExternalToolId
 			);
 
-			if (tool) {
-				const copiedTool: ContextExternalTool = await this.contextExternalToolService.copyContextExternalTool(
-					tool,
-					copy.id
-				);
+			if (linkedTool) {
+				const contextExternalToolCopy: ContextExternalTool =
+					await this.contextExternalToolService.copyContextExternalTool(linkedTool, boardElementCopy.id);
+
+				boardElementCopy.contextExternalToolId = contextExternalToolCopy.id;
 
-				copy.contextExternalToolId = copiedTool.id;
+				status = CopyStatusEnum.SUCCESS;
 			} else {
 				status = CopyStatusEnum.FAIL;
 			}
+		} else {
+			status = CopyStatusEnum.SUCCESS;
 		}
 
 		this.resultMap.set(original.id, {
-			copyEntity: copy,
+			copyEntity: boardElementCopy,
 			type: CopyElementType.EXTERNAL_TOOL_ELEMENT,
 			status,
 		});
-		this.copyMap.set(original.id, copy);
+		this.copyMap.set(original.id, boardElementCopy);
 
 		return Promise.resolve();
 	}

apps/server/src/modules/board/service/media-board/media-available-line.service.spec.ts

@@ -287,14 +287,8 @@ describe(MediaAvailableLineService.name, () => {
 
 			service.createMediaAvailableLine(availableExternalTools);
 
-			expect(externalToolLogoService.buildLogoUrl).toHaveBeenCalledWith(
-				'/v3/tools/external-tools/{id}/logo',
-				externalTool1
-			);
-			expect(externalToolLogoService.buildLogoUrl).toHaveBeenCalledWith(
-				'/v3/tools/external-tools/{id}/logo',
-				externalTool2
-			);
+			expect(externalToolLogoService.buildLogoUrl).toHaveBeenCalledWith(externalTool1);
+			expect(externalToolLogoService.buildLogoUrl).toHaveBeenCalledWith(externalTool2);
 		});
 
 		it('should create a media available line with correct elements', () => {

apps/server/src/modules/board/service/media-board/media-available-line.service.ts

@@ -134,10 +134,7 @@ export class MediaAvailableLineService {
 		externalTool: ExternalTool,
 		schoolExternalTool: SchoolExternalTool
 	): MediaAvailableLineElement {
-		const logoUrl: string | undefined = this.externalToolLogoService.buildLogoUrl(
-			'/v3/tools/external-tools/{id}/logo',
-			externalTool
-		);
+		const logoUrl: string | undefined = this.externalToolLogoService.buildLogoUrl(externalTool);
 
 		const element: MediaAvailableLineElement = new MediaAvailableLineElement({
 			schoolExternalToolId: schoolExternalTool.id ?? '',

apps/server/src/modules/group/entity/group-user.entity.ts → apps/server/src/modules/group/entity/group-user.embeddable.ts

@@ -9,7 +9,7 @@ export interface GroupUserEntityProps {
 }
 
 @Embeddable()
-export class GroupUserEntity {
+export class GroupUserEmbeddable {
 	@ManyToOne(() => User)
 	user: User;
 

apps/server/src/modules/group/entity/group-valid-period.entity.ts → apps/server/src/modules/group/entity/group-valid-period.embeddable.ts

@@ -7,7 +7,7 @@ export interface GroupValidPeriodEntityProps {
 }
 
 @Embeddable()
-export class GroupValidPeriodEntity {
+export class GroupValidPeriodEmbeddable {
 	@Property()
 	from: Date;
 

apps/server/src/modules/group/entity/group.entity.ts

@@ -1,11 +1,11 @@
 import { Collection, Embedded, Entity, Enum, ManyToOne, OneToMany, Property } from '@mikro-orm/core';
 import { Course as CourseEntity } from '@shared/domain/entity';
 import { BaseEntityWithTimestamps } from '@shared/domain/entity/base.entity';
-import { ExternalSourceEntity } from '@shared/domain/entity/external-source.entity';
+import { ExternalSourceEmbeddable } from '@shared/domain/entity/external-source.embeddable';
 import { SchoolEntity } from '@shared/domain/entity/school.entity';
 import { EntityId } from '@shared/domain/types';
-import { GroupUserEntity } from './group-user.entity';
-import { GroupValidPeriodEntity } from './group-valid-period.entity';
+import { GroupUserEmbeddable } from './group-user.embeddable';
+import { GroupValidPeriodEmbeddable } from './group-valid-period.embeddable';
 
 export enum GroupEntityTypes {
 	CLASS = 'class',
@@ -20,11 +20,11 @@ export interface GroupEntityProps {
 
 	type: GroupEntityTypes;
 
-	externalSource?: ExternalSourceEntity;
+	externalSource?: ExternalSourceEmbeddable;
 
-	validPeriod?: GroupValidPeriodEntity;
+	validPeriod?: GroupValidPeriodEmbeddable;
 
-	users: GroupUserEntity[];
+	users: GroupUserEmbeddable[];
 
 	organization?: SchoolEntity;
 }
@@ -37,14 +37,14 @@ export class GroupEntity extends BaseEntityWithTimestamps {
 	@Enum()
 	type: GroupEntityTypes;
 
-	@Embedded(() => ExternalSourceEntity, { nullable: true })
-	externalSource?: ExternalSourceEntity;
+	@Embedded(() => ExternalSourceEmbeddable, { nullable: true })
+	externalSource?: ExternalSourceEmbeddable;
 
-	@Embedded(() => GroupValidPeriodEntity, { nullable: true })
-	validPeriod?: GroupValidPeriodEntity;
+	@Embedded(() => GroupValidPeriodEmbeddable, { nullable: true })
+	validPeriod?: GroupValidPeriodEmbeddable;
 
-	@Embedded(() => GroupUserEntity, { array: true })
-	users: GroupUserEntity[];
+	@Embedded(() => GroupUserEmbeddable, { array: true })
+	users: GroupUserEmbeddable[];
 
 	@ManyToOne(() => SchoolEntity, { nullable: true })
 	organization?: SchoolEntity;

apps/server/src/modules/group/entity/index.ts

@@ -1,3 +1,3 @@
 export * from './group.entity';
-export * from './group-user.entity';
-export * from './group-valid-period.entity';
+export * from './group-user.embeddable';
+export * from './group-valid-period.embeddable';

apps/server/src/modules/group/repo/group-domain.mapper.ts

@@ -1,9 +1,9 @@
 import { EntityData } from '@mikro-orm/core';
 import { EntityManager } from '@mikro-orm/mongodb';
 import { ExternalSource } from '@shared/domain/domainobject';
-import { ExternalSourceEntity, Role, SchoolEntity, SystemEntity, User } from '@shared/domain/entity';
+import { ExternalSourceEmbeddable, Role, SchoolEntity, SystemEntity, User } from '@shared/domain/entity';
 import { Group, GroupProps, GroupTypes, GroupUser } from '../domain';
-import { GroupEntity, GroupEntityTypes, GroupUserEntity, GroupValidPeriodEntity } from '../entity';
+import { GroupEntity, GroupEntityTypes, GroupUserEmbeddable, GroupValidPeriodEmbeddable } from '../entity';
 
 const GroupEntityTypesToGroupTypesMapping: Record<GroupEntityTypes, GroupTypes> = {
 	[GroupEntityTypes.CLASS]: GroupTypes.CLASS,
@@ -21,9 +21,9 @@ export class GroupDomainMapper {
 	static mapDoToEntityData(group: Group, em: EntityManager): EntityData<GroupEntity> {
 		const props: GroupProps = group.getProps();
 
-		let validPeriod: GroupValidPeriodEntity | undefined;
+		let validPeriod: GroupValidPeriodEmbeddable | undefined;
 		if (props.validFrom && props.validUntil) {
-			validPeriod = new GroupValidPeriodEntity({
+			validPeriod = new GroupValidPeriodEmbeddable({
 				from: props.validFrom,
 				until: props.validUntil,
 			});
@@ -36,7 +36,7 @@ export class GroupDomainMapper {
 				? this.mapExternalSourceToExternalSourceEntity(props.externalSource, em)
 				: undefined,
 			users: props.users.map(
-				(groupUser): GroupUserEntity => GroupDomainMapper.mapGroupUserToGroupUserEntity(groupUser, em)
+				(groupUser): GroupUserEmbeddable => GroupDomainMapper.mapGroupUserToGroupUserEntity(groupUser, em)
 			),
 			validPeriod,
 			organization: props.organizationId ? em.getReference(SchoolEntity, props.organizationId) : undefined,
@@ -65,16 +65,16 @@ export class GroupDomainMapper {
 	static mapExternalSourceToExternalSourceEntity(
 		externalSource: ExternalSource,
 		em: EntityManager
-	): ExternalSourceEntity {
-		const externalSourceEntity: ExternalSourceEntity = new ExternalSourceEntity({
+	): ExternalSourceEmbeddable {
+		const externalSourceEntity: ExternalSourceEmbeddable = new ExternalSourceEmbeddable({
 			externalId: externalSource.externalId,
 			system: em.getReference(SystemEntity, externalSource.systemId),
 		});
 
 		return externalSourceEntity;
 	}
 
-	static mapExternalSourceEntityToExternalSource(entity: ExternalSourceEntity): ExternalSource {
+	static mapExternalSourceEntityToExternalSource(entity: ExternalSourceEmbeddable): ExternalSource {
 		const externalSource: ExternalSource = new ExternalSource({
 			externalId: entity.externalId,
 			systemId: entity.system.id,
@@ -83,16 +83,16 @@ export class GroupDomainMapper {
 		return externalSource;
 	}
 
-	static mapGroupUserToGroupUserEntity(groupUser: GroupUser, em: EntityManager): GroupUserEntity {
-		const groupUserEntity: GroupUserEntity = new GroupUserEntity({
+	static mapGroupUserToGroupUserEntity(groupUser: GroupUser, em: EntityManager): GroupUserEmbeddable {
+		const groupUserEntity: GroupUserEmbeddable = new GroupUserEmbeddable({
 			user: em.getReference(User, groupUser.userId),
 			role: em.getReference(Role, groupUser.roleId),
 		});
 
 		return groupUserEntity;
 	}
 
-	static mapGroupUserEntityToGroupUser(entity: GroupUserEntity): GroupUser {
+	static mapGroupUserEntityToGroupUser(entity: GroupUserEmbeddable): GroupUser {
 		const groupUser: GroupUser = new GroupUser({
 			userId: entity.user.id,
 			roleId: entity.role.id,

apps/server/src/modules/group/repo/group.repo.spec.ts

@@ -17,7 +17,7 @@ import {
 	userFactory,
 } from '@shared/testing';
 import { Group, GroupProps, GroupTypes, GroupUser } from '../domain';
-import { GroupEntity, GroupEntityTypes, GroupUserEntity } from '../entity';
+import { GroupEntity, GroupEntityTypes, GroupUserEmbeddable } from '../entity';
 import { GroupRepo } from './group.repo';
 
 describe('GroupRepo', () => {
@@ -216,7 +216,7 @@ describe('GroupRepo', () => {
 			const setup = async () => {
 				const userEntity: User = userFactory.buildWithId();
 				const user: UserDO = userDoFactory.build({ id: userEntity.id });
-				const groupUserEntity: GroupUserEntity = new GroupUserEntity({
+				const groupUserEntity: GroupUserEmbeddable = new GroupUserEmbeddable({
 					user: userEntity,
 					role: roleFactory.buildWithId(),
 				});

apps/server/src/modules/tool/context-external-tool/domain/error/index.ts

@@ -0,0 +1 @@
+export { RestrictedContextMismatchLoggableException } from './restricted-context-mismatch-loggabble';

apps/server/src/modules/tool/context-external-tool/service/restricted-context-mismatch-loggabble.spec.ts → apps/server/src/modules/tool/context-external-tool/domain/error/restricted-context-mismatch-loggabble.spec.ts

@@ -1,12 +1,12 @@
-import { ToolContextType } from '../../common/enum';
-import { RestrictedContextMismatchLoggable } from './restricted-context-mismatch-loggabble';
+import { ToolContextType } from '../../../common/enum';
+import { RestrictedContextMismatchLoggableException } from './restricted-context-mismatch-loggabble';
 
 describe('RestrictedContextMismatchLoggable', () => {
 	describe('getLogMessage', () => {
 		const setup = () => {
 			const externalToolName = 'name';
 			const context: ToolContextType = ToolContextType.COURSE;
-			const loggable = new RestrictedContextMismatchLoggable(externalToolName, context);
+			const loggable = new RestrictedContextMismatchLoggableException(externalToolName, context);
 
 			return { loggable, externalToolName, context };
 		};

apps/server/src/modules/tool/context-external-tool/service/restricted-context-mismatch-loggabble.ts → apps/server/src/modules/tool/context-external-tool/domain/error/restricted-context-mismatch-loggabble.ts

@@ -1,9 +1,8 @@
 import { UnprocessableEntityException } from '@nestjs/common';
-import { Loggable } from '@src/core/logger/interfaces';
-import { ErrorLogMessage, LogMessage, ValidationErrorLogMessage } from '@src/core/logger';
-import { ToolContextType } from '../../common/enum';
+import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger';
+import { ToolContextType } from '../../../common/enum';
 
-export class RestrictedContextMismatchLoggable extends UnprocessableEntityException implements Loggable {
+export class RestrictedContextMismatchLoggableException extends UnprocessableEntityException implements Loggable {
 	constructor(private readonly externalToolName: string, private readonly context: ToolContextType) {
 		super();
 	}

apps/server/src/modules/tool/context-external-tool/domain/index.ts

@@ -1,3 +1,4 @@
 export * from './context-external-tool.do';
 export * from './context-ref';
 export * from './tool-reference';
+export { RestrictedContextMismatchLoggableException } from './error';

apps/server/src/modules/tool/context-external-tool/service/context-external-tool.service.spec.ts

@@ -1,6 +1,9 @@
 import { createMock, DeepMocked } from '@golevelup/ts-jest';
+import { ObjectId } from '@mikro-orm/mongodb';
+import { AuthorizationContext, AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization';
 import { NotFoundException } from '@nestjs/common';
 import { Test, TestingModule } from '@nestjs/testing';
+import { Permission } from '@shared/domain/interface';
 import { ContextExternalToolRepo } from '@shared/repo';
 import {
 	contextExternalToolFactory,
@@ -9,19 +12,15 @@ import {
 	legacySchoolDoFactory,
 	schoolExternalToolFactory,
 } from '@shared/testing/factory/domainobject';
-import { AuthorizationContext, AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization';
-import { ObjectId } from '@mikro-orm/mongodb';
-import { Permission } from '@shared/domain/interface';
+import { CustomParameter } from '../../common/domain';
 import { ToolContextType } from '../../common/enum';
-import { SchoolExternalTool, SchoolExternalToolWithId } from '../../school-external-tool/domain';
-import { ContextExternalTool, ContextRef } from '../domain';
-import { ContextExternalToolService } from './context-external-tool.service';
-import { ExternalTool } from '../../external-tool/domain';
+import { CommonToolService } from '../../common/service';
 import { ExternalToolService } from '../../external-tool';
+import { ExternalTool } from '../../external-tool/domain';
 import { SchoolExternalToolService } from '../../school-external-tool';
-import { RestrictedContextMismatchLoggable } from './restricted-context-mismatch-loggabble';
-import { CommonToolService } from '../../common/service';
-import { CustomParameter } from '../../common/domain';
+import { SchoolExternalTool, SchoolExternalToolWithId } from '../../school-external-tool/domain';
+import { ContextExternalTool, ContextRef, RestrictedContextMismatchLoggableException } from '../domain';
+import { ContextExternalToolService } from './context-external-tool.service';
 
 describe('ContextExternalToolService', () => {
 	let module: TestingModule;
@@ -397,7 +396,7 @@ describe('ContextExternalToolService', () => {
 				const { contextExternalTool, externalTool } = setup();
 
 				await expect(service.checkContextRestrictions(contextExternalTool)).rejects.toThrow(
-					new RestrictedContextMismatchLoggable(externalTool.name, contextExternalTool.contextRef.type)
+					new RestrictedContextMismatchLoggableException(externalTool.name, contextExternalTool.contextRef.type)
 				);
 			});
 		});

apps/server/src/modules/tool/context-external-tool/service/context-external-tool.service.ts

@@ -7,9 +7,13 @@ import { ExternalTool } from '../../external-tool/domain';
 import { ExternalToolService } from '../../external-tool/service';
 import { SchoolExternalTool } from '../../school-external-tool/domain';
 import { SchoolExternalToolService } from '../../school-external-tool/service';
-import { ContextExternalTool, ContextExternalToolWithId, ContextRef } from '../domain';
+import {
+	ContextExternalTool,
+	ContextExternalToolWithId,
+	ContextRef,
+	RestrictedContextMismatchLoggableException,
+} from '../domain';
 import { ContextExternalToolQuery } from '../uc/dto/context-external-tool.types';
-import { RestrictedContextMismatchLoggable } from './restricted-context-mismatch-loggabble';
 
 @Injectable()
 export class ContextExternalToolService {
@@ -75,7 +79,7 @@ export class ContextExternalToolService {
 		const externalTool: ExternalTool = await this.externalToolService.findById(schoolExternalTool.toolId);
 
 		if (this.commonToolService.isContextRestricted(externalTool, contextExternalTool.contextRef.type)) {
-			throw new RestrictedContextMismatchLoggable(externalTool.name, contextExternalTool.contextRef.type);
+			throw new RestrictedContextMismatchLoggableException(externalTool.name, contextExternalTool.contextRef.type);
 		}
 	}
 

apps/server/src/modules/tool/context-external-tool/service/index.ts

@@ -1,3 +1,4 @@
 export * from './context-external-tool.service';
 export * from './context-external-tool-authorizable.service';
 export * from './tool-reference.service';
+export { ToolConfigurationStatusService } from './tool-configuration-status.service';

apps/server/src/modules/tool/context-external-tool/service/tool-reference.service.spec.ts

@@ -12,8 +12,8 @@ import { SchoolExternalToolService } from '../../school-external-tool';
 import { SchoolExternalToolWithId } from '../../school-external-tool/domain';
 import { ToolReference } from '../domain';
 import { ContextExternalToolService } from './context-external-tool.service';
-import { ToolReferenceService } from './tool-reference.service';
 import { ToolConfigurationStatusService } from './tool-configuration-status.service';
+import { ToolReferenceService } from './tool-reference.service';
 
 describe('ToolReferenceService', () => {
 	let module: TestingModule;
@@ -118,10 +118,7 @@ describe('ToolReferenceService', () => {
 
 				await service.getToolReference(contextExternalToolId);
 
-				expect(externalToolLogoService.buildLogoUrl).toHaveBeenCalledWith(
-					'/v3/tools/external-tools/{id}/logo',
-					externalTool
-				);
+				expect(externalToolLogoService.buildLogoUrl).toHaveBeenCalledWith(externalTool);
 			});
 
 			it('should return the tool reference', async () => {

apps/server/src/modules/tool/context-external-tool/service/tool-reference.service.ts

@@ -40,10 +40,7 @@ export class ToolReferenceService {
 			contextExternalTool,
 			status
 		);
-		toolReference.logoUrl = this.externalToolLogoService.buildLogoUrl(
-			'/v3/tools/external-tools/{id}/logo',
-			externalTool
-		);
+		toolReference.logoUrl = this.externalToolLogoService.buildLogoUrl(externalTool);
 
 		return toolReference;
 	}