From ba96bae380563def5f96b4e8099e08aa674b672f Mon Sep 17 00:00:00 2001 From: Phael Mouko Date: Tue, 16 Jul 2024 15:14:06 +0200 Subject: [PATCH] SPSH-696: Refactored according to DDD rules by moving logic frpm controller & aggregate into the repository. --- ...m-personenkontext.repo.integration-spec.ts | 18 ++++++ .../persistence/dbiam-personenkontext.repo.ts | 4 ++ src/modules/rolle/api/rolle.controller.ts | 25 +++----- src/modules/rolle/domain/rolle.factory.ts | 1 - src/modules/rolle/domain/rolle.spec.ts | 63 ++----------------- src/modules/rolle/domain/rolle.ts | 9 --- src/modules/rolle/repo/rolle.repo.spec.ts | 41 +++++------- src/modules/rolle/repo/rolle.repo.ts | 18 +++++- 8 files changed, 67 insertions(+), 112 deletions(-) diff --git a/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.integration-spec.ts b/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.integration-spec.ts index eaa777006..647ac0157 100644 --- a/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.integration-spec.ts +++ b/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.integration-spec.ts @@ -831,4 +831,22 @@ describe('dbiam Personenkontext Repo', () => { expect(result).toEqual(new MismatchedRevisionError('Personenkontext')); }); }); + + describe('isRolleAlreadyAssigned', () => { + it('should return true if there is any personenkontext for a rolle', async () => { + const person: Person = await createPerson(); + const rolle: Rolle = await rolleRepo.save(DoFactory.createRolle(false)); + + await sut.save(createPersonenkontext(false, { rolleId: rolle.id, personId: person.id })); + const result: boolean = await sut.isRolleAlreadyAssigned(rolle.id); + + expect(result).toBeTruthy(); + }); + + it('should return false if there is no personenkontext for a rolle', async () => { + const result: boolean = await sut.isRolleAlreadyAssigned(faker.string.uuid()); + + expect(result).toBeFalsy(); + }); + }); }); diff --git a/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.ts b/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.ts index 196a0400f..b73f6d88b 100644 --- a/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.ts +++ b/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.ts @@ -355,4 +355,8 @@ export class DBiamPersonenkontextRepo { rolleId: rolleId, }); } + + public async isRolleAlreadyAssigned(id: RolleID): Promise { + return (await this.findByRolle(id)).length > 0; + } } diff --git a/src/modules/rolle/api/rolle.controller.ts b/src/modules/rolle/api/rolle.controller.ts index 997cbaa9c..c847405f0 100644 --- a/src/modules/rolle/api/rolle.controller.ts +++ b/src/modules/rolle/api/rolle.controller.ts @@ -315,31 +315,22 @@ export class RolleController { @Body() params: UpdateRolleBodyParams, @Permissions() permissions: PersonPermissions, ): Promise { + if ( + params.merkmale.length > 0 && + (await this.dBiamPersonenkontextRepo.isRolleAlreadyAssigned(findRolleByIdParams.rolleId)) + ) { + throw new UpdateMerkmaleError(); + } - //Due to circular reference error, the rolleRepo needs to be passed into the aggregate. - const updatedRolle: Rolle | DomainError = await this.rolleFactory.update( + const result: Rolle | DomainError = await this.rolleRepo.updateRolle( findRolleByIdParams.rolleId, params.name, params.merkmale, params.systemrechte, params.serviceProviderIds, + permissions, ); - if (updatedRolle instanceof DomainError) { - throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( - SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(updatedRolle), - ); - } - //The check is here because it cannot be implemented in the aggregate itself in the method update - //using DBiamPersonenkontextRepo causes circular reference error. - if ( - params.merkmale.length > 0 && - (await updatedRolle.isAlreadyAssigned(this.dBiamPersonenkontextRepo, updatedRolle.id)) - ) { - throw new UpdateMerkmaleError(); - } - - const result: Rolle | DomainError = await this.rolleRepo.saveAuthorized(updatedRolle, permissions); if (result instanceof DomainError) { throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(result), diff --git a/src/modules/rolle/domain/rolle.factory.ts b/src/modules/rolle/domain/rolle.factory.ts index d152f984e..5156dc8ef 100644 --- a/src/modules/rolle/domain/rolle.factory.ts +++ b/src/modules/rolle/domain/rolle.factory.ts @@ -4,7 +4,6 @@ import { Rolle } from './rolle.js'; import { RollenArt, RollenMerkmal, RollenSystemRecht } from './rolle.enums.js'; import { OrganisationRepository } from '../../organisation/persistence/organisation.repository.js'; import { DomainError } from '../../../shared/error/domain.error.js'; -import { RolleRepo } from '../repo/rolle.repo.js'; @Injectable() export class RolleFactory { diff --git a/src/modules/rolle/domain/rolle.spec.ts b/src/modules/rolle/domain/rolle.spec.ts index b61768584..542296ded 100644 --- a/src/modules/rolle/domain/rolle.spec.ts +++ b/src/modules/rolle/domain/rolle.spec.ts @@ -13,7 +13,6 @@ import { OrganisationRepository } from '../../organisation/persistence/organisat import { Organisation } from '../../organisation/domain/organisation.js'; import { DBiamPersonenkontextRepo } from '../../personenkontext/persistence/dbiam-personenkontext.repo.js'; import { PersonenkontextFactory } from '../../personenkontext/domain/personenkontext.factory.js'; -import { Personenkontext } from '../../personenkontext/domain/personenkontext.js'; import { PersonRepository } from '../../person/persistence/person.repository.js'; describe('Rolle Aggregate', () => { @@ -21,28 +20,7 @@ describe('Rolle Aggregate', () => { let rolleFactory: RolleFactory; let serviceProviderRepoMock: DeepMocked; let organisationRepo: DeepMocked; - let rolleRepoMock: DeepMocked; - let dBiamPersonenkontextRepoMock: DeepMocked; - let personenkontextFactory: PersonenkontextFactory; - - function createPersonenkontext( - this: void, - withId: WasPersisted, - params: Partial> = {}, - ): Personenkontext { - const personenkontext: Personenkontext = personenkontextFactory.construct( - withId ? faker.string.uuid() : undefined, - withId ? faker.date.past() : undefined, - withId ? faker.date.recent() : undefined, - faker.string.uuid(), - faker.string.uuid(), - faker.string.uuid(), - ); - - Object.assign(personenkontext, params); - - return personenkontext; - } + beforeAll(async () => { module = await Test.createTestingModule({ @@ -74,9 +52,6 @@ describe('Rolle Aggregate', () => { rolleFactory = module.get(RolleFactory); serviceProviderRepoMock = module.get(ServiceProviderRepo); organisationRepo = module.get(OrganisationRepository); - rolleRepoMock = module.get(RolleRepo); - dBiamPersonenkontextRepoMock = module.get(DBiamPersonenkontextRepo); - personenkontextFactory = module.get(PersonenkontextFactory); }); afterAll(async () => { @@ -311,44 +286,16 @@ describe('Rolle Aggregate', () => { }); }); - describe('IsAlreadyAssigned', () => { - it('should return false if rolle is not assigned yet', async () => { - const rolle: Rolle = DoFactory.createRolle(true); - dBiamPersonenkontextRepoMock.findByRolle.mockResolvedValueOnce([]); - const result: boolean = await rolle.isAlreadyAssigned(dBiamPersonenkontextRepoMock, rolle.id); - expect(result).toBeFalsy(); - }); - - it('should return true if rolle is already assigned', async () => { - const rolle: Rolle = DoFactory.createRolle(true); - const personenkontext: Personenkontext = createPersonenkontext(true); - dBiamPersonenkontextRepoMock.findByRolle.mockResolvedValueOnce([personenkontext]); - const result: boolean = await rolle.isAlreadyAssigned(dBiamPersonenkontextRepoMock, rolle.id); - expect(result).toBeTruthy(); - }); - }); - describe('update', () => { - it('should return domain error if rolle is does not exist', async () => { - rolleRepoMock.findById.mockResolvedValueOnce(undefined); - const result: Rolle | DomainError = await rolleFactory.update( - rolleRepoMock, - faker.string.uuid(), - 'newName', - [], - [], - [], - ); - expect(result).toBeInstanceOf(DomainError); - }); - it('should return domain error if service provider is does not exist', async () => { - rolleRepoMock.findById.mockResolvedValueOnce(DoFactory.createRolle(true)); serviceProviderRepoMock.findById.mockResolvedValue(undefined); const result: Rolle | DomainError = await rolleFactory.update( - rolleRepoMock, faker.string.uuid(), + faker.datatype.datetime(), + faker.datatype.datetime(), 'newName', + faker.string.uuid(), + faker.helpers.enumValue(RollenArt), [faker.helpers.enumValue(RollenMerkmal)], [faker.helpers.enumValue(RollenSystemRecht)], [faker.string.uuid()], diff --git a/src/modules/rolle/domain/rolle.ts b/src/modules/rolle/domain/rolle.ts index 4ddd19c58..dbe408691 100644 --- a/src/modules/rolle/domain/rolle.ts +++ b/src/modules/rolle/domain/rolle.ts @@ -6,8 +6,6 @@ import { EntityAlreadyExistsError, EntityNotFoundError } from '../../../shared/e import { OrganisationID } from '../../../shared/types/aggregate-ids.types.js'; import { OrganisationRepository } from '../../organisation/persistence/organisation.repository.js'; import { Organisation } from '../../organisation/domain/organisation.js'; -import { DBiamPersonenkontextRepo } from '../../personenkontext/persistence/dbiam-personenkontext.repo.js'; -import { RolleRepo } from '../repo/rolle.repo.js'; export class Rolle { private constructor( @@ -122,13 +120,6 @@ export class Rolle { return !!childOrgas.find((orga: Organisation) => orga.id === orgaId); } - public async isAlreadyAssigned( - dBiamPersonenkontextRepo: DBiamPersonenkontextRepo, - rolleId: string, - ): Promise { - return (await dBiamPersonenkontextRepo.findByRolle(rolleId)).length > 0; - } - public addMerkmal(merkmal: RollenMerkmal): void { if (!this.merkmale.includes(merkmal)) { this.merkmale.push(merkmal); diff --git a/src/modules/rolle/repo/rolle.repo.spec.ts b/src/modules/rolle/repo/rolle.repo.spec.ts index dc63c822d..7bec289cb 100644 --- a/src/modules/rolle/repo/rolle.repo.spec.ts +++ b/src/modules/rolle/repo/rolle.repo.spec.ts @@ -28,7 +28,6 @@ describe('RolleRepo', () => { let orm: MikroORM; let em: EntityManager; let serviceProviderRepo: ServiceProviderRepo; - let rolleFactory: RolleFactory; beforeAll(async () => { module = await Test.createTestingModule({ @@ -40,7 +39,6 @@ describe('RolleRepo', () => { orm = module.get(MikroORM); em = module.get(EntityManager); serviceProviderRepo = module.get(ServiceProviderRepo); - rolleFactory = module.get(RolleFactory); await DatabaseTestModule.setupDatabase(orm); }, DEFAULT_TIMEOUT_FOR_TESTCONTAINERS); @@ -255,36 +253,34 @@ describe('RolleRepo', () => { }); }); - describe('saveAuthorized', () => { - it('should return the rolle', async () => { + describe('updateRolle', () => { + it('should return the updated rolle', async () => { const organisationId: OrganisationID = faker.string.uuid(); const rolle: Rolle = await sut.save( DoFactory.createRolle(false, { administeredBySchulstrukturknoten: organisationId }), ); const permissions: DeepMocked = createMock(); const newName: string = 'updatedrolle'; + const newMermale: RollenMerkmal[] = [RollenMerkmal.KOPERS_PFLICHT]; + const newSystemrechte: RollenSystemRecht[] = [RollenSystemRecht.PERSONEN_SOFORT_LOESCHEN]; permissions.getOrgIdsWithSystemrecht.mockResolvedValueOnce([organisationId]); - const updatedRolle: Rolle | DomainError = await rolleFactory.update( - sut, + + const rolleResult: Rolle | DomainError = await sut.updateRolle( rolle.id, newName, - [RollenMerkmal.KOPERS_PFLICHT], - [RollenSystemRecht.PERSONEN_SOFORT_LOESCHEN], + newMermale, + newSystemrechte, [], + permissions, ); - - if (updatedRolle instanceof DomainError) { - return; - } - const rolleResult: Rolle | DomainError = await sut.saveAuthorized(updatedRolle, permissions); if (rolleResult instanceof DomainError) { return; } - expect(rolleResult.id).toBe(updatedRolle.id); + expect(rolleResult.id).toBe(rolle.id); expect(rolleResult.name).toBe(newName); - expect(rolleResult.merkmale).toMatchObject(updatedRolle.merkmale); - expect(rolleResult.systemrechte).toMatchObject(updatedRolle.systemrechte); - expect(rolleResult.serviceProviderIds).toMatchObject(updatedRolle.serviceProviderIds); + expect(rolleResult.merkmale).toMatchObject(newMermale); + expect(rolleResult.systemrechte).toMatchObject(newSystemrechte); + expect(rolleResult.serviceProviderIds).toMatchObject([]); }); it('should return error when permissions are insufficient', async () => { @@ -295,20 +291,15 @@ describe('RolleRepo', () => { permissions.getOrgIdsWithSystemrecht.mockResolvedValueOnce([]); - const updatedRolle: Rolle | DomainError = await rolleFactory.update( - sut, + const rolleResult: Rolle | DomainError = await sut.updateRolle( rolle.id, - 'newName', + faker.company.name(), [], [], [], + permissions, ); - if (updatedRolle instanceof DomainError) { - return; - } - const rolleResult: Rolle | DomainError = await sut.saveAuthorized(updatedRolle, permissions); - expect(rolleResult).toBeInstanceOf(DomainError); }); }); diff --git a/src/modules/rolle/repo/rolle.repo.ts b/src/modules/rolle/repo/rolle.repo.ts index 84206c0b8..6c529e3bf 100644 --- a/src/modules/rolle/repo/rolle.repo.ts +++ b/src/modules/rolle/repo/rolle.repo.ts @@ -230,8 +230,22 @@ export class RolleRepo { return authorizedRole.error; } - //Update fields with aggregate - const result: Rolle = await this.update(rolle); + const updatedRolle: Rolle | DomainError = await this.rolleFactory.update( + id, + authorizedRole.value.createdAt, + authorizedRole.value.updatedAt, + name, + authorizedRole.value.administeredBySchulstrukturknoten, + authorizedRole.value.rollenart, + merkmale, + systemrechte, + serviceProviderIds, + ); + + if (updatedRolle instanceof DomainError) { + return updatedRolle; + } + const result: Rolle = await this.update(updatedRolle); return result; }