Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

N21-1996 Remove dynamic values from school external tool persistence #5084

Merged
merged 12 commits into from
Jul 3, 2024
Merged
38 changes: 38 additions & 0 deletions apps/server/src/migrations/mikro-orm/Migration20240627134214.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { Migration } from '@mikro-orm/migrations-mongodb';

export class Migration20240627134214 extends Migration {
async up(): Promise<void> {
await this.driver.aggregate('school-external-tools', [
{ $set: { isDeactivated: { $ifNull: ['$status.isDeactivated', false] } } },
{ $unset: 'status' },
{ $out: 'school-external-tools' },
]);

console.info(`'status.isDeactivated' has moved to 'isDeactivated' for all school-external-tools`);
}

async down(): Promise<void> {
await this.driver.nativeUpdate(
'school-external-tools',
{ isDeactivated: true },
{
$set: {
status: {
isOutdatedOnScopeSchool: false,
isDeactivated: true,
},
},
}
);

await this.driver.nativeUpdate(
'school-external-tools',
{},
{
$unset: { isDeactivated: '' },
}
);

console.info(`All school-external-tools were reverted to using 'status'`);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -147,14 +147,15 @@ describe(SchulconnexToolProvisioningService.name, () => {

await service.provisionSchoolExternalTools(userId, schoolId, systemId);

expect(schoolExternalToolService.saveSchoolExternalTool).toHaveBeenCalledWith({
props: {
expect(schoolExternalToolService.saveSchoolExternalTool).toHaveBeenCalledWith<[SchoolExternalTool]>(
new SchoolExternalTool({
id: expect.any(String),
toolId: externalTool.id,
isDeactivated: false,
schoolId,
parameters: [],
},
});
})
);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export class SchulconnexToolProvisioningService {
id: new ObjectId().toHexString(),
toolId: externalTool.id,
schoolId,
isDeactivated: false,
parameters: [],
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ describe(ToolConfigurationStatusService.name, () => {
const externalTool = externalToolFactory.buildWithId();
const schoolExternalTool = schoolExternalToolFactory.buildWithId({
toolId: externalTool.id,
status: { isDeactivated: true },
isDeactivated: true,
});
const contextExternalTool = contextExternalToolFactory
.withSchoolExternalToolRef(schoolExternalTool.id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export class ToolConfigurationStatusService {
}

private isToolDeactivated(externalTool: ExternalTool, schoolExternalTool: SchoolExternalTool): boolean {
return !!(externalTool.isDeactivated || (schoolExternalTool.status && schoolExternalTool.status.isDeactivated));
return externalTool.isDeactivated || schoolExternalTool.isDeactivated;
}

private async isToolLicensed(externalTool: ExternalTool, userId: EntityId): Promise<boolean> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe(ExternalToolDatasheetMapper.name, () => {
restrictToContexts: [ToolContextType.COURSE, ToolContextType.BOARD_ELEMENT],
});
const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build({
status: { isDeactivated: true },
isDeactivated: true,
});
const expectDatasheet: ExternalToolDatasheetTemplateData = externalToolDatasheetTemplateDataFactory
.withOptionalProperties()
Expand Down Expand Up @@ -68,7 +68,7 @@ describe(ExternalToolDatasheetMapper.name, () => {
const school: School = schoolFactory.build();
const externalTool = externalToolFactory.build();
const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build({
status: { isDeactivated: true },
isDeactivated: true,
});
const expectDatasheet: ExternalToolDatasheetTemplateData = externalToolDatasheetTemplateDataFactory.build({
instance: 'dBildungscloud',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export class ExternalToolDatasheetMapper {
return 'Das Tool ist instanzweit deaktiviert';
}

if (schoolExternalTool?.status?.isDeactivated) {
if (schoolExternalTool?.isDeactivated) {
return 'Das Tool ist deaktiviert';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,12 @@ describe('ExternalToolConfigurationService', () => {
availableSchoolExternalTools.forEach((tool): void => {
if (tool.id === 'deactivatedToolId') {
tool.status = schoolExternalToolConfigurationStatusFactory.build({
isDeactivated: true,
isGloballyDeactivated: true,
isOutdatedOnScopeSchool: false,
});
}
tool.status = schoolExternalToolConfigurationStatusFactory.build({
isDeactivated: false,
isGloballyDeactivated: false,
isOutdatedOnScopeSchool: false,
});
});
Expand Down Expand Up @@ -175,9 +175,7 @@ describe('ExternalToolConfigurationService', () => {
);

expect(
result.every(
(toolInfo: ContextExternalToolTemplateInfo) => !toolInfo.schoolExternalTool.status?.isDeactivated
)
result.every((toolInfo: ContextExternalToolTemplateInfo) => !toolInfo.schoolExternalTool.isDeactivated)
).toBe(true);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class ExternalToolConfigurationService {
const availableTools: ContextExternalToolTemplateInfo[] = unusedTools
.filter((toolRef): toolRef is ContextExternalToolTemplateInfo => !toolRef.externalTool.isHidden)
.filter((toolRef) => !toolRef.externalTool.isDeactivated)
.filter((toolRef) => !toolRef.schoolExternalTool.status?.isDeactivated);
.filter((toolRef) => !toolRef.schoolExternalTool.isDeactivated);

return availableTools;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { ContextExternalToolRepo, ExternalToolRepo, SchoolExternalToolRepo } fro
import { LegacyLogger } from '@src/core/logger';
import { ExternalToolSearchQuery } from '../../common/interface';
import { SchoolExternalTool } from '../../school-external-tool/domain';
import { schoolExternalToolFactory } from '../../school-external-tool/testing';
import { ExternalTool, Lti11ToolConfig, Oauth2ToolConfig } from '../domain';
import { externalToolFactory, lti11ToolConfigFactory, oauth2ToolConfigFactory } from '../testing';
import { ExternalToolServiceMapper } from './external-tool-service.mapper';
Expand Down Expand Up @@ -375,16 +376,13 @@ describe(ExternalToolService.name, () => {
const setup = () => {
createTools();

const schoolExternalTool: SchoolExternalTool = new SchoolExternalTool({
id: 'schoolTool1',
toolId: 'tool1',
schoolId: 'school1',
parameters: [],
});
const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build();

schoolToolRepo.findByExternalToolId.mockResolvedValue([schoolExternalTool]);

return { schoolExternalTool };
return {
schoolExternalTool,
};
};

describe('when tool id is set', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,9 @@ describe('ToolSchoolController (API)', () => {
const response = await loggedInClient.post().send(postParams);

expect(response.statusCode).toEqual(HttpStatus.CREATED);
expect(response.body).toEqual({
expect(response.body).toEqual<SchoolExternalToolResponse>({
id: expect.any(String),
isDeactivated: postParams.isDeactivated,
name: externalToolEntity.name,
schoolId: postParams.schoolId,
toolId: postParams.toolId,
Expand Down Expand Up @@ -231,6 +232,7 @@ describe('ToolSchoolController (API)', () => {

const externalToolEntity: ExternalToolEntity = externalToolEntityFactory.buildWithId({
parameters: [],
isDeactivated: true,
});

const schoolExternalToolEntity: SchoolExternalToolEntity = schoolExternalToolEntityFactory.buildWithId({
Expand Down Expand Up @@ -284,16 +286,18 @@ describe('ToolSchoolController (API)', () => {

expect(response.statusCode).toEqual(HttpStatus.OK);
expect(response.body).toEqual(
expect.objectContaining(<SchoolExternalToolSearchListResponse>{
expect.objectContaining<SchoolExternalToolSearchListResponse>({
data: [
{
id: schoolExternalToolEntity.id,
name: externalToolEntity.name,
schoolId: school.id,
toolId: externalToolEntity.id,
status: schoolExternalToolConfigurationStatusFactory.build({
isDeactivated: false,
status: {
isOutdatedOnScopeSchool: true,
}),
isGloballyDeactivated: true,
},
parameters: [
{
name: schoolExternalToolEntity.schoolParameters[0].name,
Expand Down Expand Up @@ -322,6 +326,7 @@ describe('ToolSchoolController (API)', () => {

const externalToolEntity: ExternalToolEntity = externalToolEntityFactory.buildWithId({
parameters: [],
isDeactivated: true,
});

const schoolExternalToolEntity: SchoolExternalToolEntity = schoolExternalToolEntityFactory.buildWithId({
Expand All @@ -331,12 +336,14 @@ describe('ToolSchoolController (API)', () => {

const schoolExternalToolResponse: SchoolExternalToolResponse = new SchoolExternalToolResponse({
id: schoolExternalToolEntity.id,
name: '',
name: externalToolEntity.name,
schoolId: school.id,
toolId: externalToolEntity.id,
status: schoolExternalToolConfigurationStatusFactory.build({
isOutdatedOnScopeSchool: false,
}),
isDeactivated: false,
status: {
isOutdatedOnScopeSchool: true,
isGloballyDeactivated: true,
},
parameters: [
{
name: schoolExternalToolEntity.schoolParameters[0].name,
Expand Down Expand Up @@ -459,9 +466,11 @@ describe('ToolSchoolController (API)', () => {
name: externalToolEntity.name,
schoolId: postParamsUpdate.schoolId,
toolId: postParamsUpdate.toolId,
status: schoolExternalToolConfigurationStatusFactory.build({
isDeactivated: false,
status: {
isOutdatedOnScopeSchool: false,
}),
isGloballyDeactivated: false,
},
parameters: [
{
name: updatedParamEntry.name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ export * from './school-external-tool-post.params';
export * from './school-external-tool-search.params';
export * from './school-external-tool-search-list.response';
export * from './school-external-tool-metadata.response';
export * from '../domain/school-external-tool-configuration-status';
export { SchoolExternalToolConfigurationStatusResponse } from './school-external-tool-configuration.response';
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ export class SchoolExternalToolConfigurationStatusResponse {

@ApiProperty({
type: Boolean,
description: 'Is the tool deactivated, because of school administrator?',
description: 'Is the tool deactivated, because of instance administrator?',
})
isDeactivated: boolean;
isGloballyDeactivated: boolean;

constructor(props: SchoolExternalToolConfigurationStatusResponse) {
this.isOutdatedOnScopeSchool = props.isOutdatedOnScopeSchool;
this.isDeactivated = props.isDeactivated;
this.isGloballyDeactivated = props.isGloballyDeactivated;
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
import { ApiProperty } from '@nestjs/swagger';
import { CustomParameterEntryResponse } from './custom-parameter-entry.response';
import { SchoolExternalToolConfigurationStatusResponse } from './school-external-tool-configuration.response';

Expand All @@ -15,22 +15,22 @@ export class SchoolExternalToolResponse {
@ApiProperty()
schoolId: string;

@ApiProperty()
isDeactivated: boolean;

@ApiProperty({ type: [CustomParameterEntryResponse] })
parameters: CustomParameterEntryResponse[];

@ApiProperty({ type: SchoolExternalToolConfigurationStatusResponse })
status: SchoolExternalToolConfigurationStatusResponse;

@ApiPropertyOptional()
logoUrl?: string;

constructor(response: SchoolExternalToolResponse) {
this.id = response.id;
this.name = response.name;
this.toolId = response.toolId;
this.schoolId = response.schoolId;
this.isDeactivated = response.isDeactivated;
this.parameters = response.parameters;
this.status = response.status;
this.logoUrl = response.logoUrl;
}
}
Loading
Loading