From 097373ef9d9d67c66509a8300ac77c366521866e Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 21 Dec 2023 18:34:09 +0100 Subject: [PATCH 01/27] session manager can load other entities --- .../session-manager.service.spec.ts | 18 ++++++++++++++++++ .../session-service/session-manager.service.ts | 18 ++++++++++++------ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/app/core/session/session-service/session-manager.service.spec.ts b/src/app/core/session/session-service/session-manager.service.spec.ts index 5bfa9ccac0..dcca182a04 100644 --- a/src/app/core/session/session-service/session-manager.service.spec.ts +++ b/src/app/core/session/session-service/session-manager.service.spec.ts @@ -38,6 +38,7 @@ import { EntityMapperService } from "../../entity/entity-mapper/entity-mapper.se import { mockEntityMapper } from "../../entity/entity-mapper/mock-entity-mapper-service"; import { User } from "../../user/user"; import { TEST_USER } from "../../user/demo-user-generator.service"; +import { Child } from "../../../child-dev-project/children/model/child"; describe("SessionManagerService", () => { let service: SessionManagerService; @@ -141,6 +142,23 @@ describe("SessionManagerService", () => { expect(currentUser.value).toEqual(adminUser); }); + it("should allow other entities to log in", async () => { + const childSession: SessionInfo = { name: "Child:123", roles: [] }; + mockKeycloak.login.and.resolveTo(childSession); + const loggedInChild = new Child("123"); + const otherChild = new Child("456"); + await TestBed.inject(EntityMapperService).saveAll([ + loggedInChild, + otherChild, + ]); + + await service.remoteLogin(); + + expect(sessionInfo.value).toBe(childSession); + expect(loginStateSubject.value).toBe(LoginState.LOGGED_IN); + expect(TestBed.inject(CurrentUserSubject).value).toEqual(loggedInChild); + }); + it("should automatically login, if the session is still valid", async () => { await service.remoteLogin(); diff --git a/src/app/core/session/session-service/session-manager.service.ts b/src/app/core/session/session-service/session-manager.service.ts index 22259249e5..85f84a0b7a 100644 --- a/src/app/core/session/session-service/session-manager.service.ts +++ b/src/app/core/session/session-service/session-manager.service.ts @@ -34,6 +34,7 @@ import { CurrentUserSubject } from "../current-user-subject"; import { EntityMapperService } from "../../entity/entity-mapper/entity-mapper.service"; import { filter } from "rxjs/operators"; import { Subscription } from "rxjs"; +import { Entity } from "../../entity/model/entity"; /** * This service handles the user session. @@ -96,18 +97,23 @@ export class SessionManagerService { return this.initializeUser(user); } - private async initializeUser(user: SessionInfo) { - await this.initializeDatabaseForCurrentUser(user); - this.sessionInfo.next(user); + private async initializeUser(session: SessionInfo) { + await this.initializeDatabaseForCurrentUser(session); + this.sessionInfo.next(session); this.loginStateSubject.next(LoginState.LOGGED_IN); + this.initUserEntity(session); + } - // TODO allow generic entities with fallback to User entity + private initUserEntity(user: SessionInfo) { + const entityType = user.name.includes(":") + ? Entity.extractTypeFromId(user.name) + : User; this.entityMapper - .load(User, user.name) + .load(entityType, user.name) .then((res) => this.currentUser.next(res)) .catch(() => undefined); this.updateSubscription = this.entityMapper - .receiveUpdates(User) + .receiveUpdates(entityType) .pipe( filter( ({ entity }) => From 7bf56b95aa32ff6fdd44ce310f520593deee8619 Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 21 Dec 2023 18:34:20 +0100 Subject: [PATCH 02/27] removed deprecated todo? --- .../session/auth/keycloak/account-page/account-page.component.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/core/session/auth/keycloak/account-page/account-page.component.ts b/src/app/core/session/auth/keycloak/account-page/account-page.component.ts index 2d69614f40..01bca2d16e 100644 --- a/src/app/core/session/auth/keycloak/account-page/account-page.component.ts +++ b/src/app/core/session/auth/keycloak/account-page/account-page.component.ts @@ -45,7 +45,6 @@ export class AccountPageComponent implements OnInit { return; } - // TODO can we use keycloak for this? this.authService.setEmail(this.email.value).subscribe({ next: () => this.alertService.addInfo( From 64c48bc03eb602da10348550a4e83668fbe515cc Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 21 Dec 2023 19:16:06 +0100 Subject: [PATCH 03/27] user security component is entity agnostic --- .../user-security.component.spec.ts | 8 ++++---- .../user-security/user-security.component.ts | 18 +++++++++++------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/app/core/user/user-security/user-security.component.spec.ts b/src/app/core/user/user-security/user-security.component.spec.ts index c939cc8ede..b7f2f7e749 100644 --- a/src/app/core/user/user-security/user-security.component.spec.ts +++ b/src/app/core/user/user-security/user-security.component.spec.ts @@ -33,7 +33,7 @@ describe("UserSecurityComponent", () => { name: "Not Assigned Role", description: "this role is not assigned to the user", }; - const user = { name: "test-user" } as User; + const user = Object.assign(new User(), { username: "test-user" }); let keycloakUser: KeycloakUser; beforeEach(async () => { @@ -57,7 +57,7 @@ describe("UserSecurityComponent", () => { { provide: SessionSubject, useValue: new BehaviorSubject({ - name: user.name, + name: user.getId(true), roles: [KeycloakAuthService.ACCOUNT_MANAGER_ROLE], }), }, @@ -80,7 +80,7 @@ describe("UserSecurityComponent", () => { expect(component.user).toBe(keycloakUser); expect(component.form).toHaveValue({ - username: user.name, + username: user.toString(), email: "my@email.de", roles: [assignedRole], }); @@ -116,7 +116,7 @@ describe("UserSecurityComponent", () => { expect(mockHttp.post).toHaveBeenCalledWith( jasmine.stringMatching(/\/account$/), { - username: user.name, + username: user.toString(), email: "new@email.com", roles: [assignedRole], enabled: true, diff --git a/src/app/core/user/user-security/user-security.component.ts b/src/app/core/user/user-security/user-security.component.ts index c0b1aa5a02..d78ea4c9e8 100644 --- a/src/app/core/user/user-security/user-security.component.ts +++ b/src/app/core/user/user-security/user-security.component.ts @@ -11,7 +11,6 @@ import { KeycloakUser, Role, } from "../../session/auth/keycloak/keycloak-auth.service"; -import { User } from "../user"; import { AlertService } from "../../alerts/alert.service"; import { HttpClient } from "@angular/common/http"; import { AppSettings } from "../../app-settings"; @@ -23,6 +22,8 @@ import { MatInputModule } from "@angular/material/input"; import { MatSelectModule } from "@angular/material/select"; import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; import { SessionSubject } from "../../session/auth/session-info"; +import { Entity } from "../../entity/model/entity"; +import { catchError } from "rxjs/operators"; @UntilDestroy() @DynamicComponent("UserSecurity") @@ -43,7 +44,7 @@ import { SessionSubject } from "../../session/auth/session-info"; standalone: true, }) export class UserSecurityComponent implements OnInit { - @Input() entity: User; + @Input() entity: Entity; form = this.fb.group({ username: [{ value: "", disabled: true }], email: ["", [Validators.required, Validators.email]], @@ -91,12 +92,15 @@ export class UserSecurityComponent implements OnInit { } ngOnInit() { - this.form.get("username").setValue(this.entity.name); + this.form.get("username").setValue(this.entity.toString()); if (this.authService) { - this.authService.getUser(this.entity.name).subscribe({ - next: (res) => this.assignUser(res), - error: () => undefined, - }); + this.authService + .getUser(this.entity.getId(true)) + .pipe(catchError(() => this.authService.getUser(this.entity.getId()))) + .subscribe({ + next: (res) => this.assignUser(res), + error: () => undefined, + }); } } From 3d2daa83ecde68f96f64a76312e9b9d2e23701e3 Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 22 Dec 2023 10:45:18 +0100 Subject: [PATCH 04/27] mage todo completion entity agnostic --- .../entity/default-datatype/view.directive.ts | 4 +-- .../display-todo-completion.component.html | 2 +- .../display-todo-completion.component.ts | 31 +++++++++++++++++-- src/app/features/todos/todo.service.ts | 4 +-- 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/app/core/entity/default-datatype/view.directive.ts b/src/app/core/entity/default-datatype/view.directive.ts index cb155918e3..5339d0f8b0 100644 --- a/src/app/core/entity/default-datatype/view.directive.ts +++ b/src/app/core/entity/default-datatype/view.directive.ts @@ -1,5 +1,5 @@ import { Entity } from "../model/entity"; -import { Directive, Input, OnChanges } from "@angular/core"; +import { Directive, Input, OnChanges, SimpleChanges } from "@angular/core"; @Directive() export abstract class ViewDirective implements OnChanges { @@ -12,7 +12,7 @@ export abstract class ViewDirective implements OnChanges { /** indicating that the value is not in its original state, so that components can explain this to the user */ isPartiallyAnonymized: boolean; - ngOnChanges() { + ngOnChanges(changes?: SimpleChanges) { this.isPartiallyAnonymized = this.entity?.anonymized && this.entity?.getSchema()?.get(this.id)?.anonymize === "retain-anonymized"; diff --git a/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.html b/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.html index 874d62e7a6..430ae0a951 100644 --- a/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.html +++ b/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.html @@ -4,6 +4,6 @@ completed
- by {{ value.completedBy }} on {{ value.completedAt | date }} + by {{ completedBy.toString() }} on {{ value.completedAt | date }}
diff --git a/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.ts b/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.ts index 85d61f1294..ab6578f49b 100644 --- a/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.ts +++ b/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.ts @@ -1,9 +1,13 @@ -import { Component } from "@angular/core"; +import { Component, OnChanges, SimpleChanges } from "@angular/core"; import { DynamicComponent } from "../../../../core/config/dynamic-components/dynamic-component.decorator"; import { ViewDirective } from "../../../../core/entity/default-datatype/view.directive"; import { TodoCompletion } from "../../model/todo-completion"; import { DatePipe, NgIf } from "@angular/common"; import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; +import { EntityMapperService } from "../../../../core/entity/entity-mapper/entity-mapper.service"; +import { Entity } from "../../../../core/entity/model/entity"; +import { User } from "../../../../core/user/user"; +import { DisplayEntityComponent } from "../../../../core/basic-datatypes/entity/display-entity/display-entity.component"; @DynamicComponent("DisplayTodoCompletion") @Component({ @@ -11,6 +15,27 @@ import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; templateUrl: "./display-todo-completion.component.html", styleUrls: ["./display-todo-completion.component.scss"], standalone: true, - imports: [NgIf, FontAwesomeModule, DatePipe], + imports: [NgIf, FontAwesomeModule, DatePipe, DisplayEntityComponent], }) -export class DisplayTodoCompletionComponent extends ViewDirective {} +export class DisplayTodoCompletionComponent + extends ViewDirective + implements OnChanges +{ + completedBy: Entity; + constructor(private entityMapper: EntityMapperService) { + super(); + } + + ngOnChanges(changes: SimpleChanges) { + super.ngOnChanges(changes); + if (changes.hasOwnProperty("value") && this.value.completedBy) { + const entityId = this.value.completedBy; + const entityType = entityId.includes(":") + ? Entity.extractTypeFromId(entityId) + : User; + this.entityMapper + .load(entityType, entityId) + .then((res) => (this.completedBy = res)); + } + } +} diff --git a/src/app/features/todos/todo.service.ts b/src/app/features/todos/todo.service.ts index 2752bb0fb9..608d26ad86 100644 --- a/src/app/features/todos/todo.service.ts +++ b/src/app/features/todos/todo.service.ts @@ -19,14 +19,12 @@ export class TodoService { const nextTodo = await this.createNextRepetition(todo); todo.completed = { - completedBy: this.currentUser.value.getId(), + completedBy: this.currentUser.value.getId(true), completedAt: new Date(), nextRepetition: nextTodo?.getId(true), }; await this.entityMapper.save(todo); - - // TODO: user block instead of id to display in template } private async createNextRepetition(originalTodo: Todo): Promise { From 4df0a8fc438a32ac8614a7912e324ab255c489a6 Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 22 Dec 2023 10:55:22 +0100 Subject: [PATCH 05/27] added test --- .../display-todo-completion.component.spec.ts | 47 ++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.spec.ts b/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.spec.ts index 7d4ed9f47b..cc1ddcd79f 100644 --- a/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.spec.ts +++ b/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.spec.ts @@ -1,14 +1,29 @@ -import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { + ComponentFixture, + fakeAsync, + TestBed, + tick, +} from "@angular/core/testing"; import { DisplayTodoCompletionComponent } from "./display-todo-completion.component"; +import { + mockEntityMapper, + MockEntityMapperService, +} from "../../../../core/entity/entity-mapper/mock-entity-mapper-service"; +import { EntityMapperService } from "../../../../core/entity/entity-mapper/entity-mapper.service"; +import { Child } from "../../../../child-dev-project/children/model/child"; +import { User } from "../../../../core/user/user"; describe("DisplayTodoCompletionComponent", () => { let component: DisplayTodoCompletionComponent; let fixture: ComponentFixture; + let entityMapper: MockEntityMapperService; beforeEach(async () => { + entityMapper = mockEntityMapper(); await TestBed.configureTestingModule({ imports: [DisplayTodoCompletionComponent], + providers: [{ provide: EntityMapperService, useValue: entityMapper }], }).compileComponents(); fixture = TestBed.createComponent(DisplayTodoCompletionComponent); @@ -19,4 +34,34 @@ describe("DisplayTodoCompletionComponent", () => { it("should create", () => { expect(component).toBeTruthy(); }); + + it("should load the entity in completedBy when it has full ID", fakeAsync(() => { + const completingChild = new Child("1"); + const otherChild = new Child("2"); + entityMapper.addAll([completingChild, otherChild]); + + component.value = { + completedBy: completingChild.getId(true), + completedAt: new Date(), + }; + component.ngOnChanges({ value: true } as any); + tick(); + + expect(component.completedBy).toEqual(completingChild); + })); + + it("should load the user entity when completedBy does not contain entity type", fakeAsync(() => { + const completingUser = new User("1"); + const otherUser = new User("2"); + entityMapper.addAll([completingUser, otherUser]); + + component.value = { + completedBy: completingUser.getId(), + completedAt: new Date(), + }; + component.ngOnChanges({ value: true } as any); + tick(); + + expect(component.completedBy).toEqual(completingUser); + })); }); From 9cf93131759ae4d2a5ab24139b73cf48efc3da4b Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 22 Dec 2023 14:13:55 +0100 Subject: [PATCH 06/27] entity select can handle foreign entity references --- .../entity-form/entity-form.service.spec.ts | 6 +- .../entity-form/entity-form.service.ts | 2 +- .../entity-select.component.spec.ts | 35 +++++++++-- .../entity-select/entity-select.component.ts | 59 ++++++++++++++----- 4 files changed, 78 insertions(+), 24 deletions(-) diff --git a/src/app/core/common-components/entity-form/entity-form.service.spec.ts b/src/app/core/common-components/entity-form/entity-form.service.spec.ts index 638bdb3ade..f0c1982edc 100644 --- a/src/app/core/common-components/entity-form/entity-form.service.spec.ts +++ b/src/app/core/common-components/entity-form/entity-form.service.spec.ts @@ -180,15 +180,15 @@ describe("EntityFormService", () => { schema.defaultValue = PLACEHOLDERS.CURRENT_USER; form = service.createFormGroup([{ id: "test" }], new Entity()); - expect(form.get("test")).toHaveValue(TEST_USER); + expect(form.get("test")).toHaveValue(`User:${TEST_USER}`); schema.dataType = ArrayDatatype.dataType; form = service.createFormGroup([{ id: "test" }], new Entity()); - expect(form.get("test")).toHaveValue([TEST_USER]); + expect(form.get("test")).toHaveValue([`User:${TEST_USER}`]); schema.dataType = EntityArrayDatatype.dataType; form = service.createFormGroup([{ id: "test" }], new Entity()); - expect(form.get("test")).toHaveValue([TEST_USER]); + expect(form.get("test")).toHaveValue([`User:${TEST_USER}`]); Entity.schema.delete("test"); }); diff --git a/src/app/core/common-components/entity-form/entity-form.service.ts b/src/app/core/common-components/entity-form/entity-form.service.ts index 9a8f536b15..5135a50e9a 100644 --- a/src/app/core/common-components/entity-form/entity-form.service.ts +++ b/src/app/core/common-components/entity-form/entity-form.service.ts @@ -188,7 +188,7 @@ export class EntityFormService { newVal = new Date(); break; case PLACEHOLDERS.CURRENT_USER: - newVal = this.currentUser.value.getId(); + newVal = this.currentUser.value.getId(true); break; default: newVal = schema.defaultValue; diff --git a/src/app/core/common-components/entity-select/entity-select.component.spec.ts b/src/app/core/common-components/entity-select/entity-select.component.spec.ts index b474f8034c..c45b4e1355 100644 --- a/src/app/core/common-components/entity-select/entity-select.component.spec.ts +++ b/src/app/core/common-components/entity-select/entity-select.component.spec.ts @@ -12,6 +12,7 @@ import { Child } from "../../../child-dev-project/children/model/child"; import { School } from "../../../child-dev-project/schools/model/school"; import { MockedTestingModule } from "../../../utils/mocked-testing.module"; import { LoginState } from "../../session/session-states/login-state.enum"; +import { LoggingService } from "../../logging/logging.service"; describe("EntitySelectComponent", () => { let component: EntitySelectComponent; @@ -139,12 +140,10 @@ describe("EntitySelectComponent", () => { component.unselectEntity(testUsers[0]); - const remainingChildren = testUsers - .filter((c) => c.getId() !== testUsers[0].getId()) - .map((c) => c.getId()); - expect(component.selectionChange.emit).toHaveBeenCalledWith( - remainingChildren, - ); + const remainingUsers = testUsers + .filter((u) => u.getId() !== testUsers[0].getId()) + .map((u) => u.getId(true)); + expect(component.selectionChange.emit).toHaveBeenCalledWith(remainingUsers); }); it("adds a new entity if it matches a known entity", fakeAsync(() => { @@ -224,6 +223,30 @@ describe("EntitySelectComponent", () => { expect(component.filteredEntities).toEqual([...testUsers, ...testChildren]); })); + it("should show selected entities of type that is not configured", fakeAsync(() => { + component.entityType = [User.ENTITY_TYPE]; + component.selection = [testUsers[0].getId(), testChildren[0].getId(true)]; + tick(); + fixture.detectChanges(); + expect(component.selectedEntities).toEqual([testUsers[0], testChildren[0]]); + expect(component.allEntities).toEqual(testUsers); + expect(component.filteredEntities).toEqual( + jasmine.arrayWithExactContents(testUsers.slice(1)), + ); + })); + + it("should not fail if entity cannot be found", fakeAsync(() => { + const warnSpy = spyOn(TestBed.inject(LoggingService), "warn"); + component.entityType = User.ENTITY_TYPE; + component.selection = [testUsers[0].getId(), "missing_user"]; + tick(); + fixture.detectChanges(); + expect(warnSpy).toHaveBeenCalledWith( + jasmine.stringContaining("missing_user"), + ); + expect(component.selectedEntities).toEqual([testUsers[0]]); + })); + it("should be able to select entities from different types", fakeAsync(() => { component.entityType = [User.ENTITY_TYPE, Child.ENTITY_TYPE]; component.selection = [ diff --git a/src/app/core/common-components/entity-select/entity-select.component.ts b/src/app/core/common-components/entity-select/entity-select.component.ts index eb9b33b874..2cbdc187e4 100644 --- a/src/app/core/common-components/entity-select/entity-select.component.ts +++ b/src/app/core/common-components/entity-select/entity-select.component.ts @@ -26,6 +26,7 @@ import { DisplayEntityComponent } from "../../basic-datatypes/entity/display-ent import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; import { MatTooltipModule } from "@angular/material/tooltip"; import { MatInputModule } from "@angular/material/input"; +import { LoggingService } from "../../logging/logging.service"; @Component({ selector: "app-entity-select", @@ -70,9 +71,12 @@ export class EntitySelectComponent implements OnChanges { } else { type = [type]; } - this.loadAvailableEntities(type); + this._entityType = type; + this.loadAvailableEntities(); } + private _entityType: string[] = []; + /** * The (initial) selection. Can be used in combination with {@link selectionChange} * to enable two-way binding to an array of strings corresponding to the id's of the entities. @@ -88,11 +92,33 @@ export class EntitySelectComponent implements OnChanges { untilDestroyed(this), filter((isLoading) => !isLoading), ) - .subscribe((_) => { - this.selectedEntities = this.allEntities.filter((e) => - sel.find((s) => s === e.getId(true) || s === e.getId()), - ); - }); + .subscribe(() => this.initSelectedEntities(sel)); + } + + private async initSelectedEntities(selected: string[]) { + const entities: E[] = []; + for (const s of selected) { + const found = this.allEntities.find( + (e) => s === e.getId(true) || s === e.getId(), + ); + if (found) { + entities.push(found); + } else { + // missing or entity from other type + try { + const type = Entity.extractTypeFromId(s); + const entity = await this.entityMapperService.load(type, s); + entities.push(entity); + } catch (e) { + this.logger.warn( + `[ENTITY_SELECT] Could not find entity with ID: ${s}: ${e}`, + ); + } + } + } + this.selectedEntities = entities; + // updating autocomplete values + this.formControl.setValue(this.formControl.value); } /** Underlying data-array */ @@ -162,16 +188,17 @@ export class EntitySelectComponent implements OnChanges { @ViewChild("inputField") inputField: ElementRef; @ViewChild(MatAutocompleteTrigger) autocomplete: MatAutocompleteTrigger; - constructor(private entityMapperService: EntityMapperService) { + constructor( + private entityMapperService: EntityMapperService, + private logger: LoggingService, + ) { this.formControl.valueChanges .pipe( untilDestroyed(this), filter((value) => value === null || typeof value === "string"), // sometimes produces entities map((searchText?: string) => this.filter(searchText)), ) - .subscribe((value) => { - this.filteredEntities = value; - }); + .subscribe((value) => (this.filteredEntities = value)); this.loading.pipe(untilDestroyed(this)).subscribe((isLoading) => { this.inputPlaceholder = isLoading ? this.loadingPlaceholder @@ -196,11 +223,11 @@ export class EntitySelectComponent implements OnChanges { @Input() additionalFilter: (e: E) => boolean = (_) => true; - private async loadAvailableEntities(types: string[]) { + private async loadAvailableEntities() { this.loading.next(true); const entities: E[] = []; - for (const type of types) { + for (const type of this._entityType) { entities.push(...(await this.entityMapperService.loadType(type))); } @@ -278,9 +305,13 @@ export class EntitySelectComponent implements OnChanges { } private emitChange() { - this.selectionChange.emit( - this.selectedEntities.map((e) => e.getId(this.withPrefix)), + // entities that do not belong to the possible types should always have full ID + const selectedIds = this.selectedEntities.map((e) => + this._entityType.includes(e.getType()) + ? e.getId(this.withPrefix) + : e.getId(true), ); + this.selectionChange.emit(selectedIds); } private isSelected(entity: E): boolean { From d50d44aa2f554e323904f4b77ec12e30b814a356 Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 22 Dec 2023 14:26:00 +0100 Subject: [PATCH 07/27] fix display entity array to show entities again --- .../display-entity-array.component.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.ts b/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.ts index a2d5589ae7..4fe0f7474d 100644 --- a/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.ts +++ b/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.ts @@ -1,4 +1,4 @@ -import { Component, OnInit } from "@angular/core"; +import { ChangeDetectorRef, Component, OnInit } from "@angular/core"; import { Entity } from "../../../entity/model/entity"; import { EntityMapperService } from "../../../entity/entity-mapper/entity-mapper.service"; import { ViewDirective } from "../../../entity/default-datatype/view.directive"; @@ -22,7 +22,10 @@ export class DisplayEntityArrayComponent entities: Entity[]; - constructor(private entityMapper: EntityMapperService) { + constructor( + private entityMapper: EntityMapperService, + private changeDetector: ChangeDetectorRef, + ) { super(); } @@ -37,6 +40,7 @@ export class DisplayEntityArrayComponent return this.entityMapper.load(type, entityId); }); this.entities = await Promise.all(entityPromises); + this.changeDetector.detectChanges(); } } } From 15ad1abecb105d0087a77e80c5a1511d052822b8 Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 22 Dec 2023 14:26:38 +0100 Subject: [PATCH 08/27] display entity array shows foreign entities --- .../display-entity-array.component.spec.ts | 12 ++++++++++++ .../display-entity-array.component.ts | 7 +++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.spec.ts b/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.spec.ts index 538db5c04e..bcb2de3334 100644 --- a/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.spec.ts +++ b/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.spec.ts @@ -94,4 +94,16 @@ describe("DisplayEntityArrayComponent", () => { expect(component.entities).toEqual(expectedEntities); }); + + it("should load entities of not configured type", async () => { + component.config = School.ENTITY_TYPE; + const existingChild = testEntities.find( + (e) => e.getType() === Child.ENTITY_TYPE, + ); + component.value = [existingChild.getId(true)]; + + await component.ngOnInit(); + + expect(component.entities).toEqual([existingChild]); + }); }); diff --git a/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.ts b/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.ts index 4fe0f7474d..c6d19afe1a 100644 --- a/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.ts +++ b/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.ts @@ -33,10 +33,9 @@ export class DisplayEntityArrayComponent const entityIds: string[] = this.value || []; if (entityIds.length < this.aggregationThreshold) { const entityPromises = entityIds.map((entityId) => { - const type = - typeof this.config === "string" - ? this.config - : Entity.extractTypeFromId(entityId); + const type = entityId.includes(":") + ? Entity.extractTypeFromId(entityId) + : this.config; return this.entityMapper.load(type, entityId); }); this.entities = await Promise.all(entityPromises); From 079ab70f57c9f27e6acd165e2eefb75599f3b281 Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 22 Dec 2023 17:25:23 +0100 Subject: [PATCH 09/27] display single entity allows foreign entities to be displayed --- .../edit-single-entity.component.spec.ts | 40 ++++++++++++++++--- .../edit-single-entity.component.ts | 31 +++++++++++--- .../basic-autocomplete.component.ts | 1 - 3 files changed, 61 insertions(+), 11 deletions(-) diff --git a/src/app/core/basic-datatypes/entity/edit-single-entity/edit-single-entity.component.spec.ts b/src/app/core/basic-datatypes/entity/edit-single-entity/edit-single-entity.component.spec.ts index 13f3f4673c..7311bf0111 100644 --- a/src/app/core/basic-datatypes/entity/edit-single-entity/edit-single-entity.component.spec.ts +++ b/src/app/core/basic-datatypes/entity/edit-single-entity/edit-single-entity.component.spec.ts @@ -7,18 +7,20 @@ import { ChildSchoolRelation } from "../../../../child-dev-project/children/mode import { School } from "../../../../child-dev-project/schools/model/school"; import { MockedTestingModule } from "../../../../utils/mocked-testing.module"; import { FormControl } from "@angular/forms"; +import { Child } from "../../../../child-dev-project/children/model/child"; +import { LoggingService } from "../../../logging/logging.service"; describe("EditSingleEntityComponent", () => { let component: EditSingleEntityComponent; let fixture: ComponentFixture; - let loadTypeSpy: jasmine.Spy; + let entityMapper: EntityMapperService; beforeEach(waitForAsync(() => { TestBed.configureTestingModule({ imports: [EditSingleEntityComponent, MockedTestingModule.withState()], providers: [EntityFormService], }).compileComponents(); - loadTypeSpy = spyOn(TestBed.inject(EntityMapperService), "loadType"); + entityMapper = TestBed.inject(EntityMapperService); })); beforeEach(() => { @@ -45,11 +47,39 @@ describe("EditSingleEntityComponent", () => { it("should load all entities of the given type as options", async () => { const school1 = School.create({ name: "First School" }); const school2 = School.create({ name: "Second School " }); - loadTypeSpy.and.resolveTo([school1, school2]); + await entityMapper.saveAll([school1, school2]); + component.formFieldConfig.additional = School.ENTITY_TYPE; await component.ngOnInit(); - expect(loadTypeSpy).toHaveBeenCalled(); - expect(component.entities).toEqual([school1, school2]); + expect(component.entities).toEqual( + jasmine.arrayWithExactContents([school1, school2]), + ); + }); + + it("should add selected entity of a not-configured type to available entities", async () => { + const someSchools = [new School(), new School()]; + const selectedChild = new Child(); + await entityMapper.saveAll(someSchools.concat(selectedChild)); + component.formFieldConfig.additional = School.ENTITY_TYPE; + component.formControl.setValue(selectedChild.getId(true)); + + await component.ngOnInit(); + + expect(component.entities).toEqual( + jasmine.arrayWithExactContents(someSchools.concat(selectedChild)), + ); + }); + + it("should log warning if entity is selected that cannot be found", async () => { + const warnSpy = spyOn(TestBed.inject(LoggingService), "warn"); + component.formFieldConfig.additional = Child.ENTITY_TYPE; + component.formControl.setValue("missing_child"); + + await component.ngOnInit(); + + expect(warnSpy).toHaveBeenCalledWith( + jasmine.stringContaining("missing_child"), + ); }); }); diff --git a/src/app/core/basic-datatypes/entity/edit-single-entity/edit-single-entity.component.ts b/src/app/core/basic-datatypes/entity/edit-single-entity/edit-single-entity.component.ts index a4c3bed2bc..95123a51d9 100644 --- a/src/app/core/basic-datatypes/entity/edit-single-entity/edit-single-entity.component.ts +++ b/src/app/core/basic-datatypes/entity/edit-single-entity/edit-single-entity.component.ts @@ -10,6 +10,7 @@ import { MatFormFieldModule } from "@angular/material/form-field"; import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; import { NgIf } from "@angular/common"; import { ErrorHintComponent } from "../../../common-components/error-hint/error-hint.component"; +import { LoggingService } from "../../../logging/logging.service"; @DynamicComponent("EditSingleEntity") @Component({ @@ -32,17 +33,37 @@ export class EditSingleEntityComponent implements OnInit { entities: Entity[] = []; - entityToId = (e: Entity) => e?.getId(); + entityToId = (e: Entity) => e?.getId(true); - constructor(private entityMapperService: EntityMapperService) { + constructor( + private entityMapper: EntityMapperService, + private logger: LoggingService, + ) { super(); } async ngOnInit() { super.ngOnInit(); - this.entities = await this.entityMapperService.loadType( - this.formFieldConfig.additional, + const availableEntities = await this.entityMapper.loadType(this.additional); + const selected = this.formControl.value; + if ( + selected && + !availableEntities.some( + (e) => e.getId(true) === selected || e.getId() === selected, + ) + ) { + try { + const type = Entity.extractTypeFromId(selected); + const entity = await this.entityMapper.load(type, selected); + availableEntities.push(entity); + } catch (e) { + this.logger.warn( + `[EDIT_SINGLE_ENTITY] Could not find entity with ID: ${selected}: ${e}`, + ); + } + } + this.entities = availableEntities.sort((e1, e2) => + e1.toString().localeCompare(e2.toString()), ); - this.entities.sort((e1, e2) => e1.toString().localeCompare(e2.toString())); } } diff --git a/src/app/core/common-components/basic-autocomplete/basic-autocomplete.component.ts b/src/app/core/common-components/basic-autocomplete/basic-autocomplete.component.ts index 9dddf24bca..44c2b11594 100644 --- a/src/app/core/common-components/basic-autocomplete/basic-autocomplete.component.ts +++ b/src/app/core/common-components/basic-autocomplete/basic-autocomplete.component.ts @@ -44,7 +44,6 @@ interface SelectableOption { selected: boolean; } -/** Custom `MatFormFieldControl` for telephone number input. */ @Component({ selector: "app-basic-autocomplete", templateUrl: "basic-autocomplete.component.html", From a830f9c1db14d967100732a3d6e9605c4ad04c5d Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 22 Dec 2023 17:38:16 +0100 Subject: [PATCH 10/27] improved error message in MockEntityMapper --- .../entity-mapper/mock-entity-mapper-service.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/app/core/entity/entity-mapper/mock-entity-mapper-service.ts b/src/app/core/entity/entity-mapper/mock-entity-mapper-service.ts index 3ff737d5dd..5a586b7367 100644 --- a/src/app/core/entity/entity-mapper/mock-entity-mapper-service.ts +++ b/src/app/core/entity/entity-mapper/mock-entity-mapper-service.ts @@ -84,7 +84,11 @@ export class MockEntityMapperService extends EntityMapperService { const entityId = Entity.extractEntityIdFromId(id); const result = this.data.get(entityType)?.get(entityId); if (!result) { - throw new HttpErrorResponse({ status: 404 }); + throw new HttpErrorResponse({ + url: "MockEntityMapperService", + status: 404, + statusText: `${entityType}:${entityId} not found`, + }); } return result; } @@ -115,12 +119,7 @@ export class MockEntityMapperService extends EntityMapperService { ): Promise { const ctor = this.resolveConstructor(entityType); const type = new ctor().getType(); - const entity = this.get(type, id) as T; - if (!entity) { - throw Error(`Entity ${id} does not exist in MockEntityMapper`); - } else { - return entity; - } + return this.get(type, id) as T; } async loadType( From 79506e44a8f6cb1830b284b775b0fbb8eebde77f Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 22 Dec 2023 17:45:36 +0100 Subject: [PATCH 11/27] added support for foreign entities in DisplayEntityComponent --- .../display-entity.component.spec.ts | 43 +++++++++++++++---- .../display-entity.component.ts | 22 +++++++--- 2 files changed, 50 insertions(+), 15 deletions(-) diff --git a/src/app/core/basic-datatypes/entity/display-entity/display-entity.component.spec.ts b/src/app/core/basic-datatypes/entity/display-entity/display-entity.component.spec.ts index 52c11757fc..72464e1baf 100644 --- a/src/app/core/basic-datatypes/entity/display-entity/display-entity.component.spec.ts +++ b/src/app/core/basic-datatypes/entity/display-entity/display-entity.component.spec.ts @@ -14,21 +14,25 @@ import { componentRegistry, ComponentRegistry, } from "../../../../dynamic-components"; +import { + mockEntityMapper, + MockEntityMapperService, +} from "../../../entity/entity-mapper/mock-entity-mapper-service"; +import { LoggingService } from "../../../logging/logging.service"; describe("DisplayEntityComponent", () => { let component: DisplayEntityComponent; let fixture: ComponentFixture; - let mockEntityMapper: jasmine.SpyObj; + let entityMapper: MockEntityMapperService; let mockRouter: jasmine.SpyObj; beforeEach(async () => { - mockEntityMapper = jasmine.createSpyObj(["load"]); - mockEntityMapper.load.and.resolveTo(new Child()); + entityMapper = mockEntityMapper(); mockRouter = jasmine.createSpyObj(["navigate"]); await TestBed.configureTestingModule({ imports: [DisplayEntityComponent], providers: [ - { provide: EntityMapperService, useValue: mockEntityMapper }, + { provide: EntityMapperService, useValue: entityMapper }, { provide: EntityRegistry, useValue: entityRegistry }, { provide: ComponentRegistry, useValue: componentRegistry }, { provide: Router, useValue: mockRouter }, @@ -48,7 +52,7 @@ describe("DisplayEntityComponent", () => { it("should use the block component when available", async () => { const school = new School(); - mockEntityMapper.load.and.resolveTo(school); + entityMapper.add(school); component.entity = new ChildSchoolRelation(); component.id = "schoolId"; @@ -57,10 +61,6 @@ describe("DisplayEntityComponent", () => { await component.ngOnInit(); expect(component.entityBlockComponent).toEqual(School.getBlockComponent()); - expect(mockEntityMapper.load).toHaveBeenCalledWith( - school.getType(), - school.getId(), - ); expect(component.entityToDisplay).toEqual(school); }); @@ -71,4 +71,29 @@ describe("DisplayEntityComponent", () => { expect(mockRouter.navigate).toHaveBeenCalledWith(["/child", "1"]); }); + + it("should show entities which are not of the configured type", async () => { + const child = new Child(); + entityMapper.add(child); + component.entityId = child.getId(true); + component.config = School.ENTITY_TYPE; + + await component.ngOnInit(); + + expect(component.entityToDisplay).toEqual(child); + }); + + it("should log a warning if entity cannot be loaded", async () => { + const warnSpy = spyOn(TestBed.inject(LoggingService), "warn"); + const child = new Child("not_existing"); + component.entityId = child.getId(true); + component.config = School.ENTITY_TYPE; + + await component.ngOnInit(); + + expect(warnSpy).toHaveBeenCalledWith( + jasmine.stringContaining(child.getId(true)), + ); + expect(component.entityToDisplay).toBeUndefined(); + }); }); diff --git a/src/app/core/basic-datatypes/entity/display-entity/display-entity.component.ts b/src/app/core/basic-datatypes/entity/display-entity/display-entity.component.ts index 97eb38bc97..dee6b02deb 100644 --- a/src/app/core/basic-datatypes/entity/display-entity/display-entity.component.ts +++ b/src/app/core/basic-datatypes/entity/display-entity/display-entity.component.ts @@ -6,6 +6,7 @@ import { EntityMapperService } from "../../../entity/entity-mapper/entity-mapper import { Router } from "@angular/router"; import { NgClass, NgIf } from "@angular/common"; import { DynamicComponentDirective } from "../../../config/dynamic-components/dynamic-component.directive"; +import { LoggingService } from "../../../logging/logging.service"; @DynamicComponent("DisplayEntity") @Component({ @@ -35,6 +36,7 @@ export class DisplayEntityComponent constructor( private entityMapper: EntityMapperService, private router: Router, + private logger: LoggingService, private changeDetector: ChangeDetectorRef, ) { super(); @@ -42,22 +44,30 @@ export class DisplayEntityComponent async ngOnInit() { if (!this.entityToDisplay) { - this.entityType = this.entityType ?? this.config; this.entityId = this.entityId ?? this.value; + this.entityType = this.entityId.includes(":") + ? Entity.extractTypeFromId(this.entityId) + : this.entityType ?? this.config; if (!this.entityType || !this.entityId) { return; } - this.entityToDisplay = await this.entityMapper.load( - this.entityType, - this.entityId, - ); - this.changeDetector.detectChanges(); + try { + this.entityToDisplay = await this.entityMapper.load( + this.entityType, + this.entityId, + ); + } catch (e) { + this.logger.warn( + `[DISPLAY_ENTITY] Could not find entity with ID: ${this.entityId}: ${e}`, + ); + } } if (this.entityToDisplay) { this.entityBlockComponent = this.entityToDisplay .getConstructor() .getBlockComponent(); } + this.changeDetector.detectChanges(); } showDetailsPage() { From 9e1583074c696cd962bff014a36d6e57cf371f83 Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 22 Dec 2023 18:05:55 +0100 Subject: [PATCH 12/27] use local storage to save paginator settings --- .../list-paginator.component.spec.ts | 50 +++++++------------ .../list-paginator.component.ts | 47 ++++++----------- 2 files changed, 32 insertions(+), 65 deletions(-) diff --git a/src/app/core/common-components/entity-subrecord/list-paginator/list-paginator.component.spec.ts b/src/app/core/common-components/entity-subrecord/list-paginator/list-paginator.component.spec.ts index 8e5203f631..f126fd971c 100644 --- a/src/app/core/common-components/entity-subrecord/list-paginator/list-paginator.component.spec.ts +++ b/src/app/core/common-components/entity-subrecord/list-paginator/list-paginator.component.spec.ts @@ -1,17 +1,9 @@ -import { - ComponentFixture, - fakeAsync, - TestBed, - tick, - waitForAsync, -} from "@angular/core/testing"; +import { ComponentFixture, TestBed, waitForAsync } from "@angular/core/testing"; import { ListPaginatorComponent } from "./list-paginator.component"; import { MatTableDataSource } from "@angular/material/table"; import { PageEvent } from "@angular/material/paginator"; -import { MockedTestingModule } from "../../../../utils/mocked-testing.module"; -import { EntityMapperService } from "../../../entity/entity-mapper/entity-mapper.service"; -import { User } from "../../../user/user"; +import { NoopAnimationsModule } from "@angular/platform-browser/animations"; describe("ListPaginatorComponent", () => { let component: ListPaginatorComponent; @@ -19,7 +11,7 @@ describe("ListPaginatorComponent", () => { beforeEach(waitForAsync(() => { TestBed.configureTestingModule({ - imports: [ListPaginatorComponent, MockedTestingModule.withState()], + imports: [ListPaginatorComponent, NoopAnimationsModule], }).compileComponents(); })); @@ -30,44 +22,38 @@ describe("ListPaginatorComponent", () => { fixture.detectChanges(); }); + afterEach(() => localStorage.clear()); + it("should create", () => { expect(component).toBeTruthy(); }); - it("should save pagination settings in the user entity", fakeAsync(() => { + it("should save pagination settings in the local storage", () => { component.idForSavingPagination = "table-id"; - const saveEntitySpy = spyOn(TestBed.inject(EntityMapperService), "save"); component.onPaginateChange({ pageSize: 20, pageIndex: 1 } as PageEvent); - tick(); - expect(saveEntitySpy).toHaveBeenCalledWith(component.user); - expect(component.user.paginatorSettingsPageSize["table-id"]).toEqual(20); - })); + expect( + localStorage.getItem(component.LOCAL_STORAGE_KEY + "table-id"), + ).toEqual("20"); + }); - it("should update pagination when the idForSavingPagination changed", fakeAsync(() => { - const userPaginationSettings = { - c1: 11, - c2: 12, - }; - component.user = { - paginatorSettingsPageSize: userPaginationSettings, - } as Partial as User; + it("should update pagination when the idForSavingPagination changed", () => { + localStorage.setItem(component.LOCAL_STORAGE_KEY + "c1", "11"); + localStorage.setItem(component.LOCAL_STORAGE_KEY + "c2", "12"); component.idForSavingPagination = "c1"; component.ngOnChanges({ idForSavingPagination: undefined }); - tick(); fixture.detectChanges(); - expect(component.pageSize).toBe(userPaginationSettings.c1); - expect(component.paginator.pageSize).toBe(userPaginationSettings.c1); + expect(component.pageSize).toBe(11); + expect(component.paginator.pageSize).toBe(11); component.idForSavingPagination = "c2"; component.ngOnChanges({ idForSavingPagination: undefined }); - tick(); fixture.detectChanges(); - expect(component.pageSize).toBe(userPaginationSettings.c2); - expect(component.paginator.pageSize).toBe(userPaginationSettings.c2); - })); + expect(component.pageSize).toBe(12); + expect(component.paginator.pageSize).toBe(12); + }); }); diff --git a/src/app/core/common-components/entity-subrecord/list-paginator/list-paginator.component.ts b/src/app/core/common-components/entity-subrecord/list-paginator/list-paginator.component.ts index e674843a9a..4c5cb91171 100644 --- a/src/app/core/common-components/entity-subrecord/list-paginator/list-paginator.component.ts +++ b/src/app/core/common-components/entity-subrecord/list-paginator/list-paginator.component.ts @@ -12,9 +12,6 @@ import { PageEvent, } from "@angular/material/paginator"; import { MatTableDataSource } from "@angular/material/table"; -import { User } from "../../../user/user"; -import { EntityMapperService } from "../../../entity/entity-mapper/entity-mapper.service"; -import { CurrentUserSubject } from "../../../session/current-user-subject"; @Component({ selector: "app-list-paginator", @@ -24,6 +21,7 @@ import { CurrentUserSubject } from "../../../session/current-user-subject"; standalone: true, }) export class ListPaginatorComponent implements OnChanges, OnInit { + readonly LOCAL_STORAGE_KEY = "PAGINATION-"; readonly pageSizeOptions = [10, 20, 50, 100]; @Input() dataSource: MatTableDataSource; @@ -31,16 +29,8 @@ export class ListPaginatorComponent implements OnChanges, OnInit { @ViewChild(MatPaginator, { static: true }) paginator: MatPaginator; - user: User; pageSize = 10; - constructor( - currentUser: CurrentUserSubject, - private entityMapperService: EntityMapperService, - ) { - currentUser.subscribe((val: User) => (this.user = val)); - } - ngOnChanges(changes: SimpleChanges): void { if (changes.hasOwnProperty("idForSavingPagination")) { this.applyUserPaginationSettings(); @@ -53,33 +43,24 @@ export class ListPaginatorComponent implements OnChanges, OnInit { onPaginateChange(event: PageEvent) { this.pageSize = event.pageSize; - this.updateUserPaginationSettings(); + this.savePageSize(this.pageSize); } - private async applyUserPaginationSettings() { - if (!this.user) { - return; - } - - const savedSize = - this.user?.paginatorSettingsPageSize[this.idForSavingPagination]; + private applyUserPaginationSettings() { + const savedSize = this.getSavedPageSize(); this.pageSize = savedSize && savedSize !== -1 ? savedSize : this.pageSize; } - private async updateUserPaginationSettings() { - if (!this.user) { - return; - } - // The page size is stored in the database, the page index is only in memory - const hasChangesToBeSaved = - this.pageSize !== - this.user.paginatorSettingsPageSize[this.idForSavingPagination]; - - this.user.paginatorSettingsPageSize[this.idForSavingPagination] = - this.pageSize; + private getSavedPageSize(): number { + return Number.parseInt( + localStorage.getItem(this.LOCAL_STORAGE_KEY + this.idForSavingPagination), + ); + } - if (hasChangesToBeSaved) { - await this.entityMapperService.save(this.user); - } + private savePageSize(size: number) { + localStorage.setItem( + this.LOCAL_STORAGE_KEY + this.idForSavingPagination, + size?.toString(), + ); } } From 5f93ef1a8bd6dd7c9abaaed9cccfda72b32d86df Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 5 Feb 2024 17:44:05 +0100 Subject: [PATCH 13/27] fixed tests --- .../display-entity-array.component.spec.ts | 2 +- .../display-entity.component.spec.ts | 2 +- .../edit-single-entity.component.spec.ts | 2 +- .../entity-form/entity-form.service.ts | 2 +- .../entity-select/entity-select.component.spec.ts | 14 +++++++++----- .../display-todo-completion.component.spec.ts | 8 ++++---- 6 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.spec.ts b/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.spec.ts index 5840a0b46d..790f63193a 100644 --- a/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.spec.ts +++ b/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.spec.ts @@ -105,7 +105,7 @@ describe("DisplayEntityArrayComponent", () => { const existingChild = testEntities.find( (e) => e.getType() === Child.ENTITY_TYPE, ); - component.value = [existingChild.getId(true)]; + component.value = [existingChild.getId()]; await component.ngOnInit(); diff --git a/src/app/core/basic-datatypes/entity/display-entity/display-entity.component.spec.ts b/src/app/core/basic-datatypes/entity/display-entity/display-entity.component.spec.ts index 72464e1baf..ea5836246a 100644 --- a/src/app/core/basic-datatypes/entity/display-entity/display-entity.component.spec.ts +++ b/src/app/core/basic-datatypes/entity/display-entity/display-entity.component.spec.ts @@ -75,7 +75,7 @@ describe("DisplayEntityComponent", () => { it("should show entities which are not of the configured type", async () => { const child = new Child(); entityMapper.add(child); - component.entityId = child.getId(true); + component.entityId = child.getId(); component.config = School.ENTITY_TYPE; await component.ngOnInit(); diff --git a/src/app/core/basic-datatypes/entity/edit-single-entity/edit-single-entity.component.spec.ts b/src/app/core/basic-datatypes/entity/edit-single-entity/edit-single-entity.component.spec.ts index 7311bf0111..2ed74707e4 100644 --- a/src/app/core/basic-datatypes/entity/edit-single-entity/edit-single-entity.component.spec.ts +++ b/src/app/core/basic-datatypes/entity/edit-single-entity/edit-single-entity.component.spec.ts @@ -62,7 +62,7 @@ describe("EditSingleEntityComponent", () => { const selectedChild = new Child(); await entityMapper.saveAll(someSchools.concat(selectedChild)); component.formFieldConfig.additional = School.ENTITY_TYPE; - component.formControl.setValue(selectedChild.getId(true)); + component.formControl.setValue(selectedChild.getId()); await component.ngOnInit(); diff --git a/src/app/core/common-components/entity-form/entity-form.service.ts b/src/app/core/common-components/entity-form/entity-form.service.ts index 87d9865377..29e744aa17 100644 --- a/src/app/core/common-components/entity-form/entity-form.service.ts +++ b/src/app/core/common-components/entity-form/entity-form.service.ts @@ -201,7 +201,7 @@ export class EntityFormService { newVal = new Date(); break; case PLACEHOLDERS.CURRENT_USER: - newVal = this.currentUser.value.getId(true); + newVal = this.currentUser.value.getId(); break; default: newVal = schema.defaultValue; diff --git a/src/app/core/common-components/entity-select/entity-select.component.spec.ts b/src/app/core/common-components/entity-select/entity-select.component.spec.ts index e9462ff68e..65aad71a97 100644 --- a/src/app/core/common-components/entity-select/entity-select.component.spec.ts +++ b/src/app/core/common-components/entity-select/entity-select.component.spec.ts @@ -134,8 +134,8 @@ describe("EntitySelectComponent", () => { component.unselectEntity(testUsers[0]); const remainingUsers = testUsers - .filter((u) => u.getId() !== testUsers[0].getId()) - .map((u) => u.getId(true)); + .map((u) => u.getId()) + .filter((id) => id !== testUsers[0].getId()); expect(component.selectionChange.emit).toHaveBeenCalledWith(remainingUsers); }); @@ -325,11 +325,15 @@ describe("EntitySelectComponent", () => { it("should show selected entities of type that is not configured", fakeAsync(() => { component.entityType = [User.ENTITY_TYPE]; - component.selection = [testUsers[0].getId(), testChildren[0].getId(true)]; + component.selection = [testUsers[0].getId(), testChildren[0].getId()]; tick(); fixture.detectChanges(); - expect(component.selectedEntities).toEqual([testUsers[0], testChildren[0]]); - expect(component.allEntities).toEqual(testUsers); + expect(component.selectedEntities).toEqual( + jasmine.arrayWithExactContents([testUsers[0], testChildren[0]]), + ); + expect(component.allEntities).toEqual( + jasmine.arrayWithExactContents(testUsers), + ); expect(component.filteredEntities).toEqual( jasmine.arrayWithExactContents(testUsers.slice(1)), ); diff --git a/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.spec.ts b/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.spec.ts index cc1ddcd79f..29d063e214 100644 --- a/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.spec.ts +++ b/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.spec.ts @@ -41,10 +41,10 @@ describe("DisplayTodoCompletionComponent", () => { entityMapper.addAll([completingChild, otherChild]); component.value = { - completedBy: completingChild.getId(true), + completedBy: completingChild.getId(), completedAt: new Date(), }; - component.ngOnChanges({ value: true } as any); + component.ngOnInit(); tick(); expect(component.completedBy).toEqual(completingChild); @@ -56,10 +56,10 @@ describe("DisplayTodoCompletionComponent", () => { entityMapper.addAll([completingUser, otherUser]); component.value = { - completedBy: completingUser.getId(), + completedBy: completingUser.getId(true), completedAt: new Date(), }; - component.ngOnChanges({ value: true } as any); + component.ngOnInit(); tick(); expect(component.completedBy).toEqual(completingUser); From 7282b5fee6ea7ec845a14f7a877826a1e819fbc4 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 5 Feb 2024 17:50:56 +0100 Subject: [PATCH 14/27] removed further usages of short id --- .../display-entity/display-entity.component.spec.ts | 4 ++-- .../edit-single-entity/edit-single-entity.component.ts | 9 ++------- .../entity-select/entity-select.component.ts | 4 +--- src/app/features/todos/todo.service.ts | 2 +- 4 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/app/core/basic-datatypes/entity/display-entity/display-entity.component.spec.ts b/src/app/core/basic-datatypes/entity/display-entity/display-entity.component.spec.ts index ea5836246a..4f88aa80c9 100644 --- a/src/app/core/basic-datatypes/entity/display-entity/display-entity.component.spec.ts +++ b/src/app/core/basic-datatypes/entity/display-entity/display-entity.component.spec.ts @@ -86,13 +86,13 @@ describe("DisplayEntityComponent", () => { it("should log a warning if entity cannot be loaded", async () => { const warnSpy = spyOn(TestBed.inject(LoggingService), "warn"); const child = new Child("not_existing"); - component.entityId = child.getId(true); + component.entityId = child.getId(); component.config = School.ENTITY_TYPE; await component.ngOnInit(); expect(warnSpy).toHaveBeenCalledWith( - jasmine.stringContaining(child.getId(true)), + jasmine.stringContaining(child.getId()), ); expect(component.entityToDisplay).toBeUndefined(); }); diff --git a/src/app/core/basic-datatypes/entity/edit-single-entity/edit-single-entity.component.ts b/src/app/core/basic-datatypes/entity/edit-single-entity/edit-single-entity.component.ts index 95123a51d9..3bae23c0ae 100644 --- a/src/app/core/basic-datatypes/entity/edit-single-entity/edit-single-entity.component.ts +++ b/src/app/core/basic-datatypes/entity/edit-single-entity/edit-single-entity.component.ts @@ -33,7 +33,7 @@ export class EditSingleEntityComponent implements OnInit { entities: Entity[] = []; - entityToId = (e: Entity) => e?.getId(true); + entityToId = (e: Entity) => e?.getId(); constructor( private entityMapper: EntityMapperService, @@ -46,12 +46,7 @@ export class EditSingleEntityComponent super.ngOnInit(); const availableEntities = await this.entityMapper.loadType(this.additional); const selected = this.formControl.value; - if ( - selected && - !availableEntities.some( - (e) => e.getId(true) === selected || e.getId() === selected, - ) - ) { + if (selected && !availableEntities.some((e) => e.getId() === selected)) { try { const type = Entity.extractTypeFromId(selected); const entity = await this.entityMapper.load(type, selected); diff --git a/src/app/core/common-components/entity-select/entity-select.component.ts b/src/app/core/common-components/entity-select/entity-select.component.ts index 24c4614fe2..508fee4434 100644 --- a/src/app/core/common-components/entity-select/entity-select.component.ts +++ b/src/app/core/common-components/entity-select/entity-select.component.ts @@ -87,9 +87,7 @@ export class EntitySelectComponent implements OnChanges { private async initSelectedEntities(selected: string[]) { const entities: E[] = []; for (const s of selected) { - const found = this.allEntities.find( - (e) => s === e.getId(true) || s === e.getId(), - ); + const found = this.allEntities.find((e) => s === e.getId()); if (found) { entities.push(found); } else { diff --git a/src/app/features/todos/todo.service.ts b/src/app/features/todos/todo.service.ts index d22327406c..2ab0806acc 100644 --- a/src/app/features/todos/todo.service.ts +++ b/src/app/features/todos/todo.service.ts @@ -19,7 +19,7 @@ export class TodoService { const nextTodo = await this.createNextRepetition(todo); todo.completed = { - completedBy: this.currentUser.value.getId(true), + completedBy: this.currentUser.value.getId(), completedAt: new Date(), nextRepetition: nextTodo?.getId(), }; From a437de6db42598f6bee7c325f9d35f55ac098a6e Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 6 Feb 2024 08:55:26 +0100 Subject: [PATCH 15/27] made user entity optional --- .../roll-call-setup/roll-call-setup.component.ts | 12 ++++++++---- .../entity-form/entity-form.service.spec.ts | 10 ++++++++++ .../entity-form/entity-form.service.ts | 2 +- src/app/features/todos/todo.service.ts | 2 +- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/app/child-dev-project/attendance/add-day-attendance/roll-call-setup/roll-call-setup.component.ts b/src/app/child-dev-project/attendance/add-day-attendance/roll-call-setup/roll-call-setup.component.ts index 6c654af05c..587bb96f18 100644 --- a/src/app/child-dev-project/attendance/add-day-attendance/roll-call-setup/roll-call-setup.component.ts +++ b/src/app/child-dev-project/attendance/add-day-attendance/roll-call-setup/roll-call-setup.component.ts @@ -106,7 +106,7 @@ export class RollCallSetupComponent implements OnInit { } else { // TODO implement a generic function that finds the property where a entity has relations to another entity type (e.g. `authors` for `Note` when looking for `User`) to allow dynamic checks this.visibleActivities = this.allActivities.filter((a) => - a.isAssignedTo(this.currentUser.value.getId()), + a.isAssignedTo(this.currentUser.value?.getId()), ); if (this.visibleActivities.length === 0) { this.visibleActivities = this.allActivities.filter( @@ -156,7 +156,9 @@ export class RollCallSetupComponent implements OnInit { activity, this.date, )) as NoteForActivitySetup; - event.authors = [this.currentUser.value.getId()]; + if (this.currentUser.value) { + event.authors = [this.currentUser.value.getId()]; + } event.isNewFromActivity = true; return event; } @@ -176,7 +178,7 @@ export class RollCallSetupComponent implements OnInit { score += 1; } - if (assignedUsers.includes(this.currentUser.value.getId())) { + if (assignedUsers.includes(this.currentUser.value?.getId())) { score += 2; } @@ -190,7 +192,9 @@ export class RollCallSetupComponent implements OnInit { createOneTimeEvent() { const newNote = Note.create(new Date()); - newNote.authors = [this.currentUser.value.getId()]; + if (this.currentUser.value) { + newNote.authors = [this.currentUser.value.getId()]; + } this.formDialog .openFormPopup(newNote, [], NoteDetailsComponent) diff --git a/src/app/core/common-components/entity-form/entity-form.service.spec.ts b/src/app/core/common-components/entity-form/entity-form.service.spec.ts index 05505e8316..ca7bfece11 100644 --- a/src/app/core/common-components/entity-form/entity-form.service.spec.ts +++ b/src/app/core/common-components/entity-form/entity-form.service.spec.ts @@ -27,6 +27,7 @@ import { EntitySchemaService } from "../../entity/schema/entity-schema.service"; import { FormFieldConfig } from "./FormConfig"; import { User } from "../../user/user"; import { TEST_USER } from "../../user/demo-user-generator.service"; +import { CurrentUserSubject } from "../../session/current-user-subject"; describe("EntityFormService", () => { let service: EntityFormService; @@ -264,6 +265,15 @@ describe("EntityFormService", () => { Entity.schema.delete("test"); }); + it("should current user default value should not fail if user entity does not exist", () => { + Entity.schema.set("user", { defaultValue: PLACEHOLDERS.CURRENT_USER }); + TestBed.inject(CurrentUserSubject).next(undefined); + + const form = service.createFormGroup([{ id: "user" }], new Entity()); + + expect(form.get("user")).toHaveValue(null); + }); + it("should not assign default values to existing entities", () => { Entity.schema.set("test", { defaultValue: 1 }); diff --git a/src/app/core/common-components/entity-form/entity-form.service.ts b/src/app/core/common-components/entity-form/entity-form.service.ts index 29e744aa17..e85caa3e75 100644 --- a/src/app/core/common-components/entity-form/entity-form.service.ts +++ b/src/app/core/common-components/entity-form/entity-form.service.ts @@ -201,7 +201,7 @@ export class EntityFormService { newVal = new Date(); break; case PLACEHOLDERS.CURRENT_USER: - newVal = this.currentUser.value.getId(); + newVal = this.currentUser.value?.getId(); break; default: newVal = schema.defaultValue; diff --git a/src/app/features/todos/todo.service.ts b/src/app/features/todos/todo.service.ts index 2ab0806acc..420c866449 100644 --- a/src/app/features/todos/todo.service.ts +++ b/src/app/features/todos/todo.service.ts @@ -19,7 +19,7 @@ export class TodoService { const nextTodo = await this.createNextRepetition(todo); todo.completed = { - completedBy: this.currentUser.value.getId(), + completedBy: this.currentUser.value?.getId(), completedAt: new Date(), nextRepetition: nextTodo?.getId(), }; From dd0343705fdf1850390e0167a5c22eb08c46da65 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 6 Feb 2024 12:53:35 +0100 Subject: [PATCH 16/27] made `username` claim optional --- .../entity-form/entity-form.service.spec.ts | 1 + .../demo-data-initializer.service.spec.ts | 2 ++ .../demo-data-initializer.service.ts | 3 +- .../keycloak/keycloak-auth.service.spec.ts | 14 +++++--- .../auth/keycloak/keycloak-auth.service.ts | 26 +++++++++++---- src/app/core/session/auth/session-info.ts | 21 ++++++++---- .../core/session/login/login.component.html | 2 +- .../session-manager.service.spec.ts | 32 ++++++++++++++++--- .../session-manager.service.ts | 16 ++++++---- src/app/utils/utils.ts | 6 ++++ 10 files changed, 90 insertions(+), 33 deletions(-) diff --git a/src/app/core/common-components/entity-form/entity-form.service.spec.ts b/src/app/core/common-components/entity-form/entity-form.service.spec.ts index ca7bfece11..a83dff6b05 100644 --- a/src/app/core/common-components/entity-form/entity-form.service.spec.ts +++ b/src/app/core/common-components/entity-form/entity-form.service.spec.ts @@ -272,6 +272,7 @@ describe("EntityFormService", () => { const form = service.createFormGroup([{ id: "user" }], new Entity()); expect(form.get("user")).toHaveValue(null); + Entity.schema.delete("user"); }); it("should not assign default values to existing entities", () => { diff --git a/src/app/core/demo-data/demo-data-initializer.service.spec.ts b/src/app/core/demo-data/demo-data-initializer.service.spec.ts index 966d0ebf35..19fca02eca 100644 --- a/src/app/core/demo-data/demo-data-initializer.service.spec.ts +++ b/src/app/core/demo-data/demo-data-initializer.service.spec.ts @@ -18,10 +18,12 @@ import { LoginState } from "../session/session-states/login-state.enum"; describe("DemoDataInitializerService", () => { const normalUser: SessionInfo = { name: DemoUserGeneratorService.DEFAULT_USERNAME, + entityId: DemoUserGeneratorService.DEFAULT_USERNAME, roles: ["user_app"], }; const adminUser: SessionInfo = { name: DemoUserGeneratorService.ADMIN_USERNAME, + entityId: DemoUserGeneratorService.ADMIN_USERNAME, roles: ["user_app", "admin_app", "account_manager"], }; let service: DemoDataInitializerService; diff --git a/src/app/core/demo-data/demo-data-initializer.service.ts b/src/app/core/demo-data/demo-data-initializer.service.ts index d2c47076ee..4af546d6a6 100644 --- a/src/app/core/demo-data/demo-data-initializer.service.ts +++ b/src/app/core/demo-data/demo-data-initializer.service.ts @@ -31,10 +31,12 @@ export class DemoDataInitializerService { private readonly normalUser: SessionInfo = { name: DemoUserGeneratorService.DEFAULT_USERNAME, roles: ["user_app"], + entityId: DemoUserGeneratorService.DEFAULT_USERNAME, }; private readonly adminUser: SessionInfo = { name: DemoUserGeneratorService.ADMIN_USERNAME, roles: ["user_app", "admin_app", KeycloakAuthService.ACCOUNT_MANAGER_ROLE], + entityId: DemoUserGeneratorService.ADMIN_USERNAME, }; constructor( private demoDataService: DemoDataService, @@ -69,7 +71,6 @@ export class DemoDataInitializerService { } private syncDatabaseOnUserChange() { - // TODO needs to work without access to entity (entity is only available once sync starts) this.loginState.subscribe((state) => { if ( state === LoginState.LOGGED_IN && diff --git a/src/app/core/session/auth/keycloak/keycloak-auth.service.spec.ts b/src/app/core/session/auth/keycloak/keycloak-auth.service.spec.ts index 907b604ec4..a33e17e52b 100644 --- a/src/app/core/session/auth/keycloak/keycloak-auth.service.spec.ts +++ b/src/app/core/session/auth/keycloak/keycloak-auth.service.spec.ts @@ -56,14 +56,18 @@ describe("KeycloakAuthService", () => { return expectAsync(service.login()).toBeResolvedTo({ name: "test", roles: ["user_app"], + entityId: "test", }); }); - it("should throw an error if username claim is not available", () => { - const tokenWithoutUser = - "eyJhbGciOiJSUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJpcjdiVjZoOVkxazBzTGh2aFRxWThlMzgzV3NMY0V3cmFsaC1TM2NJUTVjIn0.eyJleHAiOjE2OTE1Njc4NzcsImlhdCI6MTY5MTU2NzU3NywianRpIjoiNzNiNGUzODEtMzk4My00ZjI1LWE1ZGYtOTRlOTYxYmU3MjgwIiwiaXNzIjoiaHR0cHM6Ly9rZXljbG9hay5hYW0tZGlnaXRhbC5uZXQvcmVhbG1zL2RldiIsInN1YiI6IjI0YWM1Yzg5LTU3OGMtNDdmOC1hYmQ5LTE1ZjRhNmQ4M2JiNSIsInR5cCI6IkJlYXJlciIsImF6cCI6ImFwcCIsInNlc3Npb25fc3RhdGUiOiIwYjVhNGQ0OS0wOTFhLTQzOGYtOTEwNi1mNmZjYmQyMDM1Y2EiLCJzY29wZSI6ImVtYWlsIiwic2lkIjoiMGI1YTRkNDktMDkxYS00MzhmLTkxMDYtZjZmY2JkMjAzNWNhIiwiZW1haWxfdmVyaWZpZWQiOmZhbHNlLCJfY291Y2hkYi5yb2xlcyI6WyJkZWZhdWx0LXJvbGVzLWRldiIsIm9mZmxpbmVfYWNjZXNzIiwidW1hX2F1dGhvcml6YXRpb24iLCJ1c2VyX2FwcCJdfQ.EvF1wc32KwdDCUGboviRYGqKv2C3yK5B1WL_hCm-IGg58DoE_XGOchVVfbFrtphXD3yQa8uAaY58jWb6SeZQt0P92qtn5ulZOqcs3q2gQfrvxkxafMWffpCsxusVLBuGJ4B4EgoRGp_puQJIJE4p5KBwcA_u0PznFDFyLzPD18AYXevGWKLP5L8Zfgitf3Lby5AtCoOKHM7u6F_hDGSvLw-YlHEZBupqJzbpsjOs2UF1_woChMm2vbllZgIaUu9bbobcWi1mZSfNP3r9Ojk2t0NauOiKXDqtG5XyBLYMTC6wZXxsoCPGhOAwDr9LffkLDl-zvZ-0f_ujTpU8M2jzsg"; - mockKeycloak.getToken.and.resolveTo(tokenWithoutUser); - return expectAsync(service.login()).toBeRejected(); + it("should use `sub` if `username` is not available", () => { + const tokenWithoutUsername = + "eyJhbGciOiJSUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJycVB3eFM4U1hXZ2FGOFBDbDZrYWFkVUxOYWQyaEloX21vQjhmTDdUVnJJIn0.eyJleHAiOjE3MDcyMTk0MTgsImlhdCI6MTcwNzIxNTgxOCwiYXV0aF90aW1lIjoxNzA3MjE1MDQxLCJqdGkiOiI0OWZjMjEyZS0wNGMwLTRmOWItOTAwZi1mYmVlYWE5ZGZmZjUiLCJpc3MiOiJodHRwczovL2tleWNsb2FrLmFhbS1kaWdpdGFsLm5ldC9yZWFsbXMvZGV2Iiwic3ViIjoiODQ0MGFkZDAtOTdhOS00M2VkLWFmMGItMTE2YzBmYWI3ZTkwIiwidHlwIjoiQmVhcmVyIiwiYXpwIjoiYXBwIiwibm9uY2UiOiI2N2I5N2U1NS1kMTY2LTQ3YjUtYTE4NC0zZDk1ZmIxMDQxM2UiLCJzZXNzaW9uX3N0YXRlIjoiZDZiYzQ2NTMtNmRmMC00M2NmLTliMWItNjgwODVmYTMyMTAzIiwic2NvcGUiOiJvcGVuaWQgZW1haWwiLCJzaWQiOiJkNmJjNDY1My02ZGYwLTQzY2YtOWIxYi02ODA4NWZhMzIxMDMiLCJlbWFpbF92ZXJpZmllZCI6ZmFsc2UsIl9jb3VjaGRiLnJvbGVzIjpbInVzZXJfYXBwIl19.AK5qz9keozPFwBMl4xtBVt2T42AfkAdSvX5s6kSdZBdjfqnWazi3RB4YmQ-Rfik7z_uUhXayx2i72S557d3fo1G9YttLkormB2vZ-zM0GJeYXlGmG1jLUc8w3cQARdLBTrBsgWSGo2ZnZJ-eExn8UhwG5d5BUCl-IU-KJHB1C5R3sSTgXOpkED4WRaoxPOZORr40W263tHJjjNcPECUOtmpQvY0sGUbKHGWpqgWZNXE_G75DMHd0lEBeE924sIeEZcw0Y6TpjBwJULe89EVeI6sr4qhFKjNfn_2miB1HyOOM3jxUfUngR0ju0dJpm5Jmmcyr0Pah0QiA8OWVPKEZgQ\n"; + mockKeycloak.getToken.and.resolveTo(tokenWithoutUsername); + return expectAsync(service.login()).toBeResolvedTo({ + name: "8440add0-97a9-43ed-af0b-116c0fab7e90", + roles: ["user_app"], + }); }); it("should call keycloak for a password reset", () => { diff --git a/src/app/core/session/auth/keycloak/keycloak-auth.service.ts b/src/app/core/session/auth/keycloak/keycloak-auth.service.ts index 81158e0435..2d55c685fd 100644 --- a/src/app/core/session/auth/keycloak/keycloak-auth.service.ts +++ b/src/app/core/session/auth/keycloak/keycloak-auth.service.ts @@ -5,6 +5,7 @@ import { parseJwt } from "../../../../utils/utils"; import { environment } from "../../../../../environments/environment"; import { SessionInfo } from "../session-info"; import { KeycloakService } from "keycloak-angular"; +import { LoggingService } from "../../../logging/logging.service"; /** * Handles the remote session with keycloak @@ -22,6 +23,7 @@ export class KeycloakAuthService { constructor( private httpClient: HttpClient, private keycloak: KeycloakService, + private logger: LoggingService, ) {} /** @@ -59,15 +61,25 @@ export class KeycloakAuthService { this.accessToken = token; this.logSuccessfulAuth(); const parsedToken = parseJwt(this.accessToken); - if (!parsedToken.username) { - throw new Error( - `Login error: User is not correctly set up (userId: ${parsedToken.sub})`, - ); - } - return { - name: parsedToken.username, + + const sessionInfo: SessionInfo = { + name: parsedToken.username ?? parsedToken.sub, roles: parsedToken["_couchdb.roles"], }; + + if (parsedToken.username) { + sessionInfo.entityId = parsedToken.username; + } else { + this.logger.debug( + `User not linked with an entity (userId: ${sessionInfo.name})`, + ); + } + + if (parsedToken.email) { + sessionInfo.email = parsedToken.email; + } + + return sessionInfo; } /** diff --git a/src/app/core/session/auth/session-info.ts b/src/app/core/session/auth/session-info.ts index f8472cedb7..4680e7a69e 100644 --- a/src/app/core/session/auth/session-info.ts +++ b/src/app/core/session/auth/session-info.ts @@ -7,17 +7,24 @@ import { BehaviorSubject } from "rxjs"; */ export interface SessionInfo { /** - * ID of an in-app entity. - * This can be used to retrieve an ID to which the logged-in user is linked. - * - * This is either a full ID or (e.g. Child:123) or only the last part. - * In the later case it refers to the `User` entity. + * Name of user account. */ - name?: string; + name: string; + /** - * a list of roles the logged-in user hold. + * List of roles the logged-in user hold. */ roles: string[]; + + /** + * ID of the entity which is connected with the user account. + */ + entityId?: string; + + /** + * Email address of a user + */ + email?: string; } /** diff --git a/src/app/core/session/login/login.component.html b/src/app/core/session/login/login.component.html index dee49cd7ae..a427346c3e 100644 --- a/src/app/core/session/login/login.component.html +++ b/src/app/core/session/login/login.component.html @@ -89,7 +89,7 @@

[disabled]="!enableOfflineLogin" > - {{ user.name }} + {{ user.email ?? user.name }} diff --git a/src/app/core/session/session-service/session-manager.service.spec.ts b/src/app/core/session/session-service/session-manager.service.spec.ts index dcca182a04..e9375d8989 100644 --- a/src/app/core/session/session-service/session-manager.service.spec.ts +++ b/src/app/core/session/session-service/session-manager.service.spec.ts @@ -123,7 +123,11 @@ describe("SessionManagerService", () => { const currentUser = TestBed.inject(CurrentUserSubject); // first login with existing user entity - mockKeycloak.login.and.resolveTo({ name: TEST_USER, roles: [] }); + mockKeycloak.login.and.resolveTo({ + name: TEST_USER, + roles: [], + entityId: loggedInUser.getId(), + }); await service.remoteLogin(); expect(currentUser.value).toEqual(loggedInUser); @@ -131,21 +135,39 @@ describe("SessionManagerService", () => { await service.logout(); expect(currentUser.value).toBeUndefined(); + const adminUser = new User("admin-user"); // login, user entity not available yet - mockKeycloak.login.and.resolveTo({ name: "admin-user", roles: ["admin"] }); + mockKeycloak.login.and.resolveTo({ + name: "admin-user", + roles: ["admin"], + entityId: adminUser.getId(), + }); await service.remoteLogin(); expect(currentUser.value).toBeUndefined(); // user entity available -> user should be set - const adminUser = new User("admin-user"); await entityMapper.save(adminUser); expect(currentUser.value).toEqual(adminUser); }); + it("should not initialize the user entity if no entityId is set", async () => { + const loadSpy = spyOn(TestBed.inject(EntityMapperService), "load"); + + mockKeycloak.login.and.resolveTo({ name: "some-user", roles: [] }); + await service.remoteLogin(); + + expect(loadSpy).not.toHaveBeenCalled(); + expect(loginStateSubject.value).toBe(LoginState.LOGGED_IN); + }); + it("should allow other entities to log in", async () => { - const childSession: SessionInfo = { name: "Child:123", roles: [] }; - mockKeycloak.login.and.resolveTo(childSession); const loggedInChild = new Child("123"); + const childSession: SessionInfo = { + name: loggedInChild.getId(), + roles: [], + entityId: loggedInChild.getId(), + }; + mockKeycloak.login.and.resolveTo(childSession); const otherChild = new Child("456"); await TestBed.inject(EntityMapperService).saveAll([ loggedInChild, diff --git a/src/app/core/session/session-service/session-manager.service.ts b/src/app/core/session/session-service/session-manager.service.ts index dad71fe7c0..f3dc0a95b5 100644 --- a/src/app/core/session/session-service/session-manager.service.ts +++ b/src/app/core/session/session-service/session-manager.service.ts @@ -101,15 +101,17 @@ export class SessionManagerService { await this.initializeDatabaseForCurrentUser(session); this.sessionInfo.next(session); this.loginStateSubject.next(LoginState.LOGGED_IN); - this.initUserEntity(session); + if (session.entityId) { + this.initUserEntity(session.entityId); + } } - private initUserEntity(user: SessionInfo) { - const entityType = user.name.includes(":") - ? Entity.extractTypeFromId(user.name) + private initUserEntity(entityId: string) { + const entityType = entityId.includes(":") + ? Entity.extractTypeFromId(entityId) : User; this.entityMapper - .load(entityType, user.name) + .load(entityType, entityId) .then((res) => this.currentUser.next(res)) .catch(() => undefined); this.updateSubscription = this.entityMapper @@ -117,7 +119,7 @@ export class SessionManagerService { .pipe( filter( ({ entity }) => - entity.getId() === user.name || entity.getId(true) === user.name, + entity.getId() === entityId || entity.getId(true) === entityId, ), ) .subscribe(({ entity }) => this.currentUser.next(entity)); @@ -145,7 +147,7 @@ export class SessionManagerService { } // resetting app state this.sessionInfo.next(undefined); - this.updateSubscription.unsubscribe(); + this.updateSubscription?.unsubscribe(); this.currentUser.next(undefined); this.loginStateSubject.next(LoginState.LOGGED_OUT); this.remoteLoggedIn = false; diff --git a/src/app/utils/utils.ts b/src/app/utils/utils.ts index d6dbc02e92..cc9560eb19 100644 --- a/src/app/utils/utils.ts +++ b/src/app/utils/utils.ts @@ -145,8 +145,14 @@ export function compareEnums( * @param token a valid JWT */ export function parseJwt(token): { + // keycloak user ID sub: string; + // keycloak `exact_username` attribute username: string; + // email of keycloak user + email: string; + // roles according to couchdb format + "_couchdb.roles": string[]; sid: string; jti: string; } { From cdfee7f2f9e5d087a8ecf8203385eda7fa706ccb Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 6 Feb 2024 13:08:41 +0100 Subject: [PATCH 17/27] fixed issue with unassigned default values on array properties --- .../entity-form/entity-form.service.spec.ts | 12 +++++++++--- .../entity-form/entity-form.service.ts | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/app/core/common-components/entity-form/entity-form.service.spec.ts b/src/app/core/common-components/entity-form/entity-form.service.spec.ts index a83dff6b05..d265b59841 100644 --- a/src/app/core/common-components/entity-form/entity-form.service.spec.ts +++ b/src/app/core/common-components/entity-form/entity-form.service.spec.ts @@ -265,13 +265,19 @@ describe("EntityFormService", () => { Entity.schema.delete("test"); }); - it("should current user default value should not fail if user entity does not exist", () => { - Entity.schema.set("user", { defaultValue: PLACEHOLDERS.CURRENT_USER }); + it("should not fail if user entity does not exist and current user value is assigned", () => { TestBed.inject(CurrentUserSubject).next(undefined); - const form = service.createFormGroup([{ id: "user" }], new Entity()); + // simple property + Entity.schema.set("user", { defaultValue: PLACEHOLDERS.CURRENT_USER }); + let form = service.createFormGroup([{ id: "user" }], new Entity()); + expect(form.get("user")).toHaveValue(null); + // array property + Entity.schema.get("user").dataType = EntityArrayDatatype.dataType; + form = service.createFormGroup([{ id: "user" }], new Entity()); expect(form.get("user")).toHaveValue(null); + Entity.schema.delete("user"); }); diff --git a/src/app/core/common-components/entity-form/entity-form.service.ts b/src/app/core/common-components/entity-form/entity-form.service.ts index e85caa3e75..3833b2973a 100644 --- a/src/app/core/common-components/entity-form/entity-form.service.ts +++ b/src/app/core/common-components/entity-form/entity-form.service.ts @@ -206,7 +206,7 @@ export class EntityFormService { default: newVal = schema.defaultValue; } - if (isArrayDataType(schema.dataType)) { + if (newVal && isArrayDataType(schema.dataType)) { newVal = [newVal]; } return newVal; From 4c6f3e41c0e626066080ebb628a6645c238c5501 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 6 Feb 2024 13:20:24 +0100 Subject: [PATCH 18/27] only loading missing entities of other types in entity select --- .../entity-select.component.spec.ts | 33 ++++++++++++++- .../entity-select/entity-select.component.ts | 41 +++++++++++-------- 2 files changed, 56 insertions(+), 18 deletions(-) diff --git a/src/app/core/common-components/entity-select/entity-select.component.spec.ts b/src/app/core/common-components/entity-select/entity-select.component.spec.ts index 65aad71a97..51873eb7fe 100644 --- a/src/app/core/common-components/entity-select/entity-select.component.spec.ts +++ b/src/app/core/common-components/entity-select/entity-select.component.spec.ts @@ -102,7 +102,10 @@ describe("EntitySelectComponent", () => { it("discards IDs from initial selection that don't correspond to an existing entity", fakeAsync(() => { component.entityType = User.ENTITY_TYPE; - component.selection = ["not-existing-entity", testUsers[1].getId()]; + component.selection = [ + new Child("not-existing").getId(), + testUsers[1].getId(), + ]; fixture.detectChanges(); tick(); @@ -359,4 +362,32 @@ describe("EntitySelectComponent", () => { expect(component.selectedEntities).toEqual([testUsers[1], testChildren[0]]); })); + + it("should not request entities of the defined type which were not found", fakeAsync(() => { + const loadSpy = spyOn( + TestBed.inject(EntityMapperService), + "load", + ).and.callThrough(); + + component.entityType = User.ENTITY_TYPE; + const notExistingUser = new User("not-existing-user"); + component.selection = [ + testUsers[1].getId(), + testChildren[0].getId(), + notExistingUser.getId(), + ]; + + fixture.detectChanges(); + tick(); + + expect(component.selectedEntities).toEqual([testUsers[1], testChildren[0]]); + expect(loadSpy).toHaveBeenCalledWith( + Child.ENTITY_TYPE, + testChildren[0].getId(), + ); + expect(loadSpy).not.toHaveBeenCalledWith( + User.ENTITY_TYPE, + notExistingUser.getId(), + ); + })); }); diff --git a/src/app/core/common-components/entity-select/entity-select.component.ts b/src/app/core/common-components/entity-select/entity-select.component.ts index 508fee4434..f8b56310c0 100644 --- a/src/app/core/common-components/entity-select/entity-select.component.ts +++ b/src/app/core/common-components/entity-select/entity-select.component.ts @@ -63,9 +63,12 @@ export class EntitySelectComponent implements OnChanges { * @throws Error when `type` is not in the entity-map */ @Input() set entityType(type: string | string[]) { - this.loadAvailableEntities(Array.isArray(type) ? type : [type]); + this._entityType = Array.isArray(type) ? type : [type]; + this.loadAvailableEntities(); } + private _entityType: string[]; + /** * The (initial) selection. Can be used in combination with {@link selectionChange} * to enable two-way binding to an array of strings corresponding to the id's of the entities. @@ -87,27 +90,31 @@ export class EntitySelectComponent implements OnChanges { private async initSelectedEntities(selected: string[]) { const entities: E[] = []; for (const s of selected) { - const found = this.allEntities.find((e) => s === e.getId()); - if (found) { - entities.push(found); - } else { - // missing or entity from other type - try { - const type = Entity.extractTypeFromId(s); - const entity = await this.entityMapperService.load(type, s); - entities.push(entity); - } catch (e) { + await this.getEntity(s) + .then((entity) => entities.push(entity)) + .catch((err: Error) => this.logger.warn( - `[ENTITY_SELECT] Could not find entity with ID: ${s}: ${e}`, - ); - } - } + `[ENTITY_SELECT] Error loading selected entity "${s}": ${err.message}`, + ), + ); } this.selectedEntities = entities; // updating autocomplete values this.formControl.setValue(this.formControl.value); } + private async getEntity(id: string) { + const type = Entity.extractTypeFromId(id); + const entity = this._entityType.includes(type) + ? this.allEntities.find((e) => id === e.getId()) + : await this.entityMapperService.load(type, id); + + if (!entity) { + throw Error(`Entity not found`); + } + return entity; + } + /** Underlying data-array */ selectedEntities: E[] = []; /** @@ -216,11 +223,11 @@ export class EntitySelectComponent implements OnChanges { @Input() additionalFilter: (e: E) => boolean = (_) => true; - private async loadAvailableEntities(types: string[]) { + private async loadAvailableEntities() { this.loading.next(true); const entities: E[] = []; - for (const type of types) { + for (const type of this._entityType) { entities.push(...(await this.entityMapperService.loadType(type))); } this.allEntities = entities; From fb6b4b28cd8ebc4589729bcbcf160c1d64a606f4 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 6 Feb 2024 13:59:21 +0100 Subject: [PATCH 19/27] using ID as username in keycloak --- .../user-security.component.html | 10 -------- .../user-security/user-security.component.ts | 24 ++++++++++++------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/app/core/user/user-security/user-security.component.html b/src/app/core/user/user-security/user-security.component.html index 47e250ff09..6427c51a6b 100644 --- a/src/app/core/user/user-security/user-security.component.html +++ b/src/app/core/user/user-security/user-security.component.html @@ -88,16 +88,6 @@ User is currently disabled and will not be able to login to the app

-
- - Username - - - This field is required - - -
-
Email diff --git a/src/app/core/user/user-security/user-security.component.ts b/src/app/core/user/user-security/user-security.component.ts index d78ea4c9e8..9f2a96fec4 100644 --- a/src/app/core/user/user-security/user-security.component.ts +++ b/src/app/core/user/user-security/user-security.component.ts @@ -68,6 +68,8 @@ export class UserSecurityComponent implements OnInit { ) ) { this.userIsPermitted = true; + } else { + return; } // automatically skip trailing and leading whitespaces when the form changes this.form.valueChanges.pipe(untilDestroyed(this)).subscribe((next) => { @@ -92,16 +94,17 @@ export class UserSecurityComponent implements OnInit { } ngOnInit() { - this.form.get("username").setValue(this.entity.toString()); - if (this.authService) { - this.authService - .getUser(this.entity.getId(true)) - .pipe(catchError(() => this.authService.getUser(this.entity.getId()))) - .subscribe({ - next: (res) => this.assignUser(res), - error: () => undefined, - }); + if (!this.userIsPermitted) { + return; } + this.form.get("username").setValue(this.entity.getId()); + this.authService + .getUser(this.entity.getId(true)) + .pipe(catchError(() => this.authService.getUser(this.entity.getId()))) + .subscribe({ + next: (res) => this.assignUser(res), + error: () => undefined, + }); } private assignUser(user: KeycloakUser) { @@ -147,6 +150,9 @@ export class UserSecurityComponent implements OnInit { createAccount() { const user = this.getFormValues(); + if (!user) { + return; + } user.enabled = true; if (user) { this.authService.createUser(user).subscribe({ From fc112fca3d6f2b3286863718fda252e74f95168f Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 6 Feb 2024 14:17:55 +0100 Subject: [PATCH 20/27] some code cleanup --- .../display-entity-array.component.ts | 8 ++------ src/app/core/session/auth/session-info.ts | 3 +++ .../user-security.component.spec.ts | 4 ++-- .../display-todo-completion.component.spec.ts | 16 ---------------- .../display-todo-completion.component.ts | 5 +---- 5 files changed, 8 insertions(+), 28 deletions(-) diff --git a/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.ts b/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.ts index c6d19afe1a..ae01e714ee 100644 --- a/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.ts +++ b/src/app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.ts @@ -1,4 +1,4 @@ -import { ChangeDetectorRef, Component, OnInit } from "@angular/core"; +import { Component, OnInit } from "@angular/core"; import { Entity } from "../../../entity/model/entity"; import { EntityMapperService } from "../../../entity/entity-mapper/entity-mapper.service"; import { ViewDirective } from "../../../entity/default-datatype/view.directive"; @@ -22,10 +22,7 @@ export class DisplayEntityArrayComponent entities: Entity[]; - constructor( - private entityMapper: EntityMapperService, - private changeDetector: ChangeDetectorRef, - ) { + constructor(private entityMapper: EntityMapperService) { super(); } @@ -39,7 +36,6 @@ export class DisplayEntityArrayComponent return this.entityMapper.load(type, entityId); }); this.entities = await Promise.all(entityPromises); - this.changeDetector.detectChanges(); } } } diff --git a/src/app/core/session/auth/session-info.ts b/src/app/core/session/auth/session-info.ts index 4680e7a69e..8b255b1345 100644 --- a/src/app/core/session/auth/session-info.ts +++ b/src/app/core/session/auth/session-info.ts @@ -18,6 +18,9 @@ export interface SessionInfo { /** * ID of the entity which is connected with the user account. + * + * This is either a full ID or (e.g. Child:123) or only the last part. + * In the later case it refers to the `User` entity. */ entityId?: string; diff --git a/src/app/core/user/user-security/user-security.component.spec.ts b/src/app/core/user/user-security/user-security.component.spec.ts index b7f2f7e749..3ae67cebf6 100644 --- a/src/app/core/user/user-security/user-security.component.spec.ts +++ b/src/app/core/user/user-security/user-security.component.spec.ts @@ -80,7 +80,7 @@ describe("UserSecurityComponent", () => { expect(component.user).toBe(keycloakUser); expect(component.form).toHaveValue({ - username: user.toString(), + username: user.getId(), email: "my@email.de", roles: [assignedRole], }); @@ -116,7 +116,7 @@ describe("UserSecurityComponent", () => { expect(mockHttp.post).toHaveBeenCalledWith( jasmine.stringMatching(/\/account$/), { - username: user.toString(), + username: user.getId(), email: "new@email.com", roles: [assignedRole], enabled: true, diff --git a/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.spec.ts b/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.spec.ts index 29d063e214..bbaf20920a 100644 --- a/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.spec.ts +++ b/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.spec.ts @@ -12,7 +12,6 @@ import { } from "../../../../core/entity/entity-mapper/mock-entity-mapper-service"; import { EntityMapperService } from "../../../../core/entity/entity-mapper/entity-mapper.service"; import { Child } from "../../../../child-dev-project/children/model/child"; -import { User } from "../../../../core/user/user"; describe("DisplayTodoCompletionComponent", () => { let component: DisplayTodoCompletionComponent; @@ -49,19 +48,4 @@ describe("DisplayTodoCompletionComponent", () => { expect(component.completedBy).toEqual(completingChild); })); - - it("should load the user entity when completedBy does not contain entity type", fakeAsync(() => { - const completingUser = new User("1"); - const otherUser = new User("2"); - entityMapper.addAll([completingUser, otherUser]); - - component.value = { - completedBy: completingUser.getId(true), - completedAt: new Date(), - }; - component.ngOnInit(); - tick(); - - expect(component.completedBy).toEqual(completingUser); - })); }); diff --git a/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.ts b/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.ts index b88f662b54..94a2c5e0be 100644 --- a/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.ts +++ b/src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.ts @@ -4,7 +4,6 @@ import { DatePipe, NgIf } from "@angular/common"; import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; import { EntityMapperService } from "../../../../core/entity/entity-mapper/entity-mapper.service"; import { Entity } from "../../../../core/entity/model/entity"; -import { User } from "../../../../core/user/user"; import { ViewDirective } from "../../../../core/entity/default-datatype/view.directive"; import { DynamicComponent } from "../../../../core/config/dynamic-components/dynamic-component.decorator"; @@ -29,9 +28,7 @@ export class DisplayTodoCompletionComponent ngOnInit() { if (this.value?.completedBy) { const entityId = this.value.completedBy; - const entityType = entityId.includes(":") - ? Entity.extractTypeFromId(entityId) - : User; + const entityType = Entity.extractTypeFromId(entityId); this.entityMapper .load(entityType, entityId) .then((res) => (this.completedBy = res)); From 74f45ccff86ff0f060d4d06e957f0bd852a706e6 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 7 Feb 2024 10:10:58 +0100 Subject: [PATCH 21/27] Update src/app/core/session/session-service/session-manager.service.spec.ts Co-authored-by: Sebastian --- .../core/session/session-service/session-manager.service.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/core/session/session-service/session-manager.service.spec.ts b/src/app/core/session/session-service/session-manager.service.spec.ts index e9375d8989..4a5c784d40 100644 --- a/src/app/core/session/session-service/session-manager.service.spec.ts +++ b/src/app/core/session/session-service/session-manager.service.spec.ts @@ -158,6 +158,7 @@ describe("SessionManagerService", () => { expect(loadSpy).not.toHaveBeenCalled(); expect(loginStateSubject.value).toBe(LoginState.LOGGED_IN); + expect(TestBed.inject(CurrentUserSubject).value).toBeUndefined(); }); it("should allow other entities to log in", async () => { From 2e261a7045ad2e2338be485964cfe5f9fc237ef1 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 7 Feb 2024 10:11:31 +0100 Subject: [PATCH 22/27] added interface for parsed JWT --- .../auth/keycloak/keycloak-auth.service.ts | 4 +-- src/app/session/session-utils.ts | 31 +++++++++++++++++++ src/app/utils/utils.ts | 30 ------------------ 3 files changed, 33 insertions(+), 32 deletions(-) create mode 100644 src/app/session/session-utils.ts diff --git a/src/app/core/session/auth/keycloak/keycloak-auth.service.ts b/src/app/core/session/auth/keycloak/keycloak-auth.service.ts index 2d55c685fd..bf37d44041 100644 --- a/src/app/core/session/auth/keycloak/keycloak-auth.service.ts +++ b/src/app/core/session/auth/keycloak/keycloak-auth.service.ts @@ -1,11 +1,11 @@ import { Injectable } from "@angular/core"; import { HttpClient } from "@angular/common/http"; import { Observable } from "rxjs"; -import { parseJwt } from "../../../../utils/utils"; import { environment } from "../../../../../environments/environment"; import { SessionInfo } from "../session-info"; import { KeycloakService } from "keycloak-angular"; import { LoggingService } from "../../../logging/logging.service"; +import { ParsedJWT, parseJwt } from "../../../../session/session-utils"; /** * Handles the remote session with keycloak @@ -60,7 +60,7 @@ export class KeycloakAuthService { } this.accessToken = token; this.logSuccessfulAuth(); - const parsedToken = parseJwt(this.accessToken); + const parsedToken: ParsedJWT = parseJwt(this.accessToken); const sessionInfo: SessionInfo = { name: parsedToken.username ?? parsedToken.sub, diff --git a/src/app/session/session-utils.ts b/src/app/session/session-utils.ts new file mode 100644 index 0000000000..1ae09d9126 --- /dev/null +++ b/src/app/session/session-utils.ts @@ -0,0 +1,31 @@ +/** + * JSON interface as returned by Keycloak in the access_token. + */ +export interface ParsedJWT { + // keycloak user ID + sub: string; + // keycloak `exact_username` attribute + username: string; + // email of keycloak user + email: string; + // roles according to couchdb format + "_couchdb.roles": string[]; +} + +/** + * Parses and returns the payload of a JWT into a JSON object. + * For me info see {@link https://jwt.io}. + * @param token a valid JWT + */ +export function parseJwt(token): ParsedJWT { + const base64Url = token.split(".")[1]; + const base64 = base64Url.replace(/-/g, "+").replace(/_/g, "/"); + const jsonPayload = decodeURIComponent( + window + .atob(base64) + .split("") + .map((c) => "%" + ("00" + c.charCodeAt(0).toString(16)).slice(-2)) + .join(""), + ); + return JSON.parse(jsonPayload); +} diff --git a/src/app/utils/utils.ts b/src/app/utils/utils.ts index cc9560eb19..1704442b2e 100644 --- a/src/app/utils/utils.ts +++ b/src/app/utils/utils.ts @@ -139,36 +139,6 @@ export function compareEnums( return a?.id === b?.id; } -/** - * Parses and returns the payload of a JWT into a JSON object. - * For me info see {@link https://jwt.io}. - * @param token a valid JWT - */ -export function parseJwt(token): { - // keycloak user ID - sub: string; - // keycloak `exact_username` attribute - username: string; - // email of keycloak user - email: string; - // roles according to couchdb format - "_couchdb.roles": string[]; - sid: string; - jti: string; -} { - const base64Url = token.split(".")[1]; - const base64 = base64Url.replace(/-/g, "+").replace(/_/g, "/"); - const jsonPayload = decodeURIComponent( - window - .atob(base64) - .split("") - .map((c) => "%" + ("00" + c.charCodeAt(0).toString(16)).slice(-2)) - .join(""), - ); - - return JSON.parse(jsonPayload); -} - /** * This is a simple shorthand function to create factories for services. * The use case is, when multiple services extend the same class and one of these services will be provided. From 8b663a491c1acc02aae496e64e54d77e24024e34 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 7 Feb 2024 10:38:13 +0100 Subject: [PATCH 23/27] automatically adding User prefix to tokens with short entityId --- .../demo-data/demo-data-initializer.service.spec.ts | 10 +++++----- .../core/demo-data/demo-data-initializer.service.ts | 5 +++-- .../auth/keycloak/keycloak-auth.service.spec.ts | 2 +- .../session/auth/keycloak/keycloak-auth.service.ts | 6 +++++- .../session/session-service/session-manager.service.ts | 5 +---- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/app/core/demo-data/demo-data-initializer.service.spec.ts b/src/app/core/demo-data/demo-data-initializer.service.spec.ts index 19fca02eca..6ff9475060 100644 --- a/src/app/core/demo-data/demo-data-initializer.service.spec.ts +++ b/src/app/core/demo-data/demo-data-initializer.service.spec.ts @@ -17,14 +17,14 @@ import { LoginState } from "../session/session-states/login-state.enum"; describe("DemoDataInitializerService", () => { const normalUser: SessionInfo = { - name: DemoUserGeneratorService.DEFAULT_USERNAME, - entityId: DemoUserGeneratorService.DEFAULT_USERNAME, + name: "demo", + entityId: "User:demo", roles: ["user_app"], }; const adminUser: SessionInfo = { - name: DemoUserGeneratorService.ADMIN_USERNAME, - entityId: DemoUserGeneratorService.ADMIN_USERNAME, - roles: ["user_app", "admin_app", "account_manager"], + name: "demo-admin", + entityId: "User:demo-admin", + roles: ["user_app", "admin_app"], }; let service: DemoDataInitializerService; let mockDemoDataService: jasmine.SpyObj; diff --git a/src/app/core/demo-data/demo-data-initializer.service.ts b/src/app/core/demo-data/demo-data-initializer.service.ts index 4af546d6a6..e80e73bcc1 100644 --- a/src/app/core/demo-data/demo-data-initializer.service.ts +++ b/src/app/core/demo-data/demo-data-initializer.service.ts @@ -16,6 +16,7 @@ import { AppSettings } from "../app-settings"; import { LoginStateSubject, SessionType } from "../session/session-type"; import memory from "pouchdb-adapter-memory"; import PouchDB from "pouchdb-browser"; +import { User } from "../user/user"; /** * This service handles everything related to the demo-mode @@ -31,12 +32,12 @@ export class DemoDataInitializerService { private readonly normalUser: SessionInfo = { name: DemoUserGeneratorService.DEFAULT_USERNAME, roles: ["user_app"], - entityId: DemoUserGeneratorService.DEFAULT_USERNAME, + entityId: `${User.ENTITY_TYPE}:${DemoUserGeneratorService.DEFAULT_USERNAME}`, }; private readonly adminUser: SessionInfo = { name: DemoUserGeneratorService.ADMIN_USERNAME, roles: ["user_app", "admin_app", KeycloakAuthService.ACCOUNT_MANAGER_ROLE], - entityId: DemoUserGeneratorService.ADMIN_USERNAME, + entityId: `${User.ENTITY_TYPE}:${DemoUserGeneratorService.ADMIN_USERNAME}`, }; constructor( private demoDataService: DemoDataService, diff --git a/src/app/core/session/auth/keycloak/keycloak-auth.service.spec.ts b/src/app/core/session/auth/keycloak/keycloak-auth.service.spec.ts index a33e17e52b..fb47bc08b4 100644 --- a/src/app/core/session/auth/keycloak/keycloak-auth.service.spec.ts +++ b/src/app/core/session/auth/keycloak/keycloak-auth.service.spec.ts @@ -56,7 +56,7 @@ describe("KeycloakAuthService", () => { return expectAsync(service.login()).toBeResolvedTo({ name: "test", roles: ["user_app"], - entityId: "test", + entityId: "User:test", }); }); diff --git a/src/app/core/session/auth/keycloak/keycloak-auth.service.ts b/src/app/core/session/auth/keycloak/keycloak-auth.service.ts index bf37d44041..bf989bdf8a 100644 --- a/src/app/core/session/auth/keycloak/keycloak-auth.service.ts +++ b/src/app/core/session/auth/keycloak/keycloak-auth.service.ts @@ -5,6 +5,8 @@ import { environment } from "../../../../../environments/environment"; import { SessionInfo } from "../session-info"; import { KeycloakService } from "keycloak-angular"; import { LoggingService } from "../../../logging/logging.service"; +import { Entity } from "../../../entity/model/entity"; +import { User } from "../../../user/user"; import { ParsedJWT, parseJwt } from "../../../../session/session-utils"; /** @@ -68,7 +70,9 @@ export class KeycloakAuthService { }; if (parsedToken.username) { - sessionInfo.entityId = parsedToken.username; + sessionInfo.entityId = parsedToken.username.includes(":") + ? parsedToken.username + : Entity.createPrefixedId(User.ENTITY_TYPE, parsedToken.username); } else { this.logger.debug( `User not linked with an entity (userId: ${sessionInfo.name})`, diff --git a/src/app/core/session/session-service/session-manager.service.ts b/src/app/core/session/session-service/session-manager.service.ts index f3dc0a95b5..1255bda115 100644 --- a/src/app/core/session/session-service/session-manager.service.ts +++ b/src/app/core/session/session-service/session-manager.service.ts @@ -24,7 +24,6 @@ import { LoginState } from "../session-states/login-state.enum"; import { Router } from "@angular/router"; import { KeycloakAuthService } from "../auth/keycloak/keycloak-auth.service"; import { LocalAuthService } from "../auth/local/local-auth.service"; -import { User } from "../../user/user"; import { AppSettings } from "../../app-settings"; import { PouchDatabase } from "../../database/pouch-database"; import { environment } from "../../../../environments/environment"; @@ -107,9 +106,7 @@ export class SessionManagerService { } private initUserEntity(entityId: string) { - const entityType = entityId.includes(":") - ? Entity.extractTypeFromId(entityId) - : User; + const entityType = Entity.extractTypeFromId(entityId); this.entityMapper .load(entityType, entityId) .then((res) => this.currentUser.next(res)) From d1ec5c0c260ea3320af232737f213cfbd330e3fa Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 7 Feb 2024 10:38:27 +0100 Subject: [PATCH 24/27] showing linked user entity in user account page --- .../user/user-account/user-account.component.html | 4 ++++ .../user/user-account/user-account.component.scss | 12 ++++++++++++ .../core/user/user-account/user-account.component.ts | 12 +++++++++++- 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/app/core/user/user-account/user-account.component.html b/src/app/core/user/user-account/user-account.component.html index e8c12b90ac..d8d4d57b22 100644 --- a/src/app/core/user/user-account/user-account.component.html +++ b/src/app/core/user/user-account/user-account.component.html @@ -32,5 +32,9 @@
+
+ Linked entity: + +
diff --git a/src/app/core/user/user-account/user-account.component.scss b/src/app/core/user/user-account/user-account.component.scss index 6fbc04ac53..f655dbf037 100644 --- a/src/app/core/user/user-account/user-account.component.scss +++ b/src/app/core/user/user-account/user-account.component.scss @@ -28,3 +28,15 @@ .min-width-1-3 { max-width: 600px; } + +.entity-box { + border: 1px solid lightgrey; + border-radius: 10px; + width: fit-content; + display: flex; + padding: 10px; +} + +.entity-box span { + margin-right: 5px; +} diff --git a/src/app/core/user/user-account/user-account.component.ts b/src/app/core/user/user-account/user-account.component.ts index e9436e7347..e63a81b213 100644 --- a/src/app/core/user/user-account/user-account.component.ts +++ b/src/app/core/user/user-account/user-account.component.ts @@ -25,6 +25,9 @@ import { MatTooltipModule } from "@angular/material/tooltip"; import { MatInputModule } from "@angular/material/input"; import { AccountPageComponent } from "../../session/auth/keycloak/account-page/account-page.component"; import { CurrentUserSubject } from "../../session/current-user-subject"; +import { AsyncPipe, NgIf } from "@angular/common"; +import { DisplayEntityComponent } from "../../basic-datatypes/entity/display-entity/display-entity.component"; +import { SessionSubject } from "../../session/auth/session-info"; /** * User account form to allow the user to view and edit information. @@ -40,6 +43,9 @@ import { CurrentUserSubject } from "../../session/current-user-subject"; MatTooltipModule, MatInputModule, AccountPageComponent, + AsyncPipe, + DisplayEntityComponent, + NgIf, ], standalone: true, }) @@ -49,8 +55,12 @@ export class UserAccountComponent implements OnInit { passwordChangeDisabled = false; tooltipText; + entityId = this.sessionInfo.value.entityId; - constructor(private currentUser: CurrentUserSubject) {} + constructor( + private currentUser: CurrentUserSubject, + private sessionInfo: SessionSubject, + ) {} ngOnInit() { this.checkIfPasswordChangeAllowed(); From 211e229256c09cbf9b162bbda5da037c23b942b8 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 7 Feb 2024 10:52:16 +0100 Subject: [PATCH 25/27] removed account manager role from demo mode --- src/app/core/demo-data/demo-data-initializer.service.ts | 3 +-- src/app/core/session/auth/keycloak/keycloak-auth.service.ts | 3 --- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/app/core/demo-data/demo-data-initializer.service.ts b/src/app/core/demo-data/demo-data-initializer.service.ts index e80e73bcc1..9eb5c529ed 100644 --- a/src/app/core/demo-data/demo-data-initializer.service.ts +++ b/src/app/core/demo-data/demo-data-initializer.service.ts @@ -5,7 +5,6 @@ import { MatDialog } from "@angular/material/dialog"; import { DemoDataGeneratingProgressDialogComponent } from "./demo-data-generating-progress-dialog.component"; import { SessionManagerService } from "../session/session-service/session-manager.service"; import { LocalAuthService } from "../session/auth/local/local-auth.service"; -import { KeycloakAuthService } from "../session/auth/keycloak/keycloak-auth.service"; import { SessionInfo, SessionSubject } from "../session/auth/session-info"; import { LoggingService } from "../logging/logging.service"; import { Database } from "../database/database"; @@ -36,7 +35,7 @@ export class DemoDataInitializerService { }; private readonly adminUser: SessionInfo = { name: DemoUserGeneratorService.ADMIN_USERNAME, - roles: ["user_app", "admin_app", KeycloakAuthService.ACCOUNT_MANAGER_ROLE], + roles: ["user_app", "admin_app"], entityId: `${User.ENTITY_TYPE}:${DemoUserGeneratorService.ADMIN_USERNAME}`, }; constructor( diff --git a/src/app/core/session/auth/keycloak/keycloak-auth.service.ts b/src/app/core/session/auth/keycloak/keycloak-auth.service.ts index bf989bdf8a..4dd986844f 100644 --- a/src/app/core/session/auth/keycloak/keycloak-auth.service.ts +++ b/src/app/core/session/auth/keycloak/keycloak-auth.service.ts @@ -57,9 +57,6 @@ export class KeycloakAuthService { } private processToken(token: string): SessionInfo { - if (!token) { - throw new Error(); - } this.accessToken = token; this.logSuccessfulAuth(); const parsedToken: ParsedJWT = parseJwt(this.accessToken); From f97689a8ea06d798a491eb467cb64e9e0a46162b Mon Sep 17 00:00:00 2001 From: Sebastian Leidig Date: Thu, 8 Feb 2024 08:20:20 +0100 Subject: [PATCH 26/27] improve user profile ui --- src/app/core/config/config-fix.ts | 3 +-- .../user/user-account/user-account.component.html | 11 ++++++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/app/core/config/config-fix.ts b/src/app/core/config/config-fix.ts index 55977aa5cf..84b2a62b3a 100644 --- a/src/app/core/config/config-fix.ts +++ b/src/app/core/config/config-fix.ts @@ -277,8 +277,7 @@ export const defaultJsonConfig = { ] } ], - }, - "permittedUserRoles": ["admin_app"] + } }, "view:help": { "component": "MarkdownPage", diff --git a/src/app/core/user/user-account/user-account.component.html b/src/app/core/user/user-account/user-account.component.html index d8d4d57b22..76b983aad3 100644 --- a/src/app/core/user/user-account/user-account.component.html +++ b/src/app/core/user/user-account/user-account.component.html @@ -32,9 +32,14 @@ -
- Linked entity: - + +
+
+

Your profile:

+
From 2ca6de3523f0a1c8961e6aca7c6d50913fd30611 Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 8 Feb 2024 09:30:44 +0100 Subject: [PATCH 27/27] removed unused styles --- .../user-account/user-account.component.scss | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/src/app/core/user/user-account/user-account.component.scss b/src/app/core/user/user-account/user-account.component.scss index f655dbf037..438d7b9d2b 100644 --- a/src/app/core/user/user-account/user-account.component.scss +++ b/src/app/core/user/user-account/user-account.component.scss @@ -1,30 +1,3 @@ -/* - * This file is part of ndb-core. - * - * ndb-core is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * ndb-core is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with ndb-core. If not, see . - */ - -@use "@angular/material/core/style/elevation" as mat-elevation; -@use "variables/sizes"; - -.info-field { - @include mat-elevation.elevation(1); - font-style: italic; - margin: sizes.$small; - padding: sizes.$small; -} - .min-width-1-3 { max-width: 600px; } @@ -36,7 +9,3 @@ display: flex; padding: 10px; } - -.entity-box span { - margin-right: 5px; -}