SPSH-752: Modul-spezifische Fehler-Klassen aus /shared entfernen (#540)

* update error handling docs * removed module erros in shared, adjusted error mapping and exception filtering --------- Co-authored-by: Philipp Kleybolte <[email protected]>
dBildungsplattform · Jun 13, 2024 · 320dfbc · 320dfbc
1 parent 1859fbe
commit 320dfbc
Show file tree

Hide file tree

Showing 13 changed files with 75 additions and 99 deletions.
diff --git a/docs/error-handling.md b/docs/error-handling.md
@@ -1,12 +1,10 @@
 # Error Handling
 
-Error Handling for the SchulConneX API methods is done as specified by the respective documentation. We adhere to the
-standard and return the same errors.
+SchulConnex define a format for errors in the API. Unfortunately The SchulConnex error format does not contain a key that is suitable for translation. Therefore for endpoints that are to follow the SchulConnex API we return the respective error format. For our own services we return an `DbiamError` containing an i18nKey.
 
-The current list of possible Errors is listed either in the [official documentation](https://github.com/SchulConneX/v1)
-or in our [Confluence](https://docs.dbildungscloud.de/x/sIbbE).
+Error handling in Nest.js is done by `ExceptionFilters`. Each Controller and each endpoint can define what `ExceptionFilters` to use.
 
-#### SchulConnex Error Format
+### SchulConnex Errors
 ```json
 {
 "code": "401",
@@ -16,26 +14,29 @@ or in our [Confluence](https://docs.dbildungscloud.de/x/sIbbE).
 }
 ```
 
-## Error Types
-
-### General & Validation Errors
-
-General Errors and Validation Errors are handled by a NestJs `ExceptionFilter`.
-The `SchulConnexValidationErrorFilter` catches all Validation Errors thrown by the `class-validator`
-The `GlobalExceptionFilter` cathes all other Errors that get thrown by NestJs
-Annotations used to validate the API Inputs and transformed into the appropriate SchulConneX error.
+The current list of possible Errors is listed either in the [official documentation](https://github.com/SchulConneX/v1)
+or in our [Confluence](https://docs.dbildungscloud.de/x/sIbbE).
 
-To expand or alter the error mappings, you need only to edit the class
-`./shared/error/schulconnex-validation-error.filter.ts` and its appropriate tests.
+## Error Types
 
 ### Domain Errors
 
-Domain Errors are handled by mapping a descendant of `DomainError` to `SchulConnexError`
-through a general `SchulConnexErrorMapper` in the UC. It is important, that we have a clear
-mapping of `DomainError`s to appropriate SchulConneX Errorcodes and Subcodes, although several
-`DomainError`s can be mapped to the same `SchulConnexError`.
+Domain errors should extend the `DomainError` class. Each module needs to implement its own `ExceptionFilter` to transform the `DomainErrors` of the module into `DbiamErrors`. It might be necessary to add another `ExceptionFilter` to transform into `SchulConnexErrors` for the SchoolConnex endpoints.
 
 It is important, that we do NOT throw Errors from our Domain layer, but return them as part of the result.
 (For details, see the general Codestyle Documentation //TODO!!!)
 
 Those should also be documented in our [Confluence](https://docs.dbildungscloud.de/x/sIbbE)
+
+### Validation Errors
+
+If the validation rules implemented with Decorators in the DTO classes are violated, the `class-validator` lib throws `ValidationErrors`.<br>
+
+The `GlobalValidationPipe` defines rules for handling the `ValidationErrors` and aggregates multiple `ValidationErrors` for one requests into one `DetailedValidationError`.<br>
+The `SchulConnexValidationErrorFilter` catches all `DetailedValidationError` and transforms them into `SchulConnexErrors`.
+
+`SchulConnexValidationErrorFilter` is globally defined, so that it transforms all `DetailedValidationError` for all controllers.<br>
+Thus if you need your controller to transform validation errors into `DbiamErrors` you need to define an `ExceptionFilter` for that controller.
+
+### Global Error Handler
+All other errors are handled by the `GlobalExceptionFilter`.
diff --git a/src/modules/personenkontext/api/dbiam-personenkontext.controller.integration-spec.ts b/src/modules/personenkontext/api/dbiam-personenkontext.controller.integration-spec.ts
@@ -404,5 +404,30 @@ describe('dbiam Personenkontext API', () => {
                 });
             });
         });
+
+        describe('when OrganisationMatchesRollenart is not satisfied', () => {
+            it('should return error and map to 400', async () => {
+                const person: PersonDo<true> = await personRepo.save(DoFactory.createPerson(false));
+                const organisation: OrganisationDo<true> = await organisationRepo.save(
+                    DoFactory.createOrganisation(false, { typ: OrganisationsTyp.SCHULE }),
+                );
+                const rolle: Rolle<true> = await rolleRepo.save(
+                    DoFactory.createRolle(false, {
+                        administeredBySchulstrukturknoten: organisation.id,
+                        rollenart: RollenArt.SYSADMIN,
+                    }),
+                );
+
+                const personpermissions: DeepMocked<PersonPermissions> = createMock();
+                personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions);
+                personpermissions.hasSystemrechtAtOrganisation.mockResolvedValueOnce(false);
+
+                const response: Response = await request(app.getHttpServer() as App)
+                    .post('/dbiam/personenkontext')
+                    .send({ personId: person.id, organisationId: organisation.id, rolleId: rolle.id });
+
+                expect(response.status).toBe(400);
+            });
+        });
     });
 });
