feat(app): Track restart status in discovery state for better alerts (#…

…2639)

Closes #2516

app/src/components/RobotSettings/ConnectBanner.js

@@ -13,16 +13,15 @@ export default class ConnectBanner extends React.Component<Robot, State> {
   }
 
   render () {
-    const {name, connected} = this.props
+    const {displayName, connected} = this.props
     const isVisible = connected && !this.state.dismissed
-    const TITLE = `${name} successfully connected`
-
     if (!isVisible) return null
+
     return (
       <AlertItem
         type="success"
         onCloseClick={() => this.setState({dismissed: true})}
-        title={TITLE}
+        title={`${displayName} successfully connected`}
       />
     )
   }

app/src/components/RobotSettings/ReachableRobotBanner.js

@@ -7,7 +7,21 @@ import type {ReachableRobot} from '../../discovery'
 
 type State = {dismissed: boolean}
 
-const TITLE = 'Unable to establish connection with robot'
+const UNRESPONSIVE_TITLE = 'Unable to establish connection with robot'
+const RESTARTING_TITLE = 'Robot restarting'
+
+const LAST_RESORT = (
+  <p>
+    If your robot remains unresponsive, please reach out to our Support team.
+  </p>
+)
+
+const RESTARTING_MESSAGE = (
+  <div className={styles.banner}>
+    <p>Your robot is restarting, and should be back online shortly.</p>
+    {LAST_RESORT}
+  </div>
+)
 
 const NO_SERVER_MESSAGE = (
   <div className={styles.banner}>
@@ -16,9 +30,7 @@ const NO_SERVER_MESSAGE = (
       requests.
     </p>
     <p>We recommend power-cycling your robot.</p>
-    <p>
-      If your robot remains unresponsive, please reach out to our Support team.
-    </p>
+    {LAST_RESORT}
   </div>
 )
 
@@ -40,9 +52,7 @@ const SERVER_MESSAGE = (
         </p>
       </li>
     </ol>
-    <p>
-      If your robot remains unresponsive, please reach out to our Support team.
-    </p>
+    {LAST_RESORT}
   </div>
 )
 
@@ -56,17 +66,25 @@ export default class ReachableRobotBanner extends React.Component<
   }
 
   render () {
-    const {serverOk} = this.props
+    const {serverOk, restartStatus} = this.props
     const isVisible = !this.state.dismissed
-    const message = serverOk ? SERVER_MESSAGE : NO_SERVER_MESSAGE
+    let title = ''
+    let message = ''
+    if (restartStatus) {
+      title = RESTARTING_TITLE
+      message = RESTARTING_MESSAGE
+    } else {
+      title = UNRESPONSIVE_TITLE
+      message = serverOk ? SERVER_MESSAGE : NO_SERVER_MESSAGE
+    }
 
     if (!isVisible) return null
 
     return (
       <AlertItem
         type="warning"
         onCloseClick={() => this.setState({dismissed: true})}
-        title={TITLE}
+        title={title}
       >
         {message}
       </AlertItem>

app/src/discovery/__tests__/actions.test.js

@@ -30,4 +30,15 @@ describe('discovery actions', () => {
     jest.runTimersToTime(expectedTimeout)
     expect(store.getActions()).toEqual([expectedStart, expectedFinish])
   })
+
+  test('startDiscovery with timeout', () => {
+    const expectedTimeout = 60000
+    const expectedStart = {type: 'discovery:START', meta: {shell: true}}
+    const expectedFinish = {type: 'discovery:FINISH', meta: {shell: true}}
+
+    store.dispatch(startDiscovery(60000))
+    expect(store.getActions()).toEqual([expectedStart])
+    jest.runTimersToTime(expectedTimeout)
+    expect(store.getActions()).toEqual([expectedStart, expectedFinish])
+  })
 })

app/src/discovery/__tests__/reducer.test.js

