refactor(protocol-designer): gate timeline generation with form and field validation

protocol-designer/src/components/SettingsPage/SettingsPage.css

@@ -1,5 +1,9 @@
 @import '@opentrons/components';
 
+:root {
+  --mw-labeled-toggle: 25rem;
+}
+
 .sidebar_item {
   background-color: white;
   margin: 0.4rem 0.125rem;

protocol-designer/src/containers/ConnectedStepItem.js

@@ -1,6 +1,7 @@
 // @flow
 import * as React from 'react'
 import {connect} from 'react-redux'
+import isEmpty from 'lodash/isEmpty'
 import type {BaseState, ThunkDispatch} from '../types'
 
 import type {SubstepIdentifier} from '../steplist/types'
@@ -40,8 +41,8 @@ function mapStateToProps (state: BaseState, ownProps: OP): SP {
   const hoveredStep = steplistSelectors.getHoveredStepId(state)
   const selected = steplistSelectors.getSelectedStepId(state) === stepId
   const collapsed = steplistSelectors.getCollapsedSteps(state)[stepId]
-
-  const hasError = fileDataSelectors.getErrorStepId(state) === stepId
+  const formAndFieldErrors = steplistSelectors.getFormAndFieldErrorsByStepId(state)[stepId]
+  const hasError = fileDataSelectors.getErrorStepId(state) === stepId || !isEmpty(formAndFieldErrors)
   const warnings = (typeof stepId === 'number') // TODO: Ian 2018-07-13 remove when stepId always number
     ? dismissSelectors.getTimelineWarningsPerStep(state)[stepId]
     : []

protocol-designer/src/file-data/selectors/commands.js

@@ -116,47 +116,47 @@ export const getInitialRobotState: BaseState => StepGeneration.RobotState = crea
     }
   }
 )
