]> git.apps.os.sepia.ceph.com Git - ceph-ci.git/commitdiff
mgr/dashboard: Merge disable and disableDesc
authorTiago Melo <tmelo@suse.com>
Fri, 24 Jul 2020 15:59:35 +0000 (15:59 +0000)
committerTiago Melo <tmelo@suse.com>
Wed, 29 Jul 2020 20:28:03 +0000 (20:28 +0000)
Fixes: https://tracker.ceph.com/issues/46750
Signed-off-by: Tiago Melo <tmelo@suse.com>
14 files changed:
src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-list/iscsi-target-list.component.spec.ts
src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-list/iscsi-target-list.component.ts
src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.spec.ts
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-namespace-list/rbd-namespace-list.component.ts
src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-actions.model.ts
src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.spec.ts
src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.ts
src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.ts
src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.spec.ts
src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.ts
src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-actions/table-actions.component.spec.ts
src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-actions/table-actions.component.ts
src/pybind/mgr/dashboard/frontend/src/app/shared/models/cd-table-action.ts

index 2c8657f3577eac459a99c3958eb99b9ea00fb95f..1694b4bbbee15b5ec61099504d48a6d0d20e7fc1 100644 (file)
@@ -194,8 +194,7 @@ describe('IscsiTargetListComponent', () => {
             id: '-1'
           }
         ];
-        expect(action.disable(undefined)).toBeTruthy();
-        expect(action.disableDesc(undefined)).toBe('Unavailable gateway(s)');
+        expect(action.disable(undefined)).toBe('Unavailable gateway(s)');
       });
 
       it('should be enabled if active sessions', () => {
@@ -208,7 +207,6 @@ describe('IscsiTargetListComponent', () => {
           }
         ];
         expect(action.disable(undefined)).toBeFalsy();
-        expect(action.disableDesc(undefined)).toBeUndefined();
       });
 
       it('should be enabled if no active sessions', () => {
@@ -221,7 +219,6 @@ describe('IscsiTargetListComponent', () => {
           }
         ];
         expect(action.disable(undefined)).toBeFalsy();
-        expect(action.disableDesc(undefined)).toBeUndefined();
       });
     });
 
@@ -236,8 +233,7 @@ describe('IscsiTargetListComponent', () => {
             id: '-1'
           }
         ];
-        expect(action.disable(undefined)).toBeTruthy();
-        expect(action.disableDesc(undefined)).toBe('Unavailable gateway(s)');
+        expect(action.disable(undefined)).toBe('Unavailable gateway(s)');
       });
 
       it('should be disabled if active sessions', () => {
@@ -249,8 +245,7 @@ describe('IscsiTargetListComponent', () => {
             }
           }
         ];
-        expect(action.disable(undefined)).toBeTruthy();
-        expect(action.disableDesc(undefined)).toBe('Target has active sessions');
+        expect(action.disable(undefined)).toBe('Target has active sessions');
       });
 
       it('should be enabled if no active sessions', () => {
@@ -263,7 +258,6 @@ describe('IscsiTargetListComponent', () => {
           }
         ];
         expect(action.disable(undefined)).toBeFalsy();
-        expect(action.disableDesc(undefined)).toBeUndefined();
       });
     });
   });
index dd96295ca0e00023e9048ec10bdf1787daa539cf..3a417b5b0f371ba5a1252a61c219db93db573c03 100644 (file)
@@ -84,16 +84,14 @@ export class IscsiTargetListComponent extends ListWithDetails implements OnInit,
         icon: Icons.edit,
         routerLink: () => `/block/iscsi/targets/edit/${this.selection.first().target_iqn}`,
         name: this.actionLabels.EDIT,
-        disable: () => !this.selection.first() || !_.isUndefined(this.getEditDisableDesc()),
-        disableDesc: () => this.getEditDisableDesc()
+        disable: () => this.getEditDisableDesc()
       },
       {
         permission: 'delete',
         icon: Icons.destroy,
         click: () => this.deleteIscsiTargetModal(),
         name: this.actionLabels.DELETE,
-        disable: () => !this.selection.first() || !_.isUndefined(this.getDeleteDisableDesc()),
-        disableDesc: () => this.getDeleteDisableDesc()
+        disable: () => this.getDeleteDisableDesc()
       }
     ];
   }
