diff --git a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/StorageContainer.scss b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/StorageContainer.scss index 53a848111d..ecce04a79d 100644 --- a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/StorageContainer.scss +++ b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/StorageContainer.scss @@ -1,28 +1,17 @@ -@import '../../../../common/colors'; - .vmware-storage-container { - display: flex; - flex-direction: column; - - .storage-header { - padding: 10px 0; - margin-bottom: 10px; - border-bottom: 1px solid $pf-black-200; - - .storage-title { - font-size: 18px; - } - - .storage-controller-buttons { - text-align: right; + .pf-v5-c-expandable-section__toggle-text { + font-size: var(--pf-v5-global--FontSize--md); + } - > button { - margin-left: 20px; - } - } + .pf-v5-c-menu-toggle { + width: 80%; + max-width: 880px; } - .controller-container { - margin-bottom: 20px; + .disks-container { + padding-top: var(--pf-v5-global--spacer--md); + .disk-form-status-container { + align-self: center; + } } } diff --git a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/StorageContainer.fixtures.js b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/StorageContainer.fixtures.js index 5ecfe56003..1c6c0b4c6a 100644 --- a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/StorageContainer.fixtures.js +++ b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/StorageContainer.fixtures.js @@ -45,7 +45,7 @@ export const vmwareData = { volumes: [ { thin: true, - name: 'Hard disk', + name: 'Hard disk 1', mode: 'persistent', controllerKey: 1000, sizeGb: 10, @@ -61,7 +61,7 @@ export const hiddenFieldValue = { { controllerKey: 1000, mode: 'persistent', - name: 'Hard disk', + name: 'Hard disk 1', sizeGb: 10, thin: true, }, diff --git a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/integration.test.js b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/integration.test.js index 3cc657bb9b..6a00c9a2d2 100644 --- a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/integration.test.js +++ b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/integration.test.js @@ -1,50 +1,121 @@ import React from 'react'; +import { screen, act } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import userEvent from '@testing-library/user-event'; -import IntegrationTestHelper from '../../../../../common/IntegrationTestHelper'; +import { rtlHelpers } from '../../../../../common/rtlTestHelpers'; import { vmwareData, hiddenFieldValue } from './StorageContainer.fixtures'; -import hostReducers from '../../../../../redux/reducers/hosts'; import StorageContainer from '../'; -jest.unmock('jquery'); - -let helper; -let component = null; - describe('StorageContainer integration test', () => { + let store; + let container; + jest.useFakeTimers(); beforeEach(() => { - helper = new IntegrationTestHelper({ hosts: hostReducers }); - component = helper.mount(); + const initialState = { + hosts: { + storage: { + vmware: { + controllers: [], + config: {}, + cluster: '', + volumes: [], + datastores: [], + datastoresLoading: false, + datastoresError: undefined, + storagePods: [], + storagePodsLoading: false, + storagePodsError: undefined, + }, + }, + }, + }; + + const renderResult = rtlHelpers.renderWithStore( + , + initialState + ); + store = renderResult.store; + container = renderResult.container; }); it('render hidden field correctly', () => { - expect( - JSON.parse(component.find('#controller_hidden').props().value) - ).toEqual(hiddenFieldValue); + const hiddenField = container.querySelector('#controller_hidden'); + expect(hiddenField).toBeInTheDocument(); + expect(JSON.parse(hiddenField.value)).toEqual(hiddenFieldValue); + }); + + it('adds a disk when button is clicked', async () => { + const diskContainers = container.querySelectorAll('.disk-container'); + expect(diskContainers).toHaveLength(1); + + const addButton = screen.getByRole('button', { + name: /add another volume/i, + }); + + await act(async () => { + await userEvent.click(addButton); + }); + + const updatedContainers = container.querySelectorAll('.disk-container'); + expect(updatedContainers).toHaveLength(2); }); - it.each([['disk'], ['controller']])( - 'adds a %s when button is clicked', - device => { - expect(component.render().find(`.${device}-container`)).toHaveLength(1); - component.find(`button.btn-add-${device}`).simulate('click'); - expect(component.render().find(`.${device}-container`)).toHaveLength(2); - } - ); - - it('removes controller when one is selected', () => { - expect(component.render().find('.controller-container')).toHaveLength(1); - component.find('button.btn-remove-controller').simulate('click'); - expect(component.render().find('.controller-container')).toHaveLength(0); + it('adds a controller when button is clicked', async () => { + const controllerContainers = container.querySelectorAll( + '.controller-container' + ); + expect(controllerContainers).toHaveLength(1); + + const addButton = screen.getByRole('button', { + name: /create another controller/i, + }); + + await act(async () => { + await userEvent.click(addButton); + }); + + const updatedContainers = container.querySelectorAll( + '.controller-container' + ); + expect(updatedContainers).toHaveLength(2); + }); + + it('removes controller when one is selected', async () => { + const controllerContainers = container.querySelectorAll( + '.controller-container' + ); + expect(controllerContainers).toHaveLength(1); + + const removeButton = screen.getByRole('button', { + name: /remove controller/i, + }); + await userEvent.click(removeButton); + + const updatedContainers = container.querySelectorAll( + '.controller-container' + ); + expect(updatedContainers).toHaveLength(0); }); - it('changes controller type when one is selected', () => { - const cntrlType = () => - helper.getState().hosts.storage.vmware.controllers[0].type; + it('changes controller type when one is selected', async () => { + const getControllerType = () => + store.getState().hosts.storage.vmware.controllers[0].type; - expect(cntrlType()).toEqual('VirtualLsiLogicController'); - component - .find('.controller-type-container select') - .simulate('change', { target: { value: 'ParaVirtualSCSIController' } }); - expect(cntrlType()).toEqual('ParaVirtualSCSIController'); + expect(getControllerType()).toEqual('VirtualLsiLogicController'); + const controllerTypeButton = screen.getByRole('button', { + name: 'LSI Logic Parallel', + }); + expect(controllerTypeButton).toBeInTheDocument(); + await act(async () => { + await userEvent.click(controllerTypeButton); + }); + await act(async () => { + await userEvent.click( + screen.getByRole('option', { name: 'VMware Paravirtual' }) + ); + }); + act(() => jest.advanceTimersByTime(1000)); + expect(getControllerType()).toEqual('ParaVirtualSCSIController'); }); }); diff --git a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/controller/__snapshots__/controller.test.js.snap b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/controller/__snapshots__/controller.test.js.snap deleted file mode 100644 index 4ef829b435..0000000000 --- a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/controller/__snapshots__/controller.test.js.snap +++ /dev/null @@ -1,114 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`StorageContainer should render controller 1`] = ` -
-
-
- -
-
- updateStoragePod(newValues)} - options={storagePods} - allowClear - key="storagePodsSelect" - status={storagePodsStatus} - errorMessage={storagePodsError} - className="storage-pod" + + )} + + + - )} - {!(storagePod && storagePod.length) && ( - updateDisk('mode', newValues)} - options={diskModeTypes} - /> - - `${v} GB`} - parser={str => str.replace(/\D/g, '')} - className="text-vmware-size" - onChange={newValues => updateDisk('sizeGb', newValues)} - label={__('Size (GB)')} - /> - - { - updateDisk('thin', newValues); - newValues && updateDisk('eagerZero', false); - }} - /> - - { - updateDisk('eagerZero', newValues); - newValues && updateDisk('thin', false); - }} - /> +
); }; @@ -126,7 +111,7 @@ Disk.propTypes = { name: PropTypes.string, storagePod: PropTypes.string, datastore: PropTypes.string, - sizeGb: PropTypes.number, + sizeGb: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), thin: PropTypes.bool, eagerZero: PropTypes.bool, mode: PropTypes.string, @@ -138,13 +123,15 @@ Disk.propTypes = { storagePodsError: PropTypes.string, removeDisk: PropTypes.func, updateDisk: PropTypes.func, + volumeNumber: PropTypes.number, + defaultExpanded: PropTypes.bool, }; Disk.defaultProps = { name: '', storagePod: '', datastore: '', - sizeGb: null, + sizeGb: 1, thin: false, eagerZero: false, mode: '', @@ -156,6 +143,8 @@ Disk.defaultProps = { storagePodsError: undefined, removeDisk: noop, updateDisk: noop, + volumeNumber: 1, + defaultExpanded: false, }; export default Disk; diff --git a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/controller/index.js b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/controller/index.js index d761337b31..8999a2cd15 100644 --- a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/controller/index.js +++ b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/controller/index.js @@ -1,18 +1,24 @@ /* eslint-disable camelcase, no-mixed-operators, no-param-reassign */ -import { Button } from 'patternfly-react'; -import React from 'react'; +import { + Button, + Divider, + Flex, + FlexItem, + Tooltip, +} from '@patternfly/react-core'; +import { PlusCircleIcon, TrashIcon } from '@patternfly/react-icons'; +import { SimpleSelect } from '@patternfly/react-templates'; +import React, { useMemo } from 'react'; import PropTypes from 'prop-types'; import { number_to_human_size } from 'number_helpers'; -import Select from '../../../../common/forms/Select'; - import Disk from './disk'; import { sprintf, translate as __, } from '../../../../../../react_app/common/I18n'; import { noop } from '../../../../../common/helpers'; -import './controller.scss'; +import { MaxDisksPerController } from '../StorageContainer.consts'; const Controller = ({ addDiskEnabled, @@ -42,10 +48,6 @@ const Controller = ({ updateController({ [attribute]: getEventValue(e) }); }; - const _updateDisk = (uuid, attribute, e) => { - updateDisk(uuid, { [attribute]: getEventValue(e) }); - }; - const humanSize = number => number_to_human_size(number, { precision: 2 }); const datastoresStats = () => { @@ -85,11 +87,19 @@ const Controller = ({ }; const disks = () => - controllerVolumes.map(disk => ( + controllerVolumes.map((disk, index) => ( _updateDisk(disk.key, attribute, e)} + volumeNumber={index + 1} + defaultExpanded={ + config.vmExists || + index === 0 || + index === controllerVolumes.length - 1 + } + updateDisk={(attribute, e) => { + updateDisk(disk.key, { [attribute]: e }); + }} removeDisk={() => removeDisk(disk.key)} config={config} datastores={datastoresStats()} @@ -102,38 +112,86 @@ const Controller = ({ /> )); + const controllerTypeOptions = useMemo( + () => + Object.entries(config.controllerTypes).map(([key, value]) => ({ + content: value, + value: key, + selected: key === controller.type, + })), + [config.controllerTypes, controller.type] + ); + + const hasVolumes = controllerVolumes.length > 0; + const addVolumeDisabled = !addDiskEnabled || config.vmExists; + + let addVolumeTooltip = ''; + if (config.vmExists) { + addVolumeTooltip = __('Cannot add volumes to an existing VM'); + } else if (!addDiskEnabled) { + addVolumeTooltip = sprintf( + __('Maximum number of disks (%s) has been reached'), + MaxDisksPerController + ); + } + return (
-
-
- -
-
- + `${HARD_DISK_LABEL} ${oneBasedIndex}`; export const defaultControllerAttributes = { @@ -11,7 +13,7 @@ const _defaultDiskAttributes = { storagePod: '', thin: false, eagerZero: false, - name: __('Hard disk'), + name: vmwareDiskNameForIndex(1), mode: 'persistent', }; diff --git a/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/__snapshots__/vmware.test.js.snap b/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/__snapshots__/vmware.test.js.snap index 243caa84cc..eb17139264 100644 --- a/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/__snapshots__/vmware.test.js.snap +++ b/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/__snapshots__/vmware.test.js.snap @@ -19,7 +19,7 @@ Object { "eagerZero": false, "key": "5124c2d1-339b-11e9-98f5-5f761412a4c2", "mode": "persistent", - "name": "Hard disk", + "name": "Hard disk 1", "sizeGb": 10, "storagePod": "", "thin": false, @@ -28,9 +28,9 @@ Object { "controllerKey": 1001, "datastore": "", "eagerZero": false, - "key": "1547e1c0-309a-11e9-98f5-5f761412a4c2", + "key": "uuid-2", "mode": "persistent", - "name": "Hard disk", + "name": "Hard disk 2", "sizeGb": 10, "storagePod": "", "thin": false, @@ -58,7 +58,7 @@ Object { "eagerZero": false, "key": "5124c2d1-339b-11e9-98f5-5f761412a4c2", "mode": "persistent", - "name": "Hard disk", + "name": "Hard disk 1", "sizeGb": 10, "storagePod": "", "thin": false, @@ -67,9 +67,9 @@ Object { "controllerKey": 1000, "datastore": "", "eagerZero": false, - "key": "1547e1c0-309a-11e9-98f5-5f761412a4c2", + "key": "uuid-3", "mode": "persistent", - "name": "Hard disk", + "name": "Hard disk 2", "sizeGb": 10, "storagePod": "", "thin": false, @@ -91,9 +91,9 @@ Object { "controllerKey": 1000, "datastore": "", "eagerZero": false, - "key": "1547e1c0-309a-11e9-98f5-5f761412a4c2", + "key": "uuid-1", "mode": "persistent", - "name": "Hard disk", + "name": "Hard disk 1", "sizeGb": 10, "storagePod": "", "thin": false, @@ -117,7 +117,7 @@ Object { "eagerZero": false, "key": "5124c2d1-339b-11e9-98f5-5f761412a4c2", "mode": "persistent", - "name": "Hard disk", + "name": "Hard disk 1", "sizeGb": 10, "storagePod": "", "thin": false, @@ -126,9 +126,9 @@ Object { "controllerKey": 1000, "datastore": "", "eagerZero": false, - "key": "1547e1c0-309a-11e9-98f5-5f761412a4c2", + "key": "uuid-4", "mode": "persistent", - "name": "Hard disk", + "name": "Hard disk 2", "sizeGb": 10, "storagePod": "", "thin": false, diff --git a/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.fixtures.js b/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.fixtures.js index 4b9c982e34..dd846faa71 100644 --- a/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.fixtures.js +++ b/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.fixtures.js @@ -13,7 +13,7 @@ export const diskAttributes = { datastore: '', eagerZero: false, mode: 'persistent', - name: 'Hard disk', + name: 'Hard disk 1', sizeGb: 10, storagePod: '', thin: false, @@ -36,7 +36,7 @@ const _generateController = key => eagerZero: false, key: diskKey, mode: 'persistent', - name: 'Hard disk', + name: 'Hard disk 1', sizeGb: 10, storagePod: '', thin: false, diff --git a/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.js b/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.js index 089890ef55..d9d0ca0646 100644 --- a/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.js +++ b/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.js @@ -4,6 +4,10 @@ import { difference, head } from 'lodash'; import Immutable from 'seamless-immutable'; import uuidV1 from 'uuid/v1'; +import { + HARD_DISK_LABEL, + vmwareDiskNameForIndex, +} from '../../../actions/hosts/storage/vmware.consts'; import { VMWARE_CLUSTER_CHANGE, STORAGE_VMWARE_ADD_CONTROLLER, @@ -26,6 +30,17 @@ const initialState = Immutable({ volumes: [], }); +const normalizeDiskName = name => { + if (name === HARD_DISK_LABEL) return vmwareDiskNameForIndex(1); + return name; +}; + +const renumberVolumes = volumes => + volumes.map((vol, idx) => ({ + ...vol, + name: vmwareDiskNameForIndex(idx + 1), + })); + const availableControllerKeys = Array.from( { length: 8 }, (value, index) => 1000 + index @@ -45,7 +60,6 @@ export default (state = initialState, { type, payload, response }) => { case STORAGE_VMWARE_ADD_CONTROLLER: const availableKey = getAvailableKey(state.controllers); - // controller key is assigned here using getAvailableKey return state .update('controllers', ctrls => ctrls.concat( @@ -58,7 +72,8 @@ export default (state = initialState, { type, payload, response }) => { {}, payload.volume, { controllerKey: availableKey }, - { key: uuidV1() } + { key: uuidV1() }, + { name: vmwareDiskNameForIndex(volumes.length + 1) } ) ) ); @@ -69,6 +84,7 @@ export default (state = initialState, { type, payload, response }) => { ...payload.data, key: uuidV1(), controllerKey: payload.controllerKey, + name: vmwareDiskNameForIndex(state.volumes.length + 1), }) ); case STORAGE_VMWARE_REMOVE_CONTROLLER: @@ -77,8 +93,10 @@ export default (state = initialState, { type, payload, response }) => { ctrls.filter(ctrl => ctrl.key !== payload.controllerKey) ) .update('volumes', volumes => - volumes.filter( - volume => volume.controllerKey !== payload.controllerKey + renumberVolumes( + volumes.filter( + volume => volume.controllerKey !== payload.controllerKey + ) ) ); case STORAGE_VMWARE_UPDATE_CONTROLLER: @@ -95,7 +113,7 @@ export default (state = initialState, { type, payload, response }) => { case STORAGE_VMWARE_REMOVE_DISK: return state.set( 'volumes', - state.volumes.filter(v => v.key !== payload.key) + renumberVolumes(state.volumes.filter(v => v.key !== payload.key)) ); case STORAGE_VMWARE_INIT: const newState = { @@ -107,7 +125,11 @@ export default (state = initialState, { type, payload, response }) => { storagePods: [], storagePodsLoading: false, storagePodsError: undefined, - volumes: payload.volumes.map(volume => ({ ...volume, key: uuidV1() })), + volumes: payload.volumes.map(volume => ({ + ...volume, + key: uuidV1(), + name: normalizeDiskName(volume.name), + })), cluster: payload.cluster, }; return initialState diff --git a/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.test.js b/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.test.js index 80c1faa353..44bf564509 100644 --- a/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.test.js +++ b/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.test.js @@ -14,7 +14,8 @@ import { import reducer from './vmware'; jest.mock('uuid/v1'); -uuidV1.mockImplementation(() => '1547e1c0-309a-11e9-98f5-5f761412a4c2'); +let uuidCounter = 0; +uuidV1.mockImplementation(() => `uuid-${++uuidCounter}`); describe('vmware storage reducer', () => { it('returns the initial state', () => { @@ -80,6 +81,76 @@ describe('vmware storage reducer', () => { }) ).toMatchSnapshot(); }); + + it('renumbers volumes after deletion', () => { + const stateWith2Disks = reducer(stateWithController, { + type: types.STORAGE_VMWARE_ADD_DISK, + payload: { controllerKey: 1000, data: diskAttributes }, + }); + expect(stateWith2Disks.volumes).toHaveLength(2); + + const stateAfterRemove = reducer(stateWith2Disks, { + type: types.STORAGE_VMWARE_REMOVE_DISK, + payload: { key: diskKey }, + }); + expect(stateAfterRemove.volumes).toHaveLength(1); + expect(stateAfterRemove.volumes[0].name).toEqual('Hard disk 1'); + }); + + it('keeps sequential names after removing middle disks', () => { + let state = stateWithController; + + state = reducer(state, { + type: types.STORAGE_VMWARE_ADD_DISK, + payload: { controllerKey: 1000, data: diskAttributes }, + }); + state = reducer(state, { + type: types.STORAGE_VMWARE_ADD_DISK, + payload: { controllerKey: 1000, data: diskAttributes }, + }); + state = reducer(state, { + type: types.STORAGE_VMWARE_ADD_DISK, + payload: { controllerKey: 1000, data: diskAttributes }, + }); + expect(state.volumes).toHaveLength(4); + expect(state.volumes.map(vol => vol.name)).toEqual([ + 'Hard disk 1', + 'Hard disk 2', + 'Hard disk 3', + 'Hard disk 4', + ]); + + state = reducer(state, { + type: types.STORAGE_VMWARE_REMOVE_DISK, + payload: { key: state.volumes[1].key }, + }); + state = reducer(state, { + type: types.STORAGE_VMWARE_REMOVE_DISK, + payload: { key: state.volumes[1].key }, + }); + expect(state.volumes).toHaveLength(2); + expect(state.volumes.map(vol => vol.name)).toEqual([ + 'Hard disk 1', + 'Hard disk 2', + ]); + }); + }); + + describe('STORAGE_VMWARE_INIT', () => { + it('normalizes Hard disk to Hard disk 1 on init', () => { + const result = reducer(initialState, { + type: types.STORAGE_VMWARE_INIT, + payload: { + config: { controllerTypes: {} }, + controllers: [{ type: 'VirtualLsiLogicController', key: 1000 }], + volumes: [ + { name: 'Hard disk', controllerKey: 1000, sizeGb: 10 }, + ], + cluster: 'TestCluster', + }, + }); + expect(result.volumes[0].name).toEqual('Hard disk 1'); + }); }); describe('STORAGE_VMWARE_UPDATE_DISK', () => {