-function compoundCommandCreatorFromFormData (validatedForm: StepGeneration.CommandCreatorData): ?StepGeneration.CompoundCommandCreator {
-  switch (validatedForm.stepType) {
+function compoundCommandCreatorFromStepArgs (stepArgs: StepGeneration.CommandCreatorData): ?StepGeneration.CompoundCommandCreator {
+  switch (stepArgs.stepType) {
     case 'consolidate':
-      return StepGeneration.consolidate(validatedForm)
+      return StepGeneration.consolidate(stepArgs)
     case 'transfer':
-      return StepGeneration.transfer(validatedForm)
+      return StepGeneration.transfer(stepArgs)
     case 'distribute':
-      return StepGeneration.distribute(validatedForm)
+      return StepGeneration.distribute(stepArgs)
     case 'mix':
-      return StepGeneration.mix(validatedForm)
+      return StepGeneration.mix(stepArgs)
     default:
       return null
   }
 }
 
 // exposes errors and last valid robotState
 export const robotStateTimeline: Selector<StepGeneration.Timeline> = createSelector(
-  steplistSelectors.validatedForms,
+  steplistSelectors.getArgsAndErrorsByStepId,
   steplistSelectors.orderedSteps,
   getInitialRobotState,
-  (forms, orderedSteps, initialRobotState) => {
-    const allFormData: Array<StepGeneration.CommandCreatorData | null> = orderedSteps.map(stepId => {
-      return (forms[stepId] && forms[stepId].validatedForm) || null
+  (allStepArgsAndErrors, orderedSteps, initialRobotState) => {
+    const allStepArgs: Array<StepGeneration.CommandCreatorData | null> = orderedSteps.map(stepId => {
+      return (allStepArgsAndErrors[stepId] && allStepArgsAndErrors[stepId].stepArgs) || null
     })
 
     // TODO: Ian 2018-06-14 `takeWhile` isn't inferring the right type
     // $FlowFixMe
-    const continuousValidForms: Array<StepGeneration.CommandCreatorData> = takeWhile(
-      allFormData,
-      f => f
+    const continuousStepArgs: Array<StepGeneration.CommandCreatorData> = takeWhile(
+      allStepArgs,
+      stepArgs => stepArgs
     )
 
-    const commandCreators = continuousValidForms.reduce(
-      (acc: Array<StepGeneration.CommandCreator>, formData, formIndex) => {
-        const {stepType} = formData
+    const commandCreators = continuousStepArgs.reduce(
+      (acc: Array<StepGeneration.CommandCreator>, stepArgs, stepIndex) => {
+        const {stepType} = stepArgs
         let reducedCommandCreator = null
 
-        if (formData.stepType === 'pause') {
-          reducedCommandCreator = StepGeneration.delay(formData)
+        if (stepArgs.stepType === 'pause') {
+          reducedCommandCreator = StepGeneration.delay(stepArgs)
         } else { // NOTE: compound return an array of command creators, atomic steps only return one command creator
-          const compoundCommandCreator: ?StepGeneration.CompoundCommandCreator = compoundCommandCreatorFromFormData(formData)
+          const compoundCommandCreator: ?StepGeneration.CompoundCommandCreator = compoundCommandCreatorFromStepArgs(stepArgs)
           reducedCommandCreator = compoundCommandCreator && StepGeneration.reduceCommandCreators(compoundCommandCreator(initialRobotState))
         }
         if (!reducedCommandCreator) {
@@ -168,13 +168,13 @@ export const robotStateTimeline: Selector<StepGeneration.Timeline> = createSelec
         // Drop tips eagerly, per pipette
         // NOTE: this assumes all step forms that use a pipette have both
         // 'pipette' and 'changeTip' fields (and they're not named something else).
-        const pipetteId = formData.pipette
+        const pipetteId = stepArgs.pipette
         if (pipetteId) {
-          const nextFormForPipette = continuousValidForms
-            .slice(formIndex + 1)
-            .find(form => form.pipette === pipetteId)
+          const nextStepArgsForPipette = continuousStepArgs
+            .slice(stepIndex + 1)
+            .find(stepArgs => stepArgs.pipette === pipetteId)
 
-          const willReuseTip = nextFormForPipette && nextFormForPipette.changeTip === 'never'
+          const willReuseTip = nextStepArgsForPipette && nextStepArgsForPipette.changeTip === 'never'
           if (!willReuseTip) {
             return [
               ...acc,

protocol-designer/src/labware-ingred/reducers/index.js

@@ -31,7 +31,7 @@ import type {
 } from '../types'
 import * as actions from '../actions'
 import {getPDMetadata} from '../../file-types'
-import type {BaseState, Selector, Options} from '../../types'
+import type {BaseState, Options} from '../../types'
 import type {LoadFileAction} from '../../load-file'
 import type {
   RemoveWellsContents,
@@ -294,8 +294,10 @@ const rootReducer = combineReducers({
   renameLabwareFormMode,
 })
 
+type RootSlice = {labwareIngred: RootState}
+type Selector<T> = (RootSlice) => T
 // SELECTORS
-const rootSelector = (state: BaseState): RootState => state.labwareIngred
+const rootSelector = (state: RootSlice): RootState => state.labwareIngred
 
 const getLabware: Selector<{[labwareId: string]: ?Labware}> = createSelector(
   rootSelector,
@@ -318,8 +320,8 @@ const getLabwareTypes: Selector<LabwareTypeById> = createSelector(
   )
 )
 
-const getLiquidGroupsById = (state: BaseState) => rootSelector(state).ingredients
-const getIngredientLocations = (state: BaseState) => rootSelector(state).ingredLocations
+const getLiquidGroupsById = (state: RootSlice) => rootSelector(state).ingredients
+const getIngredientLocations = (state: RootSlice) => rootSelector(state).ingredLocations
 
 const getNextLiquidGroupId: Selector<string> = createSelector(
   getLiquidGroupsById,

protocol-designer/src/pipettes/reducers.js

@@ -42,42 +42,30 @@ function createPipette (
   }
 }
 
-export type PipetteReducerState = {
-  byMount: {|
-    left: ?string,
-    right: ?string,
-  |},
-  byId: {
-    [pipetteId: string]: PipetteData,
-  },
-}
+export type PipetteIdByMount = {|
+  left: ?string,
+  right: ?string,
+|}
 
-const pipettes = handleActions({
-  LOAD_FILE: (state: PipetteReducerState, action: LoadFileAction): PipetteReducerState => {
+export type PipetteById = {[pipetteId: string]: PipetteData}
+
+const byId = handleActions({
+  LOAD_FILE: (state: PipetteById, action: LoadFileAction): PipetteById => {
     const file = action.payload
     const {pipettes} = file
     // TODO: Ian 2018-06-29 create fns to access ProtocolFile data
     const {pipetteTiprackAssignments} = file['designer-application'].data
-    const pipetteIds = Object.keys(pipettes)
-    return {
-      byMount: {
-        left: pipetteIds.find(id => pipettes[id].mount === 'left'),
-        right: pipetteIds.find(id => pipettes[id].mount === 'right'),
-      },
-      byId: reduce(
-        pipettes,
-        (acc: {[pipetteId: string]: PipetteData}, p: FilePipette, id: string) => {
-          const newPipette = createPipette(p.mount, p.model, pipetteTiprackAssignments[id], id)
-          return newPipette
-            ? {...acc, [id]: newPipette}
-            : acc
-        }, {}),
-    }
+    return reduce(pipettes, (acc: {[pipetteId: string]: PipetteData}, p: FilePipette, id: string) => {
+      const newPipette = createPipette(p.mount, p.model, pipetteTiprackAssignments[id], id)
+      return newPipette
+        ? {...acc, [id]: newPipette}
+        : acc
+    }, {})
   },
   CREATE_NEW_PROTOCOL: (
-    state: PipetteReducerState,
+    state: PipetteById,
     action: {payload: NewProtocolFields}
-  ): PipetteReducerState => {
+  ): PipetteById => {
     const {left, right} = action.payload
 
     const leftPipette = (left.pipetteModel && left.tiprackModel)
@@ -88,7 +76,7 @@ const pipettes = handleActions({
       ? createPipette('right', right.pipetteModel, right.tiprackModel)
       : null
 
-    const newPipettes = ([leftPipette, rightPipette]).reduce(
+    const newPipettes: PipetteById = ([leftPipette, rightPipette]).reduce(
       (acc: {[string]: PipetteData}, pipette: ?PipetteData) => {
         if (!pipette) return acc
         return {
@@ -98,41 +86,65 @@ const pipettes = handleActions({
       }, {})
 
     return {
-      byMount: {
-        left: leftPipette ? leftPipette.id : state.byMount.left,
-        right: rightPipette ? rightPipette.id : state.byMount.right,
-      },
-      byId: {
-        ...state.byId,
-        ...newPipettes,
-      },
+      ...state,
+      ...newPipettes,
     }
   },
   SWAP_PIPETTES: (
-    state: PipetteReducerState,
+    state: PipetteById,
     action: {payload: NewProtocolFields}
-  ): PipetteReducerState => {
-    const byId = mapValues(state.byId, (pipette: PipetteData): PipetteData => ({
+  ): PipetteById => {
+    return mapValues(state, (pipette: PipetteData): PipetteData => ({
       ...pipette,
       mount: (pipette.mount === 'left') ? 'right' : 'left',
     }))
+  },
+}, {})
+
+const byMount = handleActions({
+  LOAD_FILE: (state: PipetteIdByMount, action: LoadFileAction): PipetteIdByMount => {
+    const file = action.payload
+    const {pipettes} = file
+    // TODO: Ian 2018-06-29 create fns to access ProtocolFile data
+    const pipetteIds = Object.keys(pipettes)
+    return {
+      left: pipetteIds.find(id => pipettes[id].mount === 'left'),
+      right: pipetteIds.find(id => pipettes[id].mount === 'right'),
+    }
+  },
+  CREATE_NEW_PROTOCOL: (
+    state: PipetteIdByMount,
+    action: {payload: NewProtocolFields}
+  ): PipetteIdByMount => {
+    const {left, right} = action.payload
+
+    const leftPipette = (left.pipetteModel && left.tiprackModel)
+      ? createPipette('left', left.pipetteModel, left.tiprackModel)
+      : null
 
-    return ({
-      byMount: {
-        left: state.byMount.right,
-        right: state.byMount.left,
-      },
-      byId,
-    })
+    const rightPipette = (right.pipetteModel && right.tiprackModel)
+      ? createPipette('right', right.pipetteModel, right.tiprackModel)
+      : null
+
+    return {
+      left: leftPipette ? leftPipette.id : state.left,
+      right: rightPipette ? rightPipette.id : state.right,
+    }
   },
-}, {byMount: {left: null, right: null}, byId: {}})
+  SWAP_PIPETTES: (state: PipetteIdByMount, action: {payload: NewProtocolFields}): PipetteIdByMount => ({
+    left: state.right,
+    right: state.left,
+  }),
+}, {left: null, right: null})
 
 const _allReducers = {
-  pipettes,
+  byMount,
+  byId,
 }
 
 export type RootState = {
-  pipettes: PipetteReducerState,
+  byId: PipetteById,
+  byMount: PipetteIdByMount,
 }
 
 export const rootReducer = combineReducers(_allReducers)

protocol-designer/src/pipettes/selectors.js

@@ -4,15 +4,18 @@ import reduce from 'lodash/reduce'
 import get from 'lodash/get'
 import {getPipetteModels, getPipette, getLabware} from '@opentrons/shared-data'
 
-import type {BaseState, Selector} from '../types'
 import type {DropdownOption} from '@opentrons/components'
 import type {PipetteData} from '../step-generation'
+import type {RootState} from './reducers'
 
 type PipettesById = {[pipetteId: string]: PipetteData}
+type RootSlice = {pipettes: RootState}
 
-const rootSelector = (state: BaseState) => state.pipettes.pipettes
+type Selector<T> = (RootSlice) => T
 
-export const pipettesById = createSelector(
+export const rootSelector = (state: {pipettes: RootState}) => state.pipettes
+
+export const pipettesById: Selector<PipettesById> = createSelector(
   rootSelector,
   pipettes => pipettes.byId
 )
@@ -54,7 +57,7 @@ export const equippedPipetteOptions: Selector<Array<DropdownOption>> = createSel
           },
         ]
         : acc,
-      [])
+    [])
   }
 )
 

protocol-designer/src/steplist/fieldLevel/errors.js

@@ -7,11 +7,15 @@ import isArray from 'lodash/isArray'
 
 // TODO: reconcile difference between returning error string and key
 
-export type FieldError = 'REQUIRED' | 'UNDER_WELL_MINIMUM' // TODO: add other possible field errors
+export type FieldError =
+  | 'REQUIRED'
+  | 'UNDER_WELL_MINIMUM'
+  | 'NON_ZERO'
 
 const FIELD_ERRORS: {[FieldError]: string} = {
   REQUIRED: 'This field is required',
   UNDER_WELL_MINIMUM: 'or more wells are required',
+  NON_ZERO: 'Must be greater than zero',
 }
 
 // TODO: test these
@@ -21,6 +25,7 @@ const FIELD_ERRORS: {[FieldError]: string} = {
 type errorChecker = (value: mixed) => ?string
 
 export const requiredField = (value: mixed): ?string => !value ? FIELD_ERRORS.REQUIRED : null
+export const nonZero = (value: mixed) => (value && Number(value) === 0) ? FIELD_ERRORS.NON_ZERO : null
 export const minimumWellCount = (minimum: number): errorChecker => (wells: mixed): ?string => (
   (isArray(wells) && (wells.length < minimum)) ? `${minimum} ${FIELD_ERRORS.UNDER_WELL_MINIMUM}` : null
 )