From cda389cdf371c5d4f7e53285263d6aee92b77c23 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Thu, 14 Nov 2024 10:55:31 +0100 Subject: [PATCH 01/29] Log creation of Schule and Klasse --- .../api/organisation.controller.spec.ts | 25 ++++- .../api/organisation.controller.ts | 6 +- .../domain/organisation.service.spec.ts | 3 +- .../domain/organisation.service.ts | 93 +++++++++++++++++-- 4 files changed, 110 insertions(+), 17 deletions(-) diff --git a/src/modules/organisation/api/organisation.controller.spec.ts b/src/modules/organisation/api/organisation.controller.spec.ts index ac93d2986..c7e95ab8c 100644 --- a/src/modules/organisation/api/organisation.controller.spec.ts +++ b/src/modules/organisation/api/organisation.controller.spec.ts @@ -16,7 +16,7 @@ import { OrganisationByIdBodyParams } from './organisation-by-id.body.params.js' import { OrganisationRepository } from '../persistence/organisation.repository.js'; import { Organisation } from '../domain/organisation.js'; import { OrganisationResponse } from './organisation.response.js'; -import { PersonPermissions } from '../../authentication/domain/person-permissions.js'; +import { PersonenkontextRolleFields, PersonPermissions } from '../../authentication/domain/person-permissions.js'; import { EventService } from '../../../core/eventbus/index.js'; import { OrganisationRootChildrenResponse } from './organisation.root-children.response.js'; import { OrganisationSpecificationError } from '../specification/error/organisation-specification.error.js'; @@ -30,6 +30,7 @@ import { OrganisationByNameQueryParams } from './organisation-by-name.query.js'; import { DBiamPersonenkontextRepo } from '../../personenkontext/persistence/dbiam-personenkontext.repo.js'; import { ParentOrganisationenResponse } from './organisation.parents.response.js'; import { ParentOrganisationsByIdsBodyParams } from './parent-organisations-by-ids.body.params.js'; +import { Person } from '../../person/domain/person.js'; function getFakeParamsAndBody(): [OrganisationByIdParams, OrganisationByIdBodyParams] { const params: OrganisationByIdParams = { @@ -88,6 +89,20 @@ describe('OrganisationController', () => { }); describe('createOrganisation', () => { + const permissionsMock: PersonPermissions = createMock({ + get personFields(): Person { + return createMock>({ + familienname: 'current-user', + }); + }, + getPersonenkontextewithRoles: (): Promise => + Promise.resolve([ + { + organisationsId: '', + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]), + }); describe('when usecase returns a DTO', () => { it('should not throw an error', async () => { const params: CreateOrganisationBodyParams = { @@ -101,7 +116,7 @@ describe('OrganisationController', () => { const returnedValue: Organisation = DoFactory.createOrganisation(true); organisationServiceMock.createOrganisation.mockResolvedValueOnce({ ok: true, value: returnedValue }); - await expect(organisationController.createOrganisation(params)).resolves.not.toThrow(); + await expect(organisationController.createOrganisation(permissionsMock, params)).resolves.not.toThrow(); expect(organisationServiceMock.createOrganisation).toHaveBeenCalledTimes(1); }); }); @@ -153,7 +168,7 @@ describe('OrganisationController', () => { organisationRepositoryMock.findRootDirectChildren.mockResolvedValue(mockedRepoResponse); try { - await organisationController.createOrganisation(params); + await organisationController.createOrganisation(permissionsMock, params); fail('Expected error was not thrown'); } catch (error) { @@ -202,7 +217,7 @@ describe('OrganisationController', () => { error: new OrganisationSpecificationError('error', undefined), }); await expect( - organisationController.createOrganisation({} as CreateOrganisationBodyParams), + organisationController.createOrganisation(permissionsMock, {} as CreateOrganisationBodyParams), ).rejects.toThrow(OrganisationSpecificationError); expect(organisationServiceMock.createOrganisation).toHaveBeenCalledTimes(1); }); @@ -249,7 +264,7 @@ describe('OrganisationController', () => { error: {} as EntityNotFoundError, }); await expect( - organisationController.createOrganisation({} as CreateOrganisationBodyParams), + organisationController.createOrganisation(permissionsMock, {} as CreateOrganisationBodyParams), ).rejects.toThrow(HttpException); expect(organisationServiceMock.createOrganisation).toHaveBeenCalledTimes(1); }); diff --git a/src/modules/organisation/api/organisation.controller.ts b/src/modules/organisation/api/organisation.controller.ts index 2469ae445..02feb8c6d 100644 --- a/src/modules/organisation/api/organisation.controller.ts +++ b/src/modules/organisation/api/organisation.controller.ts @@ -82,7 +82,10 @@ export class OrganisationController { @ApiUnauthorizedResponse({ description: 'Not authorized to create the organisation.' }) @ApiForbiddenResponse({ description: 'Not permitted to create the organisation.' }) @ApiInternalServerErrorResponse({ description: 'Internal server error while creating the organisation.' }) - public async createOrganisation(@Body() params: CreateOrganisationBodyParams): Promise { + public async createOrganisation( + @Permissions() permissions: PersonPermissions, + @Body() params: CreateOrganisationBodyParams, + ): Promise { const [oeffentlich]: [Organisation | undefined, Organisation | undefined] = await this.organisationRepository.findRootDirectChildren(); @@ -103,6 +106,7 @@ export class OrganisationController { } const result: Result, DomainError> = await this.organisationService.createOrganisation( organisation, + permissions, ); if (!result.ok) { if (result.error instanceof OrganisationSpecificationError) { diff --git a/src/modules/organisation/domain/organisation.service.spec.ts b/src/modules/organisation/domain/organisation.service.spec.ts index 5eea5b6f7..d83eb6ae3 100644 --- a/src/modules/organisation/domain/organisation.service.spec.ts +++ b/src/modules/organisation/domain/organisation.service.spec.ts @@ -20,6 +20,7 @@ import { Organisation } from './organisation.js'; import { NameForOrganisationWithTrailingSpaceError } from '../specification/error/name-with-trailing-space.error.js'; import { KennungForOrganisationWithTrailingSpaceError } from '../specification/error/kennung-with-trailing-space.error.js'; import { EmailAdressOnOrganisationTypError } from '../specification/error/email-adress-on-organisation-typ-error.js'; +import { LoggingTestModule } from '../../../../test/utils/logging-test.module.js'; describe('OrganisationService', () => { let module: TestingModule; @@ -29,7 +30,7 @@ describe('OrganisationService', () => { beforeAll(async () => { module = await Test.createTestingModule({ - imports: [ConfigTestModule], + imports: [ConfigTestModule, LoggingTestModule], providers: [ OrganisationService, { diff --git a/src/modules/organisation/domain/organisation.service.ts b/src/modules/organisation/domain/organisation.service.ts index f0bc20e6c..2094a5c28 100644 --- a/src/modules/organisation/domain/organisation.service.ts +++ b/src/modules/organisation/domain/organisation.service.ts @@ -35,61 +35,134 @@ import { Organisation } from './organisation.js'; import { OrganisationRepository } from '../persistence/organisation.repository.js'; import { EmailAdressOnOrganisationTyp } from '../specification/email-on-organisation-type.js'; import { EmailAdressOnOrganisationTypError } from '../specification/error/email-adress-on-organisation-typ-error.js'; +import { ClassLogger } from '../../../core/logging/class-logger.js'; +import { OrganisationsTyp } from './organisation.enums.js'; +import { PersonenkontextRolleFields, PersonPermissions } from '../../authentication/domain/person-permissions.js'; @Injectable() export class OrganisationService { - public constructor(private readonly organisationRepo: OrganisationRepository) {} + public constructor( + private readonly logger: ClassLogger, + private readonly organisationRepo: OrganisationRepository, + ) {} + + private async logCreation( + permissions: PersonPermissions, + organisation: Organisation, + error?: Error, + ): Promise { + if (organisation.typ === OrganisationsTyp.KLASSE) { + if (organisation.zugehoerigZu) { + const school: Option> = await this.organisationRepo.findById( + organisation.zugehoerigZu, + ); + const schoolName: string = school?.name ?? 'SCHOOL_NOT_FOUND'; + if (permissions) { + if (error) { + this.logger.error( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}) hat versucht eine neue Klasse ${organisation.name} (${schoolName}) anzulegen. Fehler: ${error.message}`, + ); + } else { + this.logger.info( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}) hat eine neue Klasse angelegt: ${organisation.name} (${schoolName}).`, + ); + } + } + } + } + if (organisation.typ === OrganisationsTyp.SCHULE) { + if (permissions) { + const personenkontextRolleFields: PersonenkontextRolleFields[] = + await permissions?.getPersonenkontextewithRoles(); + const organisationIdUser: string = personenkontextRolleFields.at(0)?.organisationsId as string; + const organisationUser: Option> = + await this.organisationRepo.findById(organisationIdUser); + const organisationNameUser: string = organisationUser?.name ?? 'ORGANISATION_NOT_FOUND'; + if (error) { + this.logger.error( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine neue Schule ${organisation.name} anzulegen. Fehler: ${error.message}`, + ); + } else { + this.logger.info( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat eine neue Schule angelegt: ${organisation.name}.`, + ); + } + } + } + } + public async createOrganisation( organisationDo: Organisation, + permissions: PersonPermissions | null = null, ): Promise, DomainError>> { if (organisationDo.administriertVon && !(await this.organisationRepo.exists(organisationDo.administriertVon))) { + const error: DomainError = new EntityNotFoundError('Organisation', organisationDo.administriertVon); + if (permissions) await this.logCreation(permissions, organisationDo, error); return { ok: false, - error: new EntityNotFoundError('Organisation', organisationDo.administriertVon), + error: error, }; } if (organisationDo.zugehoerigZu && !(await this.organisationRepo.exists(organisationDo.zugehoerigZu))) { + const error: DomainError = new EntityNotFoundError('Organisation', organisationDo.zugehoerigZu); + if (permissions) await this.logCreation(permissions, organisationDo, error); return { ok: false, - error: new EntityNotFoundError('Organisation', organisationDo.zugehoerigZu), + error: error, }; } const validationFieldnamesResult: void | DomainError = this.validateFieldNames(organisationDo); if (validationFieldnamesResult) { - return { ok: false, error: validationFieldnamesResult }; + const error: DomainError = validationFieldnamesResult; + if (permissions) await this.logCreation(permissions, organisationDo, error); + return { ok: false, error: error }; } let validationResult: Result = await this.validateKennungRequiredForSchule(organisationDo); if (!validationResult.ok) { - return { ok: false, error: validationResult.error }; + const error: DomainError = validationResult.error; + if (permissions) await this.logCreation(permissions, organisationDo, error); + return { ok: false, error: error }; } validationResult = await this.validateNameRequiredForSchule(organisationDo); if (!validationResult.ok) { - return { ok: false, error: validationResult.error }; + const error: DomainError = validationResult.error; + if (permissions) await this.logCreation(permissions, organisationDo, error); + return { ok: false, error: error }; } validationResult = await this.validateSchuleKennungUnique(organisationDo); if (!validationResult.ok) { - return { ok: false, error: validationResult.error }; + const error: DomainError = validationResult.error; + if (permissions) await this.logCreation(permissions, organisationDo, error); + return { ok: false, error: error }; } validationResult = await this.validateEmailAdressOnOrganisationTyp(organisationDo); if (!validationResult.ok) { - return { ok: false, error: validationResult.error }; + const error: DomainError = validationResult.error; + if (permissions) await this.logCreation(permissions, organisationDo, error); + return { ok: false, error: error }; } const validateKlassen: Result = await this.validateKlassenSpecifications(organisationDo); if (!validateKlassen.ok) { - return { ok: false, error: validateKlassen.error }; + const error: DomainError = validateKlassen.error; + if (permissions) await this.logCreation(permissions, organisationDo, error); + return { ok: false, error: error }; } const organisation: Organisation | OrganisationSpecificationError = await this.organisationRepo.save(organisationDo); if (organisation instanceof Organisation) { + if (permissions) await this.logCreation(permissions, organisation); return { ok: true, value: organisation }; } - return { ok: false, error: new EntityCouldNotBeCreated(`Organization could not be created`) }; + + const error: DomainError = new EntityCouldNotBeCreated(`Organization could not be created`); + if (permissions) await this.logCreation(permissions, organisationDo, error); + return { ok: false, error: error }; } public async updateOrganisation( From 3edd75f0ade2a3bdd3d7cebe06260e46ea247364 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Thu, 14 Nov 2024 10:59:35 +0100 Subject: [PATCH 02/29] Log update of Klasse name --- .../api/organisation.controller.spec.ts | 60 ++++++++++++++----- .../api/organisation.controller.ts | 3 +- ...rganisation.repository.integration-spec.ts | 8 ++- .../persistence/organisation.repository.ts | 14 +++++ 4 files changed, 69 insertions(+), 16 deletions(-) diff --git a/src/modules/organisation/api/organisation.controller.spec.ts b/src/modules/organisation/api/organisation.controller.spec.ts index c7e95ab8c..24203a5cf 100644 --- a/src/modules/organisation/api/organisation.controller.spec.ts +++ b/src/modules/organisation/api/organisation.controller.spec.ts @@ -734,6 +734,20 @@ describe('OrganisationController', () => { }); describe('updateOrganisationName', () => { + const permissionsMock: PersonPermissions = createMock({ + get personFields(): Person { + return createMock>({ + familienname: 'current-user', + }); + }, + getPersonenkontextewithRoles: (): Promise => + Promise.resolve([ + { + organisationsId: '', + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]), + }); describe('when usecase succeeds', () => { it('should not throw an error', async () => { const oeffentlich: Organisation = Organisation.construct( @@ -760,7 +774,9 @@ describe('OrganisationController', () => { organisationRepositoryMock.updateKlassenname.mockResolvedValueOnce(oeffentlich); - await expect(organisationController.updateOrganisationName(params, body)).resolves.not.toThrow(); + await expect( + organisationController.updateOrganisationName(params, body, permissionsMock), + ).resolves.not.toThrow(); }); }); @@ -775,9 +791,9 @@ describe('OrganisationController', () => { }; organisationRepositoryMock.updateKlassenname.mockResolvedValueOnce(new NameRequiredForKlasseError()); - await expect(organisationController.updateOrganisationName(params, body)).rejects.toThrow( - NameRequiredForKlasseError, - ); + await expect( + organisationController.updateOrganisationName(params, body, permissionsMock), + ).rejects.toThrow(NameRequiredForKlasseError); }); }); @@ -793,14 +809,28 @@ describe('OrganisationController', () => { organisationRepositoryMock.updateKlassenname.mockResolvedValueOnce(new EntityNotFoundError()); - await expect(organisationController.updateOrganisationName(params, body)).rejects.toThrow( - HttpException, - ); + await expect( + organisationController.updateOrganisationName(params, body, permissionsMock), + ).rejects.toThrow(HttpException); }); }); }); describe('updateOrganisationName', () => { + const permissionsMock: PersonPermissions = createMock({ + get personFields(): Person { + return createMock>({ + familienname: 'current-user', + }); + }, + getPersonenkontextewithRoles: (): Promise => + Promise.resolve([ + { + organisationsId: '', + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]), + }); describe('when usecase succeeds', () => { it('should not throw an error', async () => { const oeffentlich: Organisation = Organisation.construct( @@ -827,7 +857,9 @@ describe('OrganisationController', () => { organisationRepositoryMock.updateKlassenname.mockResolvedValueOnce(oeffentlich); - await expect(organisationController.updateOrganisationName(params, body)).resolves.not.toThrow(); + await expect( + organisationController.updateOrganisationName(params, body, permissionsMock), + ).resolves.not.toThrow(); }); }); @@ -842,9 +874,9 @@ describe('OrganisationController', () => { }; organisationRepositoryMock.updateKlassenname.mockResolvedValueOnce(new NameRequiredForKlasseError()); - await expect(organisationController.updateOrganisationName(params, body)).rejects.toThrow( - NameRequiredForKlasseError, - ); + await expect( + organisationController.updateOrganisationName(params, body, permissionsMock), + ).rejects.toThrow(NameRequiredForKlasseError); }); }); @@ -860,9 +892,9 @@ describe('OrganisationController', () => { organisationRepositoryMock.updateKlassenname.mockResolvedValueOnce(new EntityNotFoundError()); - await expect(organisationController.updateOrganisationName(params, body)).rejects.toThrow( - HttpException, - ); + await expect( + organisationController.updateOrganisationName(params, body, permissionsMock), + ).rejects.toThrow(HttpException); }); }); }); diff --git a/src/modules/organisation/api/organisation.controller.ts b/src/modules/organisation/api/organisation.controller.ts index 02feb8c6d..ba9bdd0cd 100644 --- a/src/modules/organisation/api/organisation.controller.ts +++ b/src/modules/organisation/api/organisation.controller.ts @@ -445,18 +445,19 @@ export class OrganisationController { public async updateOrganisationName( @Param() params: OrganisationByIdParams, @Body() body: OrganisationByNameBodyParams, + @Permissions() permissions: PersonPermissions, ): Promise { const result: DomainError | Organisation = await this.organisationRepository.updateKlassenname( params.organisationId, body.name, body.version, + permissions, ); if (result instanceof DomainError) { if (result instanceof OrganisationSpecificationError) { throw result; } - throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(result), ); diff --git a/src/modules/organisation/persistence/organisation.repository.integration-spec.ts b/src/modules/organisation/persistence/organisation.repository.integration-spec.ts index 6e8477944..3f5938586 100644 --- a/src/modules/organisation/persistence/organisation.repository.integration-spec.ts +++ b/src/modules/organisation/persistence/organisation.repository.integration-spec.ts @@ -27,6 +27,7 @@ import { OrganisationSpecificationError } from '../specification/error/organisat import { PersonPermissions } from '../../authentication/domain/person-permissions.js'; import { RollenSystemRecht } from '../../rolle/domain/rolle.enums.js'; import { OrganisationUpdateOutdatedError } from '../domain/orga-update-outdated.error.js'; +import { LoggingTestModule } from '../../../../test/utils/logging-test.module.js'; describe('OrganisationRepository', () => { let module: TestingModule; @@ -39,7 +40,12 @@ describe('OrganisationRepository', () => { beforeAll(async () => { module = await Test.createTestingModule({ - imports: [ConfigTestModule, DatabaseTestModule.forRoot({ isDatabaseRequired: true }), MapperTestModule], + imports: [ + ConfigTestModule, + DatabaseTestModule.forRoot({ isDatabaseRequired: true }), + MapperTestModule, + LoggingTestModule, + ], providers: [ OrganisationPersistenceMapperProfile, OrganisationRepository, diff --git a/src/modules/organisation/persistence/organisation.repository.ts b/src/modules/organisation/persistence/organisation.repository.ts index baa648ac9..a5c943173 100644 --- a/src/modules/organisation/persistence/organisation.repository.ts +++ b/src/modules/organisation/persistence/organisation.repository.ts @@ -430,6 +430,7 @@ export class OrganisationRepository { id: string, newName: string, version: number, + permissions: PersonPermissions | null = null, ): Promise> { const organisationFound: Option> = await this.findById(id); @@ -439,6 +440,11 @@ export class OrganisationRepository { if (organisationFound.typ !== OrganisationsTyp.KLASSE) { return new EntityCouldNotBeUpdated('Organisation', id, ['Only the name of Klassen can be updated.']); } + let schoolName: string = 'SCHOOL_NOT_FOUND'; + if (organisationFound.zugehoerigZu) { + const school: Option> = await this.findById(organisationFound.zugehoerigZu); + schoolName = school?.name ?? 'SCHOOL_NOT_FOUND'; + } //Specifications: it needs to be clarified how the specifications can be checked using DDD principles { if (organisationFound.name !== newName) { @@ -447,6 +453,10 @@ export class OrganisationRepository { await organisationFound.checkKlasseSpecifications(this); if (specificationError) { + if (permissions) + this.logger.error( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}) hat versucht den Namen einer Klasse ${organisationFound.name} (${schoolName}) zu verändern. Fehler: ${specificationError.message}`, + ); return specificationError; } } @@ -455,6 +465,10 @@ export class OrganisationRepository { const organisationEntity: Organisation | OrganisationSpecificationError = await this.save(organisationFound); this.eventService.publish(new KlasseUpdatedEvent(id, newName, organisationFound.administriertVon)); + if (permissions) + this.logger.info( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}) hat den Namen einer Klasse geändert: ${organisationFound.name} (${schoolName}).`, + ); return organisationEntity; } From b5443a7a190bdfab574d49e55018b732cb659322 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Thu, 14 Nov 2024 11:02:16 +0100 Subject: [PATCH 03/29] Add logger --- src/modules/organisation/persistence/organisation.repository.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/modules/organisation/persistence/organisation.repository.ts b/src/modules/organisation/persistence/organisation.repository.ts index a5c943173..4ec273385 100644 --- a/src/modules/organisation/persistence/organisation.repository.ts +++ b/src/modules/organisation/persistence/organisation.repository.ts @@ -29,6 +29,7 @@ import { KlasseCreatedEvent } from '../../../shared/events/klasse-created.event. import { PermittedOrgas, PersonPermissions } from '../../authentication/domain/person-permissions.js'; import { RollenSystemRecht } from '../../rolle/domain/rolle.enums.js'; import { OrganisationUpdateOutdatedError } from '../domain/orga-update-outdated.error.js'; +import { ClassLogger } from '../../../core/logging/class-logger.js'; export function mapAggregateToData(organisation: Organisation): RequiredEntityData { return { @@ -85,6 +86,7 @@ export class OrganisationRepository { private readonly eventService: EventService, private readonly em: EntityManager, config: ConfigService, + private readonly logger: ClassLogger, ) { this.ROOT_ORGANISATION_ID = config.getOrThrow('DATA').ROOT_ORGANISATION_ID; } From 5b2b6d31b167dcb9eeed61c32052d86dc7cb2a25 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Thu, 14 Nov 2024 11:03:32 +0100 Subject: [PATCH 04/29] Log deletion of Klasse --- .../organisation/api/organisation.controller.ts | 10 ++++++++-- .../persistence/organisation.repository.ts | 16 +++++++++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/modules/organisation/api/organisation.controller.ts b/src/modules/organisation/api/organisation.controller.ts index ba9bdd0cd..efed8ab1a 100644 --- a/src/modules/organisation/api/organisation.controller.ts +++ b/src/modules/organisation/api/organisation.controller.ts @@ -419,12 +419,18 @@ export class OrganisationController { @ApiBadRequestResponse({ description: 'The input was not valid.', type: DbiamOrganisationError }) @ApiNotFoundResponse({ description: 'The organisation that should be deleted does not exist.' }) @ApiUnauthorizedResponse({ description: 'Not authorized to delete the organisation.' }) - public async deleteKlasse(@Param() params: OrganisationByIdParams): Promise { + public async deleteKlasse( + @Param() params: OrganisationByIdParams, + @Permissions() permissions: PersonPermissions, + ): Promise { if (await this.dBiamPersonenkontextRepo.isOrganisationAlreadyAssigned(params.organisationId)) { throw new OrganisationIstBereitsZugewiesenError(); } - const result: Option = await this.organisationRepository.deleteKlasse(params.organisationId); + const result: Option = await this.organisationRepository.deleteKlasse( + params.organisationId, + permissions, + ); if (result instanceof DomainError) { throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(result), diff --git a/src/modules/organisation/persistence/organisation.repository.ts b/src/modules/organisation/persistence/organisation.repository.ts index 4ec273385..a8f0cdfcf 100644 --- a/src/modules/organisation/persistence/organisation.repository.ts +++ b/src/modules/organisation/persistence/organisation.repository.ts @@ -412,7 +412,10 @@ export class OrganisationRepository { return [organisations, total + entitiesForIds.length - duplicates, pageTotal]; } - public async deleteKlasse(id: OrganisationID): Promise> { + public async deleteKlasse( + id: OrganisationID, + permissions: PersonPermissions | null = null, + ): Promise> { const organisationEntity: Option = await this.em.findOne(OrganisationEntity, { id }); if (!organisationEntity) { return new EntityNotFoundError('Organisation', id); @@ -425,6 +428,17 @@ export class OrganisationRepository { await this.em.removeAndFlush(organisationEntity); this.eventService.publish(new KlasseDeletedEvent(organisationEntity.id)); + let schoolName: string = 'SCHOOL_NOT_FOUND'; + if (permissions) + if (organisationEntity.zugehoerigZu) { + const school: Option> = await this.findById(organisationEntity.zugehoerigZu); + schoolName = school?.name ?? 'SCHOOL_NOT_FOUND'; + this.logger.info( + // Admin () hat eine Klasse entfernt: ( < Schule >). + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}) hat eine Klasse entfernt: ${organisationEntity.name} (${schoolName}).`, + ); + } + return; } From 3131ac31787654c8a0af5fbae6f901e9fe5093ad Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Thu, 14 Nov 2024 11:04:20 +0100 Subject: [PATCH 05/29] Log update of Schule and Klasse --- .../domain/organisation.service.ts | 78 +++++++++++++++++-- 1 file changed, 70 insertions(+), 8 deletions(-) diff --git a/src/modules/organisation/domain/organisation.service.ts b/src/modules/organisation/domain/organisation.service.ts index 2094a5c28..9876aa1ed 100644 --- a/src/modules/organisation/domain/organisation.service.ts +++ b/src/modules/organisation/domain/organisation.service.ts @@ -91,6 +91,50 @@ export class OrganisationService { } } + private async logUpdate( + permissions: PersonPermissions, + organisation: Organisation, + error?: Error, + ): Promise { + if (organisation.typ === OrganisationsTyp.KLASSE) { + if (organisation.zugehoerigZu) { + const school: Option> = await this.organisationRepo.findById( + organisation.zugehoerigZu, + ); + const schoolName: string = school?.name ?? 'SCHOOL_NOT_FOUND'; + if (permissions) { + if (error) { + this.logger.error( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}) hat versucht eine Klasse ${organisation.name} (${schoolName}) zu verändern. Fehler: ${error.message}`, + ); + } else { + this.logger.info( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}) hat eine Klasse geändert: ${organisation.name} (${schoolName}).`, + ); + } + } + } + } + if (organisation.typ === OrganisationsTyp.SCHULE) { + if (permissions) { + const personenkontextRolleFields: PersonenkontextRolleFields[] = + await permissions?.getPersonenkontextewithRoles(); + const organisationIdUser: string = personenkontextRolleFields.at(0)?.organisationsId as string; + const organisationUser: Option> = + await this.organisationRepo.findById(organisationIdUser); + const organisationNameUser: string = organisationUser?.name ?? 'ORGANISATION_NOT_FOUND'; + if (error) { + this.logger.error( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine Schule ${organisation.name} zu verändern. Fehler: ${error.message}`, + ); + } else { + this.logger.info( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat eine Schule geändert: ${organisation.name}.`, + ); + } + } + } + } public async createOrganisation( organisationDo: Organisation, @@ -167,48 +211,66 @@ export class OrganisationService { public async updateOrganisation( organisationDo: Organisation, + permissions: PersonPermissions | null = null, ): Promise, DomainError>> { const storedOrganisation: Option> = await this.organisationRepo.findById(organisationDo.id); if (!storedOrganisation) { - return { ok: false, error: new EntityNotFoundError('Organisation', organisationDo.id) }; + const error: DomainError = new EntityNotFoundError('Organisation', organisationDo.id); + if (permissions) await this.logUpdate(permissions, organisationDo, error); + return { ok: false, error: error }; } const validationFieldnamesResult: void | DomainError = this.validateFieldNames(organisationDo); if (validationFieldnamesResult) { - return { ok: false, error: validationFieldnamesResult }; + const error: DomainError = validationFieldnamesResult; + if (permissions) await this.logUpdate(permissions, organisationDo, error); + return { ok: false, error: error }; } let validationResult: Result = await this.validateKennungRequiredForSchule(organisationDo); if (!validationResult.ok) { - return { ok: false, error: validationResult.error }; + const error: DomainError = validationResult.error; + if (permissions) await this.logUpdate(permissions, organisationDo, error); + return { ok: false, error: error }; } validationResult = await this.validateNameRequiredForSchule(organisationDo); if (!validationResult.ok) { - return { ok: false, error: validationResult.error }; + const error: DomainError = validationResult.error; + if (permissions) await this.logUpdate(permissions, organisationDo, error); + return { ok: false, error: error }; } validationResult = await this.validateSchuleKennungUnique(organisationDo); if (!validationResult.ok) { - return { ok: false, error: validationResult.error }; + const error: DomainError = validationResult.error; + if (permissions) await this.logUpdate(permissions, organisationDo, error); + return { ok: false, error: error }; } validationResult = await this.validateEmailAdressOnOrganisationTyp(organisationDo); if (!validationResult.ok) { - return { ok: false, error: validationResult.error }; + const error: DomainError = validationResult.error; + if (permissions) await this.logUpdate(permissions, organisationDo, error); + return { ok: false, error: error }; } const validateKlassen: Result = await this.validateKlassenSpecifications(organisationDo); if (!validateKlassen.ok) { - return { ok: false, error: validateKlassen.error }; + const error: DomainError = validateKlassen.error; + if (permissions) await this.logUpdate(permissions, organisationDo, error); + return { ok: false, error: error }; } const organisation: Organisation | OrganisationSpecificationError = await this.organisationRepo.save(organisationDo); if (organisation instanceof Organisation) { + if (permissions) await this.logUpdate(permissions, organisation); return { ok: true, value: organisation }; } + const error: DomainError = new EntityCouldNotBeUpdated(`Organization could not be updated`, organisationDo.id); + if (permissions) await this.logUpdate(permissions, organisationDo, error); return { ok: false, - error: new EntityCouldNotBeUpdated(`Organization could not be updated`, organisationDo.id), + error: error, }; } From 7339c6c99a667149a7399314725e138115098d90 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Thu, 14 Nov 2024 11:06:40 +0100 Subject: [PATCH 06/29] Log creation, update and deletion of Rolle --- .../rolle/api/rolle.controller.spec.ts | 27 ++++++++- src/modules/rolle/api/rolle.controller.ts | 59 +++++++++++++++++-- 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/src/modules/rolle/api/rolle.controller.spec.ts b/src/modules/rolle/api/rolle.controller.spec.ts index 05fa9614e..bf5e7838d 100644 --- a/src/modules/rolle/api/rolle.controller.spec.ts +++ b/src/modules/rolle/api/rolle.controller.spec.ts @@ -1,7 +1,12 @@ import { faker } from '@faker-js/faker'; import { APP_PIPE } from '@nestjs/core'; import { Test, TestingModule } from '@nestjs/testing'; -import { DEFAULT_TIMEOUT_FOR_TESTCONTAINERS, DoFactory, MapperTestModule } from '../../../../test/utils/index.js'; +import { + DEFAULT_TIMEOUT_FOR_TESTCONTAINERS, + DoFactory, + LoggingTestModule, + MapperTestModule, +} from '../../../../test/utils/index.js'; import { GlobalValidationPipe } from '../../../shared/validation/global-validation.pipe.js'; import { RolleRepo } from '../repo/rolle.repo.js'; import { RolleFactory } from '../domain/rolle.factory.js'; @@ -19,6 +24,8 @@ import { RollenArt, RollenMerkmal, RollenSystemRecht } from '../domain/rolle.enu import { NameForRolleWithTrailingSpaceError } from '../domain/name-with-trailing-space.error.js'; import { Organisation } from '../../organisation/domain/organisation.js'; import { RolleServiceProviderBodyParams } from './rolle-service-provider.body.params.js'; +import { PersonenkontextRolleFields, PersonPermissions } from '../../authentication/domain/person-permissions.js'; +import { Person } from '../../person/domain/person.js'; describe('Rolle API with mocked ServiceProviderRepo', () => { let rolleRepoMock: DeepMocked; @@ -28,7 +35,7 @@ describe('Rolle API with mocked ServiceProviderRepo', () => { beforeAll(async () => { const module: TestingModule = await Test.createTestingModule({ - imports: [MapperTestModule], + imports: [MapperTestModule, LoggingTestModule], providers: [ { provide: APP_PIPE, @@ -103,6 +110,20 @@ describe('Rolle API with mocked ServiceProviderRepo', () => { describe('/GET rolle mocked Rolle-repo', () => { describe('createRolle', () => { + const permissionsMock: PersonPermissions = createMock({ + get personFields(): Person { + return createMock>({ + familienname: 'current-user', + }); + }, + getPersonenkontextewithRoles: (): Promise => + Promise.resolve([ + { + organisationsId: '', + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]), + }); it('should throw an HTTP exception when rolleFactory.createNew returns DomainError', async () => { const createRolleParams: CreateRolleBodyParams = { name: ' SuS', @@ -118,7 +139,7 @@ describe('Rolle API with mocked ServiceProviderRepo', () => { value: organisation, }); - await expect(rolleController.createRolle(createRolleParams)).rejects.toThrow( + await expect(rolleController.createRolle(createRolleParams, permissionsMock)).rejects.toThrow( NameForRolleWithTrailingSpaceError, ); }); diff --git a/src/modules/rolle/api/rolle.controller.ts b/src/modules/rolle/api/rolle.controller.ts index ca1eb0529..27bdba657 100644 --- a/src/modules/rolle/api/rolle.controller.ts +++ b/src/modules/rolle/api/rolle.controller.ts @@ -51,7 +51,7 @@ import { SchulConnexError } from '../../../shared/error/schul-connex.error.js'; import { RolleExceptionFilter } from './rolle-exception-filter.js'; import { Paged, PagedResponse, PagingHeadersObject } from '../../../shared/paging/index.js'; import { Permissions } from '../../authentication/api/permissions.decorator.js'; -import { PersonPermissions } from '../../authentication/domain/person-permissions.js'; +import { PersonenkontextRolleFields, PersonPermissions } from '../../authentication/domain/person-permissions.js'; import { UpdateRolleBodyParams } from './update-rolle.body.params.js'; import { DBiamPersonenkontextRepo } from '../../personenkontext/persistence/dbiam-personenkontext.repo.js'; import { RolleDomainError } from '../domain/rolle-domain.error.js'; @@ -60,6 +60,7 @@ import { DbiamRolleError } from './dbiam-rolle.error.js'; import { OrganisationRepository } from '../../organisation/persistence/organisation.repository.js'; import { Organisation } from '../../organisation/domain/organisation.js'; import { RolleServiceProviderBodyParams } from './rolle-service-provider.body.params.js'; +import { ClassLogger } from '../../../core/logging/class-logger.js'; @UseFilters(new SchulConnexValidationErrorFilter(), new RolleExceptionFilter(), new AuthenticationExceptionFilter()) @ApiTags('rolle') @@ -74,6 +75,7 @@ export class RolleController { private readonly serviceProviderRepo: ServiceProviderRepo, private readonly dBiamPersonenkontextRepo: DBiamPersonenkontextRepo, private readonly organisationRepository: OrganisationRepository, + private readonly logger: ClassLogger, ) {} @Get() @@ -178,12 +180,20 @@ export class RolleController { @ApiUnauthorizedResponse({ description: 'Not authorized to create the rolle.' }) @ApiForbiddenResponse({ description: 'Insufficient permissions to create the rolle.' }) @ApiInternalServerErrorResponse({ description: 'Internal server error while creating the rolle.' }) - public async createRolle(@Body() params: CreateRolleBodyParams): Promise { + public async createRolle( + @Body() params: CreateRolleBodyParams, + @Permissions() permissions: PersonPermissions, + ): Promise { + const organisationNameUser: string = await this.getOrganisationNameForCurrentUser(permissions); + const orgResult: Result, DomainError> = await this.orgService.findOrganisationById( params.administeredBySchulstrukturknoten, ); if (!orgResult.ok) { + this.logger.info( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine neue Rolle ${params.name} anzulegen. Fehler: ${orgResult.error.message}`, + ); throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(orgResult.error), ); @@ -201,10 +211,16 @@ export class RolleController { ); if (rolle instanceof DomainError) { + this.logger.error( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine neue Rolle ${params.name} anzulegen. Fehler: ${rolle.message}`, + ); throw rolle; } const result: Rolle = await this.rolleRepo.save(rolle); + this.logger.info( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat eine neue Rolle angelegt: ${result.name}.`, + ); return new RolleResponse(result); } @@ -359,6 +375,8 @@ export class RolleController { @Body() params: UpdateRolleBodyParams, @Permissions() permissions: PersonPermissions, ): Promise { + const organisationNameUser: string = await this.getOrganisationNameForCurrentUser(permissions); + const isAlreadyAssigned: boolean = await this.dBiamPersonenkontextRepo.isRolleAlreadyAssigned( findRolleByIdParams.rolleId, ); @@ -375,14 +393,23 @@ export class RolleController { if (result instanceof DomainError) { if (result instanceof RolleDomainError) { + this.logger.error( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine Rolle ${result.name} zu bearbeiten. Fehler: ${result.message}`, + ); throw result; } - + this.logger.error( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine Rolle ${result.name} zu bearbeiten. Fehler: ${result.message}`, + ); throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(result), ); } + this.logger.info( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat eine Rolle bearbeitet: ${result.name}.`, + ); + return this.returnRolleWithServiceProvidersResponse(result); } @@ -397,19 +424,33 @@ export class RolleController { @Param() findRolleByIdParams: FindRolleByIdParams, @Permissions() permissions: PersonPermissions, ): Promise { + const organisationNameUser: string = await this.getOrganisationNameForCurrentUser(permissions); + const rolle: Option> = await this.rolleRepo.findById(findRolleByIdParams.rolleId); + const rolleName: string = rolle?.name ?? 'ORGANISATION_NOT_FOUND'; + const result: Option = await this.rolleRepo.deleteAuthorized( findRolleByIdParams.rolleId, permissions, ); if (result instanceof DomainError) { if (result instanceof RolleDomainError) { + // Admin (, ) hat versucht die Rolle zu entfernen. error: <> + this.logger.error( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht die Rolle ${rolleName} zu entfernen. Fehler: ${result.message}`, + ); throw result; } - + this.logger.error( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht die Rolle ${rolleName} zu entfernen. Fehler: ${result.message}`, + ); throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(result), ); } + + this.logger.info( + `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat eine Rolle entfernt: ${rolleName}.`, + ); } private async returnRolleWithServiceProvidersResponse( @@ -423,4 +464,14 @@ export class RolleController { return new RolleWithServiceProvidersResponse(rolle, rolleServiceProviders); } + + private async getOrganisationNameForCurrentUser(permissions: PersonPermissions): Promise { + const personenkontextRolleFields: PersonenkontextRolleFields[] = + await permissions?.getPersonenkontextewithRoles(); + const organisationIdUser: string = personenkontextRolleFields.at(0)?.organisationsId as string; + const organisationUser: Option> = + await this.organisationRepository.findById(organisationIdUser); + const organisationNameUser: string = organisationUser?.name ?? 'ORGANISATION_NOT_FOUND'; + return organisationNameUser; + } } From 93753d33a512871332127cd9628feb08a7502c14 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Thu, 14 Nov 2024 11:42:43 +0100 Subject: [PATCH 07/29] Add logger to tests --- .../organisation-service-specification.spec.ts | 3 ++- .../organisation-specification-mock-test.spec.ts | 15 +++++++++++++-- .../organisation-specifications-test.spec.ts | 2 ++ ...ersonuebersicht.controller.integration.spec.ts | 2 ++ ...iam-personuebersicht.controller.mocked.spec.ts | 2 ++ .../person.repository.integration-spec.ts | 8 +++++++- .../persistence/person.scope.integration-spec.ts | 2 ++ .../personenkontext.repo.integration-spec.ts | 2 ++ 8 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/modules/organisation/domain/organisation-service-specification.spec.ts b/src/modules/organisation/domain/organisation-service-specification.spec.ts index 3382dd13b..c398f0fec 100644 --- a/src/modules/organisation/domain/organisation-service-specification.spec.ts +++ b/src/modules/organisation/domain/organisation-service-specification.spec.ts @@ -4,7 +4,7 @@ import { DoFactory } from '../../../../test/utils/do-factory.js'; import { DatabaseTestModule } from '../../../../test/utils/database-test.module.js'; import { ConfigTestModule } from '../../../../test/utils/config-test.module.js'; import { MikroORM } from '@mikro-orm/core'; -import { MapperTestModule } from '../../../../test/utils/index.js'; +import { LoggingTestModule, MapperTestModule } from '../../../../test/utils/index.js'; import { OrganisationPersistenceMapperProfile } from '../persistence/organisation-persistence.mapper.profile.js'; import { ZyklusInOrganisationenError } from '../specification/error/zyklus-in-organisationen.error.js'; import { RootOrganisationImmutableError } from '../specification/error/root-organisation-immutable.error.js'; @@ -34,6 +34,7 @@ describe('OrganisationServiceSpecificationTest', () => { DatabaseTestModule.forRoot({ isDatabaseRequired: true }), MapperTestModule, EventModule, + LoggingTestModule, ], providers: [OrganisationService, OrganisationRepository, OrganisationPersistenceMapperProfile], }).compile(); diff --git a/src/modules/organisation/specification/organisation-specification-mock-test.spec.ts b/src/modules/organisation/specification/organisation-specification-mock-test.spec.ts index 2cd6a85fa..8c6c480f0 100644 --- a/src/modules/organisation/specification/organisation-specification-mock-test.spec.ts +++ b/src/modules/organisation/specification/organisation-specification-mock-test.spec.ts @@ -1,5 +1,11 @@ import { Test, TestingModule } from '@nestjs/testing'; -import { ConfigTestModule, DatabaseTestModule, DoFactory, MapperTestModule } from '../../../../test/utils/index.js'; +import { + ConfigTestModule, + DatabaseTestModule, + DoFactory, + LoggingTestModule, + MapperTestModule, +} from '../../../../test/utils/index.js'; import { OrganisationPersistenceMapperProfile } from '../persistence/organisation-persistence.mapper.profile.js'; import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { OrganisationsTyp } from '../domain/organisation.enums.js'; @@ -17,7 +23,12 @@ describe('OrganisationSpecificationMockedRepoTest', () => { beforeAll(async () => { module = await Test.createTestingModule({ - imports: [ConfigTestModule, DatabaseTestModule.forRoot({ isDatabaseRequired: false }), MapperTestModule], + imports: [ + ConfigTestModule, + DatabaseTestModule.forRoot({ isDatabaseRequired: false }), + MapperTestModule, + LoggingTestModule, + ], providers: [ OrganisationPersistenceMapperProfile, { diff --git a/src/modules/organisation/specification/organisation-specifications-test.spec.ts b/src/modules/organisation/specification/organisation-specifications-test.spec.ts index ab81caf89..287c9ea18 100644 --- a/src/modules/organisation/specification/organisation-specifications-test.spec.ts +++ b/src/modules/organisation/specification/organisation-specifications-test.spec.ts @@ -4,6 +4,7 @@ import { DatabaseTestModule, DEFAULT_TIMEOUT_FOR_TESTCONTAINERS, DoFactory, + LoggingTestModule, MapperTestModule, } from '../../../../test/utils/index.js'; import { MikroORM } from '@mikro-orm/core'; @@ -37,6 +38,7 @@ describe('OrganisationSpecificationTests', () => { DatabaseTestModule.forRoot({ isDatabaseRequired: true }), MapperTestModule, EventModule, + LoggingTestModule, ], providers: [OrganisationPersistenceMapperProfile, OrganisationRepository], }).compile(); diff --git a/src/modules/person/api/personenuebersicht/dbiam-personuebersicht.controller.integration.spec.ts b/src/modules/person/api/personenuebersicht/dbiam-personuebersicht.controller.integration.spec.ts index e2ac0547c..e432ac27f 100644 --- a/src/modules/person/api/personenuebersicht/dbiam-personuebersicht.controller.integration.spec.ts +++ b/src/modules/person/api/personenuebersicht/dbiam-personuebersicht.controller.integration.spec.ts @@ -9,6 +9,7 @@ import { DatabaseTestModule, DEFAULT_TIMEOUT_FOR_TESTCONTAINERS, DoFactory, + LoggingTestModule, MapperTestModule, } from '../../../../../test/utils/index.js'; import { GlobalValidationPipe } from '../../../../shared/validation/global-validation.pipe.js'; @@ -74,6 +75,7 @@ describe('Personenuebersicht API', () => { ConfigTestModule, DatabaseTestModule.forRoot({ isDatabaseRequired: true }), MapperTestModule, + LoggingTestModule, ], providers: [ { diff --git a/src/modules/person/api/personenuebersicht/dbiam-personuebersicht.controller.mocked.spec.ts b/src/modules/person/api/personenuebersicht/dbiam-personuebersicht.controller.mocked.spec.ts index 4c843ff7e..c3aef747d 100644 --- a/src/modules/person/api/personenuebersicht/dbiam-personuebersicht.controller.mocked.spec.ts +++ b/src/modules/person/api/personenuebersicht/dbiam-personuebersicht.controller.mocked.spec.ts @@ -5,6 +5,7 @@ import { DatabaseTestModule, DEFAULT_TIMEOUT_FOR_TESTCONTAINERS, DoFactory, + LoggingTestModule, MapperTestModule, } from '../../../../../test/utils/index.js'; import { ServiceProviderRepo } from '../../../service-provider/repo/service-provider.repo.js'; @@ -83,6 +84,7 @@ describe('Personenuebersicht API Mocked', () => { ConfigTestModule, DatabaseTestModule.forRoot({ isDatabaseRequired: false }), MapperTestModule, + LoggingTestModule, ], providers: [ServiceProviderRepo, RolleFactory, OrganisationRepository], }) diff --git a/src/modules/person/persistence/person.repository.integration-spec.ts b/src/modules/person/persistence/person.repository.integration-spec.ts index 2aea89541..58c0cf6a2 100644 --- a/src/modules/person/persistence/person.repository.integration-spec.ts +++ b/src/modules/person/persistence/person.repository.integration-spec.ts @@ -6,6 +6,7 @@ import { DatabaseTestModule, DEFAULT_TIMEOUT_FOR_TESTCONTAINERS, DoFactory, + LoggingTestModule, MapperTestModule, } from '../../../../test/utils/index.js'; import { PersonEntity } from './person.entity.js'; @@ -84,7 +85,12 @@ describe('PersonRepository Integration', () => { beforeAll(async () => { module = await Test.createTestingModule({ - imports: [ConfigTestModule, DatabaseTestModule.forRoot({ isDatabaseRequired: true }), MapperTestModule], + imports: [ + ConfigTestModule, + DatabaseTestModule.forRoot({ isDatabaseRequired: true }), + MapperTestModule, + LoggingTestModule, + ], providers: [ PersonRepository, OrganisationRepository, diff --git a/src/modules/person/persistence/person.scope.integration-spec.ts b/src/modules/person/persistence/person.scope.integration-spec.ts index f8ae8d2c9..e91cf2dbf 100644 --- a/src/modules/person/persistence/person.scope.integration-spec.ts +++ b/src/modules/person/persistence/person.scope.integration-spec.ts @@ -6,6 +6,7 @@ import { DEFAULT_TIMEOUT_FOR_TESTCONTAINERS, DatabaseTestModule, DoFactory, + LoggingTestModule, MapperTestModule, } from '../../../../test/utils/index.js'; import { ScopeOrder } from '../../../shared/persistence/scope.enums.js'; @@ -52,6 +53,7 @@ describe('PersonScope', () => { DatabaseTestModule.forRoot({ isDatabaseRequired: true }), MapperTestModule, PersonenKontextModule, + LoggingTestModule, ], providers: [ DBiamPersonenkontextRepoInternal, diff --git a/src/modules/personenkontext/persistence/personenkontext.repo.integration-spec.ts b/src/modules/personenkontext/persistence/personenkontext.repo.integration-spec.ts index 19cd49e8f..2f0c04b42 100644 --- a/src/modules/personenkontext/persistence/personenkontext.repo.integration-spec.ts +++ b/src/modules/personenkontext/persistence/personenkontext.repo.integration-spec.ts @@ -5,6 +5,7 @@ import { DEFAULT_TIMEOUT_FOR_TESTCONTAINERS, DatabaseTestModule, DoFactory, + LoggingTestModule, MapperTestModule, } from '../../../../test/utils/index.js'; import { PersonenkontextDo } from '../domain/personenkontext.do.js'; @@ -39,6 +40,7 @@ describe('PersonenkontextRepo', () => { DatabaseTestModule.forRoot({ isDatabaseRequired: true }), MapperTestModule, EventModule, + LoggingTestModule, ], providers: [ PersonPersistenceMapperProfile, From a783823f5829cfd4615df1f859d38e0645a52aed Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Thu, 14 Nov 2024 13:52:02 +0100 Subject: [PATCH 08/29] Add logger to test --- .../persistence/personenkontext.scope.integration-spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/modules/personenkontext/persistence/personenkontext.scope.integration-spec.ts b/src/modules/personenkontext/persistence/personenkontext.scope.integration-spec.ts index 1681f0b49..57877d4d5 100644 --- a/src/modules/personenkontext/persistence/personenkontext.scope.integration-spec.ts +++ b/src/modules/personenkontext/persistence/personenkontext.scope.integration-spec.ts @@ -8,6 +8,7 @@ import { DEFAULT_TIMEOUT_FOR_TESTCONTAINERS, DatabaseTestModule, DoFactory, + LoggingTestModule, MapperTestModule, } from '../../../../test/utils/index.js'; import { ScopeOrder } from '../../../shared/persistence/scope.enums.js'; @@ -44,6 +45,7 @@ describe('PersonenkontextScope', () => { DatabaseTestModule.forRoot({ isDatabaseRequired: true }), MapperTestModule, EventModule, + LoggingTestModule, ], providers: [ PersonPersistenceMapperProfile, From 8b69ca76e85dff8b8c197295bed0d9a338d61407 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Thu, 14 Nov 2024 13:57:12 +0100 Subject: [PATCH 09/29] Adapt test --- .../api/rolle.controller.integration-spec.ts | 78 ++++++++++++++----- 1 file changed, 60 insertions(+), 18 deletions(-) diff --git a/src/modules/rolle/api/rolle.controller.integration-spec.ts b/src/modules/rolle/api/rolle.controller.integration-spec.ts index dd221827e..3ea69b037 100644 --- a/src/modules/rolle/api/rolle.controller.integration-spec.ts +++ b/src/modules/rolle/api/rolle.controller.integration-spec.ts @@ -39,7 +39,7 @@ import { DBiamPersonenkontextRepoInternal } from '../../personenkontext/persiste import { PersonRepository } from '../../person/persistence/person.repository.js'; import { KeycloakUserService } from '../../keycloak-administration/domain/keycloak-user.service.js'; -import { PersonPermissions } from '../../authentication/domain/person-permissions.js'; +import { PersonenkontextRolleFields, PersonPermissions } from '../../authentication/domain/person-permissions.js'; import { Person } from '../../person/domain/person.js'; import { DomainError } from '../../../shared/error/domain.error.js'; import { PersonFactory } from '../../person/domain/person.factory.js'; @@ -139,6 +139,20 @@ describe('Rolle API', () => { }); describe('/POST rolle', () => { + const permissionsMock: PersonPermissions = createMock({ + get personFields(): Person { + return createMock>({ + familienname: 'current-user', + }); + }, + getPersonenkontextewithRoles: (): Promise => + Promise.resolve([ + { + organisationsId: '', + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]), + }); it('should return created rolle', async () => { const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); @@ -155,7 +169,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .post('/rolle') - .send(params); + .send({ params: params, permissions: permissionsMock }); expect(response.status).toBe(201); expect(response.body).toEqual(expect.objectContaining(params)); @@ -175,7 +189,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .post('/rolle') - .send(params); + .send({ params: params, permissions: permissionsMock }); const rolle: RolleResponse = response.body as RolleResponse; await em.findOneOrFail(RolleEntity, { id: rolle.id }); @@ -192,7 +206,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .post('/rolle') - .send(params); + .send({ params: params, permissions: permissionsMock }); expect(response.status).toBe(404); }); @@ -211,7 +225,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .post('/rolle') - .send(params); + .send({ params: params, permissions: permissionsMock }); expect(response.status).toBe(400); }); @@ -230,7 +244,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .post('/rolle') - .send(params); + .send({ params: params, permissions: permissionsMock }); expect(response.status).toBe(400); }); @@ -249,7 +263,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .post('/rolle') - .send(params); + .send({ params: params, permissions: permissionsMock }); expect(response.status).toBe(400); }); @@ -651,6 +665,20 @@ describe('Rolle API', () => { }); describe('/PUT rolle', () => { + const permissionsMock: PersonPermissions = createMock({ + get personFields(): Person { + return createMock>({ + familienname: 'current-user', + }); + }, + getPersonenkontextewithRoles: (): Promise => + Promise.resolve([ + { + organisationsId: '', + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]), + }); it('should return updated rolle', async () => { const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); @@ -682,7 +710,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .put(`/rolle/${rolle.id}`) - .send(params); + .send({ params: params, permissions: permissionsMock }); expect(response.status).toBe(200); expect(response.body).toMatchObject({ @@ -706,7 +734,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .put(`/rolle/${faker.string.uuid()}`) - .send(params); + .send({ params: params, permissions: permissionsMock }); expect(response.status).toBe(404); }); @@ -737,7 +765,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .put(`/rolle/${rolle.id}`) - .send(params); + .send({ params: params, permissions: permissionsMock }); expect(response.status).toBe(404); expect(response.body).toEqual({ @@ -778,7 +806,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .put(`/rolle/${rolle.id}`) - .send(params); + .send({ params: params, permissions: permissionsMock }); expect(response.status).toBe(404); expect(response.body).toEqual({ @@ -843,7 +871,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .put(`/rolle/${rolle.id}`) - .send(params); + .send({ params: params, permissions: permissionsMock }); expect(response.status).toBe(400); expect(response.body).toEqual({ @@ -880,7 +908,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .put(`/rolle/${rolle.id}`) - .send(params); + .send({ params: params, permissions: permissionsMock }); expect(response.status).toBe(400); expect(response.body).toEqual({ @@ -891,11 +919,25 @@ describe('Rolle API', () => { }); describe('/DELETE rolleId', () => { + const permissionsMock: PersonPermissions = createMock({ + get personFields(): Person { + return createMock>({ + familienname: 'current-user', + }); + }, + getPersonenkontextewithRoles: (): Promise => + Promise.resolve([ + { + organisationsId: '', + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]), + }); describe('should return error', () => { it('if rolle does NOT exist', async () => { const response: Response = await request(app.getHttpServer() as App) .delete(`/rolle/${faker.string.uuid()}`) - .send(); + .send({ permissions: permissionsMock }); expect(response.status).toBe(404); }); @@ -942,7 +984,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .delete(`/rolle/${rolle.id}`) - .send(); + .send({ permissions: permissionsMock }); expect(response.status).toBe(400); expect(response.body).toEqual({ @@ -972,7 +1014,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .delete(`/rolle/${rolle.id}`) - .send(); + .send({ permissions: permissionsMock }); expect(response.status).toBe(404); expect(response.body).toEqual({ @@ -1005,7 +1047,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .delete(`/rolle/${rolle.id}`) - .send(); + .send({ permissions: permissionsMock }); expect(response.status).toBe(404); expect(response.body).toEqual({ @@ -1044,7 +1086,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .delete(`/rolle/${rolle.id}`) - .send(); + .send({ permissions: permissionsMock }); expect(response.status).toBe(204); }); From 775c1f07b7e988c893e1b12d22aa75de7bb01885 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Mon, 18 Nov 2024 16:09:41 +0100 Subject: [PATCH 10/29] Adapt test --- .../api/rolle.controller.integration-spec.ts | 101 +++++++----------- 1 file changed, 41 insertions(+), 60 deletions(-) diff --git a/src/modules/rolle/api/rolle.controller.integration-spec.ts b/src/modules/rolle/api/rolle.controller.integration-spec.ts index 3ea69b037..6b1184f51 100644 --- a/src/modules/rolle/api/rolle.controller.integration-spec.ts +++ b/src/modules/rolle/api/rolle.controller.integration-spec.ts @@ -39,13 +39,14 @@ import { DBiamPersonenkontextRepoInternal } from '../../personenkontext/persiste import { PersonRepository } from '../../person/persistence/person.repository.js'; import { KeycloakUserService } from '../../keycloak-administration/domain/keycloak-user.service.js'; -import { PersonenkontextRolleFields, PersonPermissions } from '../../authentication/domain/person-permissions.js'; +import { PersonPermissions } from '../../authentication/domain/person-permissions.js'; import { Person } from '../../person/domain/person.js'; import { DomainError } from '../../../shared/error/domain.error.js'; import { PersonFactory } from '../../person/domain/person.factory.js'; import { KeycloakConfigModule } from '../../keycloak-administration/keycloak-config.module.js'; import { RolleServiceProviderBodyParams } from './rolle-service-provider.body.params.js'; import { generatePassword } from '../../../shared/util/password-generator.js'; +import { Geschlecht } from '../../person/domain/person.enums.js'; describe('Rolle API', () => { let app: INestApplication; @@ -58,6 +59,7 @@ describe('Rolle API', () => { let personpermissionsRepoMock: DeepMocked; let personPermissionsMock: DeepMocked; let personFactory: PersonFactory; + let permissionsMock: DeepMocked; beforeAll(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -124,6 +126,24 @@ describe('Rolle API', () => { personPermissionsMock = createMock(); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personPermissionsMock); personPermissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); + + // permissionsMock = createMock(); + permissionsMock = createMock({ + get personFields(): Person { + return createMock>({ + id: 'test-id', + keycloakUserId: 'test-keycloak', + vorname: 'test-vorname', + familienname: 'test-familienname', + rufname: 'test-rufname', + username: 'test-username', + geschlecht: Geschlecht.M, + geburtsdatum: faker.date.past(), + updatedAt: faker.date.recent(), + }); + }, + }); + await DatabaseTestModule.setupDatabase(module.get(MikroORM)); app = module.createNestApplication(); await app.init(); @@ -139,20 +159,9 @@ describe('Rolle API', () => { }); describe('/POST rolle', () => { - const permissionsMock: PersonPermissions = createMock({ - get personFields(): Person { - return createMock>({ - familienname: 'current-user', - }); - }, - getPersonenkontextewithRoles: (): Promise => - Promise.resolve([ - { - organisationsId: '', - rolle: { systemrechte: [], serviceProviderIds: [] }, - }, - ]), - }); + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); + it('should return created rolle', async () => { const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); @@ -169,7 +178,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .post('/rolle') - .send({ params: params, permissions: permissionsMock }); + .send(params); expect(response.status).toBe(201); expect(response.body).toEqual(expect.objectContaining(params)); @@ -189,7 +198,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .post('/rolle') - .send({ params: params, permissions: permissionsMock }); + .send(params); const rolle: RolleResponse = response.body as RolleResponse; await em.findOneOrFail(RolleEntity, { id: rolle.id }); @@ -206,7 +215,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .post('/rolle') - .send({ params: params, permissions: permissionsMock }); + .send(params); expect(response.status).toBe(404); }); @@ -225,7 +234,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .post('/rolle') - .send({ params: params, permissions: permissionsMock }); + .send(params); expect(response.status).toBe(400); }); @@ -244,7 +253,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .post('/rolle') - .send({ params: params, permissions: permissionsMock }); + .send(params); expect(response.status).toBe(400); }); @@ -263,7 +272,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .post('/rolle') - .send({ params: params, permissions: permissionsMock }); + .send(params); expect(response.status).toBe(400); }); @@ -665,20 +674,6 @@ describe('Rolle API', () => { }); describe('/PUT rolle', () => { - const permissionsMock: PersonPermissions = createMock({ - get personFields(): Person { - return createMock>({ - familienname: 'current-user', - }); - }, - getPersonenkontextewithRoles: (): Promise => - Promise.resolve([ - { - organisationsId: '', - rolle: { systemrechte: [], serviceProviderIds: [] }, - }, - ]), - }); it('should return updated rolle', async () => { const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); @@ -710,7 +705,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .put(`/rolle/${rolle.id}`) - .send({ params: params, permissions: permissionsMock }); + .send(params); expect(response.status).toBe(200); expect(response.body).toMatchObject({ @@ -734,7 +729,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .put(`/rolle/${faker.string.uuid()}`) - .send({ params: params, permissions: permissionsMock }); + .send(params); expect(response.status).toBe(404); }); @@ -765,7 +760,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .put(`/rolle/${rolle.id}`) - .send({ params: params, permissions: permissionsMock }); + .send(params); expect(response.status).toBe(404); expect(response.body).toEqual({ @@ -806,7 +801,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .put(`/rolle/${rolle.id}`) - .send({ params: params, permissions: permissionsMock }); + .send(params); expect(response.status).toBe(404); expect(response.body).toEqual({ @@ -871,7 +866,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .put(`/rolle/${rolle.id}`) - .send({ params: params, permissions: permissionsMock }); + .send(params); expect(response.status).toBe(400); expect(response.body).toEqual({ @@ -908,7 +903,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .put(`/rolle/${rolle.id}`) - .send({ params: params, permissions: permissionsMock }); + .send(params); expect(response.status).toBe(400); expect(response.body).toEqual({ @@ -919,25 +914,11 @@ describe('Rolle API', () => { }); describe('/DELETE rolleId', () => { - const permissionsMock: PersonPermissions = createMock({ - get personFields(): Person { - return createMock>({ - familienname: 'current-user', - }); - }, - getPersonenkontextewithRoles: (): Promise => - Promise.resolve([ - { - organisationsId: '', - rolle: { systemrechte: [], serviceProviderIds: [] }, - }, - ]), - }); describe('should return error', () => { it('if rolle does NOT exist', async () => { const response: Response = await request(app.getHttpServer() as App) .delete(`/rolle/${faker.string.uuid()}`) - .send({ permissions: permissionsMock }); + .send(); expect(response.status).toBe(404); }); @@ -984,7 +965,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .delete(`/rolle/${rolle.id}`) - .send({ permissions: permissionsMock }); + .send(); expect(response.status).toBe(400); expect(response.body).toEqual({ @@ -1014,7 +995,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .delete(`/rolle/${rolle.id}`) - .send({ permissions: permissionsMock }); + .send(); expect(response.status).toBe(404); expect(response.body).toEqual({ @@ -1047,7 +1028,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .delete(`/rolle/${rolle.id}`) - .send({ permissions: permissionsMock }); + .send(); expect(response.status).toBe(404); expect(response.body).toEqual({ @@ -1086,7 +1067,7 @@ describe('Rolle API', () => { const response: Response = await request(app.getHttpServer() as App) .delete(`/rolle/${rolle.id}`) - .send({ permissions: permissionsMock }); + .send(); expect(response.status).toBe(204); }); From 0962b4371cec3015aad3094296aa8c862dac7159 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Tue, 19 Nov 2024 08:35:01 +0100 Subject: [PATCH 11/29] Adapt test --- src/modules/rolle/api/rolle.controller.integration-spec.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/modules/rolle/api/rolle.controller.integration-spec.ts b/src/modules/rolle/api/rolle.controller.integration-spec.ts index 6b1184f51..11b358c11 100644 --- a/src/modules/rolle/api/rolle.controller.integration-spec.ts +++ b/src/modules/rolle/api/rolle.controller.integration-spec.ts @@ -143,6 +143,8 @@ describe('Rolle API', () => { }); }, }); + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); await DatabaseTestModule.setupDatabase(module.get(MikroORM)); app = module.createNestApplication(); @@ -159,9 +161,6 @@ describe('Rolle API', () => { }); describe('/POST rolle', () => { - personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); - permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); - it('should return created rolle', async () => { const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); From c9c601d8c0f962465e8f96b96df1080b08006a03 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Tue, 19 Nov 2024 09:19:26 +0100 Subject: [PATCH 12/29] Adapt test --- src/modules/rolle/api/rolle.controller.integration-spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/modules/rolle/api/rolle.controller.integration-spec.ts b/src/modules/rolle/api/rolle.controller.integration-spec.ts index 11b358c11..c3f5dcf10 100644 --- a/src/modules/rolle/api/rolle.controller.integration-spec.ts +++ b/src/modules/rolle/api/rolle.controller.integration-spec.ts @@ -144,7 +144,6 @@ describe('Rolle API', () => { }, }); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); - permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); await DatabaseTestModule.setupDatabase(module.get(MikroORM)); app = module.createNestApplication(); @@ -162,6 +161,8 @@ describe('Rolle API', () => { describe('/POST rolle', () => { it('should return created rolle', async () => { + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); + const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); From c41858ba5f03cc5135b9b8a875fc530fcfc3c6cb Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Tue, 19 Nov 2024 16:56:35 +0100 Subject: [PATCH 13/29] Adapt helper function and log statements --- src/modules/rolle/api/rolle.controller.ts | 45 ++++++++++++++--------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/src/modules/rolle/api/rolle.controller.ts b/src/modules/rolle/api/rolle.controller.ts index 27bdba657..2bb548342 100644 --- a/src/modules/rolle/api/rolle.controller.ts +++ b/src/modules/rolle/api/rolle.controller.ts @@ -184,7 +184,8 @@ export class RolleController { @Body() params: CreateRolleBodyParams, @Permissions() permissions: PersonPermissions, ): Promise { - const organisationNameUser: string = await this.getOrganisationNameForCurrentUser(permissions); + const organisationNameUser: string | void = + (await this.getOrganisationNameForCurrentUser(permissions)) ?? 'ORGANISATION_NAME_NOT_FOUND'; const orgResult: Result, DomainError> = await this.orgService.findOrganisationById( params.administeredBySchulstrukturknoten, @@ -375,7 +376,12 @@ export class RolleController { @Body() params: UpdateRolleBodyParams, @Permissions() permissions: PersonPermissions, ): Promise { - const organisationNameUser: string = await this.getOrganisationNameForCurrentUser(permissions); + const userName: string = permissions.personFields.familienname; + const userId: string = permissions.personFields.id; + const organisationNameUser: string | void = + (await this.getOrganisationNameForCurrentUser(permissions)) ?? 'ORGANISATION_NAME_NOT_FOUND'; + const rolle: Option> = await this.rolleRepo.findById(findRolleByIdParams.rolleId); + const rolleName: string = rolle?.name ?? 'ROLLE_NOT_FOUND'; const isAlreadyAssigned: boolean = await this.dBiamPersonenkontextRepo.isRolleAlreadyAssigned( findRolleByIdParams.rolleId, @@ -394,12 +400,12 @@ export class RolleController { if (result instanceof DomainError) { if (result instanceof RolleDomainError) { this.logger.error( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine Rolle ${result.name} zu bearbeiten. Fehler: ${result.message}`, + `Admin ${userName} (${userId}, ${organisationNameUser}) hat versucht eine Rolle ${params.name} zu bearbeiten. Fehler: ${result.message}`, ); throw result; } this.logger.error( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine Rolle ${result.name} zu bearbeiten. Fehler: ${result.message}`, + `Admin ${userName} (${userId}, ${organisationNameUser}) hat versucht eine Rolle ${params.name} zu bearbeiten. Fehler: ${result.message}`, ); throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(result), @@ -407,7 +413,7 @@ export class RolleController { } this.logger.info( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat eine Rolle bearbeitet: ${result.name}.`, + `Admin ${userName} (${userId}, ${organisationNameUser}) hat eine Rolle bearbeitet: ${rolleName}.`, ); return this.returnRolleWithServiceProvidersResponse(result); @@ -424,9 +430,12 @@ export class RolleController { @Param() findRolleByIdParams: FindRolleByIdParams, @Permissions() permissions: PersonPermissions, ): Promise { - const organisationNameUser: string = await this.getOrganisationNameForCurrentUser(permissions); + const userName: string = permissions.personFields.familienname; + const userId: string = permissions.personFields.id; + const organisationNameUser: string | void = + (await this.getOrganisationNameForCurrentUser(permissions)) ?? 'ORGANISATION_NAME_NOT_FOUND'; const rolle: Option> = await this.rolleRepo.findById(findRolleByIdParams.rolleId); - const rolleName: string = rolle?.name ?? 'ORGANISATION_NOT_FOUND'; + const rolleName: string = rolle?.name ?? 'ROLLE_NOT_FOUND'; const result: Option = await this.rolleRepo.deleteAuthorized( findRolleByIdParams.rolleId, @@ -434,14 +443,13 @@ export class RolleController { ); if (result instanceof DomainError) { if (result instanceof RolleDomainError) { - // Admin (, ) hat versucht die Rolle zu entfernen. error: <> this.logger.error( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht die Rolle ${rolleName} zu entfernen. Fehler: ${result.message}`, + `Admin ${userName} (${userId}, ${organisationNameUser}) hat versucht die Rolle ${rolleName} zu entfernen. Fehler: ${result.message}`, ); throw result; } this.logger.error( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht die Rolle ${rolleName} zu entfernen. Fehler: ${result.message}`, + `Admin ${userName} (${userId}, ${organisationNameUser}) hat versucht die Rolle ${rolleName} zu entfernen. Fehler: ${result.message}`, ); throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(result), @@ -449,7 +457,7 @@ export class RolleController { } this.logger.info( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat eine Rolle entfernt: ${rolleName}.`, + `Admin ${userName} (${userId}, ${organisationNameUser}) hat eine Rolle entfernt: ${rolleName}.`, ); } @@ -465,13 +473,16 @@ export class RolleController { return new RolleWithServiceProvidersResponse(rolle, rolleServiceProviders); } - private async getOrganisationNameForCurrentUser(permissions: PersonPermissions): Promise { + private async getOrganisationNameForCurrentUser(permissions: PersonPermissions): Promise { const personenkontextRolleFields: PersonenkontextRolleFields[] = await permissions?.getPersonenkontextewithRoles(); - const organisationIdUser: string = personenkontextRolleFields.at(0)?.organisationsId as string; - const organisationUser: Option> = - await this.organisationRepository.findById(organisationIdUser); - const organisationNameUser: string = organisationUser?.name ?? 'ORGANISATION_NOT_FOUND'; - return organisationNameUser; + const organisationIdUser: string | undefined = personenkontextRolleFields.at(0)?.organisationsId; + if (organisationIdUser) { + const organisationUser: Option> = + await this.organisationRepository.findById(organisationIdUser); + const organisationNameUser: string = organisationUser?.name ?? 'ORGANISATION_NOT_FOUND'; + return organisationNameUser; + } + this.logger.error(`No Organisation id found for given Person id: ${permissions.personFields.id}`); } } From 2297f3d18244b08e0726c469b828d67860541089 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Tue, 19 Nov 2024 16:58:09 +0100 Subject: [PATCH 14/29] Adapt tests --- .../api/rolle.controller.integration-spec.ts | 296 +++++++++++++++--- 1 file changed, 246 insertions(+), 50 deletions(-) diff --git a/src/modules/rolle/api/rolle.controller.integration-spec.ts b/src/modules/rolle/api/rolle.controller.integration-spec.ts index c3f5dcf10..7df089c2c 100644 --- a/src/modules/rolle/api/rolle.controller.integration-spec.ts +++ b/src/modules/rolle/api/rolle.controller.integration-spec.ts @@ -39,7 +39,7 @@ import { DBiamPersonenkontextRepoInternal } from '../../personenkontext/persiste import { PersonRepository } from '../../person/persistence/person.repository.js'; import { KeycloakUserService } from '../../keycloak-administration/domain/keycloak-user.service.js'; -import { PersonPermissions } from '../../authentication/domain/person-permissions.js'; +import { PersonenkontextRolleFields, PersonPermissions } from '../../authentication/domain/person-permissions.js'; import { Person } from '../../person/domain/person.js'; import { DomainError } from '../../../shared/error/domain.error.js'; import { PersonFactory } from '../../person/domain/person.factory.js'; @@ -47,6 +47,8 @@ import { KeycloakConfigModule } from '../../keycloak-administration/keycloak-con import { RolleServiceProviderBodyParams } from './rolle-service-provider.body.params.js'; import { generatePassword } from '../../../shared/util/password-generator.js'; import { Geschlecht } from '../../person/domain/person.enums.js'; +import { OrganisationRepository } from '../../organisation/persistence/organisation.repository.js'; +import { Organisation } from '../../organisation/domain/organisation.js'; describe('Rolle API', () => { let app: INestApplication; @@ -55,6 +57,7 @@ describe('Rolle API', () => { let rolleRepo: RolleRepo; let personRepo: PersonRepository; let serviceProviderRepo: ServiceProviderRepo; + let organisationRepo: OrganisationRepository; let dBiamPersonenkontextRepoInternal: DBiamPersonenkontextRepoInternal; let personpermissionsRepoMock: DeepMocked; let personPermissionsMock: DeepMocked; @@ -118,6 +121,7 @@ describe('Rolle API', () => { rolleRepo = module.get(RolleRepo); personRepo = module.get(PersonRepository); serviceProviderRepo = module.get(ServiceProviderRepo); + organisationRepo = module.get(OrganisationRepository); personFactory = module.get(PersonFactory); dBiamPersonenkontextRepoInternal = module.get(DBiamPersonenkontextRepoInternal); @@ -126,8 +130,21 @@ describe('Rolle API', () => { personPermissionsMock = createMock(); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personPermissionsMock); personPermissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); + await DatabaseTestModule.setupDatabase(module.get(MikroORM)); + app = module.createNestApplication(); + await app.init(); + }, 10000000); + + afterAll(async () => { + await orm.close(); + await app.close(); + }); + + beforeEach(async () => { + await DatabaseTestModule.clearDatabase(orm); + }); - // permissionsMock = createMock(); + describe('/POST rolle', () => { permissionsMock = createMock({ get personFields(): Person { return createMock>({ @@ -143,25 +160,18 @@ describe('Rolle API', () => { }); }, }); - personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); - - await DatabaseTestModule.setupDatabase(module.get(MikroORM)); - app = module.createNestApplication(); - await app.init(); - }, 10000000); - - afterAll(async () => { - await orm.close(); - await app.close(); - }); - - beforeEach(async () => { - await DatabaseTestModule.clearDatabase(orm); - }); - - describe('/POST rolle', () => { it('should return created rolle', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); @@ -185,6 +195,18 @@ describe('Rolle API', () => { }); it('should save rolle to db', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); @@ -205,6 +227,18 @@ describe('Rolle API', () => { }); it('should fail if the organisation does not exist', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + const params: CreateRolleBodyParams = { name: faker.person.jobTitle(), administeredBySchulstrukturknoten: faker.string.uuid(), @@ -221,6 +255,18 @@ describe('Rolle API', () => { }); it('should fail if rollenart is invalid', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); @@ -240,6 +286,18 @@ describe('Rolle API', () => { }); it('should fail if merkmal is invalid', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); @@ -259,6 +317,18 @@ describe('Rolle API', () => { }); it('should fail if merkmale are not unique', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); @@ -674,7 +744,34 @@ describe('Rolle API', () => { }); describe('/PUT rolle', () => { + permissionsMock = createMock({ + get personFields(): Person { + return createMock>({ + id: 'test-id', + keycloakUserId: 'test-keycloak', + vorname: 'test-vorname', + familienname: 'test-familienname', + rufname: 'test-rufname', + username: 'test-username', + geschlecht: Geschlecht.M, + geburtsdatum: faker.date.past(), + updatedAt: faker.date.recent(), + }); + }, + }); it('should return updated rolle', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); await em.findOneOrFail(OrganisationEntity, { id: organisation.id }); @@ -686,10 +783,7 @@ describe('Rolle API', () => { }), ); - personPermissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ - all: false, - orgaIds: [organisation.id], - }); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [organisation.id] }); const serviceProvider: ServiceProvider = await serviceProviderRepo.save( DoFactory.createServiceProvider(false), @@ -735,6 +829,15 @@ describe('Rolle API', () => { }); it('should return error with status-code 404 if user does NOT have permissions', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); await em.findOneOrFail(OrganisationEntity, { id: organisation.id }); @@ -746,9 +849,9 @@ describe('Rolle API', () => { }), ); - const personpermissions: DeepMocked = createMock(); - personpermissions.getOrgIdsWithSystemrecht.mockResolvedValueOnce({ all: false, orgaIds: [] }); - personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValueOnce({ all: false, orgaIds: [] }); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); const params: UpdateRolleBodyParams = { name: faker.person.jobTitle(), @@ -772,6 +875,15 @@ describe('Rolle API', () => { }); it('should return error with status-code 404 if rolle is technical', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); await em.findOneOrFail(OrganisationEntity, { id: organisation.id }); @@ -784,12 +896,9 @@ describe('Rolle API', () => { }), ); - const personpermissions: DeepMocked = createMock(); - personpermissions.getOrgIdsWithSystemrecht.mockResolvedValueOnce({ - all: false, - orgaIds: [organisation.id], - }); - personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValueOnce({ all: false, orgaIds: [organisation.id] }); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); const params: UpdateRolleBodyParams = { name: faker.person.jobTitle(), @@ -814,6 +923,15 @@ describe('Rolle API', () => { describe('Update Merkmale', () => { it('should return 400 if rolle is already assigned', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + const personData: Person | DomainError = await personFactory.createNew({ vorname: faker.person.firstName(), familienname: faker.person.lastName(), @@ -849,12 +967,9 @@ describe('Rolle API', () => { }), ); - const personpermissions: DeepMocked = createMock(); - personpermissions.getOrgIdsWithSystemrecht.mockResolvedValueOnce({ - all: false, - orgaIds: [organisation.id], - }); - personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [organisation.id] }); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); const params: UpdateRolleBodyParams = { name: faker.person.jobTitle(), @@ -877,6 +992,15 @@ describe('Rolle API', () => { }); it('should return error if new name has trailing space', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); @@ -901,6 +1025,10 @@ describe('Rolle API', () => { version: 1, }; + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [organisation.id] }); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + const response: Response = await request(app.getHttpServer() as App) .put(`/rolle/${rolle.id}`) .send(params); @@ -914,8 +1042,35 @@ describe('Rolle API', () => { }); describe('/DELETE rolleId', () => { + permissionsMock = createMock({ + get personFields(): Person { + return createMock>({ + id: 'test-id', + keycloakUserId: 'test-keycloak', + vorname: 'test-vorname', + familienname: 'test-familienname', + rufname: 'test-rufname', + username: 'test-username', + geschlecht: Geschlecht.M, + geburtsdatum: faker.date.past(), + updatedAt: faker.date.recent(), + }); + }, + }); describe('should return error', () => { it('if rolle does NOT exist', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + const response: Response = await request(app.getHttpServer() as App) .delete(`/rolle/${faker.string.uuid()}`) .send(); @@ -924,6 +1079,15 @@ describe('Rolle API', () => { }); it('if rolle is already assigned to a Personenkontext', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + const personData: Person | DomainError = await personFactory.createNew({ vorname: faker.person.firstName(), familienname: faker.person.lastName(), @@ -956,12 +1120,14 @@ describe('Rolle API', () => { organisationId: organisation.id, }), ); - const personpermissions: DeepMocked = createMock(); - personpermissions.getOrgIdsWithSystemrecht.mockResolvedValueOnce({ + + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValueOnce({ all: false, orgaIds: [organisation.id], }); - personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); const response: Response = await request(app.getHttpServer() as App) .delete(`/rolle/${rolle.id}`) @@ -975,6 +1141,15 @@ describe('Rolle API', () => { }); it('if user does NOT have permissions', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); await em.findOneOrFail(OrganisationEntity, { id: organisation.id }); @@ -986,12 +1161,13 @@ describe('Rolle API', () => { }), ); - const personpermissions: DeepMocked = createMock(); - personpermissions.getOrgIdsWithSystemrecht.mockResolvedValueOnce({ + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValueOnce({ all: false, orgaIds: [], }); - personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); const response: Response = await request(app.getHttpServer() as App) .delete(`/rolle/${rolle.id}`) @@ -1007,6 +1183,15 @@ describe('Rolle API', () => { }); it('if rolle is technical', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); await em.findOneOrFail(OrganisationEntity, { id: organisation.id }); @@ -1019,12 +1204,13 @@ describe('Rolle API', () => { }), ); - const personpermissions: DeepMocked = createMock(); - personpermissions.getOrgIdsWithSystemrecht.mockResolvedValueOnce({ + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValueOnce({ all: false, orgaIds: [organisation.id], }); - personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); const response: Response = await request(app.getHttpServer() as App) .delete(`/rolle/${rolle.id}`) @@ -1042,6 +1228,15 @@ describe('Rolle API', () => { describe('should succeed', () => { it('if all conditions are passed', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + const organisation: OrganisationEntity = new OrganisationEntity(); await em.persistAndFlush(organisation); await em.findOneOrFail(OrganisationEntity, { id: organisation.id }); @@ -1058,12 +1253,13 @@ describe('Rolle API', () => { }), ); - const personpermissions: DeepMocked = createMock(); - personpermissions.getOrgIdsWithSystemrecht.mockResolvedValueOnce({ + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValueOnce({ all: false, orgaIds: [organisation.id], }); - personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); const response: Response = await request(app.getHttpServer() as App) .delete(`/rolle/${rolle.id}`) From 491e36940edd0953f230238a0cacbd0384de9e3a Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Wed, 20 Nov 2024 09:28:49 +0100 Subject: [PATCH 15/29] Fix tests --- .../api/rolle.controller.integration-spec.ts | 71 +++++++------------ 1 file changed, 25 insertions(+), 46 deletions(-) diff --git a/src/modules/rolle/api/rolle.controller.integration-spec.ts b/src/modules/rolle/api/rolle.controller.integration-spec.ts index 7df089c2c..a08a7c72e 100644 --- a/src/modules/rolle/api/rolle.controller.integration-spec.ts +++ b/src/modules/rolle/api/rolle.controller.integration-spec.ts @@ -60,7 +60,6 @@ describe('Rolle API', () => { let organisationRepo: OrganisationRepository; let dBiamPersonenkontextRepoInternal: DBiamPersonenkontextRepoInternal; let personpermissionsRepoMock: DeepMocked; - let personPermissionsMock: DeepMocked; let personFactory: PersonFactory; let permissionsMock: DeepMocked; @@ -126,10 +125,6 @@ describe('Rolle API', () => { dBiamPersonenkontextRepoInternal = module.get(DBiamPersonenkontextRepoInternal); personpermissionsRepoMock = module.get(PersonPermissionsRepo); - - personPermissionsMock = createMock(); - personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personPermissionsMock); - personPermissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); await DatabaseTestModule.setupDatabase(module.get(MikroORM)); app = module.createNestApplication(); await app.init(); @@ -141,10 +136,6 @@ describe('Rolle API', () => { }); beforeEach(async () => { - await DatabaseTestModule.clearDatabase(orm); - }); - - describe('/POST rolle', () => { permissionsMock = createMock({ get personFields(): Person { return createMock>({ @@ -160,6 +151,12 @@ describe('Rolle API', () => { }); }, }); + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); + await DatabaseTestModule.clearDatabase(orm); + }); + + describe('/POST rolle', () => { it('should return created rolle', async () => { const userOrganisation: Organisation = DoFactory.createOrganisation(false); const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); @@ -358,7 +355,7 @@ describe('Rolle API', () => { ]) ).map((r: Rolle) => r.administeredBySchulstrukturknoten); - personPermissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds }); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds }); const response: Response = await request(app.getHttpServer() as App) .get('/rolle') @@ -388,7 +385,7 @@ describe('Rolle API', () => { DoFactory.createRolle(false), ); - personPermissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [testRolle.administeredBySchulstrukturknoten], }); @@ -422,7 +419,7 @@ describe('Rolle API', () => { ]) ).map((r: Rolle) => r.administeredBySchulstrukturknoten); - personPermissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds }); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds }); const response: Response = await request(app.getHttpServer() as App) .get('/rolle') @@ -474,7 +471,7 @@ describe('Rolle API', () => { ]) ).map((r: Rolle) => r.administeredBySchulstrukturknoten); - personPermissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds }); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds }); const response: Response = await request(app.getHttpServer() as App) .get('/rolle') @@ -492,7 +489,7 @@ describe('Rolle API', () => { it('should return rolle', async () => { const rolle: Rolle = await rolleRepo.save(DoFactory.createRolle(false)); - personPermissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [rolle.administeredBySchulstrukturknoten], }); @@ -513,7 +510,7 @@ describe('Rolle API', () => { DoFactory.createRolle(false, { serviceProviderIds: [serviceProvider.id] }), ); - personPermissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [rolle.administeredBySchulstrukturknoten], }); @@ -544,7 +541,7 @@ describe('Rolle API', () => { it('should return 404 when rolle is technical', async () => { const rolle: Rolle = await rolleRepo.save(DoFactory.createRolle(false, { istTechnisch: true })); - personPermissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [rolle.administeredBySchulstrukturknoten], }); @@ -744,21 +741,6 @@ describe('Rolle API', () => { }); describe('/PUT rolle', () => { - permissionsMock = createMock({ - get personFields(): Person { - return createMock>({ - id: 'test-id', - keycloakUserId: 'test-keycloak', - vorname: 'test-vorname', - familienname: 'test-familienname', - rufname: 'test-rufname', - username: 'test-username', - geschlecht: Geschlecht.M, - geburtsdatum: faker.date.past(), - updatedAt: faker.date.recent(), - }); - }, - }); it('should return updated rolle', async () => { const userOrganisation: Organisation = DoFactory.createOrganisation(false); const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); @@ -813,6 +795,18 @@ describe('Rolle API', () => { }); it('should fail if the rolle does not exist', async () => { + const userOrganisation: Organisation = DoFactory.createOrganisation(false); + const savedUserOrganisation: Organisation = await organisationRepo.save(userOrganisation); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: savedUserOrganisation.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; + personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); + permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + const params: UpdateRolleBodyParams = { name: faker.person.jobTitle(), merkmale: [faker.helpers.enumValue(RollenMerkmal)], @@ -1042,21 +1036,6 @@ describe('Rolle API', () => { }); describe('/DELETE rolleId', () => { - permissionsMock = createMock({ - get personFields(): Person { - return createMock>({ - id: 'test-id', - keycloakUserId: 'test-keycloak', - vorname: 'test-vorname', - familienname: 'test-familienname', - rufname: 'test-rufname', - username: 'test-username', - geschlecht: Geschlecht.M, - geburtsdatum: faker.date.past(), - updatedAt: faker.date.recent(), - }); - }, - }); describe('should return error', () => { it('if rolle does NOT exist', async () => { const userOrganisation: Organisation = DoFactory.createOrganisation(false); From f1a9c6df756715c9fb97113f4a8f50e9aca06fe5 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Wed, 20 Nov 2024 11:46:15 +0100 Subject: [PATCH 16/29] Adapt tests to increase coverage --- .../domain/organisation.service.spec.ts | 175 +++++++++++++++--- .../domain/organisation.service.ts | 4 +- ...rganisation.repository.integration-spec.ts | 36 +++- .../persistence/organisation.repository.ts | 1 - src/modules/rolle/api/rolle.controller.ts | 9 +- 5 files changed, 193 insertions(+), 32 deletions(-) diff --git a/src/modules/organisation/domain/organisation.service.spec.ts b/src/modules/organisation/domain/organisation.service.spec.ts index d83eb6ae3..f948dc73e 100644 --- a/src/modules/organisation/domain/organisation.service.spec.ts +++ b/src/modules/organisation/domain/organisation.service.spec.ts @@ -21,6 +21,9 @@ import { NameForOrganisationWithTrailingSpaceError } from '../specification/erro import { KennungForOrganisationWithTrailingSpaceError } from '../specification/error/kennung-with-trailing-space.error.js'; import { EmailAdressOnOrganisationTypError } from '../specification/error/email-adress-on-organisation-typ-error.js'; import { LoggingTestModule } from '../../../../test/utils/logging-test.module.js'; +import { PersonenkontextRolleFields, PersonPermissions } from '../../authentication/domain/person-permissions.js'; +import { Person } from '../../person/domain/person.js'; +import { Geschlecht } from '../../person/domain/person.enums.js'; describe('OrganisationService', () => { let module: TestingModule; @@ -61,11 +64,36 @@ describe('OrganisationService', () => { }); describe('createOrganisation', () => { + const permissionsMock: DeepMocked = createMock({ + get personFields(): Person { + return createMock>({ + id: 'test-id', + keycloakUserId: 'test-keycloak', + vorname: 'test-vorname', + familienname: 'test-familienname', + rufname: 'test-rufname', + username: 'test-username', + geschlecht: Geschlecht.M, + geburtsdatum: faker.date.past(), + updatedAt: faker.date.recent(), + }); + }, + }); + const organisationUser: Organisation = DoFactory.createOrganisation(true); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: organisationUser.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; it('should create an organisation', async () => { const organisation: Organisation = DoFactory.createOrganisation(false); organisationRepositoryMock.save.mockResolvedValue(organisation as unknown as Organisation); mapperMock.map.mockReturnValue(organisation as unknown as Dictionary); - const result: Result> = await organisationService.createOrganisation(organisation); + const result: Result> = await organisationService.createOrganisation( + organisation, + permissionsMock, + ); expect(result).toEqual>>({ ok: true, value: organisation as unknown as Organisation, @@ -77,7 +105,10 @@ describe('OrganisationService', () => { organisation.administriertVon = faker.string.uuid(); organisationRepositoryMock.exists.mockResolvedValueOnce(false); - const result: Result> = await organisationService.createOrganisation(organisation); + const result: Result> = await organisationService.createOrganisation( + organisation, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, @@ -90,7 +121,10 @@ describe('OrganisationService', () => { organisation.zugehoerigZu = faker.string.uuid(); organisationRepositoryMock.exists.mockResolvedValueOnce(false); - const result: Result> = await organisationService.createOrganisation(organisation); + const result: Result> = await organisationService.createOrganisation( + organisation, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, @@ -106,7 +140,10 @@ describe('OrganisationService', () => { organisationRepositoryMock.save.mockResolvedValue(organisation as unknown as Organisation); mapperMock.map.mockReturnValue(organisation as unknown as Dictionary); - const result: Result> = await organisationService.createOrganisation(organisation); + const result: Result> = await organisationService.createOrganisation( + organisation, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, @@ -115,6 +152,9 @@ describe('OrganisationService', () => { }); it('should return a domain error if name is not set and type is schule', async () => { + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + organisationRepositoryMock.findById.mockResolvedValue(organisationUser); + const organisation: Organisation = DoFactory.createOrganisation(false, { typ: OrganisationsTyp.SCHULE, kennung: '1234567', @@ -123,7 +163,10 @@ describe('OrganisationService', () => { organisationRepositoryMock.save.mockResolvedValue(organisation as unknown as Organisation); mapperMock.map.mockReturnValue(organisation as unknown as Dictionary); - const result: Result> = await organisationService.createOrganisation(organisation); + const result: Result> = await organisationService.createOrganisation( + organisation, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, @@ -140,7 +183,10 @@ describe('OrganisationService', () => { organisationRepositoryMock.save.mockResolvedValue(organisation as unknown as Organisation); mapperMock.map.mockReturnValue(organisation as unknown as Dictionary); - const result: Result> = await organisationService.createOrganisation(organisation); + const result: Result> = await organisationService.createOrganisation( + organisation, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, @@ -149,6 +195,9 @@ describe('OrganisationService', () => { }); it('should return a domain error if kennung is not unique and type is schule', async () => { + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + organisationRepositoryMock.findById.mockResolvedValue(organisationUser); + const name: string = faker.string.alpha(); const kennung: string = faker.string.numeric({ length: 7 }); const organisation: Organisation = DoFactory.createOrganisation(false, { @@ -170,7 +219,10 @@ describe('OrganisationService', () => { organisationRepositoryMock.save.mockResolvedValue(organisation as unknown as Organisation); mapperMock.map.mockReturnValue(organisation as unknown as Dictionary); - const result: Result> = await organisationService.createOrganisation(organisation); + const result: Result> = await organisationService.createOrganisation( + organisation, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, @@ -181,7 +233,10 @@ describe('OrganisationService', () => { it('should return a domain error', async () => { const organisation: Organisation = DoFactory.createOrganisation(false); organisation.id = faker.string.uuid(); - const result: Result> = await organisationService.createOrganisation(organisation); + const result: Result> = await organisationService.createOrganisation( + organisation, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, error: new EntityCouldNotBeCreated(`Organization could not be created`), @@ -191,7 +246,10 @@ describe('OrganisationService', () => { it('should return domain error if name contains trailing space', async () => { const organisationDo: Organisation = DoFactory.createOrganisation(false, { name: ' name' }); organisationRepositoryMock.exists.mockResolvedValueOnce(true).mockResolvedValueOnce(true); - const result: Result> = await organisationService.createOrganisation(organisationDo); + const result: Result> = await organisationService.createOrganisation( + organisationDo, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, error: new NameForOrganisationWithTrailingSpaceError(), @@ -201,7 +259,10 @@ describe('OrganisationService', () => { it('should return domain error if kennung contains trailing space', async () => { const organisationDo: Organisation = DoFactory.createOrganisation(false, { kennung: ' ' }); organisationRepositoryMock.exists.mockResolvedValueOnce(true).mockResolvedValueOnce(true); - const result: Result> = await organisationService.createOrganisation(organisationDo); + const result: Result> = await organisationService.createOrganisation( + organisationDo, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, error: new KennungForOrganisationWithTrailingSpaceError(), @@ -211,7 +272,10 @@ describe('OrganisationService', () => { it('should return domain error if name contains trailing space', async () => { const organisationDo: Organisation = DoFactory.createOrganisation(false, { name: ' name' }); organisationRepositoryMock.exists.mockResolvedValueOnce(true).mockResolvedValueOnce(true); - const result: Result> = await organisationService.createOrganisation(organisationDo); + const result: Result> = await organisationService.createOrganisation( + organisationDo, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, error: new NameForOrganisationWithTrailingSpaceError(), @@ -221,7 +285,10 @@ describe('OrganisationService', () => { it('should return domain error if kennung contains trailing space', async () => { const organisationDo: Organisation = DoFactory.createOrganisation(false, { kennung: ' ' }); organisationRepositoryMock.exists.mockResolvedValueOnce(true).mockResolvedValueOnce(true); - const result: Result> = await organisationService.createOrganisation(organisationDo); + const result: Result> = await organisationService.createOrganisation( + organisationDo, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, error: new KennungForOrganisationWithTrailingSpaceError(), @@ -230,10 +297,36 @@ describe('OrganisationService', () => { }); describe('updateOrganisation', () => { + const permissionsMock: DeepMocked = createMock({ + get personFields(): Person { + return createMock>({ + id: 'test-id', + keycloakUserId: 'test-keycloak', + vorname: 'test-vorname', + familienname: 'test-familienname', + rufname: 'test-rufname', + username: 'test-username', + geschlecht: Geschlecht.M, + geburtsdatum: faker.date.past(), + updatedAt: faker.date.recent(), + }); + }, + }); + const organisationUser: Organisation = DoFactory.createOrganisation(true); + const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ + { + organisationsId: organisationUser.id, + rolle: { systemrechte: [], serviceProviderIds: [] }, + }, + ]; it('should update an organisation', async () => { const organisation: Organisation = DoFactory.createOrganisation(true); organisationRepositoryMock.save.mockResolvedValue(organisation as unknown as Organisation); - const result: Result> = await organisationService.updateOrganisation(organisation); + organisationRepositoryMock.findById.mockResolvedValue(organisation); + const result: Result> = await organisationService.updateOrganisation( + organisation, + permissionsMock, + ); expect(result).toEqual>>({ ok: true, value: organisation as unknown as Organisation, @@ -244,7 +337,10 @@ describe('OrganisationService', () => { const organisation: Organisation = DoFactory.createOrganisation(true); organisation.id = ''; organisationRepositoryMock.findById.mockResolvedValue({} as Option>); - const result: Result> = await organisationService.updateOrganisation(organisation); + const result: Result> = await organisationService.updateOrganisation( + organisation, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, error: new EntityCouldNotBeUpdated(`Organization could not be updated`, organisation.id), @@ -258,7 +354,10 @@ describe('OrganisationService', () => { }); organisationRepositoryMock.findById.mockResolvedValue(organisation as unknown as Organisation); - const result: Result> = await organisationService.updateOrganisation(organisation); + const result: Result> = await organisationService.updateOrganisation( + organisation, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, @@ -267,6 +366,9 @@ describe('OrganisationService', () => { }); it('should return a domain error if name is not set and type is schule', async () => { + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + organisationRepositoryMock.findById.mockResolvedValue(organisationUser); + const organisation: Organisation = DoFactory.createOrganisation(true, { typ: OrganisationsTyp.SCHULE, kennung: '1234567', @@ -274,7 +376,10 @@ describe('OrganisationService', () => { }); organisationRepositoryMock.findById.mockResolvedValue(organisation as unknown as Organisation); - const result: Result> = await organisationService.updateOrganisation(organisation); + const result: Result> = await organisationService.updateOrganisation( + organisation, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, @@ -290,7 +395,10 @@ describe('OrganisationService', () => { }); organisationRepositoryMock.findById.mockResolvedValue(organisation as unknown as Organisation); - const result: Result> = await organisationService.updateOrganisation(organisation); + const result: Result> = await organisationService.updateOrganisation( + organisation, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, @@ -299,6 +407,9 @@ describe('OrganisationService', () => { }); it('should return a domain error if kennung is not unique and type is schule', async () => { + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + organisationRepositoryMock.findById.mockResolvedValue(organisationUser); + const name: string = faker.string.alpha(); const kennung: string = faker.string.numeric({ length: 7 }); const organisation: Organisation = DoFactory.createOrganisation(true, { @@ -312,7 +423,10 @@ describe('OrganisationService', () => { organisationRepositoryMock.save.mockResolvedValue(organisation as unknown as Organisation); mapperMock.map.mockReturnValue(organisation as unknown as Dictionary); - const result: Result> = await organisationService.updateOrganisation(organisation); + const result: Result> = await organisationService.updateOrganisation( + organisation, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, @@ -323,7 +437,10 @@ describe('OrganisationService', () => { it('should return a domain error when organisation cannot be found on update', async () => { const organisation: Organisation = DoFactory.createOrganisation(true); organisationRepositoryMock.findById.mockResolvedValue(undefined); - const result: Result> = await organisationService.updateOrganisation(organisation); + const result: Result> = await organisationService.updateOrganisation( + organisation, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, error: new EntityNotFoundError('Organisation', organisation.id), @@ -333,7 +450,10 @@ describe('OrganisationService', () => { it('should return domain error if name contains trailing space', async () => { const organisationDo: Organisation = DoFactory.createOrganisation(true, { name: ' ' }); organisationRepositoryMock.findById.mockResolvedValueOnce(DoFactory.createOrganisation(true)); - const result: Result> = await organisationService.updateOrganisation(organisationDo); + const result: Result> = await organisationService.updateOrganisation( + organisationDo, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, error: new NameForOrganisationWithTrailingSpaceError(), @@ -343,7 +463,10 @@ describe('OrganisationService', () => { it('should return domain error if kennung contains trailing space', async () => { const organisationDo: Organisation = DoFactory.createOrganisation(true, { kennung: 'kennung ' }); organisationRepositoryMock.findById.mockResolvedValueOnce(DoFactory.createOrganisation(true)); - const result: Result> = await organisationService.updateOrganisation(organisationDo); + const result: Result> = await organisationService.updateOrganisation( + organisationDo, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, error: new KennungForOrganisationWithTrailingSpaceError(), @@ -353,7 +476,10 @@ describe('OrganisationService', () => { it('should return domain error if name contains trailing space', async () => { const organisationDo: Organisation = DoFactory.createOrganisation(true, { name: ' ' }); organisationRepositoryMock.findById.mockResolvedValueOnce(DoFactory.createOrganisation(true)); - const result: Result> = await organisationService.updateOrganisation(organisationDo); + const result: Result> = await organisationService.updateOrganisation( + organisationDo, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, error: new NameForOrganisationWithTrailingSpaceError(), @@ -363,7 +489,10 @@ describe('OrganisationService', () => { it('should return domain error if kennung contains trailing space', async () => { const organisationDo: Organisation = DoFactory.createOrganisation(true, { kennung: 'kennung ' }); organisationRepositoryMock.findById.mockResolvedValueOnce(DoFactory.createOrganisation(true)); - const result: Result> = await organisationService.updateOrganisation(organisationDo); + const result: Result> = await organisationService.updateOrganisation( + organisationDo, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, error: new KennungForOrganisationWithTrailingSpaceError(), diff --git a/src/modules/organisation/domain/organisation.service.ts b/src/modules/organisation/domain/organisation.service.ts index 9876aa1ed..fc1348de6 100644 --- a/src/modules/organisation/domain/organisation.service.ts +++ b/src/modules/organisation/domain/organisation.service.ts @@ -73,7 +73,7 @@ export class OrganisationService { if (organisation.typ === OrganisationsTyp.SCHULE) { if (permissions) { const personenkontextRolleFields: PersonenkontextRolleFields[] = - await permissions?.getPersonenkontextewithRoles(); + await permissions.getPersonenkontextewithRoles(); const organisationIdUser: string = personenkontextRolleFields.at(0)?.organisationsId as string; const organisationUser: Option> = await this.organisationRepo.findById(organisationIdUser); @@ -118,7 +118,7 @@ export class OrganisationService { if (organisation.typ === OrganisationsTyp.SCHULE) { if (permissions) { const personenkontextRolleFields: PersonenkontextRolleFields[] = - await permissions?.getPersonenkontextewithRoles(); + await permissions.getPersonenkontextewithRoles(); const organisationIdUser: string = personenkontextRolleFields.at(0)?.organisationsId as string; const organisationUser: Option> = await this.organisationRepo.findById(organisationIdUser); diff --git a/src/modules/organisation/persistence/organisation.repository.integration-spec.ts b/src/modules/organisation/persistence/organisation.repository.integration-spec.ts index 3f5938586..251f6db19 100644 --- a/src/modules/organisation/persistence/organisation.repository.integration-spec.ts +++ b/src/modules/organisation/persistence/organisation.repository.integration-spec.ts @@ -28,6 +28,8 @@ import { PersonPermissions } from '../../authentication/domain/person-permission import { RollenSystemRecht } from '../../rolle/domain/rolle.enums.js'; import { OrganisationUpdateOutdatedError } from '../domain/orga-update-outdated.error.js'; import { LoggingTestModule } from '../../../../test/utils/logging-test.module.js'; +import { Person } from '../../person/domain/person.js'; +import { Geschlecht } from '../../person/domain/person.enums.js'; describe('OrganisationRepository', () => { let module: TestingModule; @@ -790,12 +792,27 @@ describe('OrganisationRepository', () => { describe('deleteKlasse', () => { describe('when all validations succeed', () => { it('should succeed', async () => { + const permissionsMock: PersonPermissions = createMock({ + get personFields(): Person { + return createMock>({ + id: 'test-id', + keycloakUserId: 'test-keycloak', + vorname: 'test-vorname', + familienname: 'test-familienname', + rufname: 'test-rufname', + username: 'test-username', + geschlecht: Geschlecht.M, + geburtsdatum: faker.date.past(), + updatedAt: faker.date.recent(), + }); + }, + }); const organisation: Organisation = DoFactory.createOrganisationAggregate(false, { typ: OrganisationsTyp.KLASSE, }); const savedOrganisaiton: Organisation = await sut.save(organisation); - await sut.deleteKlasse(savedOrganisaiton.id); + await sut.deleteKlasse(savedOrganisaiton.id, permissionsMock); const exists: boolean = await sut.exists(savedOrganisaiton.id); expect(exists).toBe(false); @@ -825,6 +842,21 @@ describe('OrganisationRepository', () => { }); }); describe('updateKlassenname', () => { + const permissionsMock: PersonPermissions = createMock({ + get personFields(): Person { + return createMock>({ + id: 'test-id', + keycloakUserId: 'test-keycloak', + vorname: 'test-vorname', + familienname: 'test-familienname', + rufname: 'test-rufname', + username: 'test-username', + geschlecht: Geschlecht.M, + geburtsdatum: faker.date.past(), + updatedAt: faker.date.recent(), + }); + }, + }); describe('when organisation does not exist', () => { it('should return EntityNotFoundError', async () => { const id: string = faker.string.uuid(); @@ -867,6 +899,7 @@ describe('OrganisationRepository', () => { savedOrganisaiton.id, '', faker.number.int(), + permissionsMock, ); expect(result).toBeInstanceOf(OrganisationSpecificationError); @@ -910,6 +943,7 @@ describe('OrganisationRepository', () => { organisationEntity2.id, 'newName', 1, + permissionsMock, ); expect(result).not.toBeInstanceOf(DomainError); diff --git a/src/modules/organisation/persistence/organisation.repository.ts b/src/modules/organisation/persistence/organisation.repository.ts index a8f0cdfcf..e1284b662 100644 --- a/src/modules/organisation/persistence/organisation.repository.ts +++ b/src/modules/organisation/persistence/organisation.repository.ts @@ -434,7 +434,6 @@ export class OrganisationRepository { const school: Option> = await this.findById(organisationEntity.zugehoerigZu); schoolName = school?.name ?? 'SCHOOL_NOT_FOUND'; this.logger.info( - // Admin () hat eine Klasse entfernt: ( < Schule >). `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}) hat eine Klasse entfernt: ${organisationEntity.name} (${schoolName}).`, ); } diff --git a/src/modules/rolle/api/rolle.controller.ts b/src/modules/rolle/api/rolle.controller.ts index 2bb548342..184908987 100644 --- a/src/modules/rolle/api/rolle.controller.ts +++ b/src/modules/rolle/api/rolle.controller.ts @@ -378,8 +378,7 @@ export class RolleController { ): Promise { const userName: string = permissions.personFields.familienname; const userId: string = permissions.personFields.id; - const organisationNameUser: string | void = - (await this.getOrganisationNameForCurrentUser(permissions)) ?? 'ORGANISATION_NAME_NOT_FOUND'; + const organisationNameUser: string = await this.getOrganisationNameForCurrentUser(permissions); const rolle: Option> = await this.rolleRepo.findById(findRolleByIdParams.rolleId); const rolleName: string = rolle?.name ?? 'ROLLE_NOT_FOUND'; @@ -432,8 +431,7 @@ export class RolleController { ): Promise { const userName: string = permissions.personFields.familienname; const userId: string = permissions.personFields.id; - const organisationNameUser: string | void = - (await this.getOrganisationNameForCurrentUser(permissions)) ?? 'ORGANISATION_NAME_NOT_FOUND'; + const organisationNameUser: string = await this.getOrganisationNameForCurrentUser(permissions); const rolle: Option> = await this.rolleRepo.findById(findRolleByIdParams.rolleId); const rolleName: string = rolle?.name ?? 'ROLLE_NOT_FOUND'; @@ -473,7 +471,7 @@ export class RolleController { return new RolleWithServiceProvidersResponse(rolle, rolleServiceProviders); } - private async getOrganisationNameForCurrentUser(permissions: PersonPermissions): Promise { + private async getOrganisationNameForCurrentUser(permissions: PersonPermissions): Promise { const personenkontextRolleFields: PersonenkontextRolleFields[] = await permissions?.getPersonenkontextewithRoles(); const organisationIdUser: string | undefined = personenkontextRolleFields.at(0)?.organisationsId; @@ -484,5 +482,6 @@ export class RolleController { return organisationNameUser; } this.logger.error(`No Organisation id found for given Person id: ${permissions.personFields.id}`); + return 'ORGANISATION_NOT_FOUND'; } } From eda234a9dd73dcb5b2fa93a75773af4e2a0f1de4 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Wed, 20 Nov 2024 14:07:15 +0100 Subject: [PATCH 17/29] Add and adapt tests to increase coverage --- .../domain/organisation.service.spec.ts | 90 ++++++++++++++++++- src/modules/rolle/api/rolle.controller.ts | 6 +- 2 files changed, 91 insertions(+), 5 deletions(-) diff --git a/src/modules/organisation/domain/organisation.service.spec.ts b/src/modules/organisation/domain/organisation.service.spec.ts index f948dc73e..7f3b46e10 100644 --- a/src/modules/organisation/domain/organisation.service.spec.ts +++ b/src/modules/organisation/domain/organisation.service.spec.ts @@ -24,6 +24,7 @@ import { LoggingTestModule } from '../../../../test/utils/logging-test.module.js import { PersonenkontextRolleFields, PersonPermissions } from '../../authentication/domain/person-permissions.js'; import { Person } from '../../person/domain/person.js'; import { Geschlecht } from '../../person/domain/person.enums.js'; +import { KlasseNurVonSchuleAdministriertError } from '../specification/error/klasse-nur-von-schule-administriert.error.js'; describe('OrganisationService', () => { let module: TestingModule; @@ -100,6 +101,46 @@ describe('OrganisationService', () => { }); }); + it('should create a Schule and log its creation', async () => { + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + organisationRepositoryMock.findById.mockResolvedValue(organisationUser); + const schule: Organisation = DoFactory.createOrganisation(false); + schule.typ = OrganisationsTyp.SCHULE; + organisationRepositoryMock.findBy.mockResolvedValueOnce([[], 0]); + organisationRepositoryMock.save.mockResolvedValue(schule as unknown as Organisation); + mapperMock.map.mockReturnValue(schule as unknown as Dictionary); + + const result: Result> = await organisationService.createOrganisation( + schule, + permissionsMock, + ); + + expect(result).toEqual>>({ + ok: true, + value: schule as unknown as Organisation, + }); + }); + + it('should fail to create a Klasse and log the creation attempt', async () => { + const schule: Organisation = DoFactory.createOrganisation(true); + organisationRepositoryMock.findById.mockResolvedValueOnce(schule); + const klasse: Organisation = DoFactory.createOrganisation(false); + klasse.typ = OrganisationsTyp.KLASSE; + klasse.zugehoerigZu = schule.id; + organisationRepositoryMock.save.mockResolvedValue(klasse as unknown as Organisation); + mapperMock.map.mockReturnValue(klasse as unknown as Dictionary); + + const result: Result> = await organisationService.createOrganisation( + klasse, + permissionsMock, + ); + + expect(result).toEqual>>({ + ok: false, + error: new KlasseNurVonSchuleAdministriertError(), + }); + }); + it('should return a domain error if first parent organisation does not exist', async () => { const organisation: Organisation = DoFactory.createOrganisation(false); organisation.administriertVon = faker.string.uuid(); @@ -133,6 +174,9 @@ describe('OrganisationService', () => { }); it('should return a domain error if kennung is not set and type is schule', async () => { + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + organisationRepositoryMock.findById.mockResolvedValue(organisationUser); + const organisation: Organisation = DoFactory.createOrganisation(false, { typ: OrganisationsTyp.SCHULE, kennung: undefined, @@ -333,6 +377,48 @@ describe('OrganisationService', () => { }); }); + it('should update a Schule and log the update', async () => { + const schule: Organisation = DoFactory.createOrganisation(true); + schule.typ = OrganisationsTyp.SCHULE; + organisationRepositoryMock.findById.mockResolvedValueOnce(schule); + organisationRepositoryMock.findBy.mockResolvedValueOnce([[], 0]); + organisationRepositoryMock.save.mockResolvedValue(schule as unknown as Organisation); + mapperMock.map.mockReturnValue(schule as unknown as Dictionary); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + organisationRepositoryMock.findById.mockResolvedValue(organisationUser); + + const result: Result> = await organisationService.updateOrganisation( + schule, + permissionsMock, + ); + + expect(result).toEqual>>({ + ok: true, + value: schule as unknown as Organisation, + }); + }); + + it('should fail to update a Klasse and log the update attempt', async () => { + const schule: Organisation = DoFactory.createOrganisation(true); + const klasse: Organisation = DoFactory.createOrganisation(true); + klasse.typ = OrganisationsTyp.KLASSE; + klasse.zugehoerigZu = schule.id; + organisationRepositoryMock.findById.mockResolvedValueOnce(klasse); + organisationRepositoryMock.save.mockResolvedValue(klasse as unknown as Organisation); + organisationRepositoryMock.findById.mockResolvedValueOnce(schule); + mapperMock.map.mockReturnValue(klasse as unknown as Dictionary); + + const result: Result> = await organisationService.updateOrganisation( + klasse, + permissionsMock, + ); + + expect(result).toEqual>>({ + ok: false, + error: new KlasseNurVonSchuleAdministriertError(klasse.id), + }); + }); + it('should return a domain error', async () => { const organisation: Organisation = DoFactory.createOrganisation(true); organisation.id = ''; @@ -352,7 +438,9 @@ describe('OrganisationService', () => { typ: OrganisationsTyp.SCHULE, kennung: undefined, }); - organisationRepositoryMock.findById.mockResolvedValue(organisation as unknown as Organisation); + organisationRepositoryMock.findById.mockResolvedValueOnce(organisation as unknown as Organisation); + permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); + organisationRepositoryMock.findById.mockResolvedValueOnce(organisationUser); const result: Result> = await organisationService.updateOrganisation( organisation, diff --git a/src/modules/rolle/api/rolle.controller.ts b/src/modules/rolle/api/rolle.controller.ts index 184908987..332c398df 100644 --- a/src/modules/rolle/api/rolle.controller.ts +++ b/src/modules/rolle/api/rolle.controller.ts @@ -184,8 +184,7 @@ export class RolleController { @Body() params: CreateRolleBodyParams, @Permissions() permissions: PersonPermissions, ): Promise { - const organisationNameUser: string | void = - (await this.getOrganisationNameForCurrentUser(permissions)) ?? 'ORGANISATION_NAME_NOT_FOUND'; + const organisationNameUser: string = await this.getOrganisationNameForCurrentUser(permissions); const orgResult: Result, DomainError> = await this.orgService.findOrganisationById( params.administeredBySchulstrukturknoten, @@ -478,8 +477,7 @@ export class RolleController { if (organisationIdUser) { const organisationUser: Option> = await this.organisationRepository.findById(organisationIdUser); - const organisationNameUser: string = organisationUser?.name ?? 'ORGANISATION_NOT_FOUND'; - return organisationNameUser; + if (organisationUser) if (organisationUser.name) return organisationUser.name; } this.logger.error(`No Organisation id found for given Person id: ${permissions.personFields.id}`); return 'ORGANISATION_NOT_FOUND'; From 7b4d908ca2cce64a93cd3041c2cc244866e6c98a Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Wed, 20 Nov 2024 15:57:15 +0100 Subject: [PATCH 18/29] Add tests for coverage --- .../domain/organisation.service.spec.ts | 59 ++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/src/modules/organisation/domain/organisation.service.spec.ts b/src/modules/organisation/domain/organisation.service.spec.ts index 7f3b46e10..f2e7946ca 100644 --- a/src/modules/organisation/domain/organisation.service.spec.ts +++ b/src/modules/organisation/domain/organisation.service.spec.ts @@ -103,7 +103,7 @@ describe('OrganisationService', () => { it('should create a Schule and log its creation', async () => { permissionsMock.getPersonenkontextewithRoles.mockResolvedValue(personenkontextewithRolesMock); - organisationRepositoryMock.findById.mockResolvedValue(organisationUser); + organisationRepositoryMock.findById.mockResolvedValueOnce(organisationUser); const schule: Organisation = DoFactory.createOrganisation(false); schule.typ = OrganisationsTyp.SCHULE; organisationRepositoryMock.findBy.mockResolvedValueOnce([[], 0]); @@ -121,12 +121,41 @@ describe('OrganisationService', () => { }); }); + it('should create a Klasse and log its creation', async () => { + const schule: Organisation = DoFactory.createOrganisation(true); + const klasse: Organisation = DoFactory.createOrganisation(false); + schule.typ = OrganisationsTyp.SCHULE; + klasse.typ = OrganisationsTyp.KLASSE; + klasse.administriertVon = schule.id; + klasse.zugehoerigZu = schule.id; + organisationRepositoryMock.findById.mockResolvedValueOnce(schule); + organisationRepositoryMock.findById.mockResolvedValueOnce(schule); + organisationRepositoryMock.findById.mockResolvedValueOnce(schule); + organisationRepositoryMock.findById.mockResolvedValueOnce(schule); + organisationRepositoryMock.save.mockResolvedValue(klasse as unknown as Organisation); + organisationRepositoryMock.findById.mockResolvedValueOnce(schule); + mapperMock.map.mockReturnValue(klasse as unknown as Dictionary); + + const result: Result> = await organisationService.createOrganisation( + klasse, + permissionsMock, + ); + + expect(result).toEqual>>({ + ok: true, + value: klasse as unknown as Organisation, + }); + }); + it('should fail to create a Klasse and log the creation attempt', async () => { const schule: Organisation = DoFactory.createOrganisation(true); organisationRepositoryMock.findById.mockResolvedValueOnce(schule); const klasse: Organisation = DoFactory.createOrganisation(false); klasse.typ = OrganisationsTyp.KLASSE; klasse.zugehoerigZu = schule.id; + klasse.administriertVon = schule.id; + organisationRepositoryMock.exists.mockResolvedValue(true); + organisationRepositoryMock.exists.mockResolvedValue(true); organisationRepositoryMock.save.mockResolvedValue(klasse as unknown as Organisation); mapperMock.map.mockReturnValue(klasse as unknown as Dictionary); @@ -398,6 +427,34 @@ describe('OrganisationService', () => { }); }); + it('should update a Klasse and log the update', async () => { + const schule: Organisation = DoFactory.createOrganisation(true); + const klasse: Organisation = DoFactory.createOrganisation(true); + schule.typ = OrganisationsTyp.SCHULE; + klasse.typ = OrganisationsTyp.KLASSE; + klasse.administriertVon = schule.id; + klasse.zugehoerigZu = schule.id; + organisationRepositoryMock.findById.mockResolvedValueOnce(klasse); + organisationRepositoryMock.findById.mockResolvedValueOnce(schule); + organisationRepositoryMock.findById.mockResolvedValueOnce(schule); + organisationRepositoryMock.findById.mockResolvedValueOnce(schule); + organisationRepositoryMock.findById.mockResolvedValueOnce(schule); + organisationRepositoryMock.findChildOrgasForIds.mockResolvedValueOnce([]); + organisationRepositoryMock.save.mockResolvedValue(klasse as unknown as Organisation); + organisationRepositoryMock.findById.mockResolvedValueOnce(schule); + mapperMock.map.mockReturnValue(klasse as unknown as Dictionary); + + const result: Result> = await organisationService.updateOrganisation( + klasse, + permissionsMock, + ); + + expect(result).toEqual>>({ + ok: true, + value: klasse as unknown as Organisation, + }); + }); + it('should fail to update a Klasse and log the update attempt', async () => { const schule: Organisation = DoFactory.createOrganisation(true); const klasse: Organisation = DoFactory.createOrganisation(true); From 8ab0fb6c2f688c1b21f97d308d03a7ad046ddbdb Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Wed, 20 Nov 2024 16:14:44 +0100 Subject: [PATCH 19/29] Adapt for coverage --- src/modules/organisation/domain/organisation.service.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/modules/organisation/domain/organisation.service.ts b/src/modules/organisation/domain/organisation.service.ts index fc1348de6..02f84fb4b 100644 --- a/src/modules/organisation/domain/organisation.service.ts +++ b/src/modules/organisation/domain/organisation.service.ts @@ -77,7 +77,8 @@ export class OrganisationService { const organisationIdUser: string = personenkontextRolleFields.at(0)?.organisationsId as string; const organisationUser: Option> = await this.organisationRepo.findById(organisationIdUser); - const organisationNameUser: string = organisationUser?.name ?? 'ORGANISATION_NOT_FOUND'; + let organisationNameUser: string = 'ORGANISATION_NOT_FOUND'; + if (organisationUser) if (organisationUser.name) organisationNameUser = organisationUser.name; if (error) { this.logger.error( `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine neue Schule ${organisation.name} anzulegen. Fehler: ${error.message}`, @@ -101,7 +102,8 @@ export class OrganisationService { const school: Option> = await this.organisationRepo.findById( organisation.zugehoerigZu, ); - const schoolName: string = school?.name ?? 'SCHOOL_NOT_FOUND'; + let schoolName: string = 'SCHOOL_NOT_FOUND'; + if (school) if (school.name) schoolName = school.name; if (permissions) { if (error) { this.logger.error( From f7edf9258d2864ec697ed434b20a8339400b6db5 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Fri, 22 Nov 2024 13:40:11 +0100 Subject: [PATCH 20/29] Log username --- .../domain/organisation.service.ts | 16 +++++++------- .../persistence/organisation.repository.ts | 6 ++--- src/modules/rolle/api/rolle.controller.ts | 22 ++++++++----------- 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/modules/organisation/domain/organisation.service.ts b/src/modules/organisation/domain/organisation.service.ts index 02f84fb4b..34890650d 100644 --- a/src/modules/organisation/domain/organisation.service.ts +++ b/src/modules/organisation/domain/organisation.service.ts @@ -60,11 +60,11 @@ export class OrganisationService { if (permissions) { if (error) { this.logger.error( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}) hat versucht eine neue Klasse ${organisation.name} (${schoolName}) anzulegen. Fehler: ${error.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht eine neue Klasse ${organisation.name} (${schoolName}) anzulegen. Fehler: ${error.message}`, ); } else { this.logger.info( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}) hat eine neue Klasse angelegt: ${organisation.name} (${schoolName}).`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat eine neue Klasse angelegt: ${organisation.name} (${schoolName}).`, ); } } @@ -81,11 +81,11 @@ export class OrganisationService { if (organisationUser) if (organisationUser.name) organisationNameUser = organisationUser.name; if (error) { this.logger.error( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine neue Schule ${organisation.name} anzulegen. Fehler: ${error.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine neue Schule ${organisation.name} anzulegen. Fehler: ${error.message}`, ); } else { this.logger.info( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat eine neue Schule angelegt: ${organisation.name}.`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat eine neue Schule angelegt: ${organisation.name}.`, ); } } @@ -107,11 +107,11 @@ export class OrganisationService { if (permissions) { if (error) { this.logger.error( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}) hat versucht eine Klasse ${organisation.name} (${schoolName}) zu verändern. Fehler: ${error.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht eine Klasse ${organisation.name} (${schoolName}) zu verändern. Fehler: ${error.message}`, ); } else { this.logger.info( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}) hat eine Klasse geändert: ${organisation.name} (${schoolName}).`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat eine Klasse geändert: ${organisation.name} (${schoolName}).`, ); } } @@ -127,11 +127,11 @@ export class OrganisationService { const organisationNameUser: string = organisationUser?.name ?? 'ORGANISATION_NOT_FOUND'; if (error) { this.logger.error( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine Schule ${organisation.name} zu verändern. Fehler: ${error.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine Schule ${organisation.name} zu verändern. Fehler: ${error.message}`, ); } else { this.logger.info( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat eine Schule geändert: ${organisation.name}.`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat eine Schule geändert: ${organisation.name}.`, ); } } diff --git a/src/modules/organisation/persistence/organisation.repository.ts b/src/modules/organisation/persistence/organisation.repository.ts index e1284b662..631ec0219 100644 --- a/src/modules/organisation/persistence/organisation.repository.ts +++ b/src/modules/organisation/persistence/organisation.repository.ts @@ -434,7 +434,7 @@ export class OrganisationRepository { const school: Option> = await this.findById(organisationEntity.zugehoerigZu); schoolName = school?.name ?? 'SCHOOL_NOT_FOUND'; this.logger.info( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}) hat eine Klasse entfernt: ${organisationEntity.name} (${schoolName}).`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat eine Klasse entfernt: ${organisationEntity.name} (${schoolName}).`, ); } @@ -470,7 +470,7 @@ export class OrganisationRepository { if (specificationError) { if (permissions) this.logger.error( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}) hat versucht den Namen einer Klasse ${organisationFound.name} (${schoolName}) zu verändern. Fehler: ${specificationError.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht den Namen einer Klasse ${organisationFound.name} (${schoolName}) zu verändern. Fehler: ${specificationError.message}`, ); return specificationError; } @@ -482,7 +482,7 @@ export class OrganisationRepository { this.eventService.publish(new KlasseUpdatedEvent(id, newName, organisationFound.administriertVon)); if (permissions) this.logger.info( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}) hat den Namen einer Klasse geändert: ${organisationFound.name} (${schoolName}).`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat den Namen einer Klasse geändert: ${organisationFound.name} (${schoolName}).`, ); return organisationEntity; } diff --git a/src/modules/rolle/api/rolle.controller.ts b/src/modules/rolle/api/rolle.controller.ts index 332c398df..61ca926a3 100644 --- a/src/modules/rolle/api/rolle.controller.ts +++ b/src/modules/rolle/api/rolle.controller.ts @@ -192,7 +192,7 @@ export class RolleController { if (!orgResult.ok) { this.logger.info( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine neue Rolle ${params.name} anzulegen. Fehler: ${orgResult.error.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine neue Rolle ${params.name} anzulegen. Fehler: ${orgResult.error.message}`, ); throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(orgResult.error), @@ -212,14 +212,14 @@ export class RolleController { if (rolle instanceof DomainError) { this.logger.error( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine neue Rolle ${params.name} anzulegen. Fehler: ${rolle.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine neue Rolle ${params.name} anzulegen. Fehler: ${rolle.message}`, ); throw rolle; } const result: Rolle = await this.rolleRepo.save(rolle); this.logger.info( - `Admin ${permissions.personFields.familienname} (${permissions.personFields.id}, ${organisationNameUser}) hat eine neue Rolle angelegt: ${result.name}.`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat eine neue Rolle angelegt: ${result.name}.`, ); return new RolleResponse(result); @@ -375,8 +375,6 @@ export class RolleController { @Body() params: UpdateRolleBodyParams, @Permissions() permissions: PersonPermissions, ): Promise { - const userName: string = permissions.personFields.familienname; - const userId: string = permissions.personFields.id; const organisationNameUser: string = await this.getOrganisationNameForCurrentUser(permissions); const rolle: Option> = await this.rolleRepo.findById(findRolleByIdParams.rolleId); const rolleName: string = rolle?.name ?? 'ROLLE_NOT_FOUND'; @@ -398,12 +396,12 @@ export class RolleController { if (result instanceof DomainError) { if (result instanceof RolleDomainError) { this.logger.error( - `Admin ${userName} (${userId}, ${organisationNameUser}) hat versucht eine Rolle ${params.name} zu bearbeiten. Fehler: ${result.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine Rolle ${params.name} zu bearbeiten. Fehler: ${result.message}`, ); throw result; } this.logger.error( - `Admin ${userName} (${userId}, ${organisationNameUser}) hat versucht eine Rolle ${params.name} zu bearbeiten. Fehler: ${result.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine Rolle ${params.name} zu bearbeiten. Fehler: ${result.message}`, ); throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(result), @@ -411,7 +409,7 @@ export class RolleController { } this.logger.info( - `Admin ${userName} (${userId}, ${organisationNameUser}) hat eine Rolle bearbeitet: ${rolleName}.`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat eine Rolle bearbeitet: ${rolleName}.`, ); return this.returnRolleWithServiceProvidersResponse(result); @@ -428,8 +426,6 @@ export class RolleController { @Param() findRolleByIdParams: FindRolleByIdParams, @Permissions() permissions: PersonPermissions, ): Promise { - const userName: string = permissions.personFields.familienname; - const userId: string = permissions.personFields.id; const organisationNameUser: string = await this.getOrganisationNameForCurrentUser(permissions); const rolle: Option> = await this.rolleRepo.findById(findRolleByIdParams.rolleId); const rolleName: string = rolle?.name ?? 'ROLLE_NOT_FOUND'; @@ -441,12 +437,12 @@ export class RolleController { if (result instanceof DomainError) { if (result instanceof RolleDomainError) { this.logger.error( - `Admin ${userName} (${userId}, ${organisationNameUser}) hat versucht die Rolle ${rolleName} zu entfernen. Fehler: ${result.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht die Rolle ${rolleName} zu entfernen. Fehler: ${result.message}`, ); throw result; } this.logger.error( - `Admin ${userName} (${userId}, ${organisationNameUser}) hat versucht die Rolle ${rolleName} zu entfernen. Fehler: ${result.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht die Rolle ${rolleName} zu entfernen. Fehler: ${result.message}`, ); throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(result), @@ -454,7 +450,7 @@ export class RolleController { } this.logger.info( - `Admin ${userName} (${userId}, ${organisationNameUser}) hat eine Rolle entfernt: ${rolleName}.`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat eine Rolle entfernt: ${rolleName}.`, ); } From 829975a4d8da032fbd4be615758dd34e0e5d14d4 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Mon, 25 Nov 2024 15:07:39 +0100 Subject: [PATCH 21/29] Use empty mock in tests --- .../api/organisation.controller.spec.ts | 48 ++----------------- 1 file changed, 4 insertions(+), 44 deletions(-) diff --git a/src/modules/organisation/api/organisation.controller.spec.ts b/src/modules/organisation/api/organisation.controller.spec.ts index 24203a5cf..b559f02f3 100644 --- a/src/modules/organisation/api/organisation.controller.spec.ts +++ b/src/modules/organisation/api/organisation.controller.spec.ts @@ -16,7 +16,7 @@ import { OrganisationByIdBodyParams } from './organisation-by-id.body.params.js' import { OrganisationRepository } from '../persistence/organisation.repository.js'; import { Organisation } from '../domain/organisation.js'; import { OrganisationResponse } from './organisation.response.js'; -import { PersonenkontextRolleFields, PersonPermissions } from '../../authentication/domain/person-permissions.js'; +import { PersonPermissions } from '../../authentication/domain/person-permissions.js'; import { EventService } from '../../../core/eventbus/index.js'; import { OrganisationRootChildrenResponse } from './organisation.root-children.response.js'; import { OrganisationSpecificationError } from '../specification/error/organisation-specification.error.js'; @@ -30,7 +30,6 @@ import { OrganisationByNameQueryParams } from './organisation-by-name.query.js'; import { DBiamPersonenkontextRepo } from '../../personenkontext/persistence/dbiam-personenkontext.repo.js'; import { ParentOrganisationenResponse } from './organisation.parents.response.js'; import { ParentOrganisationsByIdsBodyParams } from './parent-organisations-by-ids.body.params.js'; -import { Person } from '../../person/domain/person.js'; function getFakeParamsAndBody(): [OrganisationByIdParams, OrganisationByIdBodyParams] { const params: OrganisationByIdParams = { @@ -89,20 +88,7 @@ describe('OrganisationController', () => { }); describe('createOrganisation', () => { - const permissionsMock: PersonPermissions = createMock({ - get personFields(): Person { - return createMock>({ - familienname: 'current-user', - }); - }, - getPersonenkontextewithRoles: (): Promise => - Promise.resolve([ - { - organisationsId: '', - rolle: { systemrechte: [], serviceProviderIds: [] }, - }, - ]), - }); + const permissionsMock: PersonPermissions = createMock(); describe('when usecase returns a DTO', () => { it('should not throw an error', async () => { const params: CreateOrganisationBodyParams = { @@ -734,20 +720,7 @@ describe('OrganisationController', () => { }); describe('updateOrganisationName', () => { - const permissionsMock: PersonPermissions = createMock({ - get personFields(): Person { - return createMock>({ - familienname: 'current-user', - }); - }, - getPersonenkontextewithRoles: (): Promise => - Promise.resolve([ - { - organisationsId: '', - rolle: { systemrechte: [], serviceProviderIds: [] }, - }, - ]), - }); + const permissionsMock: PersonPermissions = createMock(); describe('when usecase succeeds', () => { it('should not throw an error', async () => { const oeffentlich: Organisation = Organisation.construct( @@ -817,20 +790,7 @@ describe('OrganisationController', () => { }); describe('updateOrganisationName', () => { - const permissionsMock: PersonPermissions = createMock({ - get personFields(): Person { - return createMock>({ - familienname: 'current-user', - }); - }, - getPersonenkontextewithRoles: (): Promise => - Promise.resolve([ - { - organisationsId: '', - rolle: { systemrechte: [], serviceProviderIds: [] }, - }, - ]), - }); + const permissionsMock: PersonPermissions = createMock(); describe('when usecase succeeds', () => { it('should not throw an error', async () => { const oeffentlich: Organisation = Organisation.construct( From 1e02e15a75111bf6d396533352ed5350a1eb5ff7 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Mon, 25 Nov 2024 15:44:06 +0100 Subject: [PATCH 22/29] Use empty mocks in tests --- .../domain/organisation.service.spec.ts | 34 ++----------------- ...rganisation.repository.integration-spec.ts | 34 ++----------------- .../api/rolle.controller.integration-spec.ts | 17 +--------- .../rolle/api/rolle.controller.spec.ts | 18 ++-------- 4 files changed, 7 insertions(+), 96 deletions(-) diff --git a/src/modules/organisation/domain/organisation.service.spec.ts b/src/modules/organisation/domain/organisation.service.spec.ts index f2e7946ca..62f59585b 100644 --- a/src/modules/organisation/domain/organisation.service.spec.ts +++ b/src/modules/organisation/domain/organisation.service.spec.ts @@ -22,8 +22,6 @@ import { KennungForOrganisationWithTrailingSpaceError } from '../specification/e import { EmailAdressOnOrganisationTypError } from '../specification/error/email-adress-on-organisation-typ-error.js'; import { LoggingTestModule } from '../../../../test/utils/logging-test.module.js'; import { PersonenkontextRolleFields, PersonPermissions } from '../../authentication/domain/person-permissions.js'; -import { Person } from '../../person/domain/person.js'; -import { Geschlecht } from '../../person/domain/person.enums.js'; import { KlasseNurVonSchuleAdministriertError } from '../specification/error/klasse-nur-von-schule-administriert.error.js'; describe('OrganisationService', () => { @@ -65,21 +63,7 @@ describe('OrganisationService', () => { }); describe('createOrganisation', () => { - const permissionsMock: DeepMocked = createMock({ - get personFields(): Person { - return createMock>({ - id: 'test-id', - keycloakUserId: 'test-keycloak', - vorname: 'test-vorname', - familienname: 'test-familienname', - rufname: 'test-rufname', - username: 'test-username', - geschlecht: Geschlecht.M, - geburtsdatum: faker.date.past(), - updatedAt: faker.date.recent(), - }); - }, - }); + const permissionsMock: DeepMocked = createMock(); const organisationUser: Organisation = DoFactory.createOrganisation(true); const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ { @@ -370,21 +354,7 @@ describe('OrganisationService', () => { }); describe('updateOrganisation', () => { - const permissionsMock: DeepMocked = createMock({ - get personFields(): Person { - return createMock>({ - id: 'test-id', - keycloakUserId: 'test-keycloak', - vorname: 'test-vorname', - familienname: 'test-familienname', - rufname: 'test-rufname', - username: 'test-username', - geschlecht: Geschlecht.M, - geburtsdatum: faker.date.past(), - updatedAt: faker.date.recent(), - }); - }, - }); + const permissionsMock: DeepMocked = createMock(); const organisationUser: Organisation = DoFactory.createOrganisation(true); const personenkontextewithRolesMock: PersonenkontextRolleFields[] = [ { diff --git a/src/modules/organisation/persistence/organisation.repository.integration-spec.ts b/src/modules/organisation/persistence/organisation.repository.integration-spec.ts index 251f6db19..4edc2eb4a 100644 --- a/src/modules/organisation/persistence/organisation.repository.integration-spec.ts +++ b/src/modules/organisation/persistence/organisation.repository.integration-spec.ts @@ -28,8 +28,6 @@ import { PersonPermissions } from '../../authentication/domain/person-permission import { RollenSystemRecht } from '../../rolle/domain/rolle.enums.js'; import { OrganisationUpdateOutdatedError } from '../domain/orga-update-outdated.error.js'; import { LoggingTestModule } from '../../../../test/utils/logging-test.module.js'; -import { Person } from '../../person/domain/person.js'; -import { Geschlecht } from '../../person/domain/person.enums.js'; describe('OrganisationRepository', () => { let module: TestingModule; @@ -792,21 +790,7 @@ describe('OrganisationRepository', () => { describe('deleteKlasse', () => { describe('when all validations succeed', () => { it('should succeed', async () => { - const permissionsMock: PersonPermissions = createMock({ - get personFields(): Person { - return createMock>({ - id: 'test-id', - keycloakUserId: 'test-keycloak', - vorname: 'test-vorname', - familienname: 'test-familienname', - rufname: 'test-rufname', - username: 'test-username', - geschlecht: Geschlecht.M, - geburtsdatum: faker.date.past(), - updatedAt: faker.date.recent(), - }); - }, - }); + const permissionsMock: PersonPermissions = createMock(); const organisation: Organisation = DoFactory.createOrganisationAggregate(false, { typ: OrganisationsTyp.KLASSE, }); @@ -842,21 +826,7 @@ describe('OrganisationRepository', () => { }); }); describe('updateKlassenname', () => { - const permissionsMock: PersonPermissions = createMock({ - get personFields(): Person { - return createMock>({ - id: 'test-id', - keycloakUserId: 'test-keycloak', - vorname: 'test-vorname', - familienname: 'test-familienname', - rufname: 'test-rufname', - username: 'test-username', - geschlecht: Geschlecht.M, - geburtsdatum: faker.date.past(), - updatedAt: faker.date.recent(), - }); - }, - }); + const permissionsMock: PersonPermissions = createMock(); describe('when organisation does not exist', () => { it('should return EntityNotFoundError', async () => { const id: string = faker.string.uuid(); diff --git a/src/modules/rolle/api/rolle.controller.integration-spec.ts b/src/modules/rolle/api/rolle.controller.integration-spec.ts index a08a7c72e..67fbfb748 100644 --- a/src/modules/rolle/api/rolle.controller.integration-spec.ts +++ b/src/modules/rolle/api/rolle.controller.integration-spec.ts @@ -46,7 +46,6 @@ import { PersonFactory } from '../../person/domain/person.factory.js'; import { KeycloakConfigModule } from '../../keycloak-administration/keycloak-config.module.js'; import { RolleServiceProviderBodyParams } from './rolle-service-provider.body.params.js'; import { generatePassword } from '../../../shared/util/password-generator.js'; -import { Geschlecht } from '../../person/domain/person.enums.js'; import { OrganisationRepository } from '../../organisation/persistence/organisation.repository.js'; import { Organisation } from '../../organisation/domain/organisation.js'; @@ -136,21 +135,7 @@ describe('Rolle API', () => { }); beforeEach(async () => { - permissionsMock = createMock({ - get personFields(): Person { - return createMock>({ - id: 'test-id', - keycloakUserId: 'test-keycloak', - vorname: 'test-vorname', - familienname: 'test-familienname', - rufname: 'test-rufname', - username: 'test-username', - geschlecht: Geschlecht.M, - geburtsdatum: faker.date.past(), - updatedAt: faker.date.recent(), - }); - }, - }); + permissionsMock = createMock(); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissionsMock); permissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ all: false, orgaIds: [] }); await DatabaseTestModule.clearDatabase(orm); diff --git a/src/modules/rolle/api/rolle.controller.spec.ts b/src/modules/rolle/api/rolle.controller.spec.ts index bf5e7838d..c1b50d592 100644 --- a/src/modules/rolle/api/rolle.controller.spec.ts +++ b/src/modules/rolle/api/rolle.controller.spec.ts @@ -24,8 +24,7 @@ import { RollenArt, RollenMerkmal, RollenSystemRecht } from '../domain/rolle.enu import { NameForRolleWithTrailingSpaceError } from '../domain/name-with-trailing-space.error.js'; import { Organisation } from '../../organisation/domain/organisation.js'; import { RolleServiceProviderBodyParams } from './rolle-service-provider.body.params.js'; -import { PersonenkontextRolleFields, PersonPermissions } from '../../authentication/domain/person-permissions.js'; -import { Person } from '../../person/domain/person.js'; +import { PersonPermissions } from '../../authentication/domain/person-permissions.js'; describe('Rolle API with mocked ServiceProviderRepo', () => { let rolleRepoMock: DeepMocked; @@ -110,20 +109,7 @@ describe('Rolle API with mocked ServiceProviderRepo', () => { describe('/GET rolle mocked Rolle-repo', () => { describe('createRolle', () => { - const permissionsMock: PersonPermissions = createMock({ - get personFields(): Person { - return createMock>({ - familienname: 'current-user', - }); - }, - getPersonenkontextewithRoles: (): Promise => - Promise.resolve([ - { - organisationsId: '', - rolle: { systemrechte: [], serviceProviderIds: [] }, - }, - ]), - }); + const permissionsMock: PersonPermissions = createMock(); it('should throw an HTTP exception when rolleFactory.createNew returns DomainError', async () => { const createRolleParams: CreateRolleBodyParams = { name: ' SuS', From 68f0badc381c494a933d60be7dfe9955988ea784 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Mon, 25 Nov 2024 16:17:56 +0100 Subject: [PATCH 23/29] Avoid undefined checks and adapt tests --- .../api/organisation.controller.spec.ts | 8 +- .../api/organisation.controller.ts | 2 + ...organisation-service-specification.spec.ts | 11 +- .../domain/organisation.service.ts | 129 +++++++++--------- 4 files changed, 80 insertions(+), 70 deletions(-) diff --git a/src/modules/organisation/api/organisation.controller.spec.ts b/src/modules/organisation/api/organisation.controller.spec.ts index b559f02f3..c9e717b5f 100644 --- a/src/modules/organisation/api/organisation.controller.spec.ts +++ b/src/modules/organisation/api/organisation.controller.spec.ts @@ -258,6 +258,7 @@ describe('OrganisationController', () => { }); describe('updateOrganisation', () => { + const permissionsMock: DeepMocked = createMock(); describe('when usecase returns a DTO', () => { it('should not throw an error', async () => { const params: OrganisationByIdParams = { @@ -277,7 +278,9 @@ describe('OrganisationController', () => { const returnedValue: Organisation = DoFactory.createOrganisation(true); organisationServiceMock.updateOrganisation.mockResolvedValue({ ok: true, value: returnedValue }); - await expect(organisationController.updateOrganisation(params, body)).resolves.not.toThrow(); + await expect( + organisationController.updateOrganisation(params, body, permissionsMock), + ).resolves.not.toThrow(); expect(organisationServiceMock.updateOrganisation).toHaveBeenCalledTimes(1); }); }); @@ -292,6 +295,7 @@ describe('OrganisationController', () => { organisationController.updateOrganisation( { organisationId: faker.string.uuid() } as OrganisationByIdParams, {} as UpdateOrganisationBodyParams, + permissionsMock, ), ).rejects.toThrow(OrganisationSpecificationError); expect(organisationServiceMock.updateOrganisation).toHaveBeenCalledTimes(1); @@ -308,6 +312,7 @@ describe('OrganisationController', () => { organisationController.updateOrganisation( { organisationId: faker.string.uuid() } as OrganisationByIdParams, {} as UpdateOrganisationBodyParams, + permissionsMock, ), ).rejects.toThrow(HttpException); expect(organisationServiceMock.updateOrganisation).toHaveBeenCalledTimes(1); @@ -321,6 +326,7 @@ describe('OrganisationController', () => { organisationController.updateOrganisation( { organisationId: organisationId } as OrganisationByIdParams, {} as UpdateOrganisationBodyParams, + permissionsMock, ), ).rejects.toThrow(new NotFoundException(`Organisation with ID ${organisationId} not found`)); expect(organisationRepositoryMock.findById).toHaveBeenCalledTimes(1); diff --git a/src/modules/organisation/api/organisation.controller.ts b/src/modules/organisation/api/organisation.controller.ts index efed8ab1a..6181d7785 100644 --- a/src/modules/organisation/api/organisation.controller.ts +++ b/src/modules/organisation/api/organisation.controller.ts @@ -133,6 +133,7 @@ export class OrganisationController { public async updateOrganisation( @Param() params: OrganisationByIdParams, @Body() body: UpdateOrganisationBodyParams, + @Permissions() permissions: PersonPermissions, ): Promise { const existingOrganisation: Option> = await this.organisationRepository.findById( params.organisationId, @@ -156,6 +157,7 @@ export class OrganisationController { const result: Result, DomainError> = await this.organisationService.updateOrganisation( existingOrganisation, + permissions, ); if (result.ok) { diff --git a/src/modules/organisation/domain/organisation-service-specification.spec.ts b/src/modules/organisation/domain/organisation-service-specification.spec.ts index c398f0fec..92513742b 100644 --- a/src/modules/organisation/domain/organisation-service-specification.spec.ts +++ b/src/modules/organisation/domain/organisation-service-specification.spec.ts @@ -18,6 +18,8 @@ import { EventModule } from '../../../core/eventbus/index.js'; import { Organisation } from './organisation.js'; import { OrganisationsTyp } from './organisation.enums.js'; import { OrganisationRepository } from '../persistence/organisation.repository.js'; +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { PersonPermissions } from '../../authentication/domain/person-permissions.js'; describe('OrganisationServiceSpecificationTest', () => { let module: TestingModule; @@ -74,6 +76,7 @@ describe('OrganisationServiceSpecificationTest', () => { }); describe('create', () => { + const permissionsMock: DeepMocked = createMock(); it('should return DomainError, when KlasseNurVonSchuleAdministriert specificaton is not satisfied and type is KLASSE', async () => { const klasseDo: Organisation = DoFactory.createOrganisation(false, { name: 'Klasse', @@ -84,6 +87,7 @@ describe('OrganisationServiceSpecificationTest', () => { const result: Result, DomainError> = await organisationService.createOrganisation( klasseDo, + permissionsMock, ); expect(result).toEqual>>({ @@ -113,7 +117,10 @@ describe('OrganisationServiceSpecificationTest', () => { zugehoerigZu: schule.id, typ: OrganisationsTyp.KLASSE, }); - const result: Result> = await organisationService.createOrganisation(weitereKlasseDo); + const result: Result> = await organisationService.createOrganisation( + weitereKlasseDo, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, @@ -122,6 +129,7 @@ describe('OrganisationServiceSpecificationTest', () => { }); }); describe('update', () => { + const permissionsMock: DeepMocked = createMock(); it('should return DomainError, when klasse specifications are not satisfied and type is klasse', async () => { const klasse: Organisation = DoFactory.createOrganisation(false, { name: 'klasse', @@ -133,6 +141,7 @@ describe('OrganisationServiceSpecificationTest', () => { const result: Result, DomainError> = await organisationService.updateOrganisation( klassePersisted, + permissionsMock, ); expect(result).toEqual>>({ diff --git a/src/modules/organisation/domain/organisation.service.ts b/src/modules/organisation/domain/organisation.service.ts index 34890650d..78b742e61 100644 --- a/src/modules/organisation/domain/organisation.service.ts +++ b/src/modules/organisation/domain/organisation.service.ts @@ -57,39 +57,35 @@ export class OrganisationService { organisation.zugehoerigZu, ); const schoolName: string = school?.name ?? 'SCHOOL_NOT_FOUND'; - if (permissions) { - if (error) { - this.logger.error( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht eine neue Klasse ${organisation.name} (${schoolName}) anzulegen. Fehler: ${error.message}`, - ); - } else { - this.logger.info( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat eine neue Klasse angelegt: ${organisation.name} (${schoolName}).`, - ); - } - } - } - } - if (organisation.typ === OrganisationsTyp.SCHULE) { - if (permissions) { - const personenkontextRolleFields: PersonenkontextRolleFields[] = - await permissions.getPersonenkontextewithRoles(); - const organisationIdUser: string = personenkontextRolleFields.at(0)?.organisationsId as string; - const organisationUser: Option> = - await this.organisationRepo.findById(organisationIdUser); - let organisationNameUser: string = 'ORGANISATION_NOT_FOUND'; - if (organisationUser) if (organisationUser.name) organisationNameUser = organisationUser.name; if (error) { this.logger.error( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine neue Schule ${organisation.name} anzulegen. Fehler: ${error.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht eine neue Klasse ${organisation.name} (${schoolName}) anzulegen. Fehler: ${error.message}`, ); } else { this.logger.info( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat eine neue Schule angelegt: ${organisation.name}.`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat eine neue Klasse angelegt: ${organisation.name} (${schoolName}).`, ); } } } + if (organisation.typ === OrganisationsTyp.SCHULE) { + const personenkontextRolleFields: PersonenkontextRolleFields[] = + await permissions.getPersonenkontextewithRoles(); + const organisationIdUser: string = personenkontextRolleFields.at(0)?.organisationsId as string; + const organisationUser: Option> = + await this.organisationRepo.findById(organisationIdUser); + let organisationNameUser: string = 'ORGANISATION_NOT_FOUND'; + if (organisationUser) if (organisationUser.name) organisationNameUser = organisationUser.name; + if (error) { + this.logger.error( + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine neue Schule ${organisation.name} anzulegen. Fehler: ${error.message}`, + ); + } else { + this.logger.info( + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat eine neue Schule angelegt: ${organisation.name}.`, + ); + } + } } private async logUpdate( @@ -104,47 +100,44 @@ export class OrganisationService { ); let schoolName: string = 'SCHOOL_NOT_FOUND'; if (school) if (school.name) schoolName = school.name; - if (permissions) { - if (error) { - this.logger.error( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht eine Klasse ${organisation.name} (${schoolName}) zu verändern. Fehler: ${error.message}`, - ); - } else { - this.logger.info( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat eine Klasse geändert: ${organisation.name} (${schoolName}).`, - ); - } - } - } - } - if (organisation.typ === OrganisationsTyp.SCHULE) { - if (permissions) { - const personenkontextRolleFields: PersonenkontextRolleFields[] = - await permissions.getPersonenkontextewithRoles(); - const organisationIdUser: string = personenkontextRolleFields.at(0)?.organisationsId as string; - const organisationUser: Option> = - await this.organisationRepo.findById(organisationIdUser); - const organisationNameUser: string = organisationUser?.name ?? 'ORGANISATION_NOT_FOUND'; + if (error) { this.logger.error( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine Schule ${organisation.name} zu verändern. Fehler: ${error.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht eine Klasse ${organisation.name} (${schoolName}) zu verändern. Fehler: ${error.message}`, ); } else { this.logger.info( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat eine Schule geändert: ${organisation.name}.`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat eine Klasse geändert: ${organisation.name} (${schoolName}).`, ); } } } + if (organisation.typ === OrganisationsTyp.SCHULE) { + const personenkontextRolleFields: PersonenkontextRolleFields[] = + await permissions.getPersonenkontextewithRoles(); + const organisationIdUser: string = personenkontextRolleFields.at(0)?.organisationsId as string; + const organisationUser: Option> = + await this.organisationRepo.findById(organisationIdUser); + const organisationNameUser: string = organisationUser?.name ?? 'ORGANISATION_NOT_FOUND'; + if (error) { + this.logger.error( + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine Schule ${organisation.name} zu verändern. Fehler: ${error.message}`, + ); + } else { + this.logger.info( + `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat eine Schule geändert: ${organisation.name}.`, + ); + } + } } public async createOrganisation( organisationDo: Organisation, - permissions: PersonPermissions | null = null, + permissions: PersonPermissions, ): Promise, DomainError>> { if (organisationDo.administriertVon && !(await this.organisationRepo.exists(organisationDo.administriertVon))) { const error: DomainError = new EntityNotFoundError('Organisation', organisationDo.administriertVon); - if (permissions) await this.logCreation(permissions, organisationDo, error); + await this.logCreation(permissions, organisationDo, error); return { ok: false, error: error, @@ -153,7 +146,7 @@ export class OrganisationService { if (organisationDo.zugehoerigZu && !(await this.organisationRepo.exists(organisationDo.zugehoerigZu))) { const error: DomainError = new EntityNotFoundError('Organisation', organisationDo.zugehoerigZu); - if (permissions) await this.logCreation(permissions, organisationDo, error); + await this.logCreation(permissions, organisationDo, error); return { ok: false, error: error, @@ -163,113 +156,113 @@ export class OrganisationService { const validationFieldnamesResult: void | DomainError = this.validateFieldNames(organisationDo); if (validationFieldnamesResult) { const error: DomainError = validationFieldnamesResult; - if (permissions) await this.logCreation(permissions, organisationDo, error); + await this.logCreation(permissions, organisationDo, error); return { ok: false, error: error }; } let validationResult: Result = await this.validateKennungRequiredForSchule(organisationDo); if (!validationResult.ok) { const error: DomainError = validationResult.error; - if (permissions) await this.logCreation(permissions, organisationDo, error); + await this.logCreation(permissions, organisationDo, error); return { ok: false, error: error }; } validationResult = await this.validateNameRequiredForSchule(organisationDo); if (!validationResult.ok) { const error: DomainError = validationResult.error; - if (permissions) await this.logCreation(permissions, organisationDo, error); + await this.logCreation(permissions, organisationDo, error); return { ok: false, error: error }; } validationResult = await this.validateSchuleKennungUnique(organisationDo); if (!validationResult.ok) { const error: DomainError = validationResult.error; - if (permissions) await this.logCreation(permissions, organisationDo, error); + await this.logCreation(permissions, organisationDo, error); return { ok: false, error: error }; } validationResult = await this.validateEmailAdressOnOrganisationTyp(organisationDo); if (!validationResult.ok) { const error: DomainError = validationResult.error; - if (permissions) await this.logCreation(permissions, organisationDo, error); + await this.logCreation(permissions, organisationDo, error); return { ok: false, error: error }; } const validateKlassen: Result = await this.validateKlassenSpecifications(organisationDo); if (!validateKlassen.ok) { const error: DomainError = validateKlassen.error; - if (permissions) await this.logCreation(permissions, organisationDo, error); + await this.logCreation(permissions, organisationDo, error); return { ok: false, error: error }; } const organisation: Organisation | OrganisationSpecificationError = await this.organisationRepo.save(organisationDo); if (organisation instanceof Organisation) { - if (permissions) await this.logCreation(permissions, organisation); + await this.logCreation(permissions, organisation); return { ok: true, value: organisation }; } const error: DomainError = new EntityCouldNotBeCreated(`Organization could not be created`); - if (permissions) await this.logCreation(permissions, organisationDo, error); + await this.logCreation(permissions, organisationDo, error); return { ok: false, error: error }; } public async updateOrganisation( organisationDo: Organisation, - permissions: PersonPermissions | null = null, + permissions: PersonPermissions, ): Promise, DomainError>> { const storedOrganisation: Option> = await this.organisationRepo.findById(organisationDo.id); if (!storedOrganisation) { const error: DomainError = new EntityNotFoundError('Organisation', organisationDo.id); - if (permissions) await this.logUpdate(permissions, organisationDo, error); + await this.logUpdate(permissions, organisationDo, error); return { ok: false, error: error }; } const validationFieldnamesResult: void | DomainError = this.validateFieldNames(organisationDo); if (validationFieldnamesResult) { const error: DomainError = validationFieldnamesResult; - if (permissions) await this.logUpdate(permissions, organisationDo, error); + await this.logUpdate(permissions, organisationDo, error); return { ok: false, error: error }; } let validationResult: Result = await this.validateKennungRequiredForSchule(organisationDo); if (!validationResult.ok) { const error: DomainError = validationResult.error; - if (permissions) await this.logUpdate(permissions, organisationDo, error); + await this.logUpdate(permissions, organisationDo, error); return { ok: false, error: error }; } validationResult = await this.validateNameRequiredForSchule(organisationDo); if (!validationResult.ok) { const error: DomainError = validationResult.error; - if (permissions) await this.logUpdate(permissions, organisationDo, error); + await this.logUpdate(permissions, organisationDo, error); return { ok: false, error: error }; } validationResult = await this.validateSchuleKennungUnique(organisationDo); if (!validationResult.ok) { const error: DomainError = validationResult.error; - if (permissions) await this.logUpdate(permissions, organisationDo, error); + await this.logUpdate(permissions, organisationDo, error); return { ok: false, error: error }; } validationResult = await this.validateEmailAdressOnOrganisationTyp(organisationDo); if (!validationResult.ok) { const error: DomainError = validationResult.error; - if (permissions) await this.logUpdate(permissions, organisationDo, error); + await this.logUpdate(permissions, organisationDo, error); return { ok: false, error: error }; } const validateKlassen: Result = await this.validateKlassenSpecifications(organisationDo); if (!validateKlassen.ok) { const error: DomainError = validateKlassen.error; - if (permissions) await this.logUpdate(permissions, organisationDo, error); + await this.logUpdate(permissions, organisationDo, error); return { ok: false, error: error }; } const organisation: Organisation | OrganisationSpecificationError = await this.organisationRepo.save(organisationDo); if (organisation instanceof Organisation) { - if (permissions) await this.logUpdate(permissions, organisation); + await this.logUpdate(permissions, organisation); return { ok: true, value: organisation }; } const error: DomainError = new EntityCouldNotBeUpdated(`Organization could not be updated`, organisationDo.id); - if (permissions) await this.logUpdate(permissions, organisationDo, error); + await this.logUpdate(permissions, organisationDo, error); return { ok: false, error: error, From 514829ad9b886cd57894ffbe0f28e490f16bb804 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Tue, 26 Nov 2024 09:20:30 +0100 Subject: [PATCH 24/29] Don't log Organisation name --- .../domain/organisation.service.ts | 23 +++--------- src/modules/rolle/api/rolle.controller.ts | 37 +++++-------------- 2 files changed, 15 insertions(+), 45 deletions(-) diff --git a/src/modules/organisation/domain/organisation.service.ts b/src/modules/organisation/domain/organisation.service.ts index 78b742e61..2cbf1eec9 100644 --- a/src/modules/organisation/domain/organisation.service.ts +++ b/src/modules/organisation/domain/organisation.service.ts @@ -37,7 +37,7 @@ import { EmailAdressOnOrganisationTyp } from '../specification/email-on-organisa import { EmailAdressOnOrganisationTypError } from '../specification/error/email-adress-on-organisation-typ-error.js'; import { ClassLogger } from '../../../core/logging/class-logger.js'; import { OrganisationsTyp } from './organisation.enums.js'; -import { PersonenkontextRolleFields, PersonPermissions } from '../../authentication/domain/person-permissions.js'; +import { PersonPermissions } from '../../authentication/domain/person-permissions.js'; @Injectable() export class OrganisationService { @@ -69,20 +69,13 @@ export class OrganisationService { } } if (organisation.typ === OrganisationsTyp.SCHULE) { - const personenkontextRolleFields: PersonenkontextRolleFields[] = - await permissions.getPersonenkontextewithRoles(); - const organisationIdUser: string = personenkontextRolleFields.at(0)?.organisationsId as string; - const organisationUser: Option> = - await this.organisationRepo.findById(organisationIdUser); - let organisationNameUser: string = 'ORGANISATION_NOT_FOUND'; - if (organisationUser) if (organisationUser.name) organisationNameUser = organisationUser.name; if (error) { this.logger.error( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine neue Schule ${organisation.name} anzulegen. Fehler: ${error.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht eine neue Schule ${organisation.name} anzulegen. Fehler: ${error.message}`, ); } else { this.logger.info( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat eine neue Schule angelegt: ${organisation.name}.`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat eine neue Schule angelegt: ${organisation.name}.`, ); } } @@ -113,19 +106,13 @@ export class OrganisationService { } } if (organisation.typ === OrganisationsTyp.SCHULE) { - const personenkontextRolleFields: PersonenkontextRolleFields[] = - await permissions.getPersonenkontextewithRoles(); - const organisationIdUser: string = personenkontextRolleFields.at(0)?.organisationsId as string; - const organisationUser: Option> = - await this.organisationRepo.findById(organisationIdUser); - const organisationNameUser: string = organisationUser?.name ?? 'ORGANISATION_NOT_FOUND'; if (error) { this.logger.error( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine Schule ${organisation.name} zu verändern. Fehler: ${error.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht eine Schule ${organisation.name} zu verändern. Fehler: ${error.message}`, ); } else { this.logger.info( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat eine Schule geändert: ${organisation.name}.`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat eine Schule geändert: ${organisation.name}.`, ); } } diff --git a/src/modules/rolle/api/rolle.controller.ts b/src/modules/rolle/api/rolle.controller.ts index 61ca926a3..d9640c93b 100644 --- a/src/modules/rolle/api/rolle.controller.ts +++ b/src/modules/rolle/api/rolle.controller.ts @@ -51,7 +51,7 @@ import { SchulConnexError } from '../../../shared/error/schul-connex.error.js'; import { RolleExceptionFilter } from './rolle-exception-filter.js'; import { Paged, PagedResponse, PagingHeadersObject } from '../../../shared/paging/index.js'; import { Permissions } from '../../authentication/api/permissions.decorator.js'; -import { PersonenkontextRolleFields, PersonPermissions } from '../../authentication/domain/person-permissions.js'; +import { PersonPermissions } from '../../authentication/domain/person-permissions.js'; import { UpdateRolleBodyParams } from './update-rolle.body.params.js'; import { DBiamPersonenkontextRepo } from '../../personenkontext/persistence/dbiam-personenkontext.repo.js'; import { RolleDomainError } from '../domain/rolle-domain.error.js'; @@ -184,15 +184,13 @@ export class RolleController { @Body() params: CreateRolleBodyParams, @Permissions() permissions: PersonPermissions, ): Promise { - const organisationNameUser: string = await this.getOrganisationNameForCurrentUser(permissions); - const orgResult: Result, DomainError> = await this.orgService.findOrganisationById( params.administeredBySchulstrukturknoten, ); if (!orgResult.ok) { this.logger.info( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine neue Rolle ${params.name} anzulegen. Fehler: ${orgResult.error.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht eine neue Rolle ${params.name} anzulegen. Fehler: ${orgResult.error.message}`, ); throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(orgResult.error), @@ -212,14 +210,14 @@ export class RolleController { if (rolle instanceof DomainError) { this.logger.error( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine neue Rolle ${params.name} anzulegen. Fehler: ${rolle.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht eine neue Rolle ${params.name} anzulegen. Fehler: ${rolle.message}`, ); throw rolle; } const result: Rolle = await this.rolleRepo.save(rolle); this.logger.info( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat eine neue Rolle angelegt: ${result.name}.`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat eine neue Rolle angelegt: ${result.name}.`, ); return new RolleResponse(result); @@ -375,7 +373,6 @@ export class RolleController { @Body() params: UpdateRolleBodyParams, @Permissions() permissions: PersonPermissions, ): Promise { - const organisationNameUser: string = await this.getOrganisationNameForCurrentUser(permissions); const rolle: Option> = await this.rolleRepo.findById(findRolleByIdParams.rolleId); const rolleName: string = rolle?.name ?? 'ROLLE_NOT_FOUND'; @@ -396,12 +393,12 @@ export class RolleController { if (result instanceof DomainError) { if (result instanceof RolleDomainError) { this.logger.error( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine Rolle ${params.name} zu bearbeiten. Fehler: ${result.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht eine Rolle ${params.name} zu bearbeiten. Fehler: ${result.message}`, ); throw result; } this.logger.error( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht eine Rolle ${params.name} zu bearbeiten. Fehler: ${result.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht eine Rolle ${params.name} zu bearbeiten. Fehler: ${result.message}`, ); throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(result), @@ -409,7 +406,7 @@ export class RolleController { } this.logger.info( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat eine Rolle bearbeitet: ${rolleName}.`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat eine Rolle bearbeitet: ${rolleName}.`, ); return this.returnRolleWithServiceProvidersResponse(result); @@ -426,7 +423,6 @@ export class RolleController { @Param() findRolleByIdParams: FindRolleByIdParams, @Permissions() permissions: PersonPermissions, ): Promise { - const organisationNameUser: string = await this.getOrganisationNameForCurrentUser(permissions); const rolle: Option> = await this.rolleRepo.findById(findRolleByIdParams.rolleId); const rolleName: string = rolle?.name ?? 'ROLLE_NOT_FOUND'; @@ -437,12 +433,12 @@ export class RolleController { if (result instanceof DomainError) { if (result instanceof RolleDomainError) { this.logger.error( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht die Rolle ${rolleName} zu entfernen. Fehler: ${result.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht die Rolle ${rolleName} zu entfernen. Fehler: ${result.message}`, ); throw result; } this.logger.error( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat versucht die Rolle ${rolleName} zu entfernen. Fehler: ${result.message}`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht die Rolle ${rolleName} zu entfernen. Fehler: ${result.message}`, ); throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(result), @@ -450,7 +446,7 @@ export class RolleController { } this.logger.info( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}, ${organisationNameUser}) hat eine Rolle entfernt: ${rolleName}.`, + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat eine Rolle entfernt: ${rolleName}.`, ); } @@ -465,17 +461,4 @@ export class RolleController { return new RolleWithServiceProvidersResponse(rolle, rolleServiceProviders); } - - private async getOrganisationNameForCurrentUser(permissions: PersonPermissions): Promise { - const personenkontextRolleFields: PersonenkontextRolleFields[] = - await permissions?.getPersonenkontextewithRoles(); - const organisationIdUser: string | undefined = personenkontextRolleFields.at(0)?.organisationsId; - if (organisationIdUser) { - const organisationUser: Option> = - await this.organisationRepository.findById(organisationIdUser); - if (organisationUser) if (organisationUser.name) return organisationUser.name; - } - this.logger.error(`No Organisation id found for given Person id: ${permissions.personFields.id}`); - return 'ORGANISATION_NOT_FOUND'; - } } From 8dd1055f144aa5e23cf011705f7afb7f2d379574 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Tue, 26 Nov 2024 10:32:07 +0100 Subject: [PATCH 25/29] Fix duplicate --- src/modules/organisation/persistence/organisation.repository.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/modules/organisation/persistence/organisation.repository.ts b/src/modules/organisation/persistence/organisation.repository.ts index c5abbda54..025366d00 100644 --- a/src/modules/organisation/persistence/organisation.repository.ts +++ b/src/modules/organisation/persistence/organisation.repository.ts @@ -90,7 +90,6 @@ export class OrganisationRepository { private readonly eventService: EventService, private readonly em: EntityManager, config: ConfigService, - private readonly logger: ClassLogger, ) { this.ROOT_ORGANISATION_ID = config.getOrThrow('DATA').ROOT_ORGANISATION_ID; } From 1751955755074d9139e7fde7fb29181ec91e3c89 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Tue, 26 Nov 2024 10:34:04 +0100 Subject: [PATCH 26/29] Add log for a new error case --- src/modules/rolle/api/rolle.controller.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/modules/rolle/api/rolle.controller.ts b/src/modules/rolle/api/rolle.controller.ts index 05eca04fa..df3582d30 100644 --- a/src/modules/rolle/api/rolle.controller.ts +++ b/src/modules/rolle/api/rolle.controller.ts @@ -217,6 +217,9 @@ export class RolleController { } const result: Rolle | DomainError = await this.rolleRepo.save(rolle); if (result instanceof DomainError) { + this.logger.error( + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht eine neue Rolle ${params.name} anzulegen. Fehler: ${result.message}.`, + ); throw result; } this.logger.info( From ea4030240b63b6c0b6aeb16ce8fe8324e2e604b0 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Tue, 26 Nov 2024 10:55:43 +0100 Subject: [PATCH 27/29] Fix tests --- src/modules/organisation/domain/organisation.service.spec.ts | 5 ++++- .../persistence/organisation.repository.integration-spec.ts | 1 - .../person/persistence/person.scope.integration-spec.ts | 1 - .../persistence/personenkontext.repo.integration-spec.ts | 1 - .../persistence/personenkontext.scope.integration-spec.ts | 1 - 5 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/modules/organisation/domain/organisation.service.spec.ts b/src/modules/organisation/domain/organisation.service.spec.ts index aae861d4b..89bd5c7fa 100644 --- a/src/modules/organisation/domain/organisation.service.spec.ts +++ b/src/modules/organisation/domain/organisation.service.spec.ts @@ -359,7 +359,10 @@ describe('OrganisationService', () => { typ: OrganisationsTyp.KLASSE, }); organisationRepositoryMock.exists.mockResolvedValueOnce(true).mockResolvedValueOnce(true); - const result: Result> = await organisationService.createOrganisation(organisationDo); + const result: Result> = await organisationService.createOrganisation( + organisationDo, + permissionsMock, + ); expect(result).toEqual>>({ ok: false, error: new KlasseWithoutNumberOrLetterError(), diff --git a/src/modules/organisation/persistence/organisation.repository.integration-spec.ts b/src/modules/organisation/persistence/organisation.repository.integration-spec.ts index 8fae7703f..c27c443fa 100644 --- a/src/modules/organisation/persistence/organisation.repository.integration-spec.ts +++ b/src/modules/organisation/persistence/organisation.repository.integration-spec.ts @@ -28,7 +28,6 @@ import { OrganisationSpecificationError } from '../specification/error/organisat import { PersonPermissions } from '../../authentication/domain/person-permissions.js'; import { RollenSystemRecht } from '../../rolle/domain/rolle.enums.js'; import { OrganisationUpdateOutdatedError } from '../domain/orga-update-outdated.error.js'; -import { LoggingTestModule } from '../../../../test/utils/logging-test.module.js'; describe('OrganisationRepository', () => { let module: TestingModule; diff --git a/src/modules/person/persistence/person.scope.integration-spec.ts b/src/modules/person/persistence/person.scope.integration-spec.ts index de34083a3..5a5343ea3 100644 --- a/src/modules/person/persistence/person.scope.integration-spec.ts +++ b/src/modules/person/persistence/person.scope.integration-spec.ts @@ -8,7 +8,6 @@ import { DoFactory, LoggingTestModule, MapperTestModule, - LoggingTestModule, } from '../../../../test/utils/index.js'; import { ScopeOrder } from '../../../shared/persistence/scope.enums.js'; import { PersonEntity } from './person.entity.js'; diff --git a/src/modules/personenkontext/persistence/personenkontext.repo.integration-spec.ts b/src/modules/personenkontext/persistence/personenkontext.repo.integration-spec.ts index 48e7f596b..288ffc190 100644 --- a/src/modules/personenkontext/persistence/personenkontext.repo.integration-spec.ts +++ b/src/modules/personenkontext/persistence/personenkontext.repo.integration-spec.ts @@ -7,7 +7,6 @@ import { DoFactory, LoggingTestModule, MapperTestModule, - LoggingTestModule, } from '../../../../test/utils/index.js'; import { PersonenkontextDo } from '../domain/personenkontext.do.js'; import { PersonPersistenceMapperProfile } from '../../person/persistence/person-persistence.mapper.profile.js'; diff --git a/src/modules/personenkontext/persistence/personenkontext.scope.integration-spec.ts b/src/modules/personenkontext/persistence/personenkontext.scope.integration-spec.ts index 259b17ea9..8802735c7 100644 --- a/src/modules/personenkontext/persistence/personenkontext.scope.integration-spec.ts +++ b/src/modules/personenkontext/persistence/personenkontext.scope.integration-spec.ts @@ -10,7 +10,6 @@ import { DoFactory, LoggingTestModule, MapperTestModule, - LoggingTestModule, } from '../../../../test/utils/index.js'; import { ScopeOrder } from '../../../shared/persistence/scope.enums.js'; import { PersonenkontextDo } from '../domain/personenkontext.do.js'; From 00ee6432f87e7951e9a8ca59e04c5b0da473b098 Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Tue, 26 Nov 2024 15:59:17 +0100 Subject: [PATCH 28/29] Avoid undefined checks --- ...rganisation.repository.integration-spec.ts | 12 +++++-- .../persistence/organisation.repository.ts | 36 ++++++++----------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/modules/organisation/persistence/organisation.repository.integration-spec.ts b/src/modules/organisation/persistence/organisation.repository.integration-spec.ts index c27c443fa..dd708d3cc 100644 --- a/src/modules/organisation/persistence/organisation.repository.integration-spec.ts +++ b/src/modules/organisation/persistence/organisation.repository.integration-spec.ts @@ -805,21 +805,23 @@ describe('OrganisationRepository', () => { describe('when organisation does not exist', () => { it('should return EntityNotFoundError', async () => { + const permissionsMock: DeepMocked = createMock(); const id: string = faker.string.uuid(); - const result: Option = await sut.deleteKlasse(id); + const result: Option = await sut.deleteKlasse(id, permissionsMock); expect(result).toEqual(new EntityNotFoundError('Organisation', id)); }); }); describe('when organisation is not a Klasse', () => { it('should return EntityCouldNotBeUpdated', async () => { + const permissionsMock: DeepMocked = createMock(); const organisation: Organisation = DoFactory.createOrganisationAggregate(false, { typ: OrganisationsTyp.SONSTIGE, name: 'test', }); const savedOrganisaiton: Organisation = await sut.save(organisation); - const result: Option = await sut.deleteKlasse(savedOrganisaiton.id); + const result: Option = await sut.deleteKlasse(savedOrganisaiton.id, permissionsMock); expect(result).toBeInstanceOf(EntityCouldNotBeUpdated); }); @@ -834,6 +836,7 @@ describe('OrganisationRepository', () => { id, faker.company.name(), faker.number.int(), + permissionsMock, ); expect(result).toEqual(new EntityNotFoundError('Organisation', id)); @@ -852,6 +855,7 @@ describe('OrganisationRepository', () => { savedOrganisaiton.id, faker.company.name(), faker.number.int(), + permissionsMock, ); expect(result).toBeInstanceOf(EntityCouldNotBeUpdated); @@ -949,7 +953,7 @@ describe('OrganisationRepository', () => { // Simulate concurrent updates: // 1. First update - await sut.updateKlassenname(organisationEntity2.id, 'newName1', 1); + await sut.updateKlassenname(organisationEntity2.id, 'newName1', 1, permissionsMock); // 2. Try second update with original version (should fail) await expect(async () => { @@ -957,6 +961,7 @@ describe('OrganisationRepository', () => { organisationEntity2.id, 'newName2', 1, // This is now outdated because previous update incremented it + permissionsMock, ); }).rejects.toThrow(OrganisationUpdateOutdatedError); }); @@ -988,6 +993,7 @@ describe('OrganisationRepository', () => { organisationEntity2.id, 'name', 1, + permissionsMock, ); expect(result).not.toBeInstanceOf(DomainError); diff --git a/src/modules/organisation/persistence/organisation.repository.ts b/src/modules/organisation/persistence/organisation.repository.ts index 025366d00..3b7474853 100644 --- a/src/modules/organisation/persistence/organisation.repository.ts +++ b/src/modules/organisation/persistence/organisation.repository.ts @@ -415,10 +415,7 @@ export class OrganisationRepository { return [organisations, total + entitiesForIds.length - duplicates, pageTotal]; } - public async deleteKlasse( - id: OrganisationID, - permissions: PersonPermissions | null = null, - ): Promise> { + public async deleteKlasse(id: OrganisationID, permissions: PersonPermissions): Promise> { const organisationEntity: Option = await this.em.findOne(OrganisationEntity, { id }); if (!organisationEntity) { return new EntityNotFoundError('Organisation', id); @@ -432,14 +429,13 @@ export class OrganisationRepository { this.eventService.publish(new KlasseDeletedEvent(organisationEntity.id)); let schoolName: string = 'SCHOOL_NOT_FOUND'; - if (permissions) - if (organisationEntity.zugehoerigZu) { - const school: Option> = await this.findById(organisationEntity.zugehoerigZu); - schoolName = school?.name ?? 'SCHOOL_NOT_FOUND'; - this.logger.info( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat eine Klasse entfernt: ${organisationEntity.name} (${schoolName}).`, - ); - } + if (organisationEntity.zugehoerigZu) { + const school: Option> = await this.findById(organisationEntity.zugehoerigZu); + schoolName = school?.name ?? 'SCHOOL_NOT_FOUND'; + this.logger.info( + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat eine Klasse entfernt: ${organisationEntity.name} (${schoolName}).`, + ); + } return; } @@ -448,7 +444,7 @@ export class OrganisationRepository { id: string, newName: string, version: number, - permissions: PersonPermissions | null = null, + permissions: PersonPermissions, ): Promise> { const organisationFound: Option> = await this.findById(id); @@ -471,10 +467,9 @@ export class OrganisationRepository { await organisationFound.checkKlasseSpecifications(this); if (specificationError) { - if (permissions) - this.logger.error( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht den Namen einer Klasse ${organisationFound.name} (${schoolName}) zu verändern. Fehler: ${specificationError.message}`, - ); + this.logger.error( + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht den Namen einer Klasse ${organisationFound.name} (${schoolName}) zu verändern. Fehler: ${specificationError.message}`, + ); return specificationError; } } @@ -483,10 +478,9 @@ export class OrganisationRepository { const organisationEntity: Organisation | OrganisationSpecificationError = await this.save(organisationFound); this.eventService.publish(new KlasseUpdatedEvent(id, newName, organisationFound.administriertVon)); - if (permissions) - this.logger.info( - `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat den Namen einer Klasse geändert: ${organisationFound.name} (${schoolName}).`, - ); + this.logger.info( + `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat den Namen einer Klasse geändert: ${organisationFound.name} (${schoolName}).`, + ); return organisationEntity; } From ac8c1df2300c98e0579e556ae5abe1276fd8897a Mon Sep 17 00:00:00 2001 From: Henrik Meyer Date: Tue, 26 Nov 2024 16:02:57 +0100 Subject: [PATCH 29/29] Use correct log level --- src/modules/rolle/api/rolle.controller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/rolle/api/rolle.controller.ts b/src/modules/rolle/api/rolle.controller.ts index df3582d30..daf3dca26 100644 --- a/src/modules/rolle/api/rolle.controller.ts +++ b/src/modules/rolle/api/rolle.controller.ts @@ -191,7 +191,7 @@ export class RolleController { params.administeredBySchulstrukturknoten, ); if (!orgResult.ok) { - this.logger.info( + this.logger.error( `Admin ${permissions.personFields.username} (${permissions.personFields.id}) hat versucht eine neue Rolle ${params.name} anzulegen. Fehler: ${orgResult.error.message}`, ); throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException(