From ff64ffd3640818eb0e474216361429c077f6ce6c Mon Sep 17 00:00:00 2001 From: =?utf8?q?Stephan=20M=C3=BCller?= Date: Mon, 29 Oct 2018 16:43:38 +0100 Subject: [PATCH] mgr/dashboard: Cleanup of OSD list methods MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Some test changes were required, too. Fixes: https://tracker.ceph.com/issues/36615 Signed-off-by: Stephan Müller --- .../osd/osd-list/osd-list.component.spec.ts | 122 ++++++++++-------- .../osd/osd-list/osd-list.component.ts | 51 +++----- 2 files changed, 89 insertions(+), 84 deletions(-) diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.spec.ts index 2814989c12d..71723b5cbc7 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.spec.ts @@ -35,6 +35,36 @@ describe('OsdListComponent', () => { } }; + const getTableAction = (name) => component.tableActions.find((action) => action.name === name); + + const setFakeSelection = () => { + // Default data and selection + const selection = [{ id: 1 }]; + const data = [{ id: 1 }]; + + // Table data and selection + component.selection = new CdTableSelection(); + component.selection.selected = selection; + component.selection.update(); + component.osds = data; + }; + + const openActionModal = (actionName) => { + setFakeSelection(); + getTableAction(actionName).click(); + }; + + /** + * The following modals are called after the information about their + * safety to destroy/remove/mark them lost has been retrieved, hence + * we will have to fake its request to be able to open those modals. + */ + const mockSafeToDestroy = () => { + spyOn(TestBed.get(OsdService), 'safeToDestroy').and.callFake(() => + of({ 'safe-to-destroy': true }) + ); + }; + configureTestBed({ imports: [ HttpClientTestingModule, @@ -59,18 +89,6 @@ describe('OsdListComponent', () => { modalServiceShowSpy = spyOn(TestBed.get(BsModalService), 'show').and.stub(); }); - const setFakeSelection = () => { - // Default data and selection - const selection = [{ id: 1 }]; - const data = [{ id: 1 }]; - - // Table data and selection - component.selection = new CdTableSelection(); - component.selection.selected = selection; - component.selection.update(); - component.osds = data; - }; - it('should create', () => { fixture.detectChanges(); expect(component).toBeTruthy(); @@ -142,72 +160,70 @@ describe('OsdListComponent', () => { describe('tests if all modals are opened correctly', () => { /** * Helper function to check if a function opens a modal - * @param fn + * * @param modalClass - The expected class of the modal */ - const expectOpensModal = (fn, modalClass): void => { - setFakeSelection(); - fn(); + const expectOpensModal = (actionName: string, modalClass): void => { + openActionModal(actionName); expect(modalServiceShowSpy.calls.any()).toBe(true, 'modalService.show called'); - expect(modalServiceShowSpy.calls.first()).toBeTruthy(); expect(modalServiceShowSpy.calls.first().args[0]).toBe(modalClass); modalServiceShowSpy.calls.reset(); }; - it('opens the appropriate modal', () => { - expectOpensModal(() => component.reweight(), OsdReweightModalComponent); - expectOpensModal(() => component.markOut(), ConfirmationModalComponent); - expectOpensModal(() => component.markIn(), ConfirmationModalComponent); - expectOpensModal(() => component.markDown(), ConfirmationModalComponent); - - // The following modals are called after the information about their - // safety to destroy/remove/mark them lost has been retrieved, hence - // we will have to fake its request to be able to open those modals. - spyOn(TestBed.get(OsdService), 'safeToDestroy').and.callFake(() => - of({ 'safe-to-destroy': true }) - ); + it('opens the reweight modal', () => { + expectOpensModal('Reweight', OsdReweightModalComponent); + }); - expectOpensModal(() => component.markLost(), CriticalConfirmationModalComponent); - expectOpensModal(() => component.remove(), CriticalConfirmationModalComponent); - expectOpensModal(() => component.destroy(), CriticalConfirmationModalComponent); + it('opens all confirmation modals', () => { + const modalClass = ConfirmationModalComponent; + expectOpensModal('Mark Out', modalClass); + expectOpensModal('Mark In', modalClass); + expectOpensModal('Mark Down', modalClass); + }); + + it('opens all critical confirmation modals', () => { + const modalClass = CriticalConfirmationModalComponent; + mockSafeToDestroy(); + expectOpensModal('Mark Lost', modalClass); + expectOpensModal('Remove', modalClass); + expectOpensModal('Destroy', modalClass); }); }); describe('tests if the correct methods are called on confirmation', () => { - const expectOsdServiceMethodCalled = (fn: Function, osdServiceMethodName: string): void => { - setFakeSelection(); + const expectOsdServiceMethodCalled = ( + actionName: string, + osdServiceMethodName: string + ): void => { const osdServiceSpy = spyOn(TestBed.get(OsdService), osdServiceMethodName).and.callFake( () => EMPTY ); - - modalServiceShowSpy.calls.reset(); - fn(); // calls show on BsModalService - // Calls onSubmit given to `bsModalService.show()` + openActionModal(actionName); const initialState = modalServiceShowSpy.calls.first().args[1].initialState; - const action = initialState.onSubmit || initialState.submitAction; - action.call(component); + const submit = initialState.onSubmit || initialState.submitAction; + submit.call(component); expect(osdServiceSpy.calls.count()).toBe(1); expect(osdServiceSpy.calls.first().args[0]).toBe(1); - modalServiceShowSpy.calls.reset(); + + // Reset spies to be able to recreate them osdServiceSpy.calls.reset(); + modalServiceShowSpy.calls.reset(); }; - it('calls the corresponding service methods', () => { - // Purposely `reweight` - expectOsdServiceMethodCalled(() => component.markOut(), 'markOut'); - expectOsdServiceMethodCalled(() => component.markIn(), 'markIn'); - expectOsdServiceMethodCalled(() => component.markDown(), 'markDown'); - - spyOn(TestBed.get(OsdService), 'safeToDestroy').and.callFake(() => - of({ 'safe-to-destroy': true }) - ); + it('calls the corresponding service methods in confirmation modals', () => { + expectOsdServiceMethodCalled('Mark Out', 'markOut'); + expectOsdServiceMethodCalled('Mark In', 'markIn'); + expectOsdServiceMethodCalled('Mark Down', 'markDown'); + }); - expectOsdServiceMethodCalled(() => component.markLost(), 'markLost'); - expectOsdServiceMethodCalled(() => component.remove(), 'remove'); - expectOsdServiceMethodCalled(() => component.destroy(), 'destroy'); + it('calls the corresponding service methods in critical confirmation modals', () => { + mockSafeToDestroy(); + expectOsdServiceMethodCalled('Mark Lost', 'markLost'); + expectOsdServiceMethodCalled('Remove', 'remove'); + expectOsdServiceMethodCalled('Destroy', 'destroy'); }); }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.ts index bbd94075bc7..bfa255662a4 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.ts @@ -83,42 +83,55 @@ export class OsdListComponent implements OnInit { { name: 'Mark Out', permission: 'update', - click: () => this.markOut(), + click: () => this.showConfirmationModal('out', this.osdService.markOut), disable: () => this.isNotSelectedOrInState('out'), icon: 'fa-arrow-left' }, { name: 'Mark In', permission: 'update', - click: () => this.markIn(), + click: () => this.showConfirmationModal('in', this.osdService.markIn), disable: () => this.isNotSelectedOrInState('in'), icon: 'fa-arrow-right' }, { name: 'Mark Down', permission: 'update', - click: () => this.markDown(), + click: () => this.showConfirmationModal('down', this.osdService.markDown), disable: () => this.isNotSelectedOrInState('down'), icon: 'fa-arrow-down' }, { - name: 'Mark lost', + name: 'Mark Lost', permission: 'delete', - click: () => this.markLost(), + click: () => + this.showCriticalConfirmationModal( + 'Mark', + 'OSD lost', + 'marked lost', + this.osdService.markLost + ), disable: () => this.isNotSelectedOrInState('up'), icon: 'fa-unlink' }, { name: 'Remove', permission: 'delete', - click: () => this.remove(), + click: () => + this.showCriticalConfirmationModal('Remove', 'OSD', 'removed', this.osdService.remove), disable: () => this.isNotSelectedOrInState('up'), icon: 'fa-remove' }, { name: 'Destroy', permission: 'delete', - click: () => this.destroy(), + click: () => + this.showCriticalConfirmationModal( + 'destroy', + 'OSD', + 'destroyed', + this.osdService.destroy + ), disable: () => this.isNotSelectedOrInState('up'), icon: 'fa-eraser' } @@ -238,18 +251,6 @@ export class OsdListComponent implements OnInit { }); } - markOut() { - this.showConfirmationModal('out', this.osdService.markOut); - } - - markIn() { - this.showConfirmationModal('in', this.osdService.markIn); - } - - markDown() { - this.showConfirmationModal('down', this.osdService.markDown); - } - reweight() { const selectedOsd = this.osds.filter((o) => o.id === this.selection.first().id).pop(); this.modalService.show(OsdReweightModalComponent, { @@ -285,16 +286,4 @@ export class OsdListComponent implements OnInit { }); }); } - - markLost() { - this.showCriticalConfirmationModal('Mark', 'OSD lost', 'marked lost', this.osdService.markLost); - } - - remove() { - this.showCriticalConfirmationModal('Remove', 'OSD', 'removed', this.osdService.remove); - } - - destroy() { - this.showCriticalConfirmationModal('destroy', 'OSD', 'destroyed', this.osdService.destroy); - } } -- 2.47.3