feat(app): allow app update pop-up notifications to be disabled (#6715)

Closes #6684

Author: mcous

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

@@ -1,52 +1,52 @@
 // @flow
+import * as Config from '../../config'
 import * as Actions from '../actions'
 
-import type { AlertId, AlertsAction } from '../types'
+import type { AlertId } from '../types'
 
 const MOCK_ALERT_ID: AlertId = ('mockAlert': any)
 
-type ActionSpec = {|
-  should: string,
-  creator: (...args: Array<any>) => AlertsAction,
-  args: Array<mixed>,
-  expected: AlertsAction,
-|}
-
-const SPECS: Array<ActionSpec> = [
-  {
-    should: 'allow an alert to be triggered',
-    creator: Actions.alertTriggered,
-    args: [MOCK_ALERT_ID],
-    expected: {
+describe('alerts actions', () => {
+  it('should allow an alert to be triggered', () => {
+    const result = Actions.alertTriggered(MOCK_ALERT_ID)
+
+    expect(result).toEqual({
       type: 'alerts:ALERT_TRIGGERED',
       payload: { alertId: MOCK_ALERT_ID },
-    },
-  },
-  {
-    should: 'allow an alert to be dismissed temporarily',
-    creator: Actions.alertDismissed,
-    args: [MOCK_ALERT_ID],
-    expected: {
+    })
+  })
+
+  it('should allow an alert to be dismissed temporarily', () => {
+    const result = Actions.alertDismissed(MOCK_ALERT_ID)
+
+    expect(result).toEqual({
       type: 'alerts:ALERT_DISMISSED',
       payload: { alertId: MOCK_ALERT_ID, remember: false },
-    },
-  },
-  {
-    should: 'allow an alert to be dismissed permanently',
-    creator: Actions.alertDismissed,
-    args: [MOCK_ALERT_ID, true],
-    expected: {
+    })
+  })
+
+  it('should allow an alert to be dismissed permanently', () => {
+    const result = Actions.alertDismissed(MOCK_ALERT_ID, true)
+
+    expect(result).toEqual({
       type: 'alerts:ALERT_DISMISSED',
       payload: { alertId: MOCK_ALERT_ID, remember: true },
-    },
-  },
-]
-
-describe('alerts actions', () => {
-  SPECS.forEach(({ should, creator, args, expected }) => {
-    it(`should ${should}`, () => {
-      expect(creator).toEqual(expect.any(Function))
-      expect(creator(...args)).toEqual(expected)
     })
   })
+
+  it('should allow an alert to be ignored permanently', () => {
+    const result = Actions.alertPermanentlyIgnored(MOCK_ALERT_ID)
+
+    expect(result).toEqual(
+      Config.addUniqueConfigValue('alerts.ignored', MOCK_ALERT_ID)
+    )
+  })
+
+  it('should allow an alert to be unignored', () => {
+    const result = Actions.alertUnignored(MOCK_ALERT_ID)
+
+    expect(result).toEqual(
+      Config.subtractConfigValue('alerts.ignored', MOCK_ALERT_ID)
+    )
+  })
 })

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

@@ -15,47 +15,88 @@ const MOCK_ALERT_1: AlertId = ('mockAlert1': any)
 const MOCK_ALERT_2: AlertId = ('mockAlert2': any)
 const MOCK_IGNORED_ALERT: AlertId = ('mockIgnoredAlert': any)
 
-describe('alerts selectors', () => {
-  let state: State
+const MOCK_CONFIG: $Shape<Config> = {
+  alerts: { ignored: [MOCK_IGNORED_ALERT] },
+}
 
-  beforeEach(() => {
+describe('alerts selectors', () => {
+  const stubGetConfig = (state: State, value = MOCK_CONFIG) => {
     getConfig.mockImplementation(s => {
       expect(s).toEqual(state)
-      return { alerts: { ignored: [MOCK_IGNORED_ALERT] } }
+      return value
     })
+  }
+
+  afterEach(() => {
+    jest.resetAllMocks()
   })
 
   it('should be able to get a list of active alerts', () => {
-    state = ({
+    const state = ({
       alerts: { active: [MOCK_ALERT_1, MOCK_ALERT_2], ignored: [] },
     }: $Shape<State>)
+
+    stubGetConfig(state)
+
     expect(Selectors.getActiveAlerts(state)).toEqual([
       MOCK_ALERT_1,
       MOCK_ALERT_2,
     ])
   })
 
   it('should show no active alerts until config is loaded', () => {
-    getConfig.mockReturnValue(null)
-    state = ({
+    const state = ({
       alerts: { active: [MOCK_ALERT_1, MOCK_ALERT_2], ignored: [] },
     }: $Shape<State>)
+
+    stubGetConfig(state, null)
+
     expect(Selectors.getActiveAlerts(state)).toEqual([])
   })
 
   it('should filter ignored alerts from active alerts', () => {
     // the reducer should never let this state happen, but let's protect
     // against it in the selector, too
-    state = ({
+    const state = ({
       alerts: { active: [MOCK_ALERT_1, MOCK_ALERT_2], ignored: [MOCK_ALERT_2] },
     }: $Shape<State>)
+
+    stubGetConfig(state)
+
     expect(Selectors.getActiveAlerts(state)).toEqual([MOCK_ALERT_1])
   })
 
   it('should filter perma-ignored alerts from active alerts', () => {
-    state = ({
+    const state = ({
       alerts: { active: [MOCK_ALERT_1, MOCK_IGNORED_ALERT], ignored: [] },
     }: $Shape<State>)
+
+    stubGetConfig(state)
+
     expect(Selectors.getActiveAlerts(state)).toEqual([MOCK_ALERT_1])
   })
+
+  it('should be able to tell you if an alert is perma-ignored', () => {
+    const state = ({ alerts: { active: [], ignored: [] } }: $Shape<State>)
+
+    stubGetConfig(state)
+
+    expect(
+      Selectors.getAlertIsPermanentlyIgnored(state, MOCK_IGNORED_ALERT)
+    ).toBe(true)
+
+    expect(Selectors.getAlertIsPermanentlyIgnored(state, MOCK_ALERT_1)).toBe(
+      false
+    )
+  })
+
+  it('should return null for getAlertIsPermanentlyIgnored if config not initialized', () => {
+    const state = ({ alerts: { active: [], ignored: [] } }: $Shape<State>)
+
+    stubGetConfig(state, null)
+
+    expect(
+      Selectors.getAlertIsPermanentlyIgnored(state, MOCK_IGNORED_ALERT)
+    ).toBe(null)
+  })
 })

app/src/alerts/actions.js

@@ -1,8 +1,14 @@
 // @flow
 
+import { addUniqueConfigValue, subtractConfigValue } from '../config'
 import * as Constants from './constants'
 import * as Types from './types'
 
+import type {
+  AddUniqueConfigValueAction,
+  SubtractConfigValueAction,
+} from '../config/types'
+
 export const alertTriggered = (
   alertId: Types.AlertId
 ): Types.AlertTriggeredAction => ({
@@ -17,3 +23,15 @@ export const alertDismissed = (
   type: Constants.ALERT_DISMISSED,
   payload: { alertId, remember },
 })
+
+export const alertPermanentlyIgnored = (
+  alertId: Types.AlertId
+): AddUniqueConfigValueAction => {
+  return addUniqueConfigValue(Constants.CONFIG_PATH_ALERTS_IGNORED, alertId)
+}
+
+export const alertUnignored = (
+  alertId: Types.AlertId
+): SubtractConfigValueAction => {
+  return subtractConfigValue(Constants.CONFIG_PATH_ALERTS_IGNORED, alertId)
+}

app/src/alerts/constants.js

@@ -1,5 +1,8 @@
 // @flow
 
+// config path
+export const CONFIG_PATH_ALERTS_IGNORED = 'alerts.ignored'
+
 // alert types
 export const ALERT_U2E_DRIVER_OUTDATED: 'u2eDriverOutdated' =
   'u2eDriverOutdated'

app/src/alerts/epic.js

@@ -1,7 +1,7 @@
 // @flow
 import { filter, map } from 'rxjs/operators'
 
-import { addUniqueConfigValue } from '../config'
+import { alertPermanentlyIgnored } from './actions'
 import { ALERT_DISMISSED } from './constants'
 
 import type { Action, Epic } from '../types'
@@ -14,11 +14,6 @@ export const alertsEpic: Epic = (action$, state$) => {
     filter<Action, AlertDismissedAction>(
       a => a.type === ALERT_DISMISSED && a.payload.remember
     ),
-    map(dismissAction => {
-      return addUniqueConfigValue(
-        'alerts.ignored',
-        dismissAction.payload.alertId
-      )
-    })
+    map(dismiss => alertPermanentlyIgnored(dismiss.payload.alertId))
   )
 }

app/src/alerts/selectors.js

@@ -27,3 +27,11 @@ export const getActiveAlerts: (
       : []
   }
 )
+
+export const getAlertIsPermanentlyIgnored = (
+  state: State,
+  alertId: AlertId
+): boolean | null => {
+  const permaIgnoreList = getIgnoredAlertsFromConfig(state)
+  return permaIgnoreList ? permaIgnoreList.includes(alertId) : null
+}

app/src/components/Alerts/__tests__/Alerts.test.js

@@ -42,7 +42,7 @@ describe('app-wide Alerts component', () => {
 
   const stubActiveAlerts = alertIds => {
     getActiveAlerts.mockImplementation(state => {
-      expect(state).toBe(MOCK_STATE)
+      expect(state).toEqual(MOCK_STATE)
       return alertIds
     })
   }
@@ -70,11 +70,11 @@ describe('app-wide Alerts component', () => {
   })
 
   it('should render a U2EDriverOutdatedAlert if alert is triggered', () => {
-    const { wrapper, store } = render()
+    const { wrapper, store, refresh } = render()
     expect(wrapper.exists(U2EDriverOutdatedAlert)).toBe(false)
 
     stubActiveAlerts([AppAlerts.ALERT_U2E_DRIVER_OUTDATED])
-    wrapper.setProps({})
+    refresh()
     expect(wrapper.exists(U2EDriverOutdatedAlert)).toBe(true)
 
     wrapper.find(U2EDriverOutdatedAlert).invoke('dismissAlert')(true)
@@ -85,11 +85,11 @@ describe('app-wide Alerts component', () => {
   })
 
   it('should render an UpdateAppModal if appUpdateAvailable alert is triggered', () => {
-    const { wrapper, store } = render()
+    const { wrapper, store, refresh } = render()
     expect(wrapper.exists(UpdateAppModal)).toBe(false)
 
     stubActiveAlerts([AppAlerts.ALERT_APP_UPDATE_AVAILABLE])
-    wrapper.setProps({})
+    refresh()
     expect(wrapper.exists(UpdateAppModal)).toBe(true)
 
     wrapper.find(UpdateAppModal).invoke('dismissAlert')(true)

app/src/components/RobotSettings/SelectNetwork/ConnectModal/__tests__/form-state.test.js

@@ -1,6 +1,5 @@
 // @flow
 import * as React from 'react'
-import { act } from 'react-dom/test-utils'
 import { mount } from 'enzyme'
 import * as Formik from 'formik'
 
@@ -55,9 +54,7 @@ describe('ConnectModal state hooks', () => {
       mockFormOnce({ ssid: 'foo', securityType: 'qux', psk: 'baz' })
       const wrapper = render()
 
-      act(() => {
-        wrapper.setProps({})
-      })
+      wrapper.setProps({})
 
       expect(setValues).toHaveBeenCalledTimes(1)
       expect(setValues).toHaveBeenCalledWith({
@@ -72,9 +69,7 @@ describe('ConnectModal state hooks', () => {
       mockFormOnce({ ssid: '', securityType: 'qux', psk: 'baz' }, errors)
       const wrapper = render()
 
-      act(() => {
-        wrapper.setProps({})
-      })
+      wrapper.setProps({})
 
       expect(setErrors).toHaveBeenCalledTimes(1)
       expect(setErrors).toHaveBeenCalledWith({ ssid: 'missing!' })
@@ -86,9 +81,7 @@ describe('ConnectModal state hooks', () => {
       mockFormOnce({ ssid: '', securityType: 'qux', psk: 'baz' }, {}, touched)
       const wrapper = render()
 
-      act(() => {
-        wrapper.setProps({})
-      })
+      wrapper.setProps({})
 
       expect(setTouched).toHaveBeenCalledTimes(1)
       expect(setTouched).toHaveBeenCalledWith(

app/src/components/RobotSettings/UpdateBuildroot/SkipAppUpdateMessage.js

@@ -1,6 +1,13 @@
 // @flow
 import * as React from 'react'
-import { C_BLUE, SPACING_3, Link, Text } from '@opentrons/components'
+
+import {
+  C_BLUE,
+  FONT_SIZE_INHERIT,
+  SPACING_3,
+  Btn,
+  Text,
+} from '@opentrons/components'
 
 type SkipAppUpdateMessageProps = {|
   onClick: () => mixed,
@@ -16,9 +23,9 @@ export function SkipAppUpdateMessage(
   return (
     <Text paddingLeft={SPACING_3}>
       {SKIP_APP_MESSAGE}
-      <Link href="#" color={C_BLUE} onClick={props.onClick}>
+      <Btn color={C_BLUE} onClick={props.onClick} fontSize={FONT_SIZE_INHERIT}>
         {CLICK_HERE}
-      </Link>
+      </Btn>
       .
     </Text>
   )

app/src/components/RobotSettings/__tests__/ControlsCard.test.js

@@ -232,7 +232,7 @@ describe('ControlsCard', () => {
       return Calibration.DECK_CAL_STATUS_BAD_CALIBRATION
     })
 
-    const { wrapper } = render()
+    const { wrapper, refresh } = render()
 
     expect(getCheckCalibrationControl(wrapper).prop('disabledReason')).toBe(
       'Bad deck calibration detected. Please perform a full deck calibration.'
@@ -241,8 +241,7 @@ describe('ControlsCard', () => {
     getDeckCalibrationStatus.mockReturnValue(
       Calibration.DECK_CAL_STATUS_SINGULARITY
     )
-    wrapper.setProps({})
-    wrapper.update()
+    refresh()
 
     expect(getCheckCalibrationControl(wrapper).prop('disabledReason')).toBe(
       'Bad deck calibration detected. Please perform a full deck calibration.'
@@ -251,8 +250,7 @@ describe('ControlsCard', () => {
     getDeckCalibrationStatus.mockReturnValue(
       Calibration.DECK_CAL_STATUS_IDENTITY
     )
-    wrapper.setProps({})
-    wrapper.update()
+    refresh()
 
     expect(getCheckCalibrationControl(wrapper).prop('disabledReason')).toBe(
       'Please perform a full deck calibration.'