From 46f1b1e8b8dfd37065c746d1a297440a4e9eb6f2 Mon Sep 17 00:00:00 2001 From: Uwe Ilgenstein Date: Fri, 17 Nov 2023 15:38:04 +0100 Subject: [PATCH] fix specs --- .../user-import/uc/user-import.uc.spec.ts | 5 +-- .../src/shared/domain/entity/school.entity.ts | 35 ++++++++++--------- .../legacy-school.repo.integration.spec.ts | 22 ++++++++++-- .../shared/repo/school/legacy-school.repo.ts | 9 +++-- .../user/user-do.repo.integration.spec.ts | 6 ++-- .../src/shared/repo/user/user-do.repo.ts | 9 +++-- apps/server/src/shared/repo/user/user.repo.ts | 6 ++-- 7 files changed, 55 insertions(+), 37 deletions(-) diff --git a/apps/server/src/modules/user-import/uc/user-import.uc.spec.ts b/apps/server/src/modules/user-import/uc/user-import.uc.spec.ts index ff4aa4c266c..681d13972f8 100644 --- a/apps/server/src/modules/user-import/uc/user-import.uc.spec.ts +++ b/apps/server/src/modules/user-import/uc/user-import.uc.spec.ts @@ -122,10 +122,7 @@ describe('[ImportUserModule]', () => { const officialSchoolNumber = school ? school.officialSchoolNumber : undefined; const inMaintenanceSince = school ? school.inMaintenanceSince : undefined; const inUserMigration = school ? school.inUserMigration : undefined; - const systems = - school && school.systems.isInitialized() - ? school.systems.getItems().map((system: SystemEntity) => system.id) - : []; + const systems = school ? school.systems : []; const federalState = school ? school.federalState : federalStateFactory.build(); return new LegacySchoolDo({ diff --git a/apps/server/src/shared/domain/entity/school.entity.ts b/apps/server/src/shared/domain/entity/school.entity.ts index 98502a127eb..0613f1eb7e2 100644 --- a/apps/server/src/shared/domain/entity/school.entity.ts +++ b/apps/server/src/shared/domain/entity/school.entity.ts @@ -1,19 +1,11 @@ -import { - Collection, - Embeddable, - Embedded, - Entity, - Index, - ManyToMany, - ManyToOne, - OneToOne, - Property, -} from '@mikro-orm/core'; +import { Embeddable, Embedded, Entity, Index, ManyToOne, OneToOne, Property } from '@mikro-orm/core'; import { UserLoginMigrationEntity } from '@shared/domain/entity/user-login-migration.entity'; +import { ObjectId } from 'bson'; +import { EntityId } from '../types'; import { BaseEntity } from './base.entity'; +import { FederalStateEntity } from './federal-state.entity'; import { SchoolYearEntity } from './schoolyear.entity'; import { SystemEntity } from './system.entity'; -import { FederalStateEntity } from './federal-state.entity'; export enum SchoolFeatures { ROCKET_CHAT = 'rocketChat', @@ -60,7 +52,7 @@ export class SchoolRoles { } @Entity({ tableName: 'schools' }) -@Index({ properties: ['externalId', 'systems'] }) +@Index({ properties: ['externalId', '_systems'] }) export class SchoolEntity extends BaseEntity { @Property({ nullable: true }) features?: SchoolFeatures[]; @@ -83,8 +75,19 @@ export class SchoolEntity extends BaseEntity { @Property({ nullable: true }) officialSchoolNumber?: string; - @ManyToMany(() => SystemEntity, undefined, { fieldName: 'systems' }) - systems = new Collection(this); + @Property({ fieldName: 'systems' }) + _systems: ObjectId[] = []; + + get systems() { + return this._systems.map((s) => s.toHexString()); + } + + set systems(systemIds: EntityId[]) { + this._systems = systemIds.map((sid) => new ObjectId(sid)); + } + + // @ManyToMany(() => SystemEntity, undefined, { fieldName: 'systems' }) + // systems = new Collection(this); @Embedded(() => SchoolRoles, { object: true, nullable: true, prefix: false }) permissions?: SchoolRoles; @@ -123,7 +126,7 @@ export class SchoolEntity extends BaseEntity { this.officialSchoolNumber = props.officialSchoolNumber; } if (props.systems) { - this.systems.set(props.systems); + this.systems = props.systems.map((s) => s.id); } if (props.features) { this.features = props.features; diff --git a/apps/server/src/shared/repo/school/legacy-school.repo.integration.spec.ts b/apps/server/src/shared/repo/school/legacy-school.repo.integration.spec.ts index 775c193675d..5f09ac71915 100644 --- a/apps/server/src/shared/repo/school/legacy-school.repo.integration.spec.ts +++ b/apps/server/src/shared/repo/school/legacy-school.repo.integration.spec.ts @@ -7,6 +7,7 @@ import { ISchoolProperties, LegacySchoolDo, SchoolEntity, + SchoolFeatures, SchoolRolePermission, SchoolRoles, SchoolYearEntity, @@ -78,6 +79,21 @@ describe('LegacySchoolRepo', () => { expect(result).toMatchObject(expected); expect(result.id).toBeDefined(); }); + + it('should successfully save school after adding a system', async () => { + const system: SystemEntity = systemFactory.buildWithId(); + const schoolEntity: SchoolEntity = schoolFactory.build({ externalId: 'externalId' }); + schoolEntity.systems = [system.id]; + + await em.persistAndFlush(schoolEntity); + em.clear(); + + const school = await repo.findById(schoolEntity.id); + + school.features = [SchoolFeatures.NEXTCLOUD]; + + await expect(repo.save(school)).resolves.not.toThrow(); + }); }); }); @@ -116,13 +132,13 @@ describe('LegacySchoolRepo', () => { it('should find school by external ID', async () => { const system: SystemEntity = systemFactory.buildWithId(); const schoolEntity: SchoolEntity = schoolFactory.build({ externalId: 'externalId' }); - schoolEntity.systems.add(system); + schoolEntity.systems = [system.id]; - await em.persistAndFlush(schoolEntity); + await em.persistAndFlush([system, schoolEntity]); const result: LegacySchoolDo | null = await repo.findByExternalId( schoolEntity.externalId as string, - schoolEntity.systems[0].id + schoolEntity.systems[0] ); expect(result?.externalId).toEqual(schoolEntity.externalId); diff --git a/apps/server/src/shared/repo/school/legacy-school.repo.ts b/apps/server/src/shared/repo/school/legacy-school.repo.ts index 711eaea27d9..4395909d1e6 100644 --- a/apps/server/src/shared/repo/school/legacy-school.repo.ts +++ b/apps/server/src/shared/repo/school/legacy-school.repo.ts @@ -1,5 +1,5 @@ import { EntityName } from '@mikro-orm/core'; -import { EntityManager } from '@mikro-orm/mongodb'; +import { EntityManager, ObjectId } from '@mikro-orm/mongodb'; import { Injectable, InternalServerErrorException } from '@nestjs/common'; import { EntityId, @@ -26,7 +26,10 @@ export class LegacySchoolRepo extends BaseDORepo { - const school: SchoolEntity | null = await this._em.findOne(SchoolEntity, { externalId, systems: systemId }); + const school: SchoolEntity | null = await this._em.findOne(SchoolEntity, { + externalId, + _systems: { $in: [new ObjectId(systemId)] }, + }); const schoolDo: LegacySchoolDo | null = school ? this.mapEntityToDO(school) : null; return schoolDo; @@ -57,7 +60,7 @@ export class LegacySchoolRepo extends BaseDORepo system.id) : [], + systems: entity.systems, userLoginMigrationId: entity.userLoginMigration?.id, federalState: entity.federalState, }); diff --git a/apps/server/src/shared/repo/user/user-do.repo.integration.spec.ts b/apps/server/src/shared/repo/user/user-do.repo.integration.spec.ts index 43d46a95383..d9c511de01e 100644 --- a/apps/server/src/shared/repo/user/user-do.repo.integration.spec.ts +++ b/apps/server/src/shared/repo/user/user-do.repo.integration.spec.ts @@ -148,7 +148,7 @@ describe('UserRepo', () => { beforeEach(async () => { system = systemFactory.buildWithId(); school = schoolFactory.buildWithId(); - school.systems.add(system); + school.systems = [system.id]; user = userFactory.buildWithId({ externalId, school }); await em.persistAndFlush([user, system, school]); @@ -175,7 +175,7 @@ describe('UserRepo', () => { }); it('should return null if school has no corresponding system', async () => { - school.systems.removeAll(); + school.systems = []; const result: UserDO | null = await repo.findByExternalId(user.externalId as string, system.id); @@ -192,7 +192,7 @@ describe('UserRepo', () => { beforeEach(async () => { system = systemFactory.buildWithId(); school = schoolFactory.buildWithId(); - school.systems.add(system); + school.systems = [system.id]; user = userFactory.buildWithId({ externalId, school }); await em.persistAndFlush([user, system, school]); diff --git a/apps/server/src/shared/repo/user/user-do.repo.ts b/apps/server/src/shared/repo/user/user-do.repo.ts index e9eae128a1f..b02702fae52 100644 --- a/apps/server/src/shared/repo/user/user-do.repo.ts +++ b/apps/server/src/shared/repo/user/user-do.repo.ts @@ -1,4 +1,5 @@ import { EntityName, FilterQuery, QueryOrderMap } from '@mikro-orm/core'; +import { UserQuery } from '@modules/user/service/user-query.type'; import { Injectable } from '@nestjs/common'; import { EntityNotFoundError } from '@shared/common'; import { @@ -10,14 +11,12 @@ import { SchoolEntity, SortOrder, SortOrderMap, - SystemEntity, User, } from '@shared/domain'; import { RoleReference } from '@shared/domain/domainobject'; import { Page } from '@shared/domain/domainobject/page'; import { UserDO } from '@shared/domain/domainobject/user.do'; import { BaseDORepo, Scope } from '@shared/repo'; -import { UserQuery } from '@modules/user/service/user-query.type'; import { UserScope } from './user.scope'; @Injectable() @@ -61,7 +60,7 @@ export class UserDORepo extends BaseDORepo { const userEntity: User = await this._em.findOneOrFail(this.entityName, id as FilterQuery); if (populate) { - await this._em.populate(userEntity, ['roles', 'school.systems', 'school.schoolYear']); + await this._em.populate(userEntity, ['roles', 'school._systems', 'school.schoolYear']); await this.populateRoles(userEntity.roles.getItems()); } @@ -77,10 +76,10 @@ export class UserDORepo extends BaseDORepo { } async findByExternalId(externalId: string, systemId: string): Promise { - const userEntitys: User[] = await this._em.find(User, { externalId }, { populate: ['school.systems'] }); + const userEntitys: User[] = await this._em.find(User, { externalId }, { populate: ['school._systems'] }); const userEntity: User | undefined = userEntitys.find((user: User): boolean => { const { systems } = user.school; - return systems && !!systems.getItems().find((system: SystemEntity): boolean => system.id === systemId); + return systems && !!systems.find((system) => system === systemId); }); const userDo: UserDO | null = userEntity ? this.mapEntityToDO(userEntity) : null; diff --git a/apps/server/src/shared/repo/user/user.repo.ts b/apps/server/src/shared/repo/user/user.repo.ts index a067953faba..1a605811838 100644 --- a/apps/server/src/shared/repo/user/user.repo.ts +++ b/apps/server/src/shared/repo/user/user.repo.ts @@ -26,7 +26,7 @@ export class UserRepo extends BaseRepo { const user = await super.findById(id); if (populate) { - await this._em.populate(user, ['roles', 'school.systems', 'school.schoolYear']); + await this._em.populate(user, ['roles', 'school._systems', 'school.schoolYear']); await this.populateRoles(user.roles.getItems()); } @@ -34,10 +34,10 @@ export class UserRepo extends BaseRepo { } async findByExternalIdOrFail(externalId: string, systemId: string): Promise { - const [users] = await this._em.findAndCount(User, { externalId }, { populate: ['school.systems'] }); + const [users] = await this._em.findAndCount(User, { externalId }, { populate: ['school._systems'] }); const resultUser = users.find((user) => { const { systems } = user.school; - return systems && systems.getItems().find((system) => system.id === systemId); + return systems && systems.find((system) => system === systemId); }); return resultUser ?? Promise.reject(); }