Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SPSH-1288 #774

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open

SPSH-1288 #774

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
cda389c
Log creation of Schule and Klasse
he-meyer Nov 14, 2024
3edd75f
Log update of Klasse name
he-meyer Nov 14, 2024
b5443a7
Add logger
he-meyer Nov 14, 2024
5b2b6d3
Log deletion of Klasse
he-meyer Nov 14, 2024
3131ac3
Log update of Schule and Klasse
he-meyer Nov 14, 2024
7339c6c
Log creation, update and deletion of Rolle
he-meyer Nov 14, 2024
93753d3
Add logger to tests
he-meyer Nov 14, 2024
a783823
Add logger to test
he-meyer Nov 14, 2024
8b69ca7
Adapt test
he-meyer Nov 14, 2024
775c1f0
Adapt test
he-meyer Nov 18, 2024
0962b43
Adapt test
he-meyer Nov 19, 2024
c9c601d
Adapt test
he-meyer Nov 19, 2024
c41858b
Adapt helper function and log statements
he-meyer Nov 19, 2024
2297f3d
Adapt tests
he-meyer Nov 19, 2024
491e369
Fix tests
he-meyer Nov 20, 2024
f1a9c6d
Adapt tests to increase coverage
he-meyer Nov 20, 2024
eda234a
Add and adapt tests to increase coverage
he-meyer Nov 20, 2024
7b4d908
Add tests for coverage
he-meyer Nov 20, 2024
8ab0fb6
Adapt for coverage
he-meyer Nov 20, 2024
f7edf92
Log username
he-meyer Nov 22, 2024
829975a
Use empty mock in tests
he-meyer Nov 25, 2024
1e02e15
Use empty mocks in tests
he-meyer Nov 25, 2024
68f0bad
Avoid undefined checks and adapt tests
he-meyer Nov 25, 2024
514829a
Don't log Organisation name
he-meyer Nov 26, 2024
2964f72
Merge branch 'main' of https://github.com/dBildungsplattform/dbildung…
he-meyer Nov 26, 2024
8dd1055
Fix duplicate
he-meyer Nov 26, 2024
1751955
Add log for a new error case
he-meyer Nov 26, 2024
ea40302
Fix tests
he-meyer Nov 26, 2024
00ee643
Avoid undefined checks
he-meyer Nov 26, 2024
ac8c1df
Use correct log level
he-meyer Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 66 additions & 19 deletions src/modules/organisation/api/organisation.controller.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { OrganisationByIdBodyParams } from './organisation-by-id.body.params.js'
import { OrganisationRepository } from '../persistence/organisation.repository.js';
import { Organisation } from '../domain/organisation.js';
import { OrganisationResponse } from './organisation.response.js';
import { PersonPermissions } from '../../authentication/domain/person-permissions.js';
import { PersonenkontextRolleFields, PersonPermissions } from '../../authentication/domain/person-permissions.js';
import { EventService } from '../../../core/eventbus/index.js';
import { OrganisationRootChildrenResponse } from './organisation.root-children.response.js';
import { OrganisationSpecificationError } from '../specification/error/organisation-specification.error.js';
Expand All @@ -30,6 +30,7 @@ import { OrganisationByNameQueryParams } from './organisation-by-name.query.js';
import { DBiamPersonenkontextRepo } from '../../personenkontext/persistence/dbiam-personenkontext.repo.js';
import { ParentOrganisationenResponse } from './organisation.parents.response.js';
import { ParentOrganisationsByIdsBodyParams } from './parent-organisations-by-ids.body.params.js';
import { Person } from '../../person/domain/person.js';

