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

#4285)

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

* fixup: Defer to API for restart required state

app/src/analytics/selectors.js

@@ -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'
 
@@ -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) => ({

app/src/components/RobotSettings/AdvancedSettingsCard.js

@@ -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'
@@ -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 = {|
@@ -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>()
@@ -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))
@@ -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>

app/src/components/RobotSettings/RestartRequiredBanner.js

@@ -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

app/src/components/RobotSettings/styles.css

@@ -85,3 +85,12 @@
   position: fixed;
   clip: rect(1px 1px 1px 1px);
 }
+
+.restart_banner_message {
+  display: flex;
+
+  & > p {
+    max-width: 24rem;
+    margin-right: auto;
+  }
+}

app/src/epic.js

@@ -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
 )

app/src/pages/Calibrate/Labware.js

@@ -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'
@@ -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
 
@@ -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 =

app/src/pages/Robots/RobotSettings.js

@@ -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'
@@ -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'
@@ -48,6 +50,7 @@ type SP = {|
   homeInProgress: ?boolean,
   homeError: ?Error,
   updateInProgress: boolean,
+  restartRequired: boolean,
   restarting: boolean,
 |}
 
@@ -82,6 +85,7 @@ function RobotSettingsPage(props: Props) {
     closeConnectAlert,
     showUpdateModal,
     updateInProgress,
+    restartRequired,
     restarting,
     match: { path, url },
   } = props
@@ -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}
@@ -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 ||
@@ -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,

app/src/reducer.js

@@ -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'
 
@@ -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,

app/src/robot-admin/__tests__/epic.test.js

@@ -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'
@@ -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
@@ -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) =>
@@ -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', {
@@ -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 = {
@@ -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', {

app/src/robot-admin/actions.js

@@ -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 },
 })

app/src/robot-admin/epic.js

@@ -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, {})
     })
   )
 }