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

DSP-638 Value Deletion Comment #198

Merged
merged 7 commits into from
Oct 6, 2020
27 changes: 27 additions & 0 deletions projects/dsp-ui/src/assets/style/_viewer.scss
Original file line number Diff line number Diff line change
Expand Up @@ -164,3 +164,30 @@ button.save .mat-icon,
button.cancel .mat-icon {
font-size: 18px;
}

.deletion-dialog .title {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this more general than value deletion? Wouldn't it make more sense to keep it in the component's CSS file?

Copy link
Contributor Author

@mdelez mdelez Oct 6, 2020

Choose a reason for hiding this comment

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

After some thinking, I should refactor this a little. The current ConfirmationMessage component should be renamed and made more specific to a deletion message. This way the ConfirmationDialog component can be reused and you can just supply it with the Message component of your choosing.

Ideally the ConfirmationDialog component is just a shell with a button to confirm and a button to cancel and its contents is defined by the type of message component you give it. We should handle this in another PR though.

Task created https://dasch.myjetbrains.com/youtrack/issue/DSP-781

font-weight: bold;
font-size: 18px;
text-align: center;
}

.deletion-dialog .action-buttons {
float: right;

.cancel {
padding: 0 16px;
}
}

.deletion-dialog-message {
font-size: 16px;
background-color: #ededf5;
border: 1px solid #d8d8df;
border-radius: 5px;
padding: 20px 10px 20px 10px;
text-align: center;
}