@@ -2,10 +2,10 @@
 import {discoveryReducer} from '..'
 
 jest.mock('../../shell', () => ({
-  getShellRobots: () => ([
+  getShellRobots: () => [
     {name: 'foo', ip: '192.168.1.1', port: 31950},
     {name: 'bar', ip: '192.168.1.2', port: 31950},
-  ]),
+  ],
 }))
 
 describe('discoveryReducer', () => {
@@ -20,6 +20,7 @@ describe('discoveryReducer', () => {
       initialState: undefined,
       expectedState: {
         scanning: false,
+        restartsByName: {},
         robotsByName: {
           foo: [{name: 'foo', ip: '192.168.1.1', port: 31950}],
           bar: [{name: 'bar', ip: '192.168.1.2', port: 31950}],
@@ -49,12 +50,54 @@ describe('discoveryReducer', () => {
           ],
         },
       },
-      initialState: {robotsByName: {}},
+      initialState: {robotsByName: {}, restartsByName: {}},
       expectedState: {
         robotsByName: {
           foo: [{name: 'foo', ip: '192.168.1.1', port: 31950}],
           bar: [{name: 'bar', ip: '192.168.1.2', port: 31950}],
         },
+        restartsByName: {},
+      },
+    },
+    {
+      name: 'api:SERVER_SUCCESS sets restart pending',
+      action: {
+        type: 'api:SERVER_SUCCESS',
+        payload: {path: 'restart', robot: {name: 'name'}},
+      },
+      initialState: {restartsByName: {}},
+      expectedState: {restartsByName: {name: 'pending'}},
+    },
+    {
+      name: 'discovery:UPDATE_LIST sets restart down if pending robot not ok',
+      action: {
+        type: 'discovery:UPDATE_LIST',
+        payload: {
+          robots: [{name: 'name', ip: '192.168.1.1', port: 31950, ok: false}],
+        },
+      },
+      initialState: {restartsByName: {name: 'pending'}},
+      expectedState: {
+        robotsByName: {
+          name: [{name: 'name', ip: '192.168.1.1', port: 31950, ok: false}],
+        },
+        restartsByName: {name: 'down'},
+      },
+    },
+    {
+      name: 'discovery:UPDATE_LIST clears restart if down robot ok',
+      action: {
+        type: 'discovery:UPDATE_LIST',
+        payload: {
+          robots: [{name: 'name', ip: '192.168.1.1', port: 31950, ok: true}],
+        },
+      },
+      initialState: {restartsByName: {name: 'down'}},
+      expectedState: {
+        robotsByName: {
+          name: [{name: 'name', ip: '192.168.1.1', port: 31950, ok: true}],
+        },
+        restartsByName: {name: null},
       },
     },
   ]

app/src/discovery/__tests__/selectors.test.js

@@ -6,7 +6,8 @@ const makeFullyUp = (
   ip,
   status = null,
   connected = null,
-  displayName = null
+  displayName = null,
+  restartStatus = null
 ) => ({
   name,
   ip,
@@ -19,14 +20,16 @@ const makeFullyUp = (
   status,
   connected,
   displayName,
+  restartStatus,
 })
 
 const makeConnectable = (
   name,
   ip,
   status = null,
   connected = null,
-  displayName = null
+  displayName = null,
+  restartStatus = null
 ) => ({
   name,
   ip,
@@ -37,9 +40,16 @@ const makeConnectable = (
   status,
   connected,
   displayName,
+  restartStatus,
 })
 
-const makeAdvertising = (name, ip, status = null, displayName = null) => ({
+const makeAdvertising = (
+  name,
+  ip,
+  status = null,
+  displayName = null,
+  restartStatus = null
+) => ({
   name,
   ip,
   local: false,
@@ -48,14 +58,16 @@ const makeAdvertising = (name, ip, status = null, displayName = null) => ({
   advertising: true,
   status,
   displayName,
+  restartStatus,
 })
 
 const makeServerUp = (
   name,
   ip,
   advertising,
   status = null,
-  displayName = null
+  displayName = null,
+  restartStatus = null
 ) => ({
   name,
   ip,
@@ -66,9 +78,16 @@ const makeServerUp = (
   serverHealth: {},
   status,
   displayName,
+  restartStatus,
 })
 
-const makeUnreachable = (name, ip, status = null, displayName = null) => ({
+const makeUnreachable = (
+  name,
+  ip,
+  status = null,
+  displayName = null,
+  restartStatus = null
+) => ({
   name,
   ip,
   local: false,
@@ -77,6 +96,7 @@ const makeUnreachable = (name, ip, status = null, displayName = null) => ({
   advertising: false,
   status,
   displayName,
+  restartStatus,
 })
 
 describe('discovery selectors', () => {
@@ -130,6 +150,20 @@ describe('discovery selectors', () => {
         makeConnectable('foo', '10.0.0.1', 'connectable', true, 'foo'),
       ],
     },
+    {
+      name: 'getConnectableRobots adds restartStatus if it exists',
+      state: {
+        discovery: {
+          robotsByName: {foo: [makeFullyUp('foo', '10.0.0.2')]},
+          restartsByName: {foo: 'pending'},
+        },
+        robot: {connection: {connectedTo: 'foo'}},
+      },
+      selector: discovery.getConnectableRobots,
+      expected: [
+        makeFullyUp('foo', '10.0.0.2', 'connectable', true, 'foo', 'pending'),
+      ],
+    },
     {
       name: 'getReachableRobots grabs robots with serverUp or advertising',
       selector: discovery.getReachableRobots,
@@ -186,6 +220,19 @@ describe('discovery selectors', () => {
       },
       expected: [],
     },
+    {
+      name: 'getReachableRobots adds restartStatus if it exists',
+      state: {
+        discovery: {
+          robotsByName: {foo: [makeServerUp('foo', '10.0.0.1', false)]},
+          restartsByName: {foo: 'down'},
+        },
+      },
+      selector: discovery.getReachableRobots,
+      expected: [
+        makeServerUp('foo', '10.0.0.1', false, 'reachable', 'foo', 'down'),
+      ],
+    },
     {
       name: 'getUnreachableRobots grabs robots with no ip',
       selector: discovery.getUnreachableRobots,
@@ -195,7 +242,13 @@ describe('discovery selectors', () => {
         },
       },
       expected: [
-        {name: 'foo', ip: null, status: 'unreachable', displayName: 'foo'},
+        {
+          name: 'foo',
+          ip: null,
+          status: 'unreachable',
+          displayName: 'foo',
+          restartStatus: null,
+        },
       ],
     },
     {
@@ -237,6 +290,19 @@ describe('discovery selectors', () => {
       },
       expected: [],
     },
+    {
+      name: 'getUnreachableRobots adds restartStatus if it exists',
+      state: {
+        discovery: {
+          robotsByName: {foo: [makeUnreachable('foo', '10.0.0.1')]},
+          restartsByName: {foo: 'down'},
+        },
+      },
+      selector: discovery.getUnreachableRobots,
+      expected: [
+        makeUnreachable('foo', '10.0.0.1', 'unreachable', 'foo', 'down'),
+      ],
+    },
     {
       name: 'display name removes opentrons- from connectable robot names',
       selector: discovery.getConnectableRobots,
@@ -281,17 +347,12 @@ describe('discovery selectors', () => {
       selector: discovery.getUnreachableRobots,
       state: {
         discovery: {
-          robotsByName: {'opentrons-foo': [{name: 'opentrons-foo', ip: null}]},
+          robotsByName: {
+            'opentrons-foo': [makeUnreachable('opentrons-foo', null)],
+          },
         },
       },
-      expected: [
-        {
-          name: 'opentrons-foo',
-          ip: null,
-          status: 'unreachable',
-          displayName: 'foo',
-        },
-      ],
+      expected: [makeUnreachable('opentrons-foo', null, 'unreachable', 'foo')],
     },
     {
       name: 'getAllRobots returns all robots',