From 2f1c977ec6d8a25b4e5cf7ce7f3155eac69761a6 Mon Sep 17 00:00:00 2001 From: Tiago Melo Date: Fri, 24 Jul 2020 15:59:35 +0000 Subject: [PATCH] mgr/dashboard: Merge disable and disableDesc Fixes: https://tracker.ceph.com/issues/46750 Signed-off-by: Tiago Melo --- .../iscsi-target-list.component.spec.ts | 12 ++---- .../iscsi-target-list.component.ts | 29 +++++++------- .../block/rbd-list/rbd-list.component.spec.ts | 2 +- .../ceph/block/rbd-list/rbd-list.component.ts | 17 ++++---- .../rbd-namespace-list.component.ts | 14 +++---- .../rbd-snapshot-actions.model.ts | 18 ++++----- .../cluster/hosts/hosts.component.spec.ts | 16 ++++---- .../app/ceph/cluster/hosts/hosts.component.ts | 39 ++++++++++--------- .../mgr-module-list.component.ts | 14 +++---- .../pool-list/pool-list.component.spec.ts | 8 +++- .../pool/pool-list/pool-list.component.ts | 15 ++++--- .../table-actions.component.spec.ts | 6 +-- .../table-actions/table-actions.component.ts | 7 +++- .../src/app/shared/models/cd-table-action.ts | 16 ++++---- 14 files changed, 108 insertions(+), 105 deletions(-) diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-list/iscsi-target-list.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-list/iscsi-target-list.component.spec.ts index 2c8657f3577..1694b4bbbee 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-list/iscsi-target-list.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-list/iscsi-target-list.component.spec.ts @@ -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(); }); }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-list/iscsi-target-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-list/iscsi-target-list.component.ts index dd96295ca0e..3a417b5b0f3 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-list/iscsi-target-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-list/iscsi-target-list.component.ts @@ -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[] { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.spec.ts index 193a338feb7..a8a40678ac0 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.spec.ts @@ -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.' ); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.ts index 18ac272143e..a6d8a5518e7 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.ts @@ -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()) + ); } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-namespace-list/rbd-namespace-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-namespace-list/rbd-namespace-list.component.ts index 8d449f3ce79..cd66c7cf9c4 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-namespace-list/rbd-namespace-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-namespace-list/rbd-namespace-list.component.ts @@ -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(); } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-actions.model.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-actions.model.ts index 040f5824de3..c966c52974b 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-actions.model.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-actions.model.ts @@ -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; } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.spec.ts index eb8ad20b035..0c2931f3af7 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.spec.ts @@ -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(); }); }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.ts index d1bea0bdd52..871bacd3d0d 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/hosts/hosts.component.ts @@ -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) { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.ts index 8e1da89d0f5..b0f4a9fefab 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-list/mgr-module-list.component.ts @@ -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'); } /** diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.spec.ts index 1dbd72afb2b..06506f19614 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.spec.ts @@ -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(); }); }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.ts index eae76ef70c7..fe6e4afa003 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-list/pool-list.component.ts @@ -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) { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-actions/table-actions.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-actions/table-actions.component.spec.ts index 2593a59b2be..e7b2b73ee6a 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-actions/table-actions.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-actions/table-actions.component.spec.ts @@ -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(); }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-actions/table-actions.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-actions/table-actions.component.ts index f494ae19f55..80d6fad738d 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-actions/table-actions.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-actions/table-actions.component.ts @@ -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; } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/cd-table-action.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/cd-table-action.ts index 9af10625ae6..70f06e506c3 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/cd-table-action.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/cd-table-action.ts @@ -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 -- 2.39.5