@@ -160,31 +158,36 @@ export class IscsiTargetListComponent extends ListWithDetails implements OnInit,
     }
   }
 
-  getEditDisableDesc(): string | undefined {
+  getEditDisableDesc(): string | boolean {
     const first = this.selection.first();
-    if (first && first.cdExecuting) {
+
+    if (first && first?.cdExecuting) {
       return first.cdExecuting;
     }
-    if (first && _.isUndefined(first['info'])) {
+
+    if (first && _.isUndefined(first?.['info'])) {
       return $localize`Unavailable gateway(s)`;
     }
 
-    return undefined;
+    return !first;
   }
 
-  getDeleteDisableDesc(): string | undefined {
+  getDeleteDisableDesc(): string | boolean {
     const first = this.selection.first();
-    if (first && first.cdExecuting) {
+
+    if (first?.cdExecuting) {
       return first.cdExecuting;
     }
-    if (first && _.isUndefined(first['info'])) {
+
+    if (first && _.isUndefined(first?.['info'])) {
       return $localize`Unavailable gateway(s)`;
     }
-    if (first && first['info'] && first['info']['num_sessions']) {
+
+    if (first && first?.['info']?.['num_sessions']) {
       return $localize`Target has active sessions`;
     }
 
-    return undefined;
+    return !first;
   }
 
   prepareResponse(resp: any): any[] {
index 193a338feb773c2e76f4f8b040123c2ccf0cc5a0..a8a40678ac08d2e069e09c78ffc36fee6920436c 100644 (file)
@@ -130,7 +130,7 @@ describe('RbdListComponent', () => {
           }
         ]
       });
-      expect(component.getDeleteDisableDesc()).toBe(
+      expect(component.getDeleteDisableDesc(component.selection)).toBe(
         'This RBD has cloned snapshots. Please delete related RBDs before deleting this RBD.'
       );
     });
index 18ac272143e6264031828e610232b69e2f3bb6c0..a6d8a5518e7cde12a5b2d673651df48dc262930c 100644 (file)
@@ -137,11 +137,7 @@ export class RbdListComponent extends ListWithDetails implements OnInit {
       icon: Icons.destroy,
       click: () => this.deleteRbdModal(),
       name: this.actionLabels.DELETE,
-      disable: (selection: CdTableSelection) =>
-        !this.selection.first() ||
-        !this.selection.hasSingleSelection ||
-        this.hasClonedSnapshots(selection.first()),
-      disableDesc: () => this.getDeleteDisableDesc()
+      disable: (selection: CdTableSelection) => this.getDeleteDisableDesc(selection)
     };
     const copyAction: CdTableAction = {
       permission: 'create',
@@ -429,12 +425,17 @@ export class RbdListComponent extends ListWithDetails implements OnInit {
     }, []);
   }
 
-  getDeleteDisableDesc(): string {
-    const first = this.selection.first();
+  getDeleteDisableDesc(selection: CdTableSelection): string | boolean {
+    const first = selection.first();
+
     if (first && this.hasClonedSnapshots(first)) {
       return $localize`This RBD has cloned snapshots. Please delete related RBDs before deleting this RBD.`;
     }
 
-    return '';
+    return (
+      !selection.first() ||
+      !selection.hasSingleSelection ||
+      this.hasClonedSnapshots(selection.first())
+    );
   }
 }
index 8d449f3ce79fd299fcd89ef25e43e168f0755593..cd66c7cf9c476a082b234001321870fc9f97389a 100644 (file)
@@ -54,8 +54,7 @@ export class RbdNamespaceListComponent implements OnInit {
       icon: Icons.destroy,
       click: () => this.deleteModal(),
       name: this.actionLabels.DELETE,
-      disable: () => !this.selection.first() || !_.isUndefined(this.getDeleteDisableDesc()),
-      disableDesc: () => this.getDeleteDisableDesc()
+      disable: () => this.getDeleteDisableDesc()
     };
     this.tableActions = [createAction, deleteAction];
   }
@@ -147,14 +146,13 @@ export class RbdNamespaceListComponent implements OnInit {
     });
   }
 
-  getDeleteDisableDesc(): string | undefined {
+  getDeleteDisableDesc(): string | boolean {
     const first = this.selection.first();
-    if (first) {
-      if (first.num_images > 0) {
-        return $localize`Namespace contains images`;
-      }
+
+    if (first?.num_images > 0) {
+      return $localize`Namespace contains images`;
     }
 
-    return undefined;
+    return !this.selection?.first();
   }
 }
index 040f5824de34e34bd803cb6c382f0cbc481684b2..c966c52974bb904c19294ac82e7b9015812b22bc 100644 (file)
@@ -44,11 +44,7 @@ export class RbdSnapshotActionsModel {
     this.clone = {
       permission: 'create',
       canBePrimary: (selection: CdTableSelection) => selection.hasSingleSelection,
-      disable: (selection: CdTableSelection) =>
-        !selection.hasSingleSelection ||
-        selection.first().cdExecuting ||
-        !_.isUndefined(this.getCloneDisableDesc(featuresName)),
-      disableDesc: () => this.getCloneDisableDesc(featuresName),
+      disable: (selection: CdTableSelection) => this.getCloneDisableDesc(selection, featuresName),
       icon: Icons.clone,
       name: actionLabels.CLONE
     };
@@ -87,11 +83,15 @@ export class RbdSnapshotActionsModel {
     ];
   }
 
-  getCloneDisableDesc(featuresName: string[]): string | undefined {
-    if (!featuresName?.includes('layering')) {
-      return $localize`Parent image must support Layering`;
+  getCloneDisableDesc(selection: CdTableSelection, featuresName: string[]): boolean | string {
+    if (selection.hasSingleSelection && !selection.first().cdExecuting) {
+      if (!featuresName?.includes('layering')) {
+        return $localize`Parent image must support Layering`;
+      }
+
+      return false;
     }
 
-    return undefined;
+    return true;
   }
 }
index eb8ad20b03535e1da3f65751de6219213d8c9ab7..0c2931f3af74847f97dc3385a597a5591f740fbe 100644 (file)
@@ -111,12 +111,12 @@ describe('HostsComponent', () => {
       );
     });
 
-    it('should disable button and return undefined (no selection)', () => {
+    it('should disable button and return true (no selection)', () => {
       expect(tableAction.disable(component.selection)).toBeTruthy();
-      expect(component.getEditDisableDesc(component.selection)).toBeUndefined();
+      expect(component.getEditDisableDesc(component.selection)).toBeTruthy();
     });
 
-    it('should enable button and return undefined (managed by Orchestrator)', () => {
+    it('should enable button and return false (managed by Orchestrator)', () => {
       component.selection.add({
         sources: {
           ceph: false,
@@ -124,7 +124,7 @@ describe('HostsComponent', () => {
         }
       });
       expect(tableAction.disable(component.selection)).toBeFalsy();
-      expect(component.getEditDisableDesc(component.selection)).toBeUndefined();
+      expect(component.getEditDisableDesc(component.selection)).toBeFalsy();
     });
   });
 
@@ -147,11 +147,11 @@ describe('HostsComponent', () => {
       );
     });
 
-    it('should return undefined (no selection)', () => {
-      expect(component.getDeleteDisableDesc(component.selection)).toBeUndefined();
+    it('should return true (no selection)', () => {
+      expect(component.getDeleteDisableDesc(component.selection)).toBeTruthy();
     });
 
-    it('should return undefined (managed by Orchestrator)', () => {
+    it('should return false (managed by Orchestrator)', () => {
       component.selection.add({
         sources: {
           ceph: false,
@@ -164,7 +164,7 @@ describe('HostsComponent', () => {
           orchestrator: true
         }
       });
-      expect(component.getDeleteDisableDesc(component.selection)).toBeUndefined();
+      expect(component.getDeleteDisableDesc(component.selection)).toBeFalsy();
     });
   });
 });
index d1bea0bdd52543746570e2d504d06eec105dda8c..871bacd3d0d974afd8b68e91c817c496cd8d3d78 100644 (file)
@@ -91,9 +91,7 @@ export class HostsComponent extends ListWithDetails implements OnInit {
             () => this.editAction()
           );
         },
-        disable: (selection: CdTableSelection) =>
-          !selection.hasSingleSelection || !selection.first().sources.orchestrator,
-        disableDesc: this.getEditDisableDesc.bind(this)
+        disable: this.getEditDisableDesc.bind(this)
       },
       {
         name: this.actionLabels.DELETE,
@@ -106,10 +104,7 @@ export class HostsComponent extends ListWithDetails implements OnInit {
             () => this.deleteAction()
           );
         },
-        disable: (selection: CdTableSelection) =>
-          !selection.hasSelection ||
-          !selection.selected.every((selected) => selected.sources.orchestrator),
-        disableDesc: this.getDeleteDisableDesc.bind(this)
+        disable: this.getDeleteDisableDesc.bind(this)
       }
     ];
   }
@@ -186,11 +181,16 @@ export class HostsComponent extends ListWithDetails implements OnInit {
     });
   }
 
