Skip to content

Commit

Permalink
fix(app): fix auto transitioning run from recovery-paused to waiting …
Browse files Browse the repository at this point in the history
…recovery (#16259)

Closes EXEC-702

In #16245, we made sure to block an open door while on the recovery splash view, but the logic for automatically transition a run from paused -> awaiting recovery is not quite right, which results in the app seemingly automatically issuing a POST play to transition a run from a recovery paused state to awaiting recovery whenever a user opens the door during error recovery flows (ie, anywhere but the splash screen). The tricky part is that when checking the network tab, there are no POST requests to initiate the transition.

This problem occurs because another app issues the POST request. The logic for dispatching this POST in the useEffect condition within RecoverySplash is to check if the error recovery wizard is active, but this doesn't account for any other app that isn't doing the recovery. The solution: if the current app displays the takeover modal (which is true for any app except the sole app controlling the robot), then don't fire the POST request.
  • Loading branch information
mjhuff authored Sep 16, 2024
1 parent cf27f54 commit dff6347
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 8 deletions.
14 changes: 9 additions & 5 deletions app/src/organisms/ErrorRecoveryFlows/RecoverySplash.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,11 @@ export function useRecoverySplash(
type RecoverySplashProps = ErrorRecoveryFlowsProps &
ERUtilsResults & {
isOnDevice: boolean
isWizardActive: boolean
failedCommand: ReturnType<typeof useRetainedFailedCommandBySource>
robotType: RobotType
robotName: string
/* Whether the app should resume any paused recovery state without user action. */
resumePausedRecovery: boolean
toggleERWizAsActiveUser: UseRecoveryTakeoverResult['toggleERWizAsActiveUser']
analytics: UseRecoveryAnalyticsResult
}
Expand All @@ -79,7 +80,7 @@ export function RecoverySplash(props: RecoverySplashProps): JSX.Element | null {
robotName,
runStatus,
recoveryActionMutationUtils,
isWizardActive,
resumePausedRecovery,
} = props
const { t } = useTranslation('error_recovery')
const errorKind = getErrorKind(failedCommand?.byRunRecord ?? null)
Expand All @@ -99,12 +100,15 @@ export function RecoverySplash(props: RecoverySplashProps): JSX.Element | null {

// Resume recovery when the run when the door is closed.
// The CTA/flow for handling a door open event within the ER wizard is different, and because this splash always renders
// behind the wizard, we want to ensure we only implicitly resume recovery when only viewing the splash.
// behind the wizard, we want to ensure we only implicitly resume recovery when only viewing the splash from this app.
React.useEffect(() => {
if (runStatus === RUN_STATUS_AWAITING_RECOVERY_PAUSED && !isWizardActive) {
if (
runStatus === RUN_STATUS_AWAITING_RECOVERY_PAUSED &&
resumePausedRecovery
) {
recoveryActionMutationUtils.resumeRecovery()
}
}, [runStatus, isWizardActive])
}, [runStatus, resumePausedRecovery])
const buildDoorOpenAlert = (): void => {
makeToast(t('close_door_to_resume') as string, WARNING_TOAST)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe('RecoverySplash', () => {
recoveryActionMutationUtils: {
resumeRecovery: mockResumeRecovery,
} as any,
isWizardActive: false,
resumePausedRecovery: true,
}

vi.mocked(StepInfo).mockReturnValue(<div>MOCK STEP INFO</div>)
Expand Down Expand Up @@ -177,7 +177,7 @@ describe('RecoverySplash', () => {
expect(mockMakeToast).toHaveBeenCalled()
})

it(`should transition the run status from ${RUN_STATUS_AWAITING_RECOVERY_PAUSED} to ${RUN_STATUS_AWAITING_RECOVERY}`, () => {
it(`should transition the run status from ${RUN_STATUS_AWAITING_RECOVERY_PAUSED} to ${RUN_STATUS_AWAITING_RECOVERY} when resumePausedRecovery is true`, () => {
props = { ...props, runStatus: RUN_STATUS_AWAITING_RECOVERY_PAUSED }

render(props)
Expand Down
4 changes: 3 additions & 1 deletion app/src/organisms/ErrorRecoveryFlows/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ export function ErrorRecoveryFlows(
failedCommand: failedCommandBySource,
})

console.log('=>(index.tsx:180) showTakeover', showTakeover)

return (
<>
{showTakeover ? (
Expand Down Expand Up @@ -176,7 +178,7 @@ export function ErrorRecoveryFlows(
isOnDevice={isOnDevice}
toggleERWizAsActiveUser={toggleERWizAsActiveUser}
failedCommand={failedCommandBySource}
isWizardActive={renderWizard}
resumePausedRecovery={!renderWizard && !showTakeover}
/>
) : null}
</>
Expand Down

0 comments on commit dff6347

Please sign in to comment.