refactor(core): simplify entity-remove service

in preparation of implementing anonymization (#1674)

---------
This functionality has been developed for the project “codo”.
codo is developed under the projects “Landungsbrücken – Patenschaften in Hamburg stärken” and “openTransfer Patenschaften”. It is funded through the program “Menschen stärken Menschen” by the German Federal Ministry of Family Affairs, Senior Citizens, Women and Youth.
More information at https://github.com/codo-mentoring

“Landungsbrücken – Patenschaften in Hamburg stärken” is a project of BürgerStiftung Hamburg in cooperation with the Mentor.Ring Hamburg. With a mix of networking opportunities, capacity building and financial support the project strengthens Hamburg’s scene of mentoring projects since its founding in 2016.

The “Stiftung Bürgermut” foundation since 2007 supports the digital and real exchange of experiences and connections of active citizens. Within the federal program “Menschen stärken Menschen” the foundation as part of its program “openTransfer Patenschaften” offers support services for connecting, spreading and upskilling mentoring organisations across Germany.

Diese Funktion wurde entwickelt für das Projekt codo.
codo wird entwickelt im Rahmen der Projekte Landungsbrücken – Patenschaften in Hamburg stärken und openTransfer Patenschaften. Er ist gefördert durch das Bundesprogramm Menschen stärken Menschen des Bundesministeriums für Familie, Senioren, Frauen und Jugend.
Mehr Informationen unter https://github.com/codo-mentoring

“Landungsbrücken – Patenschaften in Hamburg stärken” ist ein Projekt der BürgerStiftung Hamburg in Kooperation mit dem Mentor.Ring Hamburg. Mit einer Mischung aus Vernetzungsangeboten, Qualifizierungsmaßnahmen und finanzieller Förderung stärkt das Projekt die Hamburger Szene der Patenschaftsprojekte seit der Gründung im Jahr 2016.

Die Stiftung Bürgermut fördert seit 2007 den digitalen und realen Erfahrungsaustausch und die Vernetzung von engagierten Bürger:innen. Innerhalb des Bundesprogramms „Menschen stärken Menschen” bietet die Stiftung im Rahmen ihres Programms openTransfer Patenschaften Unterstützungsleistungen zur Vernetzung, Verbreitung und Qualifizierung von Patenschafts- und Mentoringorganisationen bundesweit.

Co-authored-by: codo-mentoring <117934638+codo-mentoring@users.noreply.github.com>

src/app/core/common-components/entity-subrecord/entity-subrecord/entity-subrecord.component.html

@@ -110,7 +110,7 @@
                 entity: rec.record,
                 operation: 'delete'
               }"
-              (click)="delete(rec)"
+              (click)="entityRemoveService.remove(rec.record)"
               angulartics2On="click"
               [angularticsCategory]="rec?.record?.getType()"
               angularticsAction="subrecord_inlineedit_delete"

src/app/core/common-components/entity-subrecord/entity-subrecord/entity-subrecord.component.ts

@@ -24,10 +24,7 @@ import {
 } from "../../entity-form/entity-form.service";
 import { LoggingService } from "../../../logging/logging.service";
 import { AnalyticsService } from "../../../analytics/analytics.service";
