feat(migrate-vm-modal): controlled form (#2259)

src/common/xo/choose-sr-for-each-vdis-modal/index.js

@@ -1,19 +1,14 @@
 import Collapse from 'collapse'
 import Component from 'base-component'
 import React from 'react'
-import { every, forEach, map } from 'lodash'
+import { map } from 'lodash'
 
 import _ from '../../intl'
 import propTypes from '../../prop-types-decorator'
 import SingleLineRow from '../../single-line-row'
-import { createSelector } from '../../selectors'
-import { SelectSr } from '../../select-objects'
-import { isSrWritable } from 'xo'
 import { Container, Col } from 'grid'
-
+import { isSrWritable } from 'xo'
-// Can 2 SRs on the same pool have 2 VDIs used by the same VM
+import { SelectSr } from '../../select-objects'
-const areSrsCompatible = (sr1, sr2) =>
-  sr1.shared || sr2.shared || sr1.$container === sr2.$container
 
 const Collapsible = ({ collapsible, children, ...props }) =>
   collapsible ? (
@@ -32,96 +27,49 @@ Collapsible.propTypes = {
 }
 
 @propTypes({
-  vdis: propTypes.array.isRequired,
+  mainSrPredicate: propTypes.func,
-  predicate: propTypes.func,
+  onChange: propTypes.func.isRequired,
+  srPredicate: propTypes.func,
+  value: propTypes.objectOf(
+    propTypes.shape({
+      mainSr: propTypes.object,
+      mapVdisSrs: propTypes.object,
+    })
+  ).isRequired,
+  vdis: propTypes.object.isRequired,
 })
 export default class ChooseSrForEachVdisModal extends Component {
-  state = {
+  _onChange = newValues => {
-    mapVdisSrs: {},
+    this.props.onChange({
-  }
+      ...this.props.value,
-
+      ...newValues,
-  componentWillReceiveProps (newProps) {
-    if (
-      this.props.predicate !== undefined &&
-      newProps.predicate !== this.props.predicate
-    ) {
-      this.state = {
-        mainSr: undefined,
-        mapVdisSrs: {},
-      }
-    }
-  }
-
-  _onChange = props => {
-    this.setState(props)
-    this.props.onChange(props)
-  }
-
-  _onChangeMainSr = newSr => {
-    const oldSr = this.state.mainSr
-
-    if (oldSr == null || newSr == null || oldSr.$pool !== newSr.$pool) {
-      this.setState({
-        mapVdisSrs: {},
-      })
-    } else if (!newSr.shared) {
-      const mapVdisSrs = { ...this.state.mapVdisSrs }
-      forEach(mapVdisSrs, (sr, vdi) => {
-        if (
-          sr != null &&
-          newSr !== sr &&
-          sr.$container !== newSr.$container &&
-          !sr.shared
-        ) {
-          delete mapVdisSrs[vdi]
-        }
-      })
-      this._onChange({ mapVdisSrs })
-    }
-
-    this._onChange({
-      mainSr: newSr,
     })
   }
 
-  _getSrPredicate = createSelector(
+  _onChangeMainSr = mainSr => this._onChange({ mainSr })
-    () => this.state.mainSr,
-    () => this.state.mapVdisSrs,
-    (mainSr, mapVdisSrs) => sr =>
-      isSrWritable(sr) &&
-      mainSr.$pool === sr.$pool &&
-      areSrsCompatible(mainSr, sr) &&
-      every(
-        mapVdisSrs,
-        selectedSr => selectedSr == null || areSrsCompatible(selectedSr, sr)
-      )
-  )
 
   render () {
-    const { props, state } = this
+    const { props } = this
-    const { vdis } = props
+    const {
-    const { mainSr, mapVdisSrs } = state
+      mainSrPredicate = isSrWritable,
-
+      srPredicate = mainSrPredicate,
-    const srPredicate = props.predicate || this._getSrPredicate()
+      value: { mainSr, mapVdisSrs },
+    } = props
 
     return (
       <div>
         <SelectSr
-          onChange={mainSr =>
+          onChange={this._onChangeMainSr}
-            props.predicate !== undefined
-              ? this._onChange({ mainSr })
-              : this._onChangeMainSr(mainSr)
-          }
-          predicate={props.predicate || isSrWritable}
           placeholder={_('chooseSrForEachVdisModalMainSr')}
+          predicate={mainSrPredicate}
           value={mainSr}
         />
         <br />
-        {vdis != null &&
+        {props.vdis != null &&
           mainSr != null && (
             <Collapsible
-              collapsible={vdis.length >= 3}
               buttonText={_('chooseSrForEachVdisModalSelectSr')}
+              collapsible={props.vdis.length >= 3}
             >
               <br />
               <Container>
@@ -133,7 +81,7 @@ export default class ChooseSrForEachVdisModal extends Component {
                     <strong>{_('chooseSrForEachVdisModalSrLabel')}</strong>
                   </Col>
                 </SingleLineRow>
-                {map(vdis, vdi => (
+                {map(props.vdis, vdi => (
                   <SingleLineRow key={vdi.uuid}>
                     <Col size={6}>{vdi.name_label || vdi.name}</Col>
                     <Col size={6}>
@@ -143,8 +91,8 @@ export default class ChooseSrForEachVdisModal extends Component {
                             mapVdisSrs: { ...mapVdisSrs, [vdi.uuid]: sr },
                           })
                         }
-                        value={mapVdisSrs[vdi.uuid]}
                         predicate={srPredicate}
+                        value={mapVdisSrs !== undefined && mapVdisSrs[vdi.uuid]}
                       />
                     </Col>
                   </SingleLineRow>

src/common/xo/migrate-vm-modal/index.js

@@ -1,7 +1,7 @@
 import BaseComponent from 'base-component'
 import every from 'lodash/every'
-import forEach from 'lodash/forEach'
 import find from 'lodash/find'
+import forEach from 'lodash/forEach'
 import map from 'lodash/map'
 import React from 'react'
 import store from 'store'
@@ -11,18 +11,17 @@ import ChooseSrForEachVdisModal from '../choose-sr-for-each-vdis-modal'
 import invoke from '../../invoke'
 import SingleLineRow from '../../single-line-row'
 import { Col } from '../../grid'
+import { connectStore, mapPlus, resolveId, resolveIds } from '../../utils'
 import { getDefaultNetworkForVif } from '../utils'
 import { SelectHost, SelectNetwork } from '../../select-objects'
-import { connectStore, mapPlus, resolveIds } from '../../utils'
 import {
   createGetObjectsOfType,
   createPicker,
   createSelector,
   getObject,
 } from '../../selectors'
-import { isSrShared } from 'xo'
 
-import { isSrWritable } from '../'
+import { isSrShared, isSrWritable } from '../'
 
 import styles from './index.css'
 
@@ -68,8 +67,8 @@ export default class MigrateVmModalBody extends BaseComponent {
     super(props)
 
     this.state = {
-      mapVdisSrs: {},
       mapVifsNetworks: {},
+      targetSrs: {},
     }
 
     this._getHostPredicate = createSelector(
@@ -126,11 +125,11 @@ export default class MigrateVmModalBody extends BaseComponent {
 
   get value () {
     return {
-      targetHost: this.state.host && this.state.host.id,
+      mapVdisSrs: resolveIds(this.state.targetSrs.mapVdisSrs),
-      sr: this.state.mainSr && this.state.mainSr.id,
-      mapVdisSrs: resolveIds(this.state.mapVdisSrs),
       mapVifsNetworks: this.state.mapVifsNetworks,
       migrationNetwork: this.state.migrationNetworkId,
+      sr: resolveId(this.state.targetSrs.mainSr),
+      targetHost: this.state.host && this.state.host.id,
     }
   }
 
@@ -174,6 +173,7 @@ export default class MigrateVmModalBody extends BaseComponent {
         intraPool,
         mapVifsNetworks: undefined,
         migrationNetwork: undefined,
+        targetSrs: {},
       })
       return
     }
@@ -205,6 +205,7 @@ export default class MigrateVmModalBody extends BaseComponent {
       intraPool,
       mapVifsNetworks: defaultNetworksForVif,
       migrationNetworkId: defaultMigrationNetworkId,
+      targetSrs: {},
     })
   }
 
@@ -219,6 +220,7 @@ export default class MigrateVmModalBody extends BaseComponent {
       intraPool,
       mapVifsNetworks,
       migrationNetworkId,
+      targetSrs,
     } = this.state
     return (
       <div>
@@ -240,8 +242,9 @@ export default class MigrateVmModalBody extends BaseComponent {
               <SingleLineRow>
                 <Col size={12}>
                   <ChooseSrForEachVdisModal
-                    onChange={props => this.setState(props)}
+                    mainSrPredicate={this._getSrPredicate()}
-                    predicate={this._getSrPredicate()}
+                    onChange={this.linkState('targetSrs')}
+                    value={targetSrs}
                     vdis={vdis}
                   />
                 </Col>

src/xo-app/backup/restore/index.js

@@ -5,7 +5,6 @@ import every from 'lodash/every'
 import filter from 'lodash/filter'
 import find from 'lodash/find'
 import forEach from 'lodash/forEach'
-import getEventValue from 'get-event-value'
 import groupBy from 'lodash/groupBy'
 import Icon from 'icon'
 import isEmpty from 'lodash/isEmpty'
@@ -125,8 +124,8 @@ const openImportModal = ({ backups }) =>
     body: <ImportModalBody vmName={backups[0].name} backups={backups} />,
   }).then(doImport)
 
-const doImport = ({ backup, mainSr, start, mapVdisSrs }) => {
+const doImport = ({ backup, targetSrs, start }) => {
-  if (!mainSr || !backup) {
+  if (targetSrs.mainSr === undefined || backup === undefined) {
     error(_('backupRestoreErrorTitle'), _('backupRestoreErrorMessage'))
     return
   }
@@ -137,10 +136,10 @@ const doImport = ({ backup, mainSr, start, mapVdisSrs }) => {
   info(_('importBackupTitle'), _('importBackupMessage'))
   try {
     const importPromise = importMethods[backup.type]({
-      remote: backup.remoteId,
-      sr: mainSr,
       file: backup.path,
-      mapVdisSrs,
+      mapVdisSrs: targetSrs.mapVdisSrs,
+      remote: backup.remoteId,
+      sr: targetSrs.mainSr,
     }).then(id => {
       return id
     })
@@ -153,12 +152,8 @@ const doImport = ({ backup, mainSr, start, mapVdisSrs }) => {
 }
 
 class _ModalBody extends Component {
-  constructor () {
+  state = {
-    super()
+    targetSrs: {},
-
-    this.state = {
-      mapVdisSrs: {},
-    }
   }
 
   get value () {
@@ -166,52 +161,52 @@ class _ModalBody extends Component {
   }
 
   _getSrPredicate = createSelector(
-    () => this.state.sr,
+    () => this.state.targetSrs.mainSr,
-    () => this.state.mapVdisSrs,
+    () => this.state.targetSrs.mapVdisSrs,
-    (defaultSr, mapVdisSrs) => sr =>
+    (mainSr, mapVdisSrs) => sr =>
-      sr !== defaultSr &&
       isSrWritable(sr) &&
-      defaultSr.$pool === sr.$pool &&
+      mainSr.$pool === sr.$pool &&
-      areSrsCompatible(defaultSr, sr) &&
+      areSrsCompatible(mainSr, sr) &&
       every(
         mapVdisSrs,
         selectedSr => selectedSr == null || areSrsCompatible(selectedSr, sr)
       )
   )
 
-  _onChangeDefaultSr = event => {
+  _onSrsChange = props => {
-    const oldSr = this.state.sr
+    const oldMainSr = this.state.targetSrs.mainSr
-    const newSr = getEventValue(event)
+    const newMainSr = props.mainSr
 
-    if (oldSr == null || newSr == null || oldSr.$pool !== newSr.$pool) {
+    const targetSrs = { ...props }
-      this.setState({
+
-        mapVdisSrs: {},
+    // This code fixes the incompatibilities between the mapVdisSrs values
-      })
+    if (oldMainSr !== newMainSr) {
-    } else if (!newSr.shared) {
+      if (
-      const mapVdisSrs = { ...this.state.mapVdisSrs }
+        oldMainSr == null ||
-      forEach(mapVdisSrs, (sr, vdi) => {
+        newMainSr == null ||
-        if (
+        oldMainSr.$pool !== newMainSr.$pool
-          sr != null &&
+      ) {
-          newSr !== sr &&
+        targetSrs.mapVdisSrs = {}
-          sr.$container !== newSr.$container &&
+      } else if (!newMainSr.shared) {
-          !sr.shared
+        forEach(targetSrs.mapVdisSrs, (sr, vdi) => {
-        ) {
+          if (
-          delete mapVdisSrs[vdi]
+            sr != null &&
-        }
+            newMainSr !== sr &&
-      })
+            sr.$container !== newMainSr.$container &&
-      this.setState({
+            !sr.shared
-        mapVdisSrs,
+          ) {
-      })
+            delete targetSrs.mapVdisSrs[vdi]
+          }
+        })
+      }
     }
 
-    this.setState({
+    this.setState({ targetSrs })
-      sr: newSr,
-    })
   }
 
   render () {
-    const { backups, intl } = this.props
+    const { props, state } = this
-    const vdis = this.state.backup && this.state.backup.vdis
+    const vdis = state.backup && state.backup.vdis
 
     return (
       <div>
@@ -219,15 +214,17 @@ class _ModalBody extends Component {
           onChange={this.linkState('backup')}
           optionKey='path'
           optionRenderer={backupOptionRenderer}
-          options={backups}
+          options={props.backups}
-          placeholder={intl.formatMessage(
+          placeholder={props.intl.formatMessage(
             messages.importBackupModalSelectBackup
           )}
         />
         <br />
         <ChooseSrForEachVdisModal
+          onChange={this._onSrsChange}
+          srPredicate={this._getSrPredicate()}
+          value={state.targetSrs}
           vdis={vdis}
-          onChange={props => this.setState(props)}
         />
         <br />
         <Toggle onChange={this.linkState('start')} />{' '}