Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: Delete Value UI Bug #171

Merged
merged 13 commits into from Sep 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion e2e/src/app.e2e-spec.ts
Expand Up @@ -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);

Expand All @@ -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.');

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a comment that this is necessary because the DOM element has been re-created

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added in 4768a6b

// 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');

Expand Down
Expand Up @@ -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';


Expand Down Expand Up @@ -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)));

});

Expand Down
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
Expand Down
Expand Up @@ -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: ``
Expand Down Expand Up @@ -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');

});

Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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)));
});

});
Expand Down
Expand Up @@ -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';

Expand Down Expand Up @@ -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';

Expand Down Expand Up @@ -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();
}
Expand Down
@@ -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';

Expand All @@ -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.
Expand All @@ -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();
}
}
14 changes: 14 additions & 0 deletions projects/dsp-ui/src/lib/viewer/services/value-type.service.ts
Expand Up @@ -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 ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for the late notice but I think there is some missing conditions:

(valueType === Constants.TextValue && objectType === this._readTextValueAsString) || ...

Otherwise text values are always included.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in #175

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.
Expand Down