From 011a35bbf3885cc12789a7a600f1a700f3bcb62c Mon Sep 17 00:00:00 2001 From: Mike Cousins Date: Thu, 3 Sep 2020 13:45:31 -0400 Subject: [PATCH 1/6] fix(app): properly enable/disable robot update from file button Fixes #5429 --- app/src/buildroot/__tests__/selectors.test.js | 125 ++++++++++++++-- app/src/buildroot/selectors.js | 93 +++++++++--- app/src/components/ConnectPanel/RobotItem.js | 5 +- .../RobotSettings/AdvancedSettingsCard.js | 12 +- .../RobotSettings/ConnectionCard.js | 8 +- .../components/RobotSettings/ControlsCard.js | 8 +- .../RobotSettings/InformationCard.js | 138 ++++++++---------- .../components/RobotSettings/StatusCard.js | 19 ++- .../UpdateBuildroot/MigrationWarningModal.js | 3 +- .../RobotSettings/UpdateBuildroot/index.js | 2 +- .../RobotSettings/UploadRobotUpdate.js | 98 +++++++++---- .../__tests__/ConnectionCard.test.js | 6 +- .../__tests__/ControlsCard.test.js | 2 +- .../components/RobotSettings/connection.js | 40 +++-- app/src/components/RobotSettings/styles.css | 8 +- app/src/nav/__tests__/selectors.test.js | 12 +- app/src/nav/selectors.js | 35 ++--- app/src/pages/Robots/RobotSettings.js | 14 +- components/src/controls/LabeledToggle.js | 4 +- components/src/primitives/Btn.js | 6 + components/src/primitives/types.js | 1 + components/src/styles/colors.js | 2 +- 22 files changed, 432 insertions(+), 209 deletions(-) diff --git a/app/src/buildroot/__tests__/selectors.test.js b/app/src/buildroot/__tests__/selectors.test.js index 7dfe9518a7a..22f91f67b9e 100644 --- a/app/src/buildroot/__tests__/selectors.test.js +++ b/app/src/buildroot/__tests__/selectors.test.js @@ -1,14 +1,23 @@ import * as selectors from '../selectors' +import { mockReachableRobot } from '../../discovery/__fixtures__' import { + HEALTH_STATUS_NOT_OK, getViewableRobots, - getRobotApiVersion, -} from '../../discovery/selectors' + getRobotApiVersionByName, + getRobotByName, +} from '../../discovery' jest.mock('../../discovery/selectors') describe('buildroot selectors', () => { + beforeEach(() => { + getViewableRobots.mockReturnValue([]) + getRobotApiVersionByName.mockReturnValue(null) + getRobotByName.mockReturnValue(null) + }) + afterEach(() => { - jest.clearAllMocks() + jest.resetAllMocks() }) const SPECS = [ @@ -81,10 +90,10 @@ describe('buildroot selectors', () => { version: '1.0.0', }, }, - args: [{ name: 'robot-name' }], + args: ['robot-name'], expected: 'upgrade', setup: () => { - getRobotApiVersion.mockReturnValueOnce('0.9.9') + getRobotApiVersionByName.mockReturnValue('0.9.9') }, }, { @@ -98,7 +107,7 @@ describe('buildroot selectors', () => { args: [{ name: 'robot-name' }], expected: 'downgrade', setup: () => { - getRobotApiVersion.mockReturnValueOnce('1.0.1') + getRobotApiVersionByName.mockReturnValue('1.0.1') }, }, { @@ -112,7 +121,7 @@ describe('buildroot selectors', () => { args: [{ name: 'robot-name' }], expected: 'reinstall', setup: () => { - getRobotApiVersion.mockReturnValueOnce('1.0.0') + getRobotApiVersionByName.mockReturnValue('1.0.0') }, }, { @@ -126,7 +135,7 @@ describe('buildroot selectors', () => { args: [{ name: 'robot-name' }], expected: null, setup: () => { - getRobotApiVersion.mockReturnValueOnce('1.0.0') + getRobotApiVersionByName.mockReturnValue('1.0.0') }, }, { @@ -159,7 +168,7 @@ describe('buildroot selectors', () => { }, expected: { name: 'robot-name', host: '10.10.0.0', port: 31950 }, setup: () => - getViewableRobots.mockReturnValueOnce([ + getViewableRobots.mockReturnValue([ { name: 'other-robot-name', host: '10.10.0.1', port: 31950 }, { name: 'robot-name', host: '10.10.0.0', port: 31950 }, { name: 'another-robot-name', host: '10.10.0.2', port: 31950 }, @@ -180,7 +189,7 @@ describe('buildroot selectors', () => { serverHealth: { capabilities: { buildrootUpdate: '/' } }, }, setup: () => - getViewableRobots.mockReturnValueOnce([ + getViewableRobots.mockReturnValue([ { name: 'other-robot-name', host: '10.10.0.1', port: 31950 }, { name: 'robot-name', @@ -191,12 +200,104 @@ describe('buildroot selectors', () => { { name: 'another-robot-name', host: '10.10.0.2', port: 31950 }, ]), }, + { + name: 'getBuildrootUpdateDisplayInfo returns not responding if no robot', + selector: selectors.getBuildrootUpdateDisplayInfo, + state: { buildroot: {} }, + setup: () => { + getRobotByName.mockReturnValue(null) + }, + expected: expect.objectContaining({ + autoUpdateDisabledReason: expect.stringMatching( + /update server is not responding/ + ), + updateFromFileDisabledReason: expect.stringMatching( + /update server is not responding/ + ), + }), + }, + { + name: + 'getBuildrootUpdateDisplayInfo returns not responding if robot has unhealthy update server', + selector: selectors.getBuildrootUpdateDisplayInfo, + state: { buildroot: {} }, + setup: () => { + getRobotByName.mockReturnValue({ + ...mockReachableRobot, + serverHealthStatus: HEALTH_STATUS_NOT_OK, + }) + }, + expected: expect.objectContaining({ + autoUpdateDisabledReason: expect.stringMatching( + /update server is not responding/ + ), + updateFromFileDisabledReason: expect.stringMatching( + /update server is not responding/ + ), + }), + }, + { + name: + 'getBuildrootUpdateDisplayInfo returns not allowed if another robot is updating', + selector: selectors.getBuildrootUpdateDisplayInfo, + state: { buildroot: { session: { robotName: 'other-robot-name' } } }, + setup: () => { + getRobotByName.mockImplementation((state, name) => { + return { ...mockReachableRobot, name } + }) + getViewableRobots.mockReturnValue([ + { name: 'other-robot-name', host: '10.10.0.1', port: 31950 }, + ]) + }, + expected: expect.objectContaining({ + autoUpdateDisabledReason: expect.stringMatching( + /updating a different robot/ + ), + updateFromFileDisabledReason: expect.stringMatching( + /updating a different robot/ + ), + }), + }, + { + name: + 'getBuildrootUpdateDisplayInfo returns allowed only from file if no auto files', + selector: selectors.getBuildrootUpdateDisplayInfo, + state: { buildroot: {} }, + setup: () => { + getRobotByName.mockReturnValue(mockReachableRobot) + }, + expected: { + autoUpdateAction: expect.stringMatching(/unavailable/i), + autoUpdateDisabledReason: expect.stringMatching( + /no update files found/i + ), + updateFromFileDisabledReason: null, + }, + }, + { + name: + 'getBuildrootUpdateDisplayInfo returns allowed with action if all good', + selector: selectors.getBuildrootUpdateDisplayInfo, + state: { buildroot: { version: '1.0.0' } }, + setup: () => { + getRobotByName.mockReturnValue(mockReachableRobot) + getRobotApiVersionByName.mockReturnValue('0.9.9') + }, + expected: { + autoUpdateAction: expect.stringMatching(/upgrade/i), + autoUpdateDisabledReason: null, + updateFromFileDisabledReason: null, + }, + }, ] SPECS.forEach(spec => { const { name, selector, state, expected, setup } = spec const args = spec.args || [] - if (typeof setup === 'function') setup() - it(name, () => expect(selector(state, ...args)).toEqual(expected)) + + it(name, () => { + if (typeof setup === 'function') setup() + expect(selector(state, ...args)).toEqual(expected) + }) }) }) diff --git a/app/src/buildroot/selectors.js b/app/src/buildroot/selectors.js index 3070c345656..5b52ba9c16c 100644 --- a/app/src/buildroot/selectors.js +++ b/app/src/buildroot/selectors.js @@ -2,7 +2,12 @@ import semver from 'semver' import { createSelector } from 'reselect' -import { getViewableRobots, getRobotApiVersion } from '../discovery/selectors' +import { + HEALTH_STATUS_OK, + getViewableRobots, + getRobotApiVersionByName, + getRobotByName, +} from '../discovery' import * as Constants from './constants' import type { State } from '../types' @@ -14,6 +19,15 @@ import type { RobotSystemType, } from './types' +// TODO(mc, 2020-08-02): i18n +const UPDATE_SERVER_UNAVAILABLE = + "Unable to update because your robot's update server is not responding" +const OTHER_ROBOT_UPDATING = + 'Unable to update because the app is currently updating a different robot' +const NO_UPDATE_FILES = + 'No update files found; please ensure your computer is connected to the internet and check again later' +const UNAVAILABLE = 'Unavailable' + export function getBuildrootUpdateVersion(state: State): string | null { return state.buildroot.version || null } @@ -87,29 +101,68 @@ export const getBuildrootRobot: State => ViewableRobot | null = createSelector( } ) -export function getBuildrootUpdateAvailable( +// TODO(mc, 2020-09-03): switch to robotName instead of robot +export const getBuildrootUpdateAvailable: ( state: State, - robot: ViewableRobot -): BuildrootUpdateType | null { - const updateVersion = getBuildrootUpdateVersion(state) - const currentVersion = getRobotApiVersion(robot) - - const validCurrent: string | null = semver.valid(currentVersion) - const validUpdate: string | null = semver.valid(updateVersion) - let type = null - - if (validUpdate) { - if (!validCurrent || semver.gt(validUpdate, validCurrent)) { - type = Constants.UPGRADE - } else if (semver.lt(validUpdate, validCurrent)) { - type = Constants.DOWNGRADE - } else if (semver.eq(validUpdate, validCurrent)) { - type = Constants.REINSTALL + robotName: string +) => BuildrootUpdateType | null = createSelector( + getRobotApiVersionByName, + state => getBuildrootUpdateVersion(state), + (currentVersion, updateVersion) => { + const validCurrent: string | null = semver.valid(currentVersion) + const validUpdate: string | null = semver.valid(updateVersion) + let type = null + + if (validUpdate) { + if (!validCurrent || semver.gt(validUpdate, validCurrent)) { + type = Constants.UPGRADE + } else if (semver.lt(validUpdate, validCurrent)) { + type = Constants.DOWNGRADE + } else if (semver.eq(validUpdate, validCurrent)) { + type = Constants.REINSTALL + } } + + return type } +) - return type -} +export const getBuildrootUpdateDisplayInfo: ( + state: State, + robotName: string +) => {| + autoUpdateAction: string, + autoUpdateDisabledReason: string | null, + updateFromFileDisabledReason: string | null, +|} = createSelector( + getRobotByName, + getBuildrootUpdateAvailable, + state => getBuildrootRobot(state), + (robot, autoUpdateType, currentUpdatingRobot) => { + const autoUpdateAction = autoUpdateType ?? UNAVAILABLE + let autoUpdateDisabledReason = null + let updateFromFileDisabledReason = null + + if (robot?.serverHealthStatus !== HEALTH_STATUS_OK) { + autoUpdateDisabledReason = UPDATE_SERVER_UNAVAILABLE + updateFromFileDisabledReason = UPDATE_SERVER_UNAVAILABLE + } else if ( + currentUpdatingRobot !== null && + currentUpdatingRobot.name !== robot?.name + ) { + autoUpdateDisabledReason = OTHER_ROBOT_UPDATING + updateFromFileDisabledReason = OTHER_ROBOT_UPDATING + } else if (autoUpdateType === null) { + autoUpdateDisabledReason = NO_UPDATE_FILES + } + + return { + autoUpdateAction, + autoUpdateDisabledReason, + updateFromFileDisabledReason, + } + } +) export function getRobotSystemType( robot: ViewableRobot diff --git a/app/src/components/ConnectPanel/RobotItem.js b/app/src/components/ConnectPanel/RobotItem.js index c01b5c74355..1b3feb13025 100644 --- a/app/src/components/ConnectPanel/RobotItem.js +++ b/app/src/components/ConnectPanel/RobotItem.js @@ -28,11 +28,10 @@ export function RobotItemComponent(props: RobotItemProps): React.Node { const { robot, match } = props const { name, displayName, status, local: isLocal } = robot const isUpgradable = useSelector((state: State) => { - return getBuildrootUpdateAvailable(state, robot) === UPGRADE + return getBuildrootUpdateAvailable(state, name) === UPGRADE }) const isConnectable = status === CONNECTABLE - // NOTE(mc, 2020-03-30): redundant && true to satisfy Flow - const isConnected = Boolean(robot.connected && true) + const isConnected = robot.connected const isSelected = robot.name === match.params.name const connectInProgress = useSelector( (state: State) => RobotSelectors.getConnectRequest(state).inProgress diff --git a/app/src/components/RobotSettings/AdvancedSettingsCard.js b/app/src/components/RobotSettings/AdvancedSettingsCard.js index 5ba880f38b3..5675337f36f 100644 --- a/app/src/components/RobotSettings/AdvancedSettingsCard.js +++ b/app/src/components/RobotSettings/AdvancedSettingsCard.js @@ -20,6 +20,7 @@ import { LabeledButton, LabeledToggle, Icon, + BORDER_SOLID_LIGHT, } from '@opentrons/components' import { UploadRobotUpdate } from './UploadRobotUpdate' @@ -60,7 +61,7 @@ export function AdvancedSettingsCard( ) const robotLogsDownloading = useSelector(getRobotLogsDownloading) const dispatch = useDispatch() - const disabled = status !== CONNECTABLE + const controlsDisabled = status !== CONNECTABLE const logsAvailable = health && health.logs const showLogOptoutModal = settings.some( @@ -74,7 +75,7 @@ export function AdvancedSettingsCard( }, [dispatch, name]) return ( - + dispatch(downloadLogs(robot)), }} > @@ -92,7 +93,7 @@ export function AdvancedSettingsCard(

