Skip to content

Commit

Permalink
fix(app): Persist UpdateBuildroot during robot restart (#13740)
Browse files Browse the repository at this point in the history
* feat(app): Wire up nice-modal-react

Adds support for Nice-Modal-React.

* fix(app): fix closing robot update flows during restart step

Relocates UpdateBuildroot rendering to the App component, eliminating certain conditional statements
in ancestor components that would abruptly close UpdateBuildroot.
  • Loading branch information
mjhuff authored Oct 11, 2023
1 parent 27e594a commit 3b92b1a
Show file tree
Hide file tree
Showing 13 changed files with 107 additions and 144 deletions.
1 change: 1 addition & 0 deletions app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
},
"homepage": "https://github.com/Opentrons/opentrons",
"dependencies": {
"@ebay/nice-modal-react": "1.2.13",
"@fontsource/dejavu-sans": "5.0.3",
"@fontsource/public-sans": "5.0.3",
"@hot-loader/react-dom": "17.0.1",
Expand Down
5 changes: 4 additions & 1 deletion app/src/App/index.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react'
import { useSelector } from 'react-redux'
import { hot } from 'react-hot-loader/root'
import NiceModal from '@ebay/nice-modal-react'

import { Flex, POSITION_FIXED, DIRECTION_ROW } from '@opentrons/components'

Expand Down Expand Up @@ -29,7 +30,9 @@ export const AppComponent = (): JSX.Element | null => {
onDrop={stopEvent}
>
<TopPortalRoot />
{isOnDevice ? <OnDeviceDisplayApp /> : <DesktopApp />}
<NiceModal.Provider>
{isOnDevice ? <OnDeviceDisplayApp /> : <DesktopApp />}
</NiceModal.Provider>
</Flex>
</>
) : null
Expand Down
18 changes: 2 additions & 16 deletions app/src/organisms/Devices/RobotOverviewOverflowMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { Divider } from '../../atoms/structure'
import { Tooltip } from '../../atoms/Tooltip'
import { ChooseProtocolSlideout } from '../../organisms/ChooseProtocolSlideout'
import { DisconnectModal } from '../../organisms/Devices/RobotSettings/ConnectNetwork/DisconnectModal'
import { UpdateBuildroot } from '../../organisms/Devices/RobotSettings/UpdateBuildroot'
import { handleUpdateBuildroot } from '../../organisms/Devices/RobotSettings/UpdateBuildroot'
import { useCurrentRunId } from '../../organisms/ProtocolUpload/hooks'
import { getRobotUpdateDisplayInfo } from '../../redux/robot-update'
import { UNREACHABLE, CONNECTABLE, REACHABLE } from '../../redux/discovery'
Expand Down Expand Up @@ -65,10 +65,6 @@ export const RobotOverviewOverflowMenu = (
dispatch(home(robot.name, ROBOT))
}

const [
showSoftwareUpdateModal,
setShowSoftwareUpdateModal,
] = React.useState<boolean>(false)
const [
showChooseProtocolSlideout,
setShowChooseProtocolSlideout,
Expand All @@ -87,10 +83,6 @@ export const RobotOverviewOverflowMenu = (
dispatch(checkShellUpdate())
})

const handleClickUpdateBuildroot: React.MouseEventHandler = () => {
setShowSoftwareUpdateModal(true)
}

const handleClickRun: React.MouseEventHandler<HTMLButtonElement> = () => {
setShowChooseProtocolSlideout(true)
}
Expand All @@ -105,12 +97,6 @@ export const RobotOverviewOverflowMenu = (
return (
<Flex data-testid="RobotOverview_overflowMenu" position={POSITION_RELATIVE}>
<Portal level="top">
{showSoftwareUpdateModal ? (
<UpdateBuildroot
robot={robot}
close={() => setShowSoftwareUpdateModal(false)}
/>
) : null}
{showDisconnectModal ? (
<DisconnectModal
onCancel={() => setShowDisconnectModal(false)}
Expand Down Expand Up @@ -138,7 +124,7 @@ export const RobotOverviewOverflowMenu = (
>
{isRobotOnWrongVersionOfSoftware && !isRobotUnavailable ? (
<MenuItem
onClick={handleClickUpdateBuildroot}
onClick={() => handleUpdateBuildroot(robot)}
data-testid={`RobotOverviewOverflowMenu_updateSoftware_${String(
robot.name
)}`}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { getRobotApiVersion } from '../../../../redux/discovery'
import { getRobotUpdateDisplayInfo } from '../../../../redux/robot-update'
import { UpdateRobotBanner } from '../../../UpdateRobotBanner'
import { useIsOT3, useRobot } from '../../hooks'
import { UpdateBuildroot } from '../UpdateBuildroot'
import { handleUpdateBuildroot } from '../UpdateBuildroot'

import type { State } from '../../../../redux/types'

Expand All @@ -35,7 +35,6 @@ export function RobotServerVersion({
const { t } = useTranslation(['device_settings', 'shared'])
const robot = useRobot(robotName)
const isOT3 = useIsOT3(robotName)
const [showVersionInfoModal, setShowVersionInfoModal] = React.useState(false)
const { autoUpdateAction } = useSelector((state: State) => {
return getRobotUpdateDisplayInfo(state, robotName)
})
Expand All @@ -45,12 +44,6 @@ export function RobotServerVersion({

return (
<>
{showVersionInfoModal ? (
<UpdateBuildroot
robot={robot}
close={() => setShowVersionInfoModal(false)}
/>
) : null}
{autoUpdateAction !== 'reinstall' && robot != null ? (
<Box marginBottom={SPACING.spacing16} width="100%">
<UpdateRobotBanner robot={robot} />
Expand Down Expand Up @@ -95,7 +88,7 @@ export function RobotServerVersion({
{t('up_to_date')}
</StyledText>
<TertiaryButton
onClick={() => setShowVersionInfoModal(true)}
onClick={() => handleUpdateBuildroot(robot)}
textTransform={TYPOGRAPHY.textTransformCapitalize}
>
{t('reinstall')}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { LegacyModal } from '../../../../molecules/LegacyModal'
import { CONNECTABLE, REACHABLE } from '../../../../redux/discovery'
import { Divider } from '../../../../atoms/structure'
import { useRobot } from '../../hooks'
import { UpdateBuildroot } from '../UpdateBuildroot'
import { handleUpdateBuildroot } from '../UpdateBuildroot'

const TECHNICAL_CHANGE_LOG_URL =
'https://github.com/Opentrons/opentrons/blob/edge/CHANGELOG.md'
Expand Down Expand Up @@ -50,22 +50,9 @@ export function SoftwareUpdateModal({
const [showUpdateModal, setShowUpdateModal] = React.useState<boolean>(false)
const robot = useRobot(robotName)

const handleCloseModal = (): void => {
setShowUpdateModal(false)
closeModal()
}

const handleLaunchUpdateModal: React.MouseEventHandler = e => {
e.preventDefault()
e.stopPropagation()
setShowUpdateModal(true)
}

if (robot?.status !== CONNECTABLE && robot?.status !== REACHABLE) return null

return showUpdateModal ? (
<UpdateBuildroot robot={robot} close={handleCloseModal} />
) : (
return !showUpdateModal ? (
<LegacyModal title={t('robot_update_available')} onClose={closeModal}>
<Banner type="informing">{t('requires_restarting_the_robot')}</Banner>
<Flex flexDirection={DIRECTION_COLUMN} marginTop={SPACING.spacing16}>
Expand Down Expand Up @@ -119,13 +106,16 @@ export function SoftwareUpdateModal({
{t('remind_me_later')}
</SecondaryButton>
<PrimaryButton
onClick={handleLaunchUpdateModal}
onClick={() => {
setShowUpdateModal(true)
handleUpdateBuildroot(robot)
}}
disabled={currentRunId != null}
>
{t('update_robot_now')}
</PrimaryButton>
</Flex>
</Flex>
</LegacyModal>
)
) : null
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { getRobotApiVersion } from '../../../../../redux/discovery'
import { getRobotUpdateDisplayInfo } from '../../../../../redux/robot-update'
import { mockConnectableRobot } from '../../../../../redux/discovery/__fixtures__'
import { useRobot } from '../../../hooks'
import { UpdateBuildroot } from '../../UpdateBuildroot'
import { handleUpdateBuildroot } from '../../UpdateBuildroot'
import { RobotServerVersion } from '../RobotServerVersion'

jest.mock('../../../hooks')
Expand All @@ -24,8 +24,8 @@ const mockGetBuildrootUpdateDisplayInfo = getRobotUpdateDisplayInfo as jest.Mock

const mockUseRobot = useRobot as jest.MockedFunction<typeof useRobot>

const mockUpdateBuildroot = UpdateBuildroot as jest.MockedFunction<
typeof UpdateBuildroot
const mockUpdateBuildroot = handleUpdateBuildroot as jest.MockedFunction<
typeof handleUpdateBuildroot
>

const MOCK_ROBOT_VERSION = '7.7.7'
Expand All @@ -40,7 +40,6 @@ const render = () => {

describe('RobotSettings RobotServerVersion', () => {
beforeEach(() => {
mockUpdateBuildroot.mockReturnValue(<div>mock update buildroot</div>)
mockUseRobot.mockReturnValue(mockConnectableRobot)
mockGetBuildrootUpdateDisplayInfo.mockReturnValue({
autoUpdateAction: 'reinstall',
Expand All @@ -66,7 +65,7 @@ describe('RobotSettings RobotServerVersion', () => {
getByText('up to date')
const reinstall = getByRole('button', { name: 'reinstall' })
fireEvent.click(reinstall)
getByText('mock update buildroot')
expect(mockUpdateBuildroot).toHaveBeenCalled()
})

it('should render the warning message if the robot server version needs to upgrade', () => {
Expand All @@ -81,7 +80,7 @@ describe('RobotSettings RobotServerVersion', () => {
)
const btn = getByText('View update')
fireEvent.click(btn)
getByText('mock update buildroot')
expect(mockUpdateBuildroot).toHaveBeenCalled()
})

it('should render the warning message if the robot server version needs to downgrade', () => {
Expand All @@ -96,7 +95,7 @@ describe('RobotSettings RobotServerVersion', () => {
)
const btn = getByText('View update')
fireEvent.click(btn)
getByText('mock update buildroot')
expect(mockUpdateBuildroot).toHaveBeenCalled()
})

it('the link should have the correct href', () => {
Expand Down
14 changes: 2 additions & 12 deletions app/src/organisms/Devices/RobotSettings/RobotSettingsAdvanced.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import {
import { RenameRobotSlideout } from './AdvancedTab/AdvancedTabSlideouts/RenameRobotSlideout'
import { DeviceResetSlideout } from './AdvancedTab/AdvancedTabSlideouts/DeviceResetSlideout'
import { DeviceResetModal } from './AdvancedTab/AdvancedTabSlideouts/DeviceResetModal'
import { UpdateBuildroot } from './UpdateBuildroot'
import { handleUpdateBuildroot } from './UpdateBuildroot'
import { UNREACHABLE } from '../../../redux/discovery'
import { Portal } from '../../../App/portal'

Expand Down Expand Up @@ -70,10 +70,6 @@ export function RobotSettingsAdvanced({
showDeviceResetModal,
setShowDeviceResetModal,
] = React.useState<boolean>(false)
const [
showSoftwareUpdateModal,
setShowSoftwareUpdateModal,
] = React.useState<boolean>(false)

const isRobotBusy = useIsRobotBusy({ poll: true })

Expand Down Expand Up @@ -124,12 +120,6 @@ export function RobotSettingsAdvanced({

return (
<>
{showSoftwareUpdateModal ? (
<UpdateBuildroot
robot={robot}
close={() => setShowSoftwareUpdateModal(false)}
/>
) : null}
<Box>
{showRenameRobotSlideout && (
<RenameRobotSlideout
Expand Down Expand Up @@ -194,7 +184,7 @@ export function RobotSettingsAdvanced({
<UpdateRobotSoftware
robotName={robotName}
isRobotBusy={isRobotBusy}
onUpdateStart={() => setShowSoftwareUpdateModal(true)}
onUpdateStart={() => handleUpdateBuildroot(robot)}
/>
<Troubleshooting robotName={robotName} />
<Divider marginY={SPACING.spacing16} />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React from 'react'
import NiceModal from '@ebay/nice-modal-react'

import { mockConnectableRobot as mockRobot } from '../../../../../redux/discovery/__fixtures__'
import * as RobotUpdate from '../../../../../redux/robot-update'

import { mountWithStore, WrapperWithStore } from '@opentrons/components'
import { UpdateBuildroot } from '..'
import { handleUpdateBuildroot } from '..'
import { ViewUpdateModal } from '../ViewUpdateModal'
import { RobotUpdateProgressModal } from '../RobotUpdateProgressModal'

Expand Down Expand Up @@ -33,12 +34,16 @@ const getRobotSystemType = RobotUpdate.getRobotSystemType as jest.MockedFunction
const MOCK_STATE: State = { mockState: true } as any

describe('UpdateBuildroot', () => {
const closeModal = jest.fn()
const render = (): WrapperWithStore<
React.ComponentProps<typeof UpdateBuildroot>
React.ComponentProps<typeof NiceModal.Provider>
> => {
return mountWithStore<React.ComponentProps<typeof UpdateBuildroot>>(
<UpdateBuildroot robot={mockRobot} close={closeModal} />,
return mountWithStore<React.ComponentProps<typeof NiceModal.Provider>>(
<NiceModal.Provider>
<button
onClick={() => handleUpdateBuildroot(mockRobot)}
id="testButton"
/>
</NiceModal.Provider>,
{ initialState: MOCK_STATE }
)
}
Expand All @@ -53,7 +58,8 @@ describe('UpdateBuildroot', () => {
})

it('should set the update as seen on mount', () => {
const { store } = render()
const { store, wrapper } = render()
wrapper.find('#testButton').simulate('click')

expect(store.dispatch).toHaveBeenCalledWith(
RobotUpdate.setRobotUpdateSeen(mockRobot.name)
Expand All @@ -62,14 +68,11 @@ describe('UpdateBuildroot', () => {

it('renders a ViewUpdateModal if no session exists', () => {
const { wrapper } = render()
wrapper.find('#testButton').simulate('click')
const viewUpdate = wrapper.find(ViewUpdateModal)

expect(viewUpdate.prop('robotName')).toBe(mockRobot.name)
expect(viewUpdate.prop('robot')).toBe(mockRobot)

expect(closeModal).not.toHaveBeenCalled()
viewUpdate.invoke('closeModal')?.()
expect(closeModal).toHaveBeenCalled()
})

it('renders RobotUpdateProgressModal if session exists', () => {
Expand All @@ -86,6 +89,7 @@ describe('UpdateBuildroot', () => {
getRobotUpdateSession.mockReturnValue(mockSession)

const { wrapper } = render()
wrapper.find('#testButton').simulate('click')
const progressModal = wrapper.find(RobotUpdateProgressModal)

expect(progressModal.prop('robotName')).toBe(mockRobot.name)
Expand Down
Loading

0 comments on commit 3b92b1a

Please sign in to comment.