]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commitdiff
mgr/dashboard: Refactoring of `DeletionModalComponent`
authorPatrick Nawracay <pnawracay@suse.com>
Thu, 6 Sep 2018 18:57:50 +0000 (20:57 +0200)
committerPatrick Nawracay <pnawracay@suse.com>
Mon, 17 Sep 2018 06:25:41 +0000 (08:25 +0200)
- 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>
src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.ts
src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.ts
src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-list/rgw-bucket-list.component.ts
src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-user-list/rgw-user-list.component.ts
src/pybind/mgr/dashboard/frontend/src/app/core/auth/role-list/role-list.component.ts
src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-list/user-list.component.ts
src/pybind/mgr/dashboard/frontend/src/app/shared/components/deletion-modal/deletion-modal.component.html
src/pybind/mgr/dashboard/frontend/src/app/shared/components/deletion-modal/deletion-modal.component.spec.ts
src/pybind/mgr/dashboard/frontend/src/app/shared/components/deletion-modal/deletion-modal.component.ts

index 2a4ac07d79e4b465990e534f32ffa3ca9eeed860..eafb9bda73fdb4a193e935c43227876869622dc1 100644 (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)
+          })
+      }
     });
   }
 
index c22ace30b2e927ca0718aed8ee453ddfefe09e97..f001b29cdd85a529be9587321a096dc06a3b2d8f 100644 (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)
+      }
     });
   }
 
index 3ba427dbdd07d39a06c650b6805643acbd7da361..ea848143a2a13b77809677cb6478879eadea5ee5 100644 (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();
+              }
+            );
+          });
+        }
+      }
     });
   }
 }
index 71c3b8fae927b196025d6504a763a3e8b98b2be4..8d33bc3e27c9d9e15a3b00bf4130e68ead6bdabb 100644 (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();
+              }
+            );
+          });
+        }
+      }
     });
   }
 }
index cc668434e3bacb9d3c4032047b03029dea36cb61..c9dffee279cc512b3d79a38ee36dbbb3c7b633e2 100644 (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)
+      }
     });
   }
 }
index ea62fc24a6c6b0d637ade9f20abdb552bb602249..37a3d77486ab0231190ad02502d209ec62b41312 100644 (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)
+      }
     });
   }
 }
index a6dc08babdad4eacdef90e65da53271970d9259b..ee4532677f94b78f2b6746a46acc368795839f3a 100644 (file)
           [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>
index 7e72bbf02f7509e460238b1e6e28a74832b1aed3..7e27c85ba4f2336c61871e6f3c3e777ceb3aad45 100644 (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();
         }));
index cab3e31dba00b239b84a657d9286cefcc7abe727..f472baccbb7d7dbab2984c092a9b0d0955b9353a 100644 (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();
     }
   }