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.

Sign up for GitHub

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

Jump to bottom

FIX: display-edit dev build errors #106

Merged
merged 9 commits into from Jun 15, 2020
Merged

FIX: display-edit dev build errors #106

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2ea88b6
refactor (display-edit): first not-so-elegant way of fixing dev build…
mdelez Jun 11, 2020
ecd48c4
fix (display-edit): fixed a bug I introduced while fixing another bug
mdelez Jun 11, 2020
3d3a0c5
tests (operations): add assertions to display edit comp.
Jun 11, 2020
ff8561d
formatting
mdelez Jun 11, 2020
184194d
Revert "refactor (display-edit): first not-so-elegant way of fixing d…
mdelez Jun 11, 2020
61ad3d3
revert (display-edit): revert to original logic
mdelez Jun 11, 2020
2f6f721
fix (display-edit): fixed incorrect member name
mdelez Jun 11, 2020
c029d91
fix (tests): display-edit unit tests now check for an instance of the…
mdelez Jun 12, 2020
2a89cfb
reworded a comment
mdelez Jun 12, 2020
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
31 changes: 16 additions & 15 deletions projects/dsp-ui/src/lib/viewer/operations/display-edit/display-edit.component.html
View file
Open in desktop
@@ -1,7 +1,7 @@
<div class="grid-container">
<div class="property-header">
<h4 class="label">
{{displayValue.propertyLabel}}
{{displayValue.propertyLabel}}
</h4>
<button mat-button
class="info"
Expand All @@ -11,22 +11,23 @@ <h4 class="label">
</div>
<div class="value-component">
<span [ngSwitch]="valueTypeOrClass">
<dsp-text-value-as-string class="parent-value-component" #displayVal *ngSwitchCase="'ReadTextValueAsString'" [mode]="mode" [displayValue]="$any(displayValue)"></dsp-text-value-as-string>
<dsp-text-value-as-html class="parent-value-component" #displayVal *ngSwitchCase="'ReadTextValueAsHtml'" [mode]="mode" [displayValue]=$any(displayValue)></dsp-text-value-as-html>
<dsp-int-value class="parent-value-component" #displayVal *ngSwitchCase="constants.IntValue" [mode]="mode" [displayValue]=$any(displayValue)></dsp-int-value>
<dsp-boolean-value class="parent-value-component" #displayVal *ngSwitchCase="constants.BooleanValue" [mode]="mode" [displayValue]=$any(displayValue)></dsp-boolean-value>
<dsp-uri-value class="parent-value-component" #displayVal *ngSwitchCase="constants.UriValue" [mode]="mode" [displayValue]=$any(displayValue)></dsp-uri-value>
<dsp-decimal-value class="parent-value-component" #displayVal *ngSwitchCase="constants.DecimalValue" [mode]="mode" [displayValue]=$any(displayValue)></dsp-decimal-value>
<dsp-color-value class="parent-value-component" #displayVal *ngSwitchCase="constants.ColorValue" [mode]="mode" [displayValue]=$any(displayValue)></dsp-color-value>
<dsp-interval-value class="parent-value-component" #displayVal *ngSwitchCase="constants.IntervalValue" [mode]="mode" [displayValue]=$any(displayValue)></dsp-interval-value>
<dsp-time-value class="parent-value-component" #displayVal *ngSwitchCase="constants.TimeValue" [mode]="mode" [displayValue]=$any(displayValue)></dsp-time-value>
<dsp-geoname-value class="parent-value-component" #displayVal *ngSwitchCase="constants.GeonameValue" [mode]="mode" [displayValue]=$any(displayValue)></dsp-geoname-value>
<dsp-link-value class="parent-value-component" #displayVal *ngSwitchCase="constants.LinkValue" [mode]="mode" [displayValue]=$any(displayValue)
<!-- display value is cast as 'any' because the compiler cannot infer the value type of the child component -->
Copy link
Contributor

Choose a reason for hiding this comment

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

