mgr/dashboard: Refactoring of DeletionModalComponent

- Simpler variable names:

    Examples:

	- `actionDescription` and `itemDescription` instead of `metaType`
	- `bodyTemplate` instead of `description`
	- `validationPattern` instead of `pattern`

    Some of these variable names have been generalized to ease the
    unification/generalization of dialog components:

	- `submitAction` instead of `deletionMethod`

- Removed unique `setUp` method.

    Benefits:

    - Creation of the component is done as intended by the developers of
    the `ngx-boostrap` package and as expected by developers which use
    the package. The `setUp` method does not have to be called anymore
    on the `DeletionModalComponent` exclusively but instead the
    component is instantiated as all other modals. Property assignment
    on the instantiated object isn't handled by the `setUp` method
    anymore but by the `modalService`.

    - With the removal of the `setUp` method, some tests could be
    removed as well.

    - No need to pass the reference of the created modal to the modal
    manually.

    Preserved:

    - The provided check within the `setUp` method, which checked if the
    component had been correctly instantiated, has been moved to the
    `ngOnInit` method of the component.

Signed-off-by: Patrick Nawracay <pnawracay@suse.com>
This commit is contained in:
Patrick Nawracay 2018-09-06 20:57:50 +02:00
parent f9d57a0b55
commit 8ba41c5e2b
9 changed files with 139 additions and 237 deletions

View File

