From 5657164657a6ea42e595a3c01b095ef124a9b1ad Mon Sep 17 00:00:00 2001 From: Ian London Date: Mon, 3 Dec 2018 16:37:19 -0500 Subject: [PATCH] fix(protocol-designer): fix missing disposal volume in new distribute forms (#2733) Closes #2705 --- .../TipPositionInput/TipPositionModal.js | 1 - .../src/components/StepEditForm/formFields.js | 20 ++------ protocol-designer/src/constants.js | 2 + .../src/steplist/actions/actions.js | 3 +- .../src/steplist/actions/handleFormChange.js | 34 +++++++++++--- .../src/steplist/actions/thunks.js | 46 ++++++++++++++----- .../src/steplist/formLevel/generateNewForm.js | 9 +--- .../formLevel/getDefaultsForStepType.js | 6 ++- .../getNextDefaultPipetteId/index.js | 11 +++-- .../test/getNextDefaultPipetteId.test.js | 19 ++++---- .../src/steplist/formLevel/warnings.js | 4 +- 11 files changed, 95 insertions(+), 60 deletions(-) diff --git a/protocol-designer/src/components/StepEditForm/TipPositionInput/TipPositionModal.js b/protocol-designer/src/components/StepEditForm/TipPositionInput/TipPositionModal.js index a3ea150234d..6dc9ec29ed8 100644 --- a/protocol-designer/src/components/StepEditForm/TipPositionInput/TipPositionModal.js +++ b/protocol-designer/src/components/StepEditForm/TipPositionInput/TipPositionModal.js @@ -60,7 +60,6 @@ class TipPositionModal extends React.Component { } applyChanges = () => { const {value} = this.state - console.log('applying changes', value) this.props.updateValue(value == null ? null : roundValue(value)) this.props.closeModal() } diff --git a/protocol-designer/src/components/StepEditForm/formFields.js b/protocol-designer/src/components/StepEditForm/formFields.js index 12d28e00d18..699bc1cf277 100644 --- a/protocol-designer/src/components/StepEditForm/formFields.js +++ b/protocol-designer/src/components/StepEditForm/formFields.js @@ -12,13 +12,11 @@ import { import i18n from '../../localization' import {selectors as pipetteSelectors} from '../../pipettes' import {selectors as labwareIngredSelectors} from '../../labware-ingred/reducers' -import {actions} from '../../steplist' import {hydrateField} from '../../steplist/fieldLevel' import type {StepFieldName} from '../../steplist/fieldLevel' -import {DISPOSAL_PERCENTAGE} from '../../steplist/formLevel/warnings' import {SOURCE_WELL_DISPOSAL_DESTINATION} from '../../steplist/formLevel/stepFormToArgs/transferLikeFormToArgs' import type {ChangeTipOptions} from '../../step-generation/types' -import type {BaseState, ThunkDispatch} from '../../types' +import type {BaseState} from '../../types' import type {StepType} from '../../form-types' import styles from './StepEditForm.css' import StepField from './StepFormField' @@ -118,18 +116,12 @@ export function DispenseDelayFields (props: DispenseDelayFieldsProps) { type PipetteFieldOP = {name: StepFieldName, stepType?: StepType} & FocusHandlers type PipetteFieldSP = {pipetteOptions: Options, getHydratedPipette: (string) => any} // TODO: real hydrated pipette type -type PipetteFieldDP = {updateDisposalVolume: (?mixed) => void} -type PipetteFieldProps = PipetteFieldOP & PipetteFieldSP & PipetteFieldDP +type PipetteFieldProps = PipetteFieldOP & PipetteFieldSP const PipetteFieldSTP = (state: BaseState, ownProps: PipetteFieldOP): PipetteFieldSP => ({ pipetteOptions: pipetteSelectors.getEquippedPipetteOptions(state), getHydratedPipette: (value) => hydrateField(state, ownProps.name, value), }) -const PipetteFieldDTP = (dispatch: ThunkDispatch<*>): PipetteFieldDP => ({ - updateDisposalVolume: (disposalVolume: ?mixed) => { - dispatch(actions.changeFormInput({update: {aspirate_disposalVol_volume: disposalVolume}})) - }, -}) -export const PipetteField = connect(PipetteFieldSTP, PipetteFieldDTP)((props: PipetteFieldProps) => ( +export const PipetteField = connect(PipetteFieldSTP)((props: PipetteFieldProps) => ( { props.onFieldFocus(props.name) }} onChange={(e: SyntheticEvent) => { updateValue(e.currentTarget.value) - if (props.stepType === 'distribute') { - const hydratedPipette = props.getHydratedPipette(e.currentTarget.value) - if (hydratedPipette) { - props.updateDisposalVolume(hydratedPipette.maxVolume * DISPOSAL_PERCENTAGE) - } - } }} /> )} /> diff --git a/protocol-designer/src/constants.js b/protocol-designer/src/constants.js index 2852cc196d1..478236fb402 100644 --- a/protocol-designer/src/constants.js +++ b/protocol-designer/src/constants.js @@ -57,4 +57,6 @@ export const DEFAULT_MM_TOUCH_TIP_OFFSET_FROM_TOP = -1 export const DEFAULT_WELL_ORDER_FIRST_OPTION: 't2b' = 't2b' export const DEFAULT_WELL_ORDER_SECOND_OPTION: 'l2r' = 'l2r' +export const DISPOSAL_VOLUME_PERCENTAGE = 0.2 // 20% percent of pipette capacity + export const WELL_LABEL_OFFSET: number = 8 diff --git a/protocol-designer/src/steplist/actions/actions.js b/protocol-designer/src/steplist/actions/actions.js index 159ea604556..3575f25a356 100644 --- a/protocol-designer/src/steplist/actions/actions.js +++ b/protocol-designer/src/steplist/actions/actions.js @@ -25,9 +25,10 @@ export type ChangeFormInputAction = { export const changeFormInput = (payload: ChangeFormPayload) => (dispatch: ThunkDispatch, getState: GetState) => { + const unsavedForm = selectors.getUnsavedForm(getState()) dispatch({ type: 'CHANGE_FORM_INPUT', - payload: handleFormChange(payload, getState), + payload: handleFormChange(payload, unsavedForm, getState), }) } diff --git a/protocol-designer/src/steplist/actions/handleFormChange.js b/protocol-designer/src/steplist/actions/handleFormChange.js index 232613c1e6d..da788618e5e 100644 --- a/protocol-designer/src/steplist/actions/handleFormChange.js +++ b/protocol-designer/src/steplist/actions/handleFormChange.js @@ -1,9 +1,11 @@ // @flow import uniq from 'lodash/uniq' +import {getPipetteNameSpecs} from '@opentrons/shared-data' import {getWellSetForMultichannel} from '../../well-selection/utils' -import {selectors} from '../index' import {selectors as pipetteSelectors} from '../../pipettes' import {selectors as labwareIngredSelectors} from '../../labware-ingred/reducers' +import {DISPOSAL_VOLUME_PERCENTAGE} from '../../constants' + import type {PipetteChannels} from '@opentrons/shared-data' import type {BaseState, GetState} from '../../types' import type {FormData} from '../../form-types' @@ -47,17 +49,20 @@ const getChannels = (pipetteId: string, state: BaseState): PipetteChannels => { // Eventually we gotta allow arbitrary actions like DELETE_LABWARE // (or more speculatively, CHANGE_PIPETTE etc), which affect saved forms from // 'outside', to cause changes that run thru all the logic in this block -function handleFormChange (payload: ChangeFormPayload, getState: GetState): ChangeFormPayload { +function handleFormChange ( + payload: ChangeFormPayload, + baseForm: ?FormData, + getState: GetState, +): ChangeFormPayload { // Use state to handle form changes const baseState = getState() - const unsavedForm = selectors.getUnsavedForm(baseState) // pass thru, unchanged - if (unsavedForm == null) { return payload } + if (baseForm == null) { return payload } let updateOverrides = getChangeLabwareEffects(payload.update) - if (unsavedForm.pipette && payload.update.pipette) { + if (baseForm.pipette && payload.update.pipette) { if (typeof payload.update.pipette !== 'string') { // this should not happen! console.error('no next pipette, could not handleFormChange') @@ -66,7 +71,23 @@ function handleFormChange (payload: ChangeFormPayload, getState: GetState): Chan const nextChannels = getChannels(payload.update.pipette, baseState) updateOverrides = { ...updateOverrides, - ...reconcileFormPipette(unsavedForm, baseState, payload.update.pipette, nextChannels), + ...reconcileFormPipette(baseForm, baseState, payload.update.pipette, nextChannels), + } + } + + if (baseForm.stepType === 'distribute') { + if (typeof payload.update.pipette === 'string') { + const pipetteData = pipetteSelectors.getPipettesById(baseState)[payload.update.pipette] + const pipetteSpecs = getPipetteNameSpecs(pipetteData.model) + const disposalVol = pipetteSpecs + ? pipetteSpecs.maxVolume * DISPOSAL_VOLUME_PERCENTAGE + : null + + updateOverrides = { + ...updateOverrides, + 'aspirate_disposalVol_checkbox': true, + 'aspirate_disposalVol_volume': disposalVol, + } } } @@ -183,6 +204,7 @@ export const reconcileFormPipette = (formData: FormData, baseState: BaseState, n } } } + return updateOverrides } diff --git a/protocol-designer/src/steplist/actions/thunks.js b/protocol-designer/src/steplist/actions/thunks.js index 12d16e86897..82b7fbed2ce 100644 --- a/protocol-designer/src/steplist/actions/thunks.js +++ b/protocol-designer/src/steplist/actions/thunks.js @@ -1,8 +1,9 @@ // @flow -import {selectors} from '../index' +import handleFormChange from './handleFormChange' import {uuid} from '../../utils' import {selectors as labwareIngredsSelectors} from '../../labware-ingred/reducers' import * as pipetteSelectors from '../../pipettes/selectors' +import {selectors as steplistSelectors} from '../../steplist' import {actions as tutorialActions} from '../../tutorial' import {getNextDefaultPipetteId, generateNewForm} from '../formLevel' @@ -16,23 +17,17 @@ export type SelectStepAction = { // get new or existing step for given stepId function getStepFormData (state: BaseState, stepId: StepIdType, newStepType?: StepType): ?FormData { - const existingStep = selectors.getSavedForms(state)[stepId] + const existingStep = steplistSelectors.getSavedForms(state)[stepId] if (existingStep) { return existingStep } - const defaultNextPipette = getNextDefaultPipetteId( - selectors.getSavedForms(state), - selectors.getOrderedSteps(state), - pipetteSelectors.getPipetteIdsByMount(state) - ) - // TODO: Ian 2018-09-19 sunset 'steps' reducer. Right now, it's needed here to get stepType // for any step that was created but never saved (never clicked Save button). // Instead, new steps should have their stepType immediately added // to 'savedStepForms' upon creation. - const steps = selectors.getSteps(state) + const steps = steplistSelectors.getSteps(state) const stepTypeFromStepReducer = steps[stepId] && steps[stepId].stepType const stepType = newStepType || stepTypeFromStepReducer @@ -44,7 +39,6 @@ function getStepFormData (state: BaseState, stepId: StepIdType, newStepType?: St return generateNewForm({ stepId, stepType: stepType, - defaultNextPipette, }) } @@ -58,7 +52,35 @@ export const selectStep = (stepId: StepIdType, newStepType?: StepType): ThunkAct dispatch(selectStepAction) - const formData = getStepFormData(getState(), stepId, newStepType) + const state = getState() + let formData = getStepFormData(state, stepId, newStepType) + + const defaultPipetteId = getNextDefaultPipetteId( + steplistSelectors.getSavedForms(state), + steplistSelectors.getOrderedSteps(state), + pipetteSelectors.getEquippedPipettes(state), + ) + + // For a pristine step, if there is a `pipette` field in the form + // (added by upstream `getDefaultsForStepType` fn), + // then set `pipette` field of new steps to the next default pipette id. + // + // In order to trigger dependent field changes (eg default disposal volume), + // update the form thru handleFormChange. + const formHasPipetteField = formData && 'pipette' in formData + if (newStepType && formHasPipetteField && defaultPipetteId) { + const updatePayload = {update: {pipette: defaultPipetteId}} + const updatedFields = handleFormChange( + updatePayload, + formData, + getState).update + + formData = { + ...formData, + ...updatedFields, + } + } + dispatch({ type: 'POPULATE_FORM', payload: formData, @@ -96,7 +118,7 @@ export type ReorderSelectedStepAction = { export const reorderSelectedStep = (delta: number) => (dispatch: ThunkDispatch, getState: GetState) => { - const stepId = selectors.getSelectedStepId(getState()) + const stepId = steplistSelectors.getSelectedStepId(getState()) if (stepId != null) { dispatch({ diff --git a/protocol-designer/src/steplist/formLevel/generateNewForm.js b/protocol-designer/src/steplist/formLevel/generateNewForm.js index 13c44973205..98d1a7a3086 100644 --- a/protocol-designer/src/steplist/formLevel/generateNewForm.js +++ b/protocol-designer/src/steplist/formLevel/generateNewForm.js @@ -8,17 +8,14 @@ import type { FormData, } from '../../form-types' -const FORMS_WITH_PIPETTE = ['transfer', 'mix', 'distribute', 'consolidate'] - type NewFormArgs = { stepId: StepIdType, stepType: StepType, - defaultNextPipette: string, } // Add default values to a new step form export default function generateNewForm (args: NewFormArgs): FormData { - const {stepId, stepType, defaultNextPipette} = args + const {stepId, stepType} = args const baseForm: BlankForm = { id: stepId, stepType: stepType, @@ -28,10 +25,6 @@ export default function generateNewForm (args: NewFormArgs): FormData { let additionalFields = {} - if (FORMS_WITH_PIPETTE.includes(stepType)) { - additionalFields.pipette = defaultNextPipette - } - return { ...baseForm, ...getDefaultsForStepType(stepType), diff --git a/protocol-designer/src/steplist/formLevel/getDefaultsForStepType.js b/protocol-designer/src/steplist/formLevel/getDefaultsForStepType.js index 48e14bd2a47..2d8d88c8919 100644 --- a/protocol-designer/src/steplist/formLevel/getDefaultsForStepType.js +++ b/protocol-designer/src/steplist/formLevel/getDefaultsForStepType.js @@ -24,6 +24,7 @@ export default function getDefaultsForStepType (stepType: StepType) { 'dispense_wellOrder_second': DEFAULT_WELL_ORDER_SECOND_OPTION, 'dispense_mmFromBottom': DEFAULT_MM_FROM_BOTTOM_DISPENSE, 'dispense_wells': [], + 'pipette': null, 'volume': undefined, } case 'consolidate': @@ -37,6 +38,7 @@ export default function getDefaultsForStepType (stepType: StepType) { 'dispense_labware': null, 'dispense_mmFromBottom': DEFAULT_MM_FROM_BOTTOM_DISPENSE, 'dispense_wells': [], + 'pipette': null, 'volume': undefined, } case 'mix': @@ -45,9 +47,10 @@ export default function getDefaultsForStepType (stepType: StepType) { 'labware': null, 'aspirate_wellOrder_first': DEFAULT_WELL_ORDER_FIRST_OPTION, 'aspirate_wellOrder_second': DEFAULT_WELL_ORDER_SECOND_OPTION, - 'wells': [], 'mix_mmFromBottom': DEFAULT_MM_FROM_BOTTOM_DISPENSE, // NOTE: mix uses dispense for both asp + disp, for now + 'pipette': null, 'volume': undefined, + 'wells': [], } case 'distribute': return { @@ -62,6 +65,7 @@ export default function getDefaultsForStepType (stepType: StepType) { 'dispense_wellOrder_second': DEFAULT_WELL_ORDER_SECOND_OPTION, 'dispense_mmFromBottom': DEFAULT_MM_FROM_BOTTOM_DISPENSE, 'dispense_wells': [], + 'pipette': null, 'volume': undefined, } default: diff --git a/protocol-designer/src/steplist/formLevel/getNextDefaultPipetteId/index.js b/protocol-designer/src/steplist/formLevel/getNextDefaultPipetteId/index.js index e3529d1f925..0aca04bb82b 100644 --- a/protocol-designer/src/steplist/formLevel/getNextDefaultPipetteId/index.js +++ b/protocol-designer/src/steplist/formLevel/getNextDefaultPipetteId/index.js @@ -1,5 +1,7 @@ // @flow +import findKey from 'lodash/findKey' import last from 'lodash/last' +import type {PipetteData} from '../../../step-generation' import type {StepIdType, FormData} from '../../../form-types' // TODO: Ian 2018-09-18 once we support switching pipettes mid-protocol, @@ -11,7 +13,7 @@ import type {StepIdType, FormData} from '../../../form-types' export default function getNextDefaultPipetteId ( savedForms: {[StepIdType]: FormData}, orderedSteps: Array, - pipetteIdsByMount: {left?: ?string, right?: ?string} + equippedPipettesById: {[string]: PipetteData} ): string { const prevPipetteSteps = orderedSteps .map(stepId => savedForms[stepId]) @@ -19,10 +21,11 @@ export default function getNextDefaultPipetteId ( const lastPipetteStep = last(prevPipetteSteps) - const nextDefaultPipette = ( + // NOTE: order of findKey not guaranteed, expecting at most one pipette on each mount + const nextDefaultPipette: ?string = ( (lastPipetteStep && lastPipetteStep.pipette) || - pipetteIdsByMount['left'] || - pipetteIdsByMount['right'] + findKey(equippedPipettesById, p => p.mount === 'left') || + findKey(equippedPipettesById, p => p.mount === 'right') ) if (!nextDefaultPipette) { diff --git a/protocol-designer/src/steplist/formLevel/getNextDefaultPipetteId/test/getNextDefaultPipetteId.test.js b/protocol-designer/src/steplist/formLevel/getNextDefaultPipetteId/test/getNextDefaultPipetteId.test.js index 195c11cc859..c2329b997dc 100644 --- a/protocol-designer/src/steplist/formLevel/getNextDefaultPipetteId/test/getNextDefaultPipetteId.test.js +++ b/protocol-designer/src/steplist/formLevel/getNextDefaultPipetteId/test/getNextDefaultPipetteId.test.js @@ -6,17 +6,20 @@ describe('getNextDefaultPipetteId', () => { const testCases = [ { testMsg: 'both pipettes present: use left pipette', - equippedPipettes: {'left': 'leftId', 'right': 'rightId'}, + equippedPipettesById: { + 'leftId': {id: 'leftId', mount: 'left'}, + 'rightId': {id: 'rightId', mount: 'right'}, + }, expected: 'leftId', }, { testMsg: 'right only: use right', - equippedPipettes: {'right': 'rightId'}, + equippedPipettesById: {'rightId': {id: 'rightId', mount: 'right'}}, expected: 'rightId', }, ] - testCases.forEach(({testMsg, equippedPipettes, expected}) => { + testCases.forEach(({testMsg, equippedPipettesById, expected}) => { test(testMsg, () => { const savedForms = {} const orderedSteps = [] @@ -24,7 +27,7 @@ describe('getNextDefaultPipetteId', () => { const result = getNextDefaultPipetteId( savedForms, orderedSteps, - equippedPipettes) + equippedPipettesById) expect(result).toBe(expected) }) @@ -41,7 +44,7 @@ describe('getNextDefaultPipetteId', () => { { testMsg: 'no previous forms use pipettes', orderedSteps: ['x', 'x', 'x'], - expected: 'default', + expected: 'defaultId', }, { testMsg: 'mix of steps with and without pipettes', @@ -56,7 +59,7 @@ describe('getNextDefaultPipetteId', () => { { testMsg: 'only missing steps', orderedSteps: ['missingStep', 'missingStep'], - expected: 'default', + expected: 'defaultId', }, ] @@ -68,12 +71,12 @@ describe('getNextDefaultPipetteId', () => { x: {}, // no 'pipette' key, eg a Pause step } - const equippedPipettes = {left: 'default'} + const equippedPipettesById = {'defaultId': {id: 'defaultId', mount: 'left'}} const result = getNextDefaultPipetteId( savedForms, orderedSteps, - equippedPipettes) + equippedPipettesById) expect(result).toBe(expected) }) diff --git a/protocol-designer/src/steplist/formLevel/warnings.js b/protocol-designer/src/steplist/formLevel/warnings.js index b1c4d868553..b64b681a674 100644 --- a/protocol-designer/src/steplist/formLevel/warnings.js +++ b/protocol-designer/src/steplist/formLevel/warnings.js @@ -1,10 +1,10 @@ // @flow import * as React from 'react' import {getWellTotalVolume} from '@opentrons/shared-data' +import {DISPOSAL_VOLUME_PERCENTAGE} from '../../constants' import type {StepFieldName} from '../../form-types' import KnowledgeBaseLink from '../../components/KnowledgeBaseLink' -export const DISPOSAL_PERCENTAGE = 0.2 // 20% percent of pipette capacity /******************* ** Warning Messages ** ********************/ @@ -62,7 +62,7 @@ export const minDisposalVolume = (fields: HydratedFormData): ?FormWarning => { if (!pipette) return null const isUnselected = !aspirate_disposalVol_checkbox || !aspirate_disposalVol_volume if (isUnselected) return FORM_WARNINGS.BELOW_MIN_DISPOSAL_VOLUME - const isBelowMin = aspirate_disposalVol_volume < (DISPOSAL_PERCENTAGE * pipette.maxVolume) + const isBelowMin = aspirate_disposalVol_volume < (DISPOSAL_VOLUME_PERCENTAGE * pipette.maxVolume) return isBelowMin ? FORM_WARNINGS.BELOW_MIN_DISPOSAL_VOLUME : null }