"because the compiler cannot infer the value type of the child component" -> because the compiler cannot infer the value type expected by the child component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, that's clearer. Fixed in 2a89cfb

<dsp-text-value-as-string class="parent-value-component" #displayVal *ngSwitchCase="'ReadTextValueAsString'" [mode]="mode" [displayValue]="$any(displayValue)"></dsp-text-value-as-string>
<dsp-text-value-as-html class="parent-value-component" #displayVal *ngSwitchCase="'ReadTextValueAsHtml'" [mode]="mode" [displayValue]="$any(displayValue)"></dsp-text-value-as-html>
<dsp-int-value class="parent-value-component" #displayVal *ngSwitchCase="constants.IntValue" [mode]="mode" [displayValue]="$any(displayValue)"></dsp-int-value>
<dsp-boolean-value class="parent-value-component" #displayVal *ngSwitchCase="constants.BooleanValue" [mode]="mode" [displayValue]="$any(displayValue)"></dsp-boolean-value>
<dsp-uri-value class="parent-value-component" #displayVal *ngSwitchCase="constants.UriValue" [mode]="mode" [displayValue]="$any(displayValue)"></dsp-uri-value>
<dsp-decimal-value class="parent-value-component" #displayVal *ngSwitchCase="constants.DecimalValue" [mode]="mode" [displayValue]="$any(displayValue)"></dsp-decimal-value>
<dsp-color-value class="parent-value-component" #displayVal *ngSwitchCase="constants.ColorValue" [mode]="mode" [displayValue]="$any(displayValue)"></dsp-color-value>
<dsp-interval-value class="parent-value-component" #displayVal *ngSwitchCase="constants.IntervalValue" [mode]="mode" [displayValue]="$any(displayVal)"></dsp-interval-value>
Copy link
Contributor

Choose a reason for hiding this comment

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

$any(displayVal) ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Fixed in 2f6f721

Copy link
Contributor

Choose a reason for hiding this comment

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

#displayVal is actually a variable, this is why this compiled. But since it is wrapped with $any(), no compatibility check was performed.

https://angular.io/guide/template-syntax#template-reference-variables-var

Copy link
Contributor

Choose a reason for hiding this comment

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

I wondered why the unit tests passed and checked the test in display-edit.component.spec:

it('should choose the apt component for an interval value in the template', () => {

      testHostComponent.assignValue('http://0.0.0.0:3333/ontology/0001/anything/v2#hasInterval');
      testHostFixture.detectChanges();

      expect(testHostComponent.displayEditValueComponent.displayValueComponent instanceof TestIntervalValueComponent).toBe(true);
      expect(testHostComponent.displayEditValueComponent.displayValueComponent.displayValue).not.toBeUndefined();
      expect(testHostComponent.displayEditValueComponent.displayValueComponent.mode).toEqual('read');
    });

expect(testHostComponent.displayEditValueComponent.displayValueComponent.displayValue).not.toBeUndefined(); should actually check for an instance of a ReadIntervalValue. That would make sure that the template correctly passed the ReadIntervalValue to the ReadIntervalValueComponent. The same applies to all other value types.

Shall we make a DSP task for this or do it in this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

in case of $any(displayVal) a reference to TestIntervalValueComponent was passed instead of the ReadIn tervalValue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wondered why the unit tests passed and checked the test in display-edit.component.spec:

it('should choose the apt component for an interval value in the template', () => {

      testHostComponent.assignValue('http://0.0.0.0:3333/ontology/0001/anything/v2#hasInterval');
      testHostFixture.detectChanges();

      expect(testHostComponent.displayEditValueComponent.displayValueComponent instanceof TestIntervalValueComponent).toBe(true);
      expect(testHostComponent.displayEditValueComponent.displayValueComponent.displayValue).not.toBeUndefined();
      expect(testHostComponent.displayEditValueComponent.displayValueComponent.mode).toEqual('read');
    });