-import {
-  EntityRemoveService,
-  RemoveResult,
-} from "../../../entity/entity-remove.service";
+import { EntityRemoveService } from "../../../entity/entity-remove.service";
 import { EntityMapperService } from "../../../entity/entity-mapper/entity-mapper.service";
 import { tableSort } from "./table-sort";
 import {
@@ -166,7 +163,7 @@ export class EntitySubrecordComponent<T extends Entity> implements OnChanges {
     private router: Router,
     private analyticsService: AnalyticsService,
     private loggingService: LoggingService,
-    private entityRemoveService: EntityRemoveService,
+    public entityRemoveService: EntityRemoveService,
     private entityMapper: EntityMapperService,
     private filterService: FilterService,
   ) {
@@ -388,28 +385,6 @@ export class EntitySubrecordComponent<T extends Entity> implements OnChanges {
     row.formGroup = null;
   }
 
-  /**
-   * Delete the given entity from the database (after explicit user confirmation).
-   * @param row The entity to be deleted.
-   */
-  delete(row: TableRow<T>) {
-    this.entityRemoveService
-      .remove(row.record, {
-        deletedEntityInformation: $localize`:Record deleted info:Record deleted`,
-        dialogText: $localize`:Delete confirmation message:Are you sure you want to delete this record?`,
-      })
-      .subscribe((result) => {
-        switch (result) {
-          case RemoveResult.REMOVED:
-            this.removeFromDataTable(row.record);
-            break;
-          case RemoveResult.UNDONE:
-            this.records.unshift(row.record);
-            this.initFormGroups();
-        }
-      });
-  }
-
   private removeFromDataTable(deleted: T) {
     // use setter so datasource is also updated
     this.records = this.records.filter(

src/app/core/entity-details/entity-details/entity-details.component.html

@@ -41,7 +41,7 @@
 <mat-menu #additional>
   <button
     mat-menu-item
-    (click)="removeEntity()"
+    (click)="entityRemoveService.remove(entity, true)"
     *appDisabledEntityOperation="{
       entity: entity,
       operation: 'delete'

src/app/core/entity-details/entity-details/entity-details.component.spec.ts

@@ -12,10 +12,7 @@ import { EntityDetailsConfig, PanelConfig } from "../EntityDetailsConfig";
 import { Child } from "../../../child-dev-project/children/model/child";
 import { ChildrenService } from "../../../child-dev-project/children/children.service";
 import { MockedTestingModule } from "../../../utils/mocked-testing.module";
-import {
-  EntityRemoveService,
-  RemoveResult,
-} from "../../entity/entity-remove.service";
+import { EntityRemoveService } from "../../entity/entity-remove.service";
 import { EntityAbility } from "../../permissions/ability/entity-ability";
 import { EntityMapperService } from "../../entity/entity-mapper/entity-mapper.service";
 
@@ -129,32 +126,6 @@ describe("EntityDetailsComponent", () => {
     expect(component.isLoading).toBeFalse();
   }));
 
-  it("should navigate back when deleting an entity", fakeAsync(() => {
-    const mockRemoveResult = of(RemoveResult.REMOVED);
-    mockEntityRemoveService.remove.and.returnValue(mockRemoveResult);
-    component.entity = new Child("Test-Child");
-    // @ts-ignore
-    const routerNavigateSpy = spyOn(component.router, "navigate");
-
-    component.removeEntity();
-    tick();
-
-    expect(routerNavigateSpy).toHaveBeenCalled();
-  }));
-
-  it("should route back when deleting is undone", fakeAsync(() => {
-    const mockResult = of(RemoveResult.REMOVED, RemoveResult.UNDONE);
-    mockEntityRemoveService.remove.and.returnValue(mockResult);
-    component.entity = new Child("Test-Child");
-    const router = fixture.debugElement.injector.get(Router);
-    spyOn(router, "navigate");
-
-    component.removeEntity();
-    tick();
-
-    expect(router.navigate).toHaveBeenCalled();
-  }));
-
   it("should call router when user is not permitted to create entities", () => {
     mockAbility.cannot.and.returnValue(true);
     const router = fixture.debugElement.injector.get(Router);

src/app/core/entity-details/entity-details/entity-details.component.ts

@@ -8,13 +8,9 @@ import {
 } from "../EntityDetailsConfig";
 import { Entity, EntityConstructor } from "../../entity/model/entity";
 import { EntityMapperService } from "../../entity/entity-mapper/entity-mapper.service";
-import { getUrlWithoutParams } from "../../../utils/utils";
 import { RouteData } from "../../config/dynamic-routing/view-config.interface";
 import { AnalyticsService } from "../../analytics/analytics.service";
-import {
-  EntityRemoveService,
-  RemoveResult,
-} from "../../entity/entity-remove.service";
+import { EntityRemoveService } from "../../entity/entity-remove.service";
 import { EntityAbility } from "../../permissions/ability/entity-ability";
 import { RouteTarget } from "../../../app.routing";
 import { EntityRegistry } from "../../entity/database-entity.decorator";
@@ -75,7 +71,7 @@ export class EntityDetailsComponent {
     private route: ActivatedRoute,
     private router: Router,
     private analyticsService: AnalyticsService,
-    private entityRemoveService: EntityRemoveService,
+    public entityRemoveService: EntityRemoveService,
     private ability: EntityAbility,
     private entities: EntityRegistry,
     private logger: LoggingService,
@@ -152,20 +148,6 @@ export class EntityDetailsComponent {
     return panelConfig;
   }
 
-  removeEntity() {
-    const currentUrl = getUrlWithoutParams(this.router);
-    const parentUrl = currentUrl.substring(0, currentUrl.lastIndexOf("/"));
-    this.entityRemoveService.remove(this.entity).subscribe(async (result) => {
-      switch (result) {
-        case RemoveResult.REMOVED:
-          await this.router.navigate([parentUrl]);
-          break;
-        case RemoveResult.UNDONE:
-          await this.router.navigate([currentUrl]);
-      }
-    });
-  }
-
   trackTabChanged(index: number) {
     this.analyticsService.eventTrack("details_tab_changed", {
       category: this.config?.entity,

src/app/core/entity/entity-remove.service.spec.ts

@@ -1,5 +1,5 @@
-import { TestBed } from "@angular/core/testing";
-import { EntityRemoveService, RemoveResult } from "./entity-remove.service";
+import { fakeAsync, TestBed, tick } from "@angular/core/testing";
+import { EntityRemoveService } from "./entity-remove.service";
 import { EntityMapperService } from "./entity-mapper/entity-mapper.service";
 import {
   MatSnackBar,
@@ -9,15 +9,16 @@ import {
 } from "@angular/material/snack-bar";
 import { ConfirmationDialogService } from "../common-components/confirmation-dialog/confirmation-dialog.service";
 import { Entity } from "./model/entity";
-import { NEVER, Observable } from "rxjs";
-import { toArray } from "rxjs/operators";
+import { NEVER, Observable, Subject } from "rxjs";
+import { Router } from "@angular/router";
 
 describe("EntityRemoveService", () => {
   let service: EntityRemoveService;
   let mockEntityMapper: jasmine.SpyObj<EntityMapperService>;
   let snackBarSpy: jasmine.SpyObj<MatSnackBar>;
   let mockSnackBarRef: jasmine.SpyObj<MatSnackBarRef<TextOnlySnackBar>>;
   let mockConfirmationDialog: jasmine.SpyObj<ConfirmationDialogService>;
+  let mockRouter;
 
   beforeEach(() => {
     mockEntityMapper = jasmine.createSpyObj(["remove", "save"]);
@@ -31,80 +32,66 @@ describe("EntityRemoveService", () => {
       providers: [
         { provide: EntityMapperService, useValue: mockEntityMapper },
         { provide: MatSnackBar, useValue: snackBarSpy },
+        Router,
         {
           provide: ConfirmationDialogService,
           useValue: mockConfirmationDialog,
         },
       ],
     });
-    service = TestBed.inject(EntityRemoveService);
-  });
+    mockRouter = TestBed.inject(Router);
+    spyOn(mockRouter, "navigate");
 
-  it("should be created", () => {
-    expect(service).toBeTruthy();
+    service = TestBed.inject(EntityRemoveService);
   });
 
-  it("emits once and closes when the user has cancelled", (done) => {
+  it("should return false when user cancels confirmation", async () => {
     mockConfirmationDialog.getConfirmation.and.resolveTo(false);
-    service
-      .remove(new Entity())
-      .pipe(toArray())
-      .subscribe({
-        next: (next) => {
-          expect(next).toEqual([RemoveResult.CANCELLED]);
-        },
-        complete: () => {
-          expect(snackBarSpy.open).not.toHaveBeenCalled();
-          expect(mockEntityMapper.remove).not.toHaveBeenCalled();
-          done();
-        },
-      });
+
+    const result = await service.remove(new Entity());
+
+    expect(result).toBe(false);
+    expect(snackBarSpy.open).not.toHaveBeenCalled();
+    expect(mockEntityMapper.remove).not.toHaveBeenCalled();
   });
 
-  it("deletes the entity and finishes if the action is never undone", (done) => {
+  it("should delete entity, show snackbar confirmation and navigate back", async () => {
     // onAction is never called
     mockSnackBarRef.onAction.and.returnValues(NEVER);
     // mock that dialog is dismissed immediately
     const afterDismissed = new Observable<MatSnackBarDismiss>((subscriber) =>
       subscriber.next({} as MatSnackBarDismiss),
     );
     mockSnackBarRef.afterDismissed.and.returnValue(afterDismissed);
-    service
-      .remove(new Entity())
-      .pipe(toArray())
-      .subscribe({
-        next: (next) => {
-          expect(next).toEqual([RemoveResult.REMOVED]);
-        },
-        complete: () => {
-          expect(snackBarSpy.open).toHaveBeenCalled();
-          expect(mockEntityMapper.remove).toHaveBeenCalled();
-          done();
-        },
-      });
+
+    const result = await service.remove(new Entity(), true);
+
+    expect(result).toBe(true);
+    expect(snackBarSpy.open).toHaveBeenCalled();
+    expect(mockEntityMapper.remove).toHaveBeenCalled();
+    expect(mockRouter.navigate).toHaveBeenCalled();
   });
 
-  it("emits twice when an entity was deleted and the user pressed undo", (done) => {
+  it("should re-save entity and navigate back to entity on undo", fakeAsync(() => {
+    const entity = new Entity();
+
     // Mock a snackbar where 'undo' is immediately pressed
-    const onSnackbarAction = new Observable<void>((subscriber) =>
-      subscriber.next(),
-    );
-    mockSnackBarRef.onAction.and.returnValue(onSnackbarAction);
+    const onSnackbarAction = new Subject<void>();
+    mockSnackBarRef.onAction.and.returnValue(onSnackbarAction.asObservable());
     mockSnackBarRef.afterDismissed.and.returnValue(NEVER);
+
     mockEntityMapper.save.and.resolveTo();
-    const entity = new Entity();
-    service
-      .remove(entity)
-      .pipe(toArray())
-      .subscribe({
-        next: (next) => {
-          expect(next).toEqual([RemoveResult.REMOVED, RemoveResult.UNDONE]);
-        },
-        complete: () => {
-          expect(mockEntityMapper.remove).toHaveBeenCalled();
-          expect(mockEntityMapper.save).toHaveBeenCalledWith(entity, true);
-          done();
-        },
-      });
-  });
+
+    service.remove(entity, true);
+    tick();
+
+    mockRouter.navigate.calls.reset();
+    onSnackbarAction.next();
+    onSnackbarAction.complete();
+    tick();
+
+    expect(mockEntityMapper.remove).toHaveBeenCalled();
+    expect(mockEntityMapper.save).toHaveBeenCalledWith(entity, true);
+    expect(mockRouter.navigate).toHaveBeenCalled();
+  }));
 });