function getFakeParamsAndBody(): [OrganisationByIdParams, OrganisationByIdBodyParams] {
const params: OrganisationByIdParams = {
Expand Down Expand Up @@ -88,6 +89,20 @@ describe('OrganisationController', () => {
});

describe('createOrganisation', () => {
const permissionsMock: PersonPermissions = createMock<PersonPermissions>({
get personFields(): Person<true> {
return createMock<Person<true>>({
familienname: 'current-user',
});
},
getPersonenkontextewithRoles: (): Promise<PersonenkontextRolleFields[]> =>
Promise.resolve([
{
organisationsId: '',
rolle: { systemrechte: [], serviceProviderIds: [] },
},
]),
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leeres Permissions-Objekt sollte an dieser Stelle ausreichend sein.

describe('when usecase returns a DTO', () => {
it('should not throw an error', async () => {
const params: CreateOrganisationBodyParams = {
Expand All @@ -101,7 +116,7 @@ describe('OrganisationController', () => {
const returnedValue: Organisation<true> = DoFactory.createOrganisation(true);

organisationServiceMock.createOrganisation.mockResolvedValueOnce({ ok: true, value: returnedValue });
await expect(organisationController.createOrganisation(params)).resolves.not.toThrow();
await expect(organisationController.createOrganisation(permissionsMock, params)).resolves.not.toThrow();
expect(organisationServiceMock.createOrganisation).toHaveBeenCalledTimes(1);
});
});
Expand Down Expand Up @@ -153,7 +168,7 @@ describe('OrganisationController', () => {
organisationRepositoryMock.findRootDirectChildren.mockResolvedValue(mockedRepoResponse);

try {
await organisationController.createOrganisation(params);
await organisationController.createOrganisation(permissionsMock, params);

fail('Expected error was not thrown');
} catch (error) {
Expand Down Expand Up @@ -202,7 +217,7 @@ describe('OrganisationController', () => {
error: new OrganisationSpecificationError('error', undefined),
});
await expect(
organisationController.createOrganisation({} as CreateOrganisationBodyParams),
organisationController.createOrganisation(permissionsMock, {} as CreateOrganisationBodyParams),
).rejects.toThrow(OrganisationSpecificationError);
expect(organisationServiceMock.createOrganisation).toHaveBeenCalledTimes(1);
});
Expand Down Expand Up @@ -249,7 +264,7 @@ describe('OrganisationController', () => {
error: {} as EntityNotFoundError,
});
await expect(
organisationController.createOrganisation({} as CreateOrganisationBodyParams),
organisationController.createOrganisation(permissionsMock, {} as CreateOrganisationBodyParams),
).rejects.toThrow(HttpException);
expect(organisationServiceMock.createOrganisation).toHaveBeenCalledTimes(1);
});
Expand Down Expand Up @@ -719,6 +734,20 @@ describe('OrganisationController', () => {
});

describe('updateOrganisationName', () => {
const permissionsMock: PersonPermissions = createMock<PersonPermissions>({
get personFields(): Person<true> {
return createMock<Person<true>>({
familienname: 'current-user',
});
},
getPersonenkontextewithRoles: (): Promise<PersonenkontextRolleFields[]> =>
Promise.resolve([
{
organisationsId: '',
rolle: { systemrechte: [], serviceProviderIds: [] },
},
]),
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Siehe oben

describe('when usecase succeeds', () => {
it('should not throw an error', async () => {
const oeffentlich: Organisation<true> = Organisation.construct(
Expand All @@ -745,7 +774,9 @@ describe('OrganisationController', () => {

organisationRepositoryMock.updateKlassenname.mockResolvedValueOnce(oeffentlich);

await expect(organisationController.updateOrganisationName(params, body)).resolves.not.toThrow();
await expect(
organisationController.updateOrganisationName(params, body, permissionsMock),
).resolves.not.toThrow();
});
});

Expand All @@ -760,9 +791,9 @@ describe('OrganisationController', () => {
};
organisationRepositoryMock.updateKlassenname.mockResolvedValueOnce(new NameRequiredForKlasseError());

await expect(organisationController.updateOrganisationName(params, body)).rejects.toThrow(
NameRequiredForKlasseError,
);
await expect(
organisationController.updateOrganisationName(params, body, permissionsMock),
).rejects.toThrow(NameRequiredForKlasseError);
});
});

Expand All @@ -778,14 +809,28 @@ describe('OrganisationController', () => {

organisationRepositoryMock.updateKlassenname.mockResolvedValueOnce(new EntityNotFoundError());

await expect(organisationController.updateOrganisationName(params, body)).rejects.toThrow(
HttpException,
);
await expect(
organisationController.updateOrganisationName(params, body, permissionsMock),
).rejects.toThrow(HttpException);
});
});
});

describe('updateOrganisationName', () => {
const permissionsMock: PersonPermissions = createMock<PersonPermissions>({
get personFields(): Person<true> {
return createMock<Person<true>>({
familienname: 'current-user',
});
},
getPersonenkontextewithRoles: (): Promise<PersonenkontextRolleFields[]> =>
Promise.resolve([
{
organisationsId: '',
rolle: { systemrechte: [], serviceProviderIds: [] },
},
]),
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Siehe oben

describe('when usecase succeeds', () => {
it('should not throw an error', async () => {
const oeffentlich: Organisation<true> = Organisation.construct(
Expand All @@ -812,7 +857,9 @@ describe('OrganisationController', () => {

organisationRepositoryMock.updateKlassenname.mockResolvedValueOnce(oeffentlich);

await expect(organisationController.updateOrganisationName(params, body)).resolves.not.toThrow();
await expect(
organisationController.updateOrganisationName(params, body, permissionsMock),
).resolves.not.toThrow();
});
});

Expand All @@ -827,9 +874,9 @@ describe('OrganisationController', () => {
};
organisationRepositoryMock.updateKlassenname.mockResolvedValueOnce(new NameRequiredForKlasseError());

await expect(organisationController.updateOrganisationName(params, body)).rejects.toThrow(
NameRequiredForKlasseError,
);
await expect(
organisationController.updateOrganisationName(params, body, permissionsMock),
).rejects.toThrow(NameRequiredForKlasseError);
});
});

Expand All @@ -845,9 +892,9 @@ describe('OrganisationController', () => {

organisationRepositoryMock.updateKlassenname.mockResolvedValueOnce(new EntityNotFoundError());

await expect(organisationController.updateOrganisationName(params, body)).rejects.toThrow(
HttpException,
);
await expect(
organisationController.updateOrganisationName(params, body, permissionsMock),
).rejects.toThrow(HttpException);
});
});
});
Expand Down
19 changes: 15 additions & 4 deletions src/modules/organisation/api/organisation.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,10 @@ export class OrganisationController {
@ApiUnauthorizedResponse({ description: 'Not authorized to create the organisation.' })
@ApiForbiddenResponse({ description: 'Not permitted to create the organisation.' })
@ApiInternalServerErrorResponse({ description: 'Internal server error while creating the organisation.' })
public async createOrganisation(@Body() params: CreateOrganisationBodyParams): Promise<OrganisationResponse> {
public async createOrganisation(
@Permissions() permissions: PersonPermissions,
@Body() params: CreateOrganisationBodyParams,
): Promise<OrganisationResponse> {
const [oeffentlich]: [Organisation<true> | undefined, Organisation<true> | undefined] =
await this.organisationRepository.findRootDirectChildren();

Expand All @@ -103,6 +106,7 @@ export class OrganisationController {
}
const result: Result<Organisation<true>, DomainError> = await this.organisationService.createOrganisation(
organisation,
permissions,
);
if (!result.ok) {
if (result.error instanceof OrganisationSpecificationError) {
Expand Down Expand Up @@ -415,12 +419,18 @@ export class OrganisationController {
@ApiBadRequestResponse({ description: 'The input was not valid.', type: DbiamOrganisationError })
@ApiNotFoundResponse({ description: 'The organisation that should be deleted does not exist.' })
@ApiUnauthorizedResponse({ description: 'Not authorized to delete the organisation.' })
public async deleteKlasse(@Param() params: OrganisationByIdParams): Promise<void> {
public async deleteKlasse(
@Param() params: OrganisationByIdParams,
@Permissions() permissions: PersonPermissions,
): Promise<void> {
if (await this.dBiamPersonenkontextRepo.isOrganisationAlreadyAssigned(params.organisationId)) {
throw new OrganisationIstBereitsZugewiesenError();
}

const result: Option<DomainError> = await this.organisationRepository.deleteKlasse(params.organisationId);
const result: Option<DomainError> = await this.organisationRepository.deleteKlasse(
params.organisationId,
permissions,
);
if (result instanceof DomainError) {
throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException(
SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(result),
Expand All @@ -441,18 +451,19 @@ export class OrganisationController {
public async updateOrganisationName(
@Param() params: OrganisationByIdParams,
@Body() body: OrganisationByNameBodyParams,
@Permissions() permissions: PersonPermissions,
): Promise<OrganisationResponse | DomainError> {
const result: DomainError | Organisation<true> = await this.organisationRepository.updateKlassenname(
params.organisationId,
body.name,
body.version,
permissions,
);

if (result instanceof DomainError) {
if (result instanceof OrganisationSpecificationError) {
throw result;
}

throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException(
SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(result),
);
Expand Down
3 changes: 2 additions & 1 deletion src/modules/organisation/domain/organisation-service-specification.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { DoFactory } from '../../../../test/utils/do-factory.js';
import { DatabaseTestModule } from '../../../../test/utils/database-test.module.js';
import { ConfigTestModule } from '../../../../test/utils/config-test.module.js';
import { MikroORM } from '@mikro-orm/core';
import { MapperTestModule } from '../../../../test/utils/index.js';
import { LoggingTestModule, MapperTestModule } from '../../../../test/utils/index.js';
import { OrganisationPersistenceMapperProfile } from '../persistence/organisation-persistence.mapper.profile.js';
import { ZyklusInOrganisationenError } from '../specification/error/zyklus-in-organisationen.error.js';
import { RootOrganisationImmutableError } from '../specification/error/root-organisation-immutable.error.js';
Expand Down Expand Up @@ -34,6 +34,7 @@ describe('OrganisationServiceSpecificationTest', () => {
DatabaseTestModule.forRoot({ isDatabaseRequired: true }),
MapperTestModule,
EventModule,
LoggingTestModule,
],
providers: [OrganisationService, OrganisationRepository, OrganisationPersistenceMapperProfile],
}).compile();
Expand Down
Loading
Loading