diff --git a/src/modules/personenkontext/api/dbiam-personenkontext.controller.ts b/src/modules/personenkontext/api/dbiam-personenkontext.controller.ts
@@ -27,6 +27,7 @@ import { PersonenkontextCreatedEvent } from '../../../shared/events/personenkont
 import { DbiamPersonenkontextError } from './dbiam-personenkontext.error.js';
 import { DBiamPersonenkontextResponse } from './dbiam-personenkontext.response.js';
 import { PersonenkontextExceptionFilter } from './personenkontext-exception-filter.js';
+import { OrganisationMatchesRollenartError } from '../specification/error/organisation-matches-rollenart.error.js';
 
 @UseFilters(new SchulConnexValidationErrorFilter(), new PersonenkontextExceptionFilter())
 @ApiTags('dbiam-personenkontexte')
@@ -103,6 +104,10 @@ export class DBiamPersonenkontextController {
         );
 
         if (!saveResult.ok) {
+            if (saveResult.error instanceof OrganisationMatchesRollenartError) {
+                throw saveResult.error;
+            }
+
             throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException(
                 SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(saveResult.error),
             );

diff --git a/src/modules/personenkontext/api/dbiam-personenkontext.error.ts b/src/modules/personenkontext/api/dbiam-personenkontext.error.ts
@@ -5,6 +5,8 @@ export enum PersonenkontextSpecificationErrorI18nTypes {
     PERSONENKONTEXT_SPECIFICATION_ERROR = 'PERSONENKONTEXT_SPECIFICATION_ERROR',
     NUR_LEHR_UND_LERN_AN_KLASSE = 'NUR_LEHR_UND_LERN_AN_KLASSE',
     GLEICHE_ROLLE_AN_KLASSE_WIE_SCHULE = 'GLEICHE_ROLLE_AN_KLASSE_WIE_SCHULE',
+    ORGANISATION_MATCHES_ROLLENART = 'ORGANISATION_MATCHES_ROLLENART',
+    PERSONENKONTEXT_ANLAGE_ERROR = 'PERSONENKONTEXT_ANLAGE_ERROR',
 }
 export type DbiamPersonenkontextErrorProps = DbiamErrorProps & {
     i18nKey: PersonenkontextSpecificationErrorI18nTypes;

diff --git a/src/modules/personenkontext/api/personenkontext-exception-filter.ts b/src/modules/personenkontext/api/personenkontext-exception-filter.ts
@@ -8,6 +8,7 @@ import {
 import { NurLehrUndLernAnKlasseError } from '../specification/error/nur-lehr-und-lern-an-klasse.error.js';
 import { GleicheRolleAnKlasseWieSchuleError } from '../specification/error/gleiche-rolle-an-klasse-wie-schule.error.js';
 import { PersonenkontextSpecificationError } from '../specification/error/personenkontext-specification.error.js';
+import { OrganisationMatchesRollenartError } from '../specification/error/organisation-matches-rollenart.error.js';
 
 @Catch(PersonenkontextSpecificationError)
 export class PersonenkontextExceptionFilter implements ExceptionFilter<PersonenkontextSpecificationError> {
@@ -26,6 +27,13 @@ export class PersonenkontextExceptionFilter implements ExceptionFilter<Personenk
                 i18nKey: PersonenkontextSpecificationErrorI18nTypes.GLEICHE_ROLLE_AN_KLASSE_WIE_SCHULE,
             }),
         ],
+        [
+            OrganisationMatchesRollenartError.name,
+            new DbiamPersonenkontextError({
+                code: 400,
+                i18nKey: PersonenkontextSpecificationErrorI18nTypes.ORGANISATION_MATCHES_ROLLENART,
+            }),
+        ],
     ]);
 
     public catch(exception: PersonenkontextSpecificationError, host: ArgumentsHost): void {

diff --git a/src/modules/personenkontext/domain/personenkontext.spec.ts b/src/modules/personenkontext/domain/personenkontext.spec.ts
@@ -11,8 +11,8 @@ import { RolleRepo } from '../../rolle/repo/rolle.repo.js';
 import { PersonenkontextFactory } from './personenkontext.factory.js';
 import { Personenkontext } from './personenkontext.js';
 import { Organisation } from '../../organisation/domain/organisation.js';
-import { PersonenkontextAnlageError } from '../../../shared/error/personenkontext-anlage.error.js';
 import { OrganisationsTyp } from '../../organisation/domain/organisation.enums.js';
+import { OrganisationMatchesRollenartError } from '../specification/error/organisation-matches-rollenart.error.js';
 
 describe('Personenkontext aggregate', () => {
     let module: TestingModule;
@@ -169,7 +169,7 @@ describe('Personenkontext aggregate', () => {
 
             const result: Option<DomainError> = await personenkontext.checkReferences();
 
-            expect(result).toBeInstanceOf(PersonenkontextAnlageError);
+            expect(result).toBeInstanceOf(OrganisationMatchesRollenartError);
         });
     });
 

diff --git a/src/modules/personenkontext/domain/personenkontext.ts b/src/modules/personenkontext/domain/personenkontext.ts
@@ -1,7 +1,6 @@
 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 { PersonenkontextAnlageError } from '../../../shared/error/personenkontext-anlage.error.js';
 import { OrganisationID, PersonID, RolleID } from '../../../shared/types/index.js';
 import { PersonPermissions } from '../../authentication/domain/person-permissions.js';
 import { Organisation } from '../../organisation/domain/organisation.js';
@@ -11,6 +10,7 @@ import { RollenSystemRecht } from '../../rolle/domain/rolle.enums.js';
 import { Rolle } from '../../rolle/domain/rolle.js';
 import { RolleRepo } from '../../rolle/repo/rolle.repo.js';
 import { OrganisationMatchesRollenart } from '../specification/organisation-matches-rollenart.js';
+import { OrganisationMatchesRollenartError } from '../specification/error/organisation-matches-rollenart.error.js';
 
 export type PersonenkontextPartial = Pick<
     Personenkontext<boolean>,
@@ -115,9 +115,7 @@ export class Personenkontext<WasPersisted extends boolean> {
         //The aimed organisation needs to match the type of role to be assigned
         const organisationMatchesRollenart: OrganisationMatchesRollenart = new OrganisationMatchesRollenart();
         if (!organisationMatchesRollenart.isSatisfiedBy(orga, rolle)) {
-            return new PersonenkontextAnlageError(
-                'PersonenkontextAnlage invalid: role type does not match organisation type',
-            );
+            return new OrganisationMatchesRollenartError({});
         }
 
         return undefined;

diff --git a/src/modules/personenkontext/specification/error/organisation-matches-rollenart.error.ts b/src/modules/personenkontext/specification/error/organisation-matches-rollenart.error.ts
@@ -0,0 +1,11 @@
+import { PersonenkontextSpecificationError } from './personenkontext-specification.error.js';
+import { PersonenkontextSpecificationErrorI18nTypes } from '../../api/dbiam-personenkontext.error.js';
+
+export class OrganisationMatchesRollenartError extends PersonenkontextSpecificationError {
+    public constructor(details?: unknown[] | Record<string, undefined>) {
+        super(
+            `Personenkontext could not be created/updates because it violates ${PersonenkontextSpecificationErrorI18nTypes.ORGANISATION_MATCHES_ROLLENART} specification`,
+            details,
+        );
+    }
+}
diff --git a/src/shared/error/personenkontext-anlage.error.spec.ts b/src/shared/error/personenkontext-anlage.error.spec.ts
diff --git a/src/shared/error/personenkontext-anlage.error.ts b/src/shared/error/personenkontext-anlage.error.ts
diff --git a/src/shared/error/role-assignment.error.spec.ts b/src/shared/error/role-assignment.error.spec.ts
diff --git a/src/shared/error/role-assignment.error.ts b/src/shared/error/role-assignment.error.ts
diff --git a/src/shared/error/schul-connex-error.mapper.ts b/src/shared/error/schul-connex-error.mapper.ts
@@ -12,10 +12,7 @@ import { EntityAlreadyExistsError } from './entity-already-exists.error.js';
 import { InvalidCharacterSetError } from './invalid-character-set.error.js';
 import { InvalidAttributeLengthError } from './invalid-attribute-length.error.js';
 import { InvalidNameError } from './invalid-name.error.js';
-import { NurLehrUndLernAnKlasseError } from '../../modules/personenkontext/specification/error/nur-lehr-und-lern-an-klasse.error.js';
-import { GleicheRolleAnKlasseWieSchuleError } from '../../modules/personenkontext/specification/error/gleiche-rolle-an-klasse-wie-schule.error.js';
 import { MissingPermissionsError } from './missing-permissions.error.js';
-import { PersonenkontextAnlageError } from './personenkontext-anlage.error.js';
 
 export class SchulConnexErrorMapper {
     private static SCHULCONNEX_ERROR_MAPPINGS: Map<string, SchulConnexError> = new Map([
@@ -119,25 +116,6 @@ export class SchulConnexErrorMapper {
                 beschreibung: 'Die Anfrage ist fehlerhaft: Es konnte kein Benutzername generiert werden',
             }),
         ],
-        [
-            NurLehrUndLernAnKlasseError.name,
-            new SchulConnexError({
-                code: 400,
-                subcode: '00',
-                titel: 'Spezifikation von Personenkontext nicht erfüllt',
-                beschreibung: 'Nur Lehrer und Lernende können Klassen zugeordnet werden.',
-            }),
-        ],
-        [
-            GleicheRolleAnKlasseWieSchuleError.name,
-            new SchulConnexError({
-                code: 400,
-                subcode: '00',
-                titel: 'Spezifikation von Personenkontext nicht erfüllt',
-                beschreibung:
-                    'Die Rollenart der Person muss für die Klasse dieselbe sein wie an der zugehörigen Schule.',
-            }),
-        ],
         [
             MissingPermissionsError.name,
             new SchulConnexError({
@@ -147,15 +125,6 @@ export class SchulConnexErrorMapper {
                 beschreibung: 'Die angeforderte Entität existiert nicht',
             }),
         ],
-        [
-            PersonenkontextAnlageError.name,
-            new SchulConnexError({
-                code: 400,
-                subcode: '00',
-                titel: 'Spezifikation von Personenkontext nicht erfüllt',
-                beschreibung: 'Die Rollenart passt nicht zu der Organisation.',
-            }),
-        ],
     ]);
 
     private static NO_MAPPING_FOUND: SchulConnexError = new SchulConnexError({