Merge pull request #28095 from Devp00l/wip-39702

mgr/dashboard: OSD custom action button removal

Reviewed-by: Kanika Murarka <kmurarka@redhat.com>
Reviewed-by: Patrick Nawracay <pnawracay@suse.com>
Reviewed-by: Ricardo Marques <rimarques@suse.com>
Reviewed-by: Tatjana Dehler <tdehler@suse.com>

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.html

@@ -11,39 +11,17 @@
         <cd-table-actions [permission]="permissions.osd"
                           [selection]="selection"
                           class="btn-group"
+                          id="osd-actions"
                           [tableActions]="tableActions">
         </cd-table-actions>
-
-        <div class="btn-group"
-             dropdown
-             *ngIf="advancedTableActions.length > 0">
-          <button type="button"
-                  class="btn btn-sm btn-default btn-label tc_configureCluster"
-                  (click)="advancedTableActions[0].click()">
-            <i class="fa fa-fw {{ advancedTableActions[0].icon }}"></i><span>{{ advancedTableActions[0].name }}</span>
-          </button>
-          <button type="button"
-                  dropdownToggle
-                  class="btn btn-sm btn-default dropdown-toggle dropdown-toggle-split"
-                  *ngIf="advancedTableActions.length > 1">
-            <span class="caret caret-black"></span>
-          </button>
-          <ul *dropdownMenu
-              class="dropdown-menu"
-              role="menu">
-            <ng-container *ngFor="let action of advancedTableActions | slice:1">
-              <li role="menuitem">
-                <a class="dropdown-item"
-                   (click)="action.click()">
-                  <i class="fa fa-fw {{ action.icon }}"
-                     aria-hidden="true">
-                  </i>
-                  <ng-container>{{ action.name }}</ng-container>
-                </a>
-              </li>
-            </ng-container>
-          </ul>
-        </div>
+        <cd-table-actions [permission]="{read: true}"
+                          [selection]="selection"
+                          dropDownOnly="Cluster-wide configuration"
+                          btnColor="default"
+                          class="btn-group"
+                          id="cluster-wide-actions"
+                          [tableActions]="clusterWideActions">
+        </cd-table-actions>
       </div>
 
       <cd-osd-details cdTableDetail

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.scss

@@ -1,3 +0,0 @@
-.caret.caret-black {
-  color: #000000;
-}

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.spec.ts

