From b0d2683945d62b1556c3bc14df759c9f01ee2175 Mon Sep 17 00:00:00 2001 From: DPDS93CT Date: Mon, 16 Dec 2024 15:33:23 +0100 Subject: [PATCH 1/7] add personId and personReferrer to more logging-statements in EmailEventHandler --- .../email/domain/email-event-handler.spec.ts | 130 ++++++++++------ .../email/domain/email-event-handler.ts | 140 ++++++++++++------ .../person/persistence/person.repository.ts | 1 + src/shared/events/ox-user-changed.event.ts | 3 +- src/shared/events/person-deleted.event.ts | 4 +- src/shared/events/person-renamed-event.ts | 6 +- 6 files changed, 188 insertions(+), 96 deletions(-) diff --git a/src/modules/email/domain/email-event-handler.spec.ts b/src/modules/email/domain/email-event-handler.spec.ts index bf85cc012..8daef36b4 100644 --- a/src/modules/email/domain/email-event-handler.spec.ts +++ b/src/modules/email/domain/email-event-handler.spec.ts @@ -30,7 +30,7 @@ import { RolleUpdatedEvent } from '../../../shared/events/rolle-updated.event.js import { RollenArt } from '../../rolle/domain/rolle.enums.js'; import { DBiamPersonenkontextRepo } from '../../personenkontext/persistence/dbiam-personenkontext.repo.js'; import { EmailAddress, EmailAddressStatus } from './email-address.js'; -import { PersonID, RolleID } from '../../../shared/types/index.js'; +import { PersonID, PersonReferrer, RolleID } from '../../../shared/types/index.js'; import { Personenkontext } from '../../personenkontext/domain/personenkontext.js'; import { PersonenkontextUpdatedEvent } from '../../../shared/events/personenkontext-updated.event.js'; import { OxMetadataInKeycloakChangedEvent } from '../../../shared/events/ox-metadata-in-keycloak-changed.event.js'; @@ -150,6 +150,7 @@ describe('EmailEventHandler', () => { describe('test private methods: createOrEnableEmail, createNewEmail, changeEmail, getPersonReferrerOrError', () => { let fakePersonId: PersonID; + let fakeReferrer: PersonReferrer; let fakeRolleId: RolleID; let fakeOrgaId: string; let fakeEmailAddress: string; @@ -166,6 +167,7 @@ describe('EmailEventHandler', () => { beforeEach(() => { jest.resetAllMocks(); fakePersonId = faker.string.uuid(); + fakeReferrer = faker.internet.userName(); fakeRolleId = faker.string.uuid(); fakeOrgaId = faker.string.uuid(); fakeEmailAddress = faker.internet.email(); @@ -174,7 +176,7 @@ describe('EmailEventHandler', () => { fakePersonId, faker.person.firstName(), faker.person.lastName(), - faker.internet.userName(), + fakeReferrer, faker.internet.userName(), ); personenkontext = createMock>({ rolleId: fakeRolleId, organisationId: fakeOrgaId }); @@ -279,9 +281,11 @@ describe('EmailEventHandler', () => { await emailEventHandler.handlePersonRenamedEvent(personRenamedEvent); expect(loggerMock.info).toHaveBeenCalledWith( - `Received PersonRenamedEvent, personId:${personRenamedEvent.personId}`, + `Received PersonRenamedEvent, personId:${personRenamedEvent.personId}, referrer:${fakeReferrer}`, + ); + expect(loggerMock.info).toHaveBeenCalledWith( + `DISABLED and saved address:${emailAddress.address}, personId:${personRenamedEvent.personId}, referrer:${fakeReferrer}`, ); - expect(loggerMock.info).toHaveBeenCalledWith(`Disabled and saved address:${emailAddress.address}`); expect(loggerMock.error).toHaveBeenLastCalledWith( `Could not retrieve orgaKennung, orgaId:${fakeOrgaId}`, ); @@ -313,6 +317,7 @@ describe('EmailEventHandler', () => { describe('handlePersonenkontextUpdatedEvent', () => { let fakePersonId: PersonID; + let fakeReferrer: PersonReferrer; let fakeRolleId: RolleID; let fakeEmailAddressString: string; let event: PersonenkontextUpdatedEvent; @@ -325,9 +330,10 @@ describe('EmailEventHandler', () => { beforeEach(() => { jest.resetAllMocks(); fakePersonId = faker.string.uuid(); + fakeReferrer = faker.internet.userName(); fakeRolleId = faker.string.uuid(); fakeEmailAddressString = faker.internet.email(); - event = createMock({ person: { id: fakePersonId } }); + event = createMock({ person: { id: fakePersonId, referrer: fakeReferrer } }); personenkontexte = [createMock>({ rolleId: fakeRolleId })]; rolle = createMock>({ id: fakeRolleId, serviceProviderIds: [] }); @@ -361,12 +367,14 @@ describe('EmailEventHandler', () => { ]); //mock person with referrer is found - personRepositoryMock.findById.mockResolvedValueOnce(createMock>()); + personRepositoryMock.findById.mockResolvedValueOnce( + createMock>({ referrer: fakeReferrer }), + ); await emailEventHandler.handlePersonenkontextUpdatedEvent(event); expect(loggerMock.info).toHaveBeenCalledWith( - `Existing email for personId:${fakePersonId} already enabled`, + `Existing email for personId:${fakePersonId}, referrer:${fakeReferrer} already enabled`, ); }); }); @@ -412,7 +420,7 @@ describe('EmailEventHandler', () => { //mock person with referrer is found personRepositoryMock.findById.mockResolvedValueOnce( - createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + createMock>({ id: faker.string.uuid(), referrer: fakeReferrer }), ); // eslint-disable-next-line @typescript-eslint/require-await @@ -433,7 +441,7 @@ describe('EmailEventHandler', () => { await emailEventHandler.handlePersonenkontextUpdatedEvent(event); expect(loggerMock.info).toHaveBeenCalledWith( - `Set Requested status and persisted address:${persistedEmail.currentAddress}`, + `Set REQUESTED status and persisted address:${persistedEmail.currentAddress}, personId:${fakePersonId}, referrer:${fakeReferrer}`, ); }); }); @@ -446,7 +454,7 @@ describe('EmailEventHandler', () => { //mock person with referrer is found personRepositoryMock.findById.mockResolvedValueOnce( - createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + createMock>({ id: faker.string.uuid(), referrer: fakeReferrer }), ); // eslint-disable-next-line @typescript-eslint/require-await @@ -466,7 +474,7 @@ describe('EmailEventHandler', () => { await emailEventHandler.handlePersonenkontextUpdatedEvent(event); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not enable email, error is requested EmailAddress with the address:${fakeEmailAddressString} was not found`, + `Could not enable email for personId:${fakePersonId}, referrer:${fakeReferrer}, error is requested EmailAddress with the address:${fakeEmailAddressString} was not found`, ); }); }); @@ -479,7 +487,7 @@ describe('EmailEventHandler', () => { emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found //mock person with referrer is found personRepositoryMock.findById.mockResolvedValue( - createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + createMock>({ id: faker.string.uuid(), referrer: fakeReferrer }), ); const persistenceResult: EmailAddress = getEmail(); @@ -504,7 +512,7 @@ describe('EmailEventHandler', () => { await emailEventHandler.handlePersonenkontextUpdatedEvent(event); expect(loggerMock.info).toHaveBeenCalledWith( - `Successfully persisted email with REQUEST status for address:${persistenceResult.currentAddress}`, + `Successfully persisted email with REQUEST status for address:${persistenceResult.currentAddress}, personId:${fakePersonId}, referrer:${fakeReferrer}`, ); }); }); @@ -517,7 +525,7 @@ describe('EmailEventHandler', () => { emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found //mock person with referrer is found personRepositoryMock.findById.mockResolvedValue( - createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + createMock>({ id: faker.string.uuid(), referrer: fakeReferrer }), ); // eslint-disable-next-line @typescript-eslint/require-await @@ -543,7 +551,7 @@ describe('EmailEventHandler', () => { await emailEventHandler.handlePersonenkontextUpdatedEvent(event); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not create email, error is: requested Person with the following ID ${fakePersonId} was not found`, + `Could not create email for personId:${fakePersonId}, referrer:${fakeReferrer}, error is: requested Person with the following ID ${fakePersonId} was not found`, ); }); }); @@ -552,13 +560,17 @@ describe('EmailEventHandler', () => { describe('createAndPersistFailedEmailAddress', () => { describe('when persisting EmailAddress with Failed status fails', () => { it('should log matching info', async () => { + //mock getting referrer (used for logging) + personRepositoryMock.findById.mockResolvedValueOnce( + createMock>({ referrer: fakeReferrer }), + ); dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found //mock person with referrer is found personRepositoryMock.findById.mockResolvedValue( - createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + createMock>({ id: faker.string.uuid(), referrer: fakeReferrer }), ); // eslint-disable-next-line @typescript-eslint/require-await @@ -575,7 +587,7 @@ describe('EmailEventHandler', () => { await emailEventHandler.handlePersonenkontextUpdatedEvent(event); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not create email, error is: requested Person with the following ID ${fakePersonId} was not found`, + `Could not create email for personId:${fakePersonId}, referrer:${fakeReferrer}, error is: requested Person with the following ID ${fakePersonId} was not found`, ); }); }); @@ -583,13 +595,17 @@ describe('EmailEventHandler', () => { describe('when email does NOT exist and error occurs during persisting', () => { it('should log matching info', async () => { + //mock getting referrer (used for logging) + personRepositoryMock.findById.mockResolvedValueOnce( + createMock>({ referrer: fakeReferrer }), + ); dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found //mock person with referrer is found personRepositoryMock.findById.mockResolvedValue( - createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + createMock>({ id: faker.string.uuid(), referrer: fakeReferrer }), ); emailRepoMock.save.mockResolvedValueOnce(new EmailAddressNotFoundError(fakeEmailAddressString)); //mock: error during saving the entity @@ -611,13 +627,17 @@ describe('EmailEventHandler', () => { await emailEventHandler.handlePersonenkontextUpdatedEvent(event); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not persist email, error is requested EmailAddress with the address:${fakeEmailAddressString} was not found`, + `Could not persist email for personId:${fakePersonId}, referrer:${fakeReferrer}, error is requested EmailAddress with the address:${fakeEmailAddressString} was not found`, ); }); }); describe('when lehrer does not have any PK, email is enabled, disable email and error occurs during persisting', () => { it('should log matching info', async () => { + //mock getting referrer (used for logging) + personRepositoryMock.findById.mockResolvedValueOnce( + createMock>({ referrer: fakeReferrer }), + ); dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); serviceProviderRepoMock.findByIds.mockResolvedValueOnce(new Map>()); @@ -631,7 +651,7 @@ describe('EmailEventHandler', () => { expect.stringContaining('Existing email found for personId'), ); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not disable email, error is requested EmailAddress with the address:${fakeEmailAddressString} was not found`, + `Could not DISABLE email, error is requested EmailAddress with the address:${fakeEmailAddressString} was not found, personId:${fakePersonId}, referrer:${fakeReferrer}`, ); }); }); @@ -641,12 +661,18 @@ describe('EmailEventHandler', () => { { status: EmailAddressStatus.REQUESTED }, { status: EmailAddressStatus.FAILED }, ].forEach(({ status }: { status: EmailAddressStatus }) => { - describe(`when lehrer does not have any PK, email is ${status}, disable email is successfull`, () => { + describe(`when lehrer does not have any PK, email is ${status}, disable email is successful`, () => { it('should log matching info', async () => { + //mock getting referrer (used for logging) + personRepositoryMock.findById.mockResolvedValueOnce( + createMock>({ referrer: fakeReferrer }), + ); dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); serviceProviderRepoMock.findByIds.mockResolvedValueOnce(new Map>()); - personRepositoryMock.findById.mockResolvedValueOnce(createMock>()); + personRepositoryMock.findById.mockResolvedValueOnce( + createMock>({ referrer: fakeReferrer }), + ); const emailAddress: EmailAddress = EmailAddress.construct( faker.string.uuid(), @@ -674,7 +700,9 @@ describe('EmailEventHandler', () => { expect(loggerMock.info).toHaveBeenCalledWith( expect.stringContaining('Existing email found for personId'), ); - expect(loggerMock.info).toHaveBeenCalledWith(`Disabled and saved address:${emailAddress.address}`); + expect(loggerMock.info).toHaveBeenCalledWith( + `DISABLED and saved address:${emailAddress.address}, personId:${fakePersonId}, referrer:${fakeReferrer}`, + ); }); }); }); @@ -712,7 +740,9 @@ describe('EmailEventHandler', () => { expect(loggerMock.info).toHaveBeenCalledWith( expect.stringContaining('Existing email found for personId'), ); - expect(loggerMock.info).toHaveBeenCalledWith(`Disabled and saved address:${emailAddress.address}`); + expect(loggerMock.info).toHaveBeenCalledWith( + `DISABLED and saved address:${emailAddress.address}, personId:${fakePersonId}, referrer:${fakeReferrer}`, + ); expect(loggerMock.error).toHaveBeenCalledWith( `Could not publish EmailAddressDisabledEvent, personId:${fakePersonId} has no username`, ); @@ -922,9 +952,11 @@ describe('EmailEventHandler', () => { await emailEventHandler.handlePersonRenamedEvent(event); expect(loggerMock.info).toHaveBeenCalledWith( - `Received PersonRenamedEvent, personId:${event.personId}`, + `Received PersonRenamedEvent, personId:${event.personId}, referrer:${event.referrer}`, + ); + expect(loggerMock.info).toHaveBeenCalledWith( + `DISABLED and saved address:${emailAddress.address}, personId:${event.personId}, referrer:${event.referrer}`, ); - expect(loggerMock.info).toHaveBeenCalledWith(`Disabled and saved address:${emailAddress.address}`); }); }); @@ -953,11 +985,13 @@ describe('EmailEventHandler', () => { await emailEventHandler.handlePersonRenamedEvent(event); expect(loggerMock.info).toHaveBeenCalledWith( - `Received PersonRenamedEvent, personId:${event.personId}`, + `Received PersonRenamedEvent, personId:${event.personId}, referrer:${event.referrer}`, + ); + expect(loggerMock.info).toHaveBeenCalledWith( + `DISABLED and saved address:${emailAddress.address}, personId:${event.personId}, referrer:${event.referrer}`, ); - expect(loggerMock.info).toHaveBeenCalledWith(`Disabled and saved address:${emailAddress.address}`); expect(loggerMock.error).toHaveBeenLastCalledWith( - 'Could not create change-email, error is EmailAddress could not be created', + `Could not create change-email for personId:${event.personId}, referrer:${event.referrer}, error is EmailAddress could not be created`, ); }); }); @@ -984,11 +1018,13 @@ describe('EmailEventHandler', () => { await emailEventHandler.handlePersonRenamedEvent(event); expect(loggerMock.info).toHaveBeenCalledWith( - `Received PersonRenamedEvent, personId:${event.personId}`, + `Received PersonRenamedEvent, personId:${event.personId}, referrer:${event.referrer}`, + ); + expect(loggerMock.info).toHaveBeenCalledWith( + `DISABLED and saved address:${emailAddress.address}, personId:${event.personId}, referrer:${event.referrer}`, ); - expect(loggerMock.info).toHaveBeenCalledWith(`Disabled and saved address:${emailAddress.address}`); expect(loggerMock.error).toHaveBeenLastCalledWith( - 'Could not persist change-email, error is EmailAddress could not be created', + `Could not persist change-email for personId:${event.personId}, referrer:${event.referrer}, error is EmailAddress could not be created`, ); }); }); @@ -1011,7 +1047,7 @@ describe('EmailEventHandler', () => { await emailEventHandler.handlePersonRenamedEvent(event); expect(loggerMock.info).toHaveBeenCalledWith( - `Received PersonRenamedEvent, personId:${event.personId}`, + `Received PersonRenamedEvent, personId:${event.personId}, referrer:${event.referrer}`, ); }); }); @@ -1027,10 +1063,10 @@ describe('EmailEventHandler', () => { expect(emailRepoMock.findByPersonSortedByUpdatedAtDesc).toHaveBeenCalledTimes(0); expect(emailRepoMock.save).toHaveBeenCalledTimes(0); expect(loggerMock.info).toHaveBeenCalledWith( - `Received PersonRenamedEvent, personId:${event.personId}`, + `Received PersonRenamedEvent, personId:${event.personId}, referrer:${event.referrer}`, ); expect(loggerMock.info).toHaveBeenLastCalledWith( - `Renamed person with personId:${event.personId} has no SP with Email, nothing to do`, + `Renamed person with personId:${event.personId}, referrer:${event.referrer} has no SP with Email, nothing to do`, ); }); }); @@ -1053,10 +1089,10 @@ describe('EmailEventHandler', () => { await emailEventHandler.handlePersonRenamedEvent(event); expect(loggerMock.info).toHaveBeenCalledWith( - `Received PersonRenamedEvent, personId:${event.personId}`, + `Received PersonRenamedEvent, personId:${event.personId}, referrer:${event.referrer}`, ); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not disable email, error is requested EmailAddress with the address:${fakeEmailAddress} was not found`, + `Could not DISABLE email, error is requested EmailAddress with the address:${fakeEmailAddress} was not found, personId:${event.personId}, referrer:${event.referrer}`, ); }); }); @@ -1065,7 +1101,7 @@ describe('EmailEventHandler', () => { describe('handlePersonDeletedEvent', () => { let personId: string; - let referrer: string; + let referrer: PersonReferrer; let emailAddress: string; let event: PersonDeletedEvent; @@ -1080,7 +1116,9 @@ describe('EmailEventHandler', () => { it('should log info', async () => { await emailEventHandler.handlePersonDeletedEvent(event); - expect(loggerMock.info).toHaveBeenCalledWith(`Successfully deactivated email-address:${emailAddress}`); + expect(loggerMock.info).toHaveBeenCalledWith( + `Successfully deactivated email-address:${emailAddress}, personId:${event.personId}, referrer:${event.referrer}`, + ); }); }); @@ -1090,7 +1128,7 @@ describe('EmailEventHandler', () => { await emailEventHandler.handlePersonDeletedEvent(event); expect(loggerMock.info).toHaveBeenCalledWith( - `Cannot deactivate email-address, person did not have an email-address`, + `Cannot deactivate email-address, personId:${event.personId}, referrer:${event.referrer}, person did not have an email-address`, ); }); }); @@ -1101,7 +1139,7 @@ describe('EmailEventHandler', () => { await emailEventHandler.handlePersonDeletedEvent(event); expect(loggerMock.error).toHaveBeenCalledWith( - `Deactivation of email-address:${event.emailAddress} failed`, + `Deactivation of email-address:${event.emailAddress} failed, personId:${event.personId}, referrer:${event.referrer}`, ); }); }); @@ -1199,7 +1237,7 @@ describe('EmailEventHandler', () => { await emailEventHandler.handleOxMetadataInKeycloakChangedEvent(event); expect(loggerMock.info).toHaveBeenLastCalledWith( - `Cannot find requested email-address for person with personId:${event.personId}, enabling not necessary`, + `Cannot find REQUESTED email-address for person with personId:${event.personId}, referrer:${event.keycloakUsername}, enabling not necessary`, ); }); }); @@ -1220,10 +1258,10 @@ describe('EmailEventHandler', () => { await emailEventHandler.handleOxMetadataInKeycloakChangedEvent(event); expect(loggerMock.warning).toHaveBeenCalledWith( - `Mismatch between requested(${emailAddress}) and received(${event.emailAddress}) address from OX, personId:${event.personId}`, + `Mismatch between REQUESTED(${emailAddress}) and received(${event.emailAddress}) address from OX, personId:${event.personId}, referrer:${event.keycloakUsername}`, ); expect(loggerMock.warning).toHaveBeenLastCalledWith( - `Overriding ${emailAddress} with ${event.emailAddress}) from OX, personId:${event.personId}`, + `Overriding ${emailAddress} with ${event.emailAddress}) from OX, personId:${event.personId}, referrer:${event.keycloakUsername}`, ); }); }); @@ -1243,7 +1281,7 @@ describe('EmailEventHandler', () => { await emailEventHandler.handleOxMetadataInKeycloakChangedEvent(event); expect(loggerMock.error).toHaveBeenLastCalledWith( - `Could not enable email for personId:${event.personId}, error is EmailAddress with ID 1 could not be updated`, + `Could not enable email for personId:${event.personId}, referrer:${event.keycloakUsername}, error is EmailAddress with ID 1 could not be updated`, ); }); }); @@ -1262,7 +1300,7 @@ describe('EmailEventHandler', () => { await emailEventHandler.handleOxMetadataInKeycloakChangedEvent(event); expect(loggerMock.info).toHaveBeenLastCalledWith( - `Changed email-address:${fakeEmail} from REQUESTED to ENABLED, personId:${event.personId}`, + `Changed email-address:${fakeEmail} from REQUESTED to ENABLED, personId:${event.personId}, referrer:${event.keycloakUsername}`, ); }); }); diff --git a/src/modules/email/domain/email-event-handler.ts b/src/modules/email/domain/email-event-handler.ts index 210d036f3..00e79a6a6 100644 --- a/src/modules/email/domain/email-event-handler.ts +++ b/src/modules/email/domain/email-event-handler.ts @@ -8,7 +8,7 @@ import { ServiceProvider } from '../../service-provider/domain/service-provider. import { ServiceProviderKategorie } from '../../service-provider/domain/service-provider.enum.js'; import { PersonDeletedEvent } from '../../../shared/events/person-deleted.event.js'; import { DomainError, EntityNotFoundError } from '../../../shared/error/index.js'; -import { OrganisationID, PersonID } from '../../../shared/types/index.js'; +import { OrganisationID, PersonID, PersonReferrer } from '../../../shared/types/index.js'; import { EmailAddressEntity } from '../persistence/email-address.entity.js'; import { EmailAddressNotFoundError } from '../error/email-address-not-found.error.js'; import { EmailRepo } from '../persistence/email.repo.js'; @@ -55,8 +55,7 @@ export class EmailEventHandler { @EventHandler(PersonRenamedEvent) public async handlePersonRenamedEvent(event: PersonRenamedEvent): Promise { - this.logger.info(`Received PersonRenamedEvent, personId:${event.personId}`); - + this.logger.info(`Received PersonRenamedEvent, personId:${event.personId}, referrer:${event.referrer}`); const rollenWithPK: Map = await this.getRollenWithPKForPerson(event.personId); const rollen: Rolle[] = Array.from(rollenWithPK.values(), (value: RolleWithPK) => { return value.rolle; @@ -66,16 +65,20 @@ export class EmailEventHandler { const existingEmail: Option> = await this.emailRepo.findEnabledByPerson(event.personId); if (existingEmail) { this.logger.info( - `Existing email found for personId:${event.personId}, address:${existingEmail.address}`, + `Existing email found for personId:${event.personId}, address:${existingEmail.address}, referrer:${event.referrer}`, ); if (existingEmail.enabledOrRequested) { existingEmail.disable(); const persistenceResult: EmailAddress | DomainError = await this.emailRepo.save(existingEmail); if (persistenceResult instanceof EmailAddress) { - this.logger.info(`Disabled and saved address:${persistenceResult.address}`); + this.logger.info( + `DISABLED and saved address:${persistenceResult.address}, personId:${event.personId}, referrer:${event.referrer}`, + ); } else { - this.logger.error(`Could not disable email, error is ${persistenceResult.message}`); + this.logger.error( + `Could not DISABLE email, error is ${persistenceResult.message}, personId:${event.personId}, referrer:${event.referrer}`, + ); } } } @@ -84,6 +87,7 @@ export class EmailEventHandler { if (existingEmail) { await this.changeEmail( event.personId, + event.referrer, pkForRolleWithSPReference.personenkontext.organisationId, existingEmail, ); @@ -92,7 +96,9 @@ export class EmailEventHandler { } } } else { - this.logger.info(`Renamed person with personId:${event.personId} has no SP with Email, nothing to do`); + this.logger.info( + `Renamed person with personId:${event.personId}, referrer:${event.referrer} has no SP with Email, nothing to do`, + ); } } @@ -182,27 +188,35 @@ export class EmailEventHandler { @EventHandler(PersonenkontextUpdatedEvent) // currently receiving of this event is not causing a deletion of email and the related addresses for the affected user, this is intentional public async handlePersonenkontextUpdatedEvent(event: PersonenkontextUpdatedEvent): Promise { - this.logger.info(`Received handlePersonenkontextUpdatedEvent, personId:${event.person.id}`); + this.logger.info( + `Received handlePersonenkontextUpdatedEvent, personId:${event.person.id}, referrer:${event.person.referrer}`, + ); - await this.handlePerson(event.person.id); + await this.handlePerson(event.person.id, event.person.referrer); } // this method cannot make use of handlePerson(personId) method, because personId is already null when event is received @EventHandler(PersonDeletedEvent) public async handlePersonDeletedEvent(event: PersonDeletedEvent): Promise { - this.logger.info(`Received PersonDeletedEvent, personId:${event.personId}`); + this.logger.info(`Received PersonDeletedEvent, personId:${event.personId}, referrer:${event.referrer}`); //Setting person_id to null in Email table is done via deleteRule, not necessary here if (!event.emailAddress) { - return this.logger.info('Cannot deactivate email-address, person did not have an email-address'); + return this.logger.info( + `Cannot deactivate email-address, personId:${event.personId}, referrer:${event.referrer}, person did not have an email-address`, + ); } const deactivationResult: EmailAddressEntity | EmailAddressNotFoundError = await this.emailRepo.deactivateEmailAddress(event.emailAddress); if (deactivationResult instanceof EmailAddressNotFoundError) { - return this.logger.error(`Deactivation of email-address:${event.emailAddress} failed`); + return this.logger.error( + `Deactivation of email-address:${event.emailAddress} failed, personId:${event.personId}, referrer:${event.referrer}`, + ); } - return this.logger.info(`Successfully deactivated email-address:${event.emailAddress}`); + return this.logger.info( + `Successfully deactivated email-address:${event.emailAddress}, personId:${event.personId}, referrer:${event.referrer}`, + ); } private async getAnyRolleReferencesEmailServiceProvider(rollen: Rolle[]): Promise> { @@ -221,19 +235,29 @@ export class EmailEventHandler { @EventHandler(RolleUpdatedEvent) // eslint-disable-next-line @typescript-eslint/require-await public async handleRolleUpdatedEvent(event: RolleUpdatedEvent): Promise { - this.logger.info(`Received RolleUpdatedEvent, rolleId:${event.rolleId}`); + this.logger.info(`Received RolleUpdatedEvent, rolleId:${event.rolleId}, rollenArt:${event.rollenart}`); const personenkontexte: Personenkontext[] = await this.dbiamPersonenkontextRepo.findByRolle( event.rolleId, ); + + //const personIdReferrerSet: Set<[PersonID, PersonReferrer]> = new Set<[PersonID, PersonReferrer]>(); + + const personIdReferrerMap: Map = new Map< + PersonID, + PersonReferrer | undefined + >(); const personIdsSet: Set = new Set(); - personenkontexte.map((pk: Personenkontext) => personIdsSet.add(pk.personId)); + personenkontexte.map((pk: Personenkontext) => { + personIdsSet.add(pk.personId); + personIdReferrerMap.set(pk.personId, pk.referrer); + }); const distinctPersonIds: PersonID[] = Array.from(personIdsSet.values()); this.logger.info(`RolleUpdatedEvent affects:${distinctPersonIds.length} persons`); const handlePersonPromises: Promise[] = distinctPersonIds.map((personId: PersonID) => { - return this.handlePerson(personId); + return this.handlePerson(personId, personIdReferrerMap.get(personId)); }); await Promise.all(handlePersonPromises); @@ -242,22 +266,22 @@ export class EmailEventHandler { @EventHandler(OxMetadataInKeycloakChangedEvent) public async handleOxMetadataInKeycloakChangedEvent(event: OxMetadataInKeycloakChangedEvent): Promise { this.logger.info( - `Received OxMetadataInKeycloakChangedEvent personId:${event.personId}, keycloakUsername: ${event.keycloakUsername}, userName:${event.oxUserName}, contextName:${event.oxContextName}, email:${event.emailAddress}`, + `Received OxMetadataInKeycloakChangedEvent personId:${event.personId}, referrer:${event.keycloakUsername}, oxUserName:${event.oxUserName}, contextName:${event.oxContextName}, email:${event.emailAddress}`, ); const email: Option> = await this.emailRepo.findRequestedByPerson(event.personId); if (!email) { return this.logger.info( - `Cannot find requested email-address for person with personId:${event.personId}, enabling not necessary`, + `Cannot find REQUESTED email-address for person with personId:${event.personId}, referrer:${event.keycloakUsername}, enabling not necessary`, ); } if (email.address !== event.emailAddress) { this.logger.warning( - `Mismatch between requested(${email.address}) and received(${event.emailAddress}) address from OX, personId:${event.personId}`, + `Mismatch between REQUESTED(${email.address}) and received(${event.emailAddress}) address from OX, personId:${event.personId}, referrer:${event.keycloakUsername}`, ); this.logger.warning( - `Overriding ${email.address} with ${event.emailAddress}) from OX, personId:${event.personId}`, + `Overriding ${email.address} with ${event.emailAddress}) from OX, personId:${event.personId}, referrer:${event.keycloakUsername}`, ); email.setAddress(event.emailAddress); } @@ -268,16 +292,16 @@ export class EmailEventHandler { if (persistenceResult instanceof DomainError) { return this.logger.error( - `Could not enable email for personId:${event.personId}, error is ${persistenceResult.message}`, + `Could not enable email for personId:${event.personId}, referrer:${event.keycloakUsername}, error is ${persistenceResult.message}`, ); } else { return this.logger.info( - `Changed email-address:${persistenceResult.address} from REQUESTED to ENABLED, personId:${event.personId}`, + `Changed email-address:${persistenceResult.address} from REQUESTED to ENABLED, personId:${event.personId}, referrer:${event.keycloakUsername}`, ); } } - private async getPersonReferrerOrError(personId: PersonID): Promise> { + private async getPersonReferrerOrError(personId: PersonID): Promise> { const person: Option> = await this.personRepository.findById(personId); if (!person) { @@ -295,14 +319,14 @@ export class EmailEventHandler { }; } - this.logger.info(`Found referrer${person.referrer} For personId:${personId}`); + this.logger.info(`Found referrer:${person.referrer} for personId:${personId}`); return { ok: true, value: person.referrer, }; } - private async handlePerson(personId: PersonID): Promise { + private async handlePerson(personId: PersonID, referrer: PersonReferrer | undefined): Promise { // Map to store combinations of rolleId and organisationId as the key const rolleIdPKMap: Map> = new Map>(); @@ -344,7 +368,9 @@ export class EmailEventHandler { // Process each valid Personenkontext if (pkOfRolleWithSPReferenceList.length > 0) { - this.logger.info(`Person with id:${personId} needs an email, creating or enabling address`); + this.logger.info( + `Person with personId:${personId}, referrer:${referrer} needs an email, creating or enabling address`, + ); // Iterate over all valid Personenkontext objects and trigger email creation for (const pkOfRolleWithSPReference of pkOfRolleWithSPReferenceList) { // eslint-disable-next-line no-await-in-loop @@ -367,7 +393,7 @@ export class EmailEventHandler { .filter((existingEmail: EmailAddress) => !existingEmail.disabled) .map(async (existingEmail: EmailAddress) => { this.logger.info( - `Existing email found for personId:${personId}, address:${existingEmail.address}`, + `Existing email found for personId:${personId}, referrer:${referrer}, address:${existingEmail.address}`, ); existingEmail.disable(); const persistenceResult: EmailAddress | DomainError = @@ -375,9 +401,13 @@ export class EmailEventHandler { if (persistenceResult instanceof EmailAddress) { anyEmailWasDisabled = true; - this.logger.info(`Disabled and saved address:${persistenceResult.address}`); + this.logger.info( + `DISABLED and saved address:${persistenceResult.address}, personId:${personId}, referrer:${referrer}`, + ); } else { - this.logger.error(`Could not disable email, error is ${persistenceResult.message}`); + this.logger.error( + `Could not DISABLE email, error is ${persistenceResult.message}, personId:${personId}, referrer:${referrer}`, + ); } }), ); @@ -429,7 +459,9 @@ export class EmailEventHandler { } for (const email of existingEmails) { if (email.enabled) { - return this.logger.info(`Existing email for personId:${personId} already enabled`); + return this.logger.info( + `Existing email for personId:${personId}, referrer:${personReferrer.value} already enabled`, + ); } else if (email.disabled) { // If we find a disabled address, we just enable it again email.enable(); @@ -438,7 +470,9 @@ export class EmailEventHandler { // eslint-disable-next-line no-await-in-loop const persistenceResult: EmailAddress | DomainError = await this.emailRepo.save(email); if (persistenceResult instanceof EmailAddress) { - this.logger.info(`Set Requested status and persisted address:${persistenceResult.address}`); + this.logger.info( + `Set REQUESTED status and persisted address:${persistenceResult.address}, personId:${personId}, referrer:${personReferrer.value}`, + ); this.eventService.publish( new EmailAddressGeneratedEvent( personId, @@ -450,17 +484,24 @@ export class EmailEventHandler { ), ); } else { - this.logger.error(`Could not enable email, error is ${persistenceResult.message}`); + this.logger.error( + `Could not enable email for personId:${personId}, referrer:${personReferrer.value}, error is ${persistenceResult.message}`, + ); } return; } } - this.logger.info(`No existing email found for personId:${personId}, creating a new one`); + this.logger.info( + `No existing email found for personId:${personId}, referrer:${personReferrer.value}, creating a new one`, + ); await this.createNewEmail(personId, organisationId); } - private async createAndPersistFailedEmailAddress(personId: PersonID): Promise { + private async createAndPersistFailedEmailAddress( + personId: PersonID, + referrer: PersonReferrer | undefined, + ): Promise { const personIdAndTimestamp: string = personId + '-' + Date.now(); const failedEmailAddress: EmailAddress = EmailAddress.createNew( personId, @@ -471,10 +512,12 @@ export class EmailEventHandler { const persistenceResult: EmailAddress | DomainError = await this.emailRepo.save(failedEmailAddress); if (persistenceResult instanceof EmailAddress) { this.logger.info( - `Successfully persisted email with FAILED status for address:${persistenceResult.address}`, + `Successfully persisted email with FAILED status for address:${persistenceResult.address}, personId:${personId}, referrer:${referrer}`, ); } else { - this.logger.error(`Could not persist email, error is ${persistenceResult.message}`); + this.logger.error( + `Could not persist email for personId:${personId}, referrer:${referrer}, error is ${persistenceResult.message}`, + ); } } @@ -487,14 +530,16 @@ export class EmailEventHandler { } const email: Result> = await this.emailFactory.createNew(personId, organisationId); if (!email.ok) { - await this.createAndPersistFailedEmailAddress(personId); - return this.logger.error(`Could not create email, error is: ${email.error.message}`); + await this.createAndPersistFailedEmailAddress(personId, personReferrer.value); + return this.logger.error( + `Could not create email for personId:${personId}, referrer:${personReferrer.value}, error is: ${email.error.message}`, + ); } email.value.request(); const persistenceResult: EmailAddress | DomainError = await this.emailRepo.save(email.value); if (persistenceResult instanceof EmailAddress) { this.logger.info( - `Successfully persisted email with REQUEST status for address:${persistenceResult.address}`, + `Successfully persisted email with REQUEST status for address:${persistenceResult.address}, personId:${personId}, referrer:${personReferrer.value}`, ); this.eventService.publish( new EmailAddressGeneratedEvent( @@ -507,12 +552,15 @@ export class EmailEventHandler { ), ); } else { - this.logger.error(`Could not persist email, error is ${persistenceResult.message}`); + this.logger.error( + `Could not persist email for personId:${personId}, referrer:${personReferrer.value}, error is ${persistenceResult.message}`, + ); } } private async changeEmail( personId: PersonID, + referrer: PersonReferrer | undefined, organisationId: OrganisationID, oldEmail: EmailAddress, ): Promise { @@ -524,15 +572,17 @@ export class EmailEventHandler { } const email: Result> = await this.emailFactory.createNew(personId, organisationId); if (!email.ok) { - await this.createAndPersistFailedEmailAddress(personId); + await this.createAndPersistFailedEmailAddress(personId, referrer); - return this.logger.error(`Could not create change-email, error is ${email.error.message}`); + return this.logger.error( + `Could not create change-email for personId:${personId}, referrer:${referrer}, error is ${email.error.message}`, + ); } email.value.request(); const persistenceResult: EmailAddress | DomainError = await this.emailRepo.save(email.value); if (persistenceResult instanceof EmailAddress) { this.logger.info( - `Successfully persisted change-email with REQUEST status for address:${persistenceResult.address}`, + `Successfully persisted change-email with REQUEST status for address:${persistenceResult.address}, personId:${personId}, referrer:${referrer}`, ); this.eventService.publish( new EmailAddressChangedEvent( @@ -546,7 +596,9 @@ export class EmailEventHandler { ), ); } else { - this.logger.error(`Could not persist change-email, error is ${persistenceResult.message}`); + this.logger.error( + `Could not persist change-email for personId:${personId}, referrer:${referrer}, error is ${persistenceResult.message}`, + ); } } diff --git a/src/modules/person/persistence/person.repository.ts b/src/modules/person/persistence/person.repository.ts index 698e94490..e36bb72e2 100644 --- a/src/modules/person/persistence/person.repository.ts +++ b/src/modules/person/persistence/person.repository.ts @@ -328,6 +328,7 @@ export class PersonRepository { const personenkontextUpdatedEvent: PersonenkontextUpdatedEvent = new PersonenkontextUpdatedEvent( { id: personId, + referrer: person.referrer, familienname: person.familienname, vorname: person.vorname, email: person.email, diff --git a/src/shared/events/ox-user-changed.event.ts b/src/shared/events/ox-user-changed.event.ts index 267a4fee1..9203bbc6f 100644 --- a/src/shared/events/ox-user-changed.event.ts +++ b/src/shared/events/ox-user-changed.event.ts @@ -1,10 +1,11 @@ import { BaseEvent } from './base-event.js'; import { OXContextID, OXContextName, OXUserID, OXUserName } from '../types/ox-ids.types.js'; +import { PersonReferrer } from '../types/aggregate-ids.types.js'; export class OxUserChangedEvent extends BaseEvent { public constructor( public readonly personId: string, - public readonly keycloakUsername: string, + public readonly keycloakUsername: PersonReferrer, public readonly oxUserId: OXUserID, public readonly oxUserName: OXUserName, public readonly oxContextId: OXContextID, diff --git a/src/shared/events/person-deleted.event.ts b/src/shared/events/person-deleted.event.ts index 815cae155..fc982d1b1 100644 --- a/src/shared/events/person-deleted.event.ts +++ b/src/shared/events/person-deleted.event.ts @@ -1,10 +1,10 @@ import { BaseEvent } from './base-event.js'; -import { PersonID } from '../types/index.js'; +import { PersonID, PersonReferrer } from '../types/index.js'; export class PersonDeletedEvent extends BaseEvent { public constructor( public readonly personId: PersonID, - public readonly referrer: string, + public readonly referrer: PersonReferrer, public readonly emailAddress?: string, ) { super(); diff --git a/src/shared/events/person-renamed-event.ts b/src/shared/events/person-renamed-event.ts index f874e1888..0ea078f11 100644 --- a/src/shared/events/person-renamed-event.ts +++ b/src/shared/events/person-renamed-event.ts @@ -1,5 +1,5 @@ import { BaseEvent } from './base-event.js'; -import { PersonID } from '../types/index.js'; +import { PersonID, PersonReferrer } from '../types/index.js'; import type { Person } from '../../modules/person/domain/person.js'; @@ -8,8 +8,8 @@ export class PersonRenamedEvent extends BaseEvent { public readonly personId: PersonID, public readonly vorname: string, public readonly familienname: string, - public readonly referrer: string | undefined, - public readonly oldReferrer: string, + public readonly referrer: PersonReferrer | undefined, + public readonly oldReferrer: PersonReferrer, ) { super(); } From 5e120684f93eac7b1c819451e7bed8eb21899bbe Mon Sep 17 00:00:00 2001 From: DPDS93CT Date: Mon, 16 Dec 2024 16:02:52 +0100 Subject: [PATCH 2/7] add personId and referrer to log-statements in OxEventHandler --- .../ox/domain/ox-event-handler.spec.ts | 46 +++++++++++------- src/modules/ox/domain/ox-event-handler.ts | 48 +++++++++++-------- 2 files changed, 56 insertions(+), 38 deletions(-) diff --git a/src/modules/ox/domain/ox-event-handler.spec.ts b/src/modules/ox/domain/ox-event-handler.spec.ts index db3492064..68c383cfa 100644 --- a/src/modules/ox/domain/ox-event-handler.spec.ts +++ b/src/modules/ox/domain/ox-event-handler.spec.ts @@ -3,7 +3,7 @@ import { DeepMocked, createMock } from '@golevelup/ts-jest'; import { Test, TestingModule } from '@nestjs/testing'; import { ConfigTestModule, LoggingTestModule } from '../../../../test/utils/index.js'; import { ClassLogger } from '../../../core/logging/class-logger.js'; -import { PersonID } from '../../../shared/types/index.js'; +import { PersonID, PersonReferrer } from '../../../shared/types/index.js'; import { OxEventHandler } from './ox-event-handler.js'; import { OxService } from './ox.service.js'; import { CreateUserAction } from '../actions/user/create-user.action.js'; @@ -476,21 +476,23 @@ describe('OxEventHandler', () => { describe('handleEmailAddressGeneratedEvent', () => { let personId: PersonID; + let referrer: PersonReferrer; let event: EmailAddressGeneratedEvent; let person: Person; beforeEach(() => { jest.resetAllMocks(); personId = faker.string.uuid(); + referrer = faker.internet.userName(); event = new EmailAddressGeneratedEvent( personId, - faker.internet.userName(), + referrer, faker.string.uuid(), faker.internet.email(), true, faker.string.numeric(), ); - person = createMock>({ email: faker.internet.email(), referrer: faker.internet.userName() }); + person = createMock>({ email: faker.internet.email(), referrer: referrer }); }); it('should skip event, if not enabled', async () => { @@ -517,7 +519,7 @@ describe('OxEventHandler', () => { expect(oxServiceMock.send).toHaveBeenLastCalledWith(expect.any(ExistsUserAction)); expect(oxServiceMock.send).toHaveBeenCalledTimes(1); expect(loggerMock.error).toHaveBeenLastCalledWith( - `Cannot create user in OX, username:${person.referrer} already exists`, + `Cannot create user in OX, username:${person.referrer} already exists, personId:${personId}`, ); }); @@ -527,7 +529,9 @@ describe('OxEventHandler', () => { await sut.handleEmailAddressGeneratedEvent(event); expect(oxServiceMock.send).toHaveBeenCalledTimes(0); - expect(loggerMock.error).toHaveBeenLastCalledWith(`Person not found for personId:${personId}`); + expect(loggerMock.error).toHaveBeenLastCalledWith( + `Person not found for personId:${personId}, referrer:${referrer}`, + ); }); it('should log error when person has no referrer set', async () => { @@ -603,7 +607,7 @@ describe('OxEventHandler', () => { expect(oxServiceMock.send).toHaveBeenCalledWith(expect.any(CreateUserAction)); expect(loggerMock.info).toHaveBeenCalledWith( - `User created in OX, oxUserId:${fakeOXUserId}, oxEmail:${event.address}, personId:${personId}`, + `User created in OX, oxUserId:${fakeOXUserId}, oxEmail:${event.address}, personId:${personId}, referrer:${referrer}`, ); expect(loggerMock.info).toHaveBeenLastCalledWith( `Successfully Added OxUser To OxGroup, oxUserId:${fakeOXUserId}, oxGroupId:${fakeOXGroupId}`, @@ -660,7 +664,7 @@ describe('OxEventHandler', () => { expect(oxServiceMock.send).toHaveBeenCalledWith(expect.any(CreateUserAction)); expect(loggerMock.error).toHaveBeenLastCalledWith( - `Could Not Adjust GlobalAddressBookDisabled For oxUserId:${fakeOXUserId}, error: Unknown OX-error`, + `Could Not Adjust GlobalAddressBookDisabled For oxUserId:${fakeOXUserId}, personId:${personId}, referrer:${referrer}, error: Unknown OX-error`, ); expect(eventServiceMock.publish).toHaveBeenCalledTimes(1); }); @@ -724,10 +728,10 @@ describe('OxEventHandler', () => { expect(oxServiceMock.send).toHaveBeenCalledWith(expect.any(CreateUserAction)); expect(loggerMock.info).toHaveBeenLastCalledWith( - `User created in OX, oxUserId:${fakeOXUserId}, oxEmail:${event.address}, personId:${personId}`, + `User created in OX, oxUserId:${fakeOXUserId}, oxEmail:${event.address}, personId:${personId}, referrer:${referrer}`, ); expect(loggerMock.error).toHaveBeenLastCalledWith( - `Persisting oxUserId on emailAddress for personId:${personId} failed`, + `Persisting oxUserId on emailAddress failed, personId:${personId}, referrer:${referrer}`, ); expect(eventServiceMock.publish).toHaveBeenCalledTimes(0); }); @@ -750,7 +754,9 @@ describe('OxEventHandler', () => { await sut.handleEmailAddressGeneratedEvent(event); expect(oxServiceMock.send).toHaveBeenLastCalledWith(expect.any(CreateUserAction)); - expect(loggerMock.error).toHaveBeenLastCalledWith(`Could not create user in OX, error: Request failed`); + expect(loggerMock.error).toHaveBeenLastCalledWith( + `Could not create user in OX, personId:${personId}, referrer:${referrer}, error: Request failed`, + ); }); }); @@ -776,7 +782,7 @@ describe('OxEventHandler', () => { contextName: faker.string.alpha(); event = new EmailAddressChangedEvent( personId, - faker.internet.userName(), + referrer, faker.string.uuid(), faker.internet.email(), faker.string.uuid(), @@ -800,7 +806,9 @@ describe('OxEventHandler', () => { await sut.handleEmailAddressChangedEvent(event); expect(oxServiceMock.send).toHaveBeenCalledTimes(0); - expect(loggerMock.error).toHaveBeenLastCalledWith(`Person not found for personId:${personId}`); + expect(loggerMock.error).toHaveBeenLastCalledWith( + `Person not found for personId:${personId}, referrer:${referrer}`, + ); }); it('should log error when person has no referrer set', async () => { @@ -822,7 +830,9 @@ describe('OxEventHandler', () => { await sut.handleEmailAddressChangedEvent(event); expect(oxServiceMock.send).toHaveBeenCalledTimes(0); - expect(loggerMock.error).toHaveBeenLastCalledWith(`Person with personId:${personId} has no OXUserId`); + expect(loggerMock.error).toHaveBeenLastCalledWith( + `Person has no OXUserId, personId:${personId}, referrer:${referrer}`, + ); }); it('should log error when no requestedEmailAddress is found for person', async () => { @@ -850,7 +860,7 @@ describe('OxEventHandler', () => { expect(oxServiceMock.send).toHaveBeenCalledTimes(1); expect(loggerMock.error).toHaveBeenLastCalledWith( - `Cannot get data for user with username:${person.referrer} from OX, Aborting Email-Address Change`, + `Cannot get data for username:${person.referrer} from OX, Aborting Email-Address Change, personId:${personId}, referrer:${referrer}`, ); }); @@ -876,7 +886,7 @@ describe('OxEventHandler', () => { expect(oxServiceMock.send).toHaveBeenCalledTimes(2); expect(loggerMock.error).toHaveBeenLastCalledWith( - `Could not change email-address for oxUserId:${person.oxUserId} in OX, error: Request failed`, + `Could not change email-address for oxUserId:${person.oxUserId}, personId:${personId}, referrer:${referrer}, error: Request failed`, ); }); @@ -909,12 +919,12 @@ describe('OxEventHandler', () => { `Found mostRecentRequested Email-Address:${JSON.stringify(email)} For personId:${personId}`, ); //use regex, because strict comparison fails, local test-var currentAliases has changed by the implemented function when expect is checked here + expect(loggerMock.info).toHaveBeenCalledWith(expect.stringMatching(/Found Current aliases:.*/)); expect(loggerMock.info).toHaveBeenCalledWith( - expect.stringMatching(/Found Current aliases:.* For personId:/), + `Added New alias:${email}, personId:${personId}, referrer:${referrer}`, ); - expect(loggerMock.info).toHaveBeenCalledWith(`Added New alias:${email} For personId:${personId}`); expect(loggerMock.info).toHaveBeenLastCalledWith( - `Changed primary email-address in OX for user, username:${person.referrer}, new email-address:${email}`, + `Changed primary email-address in OX for user, username:${person.referrer}, new email-address:${email}, personId:${personId}, referrer:${referrer}`, ); expect(eventServiceMock.publish).toHaveBeenLastCalledWith( expect.objectContaining({ diff --git a/src/modules/ox/domain/ox-event-handler.ts b/src/modules/ox/domain/ox-event-handler.ts index 9183ad28e..4cb91a5db 100644 --- a/src/modules/ox/domain/ox-event-handler.ts +++ b/src/modules/ox/domain/ox-event-handler.ts @@ -8,7 +8,7 @@ import { DomainError } from '../../../shared/error/index.js'; import { OxConfig } from '../../../shared/config/ox.config.js'; import { OxService } from './ox.service.js'; import { CreateUserAction, CreateUserParams, CreateUserResponse } from '../actions/user/create-user.action.js'; -import { PersonID } from '../../../shared/types/index.js'; +import { PersonID, PersonReferrer } from '../../../shared/types/index.js'; import { Person } from '../../person/domain/person.js'; import { PersonRepository } from '../../person/persistence/person.repository.js'; import { EmailAddressGeneratedEvent } from '../../../shared/events/email-address-generated.event.js'; @@ -84,27 +84,27 @@ export class OxEventHandler { @EventHandler(EmailAddressChangedEvent) public async handleEmailAddressChangedEvent(event: EmailAddressChangedEvent): Promise { this.logger.info( - `Received EmailAddressChangedEvent, personId:${event.personId}, oldEmailAddressId:${event.oldEmailAddressId}, oldAddress:${event.oldAddress}, newEmailAddressId:${event.newEmailAddressId}, newAddress:${event.newAddress}`, + `Received EmailAddressChangedEvent, personId:${event.personId}, referrer:${event.referrer}, oldEmailAddressId:${event.oldEmailAddressId}, oldAddress:${event.oldAddress}, newEmailAddressId:${event.newEmailAddressId}, newAddress:${event.newAddress}`, ); if (!this.ENABLED) { return this.logger.info('Not enabled, ignoring event'); } - await this.changeOxUser(event.personId); + await this.changeOxUser(event.personId, event.referrer); } @EventHandler(EmailAddressGeneratedEvent) public async handleEmailAddressGeneratedEvent(event: EmailAddressGeneratedEvent): Promise { this.logger.info( - `Received EmailAddressGeneratedEvent, personId:${event.personId}, emailAddressId:${event.emailAddressId}, address:${event.address}`, + `Received EmailAddressGeneratedEvent, personId:${event.personId}, referrer:${event.referrer}, emailAddressId:${event.emailAddressId}, address:${event.address}`, ); if (!this.ENABLED) { return this.logger.info('Not enabled, ignoring event'); } - await this.createOxUser(event.personId, event.orgaKennung); + await this.createOxUser(event.personId, event.referrer, event.orgaKennung); } @EventHandler(EmailAddressAlreadyExistsEvent) @@ -456,11 +456,11 @@ export class OxEventHandler { return result; } - private async createOxUser(personId: PersonID, orgaKennung: string): Promise { + private async createOxUser(personId: PersonID, referrer: PersonReferrer, orgaKennung: string): Promise { const person: Option> = await this.personRepository.findById(personId); if (!person) { - return this.logger.error(`Person not found for personId:${personId}`); + return this.logger.error(`Person not found for personId:${personId}, referrer:${referrer}`); } if (!person.referrer) { return this.logger.error(`Person with personId:${personId} has no referrer: cannot create OXEmailAddress`); @@ -483,7 +483,9 @@ export class OxEventHandler { const existsResult: Result = await this.oxService.send(existsAction); if (existsResult.ok && existsResult.value.exists) { - this.logger.error(`Cannot create user in OX, username:${person.referrer} already exists`); + this.logger.error( + `Cannot create user in OX, username:${person.referrer} already exists, personId:${personId}`, + ); return; } @@ -508,11 +510,13 @@ export class OxEventHandler { mostRecentRequestedEmailAddress.failed(); await this.emailRepo.save(mostRecentRequestedEmailAddress); - return this.logger.error(`Could not create user in OX, error: ${createUserResult.error.message}`); + return this.logger.error( + `Could not create user in OX, personId:${personId}, referrer:${referrer}, error: ${createUserResult.error.message}`, + ); } this.logger.info( - `User created in OX, oxUserId:${createUserResult.value.id}, oxEmail:${createUserResult.value.primaryEmail}, personId:${personId}`, + `User created in OX, oxUserId:${createUserResult.value.id}, oxEmail:${createUserResult.value.primaryEmail}, personId:${personId}, referrer:${referrer}`, ); mostRecentRequestedEmailAddress.oxUserID = createUserResult.value.id; @@ -522,7 +526,9 @@ export class OxEventHandler { if (emailAddressUpdateResult instanceof DomainError) { mostRecentRequestedEmailAddress.failed(); await this.emailRepo.save(mostRecentRequestedEmailAddress); - return this.logger.error(`Persisting oxUserId on emailAddress for personId:${personId} failed`); + return this.logger.error( + `Persisting oxUserId on emailAddress failed, personId:${personId}, referrer:${referrer}`, + ); } const oxGroupId: Result = await this.getExistingOxGroupByNameOrCreateOxGroup( @@ -563,7 +569,7 @@ export class OxEventHandler { if (!changeByModuleAccessResult.ok) { //only log error, do not set email-address status = FAILED, the ChangeByModuleAccessAction won't work against OX-DEV this.logger.error( - `Could Not Adjust GlobalAddressBookDisabled For oxUserId:${createUserResult.value.id}, error: ${changeByModuleAccessResult.error.message}`, + `Could Not Adjust GlobalAddressBookDisabled For oxUserId:${createUserResult.value.id}, personId:${personId}, referrer:${referrer}, error: ${changeByModuleAccessResult.error.message}`, ); } @@ -580,11 +586,11 @@ export class OxEventHandler { ); } - private async changeOxUser(personId: PersonID): Promise { + private async changeOxUser(personId: PersonID, referrer: PersonReferrer): Promise { const person: Option> = await this.personRepository.findById(personId); if (!person) { - return this.logger.error(`Person not found for personId:${personId}`); + return this.logger.error(`Person not found for personId:${personId}, referrer:${referrer}`); } if (!person.referrer) { return this.logger.error( @@ -592,7 +598,7 @@ export class OxEventHandler { ); } if (!person.oxUserId) { - return this.logger.error(`Person with personId:${personId} has no OXUserId`); + return this.logger.error(`Person has no OXUserId, personId:${personId}, referrer:${referrer}`); } const mostRecentRequestedEmailAddress: Option> = @@ -615,14 +621,16 @@ export class OxEventHandler { mostRecentRequestedEmailAddress.failed(); await this.emailRepo.save(mostRecentRequestedEmailAddress); return this.logger.error( - `Cannot get data for user with username:${person.referrer} from OX, Aborting Email-Address Change`, + `Cannot get data for username:${person.referrer} from OX, Aborting Email-Address Change, personId:${personId}, referrer:${referrer}`, ); } const newAliasesArray: string[] = getDataResult.value.aliases; - this.logger.info(`Found Current aliases:${JSON.stringify(newAliasesArray)} For personId:${personId}`); + this.logger.info( + `Found Current aliases:${JSON.stringify(newAliasesArray)}, personId:${personId}, referrer:${referrer}`, + ); newAliasesArray.push(requestedEmailAddressString); - this.logger.info(`Added New alias:${requestedEmailAddressString} For personId:${personId}`); + this.logger.info(`Added New alias:${requestedEmailAddressString}, personId:${personId}, referrer:${referrer}`); const params: ChangeUserParams = { contextId: this.contextID, @@ -648,12 +656,12 @@ export class OxEventHandler { await this.emailRepo.save(mostRecentRequestedEmailAddress); return this.logger.error( - `Could not change email-address for oxUserId:${person.oxUserId} in OX, error: ${result.error.message}`, + `Could not change email-address for oxUserId:${person.oxUserId}, personId:${personId}, referrer:${referrer}, error: ${result.error.message}`, ); } this.logger.info( - `Changed primary email-address in OX for user, username:${person.referrer}, new email-address:${requestedEmailAddressString}`, + `Changed primary email-address in OX for user, username:${person.referrer}, new email-address:${requestedEmailAddressString}, personId:${personId}, referrer:${referrer}`, ); this.eventService.publish( From 9f20069d9649bc4d5817ea40587d87c88dcfe8a3 Mon Sep 17 00:00:00 2001 From: DPDS93CT Date: Tue, 17 Dec 2024 07:10:37 +0100 Subject: [PATCH 3/7] adjustments to logging --- .../email/domain/email-event-handler.spec.ts | 16 +++++++++------- src/modules/email/domain/email-event-handler.ts | 16 +++++++--------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/modules/email/domain/email-event-handler.spec.ts b/src/modules/email/domain/email-event-handler.spec.ts index 8daef36b4..bac65ed14 100644 --- a/src/modules/email/domain/email-event-handler.spec.ts +++ b/src/modules/email/domain/email-event-handler.spec.ts @@ -374,7 +374,7 @@ describe('EmailEventHandler', () => { await emailEventHandler.handlePersonenkontextUpdatedEvent(event); expect(loggerMock.info).toHaveBeenCalledWith( - `Existing email for personId:${fakePersonId}, referrer:${fakeReferrer} already enabled`, + `Existing email for personId:${fakePersonId}, referrer:${fakeReferrer} already ENABLED`, ); }); }); @@ -474,7 +474,7 @@ describe('EmailEventHandler', () => { await emailEventHandler.handlePersonenkontextUpdatedEvent(event); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not enable email for personId:${fakePersonId}, referrer:${fakeReferrer}, error is requested EmailAddress with the address:${fakeEmailAddressString} was not found`, + `Could not ENABLE email for personId:${fakePersonId}, referrer:${fakeReferrer}, error is requested EmailAddress with the address:${fakeEmailAddressString} was not found`, ); }); }); @@ -886,6 +886,7 @@ describe('EmailEventHandler', () => { describe('handlePersonRenamedEvent', () => { let fakePersonId: PersonID; + let fakeReferrer: PersonReferrer; let fakeRolleId: RolleID; let fakeEmailAddress: string; let event: PersonRenamedEvent; @@ -898,6 +899,7 @@ describe('EmailEventHandler', () => { beforeEach(() => { fakePersonId = faker.string.uuid(); + fakeReferrer = faker.internet.userName(); fakeRolleId = faker.string.uuid(); fakeEmailAddress = faker.internet.email(); event = new PersonRenamedEvent( @@ -968,7 +970,7 @@ describe('EmailEventHandler', () => { emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(emailAddress); //mock person with referrer is found personRepositoryMock.findById.mockResolvedValueOnce( - createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + createMock>({ id: faker.string.uuid(), referrer: fakeReferrer }), ); emailRepoMock.save.mockResolvedValueOnce(emailAddress); @@ -991,7 +993,7 @@ describe('EmailEventHandler', () => { `DISABLED and saved address:${emailAddress.address}, personId:${event.personId}, referrer:${event.referrer}`, ); expect(loggerMock.error).toHaveBeenLastCalledWith( - `Could not create change-email for personId:${event.personId}, referrer:${event.referrer}, error is EmailAddress could not be created`, + `Could not create change-email for personId:${event.personId}, referrer:${fakeReferrer}, error is EmailAddress could not be created`, ); }); }); @@ -1004,7 +1006,7 @@ describe('EmailEventHandler', () => { emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(emailAddress); //mock person with referrer is found personRepositoryMock.findById.mockResolvedValueOnce( - createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + createMock>({ id: faker.string.uuid(), referrer: fakeReferrer }), ); emailRepoMock.save.mockResolvedValueOnce(emailAddress); @@ -1024,7 +1026,7 @@ describe('EmailEventHandler', () => { `DISABLED and saved address:${emailAddress.address}, personId:${event.personId}, referrer:${event.referrer}`, ); expect(loggerMock.error).toHaveBeenLastCalledWith( - `Could not persist change-email for personId:${event.personId}, referrer:${event.referrer}, error is EmailAddress could not be created`, + `Could not persist change-email for personId:${event.personId}, referrer:${fakeReferrer}, error is EmailAddress could not be created`, ); }); }); @@ -1281,7 +1283,7 @@ describe('EmailEventHandler', () => { await emailEventHandler.handleOxMetadataInKeycloakChangedEvent(event); expect(loggerMock.error).toHaveBeenLastCalledWith( - `Could not enable email for personId:${event.personId}, referrer:${event.keycloakUsername}, error is EmailAddress with ID 1 could not be updated`, + `Could not ENABLE email for personId:${event.personId}, referrer:${event.keycloakUsername}, error is EmailAddress with ID 1 could not be updated`, ); }); }); diff --git a/src/modules/email/domain/email-event-handler.ts b/src/modules/email/domain/email-event-handler.ts index 00e79a6a6..cda96519b 100644 --- a/src/modules/email/domain/email-event-handler.ts +++ b/src/modules/email/domain/email-event-handler.ts @@ -87,7 +87,6 @@ export class EmailEventHandler { if (existingEmail) { await this.changeEmail( event.personId, - event.referrer, pkForRolleWithSPReference.personenkontext.organisationId, existingEmail, ); @@ -292,7 +291,7 @@ export class EmailEventHandler { if (persistenceResult instanceof DomainError) { return this.logger.error( - `Could not enable email for personId:${event.personId}, referrer:${event.keycloakUsername}, error is ${persistenceResult.message}`, + `Could not ENABLE email for personId:${event.personId}, referrer:${event.keycloakUsername}, error is ${persistenceResult.message}`, ); } else { return this.logger.info( @@ -460,7 +459,7 @@ export class EmailEventHandler { for (const email of existingEmails) { if (email.enabled) { return this.logger.info( - `Existing email for personId:${personId}, referrer:${personReferrer.value} already enabled`, + `Existing email for personId:${personId}, referrer:${personReferrer.value} already ENABLED`, ); } else if (email.disabled) { // If we find a disabled address, we just enable it again @@ -485,7 +484,7 @@ export class EmailEventHandler { ); } else { this.logger.error( - `Could not enable email for personId:${personId}, referrer:${personReferrer.value}, error is ${persistenceResult.message}`, + `Could not ENABLE email for personId:${personId}, referrer:${personReferrer.value}, error is ${persistenceResult.message}`, ); } @@ -560,7 +559,6 @@ export class EmailEventHandler { private async changeEmail( personId: PersonID, - referrer: PersonReferrer | undefined, organisationId: OrganisationID, oldEmail: EmailAddress, ): Promise { @@ -572,17 +570,17 @@ export class EmailEventHandler { } const email: Result> = await this.emailFactory.createNew(personId, organisationId); if (!email.ok) { - await this.createAndPersistFailedEmailAddress(personId, referrer); + await this.createAndPersistFailedEmailAddress(personId, personReferrer.value); return this.logger.error( - `Could not create change-email for personId:${personId}, referrer:${referrer}, error is ${email.error.message}`, + `Could not create change-email for personId:${personId}, referrer:${personReferrer.value}, error is ${email.error.message}`, ); } email.value.request(); const persistenceResult: EmailAddress | DomainError = await this.emailRepo.save(email.value); if (persistenceResult instanceof EmailAddress) { this.logger.info( - `Successfully persisted change-email with REQUEST status for address:${persistenceResult.address}, personId:${personId}, referrer:${referrer}`, + `Successfully persisted change-email with REQUEST status for address:${persistenceResult.address}, personId:${personId}, referrer:${personReferrer.value}`, ); this.eventService.publish( new EmailAddressChangedEvent( @@ -597,7 +595,7 @@ export class EmailEventHandler { ); } else { this.logger.error( - `Could not persist change-email for personId:${personId}, referrer:${referrer}, error is ${persistenceResult.message}`, + `Could not persist change-email for personId:${personId}, referrer:${personReferrer.value}, error is ${persistenceResult.message}`, ); } } From b790e4d6032c8a4278e85c7a2e2baaada16b912d Mon Sep 17 00:00:00 2001 From: DPDS93CT Date: Tue, 17 Dec 2024 08:18:31 +0100 Subject: [PATCH 4/7] shorten test cases for EmailEventHandler and OxEventHandler --- .../email/domain/email-event-handler.spec.ts | 215 ++++++------------ .../ox/domain/ox-event-handler.spec.ts | 157 +++++-------- 2 files changed, 129 insertions(+), 243 deletions(-) diff --git a/src/modules/email/domain/email-event-handler.spec.ts b/src/modules/email/domain/email-event-handler.spec.ts index bac65ed14..e9c1608a9 100644 --- a/src/modules/email/domain/email-event-handler.spec.ts +++ b/src/modules/email/domain/email-event-handler.spec.ts @@ -148,6 +148,22 @@ describe('EmailEventHandler', () => { }); } + /** + * Mock dbiamPersonenkontextRepoMock.findByPerson, rolleRepoMock.findByIds and serviceProviderRepoMock.findByIds + * @param personenkontexte + * @param rolleMap + * @param spMap + */ + function mockRepositoryFindMethods( + personenkontexte: Personenkontext[], + rolleMap: Map>, + spMap: Map>, + ): void { + dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); + rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); + serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + } + describe('test private methods: createOrEnableEmail, createNewEmail, changeEmail, getPersonReferrerOrError', () => { let fakePersonId: PersonID; let fakeReferrer: PersonReferrer; @@ -202,10 +218,7 @@ describe('EmailEventHandler', () => { describe('createOrEnableEmail', () => { describe('when orgaKennung CANNOT be found', () => { it('should log matching error', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); - rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); - + mockRepositoryFindMethods(personenkontexte, rolleMap, spMap); emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found const persistenceResult: EmailAddress = getEmail(); @@ -240,10 +253,7 @@ describe('EmailEventHandler', () => { describe('createNewEmail', () => { describe('when orgaKennung CANNOT be found', () => { it('should log matching error', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); - rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); - + mockRepositoryFindMethods(personenkontexte, rolleMap, spMap); emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(undefined); //no existing email is found organisationRepositoryMock.findById.mockResolvedValue(undefined); @@ -260,9 +270,7 @@ describe('EmailEventHandler', () => { describe('changeEmail', () => { describe('when orgaKennung CANNOT be found', () => { it('should log error', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce([personenkontext]); - rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + mockRepositoryFindMethods(personenkontexte, rolleMap, spMap); emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(emailAddress); emailRepoMock.save.mockResolvedValueOnce(emailAddress); @@ -296,9 +304,7 @@ describe('EmailEventHandler', () => { describe('getPersonReferrerOrError', () => { describe('when personReferrer is NOT defined', () => { it('should log matching error', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); - rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + mockRepositoryFindMethods(personenkontexte, rolleMap, spMap); emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(undefined); //no existing email is found organisationRepositoryMock.findById.mockResolvedValue(createMock>()); //mock person without referrer is found @@ -350,9 +356,7 @@ describe('EmailEventHandler', () => { describe('when email exists, person with referrer can be found and is enabled', () => { it('should log matching info', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); - rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + mockRepositoryFindMethods(personenkontexte, rolleMap, spMap); // eslint-disable-next-line @typescript-eslint/require-await emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockImplementationOnce(async (personId: PersonID) => [ @@ -382,9 +386,7 @@ describe('EmailEventHandler', () => { //test case to cover case: getPersonReferrerOrError is returning error describe('when email exists, person WITHOUT referrer is found and is enabled', () => { it('should log matching info', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); - rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + mockRepositoryFindMethods(personenkontexte, rolleMap, spMap); // eslint-disable-next-line @typescript-eslint/require-await emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockImplementationOnce(async (personId: PersonID) => [ @@ -414,9 +416,7 @@ describe('EmailEventHandler', () => { describe('when email exists but is disabled and enabling is successful', () => { it('should log matching info', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); - rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + mockRepositoryFindMethods(personenkontexte, rolleMap, spMap); //mock person with referrer is found personRepositoryMock.findById.mockResolvedValueOnce( @@ -448,9 +448,7 @@ describe('EmailEventHandler', () => { describe('when email exists and but is disabled but enabling fails', () => { it('should log matching error', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); - rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + mockRepositoryFindMethods(personenkontexte, rolleMap, spMap); //mock person with referrer is found personRepositoryMock.findById.mockResolvedValueOnce( @@ -481,9 +479,7 @@ describe('EmailEventHandler', () => { describe('when email does NOT exist should create and persist a new one', () => { it('should log matching info', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); - rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + mockRepositoryFindMethods(personenkontexte, rolleMap, spMap); emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found //mock person with referrer is found personRepositoryMock.findById.mockResolvedValue( @@ -519,9 +515,7 @@ describe('EmailEventHandler', () => { describe('when email does NOT exist and error occurs during creation', () => { it('should log matching info', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); - rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + mockRepositoryFindMethods(personenkontexte, rolleMap, spMap); emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found //mock person with referrer is found personRepositoryMock.findById.mockResolvedValue( @@ -564,9 +558,8 @@ describe('EmailEventHandler', () => { personRepositoryMock.findById.mockResolvedValueOnce( createMock>({ referrer: fakeReferrer }), ); - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); - rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + mockRepositoryFindMethods(personenkontexte, rolleMap, spMap); + emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found //mock person with referrer is found personRepositoryMock.findById.mockResolvedValue( @@ -599,9 +592,7 @@ describe('EmailEventHandler', () => { personRepositoryMock.findById.mockResolvedValueOnce( createMock>({ referrer: fakeReferrer }), ); - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); - rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + mockRepositoryFindMethods(personenkontexte, rolleMap, spMap); emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found //mock person with referrer is found personRepositoryMock.findById.mockResolvedValue( @@ -638,9 +629,7 @@ describe('EmailEventHandler', () => { personRepositoryMock.findById.mockResolvedValueOnce( createMock>({ referrer: fakeReferrer }), ); - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); - rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(new Map>()); + mockRepositoryFindMethods(personenkontexte, rolleMap, new Map>()); emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([getEmail()]); emailRepoMock.save.mockResolvedValueOnce(new EmailAddressNotFoundError(fakeEmailAddressString)); //mock: error during saving the entity @@ -667,9 +656,8 @@ describe('EmailEventHandler', () => { personRepositoryMock.findById.mockResolvedValueOnce( createMock>({ referrer: fakeReferrer }), ); - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); - rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(new Map>()); + mockRepositoryFindMethods(personenkontexte, rolleMap, new Map>()); + personRepositoryMock.findById.mockResolvedValueOnce( createMock>({ referrer: fakeReferrer }), ); @@ -709,9 +697,8 @@ describe('EmailEventHandler', () => { describe(`When email is disabled and Person does not have an username, EmailAddressDisabledEvent is not published`, () => { it('should log matching error', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); - rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(new Map>()); + mockRepositoryFindMethods(personenkontexte, rolleMap, new Map>()); + personRepositoryMock.findById.mockResolvedValueOnce(createMock>({ referrer: undefined })); const emailAddress: EmailAddress = EmailAddress.construct( @@ -751,47 +738,38 @@ describe('EmailEventHandler', () => { }); describe('handlePersonenkontextCreatedMigrationEvent', () => { + const inputEmailAdress: string = 'test@schule-spsh.de'; + let personenkontext: Personenkontext; + let person: Person; + let rolle: Rolle; + let orga: Organisation; + let event: PersonenkontextCreatedMigrationEvent; + + beforeEach(() => { + personenkontext = createMock>(); + person = createMock>(); + rolle = createMock>(); + orga = createMock>(); + event = new PersonenkontextCreatedMigrationEvent( + PersonenkontextMigrationRuntype.STANDARD, + personenkontext, + person, + rolle, + orga, + inputEmailAdress, + ); + }); + describe('MigrationRunType: STANDARD', () => { - const migrationType: PersonenkontextMigrationRuntype = PersonenkontextMigrationRuntype.STANDARD; it('should do nothing when rolle is not LEHR', async () => { - const personenkontext: Personenkontext = createMock>(); - const person: Person = createMock>(); - const rolle: Rolle = createMock>(); - const orga: Organisation = createMock>(); - - const event: PersonenkontextCreatedMigrationEvent = new PersonenkontextCreatedMigrationEvent( - migrationType, - personenkontext, - person, - rolle, - orga, - 'test@schule-spsh.de', - ); - await emailEventHandler.handlePersonenkontextCreatedMigrationEvent(event); expect(loggerMock.info).toHaveBeenCalledWith( expect.stringContaining('No Action because Rollenart is Not LEHR'), ); }); it('should Create Email When None Exists and Rollenart is LEHR', async () => { - const inputEmailAdress: string = 'test@schule-spsh.de'; - - const personenkontext: Personenkontext = createMock>(); - const person: Person = createMock>(); - const rolle: Rolle = createMock>(); - const orga: Organisation = createMock>(); - rolle.rollenart = RollenArt.LEHR; - const event: PersonenkontextCreatedMigrationEvent = new PersonenkontextCreatedMigrationEvent( - migrationType, - personenkontext, - person, - rolle, - orga, - inputEmailAdress, - ); - emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); emailRepoMock.save.mockResolvedValueOnce(createMock>()); @@ -799,24 +777,8 @@ describe('EmailEventHandler', () => { expect(loggerMock.info).toHaveBeenCalledWith(expect.stringContaining('Successfully persisted Email')); }); it('should Log Error When Email persisting Operation fails', async () => { - const inputEmailAdress: string = 'test@schule-spsh.de'; - - const personenkontext: Personenkontext = createMock>(); - const person: Person = createMock>(); - const rolle: Rolle = createMock>(); - const orga: Organisation = createMock>(); - rolle.rollenart = RollenArt.LEHR; - const event: PersonenkontextCreatedMigrationEvent = new PersonenkontextCreatedMigrationEvent( - migrationType, - personenkontext, - person, - rolle, - orga, - inputEmailAdress, - ); - // eslint-disable-next-line @typescript-eslint/require-await emailRepoMock.findEnabledByPerson.mockImplementationOnce(async () => { return undefined; @@ -829,24 +791,8 @@ describe('EmailEventHandler', () => { ); }); it('should Abort When email is already persisted', async () => { - const inputEmailAdress: string = 'test@schule-spsh.de'; - - const personenkontext: Personenkontext = createMock>(); - const person: Person = createMock>(); - const rolle: Rolle = createMock>(); - const orga: Organisation = createMock>(); - rolle.rollenart = RollenArt.LEHR; - const event: PersonenkontextCreatedMigrationEvent = new PersonenkontextCreatedMigrationEvent( - migrationType, - personenkontext, - person, - rolle, - orga, - inputEmailAdress, - ); - // eslint-disable-next-line @typescript-eslint/require-await emailRepoMock.findEnabledByPerson.mockImplementationOnce(async () => { return createMock>(); @@ -860,23 +806,18 @@ describe('EmailEventHandler', () => { }); }); describe('MigrationRunType: ITSLEARNING', () => { - const migrationType: PersonenkontextMigrationRuntype = PersonenkontextMigrationRuntype.ITSLEARNING; it('should do nothing', async () => { - const personenkontext: Personenkontext = createMock>(); - const person: Person = createMock>(); - const rolle: Rolle = createMock>(); - const orga: Organisation = createMock>(); - - const event: PersonenkontextCreatedMigrationEvent = new PersonenkontextCreatedMigrationEvent( - migrationType, - personenkontext, - person, - rolle, - orga, - 'test@schule-spsh.de', - ); + const itsLearningTypeEvent: PersonenkontextCreatedMigrationEvent = + new PersonenkontextCreatedMigrationEvent( + PersonenkontextMigrationRuntype.ITSLEARNING, + personenkontext, + person, + rolle, + orga, + 'test@schule-spsh.de', + ); - await emailEventHandler.handlePersonenkontextCreatedMigrationEvent(event); + await emailEventHandler.handlePersonenkontextCreatedMigrationEvent(itsLearningTypeEvent); expect(loggerMock.info).toHaveBeenCalledWith( expect.stringContaining('No Action because PersonenkontextMigrationRuntype is Not STANDARD'), ); @@ -934,9 +875,7 @@ describe('EmailEventHandler', () => { describe('when rolle exists and service provider with kategorie email is found', () => { describe('when enabled email already exists and save disabling is successful', () => { it('should log info only', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce([personenkontext]); - rolleRepoMock.findByIds.mockResolvedValueOnce(rollenMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + mockRepositoryFindMethods([personenkontext], rollenMap, spMap); emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(emailAddress); //mock person with referrer is found personRepositoryMock.findById.mockResolvedValueOnce( @@ -964,9 +903,7 @@ describe('EmailEventHandler', () => { describe('when enabled email already exists and creating new (changed) email via factory fails', () => { it('should log error', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce([personenkontext]); - rolleRepoMock.findByIds.mockResolvedValueOnce(rollenMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + mockRepositoryFindMethods([personenkontext], rollenMap, spMap); emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(emailAddress); //mock person with referrer is found personRepositoryMock.findById.mockResolvedValueOnce( @@ -1000,9 +937,7 @@ describe('EmailEventHandler', () => { describe('when enabled email already exists and creating new (changed) email via factory succeeds but persisting fails', () => { it('should log error', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce([personenkontext]); - rolleRepoMock.findByIds.mockResolvedValueOnce(rollenMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + mockRepositoryFindMethods([personenkontext], rollenMap, spMap); emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(emailAddress); //mock person with referrer is found personRepositoryMock.findById.mockResolvedValueOnce( @@ -1033,9 +968,7 @@ describe('EmailEventHandler', () => { describe('when enabled email DOES NOT exist and creating new email is successful', () => { it('should log info', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce([personenkontext]); - rolleRepoMock.findByIds.mockResolvedValueOnce(rollenMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + mockRepositoryFindMethods([personenkontext], rollenMap, spMap); emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(undefined); emailRepoMock.save.mockResolvedValueOnce(emailAddress); @@ -1056,9 +989,7 @@ describe('EmailEventHandler', () => { describe('when NO rolle is referencing a SP with Email kategorie', () => { it('should log info only', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce([personenkontext]); - rolleRepoMock.findByIds.mockResolvedValueOnce(rollenMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(new Map>()); + mockRepositoryFindMethods([personenkontext], rollenMap, new Map>()); await emailEventHandler.handlePersonRenamedEvent(event); @@ -1075,9 +1006,7 @@ describe('EmailEventHandler', () => { describe('when enabled email already exists and save disabling results in error', () => { it('should log error', async () => { - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce([personenkontext]); - rolleRepoMock.findByIds.mockResolvedValueOnce(rollenMap); - serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + mockRepositoryFindMethods([personenkontext], rollenMap, spMap); emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(emailAddress); emailRepoMock.save.mockResolvedValueOnce(new EmailAddressNotFoundError(fakeEmailAddress)); diff --git a/src/modules/ox/domain/ox-event-handler.spec.ts b/src/modules/ox/domain/ox-event-handler.spec.ts index 68c383cfa..a210c385d 100644 --- a/src/modules/ox/domain/ox-event-handler.spec.ts +++ b/src/modules/ox/domain/ox-event-handler.spec.ts @@ -108,6 +108,49 @@ describe('OxEventHandler', () => { }); } + /** + * Mock group retrieval as successful + * @param oxUserId + * @param oxGroupId default is 'groupId' + */ + function mockGroupRetrievalRequestSuccessful(oxUserId: OXUserID, oxGroupId: OXGroupID = 'groupId'): void { + oxServiceMock.send.mockResolvedValueOnce({ + ok: true, + value: { + groups: [ + { + displayname: 'groupDisplayName', + id: oxGroupId, + name: 'groupName', + memberIds: [oxUserId], + }, + ], + }, + }); + } + + /** + * mock exists-oxUser-request + * @param exists default is FALSE + */ + function mockExistsUserRequest(exists: boolean = false): void { + if (exists) { + oxServiceMock.send.mockResolvedValueOnce({ + ok: true, + value: { + exists: true, + }, + }); + } else { + oxServiceMock.send.mockResolvedValueOnce({ + ok: true, + value: { + exists: false, + }, + }); + } + } + afterAll(async () => { await module.close(); }); @@ -146,12 +189,7 @@ describe('OxEventHandler', () => { ]); //mock exists-oxUser-request - oxServiceMock.send.mockResolvedValueOnce({ - ok: true, - value: { - exists: false, - }, - }); + mockExistsUserRequest(false); //mock create-oxUser-request const fakeOXUserId: string = faker.string.uuid(); mockUserCreationRequest(fakeOXUserId, event.address); @@ -208,12 +246,7 @@ describe('OxEventHandler', () => { ]); //mock exists-oxUser-request - oxServiceMock.send.mockResolvedValueOnce({ - ok: true, - value: { - exists: false, - }, - }); + mockExistsUserRequest(false); //mock create-oxUser-request const fakeOXUserId: string = faker.string.uuid(); mockUserCreationRequest(fakeOXUserId, event.address); @@ -266,12 +299,7 @@ describe('OxEventHandler', () => { ]); //mock exists-oxUser-request - oxServiceMock.send.mockResolvedValueOnce({ - ok: true, - value: { - exists: false, - }, - }); + mockExistsUserRequest(false); //mock create-oxUser-request const fakeOXUserId: string = faker.string.uuid(); mockUserCreationRequest(fakeOXUserId, event.address); @@ -301,12 +329,7 @@ describe('OxEventHandler', () => { ]); //mock exists-oxUser-request - oxServiceMock.send.mockResolvedValueOnce({ - ok: true, - value: { - exists: false, - }, - }); + mockExistsUserRequest(false); //mock create-oxUser-request const fakeOXUserId: string = faker.string.uuid(); mockUserCreationRequest(fakeOXUserId, event.address); @@ -361,12 +384,7 @@ describe('OxEventHandler', () => { ]); //mock exists-oxUser-request - oxServiceMock.send.mockResolvedValueOnce({ - ok: true, - value: { - exists: false, - }, - }); + mockExistsUserRequest(false); //mock create-oxUser-request const fakeOXUserId: string = faker.string.uuid(); mockUserCreationRequest(fakeOXUserId, event.address); @@ -433,12 +451,7 @@ describe('OxEventHandler', () => { ]); //mock exists-oxUser-request - oxServiceMock.send.mockResolvedValueOnce({ - ok: true, - value: { - exists: false, - }, - }); + mockExistsUserRequest(false); //mock create-oxUser-request const fakeOXUserId: string = faker.string.uuid(); mockUserCreationRequest(fakeOXUserId, event.address); @@ -507,12 +520,7 @@ describe('OxEventHandler', () => { personRepositoryMock.findById.mockResolvedValueOnce(person); emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([createMock>()]); //mock exists-oxUser-request - oxServiceMock.send.mockResolvedValueOnce({ - ok: true, - value: { - exists: true, - }, - }); + mockExistsUserRequest(true); await sut.handleEmailAddressGeneratedEvent(event); @@ -563,12 +571,7 @@ describe('OxEventHandler', () => { emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([createMock>()]); //mock exists-oxUser-request - oxServiceMock.send.mockResolvedValueOnce({ - ok: true, - value: { - exists: false, - }, - }); + mockExistsUserRequest(false); //mock create-oxUser-request const fakeOXUserId: string = faker.string.uuid(); mockUserCreationRequest(fakeOXUserId, event.address); @@ -620,12 +623,7 @@ describe('OxEventHandler', () => { emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([createMock>()]); //mock exists-oxUser-request - oxServiceMock.send.mockResolvedValueOnce({ - ok: true, - value: { - exists: false, - }, - }); + mockExistsUserRequest(false); //mock create-oxUser-request const fakeOXUserId: string = faker.string.uuid(); mockUserCreationRequest(fakeOXUserId, event.address); @@ -674,12 +672,7 @@ describe('OxEventHandler', () => { emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([createMock>()]); //mock exists-oxUser-request - oxServiceMock.send.mockResolvedValueOnce({ - ok: true, - value: { - exists: false, - }, - }); + mockExistsUserRequest(false); //mock create-oxUser-request const fakeOXUserId: string = faker.string.uuid(); mockUserCreationRequest(fakeOXUserId, event.address); @@ -1154,19 +1147,7 @@ describe('OxEventHandler', () => { ); // Mock group retrieval successfully - oxServiceMock.send.mockResolvedValueOnce({ - ok: true, - value: { - groups: [ - { - displayname: 'groupDisplayName', - id: 'groupId', - name: 'groupName', - memberIds: [oxUserId], - }, - ], - }, - }); + mockGroupRetrievalRequestSuccessful(oxUserId); // Mock removal as member from oxGroups successfully oxServiceMock.send.mockResolvedValueOnce({ @@ -1206,19 +1187,7 @@ describe('OxEventHandler', () => { ); // Mock group retrieval successfully - oxServiceMock.send.mockResolvedValueOnce({ - ok: true, - value: { - groups: [ - { - displayname: 'groupDisplayName', - id: 'groupId', - name: 'groupName', - memberIds: [oxUserId], - }, - ], - }, - }); + mockGroupRetrievalRequestSuccessful(oxUserId); // Mock removal as member from oxGroups successfully oxServiceMock.send.mockResolvedValueOnce({ @@ -1449,20 +1418,8 @@ describe('OxEventHandler', () => { oxUserId: oxUserId, }); personRepositoryMock.findById.mockResolvedValue(person); - //mock Ox-getOxGroupByName-request is successful - oxServiceMock.send.mockResolvedValueOnce({ - ok: true, - value: { - groups: [ - { - displayname: 'groupDisplayName', - id: oxGroupId, - name: 'groupName', - memberIds: [oxUserId], - }, - ], - }, - }); + // Mock group retrieval successfully + mockGroupRetrievalRequestSuccessful(oxUserId, oxGroupId); //mock Ox-removeOxUserFromOxGroup-request is successful oxServiceMock.send.mockResolvedValueOnce({ ok: true, From 90444c31e28b4fcb7054fe4baa85416033528d93 Mon Sep 17 00:00:00 2001 From: DPDS93CT Date: Tue, 17 Dec 2024 09:14:14 +0100 Subject: [PATCH 5/7] adjust type for persons referrer to PersonReferrer --- src/core/ldap/domain/ldap-client.service.spec.ts | 4 ++-- src/core/ldap/domain/ldap-client.service.ts | 12 +++++------- src/modules/ox/domain/ox-event-handler.spec.ts | 2 +- src/modules/person/api/person.controller.spec.ts | 4 ++-- .../person/api/person.frontend.controller.spec.ts | 3 ++- src/modules/person/persistence/person.repository.ts | 4 ++-- .../domain/personenkontext.factory.ts | 4 ++-- .../personenkontext/domain/personenkontext.ts | 8 ++++---- .../persistence/personenkontext.scope.ts | 4 ++-- ...privacy-idea-administration-event-handler.spec.ts | 3 ++- .../privacy-idea-administration.controller.ts | 11 ++++++----- .../privacy-idea-administration.service.spec.ts | 3 ++- src/shared/events/person-renamed-event.ts | 2 +- 13 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/core/ldap/domain/ldap-client.service.spec.ts b/src/core/ldap/domain/ldap-client.service.spec.ts index de02b8202..84fb5cf0d 100644 --- a/src/core/ldap/domain/ldap-client.service.spec.ts +++ b/src/core/ldap/domain/ldap-client.service.spec.ts @@ -18,7 +18,7 @@ import { Person } from '../../../modules/person/domain/person.js'; import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { LdapClient } from './ldap-client.js'; import { Attribute, Change, Client, Entry, SearchResult } from 'ldapts'; -import { PersonID } from '../../../shared/types/aggregate-ids.types.js'; +import { PersonID, PersonReferrer } from '../../../shared/types/aggregate-ids.types.js'; import { LdapSearchError } from '../error/ldap-search.error.js'; import { LdapEntityType } from './ldap.types.js'; import { ClassLogger } from '../../logging/class-logger.js'; @@ -549,7 +549,7 @@ describe('LDAP Client Service', () => { }); describe('when modifying', () => { it('Should Update LDAP When called with Attributes', async () => { - const oldReferrer: string = faker.internet.userName(); + const oldReferrer: PersonReferrer = faker.internet.userName(); const newGivenName: string = faker.person.firstName(); const newSn: string = faker.person.lastName(); const newUid: string = faker.string.alphanumeric(6); diff --git a/src/core/ldap/domain/ldap-client.service.ts b/src/core/ldap/domain/ldap-client.service.ts index 02a97318c..fd2ac647d 100644 --- a/src/core/ldap/domain/ldap-client.service.ts +++ b/src/core/ldap/domain/ldap-client.service.ts @@ -104,7 +104,7 @@ export class LdapClientService { }; } - private getLehrerUid(referrer: string, rootName: string): string { + private getLehrerUid(referrer: PersonReferrer, rootName: string): string { return `uid=${referrer},ou=${rootName},${LdapClientService.DC_SCHULE_SH_DC_DE}`; } @@ -121,7 +121,7 @@ export class LdapClientService { domain: string, mail?: string, //Wird hier erstmal seperat mit reingegeben bis die Umstellung auf primary/alternative erfolgt ): Promise> { - const referrer: string | undefined = person.referrer; + const referrer: PersonReferrer | undefined = person.referrer; if (!referrer) { return { ok: false, @@ -184,7 +184,7 @@ export class LdapClientService { }); } - public async isLehrerExisting(referrer: string, domain: string): Promise> { + public async isLehrerExisting(referrer: PersonReferrer, domain: string): Promise> { const rootName: Result = this.getRootNameOrError(domain); if (!rootName.ok) return rootName; @@ -208,7 +208,7 @@ export class LdapClientService { } public async modifyPersonAttributes( - oldReferrer: string, + oldReferrer: PersonReferrer, newGivenName?: string, newSn?: string, newUid?: string, @@ -286,7 +286,7 @@ export class LdapClientService { }); } - public async deleteLehrerByReferrer(referrer: string): Promise> { + public async deleteLehrerByReferrer(referrer: PersonReferrer): Promise> { return this.mutex.runExclusive(async () => { this.logger.info('LDAP: deleteLehrer'); const client: Client = this.ldapClient.getClient(); @@ -341,7 +341,6 @@ export class LdapClientService { newEmailAddress: string, ): Promise> { // Converted to avoid PersonRepository-ref, UEM-password-generation - //const referrer: string | undefined = await this.getPersonReferrerOrUndefined(personId); return this.mutex.runExclusive(async () => { this.logger.info('LDAP: changeEmailAddress'); const splitted: string[] = newEmailAddress.split('@'); @@ -427,7 +426,6 @@ export class LdapClientService { public async changeUserPasswordByPersonId(personId: PersonID, referrer: PersonReferrer): Promise> { // Converted to avoid PersonRepository-ref, UEM-password-generation - //const referrer: string | undefined = await this.getPersonReferrerOrUndefined(personId); const userPassword: string = generatePassword(); return this.mutex.runExclusive(async () => { diff --git a/src/modules/ox/domain/ox-event-handler.spec.ts b/src/modules/ox/domain/ox-event-handler.spec.ts index a210c385d..2c3fd6f87 100644 --- a/src/modules/ox/domain/ox-event-handler.spec.ts +++ b/src/modules/ox/domain/ox-event-handler.spec.ts @@ -757,7 +757,7 @@ describe('OxEventHandler', () => { let personId: PersonID; let event: EmailAddressChangedEvent; let person: Person; - let referrer: string; + let referrer: PersonReferrer; let email: string; let oxUserId: string; let oxUserName: string; diff --git a/src/modules/person/api/person.controller.spec.ts b/src/modules/person/api/person.controller.spec.ts index d29f1eba6..d62c438f4 100644 --- a/src/modules/person/api/person.controller.spec.ts +++ b/src/modules/person/api/person.controller.spec.ts @@ -20,7 +20,7 @@ import { PersonendatensatzResponse } from './personendatensatz.response.js'; import { KeycloakClientError } from '../../../shared/error/keycloak-client.error.js'; import { PersonFactory } from '../domain/person.factory.js'; import { PersonPermissions } from '../../authentication/domain/person-permissions.js'; -import { OrganisationID } from '../../../shared/types/index.js'; +import { OrganisationID, PersonReferrer } from '../../../shared/types/index.js'; import { EntityCouldNotBeDeleted, EntityNotFoundError, MismatchedRevisionError } from '../../../shared/error/index.js'; import { ConfigService } from '@nestjs/config'; import { DBiamPersonenkontextRepo } from '../../personenkontext/persistence/dbiam-personenkontext.repo.js'; @@ -392,7 +392,7 @@ describe('PersonController', () => { describe('findPersons', () => { const options: { - referrer: string; + referrer: PersonReferrer; lastName: string; firstName: string; } = { diff --git a/src/modules/person/api/person.frontend.controller.spec.ts b/src/modules/person/api/person.frontend.controller.spec.ts index 1e45c52e8..52c9e9d42 100644 --- a/src/modules/person/api/person.frontend.controller.spec.ts +++ b/src/modules/person/api/person.frontend.controller.spec.ts @@ -12,6 +12,7 @@ import { PersonendatensatzResponse } from './personendatensatz.response.js'; import { PersonPermissions } from '../../authentication/domain/person-permissions.js'; import { Personenkontext } from '../../personenkontext/domain/personenkontext.js'; import { UnauthorizedException } from '@nestjs/common'; +import { PersonReferrer } from '../../../shared/types/aggregate-ids.types.js'; describe('PersonFrontendController', () => { let module: TestingModule; @@ -47,7 +48,7 @@ describe('PersonFrontendController', () => { describe('findPersons', () => { const options: { - referrer: string; + referrer: PersonReferrer; lastName: string; firstName: string; } = { diff --git a/src/modules/person/persistence/person.repository.ts b/src/modules/person/persistence/person.repository.ts index e36bb72e2..df53240e2 100644 --- a/src/modules/person/persistence/person.repository.ts +++ b/src/modules/person/persistence/person.repository.ts @@ -12,7 +12,7 @@ import { MissingPermissionsError, } from '../../../shared/error/index.js'; import { ScopeOperator, ScopeOrder } from '../../../shared/persistence/scope.enums.js'; -import { PersonID } from '../../../shared/types/aggregate-ids.types.js'; +import { PersonID, PersonReferrer } from '../../../shared/types/aggregate-ids.types.js'; import { PermittedOrgas, PersonPermissions } from '../../authentication/domain/person-permissions.js'; import { KeycloakUserService, PersonHasNoKeycloakId, User } from '../../keycloak-administration/index.js'; import { RollenMerkmal, RollenSystemRecht } from '../../rolle/domain/rolle.enums.js'; @@ -441,7 +441,7 @@ export class PersonRepository { } public async update(person: Person): Promise | DomainError> { - let oldReferrer: string | undefined = ''; + let oldReferrer: PersonReferrer | undefined = ''; const personEntity: Loaded = await this.em.findOneOrFail(PersonEntity, person.id); const isPersonRenamedEventNecessary: boolean = this.hasChangedNames(personEntity, person); if (person.newPassword) { diff --git a/src/modules/personenkontext/domain/personenkontext.factory.ts b/src/modules/personenkontext/domain/personenkontext.factory.ts index 06b330d5a..49c108fa7 100644 --- a/src/modules/personenkontext/domain/personenkontext.factory.ts +++ b/src/modules/personenkontext/domain/personenkontext.factory.ts @@ -1,5 +1,5 @@ import { Injectable } from '@nestjs/common'; -import { OrganisationID, PersonID, RolleID } from '../../../shared/types/aggregate-ids.types.js'; +import { OrganisationID, PersonID, PersonReferrer, RolleID } from '../../../shared/types/aggregate-ids.types.js'; import { Personenkontext } from './personenkontext.js'; import { RolleRepo } from '../../rolle/repo/rolle.repo.js'; import { PersonRepository } from '../../person/persistence/person.repository.js'; @@ -55,7 +55,7 @@ export class PersonenkontextFactory { personId: PersonID, organisationId: OrganisationID, rolleId: RolleID, - referrer: string | undefined = undefined, + referrer: PersonReferrer | undefined = undefined, mandant: string | undefined = undefined, personenstatus: Personenstatus | undefined = undefined, jahrgangsstufe: Jahrgangsstufe | undefined = undefined, diff --git a/src/modules/personenkontext/domain/personenkontext.ts b/src/modules/personenkontext/domain/personenkontext.ts index a693a52ef..d6b2a42c7 100644 --- a/src/modules/personenkontext/domain/personenkontext.ts +++ b/src/modules/personenkontext/domain/personenkontext.ts @@ -1,7 +1,7 @@ import { DomainError } from '../../../shared/error/domain.error.js'; import { EntityNotFoundError } from '../../../shared/error/entity-not-found.error.js'; import { MissingPermissionsError } from '../../../shared/error/missing-permissions.error.js'; -import { OrganisationID, PersonID, RolleID } from '../../../shared/types/index.js'; +import { OrganisationID, PersonID, PersonReferrer, RolleID } from '../../../shared/types/index.js'; import { PersonPermissions } from '../../authentication/domain/person-permissions.js'; import { Organisation } from '../../organisation/domain/organisation.js'; import { OrganisationRepository } from '../../organisation/persistence/organisation.repository.js'; @@ -40,7 +40,7 @@ export class Personenkontext { public readonly organisationId: OrganisationID, public readonly rolleId: RolleID, // new - public readonly referrer: string | undefined, + public readonly referrer: PersonReferrer | undefined, public readonly mandant: string | undefined, public readonly personenstatus: Personenstatus | undefined, public readonly jahrgangsstufe: Jahrgangsstufe | undefined, @@ -61,7 +61,7 @@ export class Personenkontext { organisationId: OrganisationID, rolleId: RolleID, // new params - referrer: string | undefined = undefined, + referrer: PersonReferrer | undefined = undefined, mandant: string | undefined = undefined, personenstatus: Personenstatus | undefined = undefined, jahrgangsstufe: Jahrgangsstufe | undefined = undefined, @@ -100,7 +100,7 @@ export class Personenkontext { organisationId: OrganisationID, rolleId: RolleID, // new fields - referrer: string | undefined = undefined, + referrer: PersonReferrer | undefined = undefined, mandant: string | undefined = undefined, personenstatus: Personenstatus | undefined = undefined, jahrgangsstufe: Jahrgangsstufe | undefined = undefined, diff --git a/src/modules/personenkontext/persistence/personenkontext.scope.ts b/src/modules/personenkontext/persistence/personenkontext.scope.ts index 423884f8c..7e5def93d 100644 --- a/src/modules/personenkontext/persistence/personenkontext.scope.ts +++ b/src/modules/personenkontext/persistence/personenkontext.scope.ts @@ -3,12 +3,12 @@ import { ScopeBase } from '../../../shared/persistence/scope-base.js'; import { ScopeOperator } from '../../../shared/persistence/scope.enums.js'; import { Personenstatus, SichtfreigabeType } from '../domain/personenkontext.enums.js'; import { PersonenkontextEntity } from './personenkontext.entity.js'; -import { OrganisationID } from '../../../shared/types/aggregate-ids.types.js'; +import { OrganisationID, PersonReferrer } from '../../../shared/types/aggregate-ids.types.js'; import { RollenArt } from '../../rolle/domain/rolle.enums.js'; type FindProps = { personId: string; - referrer: string; + referrer: PersonReferrer; personenstatus: Personenstatus; sichtfreigabe: SichtfreigabeType; rolleart: RollenArt; diff --git a/src/modules/privacy-idea-administration/privacy-idea-administration-event-handler.spec.ts b/src/modules/privacy-idea-administration/privacy-idea-administration-event-handler.spec.ts index f740ca52e..41c17acab 100644 --- a/src/modules/privacy-idea-administration/privacy-idea-administration-event-handler.spec.ts +++ b/src/modules/privacy-idea-administration/privacy-idea-administration-event-handler.spec.ts @@ -7,6 +7,7 @@ import { TestingModule, Test } from '@nestjs/testing'; import { ConfigTestModule, LoggingTestModule } from '../../../test/utils/index.js'; import { PersonDeletedEvent } from '../../shared/events/person-deleted.event.js'; import { ResetTokenResponse, PrivacyIdeaToken } from './privacy-idea-api.types.js'; +import { PersonReferrer } from '../../shared/types/aggregate-ids.types.js'; export const mockPrivacyIdeaToken: PrivacyIdeaToken = { active: true, @@ -73,7 +74,7 @@ describe('PrivacyIdeaAdministration Event Handler', () => { describe('handlePersonDeletedEvent', () => { let personId: string; - let referrer: string; + let referrer: PersonReferrer; let emailAddress: string; let event: PersonDeletedEvent; let mockResetTokenResponse: ResetTokenResponse; diff --git a/src/modules/privacy-idea-administration/privacy-idea-administration.controller.ts b/src/modules/privacy-idea-administration/privacy-idea-administration.controller.ts index fcf9d0734..ba337de02 100644 --- a/src/modules/privacy-idea-administration/privacy-idea-administration.controller.ts +++ b/src/modules/privacy-idea-administration/privacy-idea-administration.controller.ts @@ -40,6 +40,7 @@ import { AssignTokenResponse, PrivacyIdeaToken, ResetTokenResponse } from './pri import { TokenInitBodyParams } from './token-init.body.params.js'; import { TokenStateResponse } from './token-state.response.js'; import { TokenVerifyBodyParams } from './token-verify.params.js'; +import { PersonReferrer } from '../../shared/types/aggregate-ids.types.js'; @UseFilters(new PrivacyIdeaAdministrationExceptionFilter()) @ApiTags('2FA') @@ -64,7 +65,7 @@ export class PrivacyIdeaAdministrationController { @Body() params: TokenInitBodyParams, @Permissions() permissions: PersonPermissions, ): Promise { - const referrer: string = await this.getReferrerIfAllowedOrSelf(params.personId, permissions); + const referrer: PersonReferrer = await this.getReferrerIfAllowedOrSelf(params.personId, permissions); const selfService: boolean = params.personId === permissions.personFields.id; return this.privacyIdeaAdministrationService.initializeSoftwareToken(referrer, selfService); @@ -82,7 +83,7 @@ export class PrivacyIdeaAdministrationController { @Query('personId') personId: string, @Permissions() permissions: PersonPermissions, ): Promise { - const referrer: string = await this.getReferrerIfAllowedOrSelf(personId, permissions); + const referrer: PersonReferrer = await this.getReferrerIfAllowedOrSelf(personId, permissions); const piToken: PrivacyIdeaToken | undefined = await this.privacyIdeaAdministrationService.getTwoAuthState(referrer); return new TokenStateResponse(piToken); @@ -101,7 +102,7 @@ export class PrivacyIdeaAdministrationController { @Query('personId') personId: string, @Permissions() permissions: PersonPermissions, ): Promise { - const referrer: string = await this.getReferrerIfAllowed(personId, permissions); + const referrer: PersonReferrer = await this.getReferrerIfAllowed(personId, permissions); try { const response: ResetTokenResponse = await this.privacyIdeaAdministrationService.resetToken(referrer); return response.result.status; @@ -132,7 +133,7 @@ export class PrivacyIdeaAdministrationController { @Body() params: AssignHardwareTokenBodyParams, @Permissions() permissions: PersonPermissions, ): Promise { - const referrer: string = await this.getReferrerIfAllowed(params.userId, permissions); + const referrer: PersonReferrer = await this.getReferrerIfAllowed(params.userId, permissions); try { const result: AssignTokenResponse = await this.privacyIdeaAdministrationService.assignHardwareToken( params.serial, @@ -172,7 +173,7 @@ export class PrivacyIdeaAdministrationController { @Body() params: TokenVerifyBodyParams, @Permissions() permissions: PersonPermissions, ): Promise { - const referrer: string = await this.getReferrerIfAllowedOrSelf(params.personId, permissions); + const referrer: PersonReferrer = await this.getReferrerIfAllowedOrSelf(params.personId, permissions); await this.privacyIdeaAdministrationService.verifyTokenEnrollment(referrer, params.otp); } diff --git a/src/modules/privacy-idea-administration/privacy-idea-administration.service.spec.ts b/src/modules/privacy-idea-administration/privacy-idea-administration.service.spec.ts index f1b856929..860b503b3 100644 --- a/src/modules/privacy-idea-administration/privacy-idea-administration.service.spec.ts +++ b/src/modules/privacy-idea-administration/privacy-idea-administration.service.spec.ts @@ -29,6 +29,7 @@ import { DeleteUserError } from './api/error/delete-user.error.js'; import { SoftwareTokenInitializationError } from './api/error/software-token-initialization.error.js'; import { TokenStateError } from './api/error/token-state.error.js'; import { PIUnavailableError } from './api/error/pi-unavailable.error.js'; +import { PersonReferrer } from '../../shared/types/aggregate-ids.types.js'; const mockErrorMsg: string = `Mock error`; @@ -916,7 +917,7 @@ describe(`PrivacyIdeaAdministrationService`, () => { }); describe('deleteUser', () => { - const referrer: string = faker.string.alpha(); + const referrer: PersonReferrer = faker.string.alpha(); let mockJWTToken: string; beforeEach(() => { mockJWTToken = faker.string.alpha(); diff --git a/src/shared/events/person-renamed-event.ts b/src/shared/events/person-renamed-event.ts index 0ea078f11..fff587850 100644 --- a/src/shared/events/person-renamed-event.ts +++ b/src/shared/events/person-renamed-event.ts @@ -14,7 +14,7 @@ export class PersonRenamedEvent extends BaseEvent { super(); } - public static fromPerson(person: Person, oldReferrer: string): PersonRenamedEvent { + public static fromPerson(person: Person, oldReferrer: PersonReferrer): PersonRenamedEvent { return new PersonRenamedEvent(person.id, person.vorname, person.familienname, person.referrer, oldReferrer); } } From 979e8621adfebbcc667156bbbef071e69beda949 Mon Sep 17 00:00:00 2001 From: DPDS93CT Date: Tue, 17 Dec 2024 09:56:35 +0100 Subject: [PATCH 6/7] fix sonarCloud-quality-gade issue --- src/modules/email/domain/email-event-handler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/email/domain/email-event-handler.ts b/src/modules/email/domain/email-event-handler.ts index cda96519b..72fb708b7 100644 --- a/src/modules/email/domain/email-event-handler.ts +++ b/src/modules/email/domain/email-event-handler.ts @@ -247,7 +247,7 @@ export class EmailEventHandler { PersonReferrer | undefined >(); const personIdsSet: Set = new Set(); - personenkontexte.map((pk: Personenkontext) => { + personenkontexte.forEach((pk: Personenkontext) => { personIdsSet.add(pk.personId); personIdReferrerMap.set(pk.personId, pk.referrer); }); From 8e843293d008f28479ac321ec37606a3b8493195 Mon Sep 17 00:00:00 2001 From: DPDS93CT Date: Tue, 17 Dec 2024 14:07:40 +0100 Subject: [PATCH 7/7] rm unnecessary if-condition --- .../ox/domain/ox-event-handler.spec.ts | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/modules/ox/domain/ox-event-handler.spec.ts b/src/modules/ox/domain/ox-event-handler.spec.ts index 2c3fd6f87..10c25d630 100644 --- a/src/modules/ox/domain/ox-event-handler.spec.ts +++ b/src/modules/ox/domain/ox-event-handler.spec.ts @@ -134,21 +134,12 @@ describe('OxEventHandler', () => { * @param exists default is FALSE */ function mockExistsUserRequest(exists: boolean = false): void { - if (exists) { - oxServiceMock.send.mockResolvedValueOnce({ - ok: true, - value: { - exists: true, - }, - }); - } else { - oxServiceMock.send.mockResolvedValueOnce({ - ok: true, - value: { - exists: false, - }, - }); - } + oxServiceMock.send.mockResolvedValueOnce({ + ok: true, + value: { + exists: exists, + }, + }); } afterAll(async () => {