expect(testHostComponent.displayEditValueComponent.displayValueComponent.displayValue).not.toBeUndefined(); should actually check for an instance of a ReadIntervalValue. That would make sure that the template correctly passed the ReadIntervalValue to the ReadIntervalValueComponent. The same applies to all other value types.

Shall we make a DSP task for this or do it in this PR?

Good find! Since not much has changed in this PR in the end, I'm okay making the changes in this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screen Shot 2020-06-12 at 09 25 19

The test now correctly fails when using the wrong variable in the template

<dsp-time-value class="parent-value-component" #displayVal *ngSwitchCase="constants.TimeValue" [mode]="mode" [displayValue]="$any(displayValue)"></dsp-time-value>
<dsp-geoname-value class="parent-value-component" #displayVal *ngSwitchCase="constants.GeonameValue" [mode]="mode" [displayValue]="$any(displayValue)"></dsp-geoname-value>
<dsp-link-value class="parent-value-component" #displayVal *ngSwitchCase="constants.LinkValue" [mode]="mode" [displayValue]="$any(displayValue)"
[parentResource]="parentResource" [propIri]="displayValue.property"></dsp-link-value>
<dsp-date-value class="parent-value-component" #displayVal *ngSwitchCase="constants.DateValue" [mode]="mode" [displayValue]=$any(displayValue)></dsp-date-value>
<dsp-list-value class="parent-value-component" #displayVal *ngSwitchCase="constants.ListValue" [mode]="mode" [displayValue]=$any(displayValue)
<dsp-date-value class="parent-value-component" #displayVal *ngSwitchCase="constants.DateValue" [mode]="mode" [displayValue]="$any(displayValue)"></dsp-date-value>
<dsp-list-value class="parent-value-component" #displayVal *ngSwitchCase="constants.ListValue" [mode]="mode" [displayValue]="$any(displayValue)"
[propertyDef]="$any(parentResource.entityInfo.properties[displayValue.property])"></dsp-list-value>
<span *ngSwitchDefault>{{displayValue.strval}}</span>
<span *ngSwitchDefault>{{displayValue.strval}}</span>
</span>
</div>
<!-- TEMPORARY CODE: TO HIDE BUTTONS FOR VALUE COMPONENTS NOT YET IMPLEMENTED -->
Expand Down
1 change: 1 addition & 0 deletions projects/dsp-ui/src/lib/viewer/operations/display-edit/display-edit.component.spec.ts
View file
Open in desktop
Expand Up @@ -580,6 +580,7 @@ describe('DisplayEditComponent', () => {
'uuid');

expect(testHostComponent.displayEditValueComponent.displayValue.id).toEqual('newID');
expect(testHostComponent.displayEditValueComponent.displayValueComponent.displayValue.id).toEqual('newID');
expect(testHostComponent.displayEditValueComponent.mode).toEqual('read');

});
Expand Down
248 changes: 124 additions & 124 deletions projects/dsp-ui/src/lib/viewer/operations/display-edit/display-edit.component.ts
View file
Open in desktop
Expand Up @@ -23,160 +23,160 @@ import { BaseValueComponent } from '../../values/base-value.component';
})
export class DisplayEditComponent implements OnInit {

@ViewChild('displayVal') displayValueComponent: BaseValueComponent;
@ViewChild('displayVal') displayValueComponent: BaseValueComponent;

@Input() displayValue: ReadValue;
@Input() displayValue: ReadValue;

@Input() parentResource: ReadResource;
@Input() parentResource: ReadResource;

@Input() configuration?: object;
@Input() configuration?: object;

constants = Constants;
constants = Constants;

mode: 'read' | 'update' | 'create' | 'search';
mode: 'read' | 'update' | 'create' | 'search';

canModify: boolean;
canModify: boolean;

editModeActive = false;
editModeActive = false;

shouldShowCommentToggle: boolean;
shouldShowCommentToggle: boolean;

// type of given displayValue
// or knora-api-js-lib class representing the value
valueTypeOrClass: string;
// type of given displayValue
// or knora-api-js-lib class representing the value
valueTypeOrClass: string;

// indicates if value can be edited
readOnlyValue: boolean;
// indicates if value can be edited
readOnlyValue: boolean;

private readonly readTextValueAsString = 'ReadTextValueAsString';
private readonly readTextValueAsString = 'ReadTextValueAsString';

private readonly readTextValueAsXml = 'ReadTextValueAsXml';
private readonly readTextValueAsXml = 'ReadTextValueAsXml';

private readonly readTextValueAsHtml = 'ReadTextValueAsHtml';
private readonly readTextValueAsHtml = 'ReadTextValueAsHtml';

constructor(@Inject(DspApiConnectionToken) private knoraApiConnection: KnoraApiConnection) {
}
constructor(@Inject(DspApiConnectionToken) private knoraApiConnection: KnoraApiConnection) {
}

ngOnInit() {

this.mode = 'read';

ngOnInit() {
// determine if user has modify permissions
const allPermissions = PermissionUtil.allUserPermissions(this.displayValue.userHasPermission as 'RV' | 'V' | 'M' | 'D' | 'CR');

this.mode = 'read';
this.canModify = allPermissions.indexOf(PermissionUtil.Permissions.M) !== -1;

// determine if user has modify permissions
const allPermissions = PermissionUtil.allUserPermissions(this.displayValue.userHasPermission as 'RV' | 'V' | 'M' | 'D' | 'CR');
// check if comment toggle button should be shown
this.checkCommentToggleVisibility();

this.canModify = allPermissions.indexOf(PermissionUtil.Permissions.M) !== -1;
this.valueTypeOrClass = this.getValueTypeOrClass(this.displayValue);

// check if comment toggle button should be shown
this.checkCommentToggleVisibility();
this.readOnlyValue = this.isReadOnly(this.valueTypeOrClass);
}

this.valueTypeOrClass = this.getValueTypeOrClass(this.displayValue);
activateEditMode() {
this.editModeActive = true;
this.mode = 'update';

this.readOnlyValue = this.isReadOnly(this.valueTypeOrClass);
}
// hide comment toggle button while in edit mode
this.checkCommentToggleVisibility();

activateEditMode() {
this.editModeActive = true;
this.mode = 'update';
// hide read mode comment when switching to edit mode
this.displayValueComponent.shouldShowComment = false;
}

// hide comment toggle button while in edit mode
this.checkCommentToggleVisibility();
saveEditValue() {
this.editModeActive = false;
const updatedVal = this.displayValueComponent.getUpdatedValue();

if (updatedVal instanceof UpdateValue) {

const updateRes = new UpdateResource();
updateRes.id = this.parentResource.id;
updateRes.type = this.parentResource.type;
updateRes.property = this.displayValue.property;
updateRes.value = updatedVal;
this.knoraApiConnection.v2.values.updateValue(updateRes as UpdateResource<UpdateValue>).pipe(
mergeMap((res: WriteValueResponse) => {
return this.knoraApiConnection.v2.values.getValue(this.parentResource.id, res.uuid);
})
).subscribe(
(res2: ReadResource) => {
this.displayValue = res2.getValues(this.displayValue.property)[0];
this.mode = 'read';

// hide comment once back in read mode
this.displayValueComponent.updateCommentVisibility();

// check if comment toggle button should be shown
this.checkCommentToggleVisibility();
}
);

} else {
console.error('invalid value');
}
}

// hide read mode comment when switching to edit mode
this.displayValueComponent.shouldShowComment = false;
}
cancelEditValue() {
this.editModeActive = false;
this.mode = 'read';

saveEditValue() {
this.editModeActive = false;
const updatedVal = this.displayValueComponent.getUpdatedValue();
// hide comment once back in read mode
this.displayValueComponent.updateCommentVisibility();

if (updatedVal instanceof UpdateValue) {
// check if comment toggle button should be shown
this.checkCommentToggleVisibility();
}

const updateRes = new UpdateResource();
updateRes.id = this.parentResource.id;
updateRes.type = this.parentResource.type;
updateRes.property = this.displayValue.property;
updateRes.value = updatedVal;
this.knoraApiConnection.v2.values.updateValue(updateRes as UpdateResource<UpdateValue>).pipe(
mergeMap((res: WriteValueResponse) => {
return this.knoraApiConnection.v2.values.getValue(this.parentResource.id, res.uuid);
})
).subscribe(
(res2: ReadResource) => {
this.displayValue = res2.getValues(this.displayValue.property)[0];
this.mode = 'read';
// shows or hides the comment
toggleComment() {
this.displayValueComponent.toggleCommentVisibility();
}

// hide comment once back in read mode
this.displayValueComponent.updateCommentVisibility();
// only show the comment toggle button if user is in READ mode and a comment exists for the value
checkCommentToggleVisibility() {
this.shouldShowCommentToggle = (this.mode === 'read' && this.displayValue.valueHasComment !== '' && this.displayValue.valueHasComment !== undefined);
}

// check if comment toggle button should be shown
this.checkCommentToggleVisibility();
/**
* Given a value, determines the type or class representing it.
*
* For text values, this method determines the specific class in use.
* For all other types, the given type is returned.
*
* @param value the given value.
*/
getValueTypeOrClass(value: ReadValue): string {

if (value.type === this.constants.TextValue) {
if (value instanceof ReadTextValueAsString) {
return this.readTextValueAsString;
} else if (value instanceof ReadTextValueAsXml) {
return this.readTextValueAsXml;
} else if (value instanceof ReadTextValueAsHtml) {
return this.readTextValueAsHtml;
} else {
throw new Error(`unknown TextValue class ${value}`);
}
} else {
return value.type;
}
);

} else {
console.error('invalid value');
}
}

cancelEditValue() {
this.editModeActive = false;
this.mode = 'read';

// hide comment once back in read mode
this.displayValueComponent.updateCommentVisibility();

// check if comment toggle button should be shown
this.checkCommentToggleVisibility();
}

// shows or hides the comment
toggleComment() {
this.displayValueComponent.toggleCommentVisibility();
}

// only show the comment toggle button if user is in READ mode and a comment exists for the value
checkCommentToggleVisibility() {
this.shouldShowCommentToggle = (this.mode === 'read' && this.displayValue.valueHasComment !== '' && this.displayValue.valueHasComment !== undefined);
}

/**
* Given a value, determines the type or class representing it.
*
* For text values, this method determines the specific class in use.
* For all other types, the given type is returned.
*
* @param value the given value.
*/
getValueTypeOrClass(value: ReadValue): string {

if (value.type === this.constants.TextValue) {
if (value instanceof ReadTextValueAsString) {
return this.readTextValueAsString;
} else if (value instanceof ReadTextValueAsXml) {
return this.readTextValueAsXml;
} else if (value instanceof ReadTextValueAsHtml) {
return this.readTextValueAsHtml;
} else {
throw new Error(`unknown TextValue class ${value}`);
}
} else {
return value.type;

/**
* 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.
*/

/**
* Determines if the given value is readonly.
*
* @param valueTypeOrClass the type or class of the given value.
*/
isReadOnly(valueTypeOrClass: string): boolean {
return valueTypeOrClass === this.readTextValueAsHtml ||
valueTypeOrClass === this.readTextValueAsXml ||
valueTypeOrClass === this.constants.GeomValue;
}
}

/**
* 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.
*/

/**
* Determines if the given value is readonly.
*
* @param valueTypeOrClass the type or class of the given value.
*/
isReadOnly(valueTypeOrClass: string): boolean {
return valueTypeOrClass === this.readTextValueAsHtml ||
valueTypeOrClass === this.readTextValueAsXml ||
valueTypeOrClass === this.constants.GeomValue;
}
}