refactor(app): Refactor intervention modal render behavior (#15898)

Closes RQA-2904

The logic for rendering InterventionModal on the ODD/Desktop is a little bit different when looking at the exact conditions, and this (likely) causes the InterventionModal to render on the ODD sometimes but not on the desktop app, and vice versa.

This is a good opportunity to refactor all of this logic into its own hook and use that hook where we render InterventionModal. After thinking through the render logic, there's room to simplify it a bit, too. We don't actually need stateful storage of an intervention command key. Also, I decided to separate showModal from modalProps (which lets us pass all the non-null props simply), even though we could technically just do a truthy check for modalProps for rendering InterventionModal, since this is maybe a bit more intuitive. Lastly, a few missing tests are added.

To help with bug testing intervention modals, I added a couple console.warns.

app/src/organisms/InterventionModal/__tests__/InterventionModal.test.tsx

@@ -1,28 +1,87 @@
 import * as React from 'react'
-import { fireEvent, screen } from '@testing-library/react'
+import { fireEvent, renderHook, screen } from '@testing-library/react'
 import { describe, it, expect, vi, beforeEach } from 'vitest'
+
+import { RUN_STATUS_RUNNING, RUN_STATUS_STOPPED } from '@opentrons/api-client'
 import { getLabwareDefURI } from '@opentrons/shared-data'
-import { renderWithProviders } from '../../../__testing-utils__'
 
+import { renderWithProviders } from '../../../__testing-utils__'
+import { mockTipRackDefinition } from '../../../redux/custom-labware/__fixtures__'
 import { i18n } from '../../../i18n'
-import { InterventionModal } from '..'
 import {
   mockPauseCommandWithoutStartTime,
   mockPauseCommandWithStartTime,
   mockMoveLabwareCommandFromSlot,
   mockMoveLabwareCommandFromModule,
   truncatedCommandMessage,
 } from '../__fixtures__'
-import { mockTipRackDefinition } from '../../../redux/custom-labware/__fixtures__'
+import { InterventionModal, useInterventionModal } from '..'
 import { useIsFlex } from '../../Devices/hooks'
+
 import type { CompletedProtocolAnalysis } from '@opentrons/shared-data'
+import type { RunData } from '@opentrons/api-client'
 
 const ROBOT_NAME = 'Otie'
 
 const mockOnResumeHandler = vi.fn()
 
 vi.mock('../../Devices/hooks')
 
+describe('useInterventionModal', () => {
+  const defaultProps = {
+    runData: { id: 'run1' } as RunData,
+    lastRunCommand: mockPauseCommandWithStartTime,
+    runStatus: RUN_STATUS_RUNNING,
+    robotName: 'TestRobot',
+    analysis: null,
+  }
+
+  it('should return showModal true when conditions are met', () => {
+    const { result } = renderHook(() => useInterventionModal(defaultProps))
+
+    expect(result.current.showModal).toBe(true)
+    expect(result.current.modalProps).not.toBeNull()
+  })
+
+  it('should return showModal false when runStatus is terminal', () => {
+    const props = { ...defaultProps, runStatus: RUN_STATUS_STOPPED }
+
+    const { result } = renderHook(() => useInterventionModal(props))
+
+    expect(result.current.showModal).toBe(false)
+    expect(result.current.modalProps).toBeNull()
+  })
+
+  it('should return showModal false when lastRunCommand is null', () => {
+    const props = { ...defaultProps, lastRunCommand: null }
+
+    const { result } = renderHook(() => useInterventionModal(props))
+
+    expect(result.current.showModal).toBe(false)
+    expect(result.current.modalProps).toBeNull()
+  })
+
+  it('should return showModal false when robotName is null', () => {
+    const props = { ...defaultProps, robotName: null }
+
+    const { result } = renderHook(() => useInterventionModal(props))
+
+    expect(result.current.showModal).toBe(false)
+    expect(result.current.modalProps).toBeNull()
+  })
+
+  it('should return correct modalProps when showModal is true', () => {
+    const { result } = renderHook(() => useInterventionModal(defaultProps))
+
+    expect(result.current.modalProps).toEqual({
+      command: mockPauseCommandWithStartTime,
+      run: defaultProps.runData,
+      robotName: 'TestRobot',
+      analysis: null,
+    })
+  })
+})
+
 const render = (props: React.ComponentProps<typeof InterventionModal>) => {
   return renderWithProviders(<InterventionModal {...props} />, {
     i18nInstance: i18n,

app/src/organisms/InterventionModal/index.tsx

@@ -19,37 +19,79 @@ import {
   TYPOGRAPHY,
   LegacyStyledText,
 } from '@opentrons/components'
+import {
+  RUN_STATUS_FAILED,
+  RUN_STATUS_FINISHING,
+  RUN_STATUS_STOPPED,
+  RUN_STATUS_SUCCEEDED,
+} from '@opentrons/api-client'
 
 import { SmallButton } from '../../atoms/buttons'
 import { Modal } from '../../molecules/Modal'
 import { InterventionModal as InterventionModalMolecule } from '../../molecules/InterventionModal'
 import { getIsOnDevice } from '../../redux/config'
 import { PauseInterventionContent } from './PauseInterventionContent'
 import { MoveLabwareInterventionContent } from './MoveLabwareInterventionContent'
+import { isInterventionCommand } from './utils'
+import { useRobotType } from '../Devices/hooks'
 
-import type { RunCommandSummary, RunData } from '@opentrons/api-client'
 import type { IconName } from '@opentrons/components'
 import type { CompletedProtocolAnalysis } from '@opentrons/shared-data'
-import { useRobotType } from '../Devices/hooks'
+import type {
+  RunCommandSummary,
+  RunData,
+  RunStatus,
+} from '@opentrons/api-client'
 
-const LEARN_ABOUT_MANUAL_STEPS_URL =
-  'https://support.opentrons.com/s/article/Manual-protocol-steps'
+const TERMINAL_RUN_STATUSES: RunStatus[] = [
+  RUN_STATUS_STOPPED,
+  RUN_STATUS_FAILED,
+  RUN_STATUS_FINISHING,
+  RUN_STATUS_SUCCEEDED,
+]
 
-const CONTENT_STYLE = {
-  display: DISPLAY_FLEX,
-  flexDirection: DIRECTION_COLUMN,
-  alignItems: ALIGN_FLEX_START,
-  gridGap: SPACING.spacing24,
-  padding: SPACING.spacing32,
-  borderRadius: BORDERS.borderRadius8,
-} as const
+export interface UseInterventionModalProps {
+  runData: RunData | null
+  lastRunCommand: RunCommandSummary | null
+  runStatus: RunStatus | null
+  robotName: string | null
+  analysis: CompletedProtocolAnalysis | null
+}
 
-const FOOTER_STYLE = {
-  display: DISPLAY_FLEX,
-  width: '100%',
-  alignItems: ALIGN_CENTER,
-  justifyContent: JUSTIFY_SPACE_BETWEEN,
-} as const
+export type UseInterventionModalResult =
+  | { showModal: false; modalProps: null }
+  | { showModal: true; modalProps: Omit<InterventionModalProps, 'onResume'> }
+
+// If showModal is true, modalProps are guaranteed not to be null.
+export function useInterventionModal({
+  runData,
+  lastRunCommand,
+  runStatus,
+  robotName,
+  analysis,
+}: UseInterventionModalProps): UseInterventionModalResult {
+  const isValidIntervention =
+    lastRunCommand != null &&
+    robotName != null &&
+    isInterventionCommand(lastRunCommand) &&
+    runData != null &&
+    runStatus != null &&
+    !TERMINAL_RUN_STATUSES.includes(runStatus)
+
+  if (!isValidIntervention) {
+    return { showModal: false, modalProps: null }
+  } else {
+    return {
+      showModal: true,
+      modalProps: {
+        command: lastRunCommand,
+        run: runData,
+        robotName,
+        analysis,
+      },
+    }
+  }
+}
 
 export interface InterventionModalProps {
   robotName: string
@@ -71,25 +113,28 @@ export function InterventionModal({
 
   const robotType = useRobotType(robotName)
   const childContent = React.useMemo(() => {
-    if (
-      command.commandType === 'waitForResume' ||
-      command.commandType === 'pause' // legacy pause command
-    ) {
-      return (
-        <PauseInterventionContent
-          startedAt={command.startedAt ?? null}
-          message={command.params.message ?? null}
-        />
-      )
-    } else if (command.commandType === 'moveLabware') {
-      return (
-        <MoveLabwareInterventionContent
-          {...{ command, run, analysis, robotType }}
-          isOnDevice={isOnDevice}
-        />
-      )
-    } else {
-      return null
+    switch (command.commandType) {
+      case 'waitForResume':
+      case 'pause': // legacy pause command
+        return (
+          <PauseInterventionContent
+            startedAt={command.startedAt ?? null}
+            message={command.params.message ?? null}
+          />
+        )
+      case 'moveLabware':
+        return (
+          <MoveLabwareInterventionContent
+            {...{ command, run, analysis, robotType }}
+            isOnDevice={isOnDevice}
+          />
+        )
+      default:
+        console.warn(
+          'Unhandled command passed to InterventionModal: ',
+          command.commandType
+        )
+        return null
     }
   }, [
     command.id,
@@ -98,21 +143,33 @@ export function InterventionModal({
     run.modules.map(m => m.id).join(),
   ])
 
-  let iconName: IconName | null = null
-  let headerTitle = ''
-  let headerTitleOnDevice = ''
-  if (
-    command.commandType === 'waitForResume' ||
-    command.commandType === 'pause' // legacy pause command
-  ) {
-    iconName = 'pause-circle'
-    headerTitle = t('pause_on', { robot_name: robotName })
-    headerTitleOnDevice = t('pause')
-  } else if (command.commandType === 'moveLabware') {
-    iconName = 'move-xy-circle'
-    headerTitle = t('move_labware_on', { robot_name: robotName })
-    headerTitleOnDevice = t('move_labware')
-  }
+  const { iconName, headerTitle, headerTitleOnDevice } = (() => {
+    switch (command.commandType) {
+      case 'waitForResume':
+      case 'pause':
+        return {
+          iconName: 'pause-circle' as IconName,
+          headerTitle: t('pause_on', { robot_name: robotName }),
+          headerTitleOnDevice: t('pause'),
+        }
+      case 'moveLabware':
+        return {
+          iconName: 'move-xy-circle' as IconName,
+          headerTitle: t('move_labware_on', { robot_name: robotName }),
+          headerTitleOnDevice: t('move_labware'),
+        }
+      default:
+        console.warn(
+          'Unhandled command passed to InterventionModal: ',
+          command.commandType
+        )
+        return {
+          iconName: null,
+          headerTitle: '',
+          headerTitleOnDevice: '',
+        }
+    }
+  })()
 
   // TODO(bh, 2023-7-18): this is a one-off modal implementation for desktop
   // reimplement when design system shares a modal component between desktop/ODD
@@ -171,3 +228,22 @@ export function InterventionModal({
     </InterventionModalMolecule>
   )
 }
+
+const LEARN_ABOUT_MANUAL_STEPS_URL =
+  'https://support.opentrons.com/s/article/Manual-protocol-steps'
+
+const CONTENT_STYLE = {
+  display: DISPLAY_FLEX,
+  flexDirection: DIRECTION_COLUMN,
+  alignItems: ALIGN_FLEX_START,
+  gridGap: SPACING.spacing24,
+  padding: SPACING.spacing32,
+  borderRadius: BORDERS.borderRadius8,
+} as const
+
+const FOOTER_STYLE = {
+  display: DISPLAY_FLEX,
+  width: '100%',
+  alignItems: ALIGN_CENTER,
+  justifyContent: JUSTIFY_SPACE_BETWEEN,
+} as const

app/src/organisms/RunProgressMeter/__tests__/RunProgressMeter.test.tsx

@@ -13,7 +13,10 @@ import {
 } from '@opentrons/api-client'
 
 import { i18n } from '../../../i18n'
-import { InterventionModal } from '../../InterventionModal'
+import {
+  useInterventionModal,
+  InterventionModal,
+} from '../../InterventionModal'
 import { ProgressBar } from '../../../atoms/ProgressBar'
 import { useRunStatus } from '../../RunTimeControl/hooks'
 import { useMostRecentCompletedAnalysis } from '../../LabwarePositionCheck/useMostRecentCompletedAnalysis'
@@ -27,11 +30,7 @@ import {
   mockUseCommandResultNonDeterministic,
   NON_DETERMINISTIC_COMMAND_KEY,
 } from '../__fixtures__'
-import {
-  mockMoveLabwareCommandFromSlot,
-  mockPauseCommandWithStartTime,
-  mockRunData,
-} from '../../InterventionModal/__fixtures__'
+
 import { RunProgressMeter } from '..'
 import { renderWithProviders } from '../../../__testing-utils__'
 import { useLastRunCommand } from '../../Devices/hooks/useLastRunCommand'
@@ -70,7 +69,7 @@ describe('RunProgressMeter', () => {
   beforeEach(() => {
     vi.mocked(ProgressBar).mockReturnValue(<div>MOCK PROGRESS BAR</div>)
     vi.mocked(InterventionModal).mockReturnValue(
-      <div>MOCK INTERVENTION MODAL</div>
+      <div>MOCK_INTERVENTION_MODAL</div>
     )
     vi.mocked(useRunStatus).mockReturnValue(RUN_STATUS_RUNNING)
     when(useMostRecentCompletedAnalysis)
@@ -96,6 +95,10 @@ describe('RunProgressMeter', () => {
       currentStepNumber: null,
       hasRunDiverged: true,
     })
+    vi.mocked(useInterventionModal).mockReturnValue({
+      showModal: false,
+      modalProps: {} as any,
+    })
 
     props = {
       runId: NON_DETERMINISTIC_RUN_ID,
@@ -119,31 +122,18 @@ describe('RunProgressMeter', () => {
     screen.getByText('Not started yet')
     screen.getByText('Download run log')
   })
-  it('should render an intervention modal when lastRunCommand is a pause command', () => {
-    vi.mocked(useNotifyAllCommandsQuery).mockReturnValue({
-      data: { data: [mockPauseCommandWithStartTime], meta: { totalLength: 1 } },
-    } as any)
-    vi.mocked(useNotifyRunQuery).mockReturnValue({
-      data: { data: { labware: [] } },
-    } as any)
-    vi.mocked(useCommandQuery).mockReturnValue({ data: null } as any)
-    vi.mocked(useMostRecentCompletedAnalysis).mockReturnValue({} as any)
-    render(props)
-  })
 
-  it('should render an intervention modal when lastRunCommand is a move labware command', () => {
-    vi.mocked(useNotifyAllCommandsQuery).mockReturnValue({
-      data: {
-        data: [mockMoveLabwareCommandFromSlot],
-        meta: { totalLength: 1 },
-      },
-    } as any)
-    vi.mocked(useNotifyRunQuery).mockReturnValue({
-      data: { data: mockRunData },
-    } as any)
+  it('should render an intervention modal when showInterventionModal is true', () => {
     vi.mocked(useCommandQuery).mockReturnValue({ data: null } as any)
-    vi.mocked(useMostRecentCompletedAnalysis).mockReturnValue({} as any)
+    vi.mocked(useRunStatus).mockReturnValue(RUN_STATUS_IDLE)
+    vi.mocked(useInterventionModal).mockReturnValue({
+      showModal: true,
+      modalProps: {} as any,
+    })
+
     render(props)
+
+    screen.getByText('MOCK_INTERVENTION_MODAL')
   })
 
   it('should render the correct run status when run status is completed', () => {