From ad8650fa74f9359f46f299165bf74ca191f70e1f Mon Sep 17 00:00:00 2001 From: IanLondon Date: Fri, 6 Mar 2020 12:49:53 -0500 Subject: [PATCH 01/10] wip. for now, always auto-add pause step w/o modal --- .../src/components/StepCreationButton.js | 2 +- .../StepEditForm/ButtonRow/index.js | 11 +-- .../src/step-forms/actions/index.js | 15 --- .../src/step-forms/reducers/index.js | 4 +- .../src/ui/steps/actions/thunks.js | 92 ++++++++++++++++++- 5 files changed, 97 insertions(+), 27 deletions(-) diff --git a/protocol-designer/src/components/StepCreationButton.js b/protocol-designer/src/components/StepCreationButton.js index cf97d50acc1..a6eebb2a1f4 100644 --- a/protocol-designer/src/components/StepCreationButton.js +++ b/protocol-designer/src/components/StepCreationButton.js @@ -132,7 +132,7 @@ const mapSTP = (state: BaseState): SP => { const mapDTP = (dispatch: ThunkDispatch<*>): DP => ({ makeAddStep: (stepType: StepType) => (e: SyntheticEvent<>) => - dispatch(stepsActions.addStep({ stepType })), + dispatch(stepsActions.addAndSelectStepWithHints({ stepType })), }) export const StepCreationButton = connect( diff --git a/protocol-designer/src/components/StepEditForm/ButtonRow/index.js b/protocol-designer/src/components/StepEditForm/ButtonRow/index.js index 1178cc0ab95..339ee43c0f4 100644 --- a/protocol-designer/src/components/StepEditForm/ButtonRow/index.js +++ b/protocol-designer/src/components/StepEditForm/ButtonRow/index.js @@ -5,7 +5,7 @@ import cx from 'classnames' import { OutlineButton, PrimaryButton } from '@opentrons/components' import { actions as steplistActions } from '../../../steplist' -import { actions as stepFormActions } from '../../../step-forms' +import { actions as stepsActions } from '../../../ui/steps' import { getCurrentFormCanBeSaved } from '../../../step-forms/selectors' @@ -14,9 +14,6 @@ import type { BaseState } from '../../../types' import modalStyles from '../../modals/modal.css' import styles from './styles.css' -const { cancelStepForm } = steplistActions -const { saveStepForm } = stepFormActions - type Props = {| onClickMoreOptions: (event: SyntheticEvent<>) => mixed, onDelete?: (event: SyntheticEvent<>) => mixed, @@ -44,14 +41,16 @@ export const ButtonRow = ({ onDelete, onClickMoreOptions }: Props) => {
dispatch(cancelStepForm())} + onClick={() => dispatch(steplistActions.cancelStepForm())} > Close dispatch(saveStepForm()) : undefined} + onClick={ + canSave ? () => dispatch(stepsActions.saveStepForm()) : undefined + } > Save diff --git a/protocol-designer/src/step-forms/actions/index.js b/protocol-designer/src/step-forms/actions/index.js index 32e9bf1eed3..2d77d8c1c75 100644 --- a/protocol-designer/src/step-forms/actions/index.js +++ b/protocol-designer/src/step-forms/actions/index.js @@ -1,18 +1,3 @@ // @flow -import { getUnsavedForm } from '../selectors' -import type { Dispatch } from 'redux' -import type { StepIdType } from '../../form-types' -import type { GetState } from '../../types' - export * from './modules' export * from './pipettes' - -export const SAVE_STEP_FORM: 'SAVE_STEP_FORM' = 'SAVE_STEP_FORM' - -export type SaveStepFormAction = {| - type: typeof SAVE_STEP_FORM, - payload: { id: StepIdType }, -|} - -export const saveStepForm = () => (dispatch: Dispatch<*>, getState: GetState) => - dispatch({ type: SAVE_STEP_FORM, payload: getUnsavedForm(getState()) }) diff --git a/protocol-designer/src/step-forms/reducers/index.js b/protocol-designer/src/step-forms/reducers/index.js index bb9d4afb08e..779771d9c93 100644 --- a/protocol-designer/src/step-forms/reducers/index.js +++ b/protocol-designer/src/step-forms/reducers/index.js @@ -58,7 +58,8 @@ import type { import type { DuplicateStepAction, ReorderSelectedStepAction, -} from '../../ui/steps' +} from '../../ui/steps/actions/types' +import type { SaveStepFormAction } from '../../ui/steps/actions/thunks' import type { StepItemData } from '../../steplist/types' import type { NormalizedPipetteById, @@ -73,7 +74,6 @@ import type { CreateModuleAction, EditModuleAction, DeleteModuleAction, - SaveStepFormAction, } from '../actions' type FormState = FormData | null diff --git a/protocol-designer/src/ui/steps/actions/thunks.js b/protocol-designer/src/ui/steps/actions/thunks.js index 8c03e7f7714..8df018f3242 100644 --- a/protocol-designer/src/ui/steps/actions/thunks.js +++ b/protocol-designer/src/ui/steps/actions/thunks.js @@ -1,4 +1,11 @@ // @flow +import assert from 'assert' +import { + getUnsavedForm, + getSavedStepForms, +} from '../../../step-forms/selectors' +import { changeFormInput } from '../../../steplist/actions/actions' + import { uuid } from '../../../utils' import { selectors as labwareIngredsSelectors } from '../../../labware-ingred/selectors' import { getSelectedStepId } from '../selectors' @@ -8,15 +15,16 @@ import { actions as tutorialActions } from '../../../tutorial' import * as uiModuleSelectors from '../../../ui/modules/selectors' import type { DuplicateStepAction } from './types' -import type { StepType, StepIdType } from '../../../form-types' +import type { StepType, StepIdType, FormData } from '../../../form-types' import type { GetState, ThunkDispatch } from '../../../types' // addStep thunk adds an incremental integer ID for Step reducers. +// NOTE: if this is an "add step" directly performed by the user, +// addAndSelectStepWithHints is probably what you want export const addStep = (payload: { stepType: StepType }) => ( dispatch: ThunkDispatch<*>, getState: GetState ) => { - const state = getState() const stepId = uuid() const { stepType } = payload dispatch({ @@ -26,6 +34,16 @@ export const addStep = (payload: { stepType: StepType }) => ( id: stepId, }, }) + + dispatch(selectStep(stepId, stepType)) +} + +export const addAndSelectStepWithHints = (payload: { stepType: StepType }) => ( + dispatch: ThunkDispatch<*>, + getState: GetState +) => { + dispatch(addStep(payload)) + const state = getState() const deckHasLiquid = labwareIngredsSelectors.getDeckHasLiquid(state) const magnetModuleHasLabware = uiModuleSelectors.getMagnetModuleHasLabware( state @@ -58,7 +76,6 @@ export const addStep = (payload: { stepType: StepType }) => ( if (stepModuleMissingLabware) { dispatch(tutorialActions.addHint('module_without_labware')) } - dispatch(selectStep(stepId, stepType)) } export type ReorderSelectedStepAction = { @@ -99,3 +116,72 @@ export const duplicateStep = (stepId: StepIdType) => ( }) } } + +export const SAVE_STEP_FORM: 'SAVE_STEP_FORM' = 'SAVE_STEP_FORM' + +export type SaveStepFormAction = {| + type: typeof SAVE_STEP_FORM, + payload: FormData, +|} + +export const _saveStepForm = (form: FormData): SaveStepFormAction => ({ + type: SAVE_STEP_FORM, + payload: form, +}) + +export const _isChangeToTempForm = (form: FormData): boolean => + form?.stepType === 'temperature' && form?.setTemperature === 'true' + +export const saveStepForm = () => ( + dispatch: ThunkDispatch<*>, + getState: GetState +) => { + const initialState = getState() + const unsavedForm = getUnsavedForm(initialState) + + if (!unsavedForm) { + // this is for Flow + assert( + false, + 'Tried to saveStepForm with falsey unsavedForm. This should never be able to happen.' + ) + return + } + const { id } = unsavedForm + const isPristineForm = !(id in getSavedStepForms(initialState)) + const isPristineChangeToTempForm = + isPristineForm && _isChangeToTempForm(unsavedForm) + + // save the form + dispatch(_saveStepForm(unsavedForm)) + + // TODO IMMEDIATELY: make this a different action that is gated by a modal + if (isPristineChangeToTempForm) { + const setTemperatureForm = unsavedForm + const temperature = setTemperatureForm?.targetTemperature + assert( + temperature != null && temperature !== '', + `tried to auto-add a pause until temp, but targetTemperature was ${temperature}` + ) + addStep({ stepType: 'pause' })(dispatch, getState) + // NOTE: fields should be set one at a time b/c dependentFieldsUpdate fns can filter out inputs + // contingent on other inputs (eg changing the pauseForAmountOfTime radio button may clear the pauseTemperature). + dispatch( + changeFormInput({ + update: { + pauseForAmountOfTime: 'untilTemperature', + }, + }) + ) + dispatch(changeFormInput({ update: { pauseTemperature: temperature } })) + // finally save the new pause form + const unsavedPauseForm = getUnsavedForm(getState()) + + // this conditional is for Flow, the unsaved form should always exist + if (unsavedPauseForm != null) { + dispatch(_saveStepForm(unsavedPauseForm)) + } else { + assert(false, 'could not auto-save pause form, getUnsavedForm returned') + } + } +} From fa78d2d455710d8fbc9ba9a588697beaf5c7881f Mon Sep 17 00:00:00 2001 From: IanLondon Date: Fri, 6 Mar 2020 13:45:46 -0500 Subject: [PATCH 02/10] make pseudo-modal, controlled by ButtonRow --- .../StepEditForm/ButtonRow/index.js | 113 +++++++++++++----- .../src/step-forms/selectors/index.js | 11 ++ .../src/ui/steps/actions/thunks.js | 95 ++++++++++----- 3 files changed, 155 insertions(+), 64 deletions(-) diff --git a/protocol-designer/src/components/StepEditForm/ButtonRow/index.js b/protocol-designer/src/components/StepEditForm/ButtonRow/index.js index 339ee43c0f4..e82bb06f0ae 100644 --- a/protocol-designer/src/components/StepEditForm/ButtonRow/index.js +++ b/protocol-designer/src/components/StepEditForm/ButtonRow/index.js @@ -1,5 +1,5 @@ // @flow -import React from 'react' +import React, { useCallback, useState } from 'react' import { useDispatch, useSelector } from 'react-redux' import cx from 'classnames' import { OutlineButton, PrimaryButton } from '@opentrons/components' @@ -7,7 +7,11 @@ import { OutlineButton, PrimaryButton } from '@opentrons/components' import { actions as steplistActions } from '../../../steplist' import { actions as stepsActions } from '../../../ui/steps' -import { getCurrentFormCanBeSaved } from '../../../step-forms/selectors' +import { + getCurrentFormCanBeSaved, + getUnsavedForm, + getUnsavedFormIsPristineSetTempForm, +} from '../../../step-forms/selectors' import type { BaseState } from '../../../types' @@ -20,41 +24,86 @@ type Props = {| |} export const ButtonRow = ({ onDelete, onClickMoreOptions }: Props) => { + const [ + showAddPauseUntilTempStepModal, + setShowAddPauseUntilTempStepModal, + ] = useState(false) + + const dispatch = useDispatch() const canSave = useSelector((state: BaseState): boolean => getCurrentFormCanBeSaved(state) ) - const dispatch = useDispatch() + const unsavedForm = useSelector(getUnsavedForm) + const shouldShowPauseUntilTempStepModal = useSelector( + getUnsavedFormIsPristineSetTempForm + ) + + const handleClickSave = useCallback(() => { + if (!canSave) { + return + } + if (shouldShowPauseUntilTempStepModal) { + setShowAddPauseUntilTempStepModal(true) + } else { + dispatch(stepsActions.saveStepForm()) + } + }, [canSave, shouldShowPauseUntilTempStepModal, dispatch]) return ( -
-
- - Delete - - - Notes - -
-
- dispatch(steplistActions.cancelStepForm())} - > - Close - - dispatch(stepsActions.saveStepForm()) : undefined - } - > - Save - + <> + {showAddPauseUntilTempStepModal && ( +
+

+ Pause until module is {unsavedForm?.targetTemperature} DegReeSSS? +

+ + +
+ )} +
+
+ + Delete + + + Notes + +
+
+ dispatch(steplistActions.cancelStepForm())} + > + Close + + + Save + +
-
+ ) } diff --git a/protocol-designer/src/step-forms/selectors/index.js b/protocol-designer/src/step-forms/selectors/index.js index 9988eefa957..b91b7e007b9 100644 --- a/protocol-designer/src/step-forms/selectors/index.js +++ b/protocol-designer/src/step-forms/selectors/index.js @@ -523,6 +523,17 @@ export const getIsNewStepForm: Selector = createSelector( formData && formData.id != null ? !savedForms[formData.id] : true ) +export const getUnsavedFormIsPristineSetTempForm: Selector = createSelector( + getUnsavedForm, + getIsNewStepForm, + (unsavedForm, isNewStepForm) => { + const isSetTempForm = + unsavedForm?.stepType === 'temperature' && + unsavedForm?.setTemperature === 'true' + return isNewStepForm && isSetTempForm + } +) + export const getFormLevelWarningsForUnsavedForm: Selector< Array > = createSelector( diff --git a/protocol-designer/src/ui/steps/actions/thunks.js b/protocol-designer/src/ui/steps/actions/thunks.js index 8df018f3242..1313acb5adc 100644 --- a/protocol-designer/src/ui/steps/actions/thunks.js +++ b/protocol-designer/src/ui/steps/actions/thunks.js @@ -2,7 +2,7 @@ import assert from 'assert' import { getUnsavedForm, - getSavedStepForms, + getUnsavedFormIsPristineSetTempForm, } from '../../../step-forms/selectors' import { changeFormInput } from '../../../steplist/actions/actions' @@ -132,56 +132,87 @@ export const _saveStepForm = (form: FormData): SaveStepFormAction => ({ export const _isChangeToTempForm = (form: FormData): boolean => form?.stepType === 'temperature' && form?.setTemperature === 'true' +/** take unsavedForm state and put it into the payload */ export const saveStepForm = () => ( dispatch: ThunkDispatch<*>, getState: GetState -) => { +): void => { const initialState = getState() const unsavedForm = getUnsavedForm(initialState) + // this check is only for Flow. At this point, unsavedForm should always be populated if (!unsavedForm) { - // this is for Flow assert( false, 'Tried to saveStepForm with falsey unsavedForm. This should never be able to happen.' ) return } - const { id } = unsavedForm - const isPristineForm = !(id in getSavedStepForms(initialState)) - const isPristineChangeToTempForm = - isPristineForm && _isChangeToTempForm(unsavedForm) // save the form dispatch(_saveStepForm(unsavedForm)) +} - // TODO IMMEDIATELY: make this a different action that is gated by a modal - if (isPristineChangeToTempForm) { - const setTemperatureForm = unsavedForm - const temperature = setTemperatureForm?.targetTemperature +/** "power action", mimicking saving the never-saved "set temperature X" step, + ** then creating and saving a "pause until temp X" step */ +export const saveSetTempFormWithAddedPauseUntilTemp = () => ( + dispatch: ThunkDispatch<*>, + getState: GetState +): void => { + const initialState = getState() + const unsavedSetTemperatureForm = getUnsavedForm(initialState) + const isPristineSetTempForm = getUnsavedFormIsPristineSetTempForm( + initialState + ) + + // this check is only for Flow. At this point, unsavedForm should always be populated + if (!unsavedSetTemperatureForm) { assert( - temperature != null && temperature !== '', - `tried to auto-add a pause until temp, but targetTemperature was ${temperature}` + false, + 'Tried to saveSetTempFormWithAddedPauseUntilTemp with falsey unsavedForm. This should never be able to happen.' ) - addStep({ stepType: 'pause' })(dispatch, getState) - // NOTE: fields should be set one at a time b/c dependentFieldsUpdate fns can filter out inputs - // contingent on other inputs (eg changing the pauseForAmountOfTime radio button may clear the pauseTemperature). - dispatch( - changeFormInput({ - update: { - pauseForAmountOfTime: 'untilTemperature', - }, - }) + return + } + const { id } = unsavedSetTemperatureForm + + if (!isPristineSetTempForm) { + // this check should happen upstream (before dispatching saveSetTempFormWithAddedPauseUntilTemp in the first place) + assert( + false, + `tried to saveSetTempFormWithAddedPauseUntilTemp but form ${id} is not a pristine set temp form` ) - dispatch(changeFormInput({ update: { pauseTemperature: temperature } })) - // finally save the new pause form - const unsavedPauseForm = getUnsavedForm(getState()) - - // this conditional is for Flow, the unsaved form should always exist - if (unsavedPauseForm != null) { - dispatch(_saveStepForm(unsavedPauseForm)) - } else { - assert(false, 'could not auto-save pause form, getUnsavedForm returned') - } + return + } + + const temperature = unsavedSetTemperatureForm?.targetTemperature + assert( + temperature != null && temperature !== '', + `tried to auto-add a pause until temp, but targetTemperature is missing: ${temperature}` + ) + // save the set temperature step form that is currently open + dispatch(_saveStepForm(unsavedSetTemperatureForm)) + + // add a new pause step form + addStep({ stepType: 'pause' })(dispatch, getState) + + // NOTE: fields should be set one at a time b/c dependentFieldsUpdate fns can filter out inputs + // contingent on other inputs (eg changing the pauseForAmountOfTime radio button may clear the pauseTemperature). + dispatch( + changeFormInput({ + update: { + pauseForAmountOfTime: 'untilTemperature', + }, + }) + ) + dispatch(changeFormInput({ update: { pauseTemperature: temperature } })) + + // finally save the new pause form + const unsavedPauseForm = getUnsavedForm(getState()) + + // this conditional is for Flow, the unsaved form should always exist + if (unsavedPauseForm != null) { + dispatch(_saveStepForm(unsavedPauseForm)) + } else { + assert(false, 'could not auto-save pause form, getUnsavedForm returned') } } From ed9bd755f5038b11148a79defe7dd7b9c938ae25 Mon Sep 17 00:00:00 2001 From: IanLondon Date: Fri, 6 Mar 2020 16:34:05 -0500 Subject: [PATCH 03/10] working styled modal --- .../StepEditForm/ButtonRow/index.js | 38 ++++++---------- .../modals/AutoAddPauseUntilTempStepModal.css | 20 +++++++++ .../modals/AutoAddPauseUntilTempStepModal.js | 44 +++++++++++++++++++ .../src/localization/en/modal.json | 6 +++ 4 files changed, 84 insertions(+), 24 deletions(-) create mode 100644 protocol-designer/src/components/modals/AutoAddPauseUntilTempStepModal.css create mode 100644 protocol-designer/src/components/modals/AutoAddPauseUntilTempStepModal.js diff --git a/protocol-designer/src/components/StepEditForm/ButtonRow/index.js b/protocol-designer/src/components/StepEditForm/ButtonRow/index.js index e82bb06f0ae..b7e7cc5e8c8 100644 --- a/protocol-designer/src/components/StepEditForm/ButtonRow/index.js +++ b/protocol-designer/src/components/StepEditForm/ButtonRow/index.js @@ -3,7 +3,7 @@ import React, { useCallback, useState } from 'react' import { useDispatch, useSelector } from 'react-redux' import cx from 'classnames' import { OutlineButton, PrimaryButton } from '@opentrons/components' - +import { AutoAddPauseUntilTempStepModal } from '../../modals/AutoAddPauseUntilTempStepModal' import { actions as steplistActions } from '../../../steplist' import { actions as stepsActions } from '../../../ui/steps' @@ -52,29 +52,19 @@ export const ButtonRow = ({ onDelete, onClickMoreOptions }: Props) => { return ( <> {showAddPauseUntilTempStepModal && ( -
-

- Pause until module is {unsavedForm?.targetTemperature} DegReeSSS? -

- - -
+ { + setShowAddPauseUntilTempStepModal(false) + // save normally + dispatch(stepsActions.saveStepForm()) + }} + handleContinueClick={() => { + setShowAddPauseUntilTempStepModal(false) + // save this form and add a subsequent pause + dispatch(stepsActions.saveSetTempFormWithAddedPauseUntilTemp()) + }} + /> )}
diff --git a/protocol-designer/src/components/modals/AutoAddPauseUntilTempStepModal.css b/protocol-designer/src/components/modals/AutoAddPauseUntilTempStepModal.css new file mode 100644 index 00000000000..3630c28da94 --- /dev/null +++ b/protocol-designer/src/components/modals/AutoAddPauseUntilTempStepModal.css @@ -0,0 +1,20 @@ +@import '@opentrons/components'; + +.header { + @apply --font-header-dark; + + margin-bottom: 1rem; +} + +.body { + @apply --font-body-2-dark; +} + +.later_button { + width: 16rem; +} + +.now_button { + width: 15rem; + margin-left: 1rem; +} diff --git a/protocol-designer/src/components/modals/AutoAddPauseUntilTempStepModal.js b/protocol-designer/src/components/modals/AutoAddPauseUntilTempStepModal.js new file mode 100644 index 00000000000..985aa3ad618 --- /dev/null +++ b/protocol-designer/src/components/modals/AutoAddPauseUntilTempStepModal.js @@ -0,0 +1,44 @@ +// @flow +import React from 'react' +import { AlertModal, OutlineButton, PrimaryButton } from '@opentrons/components' +import { i18n } from '../../localization' +import modalStyles from './modal.css' +import styles from './AutoAddPauseUntilTempStepModal.css' + +type Props = {| + displayTemperature: string, + handleCancelClick: () => mixed, + handleContinueClick: () => mixed, +|} + +export const AutoAddPauseUntilTempStepModal = (props: Props) => ( + +
+ {i18n.t('modal.auto_add_pause_until_temp_step.title', { + temperature: props.displayTemperature, + })} +
+

+ {i18n.t('modal.auto_add_pause_until_temp_step.body', { + temperature: props.displayTemperature, + })} +

+
+ + {i18n.t('modal.auto_add_pause_until_temp_step.later_button')} + + + {i18n.t('modal.auto_add_pause_until_temp_step.now_button')} + +
+ +) diff --git a/protocol-designer/src/localization/en/modal.json b/protocol-designer/src/localization/en/modal.json index a937881054a..a3df96a1134 100644 --- a/protocol-designer/src/localization/en/modal.json +++ b/protocol-designer/src/localization/en/modal.json @@ -100,5 +100,11 @@ "body1": "Warning: Any experimental features used in this protocol may cause inconsistent behavior in the Protocol Designer. For example, the Protocol Designer may no longer display all warnings or errors that exist when no experimental features have been used.", "body2": "We encourage you to report any bugs you find, however note that at this time we are unable to prioritize fixes as they are a result of experimental features." } + }, + "auto_add_pause_until_temp_step": { + "title": "Pause until module is {{temperature}}°C?", + "body": "Should your protocol pause until the module is {{temperature}}°C? Or should the protocol continue while the module changes temperature?", + "now_button": "Pause protocol now", + "later_button": "I will build a pause later" } } From 28f62bacfe6b5b174375795faa4c5de526eed552 Mon Sep 17 00:00:00 2001 From: IanLondon Date: Fri, 6 Mar 2020 18:33:57 -0500 Subject: [PATCH 04/10] add test for addStep thunk --- .../steps/actions/__tests__/addStep.test.js | 39 +++++++++++++++++++ .../src/ui/steps/actions/thunks.js | 3 -- 2 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 protocol-designer/src/ui/steps/actions/__tests__/addStep.test.js diff --git a/protocol-designer/src/ui/steps/actions/__tests__/addStep.test.js b/protocol-designer/src/ui/steps/actions/__tests__/addStep.test.js new file mode 100644 index 00000000000..0d4641626cf --- /dev/null +++ b/protocol-designer/src/ui/steps/actions/__tests__/addStep.test.js @@ -0,0 +1,39 @@ +// @flow +import { addStep } from '../thunks' +import { selectStep } from '../actions' +import { uuid } from '../../../../utils' +jest.mock('../actions') +jest.mock('../../../../utils') + +const dispatch = jest.fn() +const getState = jest.fn() +const uuidMock: JestMockFn<[], string> = uuid +const selectStepMock: JestMockFn<[string, string], *> = selectStep +const id = 'someUUID' + +beforeEach(() => { + dispatch.mockClear() + getState.mockClear() + + uuidMock.mockReturnValue(id) + selectStepMock.mockReturnValue('selectStepMockReturnValue') +}) + +describe('addStep', () => { + it('should dispatch an ADD_STEP action with given stepType and a UUID, then dispatch selectStep thunk', () => { + const stepType = 'transfer' + addStep({ stepType })(dispatch, getState) + + expect(dispatch.mock.calls).toEqual([ + [ + { + type: 'ADD_STEP', + payload: { stepType, id }, + }, + ], + ['selectStepMockReturnValue'], + ]) + expect(selectStepMock).toHaveBeenCalledTimes(1) + expect(selectStepMock).toHaveBeenCalledWith(id, stepType) + }) +}) diff --git a/protocol-designer/src/ui/steps/actions/thunks.js b/protocol-designer/src/ui/steps/actions/thunks.js index 1313acb5adc..641c2fef275 100644 --- a/protocol-designer/src/ui/steps/actions/thunks.js +++ b/protocol-designer/src/ui/steps/actions/thunks.js @@ -129,9 +129,6 @@ export const _saveStepForm = (form: FormData): SaveStepFormAction => ({ payload: form, }) -export const _isChangeToTempForm = (form: FormData): boolean => - form?.stepType === 'temperature' && form?.setTemperature === 'true' - /** take unsavedForm state and put it into the payload */ export const saveStepForm = () => ( dispatch: ThunkDispatch<*>, From fedc90f26ff4fed55d2ae87ec54914a23b3cc9ae Mon Sep 17 00:00:00 2001 From: IanLondon Date: Mon, 9 Mar 2020 11:34:06 -0400 Subject: [PATCH 05/10] add test coverage for addAndSelectStepWithHints --- .../src/labware-ingred/selectors.js | 4 +- .../addAndSelectStepWithHints.test.js | 140 ++++++++++++++++++ .../steps/actions/__tests__/addStep.test.js | 2 +- .../src/ui/steps/actions/thunks/addStep.js | 31 ++++ .../actions/{thunks.js => thunks/index.js} | 42 ++---- 5 files changed, 185 insertions(+), 34 deletions(-) create mode 100644 protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js create mode 100644 protocol-designer/src/ui/steps/actions/thunks/addStep.js rename protocol-designer/src/ui/steps/actions/{thunks.js => thunks/index.js} (82%) diff --git a/protocol-designer/src/labware-ingred/selectors.js b/protocol-designer/src/labware-ingred/selectors.js index 53fb0bad3ae..21e7caae48d 100644 --- a/protocol-designer/src/labware-ingred/selectors.js +++ b/protocol-designer/src/labware-ingred/selectors.js @@ -20,7 +20,7 @@ import type { LiquidGroup, OrderedLiquids, } from './types' -import type { BaseState, MemoizedSelector } from './../types' +import type { BaseState, MemoizedSelector, Selector } from './../types' // TODO: Ian 2019-02-15 no RootSlice, use BaseState type RootSlice = { labwareIngred: RootState } @@ -143,7 +143,7 @@ const getLiquidGroupsOnDeck: MemoizedSelector> = createSelector( } ) -const getDeckHasLiquid: MemoizedSelector = createSelector( +const getDeckHasLiquid: Selector = createSelector( getLiquidGroupsOnDeck, liquidGroups => liquidGroups.length > 0 ) diff --git a/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js b/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js new file mode 100644 index 00000000000..f0e5d46e2e9 --- /dev/null +++ b/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js @@ -0,0 +1,140 @@ +// @flow +import { addAndSelectStepWithHints } from '../thunks' +import { addStep } from '../thunks/addStep' +import { addHint } from '../../../../tutorial/actions' +import * as uiModuleSelectors from '../../../../ui/modules/selectors' +import { selectors as labwareIngredSelectors } from '../../../../labware-ingred/selectors' +jest.mock('../thunks/addStep') +jest.mock('../../../../tutorial/actions') +jest.mock('../../../../ui/modules/selectors') +jest.mock('../../../../labware-ingred/selectors') + +const dispatch = jest.fn() +const getState = jest.fn() +const addStepMock: JestMockFn<[{ stepType: string }, string], *> = addStep +const addHintMock: JestMockFn<[any, string], *> = addHint +const mockGetDeckHasLiquid: JestMockFn<[Object], *> = + labwareIngredSelectors.getDeckHasLiquid +const mockGetMagnetModuleHasLabware: JestMockFn<[Object], *> = + uiModuleSelectors.getMagnetModuleHasLabware +const mockGetTemperatureModuleHasLabware: JestMockFn<[Object], *> = + uiModuleSelectors.getTemperatureModuleHasLabware +const mockGetThermocyclerModuleHasLabware: JestMockFn<[Object], *> = + uiModuleSelectors.getThermocyclerModuleHasLabware +const mockGetSingleTemperatureModuleId: JestMockFn<[Object], *> = + uiModuleSelectors.getSingleTemperatureModuleId +const mockGetSingleThermocyclerModuleId: JestMockFn<[Object], *> = + uiModuleSelectors.getSingleThermocyclerModuleId + +beforeEach(() => { + dispatch.mockClear() + getState.mockClear() + + addStepMock.mockClear() + addStepMock.mockReturnValue('addStepMockReturnValue') + addHintMock.mockClear() + addHintMock.mockReturnValue('addHintReturnValue') + + mockGetDeckHasLiquid.mockClear() + mockGetDeckHasLiquid.mockReturnValue(true) + mockGetMagnetModuleHasLabware.mockClear() + mockGetMagnetModuleHasLabware.mockReturnValue(false) + mockGetTemperatureModuleHasLabware.mockClear() + mockGetTemperatureModuleHasLabware.mockReturnValue(false) + mockGetThermocyclerModuleHasLabware.mockClear() + mockGetThermocyclerModuleHasLabware.mockReturnValue(false) + mockGetSingleTemperatureModuleId.mockClear() + mockGetSingleTemperatureModuleId.mockReturnValue(null) + mockGetSingleThermocyclerModuleId.mockClear() + mockGetSingleThermocyclerModuleId.mockReturnValue(null) +}) + +describe('addAndSelectStepWithHints', () => { + test('should dispatch ADD_STEP, and no hints when no hints are applicable (eg pause step)', () => { + const stepType = 'pause' + const payload = { stepType } + addAndSelectStepWithHints(payload)(dispatch, getState) + + expect(addStepMock.mock.calls).toEqual([[payload]]) + expect(dispatch.mock.calls).toEqual([['addStepMockReturnValue']]) + }) + + it('should dispatch ADD_STEP, and also ADD_HINT "add_liquids_and_labware" if we\'re adding a step that uses liquid but have no liquids on the deck', () => { + const stepType = 'moveLiquid' + const payload = { stepType } + mockGetDeckHasLiquid.mockReturnValue(false) // no liquid! + addAndSelectStepWithHints(payload)(dispatch, getState) + + expect(addStepMock.mock.calls).toEqual([[payload]]) + expect(addHintMock.mock.calls).toEqual([['add_liquids_and_labware']]) + expect(dispatch.mock.calls).toEqual([ + ['addStepMockReturnValue'], + ['addHintReturnValue'], + ]) + }) + + describe('should dispatch ADD_STEP, and also ADD_HINT "module_without_labware" for module steps if module lacks labware', () => { + ;[ + { + testName: 'magnet step, magnetic module has no labware', + stepType: 'magnet', + selectorValues: { + getMagnetModuleHasLabware: false, + getTemperatureModuleHasLabware: false, + getThermocyclerModuleHasLabware: false, + getSingleTemperatureModuleId: null, + getSingleThermocyclerModuleId: null, + }, + }, + { + testName: 'temperature step, temperature module has no labware', + stepType: 'temperature', + selectorValues: { + getMagnetModuleHasLabware: false, + getTemperatureModuleHasLabware: false, + getThermocyclerModuleHasLabware: false, + getSingleTemperatureModuleId: 'something', + getSingleThermocyclerModuleId: null, + }, + }, + { + testName: 'temperature step, thermocycler has no labware', + stepType: 'temperature', + selectorValues: { + getMagnetModuleHasLabware: false, + getTemperatureModuleHasLabware: false, + getThermocyclerModuleHasLabware: false, + getSingleTemperatureModuleId: null, + getSingleThermocyclerModuleId: 'something', + }, + }, + ].forEach(({ testName, stepType, selectorValues }) => { + it(testName, () => { + mockGetMagnetModuleHasLabware.mockReturnValue( + selectorValues.getMagnetModuleHasLabware + ) + mockGetTemperatureModuleHasLabware.mockReturnValue( + selectorValues.getTemperatureModuleHasLabware + ) + mockGetThermocyclerModuleHasLabware.mockReturnValue( + selectorValues.getThermocyclerModuleHasLabware + ) + mockGetSingleTemperatureModuleId.mockReturnValue( + selectorValues.getSingleTemperatureModuleId + ) + mockGetSingleThermocyclerModuleId.mockReturnValue( + selectorValues.getSingleThermocyclerModuleId + ) + const payload = { stepType } + addAndSelectStepWithHints(payload)(dispatch, getState) + + expect(addStepMock.mock.calls).toEqual([[payload]]) + expect(addHintMock.mock.calls).toEqual([['module_without_labware']]) + expect(dispatch.mock.calls).toEqual([ + ['addStepMockReturnValue'], + ['addHintReturnValue'], + ]) + }) + }) + }) +}) diff --git a/protocol-designer/src/ui/steps/actions/__tests__/addStep.test.js b/protocol-designer/src/ui/steps/actions/__tests__/addStep.test.js index 0d4641626cf..876e014458a 100644 --- a/protocol-designer/src/ui/steps/actions/__tests__/addStep.test.js +++ b/protocol-designer/src/ui/steps/actions/__tests__/addStep.test.js @@ -1,5 +1,5 @@ // @flow -import { addStep } from '../thunks' +import { addStep } from '../thunks/addStep' import { selectStep } from '../actions' import { uuid } from '../../../../utils' jest.mock('../actions') diff --git a/protocol-designer/src/ui/steps/actions/thunks/addStep.js b/protocol-designer/src/ui/steps/actions/thunks/addStep.js new file mode 100644 index 00000000000..50f004a916d --- /dev/null +++ b/protocol-designer/src/ui/steps/actions/thunks/addStep.js @@ -0,0 +1,31 @@ +// @flow +import { uuid } from '../../../../utils' +import { selectStep } from '../actions' + +import type { StepType } from '../../../../form-types' +import type { GetState, ThunkDispatch } from '../../../../types' + +type AddStepPayload = { id: string, stepType: StepType } +type AddStepAction = {| type: 'ADD_STEP', payload: AddStepPayload |} +const _addStep = (payload: AddStepPayload): AddStepAction => ({ + type: 'ADD_STEP', + payload, +}) + +// addStep thunk adds an incremental integer ID for Step reducers. +// NOTE: if this is an "add step" directly performed by the user, +// addAndSelectStepWithHints is probably what you want +export const addStep = (payload: { stepType: StepType }) => ( + dispatch: ThunkDispatch<*>, + getState: GetState +) => { + const stepId = uuid() + const { stepType } = payload + dispatch( + _addStep({ + ...payload, + id: stepId, + }) + ) + dispatch(selectStep(stepId, stepType)) +} diff --git a/protocol-designer/src/ui/steps/actions/thunks.js b/protocol-designer/src/ui/steps/actions/thunks/index.js similarity index 82% rename from protocol-designer/src/ui/steps/actions/thunks.js rename to protocol-designer/src/ui/steps/actions/thunks/index.js index 641c2fef275..ab26581f632 100644 --- a/protocol-designer/src/ui/steps/actions/thunks.js +++ b/protocol-designer/src/ui/steps/actions/thunks/index.js @@ -3,40 +3,20 @@ import assert from 'assert' import { getUnsavedForm, getUnsavedFormIsPristineSetTempForm, -} from '../../../step-forms/selectors' -import { changeFormInput } from '../../../steplist/actions/actions' +} from '../../../../step-forms/selectors' +import { changeFormInput } from '../../../../steplist/actions/actions' -import { uuid } from '../../../utils' -import { selectors as labwareIngredsSelectors } from '../../../labware-ingred/selectors' -import { getSelectedStepId } from '../selectors' -import { selectStep } from './actions' -import { actions as tutorialActions } from '../../../tutorial' +import { uuid } from '../../../../utils' +import { selectors as labwareIngredsSelectors } from '../../../../labware-ingred/selectors' +import { getSelectedStepId } from '../../selectors' +import { addStep } from './addStep' +import { actions as tutorialActions } from '../../../../tutorial' -import * as uiModuleSelectors from '../../../ui/modules/selectors' -import type { DuplicateStepAction } from './types' +import * as uiModuleSelectors from '../../../../ui/modules/selectors' +import type { DuplicateStepAction } from '../types' -import type { StepType, StepIdType, FormData } from '../../../form-types' -import type { GetState, ThunkDispatch } from '../../../types' - -// addStep thunk adds an incremental integer ID for Step reducers. -// NOTE: if this is an "add step" directly performed by the user, -// addAndSelectStepWithHints is probably what you want -export const addStep = (payload: { stepType: StepType }) => ( - dispatch: ThunkDispatch<*>, - getState: GetState -) => { - const stepId = uuid() - const { stepType } = payload - dispatch({ - type: 'ADD_STEP', - payload: { - ...payload, - id: stepId, - }, - }) - - dispatch(selectStep(stepId, stepType)) -} +import type { StepType, StepIdType, FormData } from '../../../../form-types' +import type { GetState, ThunkDispatch } from '../../../../types' export const addAndSelectStepWithHints = (payload: { stepType: StepType }) => ( dispatch: ThunkDispatch<*>, From 7ef90f97f8ab1fae755d188280804627e54411c4 Mon Sep 17 00:00:00 2001 From: IanLondon Date: Mon, 9 Mar 2020 13:38:11 -0400 Subject: [PATCH 06/10] less verbose mock clearing --- .../__tests__/addAndSelectStepWithHints.test.js | 11 +---------- .../src/ui/steps/actions/__tests__/addStep.test.js | 3 +-- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js b/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js index f0e5d46e2e9..0aea65c1cd3 100644 --- a/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js +++ b/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js @@ -27,25 +27,16 @@ const mockGetSingleThermocyclerModuleId: JestMockFn<[Object], *> = uiModuleSelectors.getSingleThermocyclerModuleId beforeEach(() => { - dispatch.mockClear() - getState.mockClear() + jest.clearAllMocks() - addStepMock.mockClear() addStepMock.mockReturnValue('addStepMockReturnValue') - addHintMock.mockClear() addHintMock.mockReturnValue('addHintReturnValue') - mockGetDeckHasLiquid.mockClear() mockGetDeckHasLiquid.mockReturnValue(true) - mockGetMagnetModuleHasLabware.mockClear() mockGetMagnetModuleHasLabware.mockReturnValue(false) - mockGetTemperatureModuleHasLabware.mockClear() mockGetTemperatureModuleHasLabware.mockReturnValue(false) - mockGetThermocyclerModuleHasLabware.mockClear() mockGetThermocyclerModuleHasLabware.mockReturnValue(false) - mockGetSingleTemperatureModuleId.mockClear() mockGetSingleTemperatureModuleId.mockReturnValue(null) - mockGetSingleThermocyclerModuleId.mockClear() mockGetSingleThermocyclerModuleId.mockReturnValue(null) }) diff --git a/protocol-designer/src/ui/steps/actions/__tests__/addStep.test.js b/protocol-designer/src/ui/steps/actions/__tests__/addStep.test.js index 876e014458a..6598cdce502 100644 --- a/protocol-designer/src/ui/steps/actions/__tests__/addStep.test.js +++ b/protocol-designer/src/ui/steps/actions/__tests__/addStep.test.js @@ -12,8 +12,7 @@ const selectStepMock: JestMockFn<[string, string], *> = selectStep const id = 'someUUID' beforeEach(() => { - dispatch.mockClear() - getState.mockClear() + jest.clearAllMocks() uuidMock.mockReturnValue(id) selectStepMock.mockReturnValue('selectStepMockReturnValue') From 1db1e0436686345c2b9ffd143febb95c57a77c4c Mon Sep 17 00:00:00 2001 From: IanLondon Date: Mon, 9 Mar 2020 13:41:48 -0400 Subject: [PATCH 07/10] test -> it --- .../steps/actions/__tests__/addAndSelectStepWithHints.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js b/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js index 0aea65c1cd3..480d1026773 100644 --- a/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js +++ b/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js @@ -41,7 +41,7 @@ beforeEach(() => { }) describe('addAndSelectStepWithHints', () => { - test('should dispatch ADD_STEP, and no hints when no hints are applicable (eg pause step)', () => { + it('should dispatch ADD_STEP, and no hints when no hints are applicable (eg pause step)', () => { const stepType = 'pause' const payload = { stepType } addAndSelectStepWithHints(payload)(dispatch, getState) From c0d7aca31dba407b25541e95ff8270044651671b Mon Sep 17 00:00:00 2001 From: IanLondon Date: Tue, 10 Mar 2020 16:22:40 -0400 Subject: [PATCH 08/10] rewrite describe/it text for clarity --- .../__tests__/addAndSelectStepWithHints.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js b/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js index 480d1026773..3aa34e42aae 100644 --- a/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js +++ b/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js @@ -64,10 +64,10 @@ describe('addAndSelectStepWithHints', () => { ]) }) - describe('should dispatch ADD_STEP, and also ADD_HINT "module_without_labware" for module steps if module lacks labware', () => { + describe('ADD_HINT "module_without_labware"', () => { ;[ { - testName: 'magnet step, magnetic module has no labware', + testName: 'magnet step, when magnetic module has no labware', stepType: 'magnet', selectorValues: { getMagnetModuleHasLabware: false, @@ -78,7 +78,7 @@ describe('addAndSelectStepWithHints', () => { }, }, { - testName: 'temperature step, temperature module has no labware', + testName: 'temperature step, when temperature module has no labware', stepType: 'temperature', selectorValues: { getMagnetModuleHasLabware: false, @@ -89,7 +89,7 @@ describe('addAndSelectStepWithHints', () => { }, }, { - testName: 'temperature step, thermocycler has no labware', + testName: 'temperature step, when thermocycler has no labware', stepType: 'temperature', selectorValues: { getMagnetModuleHasLabware: false, @@ -100,7 +100,7 @@ describe('addAndSelectStepWithHints', () => { }, }, ].forEach(({ testName, stepType, selectorValues }) => { - it(testName, () => { + it(`should be dispatched (after ADD_STEP) for ${testName}`, () => { mockGetMagnetModuleHasLabware.mockReturnValue( selectorValues.getMagnetModuleHasLabware ) From fe14f7a31aa532a5e1ee5de62a57756ea74194ff Mon Sep 17 00:00:00 2001 From: IanLondon Date: Tue, 10 Mar 2020 18:58:40 -0400 Subject: [PATCH 09/10] wip: adding selectStep tests --- .../addAndSelectStepWithHints.test.js | 6 +- .../actions/__tests__/selectStep.test.js | 121 ++++++++++++++++++ 2 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 protocol-designer/src/ui/steps/actions/__tests__/selectStep.test.js diff --git a/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js b/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js index 3aa34e42aae..47906d04cdf 100644 --- a/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js +++ b/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.js @@ -41,7 +41,7 @@ beforeEach(() => { }) describe('addAndSelectStepWithHints', () => { - it('should dispatch ADD_STEP, and no hints when no hints are applicable (eg pause step)', () => { + it('should dispatch addStep thunk, and no hints when no hints are applicable (eg pause step)', () => { const stepType = 'pause' const payload = { stepType } addAndSelectStepWithHints(payload)(dispatch, getState) @@ -50,7 +50,7 @@ describe('addAndSelectStepWithHints', () => { expect(dispatch.mock.calls).toEqual([['addStepMockReturnValue']]) }) - it('should dispatch ADD_STEP, and also ADD_HINT "add_liquids_and_labware" if we\'re adding a step that uses liquid but have no liquids on the deck', () => { + it('should dispatch addStep thunk, and also ADD_HINT "add_liquids_and_labware" if we\'re adding a step that uses liquid but have no liquids on the deck', () => { const stepType = 'moveLiquid' const payload = { stepType } mockGetDeckHasLiquid.mockReturnValue(false) // no liquid! @@ -100,7 +100,7 @@ describe('addAndSelectStepWithHints', () => { }, }, ].forEach(({ testName, stepType, selectorValues }) => { - it(`should be dispatched (after ADD_STEP) for ${testName}`, () => { + it(`should be dispatched (after addStep thunk is dispatched) for ${testName}`, () => { mockGetMagnetModuleHasLabware.mockReturnValue( selectorValues.getMagnetModuleHasLabware ) diff --git a/protocol-designer/src/ui/steps/actions/__tests__/selectStep.test.js b/protocol-designer/src/ui/steps/actions/__tests__/selectStep.test.js new file mode 100644 index 00000000000..48e8927be82 --- /dev/null +++ b/protocol-designer/src/ui/steps/actions/__tests__/selectStep.test.js @@ -0,0 +1,121 @@ +// @flow +import fixture_tiprack_10_ul from '@opentrons/shared-data/labware/fixtures/2/fixture_tiprack_10_ul.json' +import { fixtureP10Single } from '@opentrons/shared-data/pipette/fixtures/name' +import configureMockStore from 'redux-mock-store' +import thunk from 'redux-thunk' +import { selectStep } from '../actions' +import { PAUSE_UNTIL_RESUME } from '../../../../constants' +import { selectors as stepFormSelectors } from '../../../../step-forms' +import { selectors as uiModulesSelectors } from '../../../modules' +import type { + InitialDeckSetup, + PipetteEntities, + LabwareEntities, + SavedStepFormState, +} from '../../../../step-forms' +import type { FormData } from '../../../../form-types' +jest.mock('../../../../step-forms') +jest.mock('../../../modules') + +const mock_getStepFormData: JestMockFn<[any, string, string], ?FormData> = + stepFormSelectors._getStepFormData +const mockGetSavedStepForms: JestMockFn<[any], SavedStepFormState> = + stepFormSelectors.getSavedStepForms +const mockGetOrderedStepIds: JestMockFn<[any], Array> = + stepFormSelectors.getOrderedStepIds +const mockGetInitialDeckSetup: JestMockFn<[any], InitialDeckSetup> = + stepFormSelectors.getInitialDeckSetup +const mockGetPipetteEntities: JestMockFn<[any], PipetteEntities> = + stepFormSelectors.getPipetteEntities +const mockGetLabwareEntities: JestMockFn<[any], LabwareEntities> = + stepFormSelectors.getLabwareEntities + +const mockGetSingleMagneticModuleId: JestMockFn<[any], string | null> = + uiModulesSelectors.getSingleMagneticModuleId +const mockGetMagnetLabwareEngageHeight: JestMockFn<[any], number | null> = + uiModulesSelectors.getMagnetLabwareEngageHeight + +let existingStep +beforeEach(() => { + jest.clearAllMocks() + + const pipetteEntity = { + id: 'somePipette', + name: 'p10_single', + tiprackDefURI: 'foo', + tiprackLabwareDef: fixture_tiprack_10_ul, + spec: fixtureP10Single, + } + + existingStep = { + stepType: 'pause', + id: 'existingStepId', + stepName: 'Example pause', + stepDetails: 'details', + pauseForAmountOfTime: PAUSE_UNTIL_RESUME, + } + + mock_getStepFormData.mockReturnValue(null) // NOTE: this should be implemented per-test if it's to be used + mockGetSavedStepForms.mockReturnValue({ + [existingStep.id]: existingStep, + }) + mockGetOrderedStepIds.mockReturnValue(['existingStepId']) + mockGetInitialDeckSetup.mockReturnValue({ + pipettes: { [pipetteEntity.id]: { ...pipetteEntity, mount: 'left' } }, + modules: {}, + labware: {}, + }) + mockGetPipetteEntities.mockReturnValue({ + [pipetteEntity.id]: pipetteEntity, + }) + mockGetLabwareEntities.mockReturnValue({}) + mockGetSingleMagneticModuleId.mockReturnValue(null) + mockGetMagnetLabwareEngageHeight.mockReturnValue(null) +}) + +const middlewares = [thunk] +const mockStore = configureMockStore(middlewares) + +describe('selectStep', () => { + describe('new (never before saved) step', () => { + it('should select a pristine step & populate initial values', () => { + // TODO(IL, 2020-03-10): refactor _getStepFormData so it's not so weird! We might remove it when we remove `legacySteps`?? + const newStepInitialValues = { id: '123', stepType: 'pause' } + mock_getStepFormData.mockReturnValue(newStepInitialValues) + const store = mockStore({}) + + // $FlowFixMe(IL, 2020-03-10): problem dispatching a thunk with mock dispatch + store.dispatch(selectStep('newStepId', 'moveLiquid')) + + expect(store.getActions()).toEqual([ + { type: 'SELECT_STEP', payload: 'newStepId' }, + { type: 'POPULATE_FORM', payload: newStepInitialValues }, + ]) + }) + + it.todo('should set a default pipette if form has a "pipette" field') + it.todo( + 'should set a default magnetic module and engage height for engage step' + ) + it.todo('should set a default magnetic module for disengage step') + it.todo('should set a default temperature module for Temperature step') + }) + + describe('selecting previously-saved step', () => { + it('should restore a saved step', () => { + mock_getStepFormData.mockReturnValue(existingStep) + const store = mockStore({}) + + // $FlowFixMe(IL, 2020-03-10): problem dispatching a thunk with mock dispatch + store.dispatch(selectStep('existingStepId')) + + expect(store.getActions()).toEqual([ + { type: 'SELECT_STEP', payload: 'existingStepId' }, + { + type: 'POPULATE_FORM', + payload: existingStep, + }, + ]) + }) + }) +}) From e27f7b9ebbcd111db932e9a572b2692b7cf025fa Mon Sep 17 00:00:00 2001 From: IanLondon Date: Wed, 11 Mar 2020 10:40:17 -0400 Subject: [PATCH 10/10] add some initial tests for selectStep thunk --- protocol-designer/src/form-types.js | 4 +- .../getNextDefaultMagnetAction/index.js | 6 +- .../actions/__tests__/selectStep.test.js | 234 +++++++++++++----- 3 files changed, 184 insertions(+), 60 deletions(-) diff --git a/protocol-designer/src/form-types.js b/protocol-designer/src/form-types.js index 151c6bc51c4..ac6eb9ac4f2 100644 --- a/protocol-designer/src/form-types.js +++ b/protocol-designer/src/form-types.js @@ -224,13 +224,15 @@ export type HydratedMixFormDataLegacy = { blowout_location: ?string, // labwareId or 'SOURCE_WELL' or 'DEST_WELL' } +export type MagnetAction = 'engage' | 'disengage' + export type HydratedMagnetFormData = {| ...AnnotationFields, id: string, stepType: 'magnet', stepDetails: string | null, moduleId: string | null, - magnetAction: 'engage' | 'disengage', + magnetAction: MagnetAction, engageHeight: string | null, |} diff --git a/protocol-designer/src/steplist/formLevel/getNextDefaultMagnetAction/index.js b/protocol-designer/src/steplist/formLevel/getNextDefaultMagnetAction/index.js index f9432a70b48..3ecd3f89a19 100644 --- a/protocol-designer/src/steplist/formLevel/getNextDefaultMagnetAction/index.js +++ b/protocol-designer/src/steplist/formLevel/getNextDefaultMagnetAction/index.js @@ -1,11 +1,11 @@ // @flow import last from 'lodash/last' -import type { StepIdType, FormData } from '../../../form-types' +import type { StepIdType, FormData, MagnetAction } from '../../../form-types' export function getNextDefaultMagnetAction( savedForms: { [StepIdType]: FormData }, orderedStepIds: Array -): ?string { +): MagnetAction { const prevMagnetSteps = orderedStepIds .map(stepId => savedForms[stepId]) .filter(form => form && form.magnetAction) @@ -14,7 +14,7 @@ export function getNextDefaultMagnetAction( // default the first magnet step to engage so that // recommended engage height can auto populate - let nextDefaultMagnetAction: ?string = 'engage' + let nextDefaultMagnetAction: MagnetAction = 'engage' if (lastMagnetStep && lastMagnetStep.magnetAction) { nextDefaultMagnetAction = diff --git a/protocol-designer/src/ui/steps/actions/__tests__/selectStep.test.js b/protocol-designer/src/ui/steps/actions/__tests__/selectStep.test.js index 48e8927be82..7ec5006fdaf 100644 --- a/protocol-designer/src/ui/steps/actions/__tests__/selectStep.test.js +++ b/protocol-designer/src/ui/steps/actions/__tests__/selectStep.test.js @@ -1,76 +1,74 @@ // @flow -import fixture_tiprack_10_ul from '@opentrons/shared-data/labware/fixtures/2/fixture_tiprack_10_ul.json' -import { fixtureP10Single } from '@opentrons/shared-data/pipette/fixtures/name' +// TODO(IL, 2020-03-10): refactor the code covered here to make it more testable, probably by factoring out +// smaller fns from selectStep. import configureMockStore from 'redux-mock-store' import thunk from 'redux-thunk' import { selectStep } from '../actions' import { PAUSE_UNTIL_RESUME } from '../../../../constants' import { selectors as stepFormSelectors } from '../../../../step-forms' import { selectors as uiModulesSelectors } from '../../../modules' -import type { - InitialDeckSetup, - PipetteEntities, - LabwareEntities, - SavedStepFormState, -} from '../../../../step-forms' -import type { FormData } from '../../../../form-types' +import { + getNextDefaultMagnetAction, + getNextDefaultTemperatureModuleId, + getNextDefaultPipetteId, + handleFormChange, +} from '../../../../steplist/formLevel' +import type { FormPatch } from '../../../../steplist/actions' +import type { FormData, MagnetAction } from '../../../../form-types' jest.mock('../../../../step-forms') jest.mock('../../../modules') +jest.mock('../../../../steplist/formLevel') const mock_getStepFormData: JestMockFn<[any, string, string], ?FormData> = stepFormSelectors._getStepFormData -const mockGetSavedStepForms: JestMockFn<[any], SavedStepFormState> = +const mockGetSavedStepForms: JestMockFn<[any], any> = stepFormSelectors.getSavedStepForms -const mockGetOrderedStepIds: JestMockFn<[any], Array> = +const mockGetOrderedStepIds: JestMockFn<[any], any> = stepFormSelectors.getOrderedStepIds -const mockGetInitialDeckSetup: JestMockFn<[any], InitialDeckSetup> = +const mockGetInitialDeckSetup: JestMockFn<[any], any> = stepFormSelectors.getInitialDeckSetup -const mockGetPipetteEntities: JestMockFn<[any], PipetteEntities> = +const mockGetPipetteEntities: JestMockFn<[any], any> = stepFormSelectors.getPipetteEntities -const mockGetLabwareEntities: JestMockFn<[any], LabwareEntities> = +const mockGetLabwareEntities: JestMockFn<[any], any> = stepFormSelectors.getLabwareEntities +const mockGetNextDefaultPipetteId: JestMockFn< + [any, any, any], + string | null +> = getNextDefaultPipetteId +const mockGetNextDefaultTemperatureModuleId: JestMockFn< + [any, any, any], + string | null +> = getNextDefaultTemperatureModuleId const mockGetSingleMagneticModuleId: JestMockFn<[any], string | null> = uiModulesSelectors.getSingleMagneticModuleId const mockGetMagnetLabwareEngageHeight: JestMockFn<[any], number | null> = uiModulesSelectors.getMagnetLabwareEngageHeight +const mockGetNextDefaultMagnetAction: JestMockFn< + [any, any], + MagnetAction +> = getNextDefaultMagnetAction +const mockHandleFormChange: JestMockFn = handleFormChange -let existingStep beforeEach(() => { jest.clearAllMocks() - const pipetteEntity = { - id: 'somePipette', - name: 'p10_single', - tiprackDefURI: 'foo', - tiprackLabwareDef: fixture_tiprack_10_ul, - spec: fixtureP10Single, - } - - existingStep = { - stepType: 'pause', - id: 'existingStepId', - stepName: 'Example pause', - stepDetails: 'details', - pauseForAmountOfTime: PAUSE_UNTIL_RESUME, - } - - mock_getStepFormData.mockReturnValue(null) // NOTE: this should be implemented per-test if it's to be used - mockGetSavedStepForms.mockReturnValue({ - [existingStep.id]: existingStep, - }) - mockGetOrderedStepIds.mockReturnValue(['existingStepId']) - mockGetInitialDeckSetup.mockReturnValue({ - pipettes: { [pipetteEntity.id]: { ...pipetteEntity, mount: 'left' } }, - modules: {}, - labware: {}, - }) - mockGetPipetteEntities.mockReturnValue({ - [pipetteEntity.id]: pipetteEntity, - }) - mockGetLabwareEntities.mockReturnValue({}) + mock_getStepFormData.mockReturnValue(null) + // NOTE: selectStep doesn't use the results of these selectors directly, + // it just passes their results into steplist/formLevel fns that we also mock + mockGetSavedStepForms.mockReturnValue('getSaveStepFormsMockReturn') + mockGetOrderedStepIds.mockReturnValue('getOrderedStepIdsMockReturn') + mockGetInitialDeckSetup.mockReturnValue('getInitialDeckSetupMockReturn') + + mockGetPipetteEntities.mockReturnValue('pipetteEntityMockReturn') + mockGetLabwareEntities.mockReturnValue('labwareEntityMockReturn') + mockGetNextDefaultPipetteId.mockReturnValue(null) mockGetSingleMagneticModuleId.mockReturnValue(null) mockGetMagnetLabwareEngageHeight.mockReturnValue(null) + + mockHandleFormChange.mockReturnValue({ + handleFormChangePatch: 'mock', + }) }) const middlewares = [thunk] @@ -78,31 +76,155 @@ const mockStore = configureMockStore(middlewares) describe('selectStep', () => { describe('new (never before saved) step', () => { - it('should select a pristine step & populate initial values', () => { - // TODO(IL, 2020-03-10): refactor _getStepFormData so it's not so weird! We might remove it when we remove `legacySteps`?? - const newStepInitialValues = { id: '123', stepType: 'pause' } - mock_getStepFormData.mockReturnValue(newStepInitialValues) + it('should select a pristine pause step & populate initial values', () => { + const mockedInitialFormData = { id: '123', stepType: 'pause' } + mock_getStepFormData.mockReturnValue(mockedInitialFormData) + const store = mockStore({}) + + // $FlowFixMe(IL, 2020-03-10): problem dispatching a thunk with mock dispatch + store.dispatch(selectStep('newStepId', 'pause')) + + expect(store.getActions()).toEqual([ + { type: 'SELECT_STEP', payload: 'newStepId' }, + { type: 'POPULATE_FORM', payload: mockedInitialFormData }, + ]) + }) + + it('should call handleFormChange with a default pipette for "moveLiquid" step', () => { + const mockedInitialFormData = { + id: 'newStepId', + stepType: 'moveLiquid', + pipette: null, + } + + mockHandleFormChange.mockReturnValue({ + pipette: 'somePipette', + }) + mockGetNextDefaultPipetteId.mockReturnValue('somePipette') + mock_getStepFormData.mockReturnValue(mockedInitialFormData) const store = mockStore({}) // $FlowFixMe(IL, 2020-03-10): problem dispatching a thunk with mock dispatch store.dispatch(selectStep('newStepId', 'moveLiquid')) + expect(mockHandleFormChange.mock.calls).toEqual([ + [ + { pipette: 'somePipette' }, + mockedInitialFormData, + 'pipetteEntityMockReturn', + 'labwareEntityMockReturn', + ], + ]) + expect(store.getActions()).toEqual([ { type: 'SELECT_STEP', payload: 'newStepId' }, - { type: 'POPULATE_FORM', payload: newStepInitialValues }, + { + type: 'POPULATE_FORM', + payload: { + ...mockedInitialFormData, + pipette: 'somePipette', + }, + }, ]) }) - it.todo('should set a default pipette if form has a "pipette" field') - it.todo( - 'should set a default magnetic module and engage height for engage step' - ) - it.todo('should set a default magnetic module for disengage step') - it.todo('should set a default temperature module for Temperature step') + it('should set a default magnetic module for magnet step, and set engage height and magnetAction=engage, for magnet > engage', () => { + mockGetSingleMagneticModuleId.mockReturnValue('someMagneticModuleId') + mockGetNextDefaultMagnetAction.mockReturnValue('engage') + mockGetMagnetLabwareEngageHeight.mockReturnValue(12) + + const mockedInitialFormData = { + id: 'newStepId', + stepType: 'magnet', + moduleId: null, + } + mock_getStepFormData.mockReturnValue(mockedInitialFormData) + const store = mockStore({}) + + // $FlowFixMe(IL, 2020-03-10): problem dispatching a thunk with mock dispatch + store.dispatch(selectStep('newStepId', 'magnet')) + + expect(store.getActions()).toEqual([ + { type: 'SELECT_STEP', payload: 'newStepId' }, + { + type: 'POPULATE_FORM', + payload: { + ...mockedInitialFormData, + moduleId: 'someMagneticModuleId', + engageHeight: '12', + magnetAction: 'engage', + }, + }, + ]) + }) + + it('should set a default magnetic module for magnet step, and set magnetAction=disengage, for magnet > disengage', () => { + mockGetSingleMagneticModuleId.mockReturnValue('someMagneticModuleId') + mockGetNextDefaultMagnetAction.mockReturnValue('disengage') + + const mockedInitialFormData = { + id: 'newStepId', + stepType: 'magnet', + moduleId: null, + } + mock_getStepFormData.mockReturnValue(mockedInitialFormData) + const store = mockStore({}) + + // $FlowFixMe(IL, 2020-03-10): problem dispatching a thunk with mock dispatch + store.dispatch(selectStep('newStepId', 'magnet')) + + expect(store.getActions()).toEqual([ + { type: 'SELECT_STEP', payload: 'newStepId' }, + { + type: 'POPULATE_FORM', + payload: { + ...mockedInitialFormData, + moduleId: 'someMagneticModuleId', + engageHeight: null, + magnetAction: 'disengage', + }, + }, + ]) + }) + + it('should set a default temperature module for Temperature step', () => { + mockGetNextDefaultTemperatureModuleId.mockReturnValue( + 'someTemperatureModuleId' + ) + + const mockedInitialFormData = { + id: 'newStepId', + stepType: 'temperature', + moduleId: null, + } + mock_getStepFormData.mockReturnValue(mockedInitialFormData) + const store = mockStore({}) + + // $FlowFixMe(IL, 2020-03-10): problem dispatching a thunk with mock dispatch + store.dispatch(selectStep('newStepId', 'temperature')) + + expect(store.getActions()).toEqual([ + { type: 'SELECT_STEP', payload: 'newStepId' }, + { + type: 'POPULATE_FORM', + payload: { + ...mockedInitialFormData, + moduleId: 'someTemperatureModuleId', + }, + }, + ]) + }) }) describe('selecting previously-saved step', () => { it('should restore a saved step', () => { + const existingStep = { + stepType: 'pause', + id: 'existingStepId', + stepName: 'Example pause', + stepDetails: 'details', + pauseForAmountOfTime: PAUSE_UNTIL_RESUME, + } mock_getStepFormData.mockReturnValue(existingStep) const store = mockStore({})