From 0d0174c864f05c916f68c35634b72ae46bac4305 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 26 Apr 2023 10:59:59 +0200 Subject: [PATCH 01/22] fix: popups ask user if changes should be saved when closing --- .../dialog-buttons.component.ts | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts index a7de09ff1d..ffc358dd27 100644 --- a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts +++ b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts @@ -17,7 +17,10 @@ import { MatMenuModule } from "@angular/material/menu"; import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; import { Router, RouterLink } from "@angular/router"; import { EntityAbility } from "../../permissions/ability/entity-ability"; +import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; +import { ConfirmationDialogService } from "../../confirmation-dialog/confirmation-dialog.service"; +@UntilDestroy() @Component({ selector: "app-dialog-buttons", standalone: true, @@ -45,8 +48,20 @@ export class DialogButtonsComponent implements OnInit { private alertService: AlertService, private entityRemoveService: EntityRemoveService, private router: Router, - private ability: EntityAbility - ) {} + private ability: EntityAbility, + private confirmation: ConfirmationDialogService + ) { + this.dialog.beforeClosed().subscribe(() => { + if (this.form.dirty) { + this.confirmation + .getConfirmation( + $localize`:Save changes header:Save Changes?`, + $localize`:Save changes message:Do you want to save the changes you made?` + ) + .then((confirmed) => (confirmed ? this.save() : undefined)); + } + }); + } ngOnInit() { if (!this.entity.isNew) { @@ -55,6 +70,9 @@ export class DialogButtonsComponent implements OnInit { } this.initializeDetailsRouteIfAvailable(); } + this.form.statusChanges + .pipe(untilDestroyed(this)) + .subscribe(() => (this.dialog.disableClose = this.form.dirty)); } private initializeDetailsRouteIfAvailable() { From 6605fc4894f1c315ea8973ab6c51dad1fc433542 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 26 Apr 2023 11:11:25 +0200 Subject: [PATCH 02/22] fix: details page ask user if changes should be saved when navigating away --- .../entity-details/form/form.component.ts | 29 +++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/src/app/core/entity-components/entity-details/form/form.component.ts b/src/app/core/entity-components/entity-details/form/form.component.ts index 1f4895a71a..0b186a2f2d 100644 --- a/src/app/core/entity-components/entity-details/form/form.component.ts +++ b/src/app/core/entity-components/entity-details/form/form.component.ts @@ -2,7 +2,7 @@ import { Component, Input, OnInit } from "@angular/core"; import { Entity } from "../../../entity/model/entity"; import { FormFieldConfig } from "../../entity-form/entity-form/FormConfig"; import { getParentUrl } from "../../../../utils/utils"; -import { Router } from "@angular/router"; +import { NavigationStart, Router } from "@angular/router"; import { Location, NgIf } from "@angular/common"; import { DynamicComponent } from "../../../view/dynamic-components/dynamic-component.decorator"; import { InvalidFormFieldError } from "../../entity-form/invalid-form-field.error"; @@ -15,11 +15,15 @@ import { toFormFieldConfig } from "../../entity-subrecord/entity-subrecord/entit import { MatButtonModule } from "@angular/material/button"; import { EntityFormComponent } from "../../entity-form/entity-form/entity-form.component"; import { DisableEntityOperationDirective } from "../../../permissions/permission-directive/disable-entity-operation.directive"; +import { filter } from "rxjs/operators"; +import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; +import { ConfirmationDialogService } from "../../../confirmation-dialog/confirmation-dialog.service"; /** * A simple wrapper function of the EntityFormComponent which can be used as a dynamic component * e.g. as a panel for the EntityDetailsComponent. */ +@UntilDestroy() @DynamicComponent("Form") @Component({ selector: "app-form", @@ -49,8 +53,27 @@ export class FormComponent implements OnInit { private router: Router, private location: Location, private entityFormService: EntityFormService, - private alertService: AlertService - ) {} + private alertService: AlertService, + private confirmation: ConfirmationDialogService + ) { + this.router.events + .pipe( + untilDestroyed(this), + filter((ev) => ev instanceof NavigationStart) + ) + .subscribe((ev) => { + if (this.form.dirty) { + this.confirmation + .getConfirmation( + $localize`:Save changes header:Save Changes?`, + $localize`:Save changes message:Do you want to save the changes you made?` + ) + .then((confirmed) => + confirmed ? this.saveClicked() : this.cancelClicked() + ); + } + }); + } ngOnInit() { this.form = this.entityFormService.createFormGroup( From 96fee54021854db2ad4c6ae9dd91155747a29339 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 8 May 2023 14:10:56 +0200 Subject: [PATCH 03/22] refactored deprecated auth guard syntax --- src/app/core/session/auth.guard.ts | 35 +++++++++++++----------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/src/app/core/session/auth.guard.ts b/src/app/core/session/auth.guard.ts index 544f9ab130..d6ab2ea598 100644 --- a/src/app/core/session/auth.guard.ts +++ b/src/app/core/session/auth.guard.ts @@ -1,28 +1,23 @@ -import { Injectable } from "@angular/core"; +import { inject } from "@angular/core"; import { ActivatedRouteSnapshot, - CanActivate, + CanActivateFn, Router, RouterStateSnapshot, } from "@angular/router"; import { SessionService } from "./session-service/session.service"; -@Injectable({ - providedIn: "root", -}) -export class AuthGuard implements CanActivate { - constructor(private session: SessionService, private router: Router) {} - - canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot) { - if (this.session.isLoggedIn()) { - return true; - } else { - this.router.navigate(["/login"], { - queryParams: { - redirect_uri: state.url, - }, - }); - return false; - } +export const AuthGuard: CanActivateFn = ( + route: ActivatedRouteSnapshot, + state: RouterStateSnapshot +) => { + if (inject(SessionService).isLoggedIn()) { + return true; + } else { + return inject(Router).createUrlTree(["/login"], { + queryParams: { + redirect_uri: state.url, + }, + }); } -} +}; From 8d37d774fb0827fcb6313ea032c79d305390a446 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 8 May 2023 14:58:03 +0200 Subject: [PATCH 04/22] added changes popup before navigation is done --- .../entity-details/form/form.component.ts | 34 +++++-------------- .../form/unsaved-changes.service.spec.ts | 16 +++++++++ .../form/unsaved-changes.service.ts | 30 ++++++++++++++++ .../view/dynamic-routing/router.service.ts | 6 +++- 4 files changed, 60 insertions(+), 26 deletions(-) create mode 100644 src/app/core/entity-components/entity-details/form/unsaved-changes.service.spec.ts create mode 100644 src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts diff --git a/src/app/core/entity-components/entity-details/form/form.component.ts b/src/app/core/entity-components/entity-details/form/form.component.ts index 0b186a2f2d..38dad51e64 100644 --- a/src/app/core/entity-components/entity-details/form/form.component.ts +++ b/src/app/core/entity-components/entity-details/form/form.component.ts @@ -2,7 +2,7 @@ import { Component, Input, OnInit } from "@angular/core"; import { Entity } from "../../../entity/model/entity"; import { FormFieldConfig } from "../../entity-form/entity-form/FormConfig"; import { getParentUrl } from "../../../../utils/utils"; -import { NavigationStart, Router } from "@angular/router"; +import { Router } from "@angular/router"; import { Location, NgIf } from "@angular/common"; import { DynamicComponent } from "../../../view/dynamic-components/dynamic-component.decorator"; import { InvalidFormFieldError } from "../../entity-form/invalid-form-field.error"; @@ -15,15 +15,12 @@ import { toFormFieldConfig } from "../../entity-subrecord/entity-subrecord/entit import { MatButtonModule } from "@angular/material/button"; import { EntityFormComponent } from "../../entity-form/entity-form/entity-form.component"; import { DisableEntityOperationDirective } from "../../../permissions/permission-directive/disable-entity-operation.directive"; -import { filter } from "rxjs/operators"; -import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; -import { ConfirmationDialogService } from "../../../confirmation-dialog/confirmation-dialog.service"; +import { UnsavedChangesService } from "./unsaved-changes.service"; /** * A simple wrapper function of the EntityFormComponent which can be used as a dynamic component * e.g. as a panel for the EntityDetailsComponent. */ -@UntilDestroy() @DynamicComponent("Form") @Component({ selector: "app-form", @@ -54,26 +51,8 @@ export class FormComponent implements OnInit { private location: Location, private entityFormService: EntityFormService, private alertService: AlertService, - private confirmation: ConfirmationDialogService - ) { - this.router.events - .pipe( - untilDestroyed(this), - filter((ev) => ev instanceof NavigationStart) - ) - .subscribe((ev) => { - if (this.form.dirty) { - this.confirmation - .getConfirmation( - $localize`:Save changes header:Save Changes?`, - $localize`:Save changes message:Do you want to save the changes you made?` - ) - .then((confirmed) => - confirmed ? this.saveClicked() : this.cancelClicked() - ); - } - }); - } + private unsavedChanges: UnsavedChangesService + ) {} ngOnInit() { this.form = this.entityFormService.createFormGroup( @@ -83,6 +62,9 @@ export class FormComponent implements OnInit { if (!this.creatingNew) { this.form.disable(); } + this.form.valueChanges.subscribe( + () => (this.unsavedChanges.pending = this.form.dirty) + ); } async saveClicked() { @@ -91,6 +73,7 @@ export class FormComponent implements OnInit { this.form.markAsPristine(); this.form.disable(); if (this.creatingNew) { + this.unsavedChanges.pending = false; await this.router.navigate([ getParentUrl(this.router), this.entity.getId(), @@ -109,5 +92,6 @@ export class FormComponent implements OnInit { } this.entityFormService.resetForm(this.form, this.entity); this.form.disable(); + this.unsavedChanges.pending = false; } } diff --git a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.spec.ts b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.spec.ts new file mode 100644 index 0000000000..e050b6514f --- /dev/null +++ b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.spec.ts @@ -0,0 +1,16 @@ +import { TestBed } from '@angular/core/testing'; + +import { UnsavedChangesService } from './unsaved-changes.service'; + +describe('UnsavedChangesService', () => { + let service: UnsavedChangesService; + + beforeEach(() => { + TestBed.configureTestingModule({}); + service = TestBed.inject(UnsavedChangesService); + }); + + it('should be created', () => { + expect(service).toBeTruthy(); + }); +}); diff --git a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts new file mode 100644 index 0000000000..d91047cbf4 --- /dev/null +++ b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts @@ -0,0 +1,30 @@ +import { Injectable } from "@angular/core"; +import { ConfirmationDialogService } from "../../../confirmation-dialog/confirmation-dialog.service"; + +@Injectable({ + providedIn: "root", +}) +export class UnsavedChangesService { + pending = false; + + constructor(private confirmation: ConfirmationDialogService) { + // prevent navigation if changes are pending + window.onbeforeunload = () => (this.pending ? "" : undefined); + } + + async checkUnsavedChanges() { + if (this.pending) { + const confirmed = await this.confirmation.getConfirmation( + "Discard changes?", + "All unsaved changes will be lost" + ); + if (confirmed) { + this.pending = false; + return true; + } else { + return false; + } + } + return true; + } +} diff --git a/src/app/core/view/dynamic-routing/router.service.ts b/src/app/core/view/dynamic-routing/router.service.ts index b05bcebad0..70ef26ae6b 100644 --- a/src/app/core/view/dynamic-routing/router.service.ts +++ b/src/app/core/view/dynamic-routing/router.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from "@angular/core"; +import { inject, Injectable } from "@angular/core"; import { Route, Router } from "@angular/router"; import { ConfigService } from "../../config/config.service"; import { LoggingService } from "../../logging/logging.service"; @@ -11,6 +11,7 @@ import { UserRoleGuard } from "../../permissions/permission-guard/user-role.guar import { NotFoundComponent } from "./not-found/not-found.component"; import { ComponentRegistry } from "../../../dynamic-components"; import { AuthGuard } from "../../session/auth.guard"; +import { UnsavedChangesService } from "../../entity-components/entity-details/form/unsaved-changes.service"; /** * The RouterService dynamically sets up Angular routing from config loaded through the {@link ConfigService}. @@ -90,6 +91,9 @@ export class RouterService { private generateRouteFromConfig(view: ViewConfig, route: Route): Route { const routeData: RouteData = {}; route.canActivate = [AuthGuard]; + route.canDeactivate = [ + () => inject(UnsavedChangesService).checkUnsavedChanges(), + ]; if (view.permittedUserRoles) { route.canActivate.push(UserRoleGuard); From f85cb36da6ad6c48e9a48ec1ff112e0280fc2b60 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 8 May 2023 14:59:29 +0200 Subject: [PATCH 05/22] added todo for forms in different panels --- .../core/entity-components/entity-details/form/form.component.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/core/entity-components/entity-details/form/form.component.ts b/src/app/core/entity-components/entity-details/form/form.component.ts index 38dad51e64..b5e44dd80e 100644 --- a/src/app/core/entity-components/entity-details/form/form.component.ts +++ b/src/app/core/entity-components/entity-details/form/form.component.ts @@ -92,6 +92,7 @@ export class FormComponent implements OnInit { } this.entityFormService.resetForm(this.form, this.entity); this.form.disable(); + // TODO this might overwrite active changes from another panel with a form this.unsavedChanges.pending = false; } } From 578b9a1ba050d98d9fcb7474c493c38f06d36f3d Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 8 May 2023 16:30:00 +0200 Subject: [PATCH 06/22] popup is only shown when backdrop is clicked --- .../form-dialog/dialog-buttons/dialog-buttons.component.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts index ffc358dd27..cbd1de05e8 100644 --- a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts +++ b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts @@ -51,7 +51,7 @@ export class DialogButtonsComponent implements OnInit { private ability: EntityAbility, private confirmation: ConfirmationDialogService ) { - this.dialog.beforeClosed().subscribe(() => { + this.dialog.backdropClick().subscribe(() => { if (this.form.dirty) { this.confirmation .getConfirmation( @@ -91,6 +91,7 @@ export class DialogButtonsComponent implements OnInit { .then((res) => { // Attachments are only saved once form is disabled this.form.disable(); + this.form.markAsPristine(); this.dialog.close(res); }) .catch((err) => { From 247183aeb58df14c9a0810456dafbb26728959d8 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 24 May 2023 09:20:24 +0200 Subject: [PATCH 07/22] fixed browser navigation popup --- .../entity-details/form/unsaved-changes.service.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts index d91047cbf4..0d38496a9c 100644 --- a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts +++ b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts @@ -9,7 +9,12 @@ export class UnsavedChangesService { constructor(private confirmation: ConfirmationDialogService) { // prevent navigation if changes are pending - window.onbeforeunload = () => (this.pending ? "" : undefined); + window.onbeforeunload = (e) => { + if (this.pending) { + e.preventDefault(); + e.returnValue = "onbeforeunload"; + } + }; } async checkUnsavedChanges() { From 23648bfd9a6da74c061dab06d4760f1b1410fa71 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 24 May 2023 09:22:06 +0200 Subject: [PATCH 08/22] Update src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts Co-authored-by: Sebastian --- .../entity-details/form/unsaved-changes.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts index d91047cbf4..6918d8e92e 100644 --- a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts +++ b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts @@ -16,7 +16,7 @@ export class UnsavedChangesService { if (this.pending) { const confirmed = await this.confirmation.getConfirmation( "Discard changes?", - "All unsaved changes will be lost" + "You have unsaved changes. Do you really want to leave this page? All unsaved changes will be lost." ); if (confirmed) { this.pending = false; From a702166ce93305702bb02d5d2017b9ca69167172 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 24 May 2023 09:25:30 +0200 Subject: [PATCH 09/22] centralised save confirmation --- .../confirmation-dialog/confirmation-dialog.service.ts | 7 +++++++ .../entity-details/form/unsaved-changes.service.ts | 5 +---- .../form-dialog/dialog-buttons/dialog-buttons.component.ts | 5 +---- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/app/core/confirmation-dialog/confirmation-dialog.service.ts b/src/app/core/confirmation-dialog/confirmation-dialog.service.ts index b2ed731714..acb1834d43 100644 --- a/src/app/core/confirmation-dialog/confirmation-dialog.service.ts +++ b/src/app/core/confirmation-dialog/confirmation-dialog.service.ts @@ -55,4 +55,11 @@ export class ConfirmationDialogService { .afterClosed() ); } + + getSaveConfirmation() { + return this.getConfirmation( + $localize`:Save changes header:Save Changes?`, + $localize`:Save changes message:You have unsaved changes. Do you really want to leave this page? All unsaved changes will be lost.` + ); + } } diff --git a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts index 0d38496a9c..f1a4e47dd0 100644 --- a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts +++ b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts @@ -19,10 +19,7 @@ export class UnsavedChangesService { async checkUnsavedChanges() { if (this.pending) { - const confirmed = await this.confirmation.getConfirmation( - "Discard changes?", - "All unsaved changes will be lost" - ); + const confirmed = await this.confirmation.getSaveConfirmation(); if (confirmed) { this.pending = false; return true; diff --git a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts index cbd1de05e8..5913739a2f 100644 --- a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts +++ b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts @@ -54,10 +54,7 @@ export class DialogButtonsComponent implements OnInit { this.dialog.backdropClick().subscribe(() => { if (this.form.dirty) { this.confirmation - .getConfirmation( - $localize`:Save changes header:Save Changes?`, - $localize`:Save changes message:Do you want to save the changes you made?` - ) + .getSaveConfirmation() .then((confirmed) => (confirmed ? this.save() : undefined)); } }); From c21ba4ab251f315499adf42c4858652f4b2eaf4a Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 24 May 2023 09:32:10 +0200 Subject: [PATCH 10/22] fixed auth guard tests --- src/app/core/session/auth.guard.spec.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/app/core/session/auth.guard.spec.ts b/src/app/core/session/auth.guard.spec.ts index f5bb5f3bdc..c75c53aa87 100644 --- a/src/app/core/session/auth.guard.spec.ts +++ b/src/app/core/session/auth.guard.spec.ts @@ -2,10 +2,10 @@ import { TestBed } from "@angular/core/testing"; import { AuthGuard } from "./auth.guard"; import { SessionService } from "./session-service/session.service"; -import { Router } from "@angular/router"; +import { CanActivateFn, Router } from "@angular/router"; describe("AuthGuard", () => { - let guard: AuthGuard; + let guard: CanActivateFn; let mockSession: jasmine.SpyObj; let mockRouter: jasmine.SpyObj; @@ -28,15 +28,13 @@ describe("AuthGuard", () => { it("should return true if user is logged in", () => { mockSession.isLoggedIn.and.returnValue(true); - expect(guard.canActivate(undefined, undefined)).toBeTrue(); + expect(guard(undefined, undefined)).toBeTrue(); }); it("should navigate to login page with redirect url if not logged in", () => { mockSession.isLoggedIn.and.returnValue(false); - expect( - guard.canActivate(undefined, { url: "/some/url" } as any) - ).toBeFalse(); + expect(guard(undefined, { url: "/some/url" } as any)).toBeFalse(); expect(mockRouter.navigate).toHaveBeenCalledWith(["/login"], { queryParams: { redirect_uri: "/some/url" }, }); From f3dc37eef3a778257e4ba8b2ce8b4d053c2533f0 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 24 May 2023 12:12:16 +0200 Subject: [PATCH 11/22] fixed existing tests --- .../note-details.component.spec.ts | 3 ++- .../row-details/row-details.component.spec.ts | 3 ++- .../dialog-buttons.component.spec.ts | 5 ++-- src/app/core/session/auth.guard.spec.ts | 27 +++++++++---------- .../dynamic-routing/router.service.spec.ts | 5 ++++ .../todo-details.component.spec.ts | 6 ++++- 6 files changed, 29 insertions(+), 20 deletions(-) diff --git a/src/app/child-dev-project/notes/note-details/note-details.component.spec.ts b/src/app/child-dev-project/notes/note-details/note-details.component.spec.ts index 2727f7b64a..7659d44d34 100644 --- a/src/app/child-dev-project/notes/note-details/note-details.component.spec.ts +++ b/src/app/child-dev-project/notes/note-details/note-details.component.spec.ts @@ -7,6 +7,7 @@ import { defaultAttendanceStatusTypes } from "../../../core/config/default-confi import { MockedTestingModule } from "../../../utils/mocked-testing.module"; import { LoginState } from "../../../core/session/session-states/login-state.enum"; import { EntityConfigService } from "../../../core/entity/entity-config.service"; +import { NEVER } from "rxjs"; function generateTestNote(forChildren: Child[]) { const testNote = Note.create(new Date(), "test note"); @@ -42,7 +43,7 @@ describe("NoteDetailsComponent", () => { ], providers: [ { provide: MAT_DIALOG_DATA, useValue: { entity: testNote } }, - { provide: MatDialogRef, useValue: {} }, + { provide: MatDialogRef, useValue: { backdropClick: () => NEVER } }, ], }).compileComponents(); diff --git a/src/app/core/entity-components/entity-subrecord/row-details/row-details.component.spec.ts b/src/app/core/entity-components/entity-subrecord/row-details/row-details.component.spec.ts index 7ef5169472..bc9a620894 100644 --- a/src/app/core/entity-components/entity-subrecord/row-details/row-details.component.spec.ts +++ b/src/app/core/entity-components/entity-subrecord/row-details/row-details.component.spec.ts @@ -8,6 +8,7 @@ import { MatDialogRef, MAT_DIALOG_DATA } from "@angular/material/dialog"; import { Entity } from "../../../entity/model/entity"; import { MockedTestingModule } from "../../../../utils/mocked-testing.module"; import { EntityAbility } from "../../../permissions/ability/entity-ability"; +import { NEVER } from "rxjs"; describe("RowDetailsComponent", () => { let component: RowDetailsComponent; @@ -23,7 +24,7 @@ describe("RowDetailsComponent", () => { imports: [RowDetailsComponent, MockedTestingModule.withState()], providers: [ { provide: MAT_DIALOG_DATA, useValue: detailsComponentData }, - { provide: MatDialogRef, useValue: {} }, + { provide: MatDialogRef, useValue: { backdropClick: () => NEVER } }, ], }).compileComponents(); spyOn(TestBed.inject(EntityAbility), "cannot").and.returnValue(true); diff --git a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.spec.ts b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.spec.ts index 8b42fd9a74..01612f1a2a 100644 --- a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.spec.ts +++ b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.spec.ts @@ -18,7 +18,7 @@ import { EntityRemoveService, RemoveResult, } from "../../entity/entity-remove.service"; -import { of } from "rxjs"; +import { EMPTY, of } from "rxjs"; describe("DialogButtonsComponent", () => { let component: DialogButtonsComponent; @@ -26,7 +26,8 @@ describe("DialogButtonsComponent", () => { let dialogRef: jasmine.SpyObj>; beforeEach(async () => { - dialogRef = jasmine.createSpyObj(["close"]); + dialogRef = jasmine.createSpyObj(["close", "backdropClick"]); + dialogRef.backdropClick.and.returnValue(EMPTY); await TestBed.configureTestingModule({ imports: [DialogButtonsComponent, MockedTestingModule.withState()], providers: [{ provide: MatDialogRef, useValue: dialogRef }], diff --git a/src/app/core/session/auth.guard.spec.ts b/src/app/core/session/auth.guard.spec.ts index c75c53aa87..6b5a4165bc 100644 --- a/src/app/core/session/auth.guard.spec.ts +++ b/src/app/core/session/auth.guard.spec.ts @@ -2,41 +2,38 @@ import { TestBed } from "@angular/core/testing"; import { AuthGuard } from "./auth.guard"; import { SessionService } from "./session-service/session.service"; -import { CanActivateFn, Router } from "@angular/router"; +import { RouterTestingModule } from "@angular/router/testing"; describe("AuthGuard", () => { - let guard: CanActivateFn; let mockSession: jasmine.SpyObj; - let mockRouter: jasmine.SpyObj; beforeEach(() => { mockSession = jasmine.createSpyObj(["isLoggedIn"]); - mockRouter = jasmine.createSpyObj(["navigate"]); TestBed.configureTestingModule({ - providers: [ - { provide: SessionService, useValue: mockSession }, - { provide: Router, useValue: mockRouter }, - ], + imports: [RouterTestingModule], + providers: [{ provide: SessionService, useValue: mockSession }], }); - guard = TestBed.inject(AuthGuard); }); it("should be created", () => { - expect(guard).toBeTruthy(); + expect(AuthGuard).toBeTruthy(); }); it("should return true if user is logged in", () => { mockSession.isLoggedIn.and.returnValue(true); - expect(guard(undefined, undefined)).toBeTrue(); + const res = TestBed.runInInjectionContext(() => + AuthGuard(undefined, undefined) + ); + expect(res).toBeTrue(); }); it("should navigate to login page with redirect url if not logged in", () => { mockSession.isLoggedIn.and.returnValue(false); - expect(guard(undefined, { url: "/some/url" } as any)).toBeFalse(); - expect(mockRouter.navigate).toHaveBeenCalledWith(["/login"], { - queryParams: { redirect_uri: "/some/url" }, - }); + const res = TestBed.runInInjectionContext(() => + AuthGuard(undefined, { url: "/some/url" } as any) + ); + expect(res.toString()).toBe("/login?redirect_uri=%2Fsome%2Furl"); }); }); diff --git a/src/app/core/view/dynamic-routing/router.service.spec.ts b/src/app/core/view/dynamic-routing/router.service.spec.ts index 32cc8e8ce6..6ae16054ab 100644 --- a/src/app/core/view/dynamic-routing/router.service.spec.ts +++ b/src/app/core/view/dynamic-routing/router.service.spec.ts @@ -65,18 +65,21 @@ describe("RouterService", () => { path: "child", loadComponent: componentRegistry.get("ChildrenList"), data: {}, + canDeactivate: [jasmine.any(Function)], canActivate: [AuthGuard], }, { path: "child/:id", loadComponent: componentRegistry.get("EntityDetails"), data: { config: testViewConfig }, + canDeactivate: [jasmine.any(Function)], canActivate: [AuthGuard], }, { path: "admin", loadComponent: componentRegistry.get("Admin"), canActivate: [AuthGuard, UserRoleGuard], + canDeactivate: [jasmine.any(Function)], data: { permittedUserRoles: ["user_app"] }, }, ]; @@ -106,6 +109,7 @@ describe("RouterService", () => { path: "other", component: TestComponent, canActivate: [AuthGuard, UserRoleGuard], + canDeactivate: [jasmine.any(Function)], data: { permittedUserRoles: ["admin_app"] }, }, { path: "child", component: ChildrenListComponent }, @@ -156,6 +160,7 @@ describe("RouterService", () => { path: "admin", loadComponent: componentRegistry.get("Admin"), canActivate: [AuthGuard, UserRoleGuard], + canDeactivate: [jasmine.any(Function)], data: { permittedUserRoles: ["admin"] }, }, ]; diff --git a/src/app/features/todos/todo-details/todo-details.component.spec.ts b/src/app/features/todos/todo-details/todo-details.component.spec.ts index 8751208824..1b2a56db6a 100644 --- a/src/app/features/todos/todo-details/todo-details.component.spec.ts +++ b/src/app/features/todos/todo-details/todo-details.component.spec.ts @@ -8,6 +8,7 @@ import { TodoService } from "../todo.service"; import { MockedTestingModule } from "../../../utils/mocked-testing.module"; import { LoginState } from "../../../core/session/session-states/login-state.enum"; import { EntityMapperService } from "../../../core/entity/entity-mapper.service"; +import { NEVER } from "rxjs"; describe("TodoDetailsComponent", () => { let component: TodoDetailsComponent; @@ -25,7 +26,10 @@ describe("TodoDetailsComponent", () => { provide: MAT_DIALOG_DATA, useValue: { entity: new Todo(), columns: [] }, }, - { provide: MatDialogRef, useValue: jasmine.createSpyObj(["close"]) }, + { + provide: MatDialogRef, + useValue: { close: () => {}, backdropClick: () => NEVER }, + }, TodoService, ], }).compileComponents(); From 0b5cf3d068e79c913fb2723c1adf0023508c7d55 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 24 May 2023 12:21:38 +0200 Subject: [PATCH 12/22] added tests for unsaved changes service --- .../form/unsaved-changes.service.spec.ts | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.spec.ts b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.spec.ts index e050b6514f..7b1ae5c08e 100644 --- a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.spec.ts +++ b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.spec.ts @@ -1,16 +1,39 @@ -import { TestBed } from '@angular/core/testing'; +import { TestBed } from "@angular/core/testing"; -import { UnsavedChangesService } from './unsaved-changes.service'; +import { UnsavedChangesService } from "./unsaved-changes.service"; +import { ConfirmationDialogService } from "../../../confirmation-dialog/confirmation-dialog.service"; -describe('UnsavedChangesService', () => { +describe("UnsavedChangesService", () => { let service: UnsavedChangesService; + let mockConfirmation: jasmine.SpyObj; beforeEach(() => { - TestBed.configureTestingModule({}); + mockConfirmation = jasmine.createSpyObj(["getSaveConfirmation"]); + TestBed.configureTestingModule({ + providers: [ + { provide: ConfirmationDialogService, useValue: mockConfirmation }, + ], + }); service = TestBed.inject(UnsavedChangesService); }); - it('should be created', () => { + it("should be created", () => { expect(service).toBeTruthy(); }); + + it("should only ask for confirmation if changes are pending", async () => { + mockConfirmation.getSaveConfirmation.and.resolveTo(false); + + await expectAsync(service.checkUnsavedChanges()).toBeResolvedTo(true); + expect(mockConfirmation.getSaveConfirmation).not.toHaveBeenCalled(); + + service.pending = true; + + await expectAsync(service.checkUnsavedChanges()).toBeResolvedTo(false); + expect(mockConfirmation.getSaveConfirmation).toHaveBeenCalled(); + + mockConfirmation.getSaveConfirmation.and.resolveTo(true); + + await expectAsync(service.checkUnsavedChanges()).toBeResolvedTo(true); + }); }); From d157083ff3cfacb2e377c82d3ec7d5e7557d91d3 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 19 Jun 2023 11:41:32 +0200 Subject: [PATCH 13/22] updated confirmation popup title --- .../confirmation-dialog/confirmation-dialog.service.ts | 6 +++--- .../form/unsaved-changes.service.spec.ts | 10 +++++----- .../entity-details/form/unsaved-changes.service.ts | 4 ++-- .../dialog-buttons/dialog-buttons.component.ts | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/app/core/confirmation-dialog/confirmation-dialog.service.ts b/src/app/core/confirmation-dialog/confirmation-dialog.service.ts index acb1834d43..f41fbea700 100644 --- a/src/app/core/confirmation-dialog/confirmation-dialog.service.ts +++ b/src/app/core/confirmation-dialog/confirmation-dialog.service.ts @@ -56,10 +56,10 @@ export class ConfirmationDialogService { ); } - getSaveConfirmation() { + getDiscardConfirmation() { return this.getConfirmation( - $localize`:Save changes header:Save Changes?`, - $localize`:Save changes message:You have unsaved changes. Do you really want to leave this page? All unsaved changes will be lost.` + $localize`:Discard changes header:Discard Changes?`, + $localize`:Discard changes message:You have unsaved changes. Do you really want to leave this page? All unsaved changes will be lost.` ); } } diff --git a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.spec.ts b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.spec.ts index 7b1ae5c08e..d3a3359666 100644 --- a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.spec.ts +++ b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.spec.ts @@ -8,7 +8,7 @@ describe("UnsavedChangesService", () => { let mockConfirmation: jasmine.SpyObj; beforeEach(() => { - mockConfirmation = jasmine.createSpyObj(["getSaveConfirmation"]); + mockConfirmation = jasmine.createSpyObj(["getDiscardConfirmation"]); TestBed.configureTestingModule({ providers: [ { provide: ConfirmationDialogService, useValue: mockConfirmation }, @@ -22,17 +22,17 @@ describe("UnsavedChangesService", () => { }); it("should only ask for confirmation if changes are pending", async () => { - mockConfirmation.getSaveConfirmation.and.resolveTo(false); + mockConfirmation.getDiscardConfirmation.and.resolveTo(false); await expectAsync(service.checkUnsavedChanges()).toBeResolvedTo(true); - expect(mockConfirmation.getSaveConfirmation).not.toHaveBeenCalled(); + expect(mockConfirmation.getDiscardConfirmation).not.toHaveBeenCalled(); service.pending = true; await expectAsync(service.checkUnsavedChanges()).toBeResolvedTo(false); - expect(mockConfirmation.getSaveConfirmation).toHaveBeenCalled(); + expect(mockConfirmation.getDiscardConfirmation).toHaveBeenCalled(); - mockConfirmation.getSaveConfirmation.and.resolveTo(true); + mockConfirmation.getDiscardConfirmation.and.resolveTo(true); await expectAsync(service.checkUnsavedChanges()).toBeResolvedTo(true); }); diff --git a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts index f1a4e47dd0..7d56d3794e 100644 --- a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts +++ b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts @@ -8,7 +8,7 @@ export class UnsavedChangesService { pending = false; constructor(private confirmation: ConfirmationDialogService) { - // prevent navigation if changes are pending + // prevent browser navigation if changes are pending window.onbeforeunload = (e) => { if (this.pending) { e.preventDefault(); @@ -19,7 +19,7 @@ export class UnsavedChangesService { async checkUnsavedChanges() { if (this.pending) { - const confirmed = await this.confirmation.getSaveConfirmation(); + const confirmed = await this.confirmation.getDiscardConfirmation(); if (confirmed) { this.pending = false; return true; diff --git a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts index 5913739a2f..4afff193d2 100644 --- a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts +++ b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts @@ -54,7 +54,7 @@ export class DialogButtonsComponent implements OnInit { this.dialog.backdropClick().subscribe(() => { if (this.form.dirty) { this.confirmation - .getSaveConfirmation() + .getDiscardConfirmation() .then((confirmed) => (confirmed ? this.save() : undefined)); } }); From 3ad617637933534f710b122af93a5c8f775eda2e Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 19 Jun 2023 11:45:30 +0200 Subject: [PATCH 14/22] disabling tabs if in edit mode --- .../entity-details/entity-details.component.html | 2 +- .../entity-details/entity-details.component.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/app/core/entity-components/entity-details/entity-details.component.html b/src/app/core/entity-components/entity-details/entity-details.component.html index 1045cef485..808257d9e0 100644 --- a/src/app/core/entity-components/entity-details/entity-details.component.html +++ b/src/app/core/entity-components/entity-details/entity-details.component.html @@ -60,7 +60,7 @@ ) => { this.config = data.config; From 54f43c3bbc4267221aa5b2fe181df762086c6432 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 19 Jun 2023 12:05:27 +0200 Subject: [PATCH 15/22] moved unsaved changes logic to form service --- .../entity-details/form/form.component.ts | 10 +------ .../entity-form/entity-form.service.ts | 22 ++++++++++++--- .../dialog-buttons.component.ts | 27 ++++++++++--------- 3 files changed, 34 insertions(+), 25 deletions(-) diff --git a/src/app/core/entity-components/entity-details/form/form.component.ts b/src/app/core/entity-components/entity-details/form/form.component.ts index b5e44dd80e..1f4895a71a 100644 --- a/src/app/core/entity-components/entity-details/form/form.component.ts +++ b/src/app/core/entity-components/entity-details/form/form.component.ts @@ -15,7 +15,6 @@ import { toFormFieldConfig } from "../../entity-subrecord/entity-subrecord/entit import { MatButtonModule } from "@angular/material/button"; import { EntityFormComponent } from "../../entity-form/entity-form/entity-form.component"; import { DisableEntityOperationDirective } from "../../../permissions/permission-directive/disable-entity-operation.directive"; -import { UnsavedChangesService } from "./unsaved-changes.service"; /** * A simple wrapper function of the EntityFormComponent which can be used as a dynamic component @@ -50,8 +49,7 @@ export class FormComponent implements OnInit { private router: Router, private location: Location, private entityFormService: EntityFormService, - private alertService: AlertService, - private unsavedChanges: UnsavedChangesService + private alertService: AlertService ) {} ngOnInit() { @@ -62,9 +60,6 @@ export class FormComponent implements OnInit { if (!this.creatingNew) { this.form.disable(); } - this.form.valueChanges.subscribe( - () => (this.unsavedChanges.pending = this.form.dirty) - ); } async saveClicked() { @@ -73,7 +68,6 @@ export class FormComponent implements OnInit { this.form.markAsPristine(); this.form.disable(); if (this.creatingNew) { - this.unsavedChanges.pending = false; await this.router.navigate([ getParentUrl(this.router), this.entity.getId(), @@ -92,7 +86,5 @@ export class FormComponent implements OnInit { } this.entityFormService.resetForm(this.form, this.entity); this.form.disable(); - // TODO this might overwrite active changes from another panel with a form - this.unsavedChanges.pending = false; } } diff --git a/src/app/core/entity-components/entity-form/entity-form.service.ts b/src/app/core/entity-components/entity-form/entity-form.service.ts index 99efa6ef34..d040a409f0 100644 --- a/src/app/core/entity-components/entity-form/entity-form.service.ts +++ b/src/app/core/entity-components/entity-form/entity-form.service.ts @@ -8,6 +8,7 @@ import { DynamicValidatorsService } from "./dynamic-form-validators/dynamic-vali import { EntityAbility } from "../../permissions/ability/entity-ability"; import { InvalidFormFieldError } from "./invalid-form-field.error"; import { omit } from "lodash-es"; +import { UnsavedChangesService } from "../entity-details/form/unsaved-changes.service"; /** * These are utility types that allow to define the type of `FormGroup` the way it is returned by `EntityFormService.create` @@ -26,7 +27,8 @@ export class EntityFormService { private entityMapper: EntityMapperService, private entitySchemaService: EntitySchemaService, private dynamicValidator: DynamicValidatorsService, - private ability: EntityAbility + private ability: EntityAbility, + private unsavedChanges: UnsavedChangesService ) {} /** @@ -106,7 +108,17 @@ export class EntityFormService { formConfig[formField.id].push(validators); } }); - return this.fb.group>(formConfig); + const formGroup = this.fb.group>(formConfig); + // TODO needs to be unsubscribed + formGroup.valueChanges.subscribe({ + next: () => (this.unsavedChanges.pending = formGroup.dirty), + complete: () => + console.log( + "complete form", + formFields.map(({ id }) => id) + ), + }); + return formGroup; } /** @@ -133,7 +145,10 @@ export class EntityFormService { return this.entityMapper .save(updatedEntity) - .then(() => Object.assign(entity, updatedEntity)) + .then(() => { + this.unsavedChanges.pending = false; + return Object.assign(entity, updatedEntity); + }) .catch((err) => { throw new Error($localize`Could not save ${entity.getType()}\: ${err}`); }); @@ -162,5 +177,6 @@ export class EntityFormService { const newKeys = Object.keys(omit(form.controls, Object.keys(entity))); newKeys.forEach((key) => form.get(key).setValue(null)); form.markAsPristine(); + this.unsavedChanges.pending = false; } } diff --git a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts index 4afff193d2..2efa4a7f5a 100644 --- a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts +++ b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts @@ -17,10 +17,9 @@ import { MatMenuModule } from "@angular/material/menu"; import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; import { Router, RouterLink } from "@angular/router"; import { EntityAbility } from "../../permissions/ability/entity-ability"; -import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; import { ConfirmationDialogService } from "../../confirmation-dialog/confirmation-dialog.service"; +import { UnsavedChangesService } from "../../entity-components/entity-details/form/unsaved-changes.service"; -@UntilDestroy() @Component({ selector: "app-dialog-buttons", standalone: true, @@ -49,15 +48,20 @@ export class DialogButtonsComponent implements OnInit { private entityRemoveService: EntityRemoveService, private router: Router, private ability: EntityAbility, - private confirmation: ConfirmationDialogService + private confirmation: ConfirmationDialogService, + private unsavedChanges: UnsavedChangesService ) { - this.dialog.backdropClick().subscribe(() => { - if (this.form.dirty) { - this.confirmation - .getDiscardConfirmation() - .then((confirmed) => (confirmed ? this.save() : undefined)); - } - }); + this.dialog.disableClose = true; + this.dialog.backdropClick().subscribe(() => + this.unsavedChanges.checkUnsavedChanges().then((confirmed) => { + if (confirmed) { + this.dialog.close(); + } + }) + ); + this.dialog + .afterClosed() + .subscribe(() => (this.unsavedChanges.pending = false)); } ngOnInit() { @@ -67,9 +71,6 @@ export class DialogButtonsComponent implements OnInit { } this.initializeDetailsRouteIfAvailable(); } - this.form.statusChanges - .pipe(untilDestroyed(this)) - .subscribe(() => (this.dialog.disableClose = this.form.dirty)); } private initializeDetailsRouteIfAvailable() { From afe98a299b01186d6e241d3f141221fbc0bdc173 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 19 Jun 2023 17:58:19 +0200 Subject: [PATCH 16/22] listening for changes only in the row details component --- .../entity-form/entity-form.service.ts | 12 +----------- .../row-details/row-details.component.ts | 8 +++++++- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/app/core/entity-components/entity-form/entity-form.service.ts b/src/app/core/entity-components/entity-form/entity-form.service.ts index d040a409f0..d6b44aaf70 100644 --- a/src/app/core/entity-components/entity-form/entity-form.service.ts +++ b/src/app/core/entity-components/entity-form/entity-form.service.ts @@ -108,17 +108,7 @@ export class EntityFormService { formConfig[formField.id].push(validators); } }); - const formGroup = this.fb.group>(formConfig); - // TODO needs to be unsubscribed - formGroup.valueChanges.subscribe({ - next: () => (this.unsavedChanges.pending = formGroup.dirty), - complete: () => - console.log( - "complete form", - formFields.map(({ id }) => id) - ), - }); - return formGroup; + return this.fb.group>(formConfig); } /** diff --git a/src/app/core/entity-components/entity-subrecord/row-details/row-details.component.ts b/src/app/core/entity-components/entity-subrecord/row-details/row-details.component.ts index e9f1a53799..423b1b4653 100644 --- a/src/app/core/entity-components/entity-subrecord/row-details/row-details.component.ts +++ b/src/app/core/entity-components/entity-subrecord/row-details/row-details.component.ts @@ -14,6 +14,7 @@ import { DynamicComponentDirective } from "../../../view/dynamic-components/dyna import { MatTooltipModule } from "@angular/material/tooltip"; import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; import { DialogButtonsComponent } from "../../../form-dialog/dialog-buttons/dialog-buttons.component"; +import { UnsavedChangesService } from "../../entity-details/form/unsaved-changes.service"; /** * Data interface that must be given when opening the dialog @@ -56,9 +57,14 @@ export class RowDetailsComponent { constructor( @Inject(MAT_DIALOG_DATA) public data: DetailsComponentData, - private formService: EntityFormService + private formService: EntityFormService, + private unsavedChanges: UnsavedChangesService ) { this.form = this.formService.createFormGroup(data.columns, data.entity); + // TODO this would make more sense inside the service but can't be unsubscribed there. That might not be a problem though + this.form.valueChanges + .pipe(untilDestroyed(this)) + .subscribe(() => (this.unsavedChanges.pending = this.form.dirty)); this.columns = data.columns.map((col) => [col]); this.tempEntity = this.data.entity; this.form.valueChanges.pipe(untilDestroyed(this)).subscribe((value) => { From c798a15cb0c39b7a61dcee7e97b5e8b4e8b35f47 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 20 Jun 2023 10:33:31 +0200 Subject: [PATCH 17/22] moved unsubscription back to form service --- .../form/unsaved-changes.service.ts | 12 +++++++++ .../entity-form/entity-form.service.ts | 26 ++++++++++++++++--- .../row-details/row-details.component.ts | 8 +----- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts index 7d56d3794e..a4287f81f5 100644 --- a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts +++ b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.ts @@ -1,10 +1,19 @@ import { Injectable } from "@angular/core"; import { ConfirmationDialogService } from "../../../confirmation-dialog/confirmation-dialog.service"; +/** + * This service handles the state whether there are currently some unsaved changes in the app. + * These pending changes might come from a form component or popup. + * If there are pending changes, certain actions in the app should trigger a user confirmation if the changes should be discarded. + */ @Injectable({ providedIn: "root", }) export class UnsavedChangesService { + /** + * Set to true if the user has pending changes that are not yet saved. + * Set to false once the changes have been saved or discarded. + */ pending = false; constructor(private confirmation: ConfirmationDialogService) { @@ -17,6 +26,9 @@ export class UnsavedChangesService { }; } + /** + * Shows a user confirmation popup if there are unsaved changes which will be discarded. + */ async checkUnsavedChanges() { if (this.pending) { const confirmed = await this.confirmation.getDiscardConfirmation(); diff --git a/src/app/core/entity-components/entity-form/entity-form.service.ts b/src/app/core/entity-components/entity-form/entity-form.service.ts index d6b44aaf70..32782d4051 100644 --- a/src/app/core/entity-components/entity-form/entity-form.service.ts +++ b/src/app/core/entity-components/entity-form/entity-form.service.ts @@ -9,6 +9,9 @@ import { EntityAbility } from "../../permissions/ability/entity-ability"; import { InvalidFormFieldError } from "./invalid-form-field.error"; import { omit } from "lodash-es"; import { UnsavedChangesService } from "../entity-details/form/unsaved-changes.service"; +import { ActivationStart, Router } from "@angular/router"; +import { Subscription } from "rxjs"; +import { filter } from "rxjs/operators"; /** * These are utility types that allow to define the type of `FormGroup` the way it is returned by `EntityFormService.create` @@ -22,14 +25,26 @@ export type EntityForm = TypedForm>; */ @Injectable({ providedIn: "root" }) export class EntityFormService { + subscriptions: Subscription[] = []; + constructor( private fb: FormBuilder, private entityMapper: EntityMapperService, private entitySchemaService: EntitySchemaService, private dynamicValidator: DynamicValidatorsService, private ability: EntityAbility, - private unsavedChanges: UnsavedChangesService - ) {} + private unsavedChanges: UnsavedChangesService, + router: Router + ) { + router.events + .pipe(filter((e) => e instanceof ActivationStart)) + .subscribe(() => { + // Clean up everything once navigation happens + this.subscriptions.forEach((sub) => sub.unsubscribe()); + this.subscriptions = []; + this.unsavedChanges.pending = false; + }); + } /** * Uses schema information to fill missing fields in the FormFieldConfig. @@ -108,7 +123,12 @@ export class EntityFormService { formConfig[formField.id].push(validators); } }); - return this.fb.group>(formConfig); + const group = this.fb.group>(formConfig); + const sub = group.valueChanges.subscribe( + () => (this.unsavedChanges.pending = group.dirty) + ); + this.subscriptions.push(sub); + return group; } /** diff --git a/src/app/core/entity-components/entity-subrecord/row-details/row-details.component.ts b/src/app/core/entity-components/entity-subrecord/row-details/row-details.component.ts index 423b1b4653..e9f1a53799 100644 --- a/src/app/core/entity-components/entity-subrecord/row-details/row-details.component.ts +++ b/src/app/core/entity-components/entity-subrecord/row-details/row-details.component.ts @@ -14,7 +14,6 @@ import { DynamicComponentDirective } from "../../../view/dynamic-components/dyna import { MatTooltipModule } from "@angular/material/tooltip"; import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; import { DialogButtonsComponent } from "../../../form-dialog/dialog-buttons/dialog-buttons.component"; -import { UnsavedChangesService } from "../../entity-details/form/unsaved-changes.service"; /** * Data interface that must be given when opening the dialog @@ -57,14 +56,9 @@ export class RowDetailsComponent { constructor( @Inject(MAT_DIALOG_DATA) public data: DetailsComponentData, - private formService: EntityFormService, - private unsavedChanges: UnsavedChangesService + private formService: EntityFormService ) { this.form = this.formService.createFormGroup(data.columns, data.entity); - // TODO this would make more sense inside the service but can't be unsubscribed there. That might not be a problem though - this.form.valueChanges - .pipe(untilDestroyed(this)) - .subscribe(() => (this.unsavedChanges.pending = this.form.dirty)); this.columns = data.columns.map((col) => [col]); this.tempEntity = this.data.entity; this.form.valueChanges.pipe(untilDestroyed(this)).subscribe((value) => { From 07733ec94db03b6633fc634571dfdb3a71fb9142 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 20 Jun 2023 15:22:36 +0200 Subject: [PATCH 18/22] added comment --- .../core/form-dialog/dialog-buttons/dialog-buttons.component.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts index 2efa4a7f5a..44963421a5 100644 --- a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts +++ b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.ts @@ -59,6 +59,7 @@ export class DialogButtonsComponent implements OnInit { } }) ); + // This happens before the `canDeactivate` check and therefore does not warn when leaving this.dialog .afterClosed() .subscribe(() => (this.unsavedChanges.pending = false)); From 9882892137322aacba9cf8aff2bc4185cc5d53c6 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 20 Jun 2023 15:30:23 +0200 Subject: [PATCH 19/22] fixed tests --- .../notes/note-details/note-details.component.spec.ts | 5 ++++- .../entity-form/entity-form.service.spec.ts | 2 ++ .../row-details/row-details.component.spec.ts | 5 ++++- .../dialog-buttons/dialog-buttons.component.spec.ts | 7 ++++--- .../todos/todo-details/todo-details.component.spec.ts | 6 +++++- 5 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/app/child-dev-project/notes/note-details/note-details.component.spec.ts b/src/app/child-dev-project/notes/note-details/note-details.component.spec.ts index 7659d44d34..1dfd9040eb 100644 --- a/src/app/child-dev-project/notes/note-details/note-details.component.spec.ts +++ b/src/app/child-dev-project/notes/note-details/note-details.component.spec.ts @@ -43,7 +43,10 @@ describe("NoteDetailsComponent", () => { ], providers: [ { provide: MAT_DIALOG_DATA, useValue: { entity: testNote } }, - { provide: MatDialogRef, useValue: { backdropClick: () => NEVER } }, + { + provide: MatDialogRef, + useValue: { backdropClick: () => NEVER, afterClosed: () => NEVER }, + }, ], }).compileComponents(); diff --git a/src/app/core/entity-components/entity-form/entity-form.service.spec.ts b/src/app/core/entity-components/entity-form/entity-form.service.spec.ts index 8e0c3ea09b..ff29e9ee83 100644 --- a/src/app/core/entity-components/entity-form/entity-form.service.spec.ts +++ b/src/app/core/entity-components/entity-form/entity-form.service.spec.ts @@ -14,6 +14,7 @@ import { School } from "../../../child-dev-project/schools/model/school"; import { ChildSchoolRelation } from "../../../child-dev-project/children/model/childSchoolRelation"; import { EntityAbility } from "../../permissions/ability/entity-ability"; import { InvalidFormFieldError } from "./invalid-form-field.error"; +import { MatDialogModule } from "@angular/material/dialog"; describe("EntityFormService", () => { let service: EntityFormService; @@ -23,6 +24,7 @@ describe("EntityFormService", () => { mockEntityMapper = jasmine.createSpyObj(["save"]); mockEntityMapper.save.and.resolveTo(); TestBed.configureTestingModule({ + imports: [MatDialogModule], providers: [ FormBuilder, EntitySchemaService, diff --git a/src/app/core/entity-components/entity-subrecord/row-details/row-details.component.spec.ts b/src/app/core/entity-components/entity-subrecord/row-details/row-details.component.spec.ts index bc9a620894..2993c63045 100644 --- a/src/app/core/entity-components/entity-subrecord/row-details/row-details.component.spec.ts +++ b/src/app/core/entity-components/entity-subrecord/row-details/row-details.component.spec.ts @@ -24,7 +24,10 @@ describe("RowDetailsComponent", () => { imports: [RowDetailsComponent, MockedTestingModule.withState()], providers: [ { provide: MAT_DIALOG_DATA, useValue: detailsComponentData }, - { provide: MatDialogRef, useValue: { backdropClick: () => NEVER } }, + { + provide: MatDialogRef, + useValue: { backdropClick: () => NEVER, afterClosed: () => NEVER }, + }, ], }).compileComponents(); spyOn(TestBed.inject(EntityAbility), "cannot").and.returnValue(true); diff --git a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.spec.ts b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.spec.ts index 01612f1a2a..908f46688c 100644 --- a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.spec.ts +++ b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.spec.ts @@ -18,7 +18,7 @@ import { EntityRemoveService, RemoveResult, } from "../../entity/entity-remove.service"; -import { EMPTY, of } from "rxjs"; +import { NEVER, of } from "rxjs"; describe("DialogButtonsComponent", () => { let component: DialogButtonsComponent; @@ -26,8 +26,9 @@ describe("DialogButtonsComponent", () => { let dialogRef: jasmine.SpyObj>; beforeEach(async () => { - dialogRef = jasmine.createSpyObj(["close", "backdropClick"]); - dialogRef.backdropClick.and.returnValue(EMPTY); + dialogRef = jasmine.createSpyObj(["close", "backdropClick", "afterClosed"]); + dialogRef.backdropClick.and.returnValue(NEVER); + dialogRef.afterClosed.and.returnValue(NEVER); await TestBed.configureTestingModule({ imports: [DialogButtonsComponent, MockedTestingModule.withState()], providers: [{ provide: MatDialogRef, useValue: dialogRef }], diff --git a/src/app/features/todos/todo-details/todo-details.component.spec.ts b/src/app/features/todos/todo-details/todo-details.component.spec.ts index 1b2a56db6a..18a57bf978 100644 --- a/src/app/features/todos/todo-details/todo-details.component.spec.ts +++ b/src/app/features/todos/todo-details/todo-details.component.spec.ts @@ -28,7 +28,11 @@ describe("TodoDetailsComponent", () => { }, { provide: MatDialogRef, - useValue: { close: () => {}, backdropClick: () => NEVER }, + useValue: { + close: () => {}, + backdropClick: () => NEVER, + afterClosed: () => NEVER, + }, }, TodoService, ], From 6b32a64add2e21bb026c74101c5a267154610d88 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 20 Jun 2023 16:55:13 +0200 Subject: [PATCH 20/22] added further tests --- .../form/unsaved-changes.service.spec.ts | 16 ++++++ .../entity-form/entity-form.service.spec.ts | 51 ++++++++++++++++++- .../entity-form/entity-form.service.ts | 2 +- .../dialog-buttons.component.spec.ts | 36 +++++++++++-- 4 files changed, 100 insertions(+), 5 deletions(-) diff --git a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.spec.ts b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.spec.ts index d3a3359666..cfe73071ba 100644 --- a/src/app/core/entity-components/entity-details/form/unsaved-changes.service.spec.ts +++ b/src/app/core/entity-components/entity-details/form/unsaved-changes.service.spec.ts @@ -36,4 +36,20 @@ describe("UnsavedChangesService", () => { await expectAsync(service.checkUnsavedChanges()).toBeResolvedTo(true); }); + + it("should prevent closing the window if changes are pending", () => { + const e = { preventDefault: jasmine.createSpy(), returnValue: undefined }; + + service.pending = false; + window.onbeforeunload(e as any); + + expect(e.preventDefault).not.toHaveBeenCalled(); + expect(e.returnValue).toBeUndefined(); + + service.pending = true; + window.onbeforeunload(e as any); + + expect(e.preventDefault).toHaveBeenCalled(); + expect(e.returnValue).toBe("onbeforeunload"); + }); }); diff --git a/src/app/core/entity-components/entity-form/entity-form.service.spec.ts b/src/app/core/entity-components/entity-form/entity-form.service.spec.ts index ff29e9ee83..316ff44002 100644 --- a/src/app/core/entity-components/entity-form/entity-form.service.spec.ts +++ b/src/app/core/entity-components/entity-form/entity-form.service.spec.ts @@ -15,6 +15,9 @@ import { ChildSchoolRelation } from "../../../child-dev-project/children/model/c import { EntityAbility } from "../../permissions/ability/entity-ability"; import { InvalidFormFieldError } from "./invalid-form-field.error"; import { MatDialogModule } from "@angular/material/dialog"; +import { UnsavedChangesService } from "../entity-details/form/unsaved-changes.service"; +import { Router } from "@angular/router"; +import { NotFoundComponent } from "../../view/dynamic-routing/not-found/not-found.component"; describe("EntityFormService", () => { let service: EntityFormService; @@ -109,7 +112,6 @@ describe("EntityFormService", () => { it("should create a error if form is invalid", () => { const formFields = [{ id: "schoolId" }, { id: "start" }]; - service.extendFormFieldConfig(formFields, ChildSchoolRelation); const formGroup = service.createFormGroup( formFields, new ChildSchoolRelation() @@ -119,4 +121,51 @@ describe("EntityFormService", () => { service.saveChanges(formGroup, new ChildSchoolRelation()) ).toBeRejectedWith(jasmine.any(InvalidFormFieldError)); }); + + it("should set pending changes once a form is edited and reset it once saved or canceled", async () => { + const formFields = [{ id: "inactive" }]; + const formGroup = service.createFormGroup(formFields, new Entity()); + const unsavedChanges = TestBed.inject(UnsavedChangesService); + + formGroup.markAsDirty(); + formGroup.get("inactive").setValue(true); + expect(unsavedChanges.pending).toBeTrue(); + + TestBed.inject(EntityAbility).update([ + { action: "manage", subject: "all" }, + ]); + await service.saveChanges(formGroup, new Entity()); + + expect(unsavedChanges.pending).toBeFalse(); + + formGroup.markAsDirty(); + formGroup.get("inactive").setValue(true); + expect(unsavedChanges.pending).toBeTrue(); + + await service.resetForm(formGroup, new Entity()); + + expect(unsavedChanges.pending).toBeFalse(); + }); + + it("should reset state once navigation happens", async () => { + const router = TestBed.inject(Router); + router.resetConfig([{ path: "test", component: NotFoundComponent }]); + const unsavedChanged = TestBed.inject(UnsavedChangesService); + const formFields = [{ id: "inactive" }]; + const formGroup = service.createFormGroup(formFields, new Entity()); + formGroup.markAsDirty(); + formGroup.get("inactive").setValue(true); + + expect(unsavedChanged.pending).toBeTrue(); + + await router.navigate(["test"]); + + expect(unsavedChanged.pending).toBeFalse(); + + // Changes are not listened to anymore + formGroup.markAsDirty(); + formGroup.get("inactive").setValue(true); + + expect(unsavedChanged.pending).toBeFalse(); + }); }); diff --git a/src/app/core/entity-components/entity-form/entity-form.service.ts b/src/app/core/entity-components/entity-form/entity-form.service.ts index 32782d4051..be650962cf 100644 --- a/src/app/core/entity-components/entity-form/entity-form.service.ts +++ b/src/app/core/entity-components/entity-form/entity-form.service.ts @@ -25,7 +25,7 @@ export type EntityForm = TypedForm>; */ @Injectable({ providedIn: "root" }) export class EntityFormService { - subscriptions: Subscription[] = []; + private subscriptions: Subscription[] = []; constructor( private fb: FormBuilder, diff --git a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.spec.ts b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.spec.ts index 28b53cc313..53108d14cb 100644 --- a/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.spec.ts +++ b/src/app/core/form-dialog/dialog-buttons/dialog-buttons.component.spec.ts @@ -19,17 +19,20 @@ import { EntityRemoveService, RemoveResult, } from "../../entity/entity-remove.service"; -import { NEVER, of } from "rxjs"; +import { firstValueFrom, of, Subject } from "rxjs"; +import { UnsavedChangesService } from "../../entity-components/entity-details/form/unsaved-changes.service"; describe("DialogButtonsComponent", () => { let component: DialogButtonsComponent; let fixture: ComponentFixture; let dialogRef: jasmine.SpyObj>; + let backdropClick = new Subject(); + let closed = new Subject(); beforeEach(waitForAsync(() => { dialogRef = jasmine.createSpyObj(["close", "backdropClick", "afterClosed"]); - dialogRef.backdropClick.and.returnValue(NEVER); - dialogRef.afterClosed.and.returnValue(NEVER); + dialogRef.backdropClick.and.returnValue(backdropClick); + dialogRef.afterClosed.and.returnValue(closed); TestBed.configureTestingModule({ imports: [DialogButtonsComponent, MockedTestingModule.withState()], providers: [{ provide: MatDialogRef, useValue: dialogRef }], @@ -102,4 +105,31 @@ describe("DialogButtonsComponent", () => { expect(dialogRef.close).toHaveBeenCalled(); }); + + it("should only close the dialog if user confirms to discard changes", fakeAsync(() => { + const confirmed = new Subject(); + spyOn( + TestBed.inject(UnsavedChangesService), + "checkUnsavedChanges" + ).and.returnValue(firstValueFrom(confirmed)); + + backdropClick.next(undefined); + tick(); + + expect(dialogRef.close).not.toHaveBeenCalled(); + + confirmed.next(true); + tick(); + + expect(dialogRef.close).toHaveBeenCalled(); + })); + + it("should reset pending changes when dialog is closed", () => { + const unsavedChanges = TestBed.inject(UnsavedChangesService); + unsavedChanges.pending = true; + + closed.next(); + + expect(unsavedChanges.pending).toBeFalse(); + }); }); From 2c4459d039e87b999030339235b50f80b8b80999 Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 22 Jun 2023 10:12:57 +0200 Subject: [PATCH 21/22] removed unneeded await --- .../notes/note-details/note-details.stories.ts | 7 +++++-- .../entity-form/entity-form.service.spec.ts | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/app/child-dev-project/notes/note-details/note-details.stories.ts b/src/app/child-dev-project/notes/note-details/note-details.stories.ts index b7b8763b87..e708848052 100644 --- a/src/app/child-dev-project/notes/note-details/note-details.stories.ts +++ b/src/app/child-dev-project/notes/note-details/note-details.stories.ts @@ -5,7 +5,7 @@ import { Note } from "../model/note"; import { Child } from "../../children/model/child"; import { MAT_DIALOG_DATA, MatDialogRef } from "@angular/material/dialog"; import { ChildrenService } from "../../children/children.service"; -import { of } from "rxjs"; +import { NEVER, of } from "rxjs"; import { MockedTestingModule } from "../../../utils/mocked-testing.module"; import { StorybookBaseModule } from "../../../utils/storybook-base.module"; @@ -26,7 +26,10 @@ export default { provide: MAT_DIALOG_DATA, useValue: { data: { entity: Note.create(new Date()) } }, }, - { provide: MatDialogRef, useValue: {} }, + { + provide: MatDialogRef, + useValue: { backdropClick: () => NEVER, afterClosed: () => NEVER }, + }, { provide: ChildrenService, useValue: { getChild: () => of(Child.create("John Doe")) }, diff --git a/src/app/core/entity-components/entity-form/entity-form.service.spec.ts b/src/app/core/entity-components/entity-form/entity-form.service.spec.ts index 316ff44002..2bf9f0c50c 100644 --- a/src/app/core/entity-components/entity-form/entity-form.service.spec.ts +++ b/src/app/core/entity-components/entity-form/entity-form.service.spec.ts @@ -142,7 +142,7 @@ describe("EntityFormService", () => { formGroup.get("inactive").setValue(true); expect(unsavedChanges.pending).toBeTrue(); - await service.resetForm(formGroup, new Entity()); + service.resetForm(formGroup, new Entity()); expect(unsavedChanges.pending).toBeFalse(); }); From 61277cbb7375f168a787c62cc497c3bd605270fb Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 22 Jun 2023 10:18:16 +0200 Subject: [PATCH 22/22] fixed todo details story --- src/app/features/todos/todo-details/todo-details.stories.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/app/features/todos/todo-details/todo-details.stories.ts b/src/app/features/todos/todo-details/todo-details.stories.ts index d255fbbb16..ea4250a897 100644 --- a/src/app/features/todos/todo-details/todo-details.stories.ts +++ b/src/app/features/todos/todo-details/todo-details.stories.ts @@ -9,6 +9,7 @@ import { Todo } from "../model/todo"; import { EntityMapperService } from "../../../core/entity/entity-mapper.service"; import { mockEntityMapper } from "../../../core/entity/mock-entity-mapper-service"; import { FormFieldConfig } from "../../../core/entity-components/entity-form/entity-form/FormConfig"; +import { NEVER } from "rxjs"; const defaultColumns: FormFieldConfig[] = [ { id: "deadline" }, @@ -34,7 +35,10 @@ export default { provide: MAT_DIALOG_DATA, useValue: { entity: todoEntity, columns: defaultColumns }, }, - { provide: MatDialogRef, useValue: {} }, + { + provide: MatDialogRef, + useValue: { backdropClick: () => NEVER, afterClosed: () => NEVER }, + }, { provide: TodoService, useValue: {},