fix: better automatic resolution of remote changes while editing forms (

#1688)

Co-authored-by: Simon <simon@aam-digital.com>

src/app/core/entity-components/entity-form/entity-form/entity-form.component.spec.ts

@@ -13,6 +13,10 @@ describe("EntityFormComponent", () => {
 
   let mockConfirmation: jasmine.SpyObj<ConfirmationDialogService>;
 
+  const testColumns = [
+    [{ id: "name" }, { id: "projectNumber" }, { id: "photo" }],
+  ];
+
   beforeEach(waitForAsync(() => {
     mockConfirmation = jasmine.createSpyObj(["getConfirmation"]);
     TestBed.configureTestingModule({
@@ -26,17 +30,20 @@ describe("EntityFormComponent", () => {
   beforeEach(() => {
     fixture = TestBed.createComponent(EntityFormComponent<Child>);
     component = fixture.componentInstance;
-    component.entity = new Child();
-    component.columns = [
-      [{ id: "name" }, { id: "projectNumber" }, { id: "photo" }],
-    ];
+
+    setupInitialForm(new Child(), testColumns);
+  });
+
+  function setupInitialForm(entity, columns) {
+    component.entity = entity;
+    component.columns = columns;
     component.form = TestBed.inject(EntityFormService).createFormGroup(
       component.columns[0],
       component.entity
     );
     component.ngOnChanges({ entity: true, form: true } as any);
     fixture.detectChanges();
-  });
+  }
 
   it("should create", () => {
     expect(component).toBeTruthy();
@@ -45,9 +52,10 @@ describe("EntityFormComponent", () => {
   it("should not change anything if changed entity has same values as form", () => {
     return expectApplyChangesPopup(
       "not-shown",
+      { _rev: "0" },
       { name: "updated" },
-      { name: "updated" },
-      { name: "updated" }
+      { name: "updated", _rev: "1" },
+      { name: "updated", _rev: "1" }
     );
   });
 
@@ -56,6 +64,7 @@ describe("EntityFormComponent", () => {
     const remoteValues = { name: "changed" };
     await expectApplyChangesPopup(
       "yes",
+      {},
       formValues,
       remoteValues,
       remoteValues
@@ -65,43 +74,67 @@ describe("EntityFormComponent", () => {
   it("should not overwrite form if user declines it", async () => {
     const formValues = { name: "other" };
     const remoteValues = { name: "changed" };
-    await expectApplyChangesPopup("no", formValues, remoteValues, formValues);
+    await expectApplyChangesPopup(
+      "no",
+      {},
+      formValues,
+      remoteValues,
+      formValues
+    );
   });
 
   it("should overwrite without popup for changes affecting untouched fields", async () => {
-    const formValues = { projectNumber: "other" };
-    const remoteValues = { name: "changed", _rev: "new rev" };
-    await expectApplyChangesPopup("not-shown", formValues, remoteValues, {
-      projectNumber: "other",
+    const originalEntity = { projectNumber: "p1" };
+    const formValues = { projectNumber: "p2" };
+    const remoteValues = {
       name: "changed",
+      projectNumber: "p1",
       _rev: "new rev",
-    });
+    };
+    await expectApplyChangesPopup(
+      "not-shown",
+      originalEntity,
+      formValues,
+      remoteValues,
+      {
+        projectNumber: "p2",
+        name: "changed",
+        _rev: "new rev",
+      }
+    );
   });
 
   async function expectApplyChangesPopup(
     popupAction: "not-shown" | "yes" | "no",
+    originalEntity: Partial<Child>,
     formChanges: Partial<Child>,
     remoteChanges: Partial<Child>,
     expectedFormValues: Partial<Child>
   ) {
+    setupInitialForm(Object.assign(new Child(), originalEntity), testColumns);
+
     mockConfirmation.getConfirmation.and.resolveTo(popupAction === "yes");
     for (const c in formChanges) {
       component.form.get(c).setValue(formChanges[c]);
       component.form.get(c).markAsDirty();
     }
     const updatedChild = new Child(component.entity.getId());
-    for (const c in remoteChanges) {
-      updatedChild[c] = remoteChanges[c];
-    }
+    Object.assign(updatedChild, remoteChanges);
 
     const entityMapper = TestBed.inject(EntityMapperService);
     await entityMapper.save(updatedChild);
 
-    for (const v in expectedFormValues) {
-      const form = component.form.get(v);
+    const entityAfterSave = Object.assign(
+      {},
+      component.entity,
+      component.form.getRawValue()
+    );
+    for (const [key, value] of Object.entries(expectedFormValues)) {
+      const form = component.form.get(key);
       if (form) {
-        expect(form).toHaveValue(expectedFormValues[v]);
+        expect(form).toHaveValue(value);
       }
+      expect(entityAfterSave[key]).toEqual(value);
     }
     expect(mockConfirmation.getConfirmation.calls.any()).toBe(
       popupAction !== "not-shown"

src/app/core/entity-components/entity-form/entity-form/entity-form.component.ts

@@ -17,6 +17,7 @@ import { DynamicComponentDirective } from "../../../view/dynamic-components/dyna
 import { MatButtonModule } from "@angular/material/button";
 import { MatTooltipModule } from "@angular/material/tooltip";
 import { FontAwesomeModule } from "@fortawesome/angular-fontawesome";
+import { Subscription } from "rxjs";
 
 /**
  * A general purpose form component for displaying and editing entities.
@@ -65,7 +66,8 @@ export class EntityFormComponent<T extends Entity = Entity>
    */
   @Input() gridLayout = true;
 
-  initialFormValues: any;
+  private initialFormValues: any;
+  private changesSubscription: Subscription;
 
   constructor(
     private entityMapper: EntityMapperService,
@@ -74,7 +76,8 @@ export class EntityFormComponent<T extends Entity = Entity>
 
   ngOnChanges(changes: SimpleChanges) {
     if (changes.entity && this.entity) {
-      this.entityMapper
+      this.changesSubscription?.unsubscribe();
+      this.changesSubscription = this.entityMapper
         .receiveUpdates(this.entity.getConstructor())
         .pipe(
           filter(({ entity }) => entity.getId() === this.entity.getId()),
@@ -89,7 +92,7 @@ export class EntityFormComponent<T extends Entity = Entity>
 
   private async applyChanges(entity: T) {
     if (this.formIsUpToDate(entity)) {
-      // this is the component that currently saves the values -> no need to apply changes.
+      Object.assign(this.entity, entity as any);
       return;
     }
     if (
@@ -101,6 +104,7 @@ export class EntityFormComponent<T extends Entity = Entity>
     ) {
       Object.assign(this.initialFormValues, entity);
       this.form.patchValue(entity as any);
+      Object.assign(this.entity, entity as any);
     }
   }