@ -198,18 +198,19 @@ export class RbdListComponent implements OnInit {
deleteRbdModal() {
const poolName = this.selection.first().pool_name;
const imageName = this.selection.first().name;
this.modalRef = this.modalService.show(DeletionModalComponent);
this.modalRef.content.setUp({
metaType: 'RBD',
deletionObserver: () =>
this.taskWrapper.wrapTaskAroundCall({
task: new FinishedTask('rbd/delete', {
pool_name: poolName,
image_name: imageName
}),
call: this.rbdService.delete(poolName, imageName)
}),
modalRef: this.modalRef
this.modalRef = this.modalService.show(DeletionModalComponent, {
initialState: {
itemDescription: 'RBD',
submitActionObservable: () =>
this.taskWrapper.wrapTaskAroundCall({
task: new FinishedTask('rbd/delete', {
pool_name: poolName,
image_name: imageName
}),
call: this.rbdService.delete(poolName, imageName)
})
}
});
}

View File

@ -253,11 +253,11 @@ export class RbdSnapshotListComponent implements OnInit, OnChanges {
deleteSnapshotModal() {
const snapshotName = this.selection.selected[0].name;
this.modalRef = this.modalService.show(DeletionModalComponent);
this.modalRef.content.setUp({
metaType: 'RBD snapshot',
deletionMethod: () => this._asyncTask('deleteSnapshot', 'rbd/snap/delete', snapshotName),
modalRef: this.modalRef
this.modalRef = this.modalService.show(DeletionModalComponent, {
initialState: {
itemDescription: 'RBD snapshot',
submitAction: () => this._asyncTask('deleteSnapshot', 'rbd/snap/delete', snapshotName)
}
});
}

View File

@ -62,35 +62,35 @@ export class RgwBucketListComponent {
}
deleteAction() {
const modalRef = this.bsModalService.show(DeletionModalComponent);
modalRef.content.setUp({
metaType: this.selection.hasSingleSelection ? 'bucket' : 'buckets',
deletionObserver: (): Observable<any> => {
return new Observable((observer: Subscriber<any>) => {
// Delete all selected data table rows.
observableForkJoin(
this.selection.selected.map((bucket: any) => {
return this.rgwBucketService.delete(bucket.bucket);
})
).subscribe(
null,
(error) => {
// Forward the error to the observer.
observer.error(error);
// Reload the data table content because some deletions might
// have been executed successfully in the meanwhile.
this.table.refreshBtn();
},
() => {
// Notify the observer that we are done.
observer.complete();
// Reload the data table content.
this.table.refreshBtn();
}
);
});
},
modalRef: modalRef
this.bsModalService.show(DeletionModalComponent, {
initialState: {
itemDescription: this.selection.hasSingleSelection ? 'bucket' : 'buckets',
submitActionObservable: () => {
return new Observable((observer: Subscriber<any>) => {
// Delete all selected data table rows.
observableForkJoin(
this.selection.selected.map((bucket: any) => {
return this.rgwBucketService.delete(bucket.bucket);
})
).subscribe(
null,
(error) => {
// Forward the error to the observer.
observer.error(error);
// Reload the data table content because some deletions might
// have been executed successfully in the meanwhile.
this.table.refreshBtn();
},
() => {
// Notify the observer that we are done.
observer.complete();
// Reload the data table content.
this.table.refreshBtn();
}
);
});
}
}
});
}
}

View File

@ -79,35 +79,35 @@ export class RgwUserListComponent {
}
deleteAction() {
const modalRef = this.bsModalService.show(DeletionModalComponent);
modalRef.content.setUp({
metaType: this.selection.hasSingleSelection ? 'user' : 'users',
deletionObserver: (): Observable<any> => {
return new Observable((observer: Subscriber<any>) => {
// Delete all selected data table rows.
observableForkJoin(
this.selection.selected.map((user: any) => {
return this.rgwUserService.delete(user.user_id);
})
).subscribe(
null,
(error) => {
// Forward the error to the observer.
observer.error(error);
// Reload the data table content because some deletions might
// have been executed successfully in the meanwhile.
this.table.refreshBtn();
},
() => {
// Notify the observer that we are done.
observer.complete();
// Reload the data table content.
this.table.refreshBtn();
}
);
});
},
modalRef: modalRef
const modalRef = this.bsModalService.show(DeletionModalComponent, {
initialState: {
itemDescription: this.selection.hasSingleSelection ? 'user' : 'users',
submitActionObservable: (): Observable<any> => {
return new Observable((observer: Subscriber<any>) => {
// Delete all selected data table rows.
observableForkJoin(
this.selection.selected.map((user: any) => {
return this.rgwUserService.delete(user.user_id);
})
).subscribe(
null,
(error) => {
// Forward the error to the observer.
observer.error(error);
// Reload the data table content because some deletions might
// have been executed successfully in the meanwhile.
this.table.refreshBtn();
},
() => {
// Notify the observer that we are done.
observer.complete();
// Reload the data table content.
this.table.refreshBtn();
}
);
});
}
}
});
}
}

View File

@ -90,12 +90,12 @@ export class RoleListComponent implements OnInit {
}
deleteRoleModal() {
this.modalRef = this.modalService.show(DeletionModalComponent);
const name = this.selection.first().name;
this.modalRef.content.setUp({
metaType: 'Role',
deletionMethod: () => this.deleteRole(name),
modalRef: this.modalRef
this.modalRef = this.modalService.show(DeletionModalComponent, {
initialState: {
itemDescription: 'Role',
submitAction: () => this.deleteRole(name)
}
});
}
}

View File

@ -100,11 +100,11 @@ export class UserListComponent implements OnInit {
);
return;
}
this.modalRef = this.modalService.show(DeletionModalComponent);
this.modalRef.content.setUp({
metaType: 'User',
deletionMethod: () => this.deleteUser(username),
modalRef: this.modalRef
this.modalRef = this.modalService.show(DeletionModalComponent, {
initialState: {
itemDescription: 'User',
submitAction: () => this.deleteUser(username)
}
});
}
}

View File

@ -10,13 +10,10 @@
[formGroup]="deletionForm"
novalidate>
<div class="modal-body">
<ng-container *ngTemplateOutlet="description"></ng-container>
<ng-container *ngTemplateOutlet="bodyTemplate"></ng-container>
<div class="question">
<p>
<ng-container i18n>
Are you sure you want to delete the selected
</ng-container>
{{ metaType }}?
<p i18n>
Are you sure that you want to {{ actionDescription | lowercase }} the selected {{ itemDescription }}?
</p>
<div class="form-group"
[ngClass]="{'has-error': deletionForm.showError('confirmation', formDir)}">
@ -28,7 +25,7 @@
autofocus>
<label i18n
for="confirmation">
I'm sure I want to proceed with the deletion.
Yes, I am sure.
</label>
</div>
</div>
@ -37,7 +34,7 @@
<div class="modal-footer">
<cd-submit-button #submitButton
[form]="deletionForm"
(submitAction)="deletionCall()">
(submitAction)="callSubmitAction()">
<ng-container *ngTemplateOutlet="deletionHeading"></ng-container>
</cd-submit-button>
<button class="btn btn-link btn-sm"
@ -52,7 +49,6 @@
<ng-template #deletionHeading>
<ng-container i18n>
Delete
{{ actionDescription | titlecase }} {{ itemDescription }}
</ng-container>
{{ metaType }}
</ng-template>

View File

@ -51,22 +51,20 @@ class MockComponent {
constructor(public modalService: BsModalService) {}
openCtrlDriven() {
this.ctrlRef = this.modalService.show(DeletionModalComponent);
this.ctrlRef.content.setUp({
metaType: 'Controller delete handling',
deletionMethod: this.fakeDeleteController.bind(this),
description: this.ctrlDescription,
modalRef: this.ctrlRef
this.ctrlRef = this.modalService.show(DeletionModalComponent, {
initialState: {
submitAction: this.fakeDeleteController.bind(this),
bodyTemplate: this.ctrlDescription
}
});
}
openModalDriven() {
this.modalRef = this.modalService.show(DeletionModalComponent);
this.modalRef.content.setUp({
metaType: 'Modal delete handling',
deletionObserver: this.fakeDelete(),
description: this.modalDescription,
modalRef: this.modalRef
this.modalRef = this.modalService.show(DeletionModalComponent, {
initialState: {
submitActionObservable: this.fakeDelete(),
bodyTemplate: this.modalDescription
}
});
}
@ -102,17 +100,21 @@ describe('DeletionModalComponent', () => {
configureTestBed({
declarations: [MockComponent, DeletionModalComponent],
schemas: [NO_ERRORS_SCHEMA],
imports: [ModalModule.forRoot(), ReactiveFormsModule, MockModule, DirectivesModule]
imports: [ModalModule.forRoot(), ReactiveFormsModule, MockModule, DirectivesModule],
providers: [BsModalRef]
});
beforeEach(() => {
mockFixture = TestBed.createComponent(MockComponent);
mockComponent = mockFixture.componentInstance;
// Mocking the modals as a lot would be left over
spyOn(mockComponent.modalService, 'show').and.callFake(() => {
spyOn(mockComponent.modalService, 'show').and.callFake((modalComp, config) => {
const ref = new BsModalRef();
fixture = TestBed.createComponent(DeletionModalComponent);
component = fixture.componentInstance;
if (config.initialState) {
component = Object.assign(component, config.initialState);
}
fixture.detectChanges();
ref.content = component;
return ref;
@ -126,7 +128,6 @@ describe('DeletionModalComponent', () => {
});
it('should focus the checkbox form field', () => {
fixture = TestBed.createComponent(DeletionModalComponent);
fixture.detectChanges();
fixture.whenStable().then(() => {
const focused = fixture.debugElement.query(By.css(':focus'));
@ -137,95 +138,25 @@ describe('DeletionModalComponent', () => {
});
});
describe('setUp', () => {
const clearSetup = () => {
component.metaType = undefined;
component.deletionObserver = undefined;
component.description = undefined;
component.modalRef = undefined;
};
const expectSetup = (metaType, observer: boolean, method: boolean, template: boolean) => {
expect(component.modalRef).toBeTruthy();
expect(component.metaType).toBe(metaType);
expect(!!component.deletionObserver).toBe(observer);
expect(!!component.deletionMethod).toBe(method);
expect(!!component.description).toBe(template);
};
beforeEach(() => {
clearSetup();
});
it('should throw error if no modal reference is given', () => {
expect(() =>
component.setUp({
metaType: undefined,
modalRef: undefined
})
).toThrowError('No modal reference');
});
it('should throw error if no meta type is given', () => {
expect(() =>
component.setUp({
metaType: undefined,
modalRef: mockComponent.ctrlRef
})
).toThrowError('No meta type');
});
it('should throw error if no deletion method is given', () => {
expect(() =>
component.setUp({
metaType: 'Sth',
modalRef: mockComponent.ctrlRef
})
).toThrowError('No deletion method');
});
it('should throw no errors if metaType, modalRef and a deletion method were given', () => {
component.setUp({
metaType: 'Observer',
modalRef: mockComponent.ctrlRef,
deletionObserver: mockComponent.fakeDelete()
});
expectSetup('Observer', true, false, false);
clearSetup();
component.setUp({
metaType: 'Controller',
modalRef: mockComponent.ctrlRef,
deletionMethod: mockComponent.fakeDeleteController
});
expectSetup('Controller', false, true, false);
});
it('should test optional parameters - description', () => {
component.setUp({
metaType: 'Description only',
modalRef: mockComponent.ctrlRef,
deletionObserver: mockComponent.fakeDelete(),
description: mockComponent.modalDescription
});
expectSetup('Description only', true, false, true);
it('should throw an error if no action is defined', () => {
component = Object.assign(component, {
submitAction: null,
submitActionObservable: null
});
expect(() => component.ngOnInit()).toThrowError('No submit action defined');
});
it('should test if the ctrl driven mock is set correctly through mock component', () => {
expect(component.metaType).toBe('Controller delete handling');
expect(component.description).toBeTruthy();
expect(component.modalRef).toBeTruthy();
expect(component.deletionMethod).toBeTruthy();
expect(component.deletionObserver).not.toBeTruthy();
expect(component.bodyTemplate).toBeTruthy();
expect(component.submitAction).toBeTruthy();
expect(component.submitActionObservable).not.toBeTruthy();
});
it('should test if the modal driven mock is set correctly through mock component', () => {
mockComponent.openModalDriven();
expect(component.metaType).toBe('Modal delete handling');
expect(component.description).toBeTruthy();
expect(component.modalRef).toBeTruthy();
expect(component.deletionObserver).toBeTruthy();
expect(component.deletionMethod).not.toBeTruthy();
expect(component.bodyTemplate).toBeTruthy();
expect(component.submitActionObservable).toBeTruthy();
expect(component.submitAction).not.toBeTruthy();
});
describe('component functions', () => {
@ -276,19 +207,19 @@ describe('DeletionModalComponent', () => {
describe('Controller driven', () => {
beforeEach(() => {
spyOn(component, 'deletionMethod').and.callThrough();
spyOn(component, 'submitAction').and.callThrough();
spyOn(mockComponent.ctrlRef, 'hide').and.callThrough();
});
it('should test fake deletion that closes modal', <any>fakeAsync(() => {
// Before deletionCall
expect(component.deletionMethod).not.toHaveBeenCalled();
expect(component.submitAction).not.toHaveBeenCalled();
// During deletionCall
component.deletionCall();
component.callSubmitAction();
expect(component.stopLoadingSpinner).not.toHaveBeenCalled();
expect(component.hideModal).not.toHaveBeenCalled();
expect(mockComponent.ctrlRef.hide).not.toHaveBeenCalled();
expect(component.deletionMethod).toHaveBeenCalled();
expect(component.submitAction).toHaveBeenCalled();
expect(mockComponent.finished).toBe(undefined);
// After deletionCall
tick(2000);
@ -301,7 +232,6 @@ describe('DeletionModalComponent', () => {
describe('Modal driven', () => {
beforeEach(() => {
mockComponent.openModalDriven();
spyOn(mockComponent.modalRef, 'hide').and.callThrough();
spyOn(component, 'stopLoadingSpinner').and.callThrough();
spyOn(component, 'hideModal').and.callThrough();
spyOn(mockComponent, 'fakeDelete').and.callThrough();
@ -309,14 +239,12 @@ describe('DeletionModalComponent', () => {
it('should delete and close modal', <any>fakeAsync(() => {
// During deletionCall
component.deletionCall();
component.callSubmitAction();
expect(mockComponent.finished).toBe(undefined);
expect(component.hideModal).not.toHaveBeenCalled();
expect(mockComponent.modalRef.hide).not.toHaveBeenCalled();
// After deletionCall
tick(2000);
expect(mockComponent.finished).toEqual([6, 7, 8, 9]);
expect(mockComponent.modalRef.hide).toHaveBeenCalled();
expect(component.stopLoadingSpinner).not.toHaveBeenCalled();
expect(component.hideModal).toHaveBeenCalled();
}));

View File

@ -15,57 +15,34 @@ import { SubmitButtonComponent } from '../submit-button/submit-button.component'
export class DeletionModalComponent implements OnInit {
@ViewChild(SubmitButtonComponent)
submitButton: SubmitButtonComponent;
description: TemplateRef<any>;
metaType: string;
deletionObserver: () => Observable<any>;
deletionMethod: Function;
modalRef: BsModalRef;
bodyTemplate: TemplateRef<any>;
submitActionObservable: () => Observable<any>;
submitAction: Function;
deletionForm: CdFormGroup;
itemDescription: 'entry';
actionDescription = 'delete';
// Parameters are destructed here than assigned to specific types and marked as optional
setUp({
modalRef,
metaType,
deletionMethod,
deletionObserver,
description
}: {
modalRef: BsModalRef;
metaType: string;
deletionMethod?: Function;
deletionObserver?: () => Observable<any>;
description?: TemplateRef<any>;
}) {
if (!modalRef) {
throw new Error('No modal reference');
} else if (!metaType) {
throw new Error('No meta type');
} else if (!(deletionMethod || deletionObserver)) {
throw new Error('No deletion method');
}
this.metaType = metaType;
this.modalRef = modalRef;
this.deletionMethod = deletionMethod;
this.deletionObserver = deletionObserver;
this.description = description;
}
constructor(public modalRef: BsModalRef) {}
ngOnInit() {
this.deletionForm = new CdFormGroup({
confirmation: new FormControl(false, [Validators.requiredTrue])
});
if (!(this.submitAction || this.submitActionObservable)) {
throw new Error('No submit action defined');
}
}
deletionCall() {
if (this.deletionObserver) {
this.deletionObserver().subscribe(
undefined,
callSubmitAction() {
if (this.submitActionObservable) {
this.submitActionObservable().subscribe(
null,
this.stopLoadingSpinner.bind(this),
this.hideModal.bind(this)
);
} else {
this.deletionMethod();
this.submitAction();
}
}