FIX: Delete Value UI Bug (#171)

* added logs to figure out what's going on * fix (resource-viewer): fixed bug where a recently updated value will not be removed from the UI when deleted * fix (e2e): fixed e2e tests * refactor (resource-view): value operation event subscription is now an array and the subscriptions are added to it * make the subscription array variable plural... * refactor (resource-view): split the updateResource method into three smaller methods that are easier to understand * fix (resource-view): added a new method in the value-type.service to check if the objectType of a PropertyDefinition is equal to one of the readonly strings defined in the value-type.service for text values * added comment for clarification on why we need to get the DOM element again after updating a value * refactor (value operation event service): added subclasses for each value operation
dasch-swiss · Sep 14, 2020 · c2dff4d · c2dff4d
1 parent a8a1ddb
commit c2dff4d
Show file tree

Hide file tree

Showing 9 changed files with 302 additions and 128 deletions.
diff --git a/e2e/src/app.e2e-spec.ts b/e2e/src/app.e2e-spec.ts
@@ -37,7 +37,7 @@ describe('Test App', () => {
 
             await page.navigateTo('modify');
 
-            const valueEleComp: WebElement = await page.getComponentBySelector('dsp-int-value', timeout);
+            let valueEleComp: WebElement = await page.getComponentBySelector('dsp-int-value', timeout);
 
             const displayEditComp: WebElement = await page.getDisplayEditComponentFromValueComponent(valueEleComp);
 
@@ -64,6 +64,10 @@ describe('Test App', () => {
             await browser.wait(EC.presenceOf(element(by.css('.rm-value'))), timeout,
                 'Wait for read value to be visible.');
 
+            // a new element is created in the DOM when we update a value
+            // therefore we need to get a reference to the element again, otherwise it will be stale
+            valueEleComp = await page.getComponentBySelector('dsp-int-value', timeout);
+
             const readEle = await page.getReadValueFieldFromValueComponent(valueEleComp);
             expect(await readEle.getText()).toEqual('3');
 

diff --git a/projects/dsp-ui/src/lib/viewer/operations/add-value/add-value.component.spec.ts b/projects/dsp-ui/src/lib/viewer/operations/add-value/add-value.component.spec.ts
@@ -19,7 +19,7 @@ import {
 import { of, throwError } from 'rxjs';
 import { AjaxError } from 'rxjs/ajax';
 import { DspApiConnectionToken } from '../../../core';
-import { EmitEvent, Events, ValueOperationEventService } from '../../services/value-operation-event.service';
+import { AddedEventValue, EmitEvent, Events, ValueOperationEventService } from '../../services/value-operation-event.service';
 import { AddValueComponent } from './add-value.component';
 
 
@@ -283,7 +283,7 @@ describe('AddValueComponent', () => {
             expect(valuesSpy.v2.values.getValue).toHaveBeenCalledWith(testHostComponent.readResource.id, 'uuid');
 
             expect(valueEventSpy.emit).toHaveBeenCalledTimes(1);
-            expect(valueEventSpy.emit).toHaveBeenCalledWith(new EmitEvent(Events.ValueAdded, newReadValue));
+            expect(valueEventSpy.emit).toHaveBeenCalledWith(new EmitEvent(Events.ValueAdded, new AddedEventValue(newReadValue)));
 
         });
 

diff --git a/projects/dsp-ui/src/lib/viewer/operations/add-value/add-value.component.ts b/projects/dsp-ui/src/lib/viewer/operations/add-value/add-value.component.ts
@@ -18,7 +18,7 @@ import {
 } from '@dasch-swiss/dsp-js';
 import { mergeMap } from 'rxjs/operators';
 import { DspApiConnectionToken } from '../../../core/core.module';
-import { EmitEvent, Events, ValueOperationEventService } from '../../services/value-operation-event.service';
+import { AddedEventValue, EmitEvent, Events, ValueOperationEventService } from '../../services/value-operation-event.service';
 import { ValueTypeService } from '../../services/value-type.service';
 import { BaseValueComponent } from '../../values/base-value.component';
 
@@ -110,7 +110,8 @@ export class AddValueComponent implements OnInit, AfterViewInit {
                     // emit a ValueAdded event to the listeners in:
                     // property-view component to hide the add value form
                     // resource-view component to trigger a refresh of the resource
-                    this._valueOperationEventService.emit(new EmitEvent(Events.ValueAdded, res2.getValues(updateRes.property)[0]));
+                    this._valueOperationEventService.emit(
+                        new EmitEvent(Events.ValueAdded, new AddedEventValue(res2.getValues(updateRes.property)[0])));
 
                     // hide the progress indicator
                     this.submittingValue = false;

diff --git a/projects/dsp-ui/src/lib/viewer/operations/display-edit/display-edit.component.spec.ts b/projects/dsp-ui/src/lib/viewer/operations/display-edit/display-edit.component.spec.ts
@@ -39,12 +39,16 @@ import {
 import { of, throwError } from 'rxjs';
 import { AjaxError } from 'rxjs/ajax';
 import { DspApiConnectionToken } from '../../../core';
-import { EmitEvent, Events, ValueOperationEventService } from '../../services/value-operation-event.service';
+import {
+    DeletedEventValue,
+    EmitEvent,
+    Events,
+    UpdatedEventValues,
+    ValueOperationEventService
+} from '../../services/value-operation-event.service';
 import { ValueTypeService } from '../../services/value-type.service';
 import { DisplayEditComponent } from './display-edit.component';
 
-
-
 @Component({
   selector: `dsp-text-value-as-string`,
   template: ``
@@ -578,79 +582,88 @@ describe('DisplayEditComponent', () => {
 
     it('should save a new version of a value', () => {
 
-      const valuesSpy = TestBed.inject(DspApiConnectionToken);
+        const valueEventSpy = TestBed.inject(ValueOperationEventService);
 
-      (valuesSpy.v2.values as jasmine.SpyObj<ValuesEndpointV2>).updateValue.and.callFake(
-        () => {
+        const valuesSpy = TestBed.inject(DspApiConnectionToken);
 
-          const response = new WriteValueResponse();
+        (valueEventSpy as jasmine.SpyObj<ValueOperationEventService>).emit.and.stub();
 
-          response.id = 'newID';
-          response.type = 'type';
-          response.uuid = 'uuid';
+        (valuesSpy.v2.values as jasmine.SpyObj<ValuesEndpointV2>).updateValue.and.callFake(
+            () => {
 
-          return of(response);
-        }
-      );
+                const response = new WriteValueResponse();
 
-      (valuesSpy.v2.values as jasmine.SpyObj<ValuesEndpointV2>).getValue.and.callFake(
-        () => {
+                response.id = 'newID';
+                response.type = 'type';
+                response.uuid = 'uuid';
 
-          const updatedVal = new ReadIntValue();
+                return of(response);
+            }
+        );
 
-          updatedVal.id = 'newID';
-          updatedVal.int = 1;
+        (valuesSpy.v2.values as jasmine.SpyObj<ValuesEndpointV2>).getValue.and.callFake(
+            () => {
 
-          const resource = new ReadResource();
+                const updatedVal = new ReadIntValue();
 
-          resource.properties = {
-            'http://0.0.0.0:3333/ontology/0001/anything/v2#hasInteger': [updatedVal]
-          };
+                updatedVal.id = 'newID';
+                updatedVal.int = 1;
 
-          return of(resource);
-        }
-      );
+                const resource = new ReadResource();
 
-      testHostComponent.displayEditValueComponent.canModify = true;
-      testHostComponent.displayEditValueComponent.editModeActive = true;
-      testHostComponent.displayEditValueComponent.mode = 'update';
+                resource.properties = {
+                    'http://0.0.0.0:3333/ontology/0001/anything/v2#hasInteger': [updatedVal]
+                };
 
-      testHostComponent.displayEditValueComponent.displayValueComponent.form.controls.test.clearValidators();
-      testHostComponent.displayEditValueComponent.displayValueComponent.form.controls.test.updateValueAndValidity();
+                return of(resource);
+            }
+        );
 
-      testHostFixture.detectChanges();
+        testHostComponent.displayEditValueComponent.canModify = true;
+        testHostComponent.displayEditValueComponent.editModeActive = true;
+        testHostComponent.displayEditValueComponent.mode = 'update';
 
-      const saveButtonDebugElement = displayEditComponentDe.query(By.css('button.save'));
-      const saveButtonNativeElement = saveButtonDebugElement.nativeElement;
+        testHostComponent.displayEditValueComponent.displayValueComponent.form.controls.test.clearValidators();
+        testHostComponent.displayEditValueComponent.displayValueComponent.form.controls.test.updateValueAndValidity();
 
-      expect(saveButtonNativeElement.disabled).toBeFalsy();
+        testHostFixture.detectChanges();
 
-      saveButtonNativeElement.click();
+        const saveButtonDebugElement = displayEditComponentDe.query(By.css('button.save'));
+        const saveButtonNativeElement = saveButtonDebugElement.nativeElement;
 
-      testHostFixture.detectChanges();
+        expect(saveButtonNativeElement.disabled).toBeFalsy();
 
-      const expectedUpdateResource = new UpdateResource();
+        saveButtonNativeElement.click();
 
-      expectedUpdateResource.id = 'http://rdfh.ch/0001/H6gBWUuJSuuO-CilHV8kQw';
-      expectedUpdateResource.type = 'http://0.0.0.0:3333/ontology/0001/anything/v2#Thing';
-      expectedUpdateResource.property = 'http://0.0.0.0:3333/ontology/0001/anything/v2#hasInteger';
+        testHostFixture.detectChanges();
 
-      const expectedUpdateVal = new UpdateIntValue();
-      expectedUpdateVal.id = 'http://rdfh.ch/0001/H6gBWUuJSuuO-CilHV8kQw/values/dJ1ES8QTQNepFKF5-EAqdg';
-      expectedUpdateVal.int = 1;
+        const expectedUpdateResource = new UpdateResource();
 
-      expectedUpdateResource.value = expectedUpdateVal;
+        expectedUpdateResource.id = 'http://rdfh.ch/0001/H6gBWUuJSuuO-CilHV8kQw';
+        expectedUpdateResource.type = 'http://0.0.0.0:3333/ontology/0001/anything/v2#Thing';
+        expectedUpdateResource.property = 'http://0.0.0.0:3333/ontology/0001/anything/v2#hasInteger';
 
-      expect(valuesSpy.v2.values.updateValue).toHaveBeenCalledWith(expectedUpdateResource);
-      expect(valuesSpy.v2.values.updateValue).toHaveBeenCalledTimes(1);
+        const expectedUpdateVal = new UpdateIntValue();
+        expectedUpdateVal.id = 'http://rdfh.ch/0001/H6gBWUuJSuuO-CilHV8kQw/values/dJ1ES8QTQNepFKF5-EAqdg';
+        expectedUpdateVal.int = 1;
+
+        expectedUpdateResource.value = expectedUpdateVal;
+
+        expect(valuesSpy.v2.values.updateValue).toHaveBeenCalledWith(expectedUpdateResource);
+        expect(valuesSpy.v2.values.updateValue).toHaveBeenCalledTimes(1);
+
+        expect(valueEventSpy.emit).toHaveBeenCalledTimes(1);
+        expect(valueEventSpy.emit).toHaveBeenCalledWith(new EmitEvent(Events.ValueUpdated, new UpdatedEventValues(
+            testHostComponent.readValue, testHostComponent.displayEditValueComponent.displayValue)));
+
+        expect(valuesSpy.v2.values.getValue).toHaveBeenCalledTimes(1);
+        expect(valuesSpy.v2.values.getValue).toHaveBeenCalledWith(testHostComponent.readResource.id, 'uuid');
+
+        expect(testHostComponent.displayEditValueComponent.displayValue.id).toEqual('newID');
+        expect(testHostComponent.displayEditValueComponent.displayValueComponent.displayValue.id).toEqual('newID');
+        expect(testHostComponent.displayEditValueComponent.mode).toEqual('read');
 
-      expect(valuesSpy.v2.values.getValue).toHaveBeenCalledTimes(1);
-      expect(valuesSpy.v2.values.getValue).toHaveBeenCalledWith(testHostComponent.readResource.id,
-        'uuid');
 
-      expect(testHostComponent.displayEditValueComponent.displayValue.id).toEqual('newID');
-      expect(testHostComponent.displayEditValueComponent.displayValueComponent.displayValue.id).toEqual('newID');
-      expect(testHostComponent.displayEditValueComponent.mode).toEqual('read');
 
     });
 
@@ -743,8 +756,6 @@ describe('DisplayEditComponent', () => {
     });
 
     it('should display a comment button if the value has a comment', () => {
-      //console.log(testHostComponent.displayEditValueComponent.displayValueComponent.displayValue);
-
       expect(testHostComponent.displayEditValueComponent.editModeActive).toBeFalsy();
       expect(testHostComponent.displayEditValueComponent.shouldShowCommentToggle).toBeTruthy()
 
@@ -888,15 +899,6 @@ describe('DisplayEditComponent', () => {
             }
         );
 
-        // const deleteButtonDebugElement = displayEditComponentDe.query(By.css('button.delete'));
-        // const deleteButtonNativeElement = deleteButtonDebugElement.nativeElement;
-
-        // expect(deleteButtonNativeElement.disabled).toBeFalsy();
-
-        // deleteButtonNativeElement.click();
-
-        // testHostFixture.detectChanges();
-
         const deleteButton = await rootLoader.getHarness(MatButtonHarness.with({selector: '.delete'}));
         await deleteButton.click();
 
@@ -925,7 +927,7 @@ describe('DisplayEditComponent', () => {
             expect(valuesSpy.v2.values.deleteValue).toHaveBeenCalledTimes(1);
 
             expect(valueEventSpy.emit).toHaveBeenCalledTimes(1);
-            expect(valueEventSpy.emit).toHaveBeenCalledWith(new EmitEvent(Events.ValueDeleted, deleteVal));
+            expect(valueEventSpy.emit).toHaveBeenCalledWith(new EmitEvent(Events.ValueDeleted, new DeletedEventValue(deleteVal)));
         });
 
     });

diff --git a/projects/dsp-ui/src/lib/viewer/operations/display-edit/display-edit.component.ts b/projects/dsp-ui/src/lib/viewer/operations/display-edit/display-edit.component.ts
@@ -20,7 +20,13 @@ import {
     ConfirmationDialogData
 } from '../../../action/components/confirmation-dialog/confirmation-dialog.component';
 import { DspApiConnectionToken } from '../../../core/core.module';
-import { EmitEvent, Events, ValueOperationEventService } from '../../services/value-operation-event.service';
+import {
+    DeletedEventValue,
+    EmitEvent,
+    Events,
+    UpdatedEventValues,
+    ValueOperationEventService
+} from '../../services/value-operation-event.service';
 import { ValueTypeService } from '../../services/value-type.service';
 import { BaseValueComponent } from '../../values/base-value.component';
 
@@ -152,6 +158,10 @@ export class DisplayEditComponent implements OnInit {
                 })
             ).subscribe(
                 (res2: ReadResource) => {
+                    this._valueOperationEventService.emit(
+                        new EmitEvent(Events.ValueUpdated, new UpdatedEventValues(
+                            this.displayValue, res2.getValues(this.displayValue.property)[0])));
+
                     this.displayValue = res2.getValues(this.displayValue.property)[0];
                     this.mode = 'read';
 
@@ -218,7 +228,7 @@ export class DisplayEditComponent implements OnInit {
         this._dspApiConnection.v2.values.deleteValue(updateRes as UpdateResource<DeleteValue>).pipe(
         mergeMap((res: DeleteValueResponse) => {
             // emit a ValueDeleted event to the listeners in resource-view component to trigger an update of the UI
-            this._valueOperationEventService.emit(new EmitEvent(Events.ValueDeleted, deleteVal));
+            this._valueOperationEventService.emit(new EmitEvent(Events.ValueDeleted, new DeletedEventValue(deleteVal)));
             return res.result;
         })).subscribe();
     }

diff --git a/projects/dsp-ui/src/lib/viewer/services/value-operation-event.service.ts b/projects/dsp-ui/src/lib/viewer/services/value-operation-event.service.ts
@@ -1,4 +1,4 @@
-import { BaseValue } from '@dasch-swiss/dsp-js';
+import { DeleteValue, ReadValue } from '@dasch-swiss/dsp-js';
 import { Subject, Subscription } from 'rxjs';
 import { filter, map } from 'rxjs/operators';
 
@@ -17,7 +17,7 @@ export class ValueOperationEventService {
 
     // Used in the listening component.
     // i.e. this.valueOperationEventSubscription = this._valueOperationEventService.on(Events.ValueAdded, () => doSomething());
-    on(event: Events, action: (newValue) => void): Subscription {
+    on(event: Events, action: (value: EventValue) => void): Subscription {
         return this._subject$
             .pipe(
                 // Filter down based on event name to any events that are emitted out of the subject from the emit method below.
@@ -28,18 +28,39 @@ export class ValueOperationEventService {
     }
 
     // Used in the emitting component.
-    // i.e. this.valueOperationEventService.emit(new EmitEvent(Events.ValueAdded));
+    // i.e. this.valueOperationEventService.emit(new EmitEvent(Events.ValueAdded, new EventValues(new ReadValue()));
     emit(event: EmitEvent) {
         this._subject$.next(event);
     }
 }
 
 export class EmitEvent {
-    constructor(public name: any, public value?: BaseValue) { }
+    constructor(public name: Events, public value?: EventValue) { }
 }
 
 // Possible events that can be emitted.
 export enum Events {
     ValueAdded,
-    ValueDeleted
+    ValueDeleted,
+    ValueUpdated
+}
+
+export abstract class EventValue { }
+
+export class AddedEventValue extends EventValue {
+    constructor(public addedValue: ReadValue) {
+        super();
+    }
+}
+
+export class UpdatedEventValues extends EventValue {
+    constructor(public currentValue: ReadValue, public updatedValue: ReadValue) {
+        super();
+    }
+}
+
+export class DeletedEventValue extends EventValue {
+    constructor(public deletedValue: DeleteValue) {
+        super();
+    }
 }
diff --git a/projects/dsp-ui/src/lib/viewer/services/value-type.service.ts b/projects/dsp-ui/src/lib/viewer/services/value-type.service.ts
@@ -62,6 +62,20 @@ export class ValueTypeService {
         }
     }
 
+    /**
+     * Given the ObjectType of a PropertyDefinition, compares it to the type of the type of the provided value.
+     * Primarily used to check if a TextValue type is equal to one of the readonly strings in this class.
+     *
+     * @param objectType PropertyDefinition ObjectType
+     * @param valueType Value type (ReadValue, DeleteValue, BaseValue, etc.)
+     */
+    compareObjectTypeWithValueType(objectType: string, valueType: string): boolean {
+        return objectType === this._readTextValueAsString ||
+                objectType === this._readTextValueAsHtml ||
+                objectType === this._readTextValueAsXml ||
+                objectType === valueType;
+    }
+
     /**
      * Equality checks with constants below are TEMPORARY until component is implemented.
      * Used so that the CRUD buttons do not show if a property doesn't have a value component.