Skip to content

Commit

Permalink
fix(app): resume at appropriate drop tip route after door open event (#…
Browse files Browse the repository at this point in the history
…15871)

Closes EXEC-605

This PR adds something between a feature and a bug fix. Earlier, we added rough support for a "submap", which is basically a way to get more granular routing when necessary. So for example, if you have an error recovery route and step, you could have a sub route and sub step. The idea is this would probably be necessary at some point when we had multiple apps looking at the same exact thing at the same time, but it wouldn't be needed for this release.

Turns out we needed it this release in exactly one case. When we do drop tip wizard within error recovery, we effectively go to the "drop tip route" and the "before beginning" or "drop tip" or "blowout" step. When the door opens, we change the route and step to the door route and step, but when we navigate back to where we were after resuming the door open event, we effectively end up back at the start of the flows, because error recovery can't exactly pinpoint where we are in the drop tip flow.

What we really need is something roughly equivalent to (for example) "recovery-drop-tip/recovery-blowout/drop-tip-flow-blowout/drop-tip-flow-jog-to-position" in order to retain the routing correctly. This is the perfect spot for using a submap!
  • Loading branch information
mjhuff authored Aug 2, 2024
1 parent dceee85 commit c66e43e
Show file tree
Hide file tree
Showing 12 changed files with 137 additions and 77 deletions.
11 changes: 8 additions & 3 deletions app/src/organisms/DropTipWizardFlows/DropTipWizard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -324,10 +324,15 @@ export const DropTipWizardContent = (
const handleProceed = (): void => {
if (currentStep === BLOWOUT_SUCCESS) {
void proceedToRoute(DT_ROUTES.DROP_TIP)
} else if (tipDropComplete != null) {
tipDropComplete()
} else {
proceedWithConditionalClose()
// Clear the error recovery submap upon completion of drop tip wizard.
fixitCommandTypeUtils?.reportMap(null)

if (tipDropComplete != null) {
tipDropComplete()
} else {
proceedWithConditionalClose()
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe('useSeenBlowoutSuccess', () => {
it('should not render step counter when currentRoute is BEFORE_BEGINNING', () => {
const { result } = renderHook(() =>
useSeenBlowoutSuccess({
currentStep: 'SOME_STEP',
currentStep: 'SOME_STEP' as any,
currentRoute: DT_ROUTES.BEFORE_BEGINNING,
currentStepIdx: 0,
})
Expand Down
10 changes: 5 additions & 5 deletions app/src/organisms/DropTipWizardFlows/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ export const POSITION_AND_DROP_TIP = 'POSITION_AND_DROP_TIP' as const
export const DROP_TIP_SUCCESS = 'DROP_TIP_SUCCESS' as const
export const INVALID = 'INVALID' as const

const BEFORE_BEGINNING_STEPS = [BEFORE_BEGINNING]
const BLOWOUT_STEPS = [
export const BEFORE_BEGINNING_STEPS = [BEFORE_BEGINNING] as const
export const BLOWOUT_STEPS = [
CHOOSE_BLOWOUT_LOCATION,
POSITION_AND_BLOWOUT,
BLOWOUT_SUCCESS,
]
const DROP_TIP_STEPS = [
] as const
export const DROP_TIP_STEPS = [
CHOOSE_DROP_TIP_LOCATION,
POSITION_AND_DROP_TIP,
DROP_TIP_SUCCESS,
]
] as const

export const DT_ROUTES = {
BEFORE_BEGINNING: BEFORE_BEGINNING_STEPS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@ describe('useDropTipRouting', () => {
})
})

describe('useExternalMapUpdates', () => {
it('should call trackCurrentMap when the drop tip flow map updates', async () => {
const mockTrackCurrentMap = vi.fn()
describe('useReportMap', () => {
it('should call reportMap when the drop tip flow map updates', async () => {
const mockReportMap = vi.fn()

const mockFixitUtils = {
trackCurrentMap: mockTrackCurrentMap,
reportMap: mockReportMap,
} as any

const { result } = renderHook(() => useDropTipRouting(mockFixitUtils))
Expand All @@ -101,18 +101,18 @@ describe('useExternalMapUpdates', () => {
await result.current.proceedToRoute(DT_ROUTES.BLOWOUT)
})

expect(mockTrackCurrentMap).toHaveBeenCalledWith({
currentRoute: DT_ROUTES.BLOWOUT,
currentStep: expect.any(String),
expect(mockReportMap).toHaveBeenCalledWith({
route: DT_ROUTES.BLOWOUT,
step: expect.any(String),
})

await act(async () => {
await result.current.proceed()
})

expect(mockTrackCurrentMap).toHaveBeenCalledWith({
currentRoute: DT_ROUTES.BLOWOUT,
currentStep: expect.any(String),
expect(mockReportMap).toHaveBeenCalledWith({
route: DT_ROUTES.BLOWOUT,
step: expect.any(String),
})
})
})
Expand All @@ -126,9 +126,7 @@ describe('getInitialRouteAndStep', () => {
})

it('should return the default initial route and step when fixitUtils.routeOverride is not provided', () => {
const fixitUtils = {
routeOverride: undefined,
} as any
const fixitUtils = undefined

const [initialRoute, initialStep] = getInitialRouteAndStep(fixitUtils)

Expand All @@ -138,12 +136,12 @@ describe('getInitialRouteAndStep', () => {

it('should return the overridden route and step when fixitUtils.routeOverride is provided', () => {
const fixitUtils = {
routeOverride: DT_ROUTES.DROP_TIP,
routeOverride: { route: DT_ROUTES.DROP_TIP, step: DT_ROUTES.DROP_TIP[2] },
} as any

const [initialRoute, initialStep] = getInitialRouteAndStep(fixitUtils)

expect(initialRoute).toBe(DT_ROUTES.DROP_TIP)
expect(initialStep).toBe(DT_ROUTES.DROP_TIP[0])
expect(initialStep).toBe(DT_ROUTES.DROP_TIP[2])
})
})
20 changes: 10 additions & 10 deletions app/src/organisms/DropTipWizardFlows/hooks/useDropTipRouting.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as React from 'react'
import head from 'lodash/head'
import last from 'lodash/last'

import { DT_ROUTES, INVALID } from '../constants'
import { BEFORE_BEGINNING_STEPS, DT_ROUTES, INVALID } from '../constants'

import type {
DropTipFlowsRoute,
Expand Down Expand Up @@ -46,7 +46,7 @@ export function useDropTipRouting(
): UseDropTipRoutingResult {
const [initialRoute, initialStep] = React.useMemo(
() => getInitialRouteAndStep(fixitUtils),
[fixitUtils]
[]
)

const [dropTipFlowsMap, setDropTipFlowsMap] = React.useState<DropTipFlowsMap>(
Expand All @@ -57,7 +57,7 @@ export function useDropTipRouting(
}
)

useExternalMapUpdates(dropTipFlowsMap, fixitUtils)
useReportMap(dropTipFlowsMap, fixitUtils)

const { currentStep, currentRoute } = dropTipFlowsMap

Expand Down Expand Up @@ -126,7 +126,7 @@ interface DropTipRouteNavigationResult {

// Returns functions that calculate the next and previous steps of a route given a step.
function getDropTipRouteNavigation(
route: DropTipFlowsStep[]
route: readonly DropTipFlowsStep[]
): DropTipRouteNavigationResult {
const getNextStep = (step: DropTipFlowsStep): StepNavigationResult => {
const isStepFinalStep = step === last(route)
Expand Down Expand Up @@ -180,26 +180,26 @@ function determineValidRoute(
}

// If an external flow is keeping track of the Drop tip flow map, update it when the drop tip flow map updates.
export function useExternalMapUpdates(
export function useReportMap(
map: DropTipFlowsMap,
fixitUtils?: FixitCommandTypeUtils
): void {
const { currentStep, currentRoute } = map

React.useEffect(() => {
if (fixitUtils != null) {
fixitUtils.trackCurrentMap({ currentRoute, currentStep })
fixitUtils.reportMap({ route: currentRoute, step: currentStep })
}
}, [currentStep, currentRoute, fixitUtils])
}, [currentStep, currentRoute])
}

// If present, return fixit route overrides for setting the initial Drop Tip Wizard route.
export function getInitialRouteAndStep(
fixitUtils?: FixitCommandTypeUtils
): [DropTipFlowsRoute, DropTipFlowsStep] {
const routeOverride = fixitUtils?.routeOverride
const initialRoute = routeOverride ?? DT_ROUTES.BEFORE_BEGINNING
const initialStep = head(routeOverride) ?? head(DT_ROUTES.BEFORE_BEGINNING)
const initialRoute = routeOverride?.route ?? DT_ROUTES.BEFORE_BEGINNING
const initialStep = routeOverride?.step ?? BEFORE_BEGINNING_STEPS[0]

return [initialRoute as DropTipFlowsRoute, initialStep as DropTipFlowsStep]
return [initialRoute, initialStep]
}
13 changes: 9 additions & 4 deletions app/src/organisms/DropTipWizardFlows/types.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import type { DT_ROUTES } from './constants'
import type { DropTipErrorComponents } from './hooks'
import type { DropTipWizardProps } from './DropTipWizard'
import type { ERUtilsResults } from '../ErrorRecoveryFlows/hooks'

export type DropTipFlowsRoute = typeof DT_ROUTES[keyof typeof DT_ROUTES]
export type DropTipFlowsStep = DropTipFlowsRoute[number]

export interface ErrorDetails {
message: string
header?: string
Expand All @@ -30,14 +28,21 @@ interface ButtonOverrides {
tipDropComplete: (() => void) | null
}

export interface DropTipWizardRouteOverride {
route: DropTipFlowsRoute
step: DropTipFlowsStep | null
}

export interface FixitCommandTypeUtils {
runId: string
failedCommandId: string
trackCurrentMap: ERUtilsResults['trackExternalMap']
copyOverrides: CopyOverrides
errorOverrides: ErrorOverrides
buttonOverrides: ButtonOverrides
routeOverride?: typeof DT_ROUTES[keyof typeof DT_ROUTES]
/* Report to an external flow (ex, Error Recovery) the current step of drop tip wizard. */
reportMap: (dropTipMap: DropTipWizardRouteOverride | null) => void
/* If supplied, begin drop tip flows on the specified route & step. If no step is supplied, begin at the start of the route. */
routeOverride?: DropTipWizardRouteOverride
}

export type DropTipWizardContainerProps = DropTipWizardProps & {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ export function useDropTipFlowUtils({
tipStatusUtils,
failedCommand,
currentRecoveryOptionUtils,
trackExternalMap,
subMapUtils,
routeUpdateActions,
recoveryMap,
}: RecoveryContentProps): FixitCommandTypeUtils {
Expand All @@ -215,6 +215,7 @@ export function useDropTipFlowUtils({
const { step } = recoveryMap
const { selectedRecoveryOption } = currentRecoveryOptionUtils
const { proceedToRouteAndStep } = routeUpdateActions
const { updateSubMap, subMap } = subMapUtils
const failedCommandId = failedCommand?.id ?? '' // We should have a failed command here unless the run is not in AWAITING_RECOVERY.

const buildTipDropCompleteBtn = (): string => {
Expand Down Expand Up @@ -288,22 +289,28 @@ export function useDropTipFlowUtils({
}

// If a specific step within the DROP_TIP_FLOWS route is selected, begin the Drop Tip Flows at its related route.
//
// NOTE: The substep is cleared by drop tip wizard after the completion of the wizard flow.
const buildRouteOverride = (): FixitCommandTypeUtils['routeOverride'] => {
if (subMap?.route != null) {
return { route: subMap.route, step: subMap.step }
}

switch (step) {
case DROP_TIP_FLOWS.STEPS.CHOOSE_TIP_DROP:
return DT_ROUTES.DROP_TIP
return { route: DT_ROUTES.DROP_TIP, step: subMap?.step ?? null }
case DROP_TIP_FLOWS.STEPS.CHOOSE_BLOWOUT:
return DT_ROUTES.BLOWOUT
return { route: DT_ROUTES.BLOWOUT, step: subMap?.step ?? null }
}
}

return {
runId,
failedCommandId,
copyOverrides: buildCopyOverrides(),
trackCurrentMap: trackExternalMap,
errorOverrides: buildErrorOverrides(),
buttonOverrides: buildButtonOverrides(),
routeOverride: buildRouteOverride(),
reportMap: updateSubMap,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ describe('ManageTips', () => {
currentRecoveryOptionUtils: {
selectedRecoveryOption: null,
} as any,
subMapUtils: { subMap: null, updateSubMap: vi.fn() },
}

vi.mocked(DropTipWizardFlows).mockReturnValue(
Expand Down Expand Up @@ -176,13 +177,16 @@ describe('useDropTipFlowUtils', () => {
const mockRunId = 'MOCK_RUN_ID'
const mockTipStatusUtils = { runId: mockRunId }
const mockProceedToRouteAndStep = vi.fn()
const mockUpdateSubMap = vi.fn()
const { ERROR_WHILE_RECOVERING, DROP_TIP_FLOWS } = RECOVERY_MAP

const mockProps = {
tipStatusUtils: mockTipStatusUtils,
failedCommand: null,
previousRoute: null,
trackExternalMap: vi.fn(),
subMapUtils: {
updateSubMap: mockUpdateSubMap,
subMap: null,
},
currentRecoveryOptionUtils: {
selectedRecoveryOption: null,
} as any,
Expand Down Expand Up @@ -225,19 +229,13 @@ describe('useDropTipFlowUtils', () => {
screen.getByText('Proceed to cancel')
})

it('should call trackExternalMap with the current map', () => {
const mockTrackExternalMap = vi.fn()
const { result } = renderHook(() =>
useDropTipFlowUtils({
...mockProps,
trackExternalMap: mockTrackExternalMap,
})
)
it('should call updateSubMap with the current map', () => {
const { result } = renderHook(() => useDropTipFlowUtils(mockProps))

const currentMap = { route: 'route', step: 'step' }
result.current.trackCurrentMap(currentMap)
const currentMap = { route: 'route', step: 'step' } as any
result.current.reportMap(currentMap)

expect(mockTrackExternalMap).toHaveBeenCalledWith(currentMap)
expect(mockUpdateSubMap).toHaveBeenCalledWith(currentMap)
})

it('should return the correct error overrides', () => {
Expand Down Expand Up @@ -296,19 +294,43 @@ describe('useDropTipFlowUtils', () => {
)
})

it(`should return correct route overrides when the route is ${DROP_TIP_FLOWS.STEPS.CHOOSE_TIP_DROP}`, () => {
it(`should return correct route override when the step is ${DROP_TIP_FLOWS.STEPS.CHOOSE_TIP_DROP}`, () => {
const { result } = renderHook(() => useDropTipFlowUtils(mockProps))

expect(result.current.routeOverride).toEqual(DT_ROUTES.DROP_TIP)
expect(result.current.routeOverride).toEqual({
route: DT_ROUTES.DROP_TIP,
step: null,
})
})

it(`should return correct route overrides when the route is ${DROP_TIP_FLOWS.STEPS.CHOOSE_BLOWOUT}`, () => {
it(`should return correct route override when the step is ${DROP_TIP_FLOWS.STEPS.CHOOSE_BLOWOUT}`, () => {
const mockPropsBlowout = {
...mockProps,
recoveryMap: { step: DROP_TIP_FLOWS.STEPS.CHOOSE_BLOWOUT },
}
const { result } = renderHook(() => useDropTipFlowUtils(mockPropsBlowout))

expect(result.current.routeOverride).toEqual(DT_ROUTES.BLOWOUT)
expect(result.current.routeOverride).toEqual({
route: DT_ROUTES.BLOWOUT,
step: null,
})
})

it('should use subMap.step in routeOverride if available', () => {
const mockPropsWithSubMap = {
...mockProps,
subMapUtils: {
...mockProps.subMapUtils,
subMap: { route: DT_ROUTES.DROP_TIP, step: 'SOME_STEP' },
},
}
const { result } = renderHook(() =>
useDropTipFlowUtils(mockPropsWithSubMap)
)

expect(result.current.routeOverride).toEqual({
route: DT_ROUTES.DROP_TIP,
step: 'SOME_STEP',
})
})
})
2 changes: 1 addition & 1 deletion app/src/organisms/ErrorRecoveryFlows/__fixtures__/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export const mockRecoveryContentProps: RecoveryContentProps = {
deckMapUtils: { setSelectedLocation: () => {} } as any,
stepCounts: {} as any,
protocolAnalysis: mockRobotSideAnalysis,
trackExternalMap: () => null,
subMapUtils: { subMap: null, updateSubMap: () => null } as any,
hasLaunchedRecovery: true,
getRecoveryOptionCopy: () => 'MOCK_COPY',
commandsAfterFailedCommand: [
Expand Down
Loading

0 comments on commit c66e43e

Please sign in to comment.