-  getEditDisableDesc(selection: CdTableSelection): string | undefined {
-    if (selection && selection.hasSingleSelection && !selection.first().sources.orchestrator) {
-      return $localize`Host editing is disabled because the selected host is not managed by Orchestrator.`;
+  getEditDisableDesc(selection: CdTableSelection): boolean | string {
+    if (selection?.hasSingleSelection) {
+      if (!selection?.first().sources.orchestrator) {
+        return $localize`Host editing is disabled because the selected host is not managed by Orchestrator.`;
+      }
+
+      return false;
     }
-    return undefined;
+
+    return true;
   }
 
   deleteAction() {
@@ -207,15 +207,16 @@ export class HostsComponent extends ListWithDetails implements OnInit {
     });
   }
 
-  getDeleteDisableDesc(selection: CdTableSelection): string | undefined {
-    if (
-      selection &&
-      selection.hasSelection &&
-      !selection.selected.every((selected) => selected.sources.orchestrator)
-    ) {
-      return $localize`Host deletion is disabled because a selected host is not managed by Orchestrator.`;
+  getDeleteDisableDesc(selection: CdTableSelection): boolean | string {
+    if (selection?.hasSelection) {
+      if (!selection.selected.every((selected) => selected.sources.orchestrator)) {
+        return $localize`Host deletion is disabled because a selected host is not managed by Orchestrator.`;
+      }
+
+      return false;
     }
-    return undefined;
+
+    return true;
   }
 
   getHosts(context: CdTableFetchDataContext) {
index 8e1da89d0f56fe65e001928bfa30572b88366c0f..b0f4a9fefab1ebc260354178ec1ee4cacfc63874 100644 (file)
@@ -89,8 +89,7 @@ export class MgrModuleListComponent extends ListWithDetails {
         name: $localize`Disable`,
         permission: 'update',
         click: () => this.updateModuleState(),
-        disable: () => this.isTableActionDisabled('disabled'),
-        disableDesc: () => this.getTableActionDisabledDesc(),
+        disable: () => () => this.getTableActionDisabledDesc(),
         icon: Icons.stop
       }
     ];
@@ -140,15 +139,12 @@ export class MgrModuleListComponent extends ListWithDetails {
     }
   }
 
-  getTableActionDisabledDesc(): string | undefined {
-    if (this.selection.hasSelection) {
-      const selected = this.selection.first();
-      if (selected.always_on) {
-        return $localize`This Manager module is always on.`;
-      }
+  getTableActionDisabledDesc(): string | boolean {
+    if (this.selection.first().always_on) {
+      return $localize`This Manager module is always on.`;
     }
 
-    return undefined;
+    return this.isTableActionDisabled('disabled');
   }
 
   /**
index 1dbd72afb2b034f9a5edd9a0814625676a6cc95a..06506f196146e345ff21d80ac23e6abb585bf8db 100644 (file)
@@ -475,6 +475,10 @@ describe('PoolListComponent', () => {
   });
 
   describe('getDisableDesc', () => {
+    beforeEach(() => {
+      component.selection.selected = [{ pool_name: 'foo' }];
+    });
+
     it('should return message if mon_allow_pool_delete flag is set to false', () => {
       component.monAllowPoolDelete = false;
       expect(component.getDisableDesc()).toBe(
@@ -482,9 +486,9 @@ describe('PoolListComponent', () => {
       );
     });
 
-    it('should return undefined if mon_allow_pool_delete flag is set to true', () => {
+    it('should return false if mon_allow_pool_delete flag is set to true', () => {
       component.monAllowPoolDelete = true;
-      expect(component.getDisableDesc()).toBeUndefined();
+      expect(component.getDisableDesc()).toBeFalsy();
     });
   });
 });
index eae76ef70c7f2fc2b36d8cbdcaa18017e065fecf..fe6e4afa003f6f6cad1717d170cb153f362f69c2 100644 (file)
@@ -92,8 +92,7 @@ export class PoolListComponent extends ListWithDetails implements OnInit {
         icon: Icons.destroy,
         click: () => this.deletePoolModal(),
         name: this.actionLabels.DELETE,
-        disable: () => !this.selection.first() || !this.monAllowPoolDelete,
-        disableDesc: () => this.getDisableDesc()
+        disable: this.getDisableDesc.bind(this)
       }
     ];
 
@@ -291,12 +290,16 @@ export class PoolListComponent extends ListWithDetails implements OnInit {
     }
   }
 
-  getDisableDesc(): string | undefined {
-    if (!this.monAllowPoolDelete) {
-      return $localize`Pool deletion is disabled by the mon_allow_pool_delete configuration setting.`;
+  getDisableDesc(): boolean | string {
+    if (this.selection?.hasSelection) {
+      if (!this.monAllowPoolDelete) {
+        return $localize`Pool deletion is disabled by the mon_allow_pool_delete configuration setting.`;
+      }
+
+      return false;
     }
 
-    return undefined;
+    return true;
   }
 
   setExpandedRow(expandedRow: any) {
index 2593a59b2bea74643c518cd4f4bad704c0445cce..e7b2b73ee6a5c9a79e28e07f1c231d2d5e24aced 100644 (file)
@@ -163,12 +163,12 @@ describe('TableActionsComponent', () => {
   });
 
   describe('useDisableDesc', () => {
-    it('should return a description if disableDesc is set for action', () => {
+    it('should return a description if disable method returns a string', () => {
       const deleteWithDescAction: CdTableAction = {
         permission: 'delete',
         icon: 'fa-times',
         canBePrimary: (selection: CdTableSelection) => selection.hasSelection,
-        disableDesc: () => {
+        disable: () => {
           return 'Delete action disabled description';
         },
         name: 'DeleteDesc'
@@ -179,7 +179,7 @@ describe('TableActionsComponent', () => {
       );
     });
 
-    it('should return no description if disableDesc is not set for action', () => {
+    it('should return no description if disable does not return string', () => {
       expect(component.useDisableDesc(deleteAction)).toBeUndefined();
     });
   });
index f494ae19f555ecc646c9ce6b3fd732d266aaf14c..80d6fad738d1ef7298f188c0909b19cd6106aeb9 100644 (file)
@@ -146,6 +146,11 @@ export class TableActionsComponent implements OnInit {
   }
 
   useDisableDesc(action: CdTableAction) {
-    return action.disableDesc && action.disableDesc(this.selection);
+    if (action.disable) {
+      const result = action.disable(this.selection);
+      return _.isString(result) ? result : undefined;
+    }
+
+    return undefined;
   }
 }
index 9af10625ae65bd3226dfeec3ff4cf6b5cee5660e..70f06e506c365637d645eb8a7f6e3c835ff8520c 100644 (file)
@@ -19,17 +19,15 @@ export class CdTableAction {
   // The font awesome icon that will be used
   icon: string;
 
-  // You can define the condition to disable the action.
-  // By default all 'update' and 'delete' actions will only be enabled
-  // if one selection is made and no task is running on the selected item.
-  disable?: (_: CdTableSelection) => boolean;
-
   /**
+   * You can define the condition to disable the action.
+   * By default all 'update' and 'delete' actions will only be enabled
+   * if one selection is made and no task is running on the selected item.`
+   *
    * In some cases you might want to give the user a hint why a button is
-   * disabled. The specified message will be shown to the user as a button
-   * tooltip.
-   */
-  disableDesc?: (_: CdTableSelection) => string | undefined;
+   * disabled. This is achieved by returning a string.
+   * */
+  disable?: (_: CdTableSelection) => boolean | string;
 
   /**
    * Defines if the button can become 'primary' (displayed as button and not