Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(app): add robot restart alert for FF changes that require restart #4285

Merged
merged 2 commits into from
Oct 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions app/src/analytics/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import {
getRobotSystemType,
} from '../shell'

import { getRobotSettingsState, getPipettesState } from '../robot-api'
import { getRobotSettings } from '../robot-settings'
import { getPipettesState } from '../robot-api'

import hash from './hash'

Expand Down Expand Up @@ -77,7 +78,7 @@ export function getRobotAnalyticsData(state: State): RobotAnalyticsData | null {

if (robot) {
const pipettes = getPipettesState(state, robot.name)
const settings = getRobotSettingsState(state, robot.name)
const settings = getRobotSettings(state, robot.name)

return settings.reduce(
(result, setting) => ({
Expand Down
17 changes: 9 additions & 8 deletions app/src/components/RobotSettings/AdvancedSettingsCard.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import { Link } from 'react-router-dom'

import {
fetchSettings,
setSettings,
getRobotSettingsState,
} from '../../robot-api'
updateSetting,
getRobotSettings,
} from '../../robot-settings'

import { CONNECTABLE } from '../../discovery'
import { downloadLogs } from '../../shell/robot-logs/actions'
Expand All @@ -23,8 +23,9 @@ import {
} from '@opentrons/components'

import type { State, Dispatch } from '../../types'
import type { ViewableRobot } from '../../discovery'
import type { RobotSettings } from '../../robot-api'
import type { ViewableRobot } from '../../discovery/types'
import type { RobotSettings } from '../../robot-settings/types'

import UploadRobotUpdate from './UploadRobotUpdate'

type Props = {|
Expand Down Expand Up @@ -53,7 +54,7 @@ export default function AdvancedSettingsCard(props: Props) {
const { robot, resetUrl } = props
const { name, health, status } = robot
const settings = useSelector<State, RobotSettings>(state =>
getRobotSettingsState(state, name)
getRobotSettings(state, name)
)
const robotLogsDownloading = useSelector(getRobotLogsDownloading)
const dispatch = useDispatch<Dispatch>()
Expand All @@ -64,7 +65,7 @@ export default function AdvancedSettingsCard(props: Props) {
s => s.id === ROBOT_LOGS_OPTOUT_ID && s.value === null
)
const setLogOptout = (value: boolean) =>
dispatch(setSettings(robot, { id: ROBOT_LOGS_OPTOUT_ID, value }))
dispatch(updateSetting(robot, ROBOT_LOGS_OPTOUT_ID, value))

React.useEffect(() => {
dispatch(fetchSettings(robot))
Expand Down Expand Up @@ -102,7 +103,7 @@ export default function AdvancedSettingsCard(props: Props) {
key={id}
label={title}
toggledOn={value === true}
onClick={() => dispatch(setSettings(robot, { id, value: !value }))}
onClick={() => dispatch(updateSetting(robot, id, !value))}
>
<p>{description}</p>
</LabeledToggle>
Expand Down
47 changes: 47 additions & 0 deletions app/src/components/RobotSettings/RestartRequiredBanner.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// @flow
import * as React from 'react'
import { useDispatch } from 'react-redux'
import { AlertItem, OutlineButton } from '@opentrons/components'

import { restartRobot } from '../../robot-admin'
import styles from './styles.css'

import type { Dispatch } from '../../types'
import type { RobotHost } from '../../robot-api/types'

export type RestartRequiredBannerProps = {|
robot: RobotHost,
|}

// TODO(mc, 2019-10-24): i18n
const TITLE = 'Robot restart required'
const MESSAGE =
'You must restart your robot for your settings changes to take effect'
const RESTART_NOW = 'Restart Now'

function RestartRequiredBanner(props: RestartRequiredBannerProps) {
const { robot } = props
const [dismissed, setDismissed] = React.useState(false)
const dispatch = useDispatch<Dispatch>()
const restart = React.useCallback(() => dispatch(restartRobot(robot)), [
dispatch,
robot,
])

if (dismissed) return null

return (
<AlertItem
type="warning"
onCloseClick={() => setDismissed(true)}
title={TITLE}
>
<div className={styles.restart_banner_message}>
<p>{MESSAGE}</p>
<OutlineButton onClick={restart}>{RESTART_NOW}</OutlineButton>
</div>
</AlertItem>
)
}

export default RestartRequiredBanner
9 changes: 9 additions & 0 deletions app/src/components/RobotSettings/styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,12 @@
position: fixed;
clip: rect(1px 1px 1px 1px);
}

.restart_banner_message {
display: flex;

& > p {
max-width: 24rem;
margin-right: auto;
}
}
2 changes: 2 additions & 0 deletions app/src/epic.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ import { analyticsEpic } from './analytics'
import { discoveryEpic } from './discovery/epic'
import { robotApiEpic } from './robot-api'
import { robotAdminEpic } from './robot-admin/epic'
import { robotSettingsEpic } from './robot-settings/epic'
import { shellEpic } from './shell'

export default combineEpics(
analyticsEpic,
discoveryEpic,
robotApiEpic,
robotAdminEpic,
robotSettingsEpic,
shellEpic
)
5 changes: 3 additions & 2 deletions app/src/pages/Calibrate/Labware.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { push } from 'connected-react-router'

import { selectors as robotSelectors } from '../../robot'
import { getConnectedRobot } from '../../discovery'
import { getRobotSettingsState, type Module } from '../../robot-api'
import { getRobotSettings } from '../../robot-settings'
import { getUnpreparedModules } from '../../robot-api/resources/modules'

import Page from '../../components/Page'
Expand All @@ -22,6 +22,7 @@ import type { ContextRouter } from 'react-router-dom'
import type { State, Dispatch } from '../../types'
import type { Labware } from '../../robot'
import type { Robot } from '../../discovery'
import type { Module } from '../../robot-api'

type OP = ContextRouter

Expand Down Expand Up @@ -106,7 +107,7 @@ function mapStateToProps(state: State, ownProps: OP): SP {
const hasModulesLeftToReview =
modules.length > 0 && !robotSelectors.getModulesReviewed(state)
const robot = getConnectedRobot(state)
const settings = robot && getRobotSettingsState(state, robot.name)
const settings = robot && getRobotSettings(state, robot.name)

// TODO(mc, 2018-07-23): make diagram component a container
const calToBottomFlag =
Expand Down
11 changes: 9 additions & 2 deletions app/src/pages/Robots/RobotSettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {

import { makeGetRobotHome, clearHomeResponse } from '../../http-api-client'
import { getRobotRestarting } from '../../robot-admin'
import { getRobotRestartRequired } from '../../robot-settings'

import { SpinnerModalPage } from '@opentrons/components'
import { ErrorModal } from '../../components/modals'
Expand All @@ -29,6 +30,7 @@ import UpdateBuildroot from '../../components/RobotSettings/UpdateBuildroot'
import CalibrateDeck from '../../components/CalibrateDeck'
import ConnectBanner from '../../components/RobotSettings/ConnectBanner'
import ReachableRobotBanner from '../../components/RobotSettings/ReachableRobotBanner'
import RestartRequiredBanner from '../../components/RobotSettings/RestartRequiredBanner'
import ResetRobotModal from '../../components/RobotSettings/ResetRobotModal'

import type { ContextRouter } from 'react-router-dom'
Expand All @@ -48,6 +50,7 @@ type SP = {|
homeInProgress: ?boolean,
homeError: ?Error,
updateInProgress: boolean,
restartRequired: boolean,
restarting: boolean,
|}

Expand Down Expand Up @@ -82,6 +85,7 @@ function RobotSettingsPage(props: Props) {
closeConnectAlert,
showUpdateModal,
updateInProgress,
restartRequired,
restarting,
match: { path, url },
} = props
Expand All @@ -103,6 +107,9 @@ function RobotSettingsPage(props: Props) {
{robot.status === CONNECTABLE && (
<ConnectBanner {...robot} key={Number(robot.connected)} />
)}
{restartRequired && !restarting && (
<RestartRequiredBanner robot={robot} />
)}

<RobotSettings
robot={robot}
Expand Down Expand Up @@ -195,7 +202,6 @@ function makeMapStateToProps(): (state: State, ownProps: OP) => SP {
const buildrootUpdateType = getBuildrootUpdateAvailable(state, robot)
const updateInProgress = getBuildrootUpdateInProgress(state, robot)
const currentBrRobot = getBuildrootRobot(state)
const restarting = getRobotRestarting(state, robot.name)

const showUpdateModal =
updateInProgress ||
Expand All @@ -205,7 +211,8 @@ function makeMapStateToProps(): (state: State, ownProps: OP) => SP {

return {
updateInProgress,
restarting,
restarting: getRobotRestarting(state, robot.name),
restartRequired: getRobotRestartRequired(state, robot.name),
showUpdateModal: !!showUpdateModal,
homeInProgress: homeRequest && homeRequest.inProgress,
homeError: homeRequest && homeRequest.error,
Expand Down
4 changes: 4 additions & 0 deletions app/src/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import { robotApiReducer } from './robot-api'
// robot administration state
import { robotAdminReducer } from './robot-admin/reducer'

// robot settings state
import { robotSettingsReducer } from './robot-settings/reducer'

// app shell state
import { shellReducer } from './shell'

Expand All @@ -42,6 +45,7 @@ const rootReducer: Reducer<State, Action> = combineReducers<_, Action>({
api: apiReducer,
robotApi: robotApiReducer,
robotAdmin: robotAdminReducer,
robotSettings: robotSettingsReducer,
config: configReducer,
discovery: discoveryReducer,
labware: customLabwareReducer,
Expand Down
31 changes: 29 additions & 2 deletions app/src/robot-admin/__tests__/epic.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { TestScheduler } from 'rxjs/testing'

import * as ApiUtils from '../../robot-api/utils'
import * as SettingsSelectors from '../../robot-settings/selectors'
import * as DiscoveryActions from '../../discovery/actions'
import * as Actions from '../actions'
import { robotAdminEpic } from '../epic'
Expand All @@ -14,6 +15,7 @@ import type {
} from '../../robot-api/types'

jest.mock('../../robot-api/utils')
jest.mock('../../robot-settings/selectors')

const mockMakeApiRequest: JestMockFn<[RobotApiRequest, RequestMeta], mixed> =
ApiUtils.makeRobotApiRequest
Expand All @@ -23,7 +25,11 @@ const mockPassRobotApiResponseAction: JestMockFn<
RobotApiResponseAction | null
> = ApiUtils.passRobotApiResponseAction

const mockGetRestartPath: JestMockFn<Array<any>, string | null> =
SettingsSelectors.getRobotRestartPath

const mockRobot = { name: 'robot', ip: '127.0.0.1', port: 31950 }
const mockState = { mock: true }

const setupMockMakeApiRequest = cold => {
mockMakeApiRequest.mockImplementation((req, meta) =>
Expand All @@ -49,9 +55,10 @@ describe('robotAdminEpic', () => {

testScheduler.run(({ hot, cold, expectObservable }) => {
setupMockMakeApiRequest(cold)
mockGetRestartPath.mockReturnValue(null)

const action$ = hot('-a', { a: action })
const state$: any = null
const state$ = hot('a-', { a: mockState })
const output$ = robotAdminEpic(action$, state$)

expectObservable(output$).toBe('--a', {
Expand All @@ -63,6 +70,26 @@ describe('robotAdminEpic', () => {
})
})

test('makes a POST to the settings restart path on RESTART is applicable', () => {
const action = Actions.restartRobot(mockRobot)

testScheduler.run(({ hot, cold, expectObservable }) => {
setupMockMakeApiRequest(cold)
mockGetRestartPath.mockReturnValue('/restart')

const action$ = hot('-a', { a: action })
const state$ = hot('a-', { a: mockState })
const output$ = robotAdminEpic(action$, state$)

expectObservable(output$).toBe('--a', {
a: {
req: { host: mockRobot, method: 'POST', path: '/restart' },
meta: {},
},
})
})
})

test('starts discovery on restart request success', () => {
testScheduler.run(({ hot, expectObservable }) => {
const serverSuccessAction = {
Expand All @@ -81,7 +108,7 @@ describe('robotAdminEpic', () => {
mockPassRobotApiResponseAction.mockReturnValue(serverSuccessAction)

const action$ = hot('-a', { a: serverSuccessAction })
const state$: any = null
const state$ = hot('a-', { a: mockState })
const output$ = robotAdminEpic(action$, state$)

expectObservable(output$).toBe('-a', {
Expand Down
2 changes: 1 addition & 1 deletion app/src/robot-admin/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ import type { RobotAdminAction } from './types'

export const restartRobot = (host: RobotHost): RobotAdminAction => ({
type: RESTART,
payload: { host, method: POST, path: RESTART_PATH },
payload: { host, path: RESTART_PATH, method: POST },
meta: { robot: true },
})
24 changes: 16 additions & 8 deletions app/src/robot-admin/epic.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,35 @@
// @flow
import { of } from 'rxjs'
import { ofType, combineEpics } from 'redux-observable'
import { filter, switchMap } from 'rxjs/operators'
import { filter, switchMap, withLatestFrom } from 'rxjs/operators'

import {
makeRobotApiRequest,
passRobotApiResponseAction,
} from '../robot-api/utils'
import { startDiscovery } from '../discovery/actions'
import { startDiscovery } from '../discovery'
import { getRobotRestartPath } from '../robot-settings'
import { RESTART, RESTART_PATH } from './constants'

import type { Epic, LooseEpic } from '../types'
import type { RequestMeta, RobotApiResponseAction } from '../robot-api/types'
import type { State, Epic, LooseEpic } from '../types'
import type { RobotApiResponseAction } from '../robot-api/types'
import type { RobotAdminAction } from './types'

export const RESTART_DISCOVERY_TIMEOUT_MS = 60000

const robotAdminApiEpic: Epic = action$ => {
const robotAdminApiEpic: Epic = (action$, state$) => {
return action$.pipe(
ofType(RESTART),
switchMap<RobotAdminAction, _, _>(a => {
const meta: RequestMeta = {}
return makeRobotApiRequest(a.payload, meta)
withLatestFrom(state$),
switchMap<[RobotAdminAction, State], _, _>(([action, state]) => {
const { name: robotName } = action.payload.host
const restartPath = getRobotRestartPath(state, robotName)
const payload =
restartPath !== null
? { ...action.payload, path: restartPath }
: action.payload

return makeRobotApiRequest(payload, {})
})
)
}
Expand Down
Loading