Restore robot to factory configuration

+ {settings.map(({ id, title, description, value }) => ( dispatch(updateSetting(name, id, !value))} >

{description}

))} - {showLogOptoutModal && ( () const internetStatus = useSelector((state: State) => getInternetStatus(state, robotName) @@ -36,10 +36,12 @@ export function ConnectionCard(props: Props): React.Node { useInterval(() => dispatch(fetchStatus(robotName)), STATUS_REFRESH_MS, true) return ( - + diff --git a/app/src/components/RobotSettings/ControlsCard.js b/app/src/components/RobotSettings/ControlsCard.js index 306aa643653..a0c9b11f9b9 100644 --- a/app/src/components/RobotSettings/ControlsCard.js +++ b/app/src/components/RobotSettings/ControlsCard.js @@ -37,6 +37,7 @@ type Props = {| const TITLE = 'Robot Controls' +const CANNOT_CONNECT = 'Cannot connect to robot' const CONNECT_TO_ROBOT = 'Connect to robot to control' const PROTOCOL_IS_RUNNING = 'Protocol is running' const BAD_DECK_CALIBRATION = @@ -73,7 +74,9 @@ export function ControlsCard(props: Props): React.Node { ) let buttonDisabledReason = null - if (notConnectable || !robot.connected) { + if (notConnectable) { + buttonDisabledReason = CANNOT_CONNECT + } else if (!robot.connected) { buttonDisabledReason = CONNECT_TO_ROBOT } else if (isRunning) { buttonDisabledReason = PROTOCOL_IS_RUNNING @@ -92,7 +95,7 @@ export function ControlsCard(props: Props): React.Node { const buttonDisabled = Boolean(buttonDisabledReason) return ( - +

Control lights on deck.

diff --git a/app/src/components/RobotSettings/InformationCard.js b/app/src/components/RobotSettings/InformationCard.js index 2918a7e972c..f6dc920d7da 100644 --- a/app/src/components/RobotSettings/InformationCard.js +++ b/app/src/components/RobotSettings/InformationCard.js @@ -1,29 +1,32 @@ // @flow // RobotSettings card for robot status import * as React from 'react' +import cx from 'classnames' import { useSelector, useDispatch } from 'react-redux' import { Link } from 'react-router-dom' -import { - getRobotApiVersion, - getRobotFirmwareVersion, - getRobotProtocolApiVersion, - HEALTH_STATUS_OK, -} from '../../discovery' - -import { getBuildrootRobot, getBuildrootUpdateAvailable } from '../../buildroot' - -import { checkShellUpdate } from '../../shell' - import { Card, + Flex, + Box, LabeledValue, - OutlineButton, - HoverTooltip, + SecondaryBtn, + Tooltip, useInterval, + useHoverTooltip, + ALIGN_FLEX_START, + FLEX_NONE, + SPACING_AUTO, + SPACING_3, } from '@opentrons/components' -import { CardRow, CardContentThird } from '../layout' +import { getBuildrootUpdateDisplayInfo } from '../../buildroot' +import { checkShellUpdate } from '../../shell' +import { + getRobotApiVersion, + getRobotFirmwareVersion, + getRobotProtocolApiVersion, +} from '../../discovery' import type { State, Dispatch } from '../../types' import type { ViewableRobot } from '../../discovery/types' @@ -38,13 +41,7 @@ const NAME_LABEL = 'Robot name' const SERVER_VERSION_LABEL = 'Server version' const FIRMWARE_VERSION_LABEL = 'Firmware version' const MAX_PROTOCOL_API_VERSION_LABEL = 'Max Protocol API Version' - -const UPDATE_SERVER_UNAVAILABLE = - "Unable to update because your robot's update server is not responding" -const OTHER_ROBOT_UPDATING = - 'Unable to update because your app is currently updating a different robot' -const NO_UPDATE_FILES = - 'No robot update files found for this version of the app; please check again later' +const UNKNOWN = 'Unknown' const DEFAULT_MAX_API_VERSION = '1.0' @@ -52,77 +49,68 @@ const UPDATE_RECHECK_DELAY_MS = 60000 export function InformationCard(props: InformationCardProps): React.Node { const { robot, updateUrl } = props - const updateType = useSelector((state: State) => - getBuildrootUpdateAvailable(state, robot) + const [updateBtnProps, updateBtnTooltipProps] = useHoverTooltip() + const { autoUpdateAction, autoUpdateDisabledReason } = useSelector( + (state: State) => { + return getBuildrootUpdateDisplayInfo(state, robot.name) + } ) + const dispatch = useDispatch() const checkAppUpdate = React.useCallback(() => dispatch(checkShellUpdate()), [ dispatch, ]) - const { displayName, serverHealthStatus } = robot - const buildrootRobot = useSelector(getBuildrootRobot) + const { displayName } = robot const version = getRobotApiVersion(robot) const firmwareVersion = getRobotFirmwareVersion(robot) const maxApiVersion = getRobotProtocolApiVersion(robot) - - const updateFilesUnavailable = updateType === null - const updateServerUnavailable = serverHealthStatus !== HEALTH_STATUS_OK - const otherRobotUpdating = Boolean(buildrootRobot && buildrootRobot !== robot) - const updateDisabled = - updateFilesUnavailable || updateServerUnavailable || otherRobotUpdating - - const updateButtonText = updateType || 'up to date' - let updateButtonTooltip = null - if (otherRobotUpdating) { - updateButtonTooltip = {OTHER_ROBOT_UPDATING} - } else if (updateServerUnavailable) { - updateButtonTooltip = {UPDATE_SERVER_UNAVAILABLE} - } else if (updateFilesUnavailable) { - updateButtonTooltip = {NO_UPDATE_FILES} - } + const updateDisabled = autoUpdateDisabledReason !== null // check for available updates on an interval useInterval(checkAppUpdate, UPDATE_RECHECK_DELAY_MS) return ( - - - - - - - - + + + + + + + + + + + - - - - - - {hoverTooltipHandlers => ( -
- - {updateButtonText} - -
- )} -
-
+ + + {autoUpdateAction} + + {autoUpdateDisabledReason !== null && ( + + {autoUpdateDisabledReason} + + )} +
) } diff --git a/app/src/components/RobotSettings/StatusCard.js b/app/src/components/RobotSettings/StatusCard.js index 3a7335ff62a..1132ecc34f7 100644 --- a/app/src/components/RobotSettings/StatusCard.js +++ b/app/src/components/RobotSettings/StatusCard.js @@ -22,6 +22,7 @@ type Props = {| robot: ViewableRobot |} const TITLE = 'Status' const STATUS_LABEL = 'This robot is currently' const STATUS_VALUE_DISCONNECTED = 'Unknown - connect to view status' +const STATUS_VALUE_NOT_CONNECTABLE = 'Not connectable' const STATUS_VALUE_DEFAULT = 'Idle' const CONNECT = 'connect' const DISCONNECT = 'disconnect' @@ -29,14 +30,20 @@ const DISCONNECT = 'disconnect' export function StatusCard(props: Props): React.Node { const { robot } = props const dispatch = useDispatch() + const connectable = robot.status === CONNECTABLE const connected = robot.connected != null && robot.connected === true const sessionStatus = useSelector(robotSelectors.getSessionStatus) const connectRequest = useSelector(robotSelectors.getConnectRequest) - const disabled = robot.status !== CONNECTABLE || connectRequest.inProgress + const connectButtonDisabled = !connectable || connectRequest.inProgress + let status = STATUS_VALUE_DEFAULT - const status = connected - ? (sessionStatus && capitalize(sessionStatus)) || STATUS_VALUE_DEFAULT - : STATUS_VALUE_DISCONNECTED + if (!connectable) { + status = STATUS_VALUE_NOT_CONNECTABLE + } else if (!connected) { + status = STATUS_VALUE_DISCONNECTED + } else if (sessionStatus) { + status = capitalize(sessionStatus) + } const handleClick = () => { if (connected) { @@ -47,12 +54,12 @@ export function StatusCard(props: Props): React.Node { } return ( - + - + {connected ? ( DISCONNECT ) : connectRequest.name === robot.name ? ( diff --git a/app/src/components/RobotSettings/UpdateBuildroot/MigrationWarningModal.js b/app/src/components/RobotSettings/UpdateBuildroot/MigrationWarningModal.js index f05b220f736..94478879f41 100644 --- a/app/src/components/RobotSettings/UpdateBuildroot/MigrationWarningModal.js +++ b/app/src/components/RobotSettings/UpdateBuildroot/MigrationWarningModal.js @@ -2,6 +2,7 @@ import * as React from 'react' import { AlertModal } from '@opentrons/components' +import { UPGRADE } from '../../../buildroot' import styles from './styles.css' import type { ButtonProps } from '@opentrons/components' @@ -23,7 +24,7 @@ export function MigrationWarningModal( const buttons: Array = [ notNowButton, { - children: updateType === 'upgrade' ? 'view robot update' : 'update robot', + children: updateType === UPGRADE ? 'view robot update' : 'update robot', className: styles.view_update_button, onClick: proceed, }, diff --git a/app/src/components/RobotSettings/UpdateBuildroot/index.js b/app/src/components/RobotSettings/UpdateBuildroot/index.js index 95edbf3eb41..8306befa22b 100644 --- a/app/src/components/RobotSettings/UpdateBuildroot/index.js +++ b/app/src/components/RobotSettings/UpdateBuildroot/index.js @@ -29,7 +29,7 @@ export function UpdateBuildroot(props: UpdateBuildrootProps): React.Node { const [viewUpdateInfo, setViewUpdateInfo] = React.useState(false) const session = useSelector(getBuildrootSession) const robotUpdateType = useSelector((state: State) => - getBuildrootUpdateAvailable(state, robot) + getBuildrootUpdateAvailable(state, robotName) ) const dispatch = useDispatch() const { step, error } = session || { step: null, error: null } diff --git a/app/src/components/RobotSettings/UploadRobotUpdate.js b/app/src/components/RobotSettings/UploadRobotUpdate.js index fa8be791e61..11784ca3634 100644 --- a/app/src/components/RobotSettings/UploadRobotUpdate.js +++ b/app/src/components/RobotSettings/UploadRobotUpdate.js @@ -1,23 +1,61 @@ // @flow import * as React from 'react' -import { useDispatch } from 'react-redux' +import cx from 'classnames' +import { useDispatch, useSelector } from 'react-redux' +import { css } from 'styled-components' -import { LabeledButton } from '@opentrons/components' -import { startBuildrootUpdate } from '../../buildroot' -import styles from './styles.css' +import { + Link, + SecondaryBtn, + Tooltip, + useHoverTooltip, +} from '@opentrons/components' -import type { Dispatch } from '../../types' +import { + getBuildrootUpdateDisplayInfo, + startBuildrootUpdate, +} from '../../buildroot' + +import { TitledControl } from '../TitledControl' + +import type { StyleProps } from '@opentrons/components' +import type { State, Dispatch } from '../../types' + +// TODO(mc, 2020-08-03): i18n +const BROWSE = 'browse' +const UPDATE_FROM_FILE = 'Update robot software from file' +const UPDATE_FROM_FILE_DESCRIPTION = ( + <> + If your app is unable to auto-download robot updates, you can{' '} + + download the robot update yourself + {' '} + and update your robot manually + +) + +const HIDDEN_CSS = css` + position: fixed; + clip: rect(1px 1px 1px 1px); +` export type UploadRobotUpdateProps = {| robotName: string, + ...StyleProps, |} export function UploadRobotUpdate(props: UploadRobotUpdateProps): React.Node { - const { robotName } = props + const { robotName, ...styleProps } = props const dispatch = useDispatch() + const { updateFromFileDisabledReason } = useSelector((state: State) => { + return getBuildrootUpdateDisplayInfo(state, robotName) + }) + const updateDisabled = updateFromFileDisabledReason !== null + const [updateBtnProps, updateBtnTooltipProps] = useHoverTooltip() + const handleChange = (event: SyntheticInputEvent) => { const { files } = event.target - if (files.length === 1) { + if (files.length === 1 && !updateDisabled) { // NOTE: File.path is Electron-specific // https://electronjs.org/docs/api/file-object dispatch(startBuildrootUpdate(robotName, (files[0]: any).path)) @@ -27,26 +65,32 @@ export function UploadRobotUpdate(props: UploadRobotUpdateProps): React.Node { } return ( - - browse - - - ), - }} + + {BROWSE} + + + } + {...styleProps} > -

- If your app is unable to auto-download robot updates, you can download - the robot update yourself and update your robot manually -

-
+ {updateFromFileDisabledReason !== null && ( + + {updateFromFileDisabledReason} + + )} + ) } diff --git a/app/src/components/RobotSettings/__tests__/ConnectionCard.test.js b/app/src/components/RobotSettings/__tests__/ConnectionCard.test.js index 63890deef29..d3ce37b1d82 100644 --- a/app/src/components/RobotSettings/__tests__/ConnectionCard.test.js +++ b/app/src/components/RobotSettings/__tests__/ConnectionCard.test.js @@ -82,13 +82,15 @@ describe('ConnectionCard', () => { expect(dispatch).toHaveBeenNthCalledWith(3, expected) }) - it('passes internet status to ConnectionStatusMessage', () => { + it('passes robot and internet status to ConnectionStatusMessage', () => { mockGetInternetStatus.mockReturnValue(Networking.STATUS_FULL) const wrapper = render() const status = wrapper.find(ConnectionStatusMessage) - expect(status.prop('status')).toEqual(Networking.STATUS_FULL) + expect(status.prop('status')).toEqual(mockRobot.status) + expect(status.prop('ipAdress')).toEqual(mockRobot.ip) + expect(status.prop('internetStatus')).toEqual(Networking.STATUS_FULL) }) it('passes type ConnectionStatusMessage based on robot.local', () => { diff --git a/app/src/components/RobotSettings/__tests__/ControlsCard.test.js b/app/src/components/RobotSettings/__tests__/ControlsCard.test.js index b2825d897f7..43366adb6dd 100644 --- a/app/src/components/RobotSettings/__tests__/ControlsCard.test.js +++ b/app/src/components/RobotSettings/__tests__/ControlsCard.test.js @@ -192,7 +192,7 @@ describe('ControlsCard', () => { expect(getDeckCalButton(wrapper).prop('disabled')).toBe(true) expect(getCheckCalibrationControl(wrapper).prop('disabledReason')).toBe( - 'Connect to robot to control' + 'Cannot connect to robot' ) expect(getHomeButton(wrapper).prop('disabled')).toBe(true) expect(getRestartButton(wrapper).prop('disabled')).toBe(true) diff --git a/app/src/components/RobotSettings/connection.js b/app/src/components/RobotSettings/connection.js index e0bd229c5d8..bdd8571ee56 100644 --- a/app/src/components/RobotSettings/connection.js +++ b/app/src/components/RobotSettings/connection.js @@ -2,6 +2,8 @@ // UI components for displaying connection info import * as React from 'react' import cx from 'classnames' + +import { CONNECTABLE, REACHABLE } from '../../discovery' import { CardContentHalf } from '../layout' import styles from './styles.css' @@ -10,9 +12,27 @@ import type { SimpleInterfaceStatus, } from '../../networking/types' -type ConnectionStatusProps = { type: string, status: ?InternetStatus } +const USB: 'USB' = 'USB' +const WI_FI: 'Wi-Fi' = 'Wi-Fi' + +type ConnectionStatusProps = {| + type: typeof USB | typeof WI_FI, + ipAddress: string, + status: typeof CONNECTABLE | typeof REACHABLE, + internetStatus: InternetStatus | null, +|} -function shortStatusToDescription(status: ?InternetStatus) { +const statusToDescription = ( + status: typeof CONNECTABLE | typeof REACHABLE, + type: typeof USB | typeof WI_FI, + ipAddress: string +) => { + return `Your app is ${ + status === CONNECTABLE ? 'currently connected' : 'trying to connect' + } to your robot via ${type} at IP address ${ipAddress}` +} + +const internetStatusToDescription = (status: InternetStatus | null) => { switch (status) { case 'full': return 'The robot is connected to a network and has full access to the Internet.' @@ -30,14 +50,14 @@ function shortStatusToDescription(status: ?InternetStatus) { export function ConnectionStatusMessage( props: ConnectionStatusProps ): React.Node { - const { type, status } = props + const { type, ipAddress, status, internetStatus } = props return (
-

Your app is currently connected to your robot via {type}

+

{statusToDescription(status, type, ipAddress)}

Internet: - {shortStatusToDescription(status)} + {internetStatusToDescription(internetStatus)}

) @@ -85,22 +105,22 @@ function NetworkAddresses(props: NetworkAddressProps) { const ip = props.connection?.ipAddress || 'Unknown' const subnet = props.connection?.subnetMask || 'Unknown' const mac = props.connection?.macAddress || 'Unknown' - const labelStyles = cx(styles.connection_label, { + const classNames = cx(styles.wireless_info, { [styles.disabled]: props.disabled, }) return ( -
+

- {type} IP: + {type} IP: {ip}

- {type} Subnet Mask: + {type} Subnet Mask: {subnet}

- {type} MAC Address: + {type} MAC Address: {mac}

diff --git a/app/src/components/RobotSettings/styles.css b/app/src/components/RobotSettings/styles.css index acc02392963..d1bac590556 100644 --- a/app/src/components/RobotSettings/styles.css +++ b/app/src/components/RobotSettings/styles.css @@ -59,8 +59,7 @@ } .connection_label { - @apply --font-body-1-dark; - + font-size: var(--fs-body-1); font-weight: var(--fw-semibold); padding-bottom: 0.25rem; } @@ -81,11 +80,6 @@ color: var(--c-font-disabled); } -.file_input { - position: fixed; - clip: rect(1px 1px 1px 1px); -} - .restart_banner_message { display: flex; diff --git a/app/src/nav/__tests__/selectors.test.js b/app/src/nav/__tests__/selectors.test.js index 4f149e15473..d7a328c2f51 100644 --- a/app/src/nav/__tests__/selectors.test.js +++ b/app/src/nav/__tests__/selectors.test.js @@ -12,7 +12,7 @@ import * as Selectors from '../selectors' import { NOT_APPLICABLE, OUTDATED } from '../../system-info' import type { State } from '../../types' -import type { Robot, ViewableRobot } from '../../discovery/types' +import type { Robot } from '../../discovery/types' type SelectorSpec = {| name: string, @@ -51,12 +51,8 @@ const mockGetU2EWindowsDriverStatus: JestMockFn< > = SystemInfoSelectors.getU2EWindowsDriverStatus const mockGetBuildrootUpdateAvailable: JestMockFn< - [State, ViewableRobot], - $Call< - typeof BuildrootSelectors.getBuildrootUpdateAvailable, - State, - ViewableRobot - > + [State, string], + $Call > = BuildrootSelectors.getBuildrootUpdateAvailable const mockGetIsRunning: JestMockFn< @@ -336,7 +332,7 @@ describe('nav selectors', () => { after: () => { expect(mockGetBuildrootUpdateAvailable).toHaveBeenCalledWith( mockState, - mockRobot + mockRobot.name ) }, expected: [ diff --git a/app/src/nav/selectors.js b/app/src/nav/selectors.js index 6ed4c31c9de..1d8fd20316c 100644 --- a/app/src/nav/selectors.js +++ b/app/src/nav/selectors.js @@ -4,7 +4,7 @@ import { createSelector } from 'reselect' import { getConnectedRobot } from '../discovery' import { getProtocolPipettesMatch } from '../pipettes' import { selectors as RobotSelectors } from '../robot' -import { getBuildrootUpdateAvailable } from '../buildroot' +import { UPGRADE, getBuildrootUpdateAvailable } from '../buildroot' import { getAvailableShellUpdate } from '../shell' import { getU2EWindowsDriverStatus, OUTDATED } from '../system-info' import { getFeatureFlags } from '../config' @@ -36,25 +36,22 @@ const APP_UPDATE_AVAILABLE = 'An app update is available' const DRIVER_UPDATE_AVAILABLE = 'A driver update is available' const ROBOT_UPDATE_AVAILABLE = 'A robot software update is available' -const getConnectedRobotPipettesMatch: State => boolean = createSelector( - state => state, - getConnectedRobot, - (state, connectedRobot) => - connectedRobot - ? getProtocolPipettesMatch(state, connectedRobot.name) - : false -) +const getConnectedRobotPipettesMatch = (state: State): boolean => { + const connectedRobot = getConnectedRobot(state) -const getConnectedRobotUpdateAvailable: State => boolean = createSelector( - state => state, - getConnectedRobot, - (state, connectedRobot) => { - const robotUpdateType = connectedRobot - ? getBuildrootUpdateAvailable(state, connectedRobot) - : null - return robotUpdateType === 'upgrade' - } -) + return connectedRobot + ? getProtocolPipettesMatch(state, connectedRobot.name) + : false +} + +const getConnectedRobotUpdateAvailable = (state: State): boolean => { + const connectedRobot = getConnectedRobot(state) + const robotUpdateType = connectedRobot + ? getBuildrootUpdateAvailable(state, connectedRobot.name) + : null + + return robotUpdateType === UPGRADE +} const getRunDisabledReason: State => string | null = createSelector( getConnectedRobot, diff --git a/app/src/pages/Robots/RobotSettings.js b/app/src/pages/Robots/RobotSettings.js index a045419971e..afb588ae6fc 100644 --- a/app/src/pages/Robots/RobotSettings.js +++ b/app/src/pages/Robots/RobotSettings.js @@ -10,8 +10,9 @@ import { } from '../../robot' import { CONNECTABLE, REACHABLE } from '../../discovery' import { + UPGRADE, getBuildrootUpdateSeen, - getBuildrootRobot, + getBuildrootUpdateDisplayInfo, getBuildrootUpdateInProgress, getBuildrootUpdateAvailable, } from '../../buildroot' @@ -205,15 +206,18 @@ function mapStateToProps(state: State, ownProps: OP): SP { const movementStatus = getMovementStatus(state, robot.name) const movementError = getMovementError(state, robot.name) const buildrootUpdateSeen = getBuildrootUpdateSeen(state) - const buildrootUpdateType = getBuildrootUpdateAvailable(state, robot) + const buildrootUpdateType = getBuildrootUpdateAvailable(state, robot.name) const updateInProgress = getBuildrootUpdateInProgress(state, robot) - const currentBrRobot = getBuildrootRobot(state) + const { autoUpdateDisabledReason } = getBuildrootUpdateDisplayInfo( + state, + robot.name + ) const showUpdateModal = updateInProgress || (!buildrootUpdateSeen && - buildrootUpdateType === 'upgrade' && - currentBrRobot === null) + buildrootUpdateType === UPGRADE && + autoUpdateDisabledReason === null) return { updateInProgress, diff --git a/components/src/controls/LabeledToggle.js b/components/src/controls/LabeledToggle.js index 14507e47065..310fae67872 100644 --- a/components/src/controls/LabeledToggle.js +++ b/components/src/controls/LabeledToggle.js @@ -8,6 +8,7 @@ import styles from './styles.css' export type LabeledToggleProps = {| label: string, toggledOn: boolean, + disabled?: boolean, children?: React.Node, onClick: () => mixed, /** optional data test id for the container */ @@ -15,7 +16,7 @@ export type LabeledToggleProps = {| |} export function LabeledToggle(props: LabeledToggleProps): React.Node { - const { label, toggledOn, onClick } = props + const { label, toggledOn, disabled, onClick } = props return ( } diff --git a/components/src/primitives/Btn.js b/components/src/primitives/Btn.js index 9af93a0494d..83adba224ac 100644 --- a/components/src/primitives/Btn.js +++ b/components/src/primitives/Btn.js @@ -84,6 +84,8 @@ export const PrimaryBtn: BtnComponent = styled(Btn)` color: ${Styles.C_MED_GRAY}; box-shadow: none; } + + ${styleProps} ` /** @@ -112,6 +114,8 @@ export const SecondaryBtn: BtnComponent = styled(Btn)` background-color: ${Styles.C_WHITE}; color: ${Styles.C_MED_GRAY}; } + + ${styleProps} ` /** @@ -138,6 +142,8 @@ export const LightSecondaryBtn: BtnComponent = styled(SecondaryBtn)` background-color: ${Styles.C_TRANSPARENT}; color: ${Styles.C_MED_GRAY}; } + + ${styleProps} ` /** diff --git a/components/src/primitives/types.js b/components/src/primitives/types.js index 533c9de7dc0..1f97a955a2e 100644 --- a/components/src/primitives/types.js +++ b/components/src/primitives/types.js @@ -83,6 +83,7 @@ export type StyleProps = {| ...FlexboxProps, ...LayoutProps, ...PositionProps, + className?: string, |} export type PrimitiveComponent = StyledComponent< diff --git a/components/src/styles/colors.js b/components/src/styles/colors.js index b055116205b..59737e42f2f 100644 --- a/components/src/styles/colors.js +++ b/components/src/styles/colors.js @@ -24,5 +24,5 @@ export const OVERLAY_WHITE_20 = 'rgba(255, 255, 255, 0.2)' // opacities export const OPACITY_DISABLED = 0.3 -// TDOD(isk: 3/2/20): Rename to be more generic (e.g. not FONT) +// TODO(isk: 3/2/20): Rename to be more generic (e.g. not FONT) export const C_FONT_DISABLED = '#9c9c9c' From b42174351e20570db56d8f004ab1a511cb425cc6 Mon Sep 17 00:00:00 2001 From: Mike Cousins Date: Thu, 3 Sep 2020 15:08:47 -0400 Subject: [PATCH 2/6] fixup: add some missing changes --- app/src/buildroot/selectors.js | 1 - .../RobotSettings/UpdateBuildroot/ViewUpdateModal.js | 6 ++++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/src/buildroot/selectors.js b/app/src/buildroot/selectors.js index 5b52ba9c16c..90c05272313 100644 --- a/app/src/buildroot/selectors.js +++ b/app/src/buildroot/selectors.js @@ -101,7 +101,6 @@ export const getBuildrootRobot: State => ViewableRobot | null = createSelector( } ) -// TODO(mc, 2020-09-03): switch to robotName instead of robot export const getBuildrootUpdateAvailable: ( state: State, robotName: string diff --git a/app/src/components/RobotSettings/UpdateBuildroot/ViewUpdateModal.js b/app/src/components/RobotSettings/UpdateBuildroot/ViewUpdateModal.js index 89142eee42f..cd8f56d60ca 100644 --- a/app/src/components/RobotSettings/UpdateBuildroot/ViewUpdateModal.js +++ b/app/src/components/RobotSettings/UpdateBuildroot/ViewUpdateModal.js @@ -3,6 +3,8 @@ import * as React from 'react' import { useSelector } from 'react-redux' import { + BALENA, + UPGRADE, getBuildrootUpdateInfo, getBuildrootDownloadProgress, getBuildrootDownloadError, @@ -34,13 +36,13 @@ export function ViewUpdateModal(props: ViewUpdateModalProps): React.Node { const [ showMigrationWarning, setShowMigrationWarning, - ] = React.useState(robotSystemType === 'balena') + ] = React.useState(robotSystemType === BALENA) const notNowButton = { onClick: close, children: downloadError !== null ? 'close' : 'not now', } - const showReleaseNotes = robotUpdateType === 'upgrade' + const showReleaseNotes = robotUpdateType === UPGRADE React.useLayoutEffect(() => { if (updateInfo && !showReleaseNotes && !showMigrationWarning) { From 65b3d19dd73656744c629299abf99a92a83be9f8 Mon Sep 17 00:00:00 2001 From: Mike Cousins Date: Tue, 8 Sep 2020 12:28:02 -0400 Subject: [PATCH 3/6] fixup: revert some selector changes and tweak copy --- app/src/buildroot/__tests__/selectors.test.js | 44 +++++++++--- app/src/buildroot/selectors.js | 68 +++++++++++-------- app/src/components/ConnectPanel/RobotItem.js | 2 +- .../RobotSettings/InformationCard.js | 3 +- .../RobotSettings/UpdateBuildroot/index.js | 2 +- app/src/nav/__tests__/selectors.test.js | 12 ++-- app/src/nav/selectors.js | 2 +- app/src/pages/Robots/RobotSettings.js | 2 +- 8 files changed, 87 insertions(+), 48 deletions(-) diff --git a/app/src/buildroot/__tests__/selectors.test.js b/app/src/buildroot/__tests__/selectors.test.js index 22f91f67b9e..4d1d5c748e3 100644 --- a/app/src/buildroot/__tests__/selectors.test.js +++ b/app/src/buildroot/__tests__/selectors.test.js @@ -3,7 +3,7 @@ import { mockReachableRobot } from '../../discovery/__fixtures__' import { HEALTH_STATUS_NOT_OK, getViewableRobots, - getRobotApiVersionByName, + getRobotApiVersion, getRobotByName, } from '../../discovery' @@ -12,7 +12,7 @@ jest.mock('../../discovery/selectors') describe('buildroot selectors', () => { beforeEach(() => { getViewableRobots.mockReturnValue([]) - getRobotApiVersionByName.mockReturnValue(null) + getRobotApiVersion.mockReturnValue(null) getRobotByName.mockReturnValue(null) }) @@ -90,10 +90,13 @@ describe('buildroot selectors', () => { version: '1.0.0', }, }, - args: ['robot-name'], + args: [{ name: 'robot-name' }], expected: 'upgrade', setup: () => { - getRobotApiVersionByName.mockReturnValue('0.9.9') + getRobotApiVersion.mockImplementation(robot => { + expect(robot).toEqual({ name: 'robot-name' }) + return '0.9.9' + }) }, }, { @@ -107,7 +110,10 @@ describe('buildroot selectors', () => { args: [{ name: 'robot-name' }], expected: 'downgrade', setup: () => { - getRobotApiVersionByName.mockReturnValue('1.0.1') + getRobotApiVersion.mockImplementation(robot => { + expect(robot).toEqual({ name: 'robot-name' }) + return '1.0.1' + }) }, }, { @@ -121,7 +127,10 @@ describe('buildroot selectors', () => { args: [{ name: 'robot-name' }], expected: 'reinstall', setup: () => { - getRobotApiVersionByName.mockReturnValue('1.0.0') + getRobotApiVersion.mockImplementation(robot => { + expect(robot).toEqual({ name: 'robot-name' }) + return '1.0.0' + }) }, }, { @@ -135,8 +144,22 @@ describe('buildroot selectors', () => { args: [{ name: 'robot-name' }], expected: null, setup: () => { - getRobotApiVersionByName.mockReturnValue('1.0.0') + getRobotApiVersion.mockImplementation(robot => { + expect(robot).toEqual({ name: 'robot-name' }) + return '1.0.0' + }) + }, + }, + { + name: 'getBuildrootUpdateAvailable with no robot version available', + selector: selectors.getBuildrootUpdateAvailable, + state: { + buildroot: { + version: '1.0.0', + }, }, + args: [{ name: 'robot-name' }], + expected: null, }, { name: 'getBuildrootUpdateSession', @@ -269,7 +292,7 @@ describe('buildroot selectors', () => { expected: { autoUpdateAction: expect.stringMatching(/unavailable/i), autoUpdateDisabledReason: expect.stringMatching( - /no update files found/i + /unable to retrieve update/i ), updateFromFileDisabledReason: null, }, @@ -281,7 +304,10 @@ describe('buildroot selectors', () => { state: { buildroot: { version: '1.0.0' } }, setup: () => { getRobotByName.mockReturnValue(mockReachableRobot) - getRobotApiVersionByName.mockReturnValue('0.9.9') + getRobotApiVersion.mockImplementation(robot => { + expect(robot).toEqual(mockReachableRobot) + return '0.9.9' + }) }, expected: { autoUpdateAction: expect.stringMatching(/upgrade/i), diff --git a/app/src/buildroot/selectors.js b/app/src/buildroot/selectors.js index 90c05272313..baff05ff9e4 100644 --- a/app/src/buildroot/selectors.js +++ b/app/src/buildroot/selectors.js @@ -5,7 +5,7 @@ import { createSelector } from 'reselect' import { HEALTH_STATUS_OK, getViewableRobots, - getRobotApiVersionByName, + getRobotApiVersion, getRobotByName, } from '../discovery' import * as Constants from './constants' @@ -21,12 +21,12 @@ import type { // TODO(mc, 2020-08-02): i18n const UPDATE_SERVER_UNAVAILABLE = - "Unable to update because your robot's update server is not responding" + "Unable to update because your robot's update server is not responding." const OTHER_ROBOT_UPDATING = - 'Unable to update because the app is currently updating a different robot' + 'Unable to update because the app is currently updating a different robot.' const NO_UPDATE_FILES = - 'No update files found; please ensure your computer is connected to the internet and check again later' -const UNAVAILABLE = 'Unavailable' + 'Unable to retrieve update for this robot. Ensure your computer is connected to the internet and try again later.' +const UNAVAILABLE = 'Update unavailable' export function getBuildrootUpdateVersion(state: State): string | null { return state.buildroot.version || null @@ -101,30 +101,38 @@ export const getBuildrootRobot: State => ViewableRobot | null = createSelector( } ) -export const getBuildrootUpdateAvailable: ( - state: State, - robotName: string -) => BuildrootUpdateType | null = createSelector( - getRobotApiVersionByName, - state => getBuildrootUpdateVersion(state), - (currentVersion, updateVersion) => { - const validCurrent: string | null = semver.valid(currentVersion) - const validUpdate: string | null = semver.valid(updateVersion) - let type = null - - if (validUpdate) { - if (!validCurrent || semver.gt(validUpdate, validCurrent)) { - type = Constants.UPGRADE - } else if (semver.lt(validUpdate, validCurrent)) { - type = Constants.DOWNGRADE - } else if (semver.eq(validUpdate, validCurrent)) { - type = Constants.REINSTALL - } +const getBuildrootUpdateType = ( + currentVersion: string | null, + updateVersion: string | null +): BuildrootUpdateType | null => { + const validCurrent: string | null = semver.valid(currentVersion) + const validUpdate: string | null = semver.valid(updateVersion) + let type = null + + if (validUpdate && validCurrent) { + if (semver.gt(validUpdate, validCurrent)) { + type = Constants.UPGRADE + } else if (semver.lt(validUpdate, validCurrent)) { + type = Constants.DOWNGRADE + } else if (semver.eq(validUpdate, validCurrent)) { + type = Constants.REINSTALL } - - return type } -) + + console.log(currentVersion, updateVersion, type) + + return type +} + +export function getBuildrootUpdateAvailable( + state: State, + robot: ViewableRobot +): BuildrootUpdateType | null { + const currentVersion = getRobotApiVersion(robot) + const updateVersion = getBuildrootUpdateVersion(state) + + return getBuildrootUpdateType(currentVersion, updateVersion) +} export const getBuildrootUpdateDisplayInfo: ( state: State, @@ -135,9 +143,11 @@ export const getBuildrootUpdateDisplayInfo: ( updateFromFileDisabledReason: string | null, |} = createSelector( getRobotByName, - getBuildrootUpdateAvailable, state => getBuildrootRobot(state), - (robot, autoUpdateType, currentUpdatingRobot) => { + state => getBuildrootUpdateVersion(state), + (robot, currentUpdatingRobot, updateVersion) => { + const robotVersion = robot ? getRobotApiVersion(robot) : null + const autoUpdateType = getBuildrootUpdateType(robotVersion, updateVersion) const autoUpdateAction = autoUpdateType ?? UNAVAILABLE let autoUpdateDisabledReason = null let updateFromFileDisabledReason = null diff --git a/app/src/components/ConnectPanel/RobotItem.js b/app/src/components/ConnectPanel/RobotItem.js index 1b3feb13025..0458496fb13 100644 --- a/app/src/components/ConnectPanel/RobotItem.js +++ b/app/src/components/ConnectPanel/RobotItem.js @@ -28,7 +28,7 @@ export function RobotItemComponent(props: RobotItemProps): React.Node { const { robot, match } = props const { name, displayName, status, local: isLocal } = robot const isUpgradable = useSelector((state: State) => { - return getBuildrootUpdateAvailable(state, name) === UPGRADE + return getBuildrootUpdateAvailable(state, robot) === UPGRADE }) const isConnectable = status === CONNECTABLE const isConnected = robot.connected diff --git a/app/src/components/RobotSettings/InformationCard.js b/app/src/components/RobotSettings/InformationCard.js index f6dc920d7da..5ce5170bd65 100644 --- a/app/src/components/RobotSettings/InformationCard.js +++ b/app/src/components/RobotSettings/InformationCard.js @@ -99,8 +99,7 @@ export function InformationCard(props: InformationCardProps): React.Node { as={Link} to={!updateDisabled ? updateUrl : '#'} flex={FLEX_NONE} - paddingX={0} - width="9rem" + minWidth="9rem" className={cx({ disabled: updateDisabled })} > {autoUpdateAction} diff --git a/app/src/components/RobotSettings/UpdateBuildroot/index.js b/app/src/components/RobotSettings/UpdateBuildroot/index.js index 8306befa22b..95edbf3eb41 100644 --- a/app/src/components/RobotSettings/UpdateBuildroot/index.js +++ b/app/src/components/RobotSettings/UpdateBuildroot/index.js @@ -29,7 +29,7 @@ export function UpdateBuildroot(props: UpdateBuildrootProps): React.Node { const [viewUpdateInfo, setViewUpdateInfo] = React.useState(false) const session = useSelector(getBuildrootSession) const robotUpdateType = useSelector((state: State) => - getBuildrootUpdateAvailable(state, robotName) + getBuildrootUpdateAvailable(state, robot) ) const dispatch = useDispatch() const { step, error } = session || { step: null, error: null } diff --git a/app/src/nav/__tests__/selectors.test.js b/app/src/nav/__tests__/selectors.test.js index d7a328c2f51..4f149e15473 100644 --- a/app/src/nav/__tests__/selectors.test.js +++ b/app/src/nav/__tests__/selectors.test.js @@ -12,7 +12,7 @@ import * as Selectors from '../selectors' import { NOT_APPLICABLE, OUTDATED } from '../../system-info' import type { State } from '../../types' -import type { Robot } from '../../discovery/types' +import type { Robot, ViewableRobot } from '../../discovery/types' type SelectorSpec = {| name: string, @@ -51,8 +51,12 @@ const mockGetU2EWindowsDriverStatus: JestMockFn< > = SystemInfoSelectors.getU2EWindowsDriverStatus const mockGetBuildrootUpdateAvailable: JestMockFn< - [State, string], - $Call + [State, ViewableRobot], + $Call< + typeof BuildrootSelectors.getBuildrootUpdateAvailable, + State, + ViewableRobot + > > = BuildrootSelectors.getBuildrootUpdateAvailable const mockGetIsRunning: JestMockFn< @@ -332,7 +336,7 @@ describe('nav selectors', () => { after: () => { expect(mockGetBuildrootUpdateAvailable).toHaveBeenCalledWith( mockState, - mockRobot.name + mockRobot ) }, expected: [ diff --git a/app/src/nav/selectors.js b/app/src/nav/selectors.js index 1d8fd20316c..aa24b2c105c 100644 --- a/app/src/nav/selectors.js +++ b/app/src/nav/selectors.js @@ -47,7 +47,7 @@ const getConnectedRobotPipettesMatch = (state: State): boolean => { const getConnectedRobotUpdateAvailable = (state: State): boolean => { const connectedRobot = getConnectedRobot(state) const robotUpdateType = connectedRobot - ? getBuildrootUpdateAvailable(state, connectedRobot.name) + ? getBuildrootUpdateAvailable(state, connectedRobot) : null return robotUpdateType === UPGRADE diff --git a/app/src/pages/Robots/RobotSettings.js b/app/src/pages/Robots/RobotSettings.js index afb588ae6fc..959e9f385fc 100644 --- a/app/src/pages/Robots/RobotSettings.js +++ b/app/src/pages/Robots/RobotSettings.js @@ -206,7 +206,7 @@ function mapStateToProps(state: State, ownProps: OP): SP { const movementStatus = getMovementStatus(state, robot.name) const movementError = getMovementError(state, robot.name) const buildrootUpdateSeen = getBuildrootUpdateSeen(state) - const buildrootUpdateType = getBuildrootUpdateAvailable(state, robot.name) + const buildrootUpdateType = getBuildrootUpdateAvailable(state, robot) const updateInProgress = getBuildrootUpdateInProgress(state, robot) const { autoUpdateDisabledReason } = getBuildrootUpdateDisplayInfo( state, From 95ad791c7316141a95029c97b13e6bd84f00f297 Mon Sep 17 00:00:00 2001 From: Mike Cousins Date: Thu, 10 Sep 2020 17:16:28 -0400 Subject: [PATCH 4/6] fixup: remove console.log --- app/src/buildroot/selectors.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/src/buildroot/selectors.js b/app/src/buildroot/selectors.js index baff05ff9e4..7e03b7a7ec1 100644 --- a/app/src/buildroot/selectors.js +++ b/app/src/buildroot/selectors.js @@ -119,8 +119,6 @@ const getBuildrootUpdateType = ( } } - console.log(currentVersion, updateVersion, type) - return type } From 3fed4cd2a1fbe2622be0384b481e58c2a2e036db Mon Sep 17 00:00:00 2001 From: Mike Cousins Date: Mon, 14 Sep 2020 14:22:38 -0400 Subject: [PATCH 5/6] fixup: refactor buildroot selector tests for correctness and readability --- app/src/buildroot/__tests__/selectors.test.js | 565 +++++++++--------- 1 file changed, 273 insertions(+), 292 deletions(-) diff --git a/app/src/buildroot/__tests__/selectors.test.js b/app/src/buildroot/__tests__/selectors.test.js index 4d1d5c748e3..78ff29a0968 100644 --- a/app/src/buildroot/__tests__/selectors.test.js +++ b/app/src/buildroot/__tests__/selectors.test.js @@ -20,310 +20,291 @@ describe('buildroot selectors', () => { jest.resetAllMocks() }) - const SPECS = [ - { - name: 'getBuildrootUpdateInfo', - selector: selectors.getBuildrootUpdateInfo, - state: { - buildroot: { - info: { - releaseNotes: 'some release notes', - }, - }, - }, - expected: { - releaseNotes: 'some release notes', - }, - }, - { - name: 'getBuildrootTargetVersion with auto-downloaded file', - selector: selectors.getBuildrootTargetVersion, - state: { buildroot: { version: '1.0.0' } }, - expected: '1.0.0', - }, - { - name: 'getBuildrootTargetVersion with user file', - selector: selectors.getBuildrootTargetVersion, - state: { - buildroot: { - version: '1.0.0', - session: { userFileInfo: { version: '1.0.1' } }, - }, - }, - expected: '1.0.1', - }, - { - name: 'getBuildrootDownloadError', - selector: selectors.getBuildrootDownloadError, - state: { - buildroot: { - downloadError: 'error with download', - }, - }, - expected: 'error with download', - }, - { - name: 'getBuildrootDownloadProgress', - selector: selectors.getBuildrootDownloadProgress, - state: { - buildroot: { - downloadProgress: 10, - }, - }, - expected: 10, - }, - { - name: 'getBuildrootUpdateSeen', - selector: selectors.getBuildrootUpdateSeen, - state: { - buildroot: { - seen: false, - }, - }, - expected: false, - }, - { - name: 'getBuildrootUpdateAvailable with lesser version', - selector: selectors.getBuildrootUpdateAvailable, - state: { - buildroot: { - version: '1.0.0', - }, - }, - args: [{ name: 'robot-name' }], - expected: 'upgrade', - setup: () => { - getRobotApiVersion.mockImplementation(robot => { - expect(robot).toEqual({ name: 'robot-name' }) - return '0.9.9' - }) - }, - }, - { - name: 'getBuildrootUpdateAvailable with greater version', - selector: selectors.getBuildrootUpdateAvailable, - state: { - buildroot: { - version: '1.0.0', - }, - }, - args: [{ name: 'robot-name' }], - expected: 'downgrade', - setup: () => { - getRobotApiVersion.mockImplementation(robot => { - expect(robot).toEqual({ name: 'robot-name' }) - return '1.0.1' - }) - }, - }, - { - name: 'getBuildrootUpdateAvailable with same version', - selector: selectors.getBuildrootUpdateAvailable, - state: { - buildroot: { - version: '1.0.0', - }, - }, - args: [{ name: 'robot-name' }], - expected: 'reinstall', - setup: () => { - getRobotApiVersion.mockImplementation(robot => { - expect(robot).toEqual({ name: 'robot-name' }) - return '1.0.0' - }) - }, - }, - { - name: 'getBuildrootUpdateAvailable with no update available', - selector: selectors.getBuildrootUpdateAvailable, - state: { - buildroot: { - version: null, - }, - }, - args: [{ name: 'robot-name' }], - expected: null, - setup: () => { - getRobotApiVersion.mockImplementation(robot => { - expect(robot).toEqual({ name: 'robot-name' }) - return '1.0.0' - }) - }, - }, - { - name: 'getBuildrootUpdateAvailable with no robot version available', - selector: selectors.getBuildrootUpdateAvailable, - state: { - buildroot: { - version: '1.0.0', - }, + it('should get buildroot update info', () => { + const state = { + buildroot: { info: { releaseNotes: 'some release notes' } }, + } + const result = selectors.getBuildrootUpdateInfo(state) + + expect(result).toEqual({ releaseNotes: 'some release notes' }) + }) + + it('should get the update version from the auto-downloaded file', () => { + const state = { buildroot: { version: '1.0.0' } } + const result = selectors.getBuildrootTargetVersion(state) + + expect(result).toBe('1.0.0') + }) + + it('should get the update version from the user-provided file', () => { + const state = { + buildroot: { + version: '1.0.0', + session: { userFileInfo: { version: '1.0.1' } }, }, - args: [{ name: 'robot-name' }], - expected: null, - }, - { - name: 'getBuildrootUpdateSession', - selector: selectors.getBuildrootSession, - state: { - buildroot: { - session: { robotName: 'robot-name', token: null, pathPrefix: null }, - }, + } + const result = selectors.getBuildrootTargetVersion(state) + + expect(result).toBe('1.0.1') + }) + + it('should get the update download error', () => { + const state = { buildroot: { downloadError: 'error with download' } } + const result = selectors.getBuildrootDownloadError(state) + + expect(result).toBe('error with download') + }) + + it('should get the update download progress', () => { + const state = { buildroot: { downloadProgress: 10 } } + const result = selectors.getBuildrootDownloadProgress(state) + + expect(result).toBe(10) + }) + + it('should get the update seen flag', () => { + const state = { buildroot: { seen: false } } + const result = selectors.getBuildrootUpdateSeen(state) + + expect(result).toBe(false) + }) + + it('should get update type when robot is behind the update', () => { + const state = { buildroot: { version: '1.0.0' } } + const robot = { name: 'robot-name' } + + getRobotApiVersion.mockImplementation(inputRobot => { + expect(inputRobot).toBe(robot) + return '0.9.9' + }) + + const result = selectors.getBuildrootUpdateAvailable(state, robot) + + expect(result).toBe('upgrade') + }) + + it('should get update type when robot is ahead of the update', () => { + const state = { buildroot: { version: '1.0.0' } } + const robot = { name: 'robot-name' } + + getRobotApiVersion.mockReturnValue('1.0.1') + + const result = selectors.getBuildrootUpdateAvailable(state, robot) + + expect(result).toBe('downgrade') + }) + + it('should get update type when robot is matches the update', () => { + const state = { buildroot: { version: '1.0.0' } } + const robot = { name: 'robot-name' } + + getRobotApiVersion.mockReturnValue('1.0.0') + + const result = selectors.getBuildrootUpdateAvailable(state, robot) + + expect(result).toBe('reinstall') + }) + + it('should return null update type when no update available', () => { + const state = { buildroot: { version: null } } + const robot = { name: 'robot-name' } + + getRobotApiVersion.mockReturnValue('1.0.0') + + const result = selectors.getBuildrootUpdateAvailable(state, robot) + + expect(result).toBe(null) + }) + + it('should return null update type when no robot version available', () => { + const state = { buildroot: { version: '1.0.0' } } + const robot = { name: 'robot-name' } + + getRobotApiVersion.mockReturnValue(null) + + const result = selectors.getBuildrootUpdateAvailable(state, robot) + + expect(result).toBe(null) + }) + + it('should get the buildroot update session', () => { + const state = { + buildroot: { + session: { robotName: 'robot-name', token: null, pathPrefix: null }, }, - expected: { robotName: 'robot-name', token: null, pathPrefix: null }, - }, - { - name: 'getBuildrootRobotName', - selector: selectors.getBuildrootRobotName, - state: { - buildroot: { - session: { robotName: 'robot-name', token: null, pathPrefix: null }, - }, + } + const result = selectors.getBuildrootSession(state) + + expect(result).toEqual({ + robotName: 'robot-name', + token: null, + pathPrefix: null, + }) + }) + + it('should get the robot name from the update session', () => { + const state = { + buildroot: { + session: { robotName: 'robot-name', token: null, pathPrefix: null }, }, - expected: 'robot-name', - }, - { - name: 'getBuildrootRobot', - selector: selectors.getBuildrootRobot, - state: { - buildroot: { - session: { robotName: 'robot-name' }, - }, + } + const result = selectors.getBuildrootRobotName(state) + + expect(result).toBe('robot-name') + }) + + it('should get the full robot from the update session', () => { + const state = { + buildroot: { + session: { robotName: 'robot-name' }, }, - expected: { name: 'robot-name', host: '10.10.0.0', port: 31950 }, - setup: () => - getViewableRobots.mockReturnValue([ - { name: 'other-robot-name', host: '10.10.0.1', port: 31950 }, - { name: 'robot-name', host: '10.10.0.0', port: 31950 }, - { name: 'another-robot-name', host: '10.10.0.2', port: 31950 }, - ]), - }, - { - name: 'getBuildrootRobot after migration with opentrons-robot-name', - selector: selectors.getBuildrootRobot, - state: { - buildroot: { - session: { robotName: 'opentrons-robot-name' }, - }, + } + + getViewableRobots.mockImplementation(inputState => { + expect(inputState).toBe(state) + return [ + { name: 'other-robot-name', host: '10.10.0.1', port: 31950 }, + { name: 'robot-name', host: '10.10.0.0', port: 31950 }, + { name: 'another-robot-name', host: '10.10.0.2', port: 31950 }, + ] + }) + + const result = selectors.getBuildrootRobot(state) + expect(result).toEqual({ + name: 'robot-name', + host: '10.10.0.0', + port: 31950, + }) + }) + + it('should get the robot from session after migration with opentrons- name prefix', () => { + const state = { + buildroot: { + session: { robotName: 'opentrons-robot-name' }, }, - expected: { + } + + getViewableRobots.mockReturnValue([ + { name: 'other-robot-name', host: '10.10.0.1', port: 31950 }, + { name: 'robot-name', host: '10.10.0.0', port: 31950, serverHealth: { capabilities: { buildrootUpdate: '/' } }, }, - setup: () => - getViewableRobots.mockReturnValue([ - { name: 'other-robot-name', host: '10.10.0.1', port: 31950 }, - { - name: 'robot-name', - host: '10.10.0.0', - port: 31950, - serverHealth: { capabilities: { buildrootUpdate: '/' } }, - }, - { name: 'another-robot-name', host: '10.10.0.2', port: 31950 }, - ]), - }, - { - name: 'getBuildrootUpdateDisplayInfo returns not responding if no robot', - selector: selectors.getBuildrootUpdateDisplayInfo, - state: { buildroot: {} }, - setup: () => { - getRobotByName.mockReturnValue(null) - }, - expected: expect.objectContaining({ - autoUpdateDisabledReason: expect.stringMatching( - /update server is not responding/ - ), - updateFromFileDisabledReason: expect.stringMatching( - /update server is not responding/ - ), - }), - }, - { - name: - 'getBuildrootUpdateDisplayInfo returns not responding if robot has unhealthy update server', - selector: selectors.getBuildrootUpdateDisplayInfo, - state: { buildroot: {} }, - setup: () => { - getRobotByName.mockReturnValue({ - ...mockReachableRobot, - serverHealthStatus: HEALTH_STATUS_NOT_OK, - }) - }, - expected: expect.objectContaining({ - autoUpdateDisabledReason: expect.stringMatching( - /update server is not responding/ - ), - updateFromFileDisabledReason: expect.stringMatching( - /update server is not responding/ - ), - }), - }, - { - name: - 'getBuildrootUpdateDisplayInfo returns not allowed if another robot is updating', - selector: selectors.getBuildrootUpdateDisplayInfo, - state: { buildroot: { session: { robotName: 'other-robot-name' } } }, - setup: () => { - getRobotByName.mockImplementation((state, name) => { - return { ...mockReachableRobot, name } - }) - getViewableRobots.mockReturnValue([ - { name: 'other-robot-name', host: '10.10.0.1', port: 31950 }, - ]) - }, - expected: expect.objectContaining({ - autoUpdateDisabledReason: expect.stringMatching( - /updating a different robot/ - ), - updateFromFileDisabledReason: expect.stringMatching( - /updating a different robot/ - ), - }), - }, - { - name: - 'getBuildrootUpdateDisplayInfo returns allowed only from file if no auto files', - selector: selectors.getBuildrootUpdateDisplayInfo, - state: { buildroot: {} }, - setup: () => { - getRobotByName.mockReturnValue(mockReachableRobot) - }, - expected: { - autoUpdateAction: expect.stringMatching(/unavailable/i), - autoUpdateDisabledReason: expect.stringMatching( - /unable to retrieve update/i - ), - updateFromFileDisabledReason: null, - }, - }, - { - name: - 'getBuildrootUpdateDisplayInfo returns allowed with action if all good', - selector: selectors.getBuildrootUpdateDisplayInfo, - state: { buildroot: { version: '1.0.0' } }, - setup: () => { - getRobotByName.mockReturnValue(mockReachableRobot) - getRobotApiVersion.mockImplementation(robot => { - expect(robot).toEqual(mockReachableRobot) - return '0.9.9' - }) - }, - expected: { - autoUpdateAction: expect.stringMatching(/upgrade/i), - autoUpdateDisabledReason: null, - updateFromFileDisabledReason: null, - }, - }, - ] + { name: 'another-robot-name', host: '10.10.0.2', port: 31950 }, + ]) + + const result = selectors.getBuildrootRobot(state) + expect(result).toEqual({ + name: 'robot-name', + host: '10.10.0.0', + port: 31950, + serverHealth: { capabilities: { buildrootUpdate: '/' } }, + }) + }) + + it('should return update disabled because not responding if no robot', () => { + const state = { buildroot: {} } + const robotName = 'robot-name' + + getRobotByName.mockImplementation((inputState, inputName) => { + expect(inputState).toBe(state) + expect(inputName).toBe(robotName) + return null + }) + + const result = selectors.getBuildrootUpdateDisplayInfo(state, robotName) + + expect(result).toMatchObject({ + autoUpdateDisabledReason: expect.stringMatching( + /update server is not responding/ + ), + updateFromFileDisabledReason: expect.stringMatching( + /update server is not responding/ + ), + }) + }) + + it('should return update disabled if robot has unhealthy update sever', () => { + const state = { buildroot: {} } + const robotName = 'robot-name' + + getRobotByName.mockReturnValue({ + ...mockReachableRobot, + serverHealthStatus: HEALTH_STATUS_NOT_OK, + }) + + const result = selectors.getBuildrootUpdateDisplayInfo(state, robotName) + + expect(result).toMatchObject({ + autoUpdateDisabledReason: expect.stringMatching( + /update server is not responding/ + ), + updateFromFileDisabledReason: expect.stringMatching( + /update server is not responding/ + ), + }) + }) + + it('should return update disabled because other robot updating', () => { + const state = { buildroot: { session: { robotName: 'other-name' } } } + const robotName = 'robot-name' + const robot = { ...mockReachableRobot, name: robotName } + const otherRobot = { ...mockReachableRobot, name: 'other-name' } + + getRobotByName.mockReturnValue(robot) + getViewableRobots.mockReturnValue([robot, otherRobot]) + getRobotApiVersion.mockImplementation(inputRobot => { + expect(inputRobot).toBe(robot) + return '1.0.0' + }) + + const result = selectors.getBuildrootUpdateDisplayInfo(state, robotName) + + expect(result).toMatchObject({ + autoUpdateDisabledReason: expect.stringMatching( + /updating a different robot/ + ), + updateFromFileDisabledReason: expect.stringMatching( + /updating a different robot/ + ), + }) + }) + + it('should return auto-update disabled but from-file enabled if no downloaded files', () => { + const state = { buildroot: {} } + const robotName = 'robot-name' + const robot = { ...mockReachableRobot, name: robotName } + + getRobotByName.mockReturnValue(robot) + getRobotApiVersion.mockReturnValue('1.0.0') + + const result = selectors.getBuildrootUpdateDisplayInfo(state, robotName) + + expect(result).toEqual({ + autoUpdateAction: expect.stringMatching(/unavailable/i), + autoUpdateDisabledReason: expect.stringMatching( + /unable to retrieve update/i + ), + updateFromFileDisabledReason: null, + }) + }) + + it('should return all updates allowed if update files exist and robot is healthy', () => { + const state = { buildroot: { version: '1.0.0' } } + const robotName = 'robot-name' + const robot = { ...mockReachableRobot, name: robotName } + + getRobotByName.mockReturnValue(robot) + getRobotApiVersion.mockReturnValue('0.9.9') - SPECS.forEach(spec => { - const { name, selector, state, expected, setup } = spec - const args = spec.args || [] + const result = selectors.getBuildrootUpdateDisplayInfo(state, robotName) - it(name, () => { - if (typeof setup === 'function') setup() - expect(selector(state, ...args)).toEqual(expected) + expect(result).toEqual({ + autoUpdateAction: expect.stringMatching(/upgrade/i), + autoUpdateDisabledReason: null, + updateFromFileDisabledReason: null, }) }) }) From 9492dff2ae2bc59378b5a87a9877397d0aafe0c1 Mon Sep 17 00:00:00 2001 From: Mike Cousins Date: Mon, 14 Sep 2020 16:11:17 -0400 Subject: [PATCH 6/6] fixup: tweak update type selector test names --- app/src/buildroot/__tests__/selectors.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/buildroot/__tests__/selectors.test.js b/app/src/buildroot/__tests__/selectors.test.js index 78ff29a0968..286a845758d 100644 --- a/app/src/buildroot/__tests__/selectors.test.js +++ b/app/src/buildroot/__tests__/selectors.test.js @@ -69,7 +69,7 @@ describe('buildroot selectors', () => { expect(result).toBe(false) }) - it('should get update type when robot is behind the update', () => { + it('should return "upgrade" update type when robot is behind the update', () => { const state = { buildroot: { version: '1.0.0' } } const robot = { name: 'robot-name' } @@ -83,7 +83,7 @@ describe('buildroot selectors', () => { expect(result).toBe('upgrade') }) - it('should get update type when robot is ahead of the update', () => { + it('should return "downgrade" update type when robot is ahead of the update', () => { const state = { buildroot: { version: '1.0.0' } } const robot = { name: 'robot-name' } @@ -94,7 +94,7 @@ describe('buildroot selectors', () => { expect(result).toBe('downgrade') }) - it('should get update type when robot is matches the update', () => { + it('should get "reinstall" update type when robot matches the update', () => { const state = { buildroot: { version: '1.0.0' } } const robot = { name: 'robot-name' }