.deletion-dialog-message .deletion-comment {
min-width: 80%;
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
<mat-dialog-content>
<p class="title"> Are you sure you want to delete this value from {{data.value.propertyLabel}}?</p>
<dsp-confirmation-message [value]="data.value"></dsp-confirmation-message>
</mat-dialog-content>
<mat-dialog-actions class="action-buttons">
<button class="cancel" mat-raised-button mat-dialog-close>{{data.buttonTextCancel}}</button>
<button class="ok" mat-raised-button color="primary" (click)="onConfirmClick()">{{data.buttonTextOk}}</button>
</mat-dialog-actions>
<div class="deletion-dialog">
<mat-dialog-content>
<p class="title"> Are you sure you want to delete this value from {{data.value.propertyLabel}}?</p>
<dsp-confirmation-message #confirmMessage [value]="data.value"></dsp-confirmation-message>
</mat-dialog-content>
<mat-dialog-actions class="action-buttons">
<button class="cancel" mat-raised-button mat-dialog-close>{{data.buttonTextCancel}}</button>
<button class="ok" mat-raised-button color="primary" (click)="onConfirmClick()">{{data.buttonTextOk}}</button>
</mat-dialog-actions>
</div>

Original file line number Diff line number Diff line change
@@ -1,9 +1 @@
.title {
font-weight: bold;
font-size: 18px;
text-align: center;
}

.action-buttons {
float: right;
}
@import "../../../../assets/style/viewer";
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { MAT_DIALOG_DATA, MatDialog, MatDialogModule, MatDialogRef } from '@angu
import { MatDialogHarness } from '@angular/material/dialog/testing';
import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
import { MockResource, ReadIntValue, ReadValue } from '@dasch-swiss/dsp-js';
import { ConfirmationDialogComponent } from './confirmation-dialog.component';
import { ConfirmationDialogComponent, ConfirmationDialogValueDeletionPayload } from './confirmation-dialog.component';

/**
* Test host component to simulate parent component with a confirmation dialog.
Expand Down Expand Up @@ -39,8 +39,8 @@ class ConfirmationDialogTestHostComponent implements OnInit {
buttonTextOk: 'OK',
buttonTextCancel: 'Cancel'
}
}).afterClosed().subscribe((confirmed: boolean) => {
if (confirmed) {
}).afterClosed().subscribe((payload: ConfirmationDialogValueDeletionPayload) => {
if (payload.confirmed) {
this.confirmationDialogResponse = 'Action was confirmed!';
} else {
this.confirmationDialogResponse = 'Action was cancelled';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
import { Component, Inject } from '@angular/core';
import { Component, Inject, ViewChild } from '@angular/core';
import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog';
import { ReadValue } from '@dasch-swiss/dsp-js';
import { ConfirmationMessageComponent } from './confirmation-message/confirmation-message.component';

export class ConfirmationDialogData {
value: ReadValue;
buttonTextOk: string;
buttonTextCancel: string;
}

export class ConfirmationDialogValueDeletionPayload {
confirmed: boolean;
deletionComment?: string;
}

@Component({
selector: 'dsp-confirmation-dialog',
templateUrl: './confirmation-dialog.component.html',
styleUrls: ['./confirmation-dialog.component.scss']
})
export class ConfirmationDialogComponent {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this component specific for value deletion? If so, could its name say so?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed in 050a4cd

@ViewChild('confirmMessage') confirmationMessageComponent: ConfirmationMessageComponent;

// type assertion doesn't seem to be enforced
// https://stackoverflow.com/a/57787554
Expand All @@ -23,7 +30,10 @@ export class ConfirmationDialogComponent {
) { }

onConfirmClick(): void {
this._dialogRef.close(true);
const payload = new ConfirmationDialogValueDeletionPayload();
payload.confirmed = true;
payload.deletionComment = this.confirmationMessageComponent.comment ? this.confirmationMessageComponent.comment : undefined;
this._dialogRef.close(payload);
}

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
<div class="message">
<div class="deletion-dialog-message">
<p class="val-label">Confirming this action will delete the following value from {{value.propertyLabel}}:</p>
<p class="val-value">Value: {{value.strval}}</p>
<p class="val-comment">Value Comment: {{value.valueHasComment ? value.valueHasComment : 'No comment'}}</p>
<p class="val-creation-date">Value Creation Date: {{value.valueCreationDate}}</p>
<textarea
matinput
class="deletion-comment"
type="text"
(keyup)="onKey($event)"
[placeholder]="'Comment why value is being deleted'"
></textarea>
</div>
Original file line number Diff line number Diff line change
@@ -1,8 +1 @@
.message {
font-size: 16px;
background-color: #ededf5;
border: 1px solid #d8d8df;
border-radius: 5px;
padding: 20px 10px 20px 10px;
text-align: center;
}
@import "../../../../../assets/style/viewer";
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ import { ReadValue } from '@dasch-swiss/dsp-js';
export class ConfirmationMessageComponent {

@Input() value: ReadValue;
comment?: string;

constructor() { }

onKey(event: KeyboardEvent) {
this.comment = (event.target as HTMLInputElement).value;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,6 @@ describe('MessageComponent', () => {
expect(spanShortMessageElement.nativeElement.innerText).toEqual('You just updated the user profile.');

shortMsgDurationTestHostFixture.whenStable().then(() => {
console.log(shortMsgDurationTestHostComponent);

expect(shortMsgDurationTestHostComponent.messageComponent.disable).toBeTruthy();
});
}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { MatButtonHarness } from '@angular/material/button/testing';
import { MAT_DIALOG_DATA, MatDialogModule, MatDialogRef } from '@angular/material/dialog';
import { MatDialogHarness } from '@angular/material/dialog/testing';
import { MatIconModule } from '@angular/material/icon';
import { MatInputHarness } from '@angular/material/input/testing';
import { MatTooltipModule } from '@angular/material/tooltip';
import { By } from '@angular/platform-browser';
import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
Expand Down Expand Up @@ -937,6 +938,7 @@ describe('DisplayEditComponent', () => {
const deleteVal = new DeleteValue();
deleteVal.id = 'http://rdfh.ch/0001/H6gBWUuJSuuO-CilHV8kQw/values/dJ1ES8QTQNepFKF5-EAqdg';
deleteVal.type = 'http://api.knora.org/ontology/knora-api/v2#IntValue';
deleteVal.deleteComment = undefined;

expectedUpdateResource.value = deleteVal;

Expand All @@ -949,5 +951,44 @@ describe('DisplayEditComponent', () => {
});

});

it('should send a deletion comment to Knora if one is provided', async () => {
const valueEventSpy = TestBed.inject(ValueOperationEventService);

const valuesSpy = TestBed.inject(DspApiConnectionToken);

(valueEventSpy as jasmine.SpyObj<ValueOperationEventService>).emit.and.stub();

(valuesSpy.v2.values as jasmine.SpyObj<ValuesEndpointV2>).deleteValue.and.callFake(
() => {

const response = new DeleteValueResponse();

response.result = 'success';

return of(response);
}
);

testHostComponent.displayEditValueComponent.deleteValue('my deletion comment');

const expectedUpdateResource = new UpdateResource();

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

const deleteVal = new DeleteValue();
deleteVal.id = 'http://rdfh.ch/0001/H6gBWUuJSuuO-CilHV8kQw/values/dJ1ES8QTQNepFKF5-EAqdg';
deleteVal.type = 'http://api.knora.org/ontology/knora-api/v2#IntValue';
deleteVal.deleteComment = 'my deletion comment';

expectedUpdateResource.value = deleteVal;

testHostFixture.whenStable().then(() => {
expect(valuesSpy.v2.values.deleteValue).toHaveBeenCalledWith(expectedUpdateResource);
expect(valuesSpy.v2.values.deleteValue).toHaveBeenCalledTimes(1);
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ import {
import { mergeMap } from 'rxjs/operators';
import {
ConfirmationDialogComponent,
ConfirmationDialogData
ConfirmationDialogData,
ConfirmationDialogValueDeletionPayload
} from '../../../action/components/confirmation-dialog/confirmation-dialog.component';
import { DspApiConnectionToken } from '../../../core/core.module';
import {
Expand Down Expand Up @@ -222,9 +223,9 @@ export class DisplayEditComponent implements OnInit {
const dialogRef =
this._dialog.open<ConfirmationDialogComponent, ConfirmationDialogData>(ConfirmationDialogComponent, { data: dialogData});

dialogRef.afterClosed().subscribe((confirmed: boolean) => {
if (confirmed) {
this.deleteValue();
dialogRef.afterClosed().subscribe((payload: ConfirmationDialogValueDeletionPayload) => {
if (payload && payload.confirmed) {
this.deleteValue(payload.deletionComment);
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if null is passed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

made optional in 050a4cd

}
});
}
Expand All @@ -233,10 +234,11 @@ export class DisplayEditComponent implements OnInit {
* Delete a value from a property.
* Emits an event that can be listened to.
*/
deleteValue() {
deleteValue(comment?: string) {
const deleteVal = new DeleteValue();
deleteVal.id = this.displayValue.id;
deleteVal.type = this.displayValue.type;
deleteVal.deleteComment = comment;

const updateRes = new UpdateResource();
updateRes.type = this.parentResource.type;
Expand Down