fix: Opentrons ai client create protocol fixes (#16802)

# Overview This PR fixes many defects opened by the Opentrons team and refactors some code to remove duplication defects [AUTH-1031](https://opentrons.atlassian.net/browse/AUTH-1031), [AUTH-1032](https://opentrons.atlassian.net/browse/AUTH-1032), [AUTH-1033](https://opentrons.atlassian.net/browse/AUTH-1033), [AUTH-1034](https://opentrons.atlassian.net/browse/AUTH-1034), [AUTH-1035](https://opentrons.atlassian.net/browse/AUTH-1035), [AUTH-1040](https://opentrons.atlassian.net/browse/AUTH-1040), [AUTH-1042](https://opentrons.atlassian.net/browse/AUTH-1042) ## Test Plan and Hands on Testing Retested manually ## Risk assessment low risk [AUTH-1031]: https://opentrons.atlassian.net/browse/AUTH-1031?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [AUTH-1032]: https://opentrons.atlassian.net/browse/AUTH-1032?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [AUTH-1033]: https://opentrons.atlassian.net/browse/AUTH-1033?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [AUTH-1034]: https://opentrons.atlassian.net/browse/AUTH-1034?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [AUTH-1035]: https://opentrons.atlassian.net/browse/AUTH-1035?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [AUTH-1040]: https://opentrons.atlassian.net/browse/AUTH-1040?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [AUTH-1042]: https://opentrons.atlassian.net/browse/AUTH-1042?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Opentrons · Nov 14, 2024 · 72178ca · 72178ca
1 parent 1b67a37
commit 72178ca
Show file tree

Hide file tree

Showing 20 changed files with 157 additions and 284 deletions.
diff --git a/opentrons-ai-client/src/assets/localization/en/create_protocol.json b/opentrons-ai-client/src/assets/localization/en/create_protocol.json
@@ -17,7 +17,7 @@
   "opentrons_ot2_label": "Opentrons OT-2",
   "opentrons_ot2": "Opentrons OT-2",
   "instruments_pipettes_title": "What pipettes would you like to use?",
-  "two_pipettes_label": "Two pipettes",
+  "two_pipettes_label": "1-Channel or 8-Channel pipettes",
   "right_pipette_label": "Right mount",
   "left_pipette_label": "Left mount",
   "choose_pipette_placeholder": "Choose pipette",

diff --git a/opentrons-ai-client/src/molecules/ControlledAddTextAreaFields/index.tsx b/opentrons-ai-client/src/molecules/ControlledAddTextAreaFields/index.tsx
@@ -63,7 +63,13 @@ export function ControlledAddTextAreaFields({
                   <Link
                     role="button"
                     onClick={() => {
-                      field.onChange(values.filter((_, i) => i !== index))
+                      const newValues = values
+                        .filter((_, i) => i !== index)
+                        .map(
+                          (value, i) =>
+                            `${t(name)} ${i + 1}: ${value.split(': ')[1]}`
+                        )
+                      field.onChange(newValues)
                     }}
                     css={css`
                       justify-content: flex-end;

diff --git a/opentrons-ai-client/src/molecules/ExitConfirmModal/index.tsx b/opentrons-ai-client/src/molecules/ExitConfirmModal/index.tsx
@@ -37,7 +37,10 @@ export function ExitConfirmModal(): JSX.Element {
   return (
     <Modal type="info" title={t('exit_confirmation_title')} marginLeft="0">
       <Flex flexDirection={DIRECTION_COLUMN}>
-        <StyledText padding={`${SPACING.spacing24} 0`}>
+        <StyledText
+          paddingTop={`${SPACING.spacing8}`}
+          paddingBottom={`${SPACING.spacing24}`}
+        >
           {t('exit_confirmation_body')}
         </StyledText>
         <Flex justifyContent={JUSTIFY_FLEX_END} gap={SPACING.spacing8}>

diff --git a/opentrons-ai-client/src/organisms/ApplicationSection/__tests__/ApplicationSection.test.tsx b/opentrons-ai-client/src/organisms/ApplicationSection/__tests__/ApplicationSection.test.tsx
@@ -13,6 +13,8 @@ const TestFormProviderComponent = () => {
   return (
     <FormProvider {...methods}>
       <ApplicationSection />
+
+      <p>{`form is ${methods.formState.isValid ? 'valid' : 'invalid'}`}</p>
     </FormProvider>
   )
 }
@@ -33,7 +35,6 @@ describe('ApplicationSection', () => {
     expect(
       screen.getByText('Describe what you are trying to do')
     ).toBeInTheDocument()
-    expect(screen.getByText('Confirm')).toBeInTheDocument()
   })
 
   it('should not render other application dropdown if Other option is not selected', () => {
@@ -54,10 +55,10 @@ describe('ApplicationSection', () => {
     expect(screen.getByText('Other application')).toBeInTheDocument()
   })
 
-  it('should enable confirm button when all fields are filled', async () => {
+  it('should update the form state to valid when all fields are filled', async () => {
     render()
 
-    expect(screen.getByRole('button', { name: 'Confirm' })).toBeDisabled()
+    expect(screen.getByText('form is invalid')).toBeInTheDocument()
 
     const applicationDropdown = screen.getByText('Select an option')
     fireEvent.click(applicationDropdown)
@@ -69,14 +70,13 @@ describe('ApplicationSection', () => {
     fireEvent.change(describeInput, { target: { value: 'Test description' } })
 
     await waitFor(() => {
-      expect(screen.getByRole('button', { name: 'Confirm' })).toBeEnabled()
+      expect(screen.getByText('form is valid')).toBeInTheDocument()
     })
   })
 
-  it('should disable confirm button when all fields are not filled', () => {
+  it('should update the form state to invalid when not all fields are filled', () => {
     render()
 
-    const confirmButton = screen.getByRole('button')
-    expect(confirmButton).toBeDisabled()
+    expect(screen.getByText('form is invalid')).toBeInTheDocument()
   })
 })
diff --git a/opentrons-ai-client/src/organisms/ApplicationSection/index.tsx b/opentrons-ai-client/src/organisms/ApplicationSection/index.tsx
@@ -1,19 +1,8 @@
-import {
-  DIRECTION_COLUMN,
-  DISPLAY_FLEX,
-  Flex,
-  JUSTIFY_FLEX_END,
-  LargeButton,
-  SPACING,
-} from '@opentrons/components'
+import { DIRECTION_COLUMN, Flex, SPACING } from '@opentrons/components'
 import { useFormContext } from 'react-hook-form'
 import { useTranslation } from 'react-i18next'
-import styled from 'styled-components'
 import { ControlledDropdownMenu } from '../../atoms/ControlledDropdownMenu'
 import { ControlledInputField } from '../../atoms/ControlledInputField'
-import { useAtom } from 'jotai'
-import { createProtocolAtom } from '../../resources/atoms'
-import { APPLICATION_STEP } from '../ProtocolSectionsContainer'
 
 export const BASIC_ALIQUOTING = 'basic_aliquoting'
 export const PCR = 'pcr'
@@ -25,11 +14,7 @@ export const APPLICATION_DESCRIBE = 'application.description'
 
 export function ApplicationSection(): JSX.Element | null {
   const { t } = useTranslation('create_protocol')
-  const {
-    watch,
-    formState: { isValid },
-  } = useFormContext()
-  const [{ currentStep }, setCreateProtocolAtom] = useAtom(createProtocolAtom)
+  const { watch } = useFormContext()
 
   const options = [
     { name: t(BASIC_ALIQUOTING), value: BASIC_ALIQUOTING },
@@ -39,16 +24,6 @@ export function ApplicationSection(): JSX.Element | null {
 
   const isOtherSelected = watch(APPLICATION_SCIENTIFIC_APPLICATION) === OTHER
 
-  function handleConfirmButtonClick(): void {
-    const step =
-      currentStep > APPLICATION_STEP ? currentStep : APPLICATION_STEP + 1
-
-    setCreateProtocolAtom({
-      currentStep: step,
-      focusStep: step,
-    })
-  }
-
   return (
     <Flex
       flexDirection={DIRECTION_COLUMN}
@@ -80,19 +55,6 @@ export function ApplicationSection(): JSX.Element | null {
         caption={t('application_describe_caption')}
         rules={{ required: true, minLength: 3 }}
       />
-
-      <ButtonContainer>
-        <LargeButton
-          onClick={handleConfirmButtonClick}
-          disabled={!isValid}
-          buttonText={t('section_confirm_button')}
-        ></LargeButton>
-      </ButtonContainer>
     </Flex>
   )
 }
-
-const ButtonContainer = styled.div`
-  display: ${DISPLAY_FLEX};
-  justify-content: ${JUSTIFY_FLEX_END};
-`
diff --git a/opentrons-ai-client/src/organisms/InstrumentsSection/__tests__/InstrumentsSection.test.tsx b/opentrons-ai-client/src/organisms/InstrumentsSection/__tests__/InstrumentsSection.test.tsx
@@ -13,6 +13,8 @@ const TestFormProviderComponent = () => {
   return (
     <FormProvider {...methods}>
       <InstrumentsSection />
+
+      <p>{`form is ${methods.formState.isValid ? 'valid' : 'invalid'}`}</p>
     </FormProvider>
   )
 }
@@ -24,7 +26,7 @@ const render = (): ReturnType<typeof renderWithProviders> => {
 }
 
 describe('ApplicationSection', () => {
-  it('should render robot, pipette, flex gripper radios, mounts dropdowns, and confirm button', async () => {
+  it('should render robot, pipette, flex gripper radios and mounts dropdowns', async () => {
     render()
 
     expect(
@@ -40,7 +42,6 @@ describe('ApplicationSection', () => {
     expect(
       screen.getByText('Do you want to use the Flex Gripper?')
     ).toBeInTheDocument()
-    expect(screen.getByText('Confirm')).toBeInTheDocument()
   })
 
   it('should not render left and right mount dropdowns if 96-Channel 1000µL pipette radio is selected', () => {
@@ -80,7 +81,7 @@ describe('ApplicationSection', () => {
     render()
 
     await waitFor(() => {
-      expect(screen.getByRole('button', { name: 'Confirm' })).toBeDisabled()
+      expect(screen.getByText('form is invalid')).toBeInTheDocument()
     })
 
     const leftMount = screen.getAllByText('Choose pipette')[0]
@@ -96,14 +97,14 @@ describe('ApplicationSection', () => {
     })
     expect(screen.getByText('None')).toBeInTheDocument()
 
-    expect(screen.getByRole('button', { name: 'Confirm' })).toBeEnabled()
+    expect(screen.getByText('form is valid')).toBeInTheDocument()
   })
 
   it('should not be able to select two pipettes with none value', async () => {
     render()
 
     await waitFor(() => {
-      expect(screen.getByRole('button', { name: 'Confirm' })).toBeDisabled()
+      expect(screen.getByText('form is invalid')).toBeInTheDocument()
     })
 
     const leftMount = screen.getAllByText('Choose pipette')[0]
@@ -115,15 +116,15 @@ describe('ApplicationSection', () => {
     fireEvent.click(screen.getAllByText('None')[1])
 
     await waitFor(() => {
-      expect(screen.getByRole('button', { name: 'Confirm' })).toBeDisabled()
+      expect(screen.getByText('form is invalid')).toBeInTheDocument()
     })
   })
 
-  it('should enable confirm button when all fields are filled', async () => {
+  it('should update the form state to valid when all fields are filled', async () => {
     render()
 
     await waitFor(() => {
-      expect(screen.getByRole('button', { name: 'Confirm' })).toBeDisabled()
+      expect(screen.getByText('form is invalid')).toBeInTheDocument()
     })
 
     const leftMount = screen.getAllByText('Choose pipette')[0]
@@ -135,16 +136,7 @@ describe('ApplicationSection', () => {
     fireEvent.click(screen.getByText('Flex 8-Channel 50 μL'))
 
     await waitFor(() => {
-      expect(screen.getByRole('button', { name: 'Confirm' })).toBeEnabled()
-    })
-  })
-
-  it('should disable confirm button when all fields are not filled', async () => {
-    render()
-
-    const confirmButton = screen.getByRole('button')
-    await waitFor(() => {
-      expect(confirmButton).not.toBeEnabled()
+      expect(screen.getByText('form is valid')).toBeInTheDocument()
     })
   })
 })
diff --git a/opentrons-ai-client/src/organisms/InstrumentsSection/index.tsx b/opentrons-ai-client/src/organisms/InstrumentsSection/index.tsx
@@ -1,27 +1,20 @@
 import {
   COLORS,
   DIRECTION_COLUMN,
-  DISPLAY_FLEX,
   Flex,
-  JUSTIFY_FLEX_END,
-  LargeButton,
   SPACING,
   StyledText,
 } from '@opentrons/components'
 import { useFormContext } from 'react-hook-form'
 import { useTranslation } from 'react-i18next'
 import styled from 'styled-components'
-import { useAtom } from 'jotai'
-import { createProtocolAtom } from '../../resources/atoms'
-import { INSTRUMENTS_STEP } from '../ProtocolSectionsContainer'
 import { ControlledDropdownMenu } from '../../atoms/ControlledDropdownMenu'
 import { ControlledRadioButtonGroup } from '../../molecules/ControlledRadioButtonGroup'
 import { useMemo } from 'react'
 import {
   getAllPipetteNames,
   getPipetteSpecsV2,
   OT2_PIPETTES,
-  OT2_ROBOT_TYPE,
   OT3_PIPETTES,
 } from '@opentrons/shared-data'
 
@@ -40,11 +33,7 @@ export const NO_PIPETTES = 'none'
 
 export function InstrumentsSection(): JSX.Element | null {
   const { t } = useTranslation('create_protocol')
-  const {
-    formState: { isValid },
-    watch,
-  } = useFormContext()
-  const [{ currentStep }, setCreateProtocolAtom] = useAtom(createProtocolAtom)
+  const { watch } = useFormContext()
   const robotType = watch(ROBOT_FIELD_NAME)
   const isOtherPipettesSelected = watch(PIPETTES_FIELD_NAME) === TWO_PIPETTES
   const isOpentronsOT2Selected = robotType === OPENTRONS_OT2
@@ -91,7 +80,7 @@ export function InstrumentsSection(): JSX.Element | null {
   const pipetteOptions = useMemo(() => {
     const allPipetteOptions = getAllPipetteNames('maxVolume', 'channels')
       .filter(name =>
-        (robotType === OT2_ROBOT_TYPE ? OT2_PIPETTES : OT3_PIPETTES).includes(
+        (robotType === OPENTRONS_OT2 ? OT2_PIPETTES : OT3_PIPETTES).includes(
           name
         )
       )
@@ -103,16 +92,6 @@ export function InstrumentsSection(): JSX.Element | null {
     return [{ name: t('none'), value: NO_PIPETTES }, ...allPipetteOptions]
   }, [robotType])
 
-  function handleConfirmButtonClick(): void {
-    const step =
-      currentStep > INSTRUMENTS_STEP ? currentStep : INSTRUMENTS_STEP + 1
-
-    setCreateProtocolAtom({
-      currentStep: step,
-      focusStep: step,
-    })
-  }
-
   return (
     <Flex
       flexDirection={DIRECTION_COLUMN}
@@ -186,23 +165,10 @@ export function InstrumentsSection(): JSX.Element | null {
           rules={{ required: true }}
         />
       )}
-
-      <ButtonContainer>
-        <LargeButton
-          onClick={handleConfirmButtonClick}
-          disabled={!isValid}
-          buttonText={t('section_confirm_button')}
-        ></LargeButton>
-      </ButtonContainer>
     </Flex>
   )
 }
 
-const ButtonContainer = styled.div`
-  display: ${DISPLAY_FLEX};
-  justify-content: ${JUSTIFY_FLEX_END};
-`
-
 const PipettesDropdown = styled.div<{ isOpentronsOT2Selected?: boolean }>`
   display: flex;
   flex-direction: column;

diff --git a/...ns-ai-client/src/organisms/LabwareLiquidsSection/__tests__/LabwareLiquidsSection.test.tsx b/...ns-ai-client/src/organisms/LabwareLiquidsSection/__tests__/LabwareLiquidsSection.test.tsx
@@ -1,4 +1,4 @@
-import { fireEvent, screen } from '@testing-library/react'
+import { fireEvent, screen, waitFor } from '@testing-library/react'
 import { describe, it, expect } from 'vitest'
 import { renderWithProviders } from '../../../__testing-utils__'
 import { i18n } from '../../../i18n'
@@ -15,6 +15,8 @@ const TestFormProviderComponent = () => {
   return (
     <FormProvider {...methods}>
       <LabwareLiquidsSection />
+
+      <p>{`form is ${methods.formState.isValid ? 'valid' : 'invalid'}`}</p>
     </FormProvider>
   )
 }
@@ -31,7 +33,6 @@ describe('LabwareLiquidsSection', () => {
 
     expect(screen.getByText('Add Opentrons labware')).toBeInTheDocument()
     expect(screen.getByText('No labware added yet')).toBeInTheDocument()
-    expect(screen.getByText('Confirm')).toBeInTheDocument()
   })
 
   it('should not display the no labware added message if labwares have been added', async () => {
@@ -51,20 +52,27 @@ describe('LabwareLiquidsSection', () => {
     expect(screen.queryByText('No labware added yet')).not.toBeInTheDocument()
   })
 
-  //   it('should enable the confirm button when labwares have been added', async () => {
-  //     render()
+  it('should update form state to valid when labwares and liquids have been added', async () => {
+    render()
 
-  //     expect(screen.getByText('Confirm')).toBeDisabled()
+    await waitFor(() => {
+      expect(screen.getByText('form is invalid')).toBeInTheDocument()
+    })
+    const addButton = screen.getByText('Add Opentrons labware')
+    fireEvent.click(addButton)
 
-  //     const addButton = screen.getByText('Add Opentrons labware')
-  //     fireEvent.click(addButton)
+    fireEvent.click(screen.getByText('Tip rack'))
+    fireEvent.click(
+      await screen.findByText('Opentrons Flex 96 Tip Rack 1000 µL')
+    )
+    fireEvent.click(screen.getByText('Save'))
 
-  //     fireEvent.click(screen.getByText('Tip rack'))
-  //     fireEvent.click(
-  //       await screen.findByText('Opentrons Flex 96 Tip Rack 1000 µL')
-  //     )
-  //     fireEvent.click(screen.getByText('Save'))
+    fireEvent.change(screen.getByRole('textbox'), {
+      target: { value: 'test liquid' },
+    })
 
-  //     expect(screen.getByText('Confirm')).toBeEnabled()
-  //   })
+    await waitFor(() => {
+      expect(screen.getByText('form is valid')).toBeInTheDocument()
+    })
+  })
 })