@@ -37,7 +37,10 @@ describe('OsdListComponent', () => {
 
   const fakeAuthStorageService = {
     getPermissions: () => {
-      return new Permissions({ osd: ['read', 'update', 'create', 'delete'] });
+      return new Permissions({
+        'config-opt': ['read', 'update', 'create', 'delete'],
+        osd: ['read', 'update', 'create', 'delete']
+      });
     }
   };
 
@@ -92,7 +95,6 @@ describe('OsdListComponent', () => {
 
   beforeEach(() => {
     fixture = TestBed.createComponent(OsdListComponent);
-    fixture.detectChanges();
     component = fixture.componentInstance;
     osdService = TestBed.get(OsdService);
     modalServiceShowSpy = spyOn(TestBed.get(BsModalService), 'show').and.stub();
@@ -104,6 +106,7 @@ describe('OsdListComponent', () => {
   });
 
   it('should have columns that are sortable', () => {
+    fixture.detectChanges();
     expect(component.columns.every((column) => Boolean(column.prop))).toBeTruthy();
   });
 
@@ -170,6 +173,49 @@ describe('OsdListComponent', () => {
     });
   });
 
+  describe('show osd actions as defined', () => {
+    const getOsdActions = () => {
+      fixture.detectChanges();
+      return fixture.debugElement.query(By.css('#cluster-wide-actions')).componentInstance
+        .dropDownActions;
+    };
+
+    it('shows osd actions after osd-actions', () => {
+      fixture.detectChanges();
+      expect(fixture.debugElement.query(By.css('#cluster-wide-actions'))).toBe(
+        fixture.debugElement.queryAll(By.directive(TableActionsComponent))[1]
+      );
+    });
+
+    it('shows both osd actions', () => {
+      const osdActions = getOsdActions();
+      expect(osdActions).toEqual(component.clusterWideActions);
+      expect(osdActions.length).toBe(3);
+    });
+
+    it('shows only "Flags" action', () => {
+      component.permissions.configOpt.read = false;
+      const osdActions = getOsdActions();
+      expect(osdActions[0].name).toBe('Flags');
+      expect(osdActions.length).toBe(1);
+    });
+
+    it('shows only "Recovery Priority" action', () => {
+      component.permissions.osd.read = false;
+      const osdActions = getOsdActions();
+      expect(osdActions[0].name).toBe('Recovery Priority');
+      expect(osdActions[1].name).toBe('PG scrub');
+      expect(osdActions.length).toBe(2);
+    });
+
+    it('shows no osd actions', () => {
+      component.permissions.configOpt.read = false;
+      component.permissions.osd.read = false;
+      const osdActions = getOsdActions();
+      expect(osdActions).toEqual([]);
+    });
+  });
+
   describe('show table actions as defined', () => {
     let tableActions: TableActionsComponent;
     let scenario: { fn; empty; single };
@@ -177,7 +223,7 @@ describe('OsdListComponent', () => {
 
     const getTableActionComponent = () => {
       fixture.detectChanges();
-      return fixture.debugElement.query(By.directive(TableActionsComponent)).componentInstance;
+      return fixture.debugElement.query(By.css('#osd-actions')).componentInstance;
     };
 
     beforeEach(() => {
@@ -201,6 +247,10 @@ describe('OsdListComponent', () => {
   });
 
   describe('test table actions in submenu', () => {
+    beforeEach(() => {
+      fixture.detectChanges();
+    });
+
     beforeEach(fakeAsync(() => {
       // The menu needs a click to render the dropdown!
       const dropDownToggle = fixture.debugElement.query(By.css('.dropdown-toggle'));

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.ts

@@ -47,7 +47,7 @@ export class OsdListComponent implements OnInit {
   tableActions: CdTableAction[];
   bsModalRef: BsModalRef;
   columns: CdTableColumn[];
-  advancedTableActions: any[];
+  clusterWideActions: CdTableAction[];
 
   osds = [];
   selection = new CdTableSelection();
@@ -148,29 +148,32 @@ export class OsdListComponent implements OnInit {
         icon: 'fa-remove'
       }
     ];
-    this.advancedTableActions = [
+  }
+
+  ngOnInit() {
+    this.clusterWideActions = [
       {
-        name: this.i18n('Cluster-wide Flags'),
+        name: this.i18n('Flags'),
         icon: 'fa-flag',
         click: () => this.configureFlagsAction(),
-        permission: this.permissions.osd.read
+        permission: 'read',
+        visible: () => this.permissions.osd.read
       },
       {
-        name: this.i18n('Cluster-wide Recovery Priority'),
+        name: this.i18n('Recovery Priority'),
         icon: 'fa-cog',
         click: () => this.configureQosParamsAction(),
-        permission: this.permissions.configOpt.read
+        permission: 'read',
+        visible: () => this.permissions.configOpt.read
       },
       {
         name: this.i18n('PG scrub'),
         icon: 'fa-stethoscope',
         click: () => this.configurePgScrubAction(),
-        permission: this.permissions.configOpt.read
+        permission: 'read',
+        visible: () => this.permissions.configOpt.read
       }
     ];
-  }
-
-  ngOnInit() {
     this.columns = [
       { prop: 'host.name', name: this.i18n('Host') },
       { prop: 'id', name: this.i18n('ID'), cellTransformation: CellTemplate.bold },
@@ -199,8 +202,6 @@ export class OsdListComponent implements OnInit {
         cellTransformation: CellTemplate.perSecond
       }
     ];
-
-    this.removeActionsWithNoPermissions();
   }
 
   get hasOsdSelected() {
@@ -336,16 +337,4 @@ export class OsdListComponent implements OnInit {
   configurePgScrubAction() {
     this.bsModalRef = this.modalService.show(OsdPgScrubModalComponent, { class: 'modal-lg' });
   }
-
-  /**
-   * Removes all actions from 'advancedTableActions' that need a permission the user doesn't have.
-   */
-  private removeActionsWithNoPermissions() {
-    if (!this.permissions) {
-      this.advancedTableActions = [];
-      return;
-    }
-
-    this.advancedTableActions = this.advancedTableActions.filter((action) => action.permission);
-  }
 }

src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-actions/table-actions.component.html

@@ -2,7 +2,7 @@
      dropdown>
   <ng-container *ngIf="getCurrentButton() as action">
     <button type="button"
-            class="btn btn-sm btn-primary"
+            class="btn btn-sm btn-{{btnColor}}"
             [ngClass]="{'disabled': disableSelectionAction(action)}"
             (click)="useClickAction(action)"
             [routerLink]="useRouterLink(action)">
@@ -12,10 +12,10 @@
   <button type="button"
           dropdownToggle
           *ngIf="showDropDownActions()"
-          class="btn btn-sm btn-primary dropdown-toggle dropdown-toggle-split">
-    <ng-container *ngIf="onlyDropDown">{{ onlyDropDown }}</ng-container>
+          class="btn btn-sm btn-{{btnColor}} dropdown-toggle dropdown-toggle-split">
+    <ng-container *ngIf="dropDownOnly">{{ dropDownOnly }} </ng-container>
     <span class="caret"></span>
-    <span *ngIf="!onlyDropDown"
+    <span *ngIf="!dropDownOnly"
           class="sr-only"></span>
   </button>
   <ul *dropdownMenu

src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-actions/table-actions.component.spec.ts

@@ -20,7 +20,7 @@ describe('TableActionsComponent', () => {
   let scenario;
   let permissionHelper: PermissionHelper;
 
-  const setUpTableActions = () => {
+  const getTableActionComponent = (): TableActionsComponent => {
     component.tableActions = [
       addAction,
       editAction,
@@ -29,10 +29,6 @@ describe('TableActionsComponent', () => {
       copyAction,
       deleteAction
     ];
-  };
-
-  const getTableActionComponent = (): TableActionsComponent => {
-    setUpTableActions();
     component.ngOnInit();
     return component;
   };
@@ -236,7 +232,7 @@ describe('TableActionsComponent', () => {
       permissionHelper.testScenarios(scenario);
     });
 
-    it('should not get any button with no permissions', () => {
+    it('should not get any button with no permissions, except the true action', () => {
       hiddenScenario();
       permissionHelper.setPermissionsAndGetActions(0, 0, 0);
       permissionHelper.testScenarios(scenario);
@@ -244,7 +240,7 @@ describe('TableActionsComponent', () => {
 
     it('should not get any button if only a drop down should be shown', () => {
       hiddenScenario();
-      component.onlyDropDown = 'Drop down label';
+      component.dropDownOnly = 'Drop down label that is shown';
       permissionHelper.setPermissionsAndGetActions(1, 1, 1);
       permissionHelper.testScenarios(scenario);
     });
@@ -269,13 +265,56 @@ describe('TableActionsComponent', () => {
     });
   });
 
-  describe('with drop down only', () => {
-    beforeEach(() => {
-      component.onlyDropDown = 'displayMe';
+  describe('all visible actions with all different permissions', () => {
+    it('with create, update and delete', () => {
+      permissionHelper.setPermissionsAndGetActions(1, 1, 1);
+      expect(component.dropDownActions).toEqual([
+        addAction,
+        editAction,
+        unprotectAction,
+        copyAction,
+        deleteAction
+      ]);
     });
 
-    it('should not return any button with getCurrentButton', () => {
-      expect(component.getCurrentButton()).toBeFalsy();
+    it('with create and delete', () => {
+      permissionHelper.setPermissionsAndGetActions(1, 0, 1);
+      expect(component.dropDownActions).toEqual([addAction, copyAction, deleteAction]);
+    });
+
+    it('with create and update', () => {
+      permissionHelper.setPermissionsAndGetActions(1, 1, 0);
+      expect(component.dropDownActions).toEqual([
+        addAction,
+        editAction,
+        unprotectAction,
+        copyAction
+      ]);
+    });
+
+    it('with create', () => {
+      permissionHelper.setPermissionsAndGetActions(1, 0, 0);
+      expect(component.dropDownActions).toEqual([addAction, copyAction]);
+    });
+
+    it('with update and delete', () => {
+      permissionHelper.setPermissionsAndGetActions(0, 1, 1);
+      expect(component.dropDownActions).toEqual([editAction, unprotectAction, deleteAction]);
+    });
+
+    it('with update', () => {
+      permissionHelper.setPermissionsAndGetActions(0, 1, 0);
+      expect(component.dropDownActions).toEqual([editAction, unprotectAction]);
+    });
+
+    it('with delete', () => {
+      permissionHelper.setPermissionsAndGetActions(0, 0, 1);
+      expect(component.dropDownActions).toEqual([deleteAction]);
+    });
+
+    it('without any', () => {
+      permissionHelper.setPermissionsAndGetActions(0, 0, 0);
+      expect(component.dropDownActions).toEqual([]);
     });
   });
 

src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table-actions/table-actions.component.ts

@@ -18,12 +18,14 @@ export class TableActionsComponent implements OnInit {
   selection: CdTableSelection;
   @Input()
   tableActions: CdTableAction[];
+  @Input()
+  btnColor = 'primary';
 
   // Use this if you just want to display a drop down button,
   // labeled with the given text, with all actions in it.
   // This disables the main action button.
   @Input()
-  onlyDropDown?: string;
+  dropDownOnly?: string;
 
   // Array with all visible actions
   dropDownActions: CdTableAction[] = [];
@@ -74,7 +76,7 @@ export class TableActionsComponent implements OnInit {
    * @returns {CdTableAction}
    */
   getCurrentButton(): CdTableAction {
-    if (this.onlyDropDown) {
+    if (this.dropDownOnly) {
       return;
     }
     let buttonAction = this.dropDownActions.find((tableAction) => this.showableAction(tableAction));
@@ -115,11 +117,11 @@ export class TableActionsComponent implements OnInit {
    * @returns {Boolean}
    */
   disableSelectionAction(action: CdTableAction): Boolean {
-    const permission = action.permission;
     const disable = action.disable;
     if (disable) {
       return Boolean(disable(this.selection));
     }
+    const permission = action.permission;
     const selected = this.selection.hasSingleSelection && this.selection.first();
     return Boolean(
       ['update', 'delete'].includes(permission) && (!selected || selected.cdExecuting)

src/pybind/mgr/dashboard/frontend/src/styles.scss

@@ -141,9 +141,6 @@ fieldset[disabled] .btn-primary.active {
   color: $color-primary;
   background-color: $color-button-badge;
 }
-.btn-primary .caret {
-  color: $color-button-caret;
-}
 .btn-default {
   border-radius: $button-radius;
 }
@@ -244,10 +241,6 @@ button.btn.btn-label > i.fa {
   padding-left: 30px;
   padding-right: 30px;
 }
-/* Caret */
-.caret {
-  color: $color-caret-text;
-}
 /* Progressbar */
 .progress-bar {
   background-image: none !important;