Skip to content

Commit 2532997

Browse files
authored
fix(app): prevent "run again" banner from rendering after navigating away from the current run (#14973)
Closes RQA-2620
1 parent ec73d82 commit 2532997

File tree

2 files changed

+76
-54
lines changed

2 files changed

+76
-54
lines changed

app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx

Lines changed: 61 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,6 @@ export function ProtocolRunHeader({
174174
const [pipettesWithTip, setPipettesWithTip] = React.useState<
175175
PipettesWithTip[]
176176
>([])
177-
const [closeTerminalBanner, setCloseTerminalBanner] = React.useState(false)
178177
const isResetRunLoadingRef = React.useRef(false)
179178
const { data: runRecord } = useNotifyRunQuery(runId, { staleTime: Infinity })
180179
const highestPriorityError =
@@ -200,7 +199,7 @@ export function ProtocolRunHeader({
200199
const { data: doorStatus } = useDoorQuery({
201200
refetchInterval: EQUIPMENT_POLL_MS,
202201
})
203-
let isDoorOpen = false
202+
let isDoorOpen: boolean
204203
if (isFlex) {
205204
isDoorOpen = doorStatus?.data.status === 'open'
206205
} else if (!isFlex && Boolean(doorSafetySetting?.value)) {
@@ -248,7 +247,9 @@ export function ProtocolRunHeader({
248247
}
249248
}, [protocolData, isRobotViewable, history])
250249

250+
// Side effects dependent on the current run state.
251251
React.useEffect(() => {
252+
// After a user-initiated stopped run, close the run current run automatically.
252253
if (runStatus === RUN_STATUS_STOPPED && isRunCurrent && runId != null) {
253254
trackProtocolRunEvent({
254255
name: ANALYTICS_PROTOCOL_RUN_FINISH,
@@ -260,12 +261,6 @@ export function ProtocolRunHeader({
260261
}
261262
}, [runStatus, isRunCurrent, runId, closeCurrentRun])
262263

263-
React.useEffect(() => {
264-
if (runStatus === RUN_STATUS_IDLE) {
265-
setCloseTerminalBanner(false)
266-
}
267-
}, [runStatus])
268-
269264
const startedAtTimestamp =
270265
startedAt != null ? formatTimestamp(startedAt) : EMPTY_TIMESTAMP
271266

@@ -310,7 +305,6 @@ export function ProtocolRunHeader({
310305
properties: robotAnalyticsData ?? undefined,
311306
})
312307
closeCurrentRun()
313-
setCloseTerminalBanner(true)
314308
}
315309

316310
return (
@@ -375,7 +369,7 @@ export function ProtocolRunHeader({
375369
CANCELLABLE_STATUSES.includes(runStatus) ? (
376370
<Banner type="warning">{t('shared:close_robot_door')}</Banner>
377371
) : null}
378-
{mostRecentRunId === runId && !closeTerminalBanner ? (
372+
{mostRecentRunId === runId ? (
379373
<TerminalRunBanner
380374
{...{
381375
runStatus,
@@ -385,6 +379,7 @@ export function ProtocolRunHeader({
385379
highestPriorityError,
386380
}}
387381
isResetRunLoading={isResetRunLoadingRef.current}
382+
isRunCurrent={isRunCurrent}
388383
/>
389384
) : null}
390385
{mostRecentRunId === runId &&
@@ -479,7 +474,9 @@ export function ProtocolRunHeader({
479474
setShowDropTipWizard(false)
480475
setPipettesWithTip(prevPipettesWithTip => {
481476
const pipettesWithTip = prevPipettesWithTip.slice(1) ?? []
482-
if (pipettesWithTip.length === 0) closeCurrentRun()
477+
if (pipettesWithTip.length === 0) {
478+
closeCurrentRun()
479+
}
483480
return pipettesWithTip
484481
})
485482
}}
@@ -570,6 +567,7 @@ interface ActionButtonProps {
570567
isResetRunLoadingRef: React.MutableRefObject<boolean>
571568
}
572569

570+
// TODO(jh, 04-22-2024): Refactor switch cases into separate factories to increase readability and testability.
573571
function ActionButton(props: ActionButtonProps): JSX.Element {
574572
const {
575573
runId,
@@ -613,9 +611,7 @@ function ActionButton(props: ActionButtonProps): JSX.Element {
613611
robotName,
614612
runId
615613
)
616-
const [showIsShakingModal, setShowIsShakingModal] = React.useState<boolean>(
617-
false
618-
)
614+
const [showIsShakingModal, setShowIsShakingModal] = React.useState(false)
619615
const isSetupComplete =
620616
isCalibrationComplete &&
621617
isModuleCalibrationComplete &&
@@ -804,12 +800,14 @@ function ActionButton(props: ActionButtonProps): JSX.Element {
804800
)
805801
}
806802

803+
// TODO(jh 04-24-2024): Split TerminalRunBanner into a RunSuccessBanner and RunFailedBanner.
807804
interface TerminalRunProps {
808805
runStatus: RunStatus | null
809806
handleClearClick: () => void
810807
isClosingCurrentRun: boolean
811808
setShowRunFailedModal: (showRunFailedModal: boolean) => void
812809
isResetRunLoading: boolean
810+
isRunCurrent: boolean
813811
highestPriorityError?: RunError | null
814812
}
815813
function TerminalRunBanner(props: TerminalRunProps): JSX.Element | null {
@@ -820,51 +818,64 @@ function TerminalRunBanner(props: TerminalRunProps): JSX.Element | null {
820818
setShowRunFailedModal,
821819
highestPriorityError,
822820
isResetRunLoading,
821+
isRunCurrent,
823822
} = props
824823
const { t } = useTranslation('run_details')
825824

826-
const handleClick = (): void => {
825+
const handleRunSuccessClick = (): void => {
826+
handleClearClick()
827+
}
828+
829+
const handleFailedRunClick = (): void => {
827830
handleClearClick()
828831
setShowRunFailedModal(true)
829832
}
830833

831-
if (
832-
isResetRunLoading === false &&
833-
(runStatus === RUN_STATUS_FAILED || runStatus === RUN_STATUS_SUCCEEDED)
834-
) {
834+
const buildSuccessBanner = (): JSX.Element => {
835835
return (
836-
<>
837-
{runStatus === RUN_STATUS_SUCCEEDED ? (
838-
<Banner
839-
type="success"
840-
onCloseClick={handleClearClick}
841-
isCloseActionLoading={isClosingCurrentRun}
842-
>
843-
<Flex justifyContent={JUSTIFY_SPACE_BETWEEN} width="100%">
844-
{t('run_completed')}
845-
</Flex>
846-
</Banner>
847-
) : (
848-
<Banner type="error">
849-
<Flex justifyContent={JUSTIFY_SPACE_BETWEEN} width="100%">
850-
<StyledText>
851-
{t('error_info', {
852-
errorType: highestPriorityError?.errorType,
853-
errorCode: highestPriorityError?.errorCode,
854-
})}
855-
</StyledText>
836+
<Banner
837+
type="success"
838+
onCloseClick={handleRunSuccessClick}
839+
isCloseActionLoading={isClosingCurrentRun}
840+
>
841+
<Flex justifyContent={JUSTIFY_SPACE_BETWEEN} width="100%">
842+
{t('run_completed')}
843+
</Flex>
844+
</Banner>
845+
)
846+
}
856847

857-
<LinkButton
858-
onClick={handleClick}
859-
textDecoration={TYPOGRAPHY.textDecorationUnderline}
860-
>
861-
{t('view_error')}
862-
</LinkButton>
863-
</Flex>
864-
</Banner>
865-
)}
866-
</>
848+
const buildErrorBanner = (): JSX.Element => {
849+
return (
850+
<Banner type="error">
851+
<Flex justifyContent={JUSTIFY_SPACE_BETWEEN} width="100%">
852+
<StyledText>
853+
{t('error_info', {
854+
errorType: highestPriorityError?.errorType,
855+
errorCode: highestPriorityError?.errorCode,
856+
})}
857+
</StyledText>
858+
859+
<LinkButton
860+
onClick={handleFailedRunClick}
861+
textDecoration={TYPOGRAPHY.textDecorationUnderline}
862+
>
863+
{t('view_error')}
864+
</LinkButton>
865+
</Flex>
866+
</Banner>
867867
)
868868
}
869-
return null
869+
870+
if (
871+
runStatus === RUN_STATUS_SUCCEEDED &&
872+
isRunCurrent &&
873+
!isResetRunLoading
874+
) {
875+
return buildSuccessBanner()
876+
} else if (runStatus === RUN_STATUS_FAILED && !isResetRunLoading) {
877+
return buildErrorBanner()
878+
} else {
879+
return null
880+
}
870881
}

app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -814,7 +814,7 @@ describe('ProtocolRunHeader', () => {
814814

815815
screen.getByText('Run completed.')
816816
})
817-
it('clicking close on a terminal run banner closes the run context and dismisses the banner', async () => {
817+
it('clicking close on a terminal run banner closes the run context', async () => {
818818
when(vi.mocked(useNotifyRunQuery))
819819
.calledWith(RUN_ID)
820820
.thenReturn({
@@ -827,9 +827,20 @@ describe('ProtocolRunHeader', () => {
827827

828828
fireEvent.click(screen.getByTestId('Banner_close-button'))
829829
expect(mockCloseCurrentRun).toBeCalled()
830-
await waitFor(() => {
831-
expect(screen.queryByText('Run completed.')).not.toBeInTheDocument()
832-
})
830+
})
831+
832+
it('does not display the "run successful" banner if the successful run is not current', async () => {
833+
when(vi.mocked(useNotifyRunQuery))
834+
.calledWith(RUN_ID)
835+
.thenReturn({
836+
data: { data: { ...mockSucceededRun, current: false } },
837+
} as UseQueryResult<OpentronsApiClient.Run>)
838+
when(vi.mocked(useRunStatus))
839+
.calledWith(RUN_ID)
840+
.thenReturn(RUN_STATUS_SUCCEEDED)
841+
render()
842+
843+
expect(screen.queryByText('Run completed.')).not.toBeInTheDocument()
833844
})
834845

835846
it('if a heater shaker is shaking, clicking on start run should render HeaterShakerIsRunningModal', async () => {

0 commit comments

Comments
 (0)