From 2c1069268ddc1400b98aaccf4450afb278d1ed36 Mon Sep 17 00:00:00 2001 From: Rastislav Wagner Date: Tue, 30 Apr 2019 16:30:22 +0200 Subject: [PATCH] Delete new row when cancel is clicked Disable back/next buttons while editing table https://bugzilla.redhat.com/show_bug.cgi?id=1646953 --- .../Table/EditableDraggableTable.js | 53 ++++++++++++------- src/components/Table/TableFactory.js | 2 +- .../Wizard/CreateVmWizard/CreateVmWizard.js | 16 +++--- .../Wizard/CreateVmWizard/NetworksTab.js | 26 +++++---- .../Wizard/CreateVmWizard/StorageTab.js | 28 ++++++---- .../tests/CreateVmWizard.test.js | 20 +++++++ .../CreateVmWizard/tests/NetworksTab.test.js | 20 ++++--- .../CreateVmWizard/tests/StorageTab.test.js | 9 ++-- 8 files changed, 118 insertions(+), 56 deletions(-) diff --git a/src/components/Table/EditableDraggableTable.js b/src/components/Table/EditableDraggableTable.js index 57cc6d50f..c753bca47 100644 --- a/src/components/Table/EditableDraggableTable.js +++ b/src/components/Table/EditableDraggableTable.js @@ -43,6 +43,21 @@ class EditableDraggableTable extends React.Component { } } + onRowDelete = rowData => { + const { id } = rowData; + const editing = false; + const rows = cloneDeep(this.props.rows.filter(row => row.id !== id)); + + this.flagUpdate(rows, editing); + this.setState({ editing }); + + this.props.onChange(rows, { + type: ON_DELETE, + editing, + id, + }); + }; + // needed for refiring row renders // eslint-disable-next-line no-return-assign flagUpdate = (rows, editingInProgress) => rows.forEach(row => (row.editingInProgress = editingInProgress)); @@ -81,6 +96,7 @@ class EditableDraggableTable extends React.Component { const rows = cloneDeep(this.props.rows); const index = findIndex(rows, { id }); + delete rows[index].newRow; delete rows[index].backup; this.flagUpdate(rows, editing); @@ -94,22 +110,26 @@ class EditableDraggableTable extends React.Component { }, onCancel: ({ rowData }) => { - const editing = false; const { id } = rowData; - const rows = cloneDeep(this.props.rows); - const index = findIndex(rows, { id }); + const index = findIndex(this.props.rows, { id }); - rows[index] = cloneDeep(rows[index].backup); - delete rows[index].backup; + if (this.props.rows[index].newRow) { + this.onRowDelete(rowData); + } else { + const editing = false; + const rows = cloneDeep(this.props.rows); + rows[index] = cloneDeep(rows[index].backup); + delete rows[index].backup; - this.flagUpdate(rows, editing); - this.setState({ editing }); + this.flagUpdate(rows, editing); + this.setState({ editing }); - this.props.onChange(rows, { - type: ON_CANCEL, - id, - editing, - }); + this.props.onChange(rows, { + type: ON_CANCEL, + id, + editing, + }); + } }, onChange: (value, { rowData, property }) => { @@ -131,14 +151,7 @@ class EditableDraggableTable extends React.Component { }; crudController = { - onDelete: ({ rowData }) => { - const { id } = rowData; - this.props.onChange(this.props.rows.filter(row => row.id !== id), { - type: ON_DELETE, - editing: false, - id, - }); - }, + onDelete: ({ rowData }) => this.onRowDelete(rowData), }; getActionButton = (action, additionalData, id) => { diff --git a/src/components/Table/TableFactory.js b/src/components/Table/TableFactory.js index 5ad5e37f4..09a8ffbd1 100644 --- a/src/components/Table/TableFactory.js +++ b/src/components/Table/TableFactory.js @@ -17,7 +17,7 @@ const onChange = (rows, { editing, type, id, key, newValue }, onRowUpdate, onRow break; case ON_DELETE: case ON_MOVE: - onRowDeleteOrMove(rows); + onRowDeleteOrMove(rows, editing); break; default: // eslint-disable-next-line diff --git a/src/components/Wizard/CreateVmWizard/CreateVmWizard.js b/src/components/Wizard/CreateVmWizard/CreateVmWizard.js index 82b68afab..74f691802 100644 --- a/src/components/Wizard/CreateVmWizard/CreateVmWizard.js +++ b/src/components/Wizard/CreateVmWizard/CreateVmWizard.js @@ -116,12 +116,15 @@ export class CreateVmWizard extends React.Component { lastStepReached = () => this.state.activeStepIndex === this.getLastStepIndex(); - onStepDataChanged = (tabKey, value, valid) => { + onStepDataChanged = (tabKey, value, valid, lockStep = false) => { const validatedTabData = validateTabData(tabKey, value, valid); this.safeSetState(state => ({ stepData: { ...state.stepData, - [tabKey]: validatedTabData, + [tabKey]: { + ...validatedTabData, + lockStep, + }, }, })); }; @@ -220,7 +223,7 @@ export class CreateVmWizard extends React.Component { this.onStepDataChanged(NETWORKS_TAB_KEY, value, valid)} + onChange={(value, valid, lockStep) => this.onStepDataChanged(NETWORKS_TAB_KEY, value, valid, lockStep)} networkConfigs={this.props.networkConfigs} networks={this.state.stepData[NETWORKS_TAB_KEY].value || []} sourceType={sourceType} @@ -246,7 +249,7 @@ export class CreateVmWizard extends React.Component { this.onStepDataChanged(STORAGE_TAB_KEY, value, valid)} + onChange={(value, valid, lockStep) => this.onStepDataChanged(STORAGE_TAB_KEY, value, valid, lockStep)} units={this.props.units} sourceType={sourceType} namespace={getVmSettingValue(this.state, NAMESPACE_KEY)} @@ -278,6 +281,7 @@ export class CreateVmWizard extends React.Component { const lastStepReached = this.lastStepReached(); const createVmText = this.props.createTemplate ? CREATE_VM_TEMPLATE : CREATE_VM; + const currentStepData = this.state.stepData[this.wizardStepsNewVM[this.state.activeStepIndex].key]; return ( network.id || 0), 0) + 1, @@ -140,7 +140,7 @@ export class NetworksTab extends React.Component { }; } - publishResults = rows => { + publishResults = (rows, editing) => { let valid = this.props.sourceType === PROVISION_SOURCE_PXE ? rows.some(row => row.isBootable) : true; const nics = rows.map( ({ templateNetwork, rootNetwork, id, isBootable, name, mac, network, errors, networkType, binding }) => { @@ -172,11 +172,12 @@ export class NetworksTab extends React.Component { } ); - this.props.onChange(nics, valid); + this.props.onChange(nics, valid, editing); }; onRowActivate = rows => { this.setState({ rows, editing: true }); + this.publishResults(rows, true); }; onRowUpdate = (rows, updatedRowId, editing, property, newValue) => { @@ -199,14 +200,13 @@ export class NetworksTab extends React.Component { rowsChanged = (rows, editing) => { resolveBootableNetwork(this.props.sourceType, rows); - this.publishResults(rows); + this.publishResults(rows, editing); this.setState({ rows, editing }); }; createNic = () => { - this.setState(state => ({ - nextId: state.nextId + 1, - rows: [ + this.setState(state => { + const rows = [ ...state.rows, { id: state.nextId, @@ -217,9 +217,15 @@ export class NetworksTab extends React.Component { mac: '', network: '', binding: '', + newRow: true, }, - ], - })); + ]; + this.publishResults(rows, true); + return { + nextId: state.nextId + 1, + rows, + }; + }); }; getColumns = () => { @@ -354,7 +360,7 @@ export class NetworksTab extends React.Component { state.rows.forEach(row => { row.isBootable = row.id === newValue.value.id; }); - this.publishResults(state.rows); + this.publishResults(state.rows, state.editing); return state.rows; }); }; diff --git a/src/components/Wizard/CreateVmWizard/StorageTab.js b/src/components/Wizard/CreateVmWizard/StorageTab.js index 8fe57f65e..815f2424c 100644 --- a/src/components/Wizard/CreateVmWizard/StorageTab.js +++ b/src/components/Wizard/CreateVmWizard/StorageTab.js @@ -258,7 +258,7 @@ const resolveInitialStorages = ( return storages; }; -const publishResults = (rows, otherStorages, sourceType, publish) => { +const publishResults = (rows, otherStorages, sourceType, publish, editing) => { let valid = !needsBootableDisk(rows, sourceType); const storages = rows.map( @@ -295,7 +295,7 @@ const publishResults = (rows, otherStorages, sourceType, publish) => { } ); storages.push(...otherStorages); - publish(storages, valid); + publish(storages, valid, editing); }; export class StorageTab extends React.Component { @@ -320,14 +320,16 @@ export class StorageTab extends React.Component { editing: false, }; - publishResults(this.state.rows, this.state.otherStorages, sourceType, onChange); + publishResults(this.state.rows, this.state.otherStorages, sourceType, onChange, false); } onRowActivate = rows => { + const { sourceType, onChange } = this.props; this.setState({ rows, editing: true, }); + publishResults(rows, this.state.otherStorages, sourceType, onChange, true); }; onRowUpdate = (rows, updatedRowId, editing) => { @@ -348,7 +350,7 @@ export class StorageTab extends React.Component { rowsChanged = (rows, editing) => { const { sourceType, onChange } = this.props; resolveBootability(rows, sourceType); - publishResults(rows, this.state.otherStorages, sourceType, onChange); + publishResults(rows, this.state.otherStorages, sourceType, onChange, editing); this.setState({ rows, editing, @@ -356,9 +358,9 @@ export class StorageTab extends React.Component { }; create = storageType => { - this.setState(state => ({ - nextId: state.nextId + 1, - rows: [ + this.setState(state => { + const { sourceType, onChange } = this.props; + const rows = [ ...state.rows, { id: state.nextId, @@ -366,9 +368,15 @@ export class StorageTab extends React.Component { editable: true, edit: true, // trigger immediate edit storageType, + newRow: true, }, - ], - })); + ]; + publishResults(rows, state.otherStorages, sourceType, onChange, true); + return { + nextId: state.nextId + 1, + rows, + }; + }); }; getColumns = () => [ @@ -513,7 +521,7 @@ export class StorageTab extends React.Component { }); const { sourceType, onChange } = this.props; - publishResults(state.rows, state.otherStorages, sourceType, onChange); + publishResults(state.rows, state.otherStorages, sourceType, onChange, state.editing); return state.rows; }); }; diff --git a/src/components/Wizard/CreateVmWizard/tests/CreateVmWizard.test.js b/src/components/Wizard/CreateVmWizard/tests/CreateVmWizard.test.js index 8c28aaf45..67581df9b 100644 --- a/src/components/Wizard/CreateVmWizard/tests/CreateVmWizard.test.js +++ b/src/components/Wizard/CreateVmWizard/tests/CreateVmWizard.test.js @@ -278,6 +278,26 @@ describe('', () => { expect(component.find(WizardPattern).props().nextStepDisabled).toBeTruthy(); }); + it('disables back/next step when requested', () => { + const component = mount(testCreateVmWizard()); + + component.instance().onStepDataChanged(VM_SETTINGS_TAB_KEY, validVmSettings, true); + component.instance().onStepChanged(1); + component.update(); + expect(component.state().activeStepIndex).toEqual(1); + component.instance().onStepDataChanged(NETWORKS_TAB_KEY, null, true, true); + expect(component.state().stepData[NETWORKS_TAB_KEY].lockStep).toBeTruthy(); + component.update(); + expect(component.find(WizardPattern).props().nextStepDisabled).toBeTruthy(); + expect(component.find(WizardPattern).props().previousStepDisabled).toBeTruthy(); + + component.instance().onStepDataChanged(NETWORKS_TAB_KEY, null, true, false); + component.update(); + expect(component.state().stepData[NETWORKS_TAB_KEY].lockStep).toBeFalsy(); + expect(component.find(WizardPattern).props().nextStepDisabled).toBeFalsy(); + expect(component.find(WizardPattern).props().previousStepDisabled).toBeFalsy(); + }); + it('creates vm', () => { createVm.mockReturnValueOnce(new Promise((resolve, reject) => resolve([{ result: 'VM created' }]))); testWalkThrough(); diff --git a/src/components/Wizard/CreateVmWizard/tests/NetworksTab.test.js b/src/components/Wizard/CreateVmWizard/tests/NetworksTab.test.js index 318ae5c3e..0401f59b2 100644 --- a/src/components/Wizard/CreateVmWizard/tests/NetworksTab.test.js +++ b/src/components/Wizard/CreateVmWizard/tests/NetworksTab.test.js @@ -107,8 +107,13 @@ describe('', () => { expect(getTableRows(component)).toHaveLength(0); expect(onChange).toHaveBeenCalledTimes(1); expect(onChange.mock.calls[0][1]).toBeFalsy(); + expect(onChange.mock.calls[0][2]).toBeFalsy(); getNetworkButton(component).simulate('click'); + expect(onChange).toHaveBeenCalledTimes(3); + expect(onChange.mock.calls[1][1]).toBeFalsy(); + expect(onChange.mock.calls[1][2]).toBeTruthy(); + const rows = getTableRows(component); expect(rows).toHaveLength(1); const columns = rows.find('td'); @@ -158,8 +163,9 @@ describe('', () => { // add PXE-bootable network component.instance().rowsChanged([pxeRow], false); - expect(onChange).toHaveBeenCalledTimes(2); - expect(onChange.mock.calls[1][1]).toBeTruthy(); + expect(onChange).toHaveBeenCalledTimes(4); + expect(onChange.mock.calls[3][1]).toBeTruthy(); + expect(onChange.mock.calls[3][2]).toBeFalsy(); component.update(); expect( @@ -172,8 +178,9 @@ describe('', () => { // add Pod network - not PXE bootable component.instance().rowsChanged([podRow], false); - expect(onChange).toHaveBeenCalledTimes(3); - expect(onChange.mock.calls[2][1]).toBeFalsy(); + expect(onChange).toHaveBeenCalledTimes(5); + expect(onChange.mock.calls[4][1]).toBeFalsy(); + expect(onChange.mock.calls[4][2]).toBeFalsy(); component.update(); expect( @@ -186,8 +193,9 @@ describe('', () => { // add Pod network and PXE-bootable network component.instance().rowsChanged([podRow, pxeRow], false); - expect(onChange).toHaveBeenCalledTimes(4); - expect(onChange.mock.calls[3][1]).toBeTruthy(); + expect(onChange).toHaveBeenCalledTimes(6); + expect(onChange.mock.calls[5][1]).toBeTruthy(); + expect(onChange.mock.calls[5][2]).toBeFalsy(); component.update(); expect( diff --git a/src/components/Wizard/CreateVmWizard/tests/StorageTab.test.js b/src/components/Wizard/CreateVmWizard/tests/StorageTab.test.js index 3bc1b8d9c..18b93c875 100644 --- a/src/components/Wizard/CreateVmWizard/tests/StorageTab.test.js +++ b/src/components/Wizard/CreateVmWizard/tests/StorageTab.test.js @@ -251,6 +251,7 @@ describe('', () => { component.instance().rowsChanged([containerStorage], false); expect(onChange).toHaveBeenCalledTimes(2); + expect(onChange.mock.calls[1][2]).toBeFalsy(); }); it('calls onChange when row is updated', () => { @@ -260,12 +261,13 @@ describe('', () => { const component = shallow(testStorageTab(onChange, newRows)); expect(onChange).toHaveBeenCalledTimes(1); - component.instance().onRowUpdate(newRows, 1, false); + component.instance().onRowUpdate(newRows, 1, true); expect(onChange).toHaveBeenCalledTimes(2); + expect(onChange.mock.calls[1][2]).toBeTruthy(); }); - it('does not call onChange when row is activated', () => { + it('calls onChange when row is activated', () => { const onChange = jest.fn(); const testRows = [dataVolumeStorage, pvcStorage]; @@ -274,7 +276,8 @@ describe('', () => { component.instance().onRowActivate(testRows); - expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledTimes(2); + expect(onChange.mock.calls[1][2]).toBeTruthy(); }); it('dropdown updates bootable storage', () => {