Skip to content

Commit

Permalink
refactor(app): remove RTP feature flag (#14837)
Browse files Browse the repository at this point in the history
* refactor(app): remove RTP feature flag
  • Loading branch information
koji authored Apr 8, 2024
1 parent 1a5052c commit e620a8c
Show file tree
Hide file tree
Showing 13 changed files with 73 additions and 69 deletions.
1 change: 0 additions & 1 deletion app/src/assets/localization/en/app_settings.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
{
"__dev_internal__protocolStats": "Protocol Stats",
"__dev_internal__enableRunTimeParameters": "Enable Run Time Parameters",
"__dev_internal__enableRunNotes": "Display Notes During a Protocol Run",
"__dev_internal__enableQuickTransfer": "Enable Quick Transfer",
"add_folder_button": "Add labware source folder",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ describe('ChooseProtocolSlideout', () => {
screen.getByText(/choose protocol to run/i)
screen.getByText(/opentrons-robot-name/i)
})

it('renders an available protocol option for every stored protocol if any', () => {
render({
robot: mockConnectableRobot,
Expand All @@ -70,6 +71,7 @@ describe('ChooseProtocolSlideout', () => {
screen.queryByRole('heading', { name: 'No protocols found' })
).toBeNull()
})

it('renders an empty state if no protocol options', () => {
vi.mocked(getStoredProtocols).mockReturnValue([])
render({
Expand All @@ -83,22 +85,55 @@ describe('ChooseProtocolSlideout', () => {
screen.getByRole('heading', { name: 'No protocols found' })
).toBeInTheDocument()
})
it('calls createRunFromProtocolSource if CTA clicked', () => {

// it('calls createRunFromProtocolSource if CTA clicked', () => {
// const protocolDataWithoutRunTimeParameter = {
// ...storedProtocolDataFixture,
// runTimeParameters: [],
// }
// vi.mocked(getStoredProtocols).mockReturnValue([
// protocolDataWithoutRunTimeParameter,
// ])
// render({
// robot: mockConnectableRobot,
// onCloseClick: vi.fn(),
// showSlideout: true,
// })
// const proceedButton = screen.getByRole('button', {
// name: 'Proceed to setup',
// })
// fireEvent.click(proceedButton)
// expect(mockCreateRunFromProtocol).toHaveBeenCalledWith({
// files: [expect.any(File)],
// protocolKey: storedProtocolDataFixture.protocolKey,
// })
// expect(mockTrackCreateProtocolRunEvent).toHaveBeenCalled()
// })

it('move to the second slideout if CTA clicked', () => {
const protocolDataWithoutRunTimeParameter = {
...storedProtocolDataFixture,
runTimeParameters: [],
}
vi.mocked(getStoredProtocols).mockReturnValue([
protocolDataWithoutRunTimeParameter,
])
render({
robot: mockConnectableRobot,
onCloseClick: vi.fn(),
showSlideout: true,
})
const proceedButton = screen.getByRole('button', {
name: 'Proceed to setup',
name: 'Continue to parameters',
})
fireEvent.click(proceedButton)
expect(mockCreateRunFromProtocol).toHaveBeenCalledWith({
files: [expect.any(File)],
protocolKey: storedProtocolDataFixture.protocolKey,
})
expect(mockTrackCreateProtocolRunEvent).toHaveBeenCalled()
screen.getByText('Step 2 / 2')
screen.getByText('number of samples')
screen.getByText('Restore default values')
})

// ToDo (kk:04/08) update test for RTP
/*
it('renders error state when there is a run creation error', () => {
vi.mocked(useCreateRunFromProtocol).mockReturnValue({
runCreationError: 'run creation error',
Expand Down Expand Up @@ -153,4 +188,5 @@ describe('ChooseProtocolSlideout', () => {
fireEvent.click(link)
expect(link.getAttribute('href')).toEqual('/devices/opentrons-robot-name')
})
*/
})
11 changes: 5 additions & 6 deletions app/src/organisms/ChooseProtocolSlideout/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as React from 'react'
import first from 'lodash/first'
import { Trans, useTranslation } from 'react-i18next'
import { Link, NavLink, useHistory } from 'react-router-dom'
import { ApiHostProvider } from '@opentrons/react-api-client'
import { useSelector } from 'react-redux'
import { css } from 'styled-components'

Expand All @@ -14,7 +13,6 @@ import {
DIRECTION_COLUMN,
DIRECTION_ROW,
DISPLAY_BLOCK,
DropdownOption,
Flex,
Icon,
Link as LinkComponent,
Expand All @@ -30,12 +28,12 @@ import {
TYPOGRAPHY,
useHoverTooltip,
} from '@opentrons/components'
import { ApiHostProvider } from '@opentrons/react-api-client'

import { useLogger } from '../../logger'
import { OPENTRONS_USB } from '../../redux/discovery'
import { getStoredProtocols } from '../../redux/protocol-storage'
import { appShellRequestor } from '../../redux/shell/remote'
import { useFeatureFlag } from '../../redux/config'
import { MultiSlideout } from '../../atoms/Slideout/MultiSlideout'
import { Tooltip } from '../../atoms/Tooltip'
import { ToggleButton } from '../../atoms/buttons'
Expand All @@ -47,8 +45,10 @@ import { useCreateRunFromProtocol } from '../ChooseRobotToRunProtocolSlideout/us
import { ApplyHistoricOffsets } from '../ApplyHistoricOffsets'
import { useOffsetCandidatesForAnalysis } from '../ApplyHistoricOffsets/hooks/useOffsetCandidatesForAnalysis'
import { getAnalysisStatus } from '../ProtocolsLanding/utils'

import type { RunTimeParameterCreateData } from '@opentrons/api-client'
import type { RunTimeParameter } from '@opentrons/shared-data'
import type { DropdownOption } from '@opentrons/components'
import type { Robot } from '../../redux/discovery/types'
import type { StoredProtocolData } from '../../redux/protocol-storage'
import type { State } from '../../redux/types'
Expand Down Expand Up @@ -93,7 +93,6 @@ export function ChooseProtocolSlideoutComponent(
] = React.useState<RunTimeParameter[]>([])
const [currentPage, setCurrentPage] = React.useState<number>(1)
const [hasParamError, setHasParamError] = React.useState<boolean>(false)
const enableRunTimeParametersFF = useFeatureFlag('enableRunTimeParameters')

React.useEffect(() => {
setRunTimeParametersOverrides(
Expand All @@ -106,9 +105,9 @@ export function ChooseProtocolSlideoutComponent(

const runTimeParametersFromAnalysis =
selectedProtocol?.mostRecentAnalysis?.runTimeParameters ?? []
console.log('runTimeParametersFromAnalysis', runTimeParametersFromAnalysis)

const hasRunTimeParameters =
enableRunTimeParametersFF && runTimeParametersFromAnalysis.length > 0
const hasRunTimeParameters = runTimeParametersFromAnalysis.length > 0

const analysisStatus = getAnalysisStatus(
false,
Expand Down
4 changes: 1 addition & 3 deletions app/src/organisms/ChooseRobotSlideout/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ import type { SlideoutProps } from '../../atoms/Slideout'
import type { UseCreateRun } from '../../organisms/ChooseRobotToRunProtocolSlideout/useCreateRunFromProtocol'
import type { State, Dispatch } from '../../redux/types'
import type { Robot } from '../../redux/discovery/types'
import { useFeatureFlag } from '../../redux/config'
import type { DropdownOption } from '../../atoms/MenuList/DropdownMenu'

export const CARD_OUTLINE_BORDER_STYLE = css`
Expand Down Expand Up @@ -142,7 +141,6 @@ export function ChooseRobotSlideout(
setHasParamError,
} = props

const enableRunTimeParametersFF = useFeatureFlag('enableRunTimeParameters')
const dispatch = useDispatch<Dispatch>()
const isScanning = useSelector((state: State) => getScanning(state))
const [targetProps, tooltipProps] = useHoverTooltip()
Expand Down Expand Up @@ -526,7 +524,7 @@ export function ChooseRobotSlideout(
</Flex>
) : null

return multiSlideout != null && enableRunTimeParametersFF ? (
return multiSlideout != null ? (
<MultiSlideout
isExpanded={isExpanded}
onCloseClick={onCloseClick}
Expand Down
5 changes: 1 addition & 4 deletions app/src/organisms/ChooseRobotToRunProtocolSlideout/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
import { getRobotUpdateDisplayInfo } from '../../redux/robot-update'
import { OPENTRONS_USB } from '../../redux/discovery'
import { appShellRequestor } from '../../redux/shell/remote'
import { useFeatureFlag } from '../../redux/config'
import { useTrackCreateProtocolRunEvent } from '../Devices/hooks'
import { ApplyHistoricOffsets } from '../ApplyHistoricOffsets'
import { useOffsetCandidatesForAnalysis } from '../ApplyHistoricOffsets/hooks/useOffsetCandidatesForAnalysis'
Expand Down Expand Up @@ -53,7 +52,6 @@ export function ChooseRobotToRunProtocolSlideoutComponent(
srcFiles,
mostRecentAnalysis,
} = storedProtocolData
const enableRunTimeParametersFF = useFeatureFlag('enableRunTimeParameters')
const [currentPage, setCurrentPage] = React.useState<number>(1)
const [selectedRobot, setSelectedRobot] = React.useState<Robot | null>(null)
const { trackCreateProtocolRunEvent } = useTrackCreateProtocolRunEvent(
Expand Down Expand Up @@ -176,8 +174,7 @@ export function ChooseRobotToRunProtocolSlideoutComponent(
</PrimaryButton>
)

const hasRunTimeParameters =
enableRunTimeParametersFF && runTimeParameters.length > 0
const hasRunTimeParameters = runTimeParameters.length > 0

return (
<ChooseRobotSlideout
Expand Down
10 changes: 3 additions & 7 deletions app/src/organisms/ProtocolDetails/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,8 @@ export function ProtocolDetails(
const { protocolKey, srcFileNames, mostRecentAnalysis, modified } = props
const { t, i18n } = useTranslation(['protocol_details', 'shared'])
const enableProtocolStats = useFeatureFlag('protocolStats')
const enableRunTimeParameters = useFeatureFlag('enableRunTimeParameters')
const runTimeParameters = mostRecentAnalysis?.runTimeParameters ?? []
const hasRunTimeParameters =
enableRunTimeParameters && runTimeParameters.length > 0
const hasRunTimeParameters = runTimeParameters.length > 0
const [currentTab, setCurrentTab] = React.useState<
'robot_config' | 'labware' | 'liquids' | 'stats' | 'parameters'
>(hasRunTimeParameters ? 'parameters' : 'robot_config')
Expand Down Expand Up @@ -333,9 +331,7 @@ export function ProtocolDetails(
stats: enableProtocolStats ? (
<ProtocolStats analysis={mostRecentAnalysis} />
) : null,
parameters: enableRunTimeParameters ? (
<ProtocolParameters runTimeParameters={runTimeParameters} />
) : null,
parameters: <ProtocolParameters runTimeParameters={runTimeParameters} />,
}

const deckMap = <ProtocolDeck protocolAnalysis={mostRecentAnalysis} />
Expand Down Expand Up @@ -596,7 +592,7 @@ export function ProtocolDetails(
gridGap={SPACING.spacing8}
>
<Flex gridGap={SPACING.spacing8}>
{enableRunTimeParameters && mostRecentAnalysis != null && (
{mostRecentAnalysis != null && (
<RoundTab
data-testid="ProtocolDetails_parameters"
isCurrent={currentTab === 'parameters'}
Expand Down
9 changes: 1 addition & 8 deletions app/src/organisms/RunTimeControl/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
useRunCommands,
} from '../ProtocolUpload/hooks'
import { useNotifyRunQuery } from '../../resources/runs'
import { useFeatureFlag } from '../../redux/config'
import { useMostRecentCompletedAnalysis } from '../LabwarePositionCheck/useMostRecentCompletedAnalysis'

import type { UseQueryOptions } from 'react-query'
Expand Down Expand Up @@ -188,11 +187,5 @@ export function useRunErrors(runId: string | null): RunData['errors'] {

export function useProtocolHasRunTimeParameters(runId: string | null): boolean {
const mostRecentAnalysis = useMostRecentCompletedAnalysis(runId)
const runTimeParametersFF = useFeatureFlag('enableRunTimeParameters')

console.log(
'TODO: delete the feature flag logic',
mostRecentAnalysis?.runTimeParameters
)
return runTimeParametersFF
return mostRecentAnalysis?.runTimeParameters != null
}
7 changes: 2 additions & 5 deletions app/src/pages/Devices/ProtocolRunDetails/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import { useCurrentRunId } from '../../../organisms/ProtocolUpload/hooks'
import { OPENTRONS_USB } from '../../../redux/discovery'
import { fetchProtocols } from '../../../redux/protocol-storage'
import { appShellRequestor } from '../../../redux/shell/remote'
import { useFeatureFlag } from '../../../redux/config'

import type {
DesktopRouteParams,
Expand Down Expand Up @@ -180,7 +179,7 @@ function PageContents(props: PageContentsProps): JSX.Element {
const protocolRunHeaderRef = React.useRef<HTMLDivElement>(null)
const listRef = React.useRef<ViewportListRef | null>(null)
const [jumpedIndex, setJumpedIndex] = React.useState<number | null>(null)
const enableRunTimeParameters = useFeatureFlag('enableRunTimeParameters')

React.useEffect(() => {
if (jumpedIndex != null) {
setTimeout(() => setJumpedIndex(null), JUMPED_STEP_HIGHLIGHT_DELAY_MS)
Expand Down Expand Up @@ -236,9 +235,7 @@ function PageContents(props: PageContentsProps): JSX.Element {
/>
<Flex gridGap={SPACING.spacing8} marginBottom={SPACING.spacing12}>
<SetupTab robotName={robotName} runId={runId} />
{enableRunTimeParameters ? (
<ParametersTab robotName={robotName} runId={runId} />
) : null}
<ParametersTab robotName={robotName} runId={runId} />
<ModuleControlsTab robotName={robotName} runId={runId} />
<RunPreviewTab robotName={robotName} runId={runId} />
</Flex>
Expand Down
12 changes: 3 additions & 9 deletions app/src/pages/ProtocolDetails/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import {
getApplyHistoricOffsets,
getPinnedProtocolIds,
updateConfigValue,
useFeatureFlag,
} from '../../redux/config'
import { useOffsetCandidatesForAnalysis } from '../../organisms/ApplyHistoricOffsets/hooks/useOffsetCandidatesForAnalysis'
import {
Expand Down Expand Up @@ -189,10 +188,8 @@ const ProtocolSectionTabs = ({
currentOption,
setCurrentOption,
}: ProtocolSectionTabsProps): JSX.Element => {
const enableRtpFF = useFeatureFlag('enableRunTimeParameters')
const options = enableRtpFF
? protocolSectionTabOptions
: protocolSectionTabOptionsWithoutParameters
const options = protocolSectionTabOptions

return (
<Flex gridGap={SPACING.spacing8}>
{options.map(option => {
Expand Down Expand Up @@ -308,7 +305,6 @@ export function ProtocolDetails(): JSX.Element | null {
'protocol_info',
'shared',
])
const enableRtpFF = useFeatureFlag('enableRunTimeParameters')
const { protocolId } = useParams<OnDeviceRouteParams>()
const {
missingProtocolHardware,
Expand All @@ -326,9 +322,7 @@ export function ProtocolDetails(): JSX.Element | null {
const [showParameters, setShowParameters] = React.useState<boolean>(false)
const queryClient = useQueryClient()
const [currentOption, setCurrentOption] = React.useState<TabOption>(
enableRtpFF
? protocolSectionTabOptions[0]
: protocolSectionTabOptionsWithoutParameters[0]
protocolSectionTabOptions[0]
)

const [showMaxPinsAlert, setShowMaxPinsAlert] = React.useState<boolean>(false)
Expand Down
29 changes: 13 additions & 16 deletions app/src/pages/ProtocolSetup/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ import {
ANALYTICS_PROTOCOL_RUN_START,
useTrackEvent,
} from '../../redux/analytics'
import { getIsHeaterShakerAttached, useFeatureFlag } from '../../redux/config'
import { getIsHeaterShakerAttached } from '../../redux/config'
import { ConfirmAttachedModal } from './ConfirmAttachedModal'
import { getLatestCurrentOffsets } from '../../organisms/Devices/ProtocolRun/SetupLabwarePositionCheck/utils'
import { CloseButton, PlayButton } from './Buttons'
Expand Down Expand Up @@ -257,7 +257,6 @@ function PrepareToRun({
const { t, i18n } = useTranslation(['protocol_setup', 'shared'])
const history = useHistory()
const { makeSnackbar } = useToaster()
const enableRunTimeParametersFF = useFeatureFlag('enableRunTimeParameters')
const hasRunTimeParameters = useProtocolHasRunTimeParameters(runId)
// Watch for scrolling to toggle dropshadow
const scrollRef = React.useRef<HTMLDivElement>(null)
Expand Down Expand Up @@ -730,20 +729,18 @@ function PrepareToRun({
disabled={lpcDisabledReason != null}
disabledReason={lpcDisabledReason}
/>
{enableRunTimeParametersFF ? (
<ProtocolSetupStep
onClickSetupStep={() => setSetupScreen('view only parameters')}
title={t('parameters')}
detail={t(
hasRunTimeParameters
? parametersDetail
: t('no_parameters_specified')
)}
subDetail={null}
status="general"
disabled={!hasRunTimeParameters}
/>
) : null}
<ProtocolSetupStep
onClickSetupStep={() => setSetupScreen('view only parameters')}
title={t('parameters')}
detail={t(
hasRunTimeParameters
? parametersDetail
: t('no_parameters_specified')
)}
subDetail={null}
status="general"
disabled={!hasRunTimeParameters}
/>
<ProtocolSetupStep
onClickSetupStep={() => setSetupScreen('labware')}
title={t('labware')}
Expand Down
1 change: 0 additions & 1 deletion app/src/redux/config/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import type { DevInternalFlag } from './types'

export const DEV_INTERNAL_FLAGS: DevInternalFlag[] = [
'protocolStats',
'enableRunTimeParameters',
'enableRunNotes',
'enableQuickTransfer',
]
Expand Down
1 change: 0 additions & 1 deletion app/src/redux/config/schema-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ export type DiscoveryCandidates = string[]

export type DevInternalFlag =
| 'protocolStats'
| 'enableRunTimeParameters'
| 'enableRunNotes'
| 'enableQuickTransfer'

Expand Down
2 changes: 1 addition & 1 deletion app/src/redux/protocol-storage/__fixtures__/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { simpleAnalysisFileFixture } from '@opentrons/api-client'
import { StoredProtocolData, StoredProtocolDir } from '../types'
import type { StoredProtocolData, StoredProtocolDir } from '../types'

import type { ProtocolAnalysisOutput } from '@opentrons/shared-data'

Expand Down

0 comments on commit e620a8c

Please sign in to comment.