diff --git a/app-shell/src/labware/__tests__/dispatch.test.js b/app-shell/src/labware/__tests__/dispatch.test.js index d8991433674..c5e6d2f823a 100644 --- a/app-shell/src/labware/__tests__/dispatch.test.js +++ b/app-shell/src/labware/__tests__/dispatch.test.js @@ -206,9 +206,26 @@ describe('labware module dispatches', () => { changeHandler() - return flush().then(() => + return flush().then(() => { expect(readLabwareDirectory).toHaveBeenCalledWith(labwareDir) - ) + expect(dispatch).toHaveBeenCalledWith( + CustomLabware.customLabwareList([], 'changeDirectory') + ) + }) + }) + + test('dispatches labware directory list error on config change', () => { + const changeHandler = handleConfigChange.mock.calls[0][1] + + readLabwareDirectory.mockRejectedValue((new Error('AH'): any)) + changeHandler() + + return flush().then(() => { + expect(readLabwareDirectory).toHaveBeenCalledWith(labwareDir) + expect(dispatch).toHaveBeenCalledWith( + CustomLabware.customLabwareListFailure('AH', 'changeDirectory') + ) + }) }) test('opens file picker on ADD_CUSTOM_LABWARE', () => { @@ -270,7 +287,10 @@ describe('labware module dispatches', () => { test('adds file and triggers a re-scan if valid', () => { const mockValidFile = CustomLabwareFixtures.mockValidLabware - const expectedAction = CustomLabware.customLabwareList([mockValidFile]) + const expectedAction = CustomLabware.customLabwareList( + [mockValidFile], + 'addLabware' + ) showOpenFileDialog.mockResolvedValue([mockValidFile.filename]) validateNewLabwareFile.mockReturnValueOnce(mockValidFile) @@ -314,7 +334,10 @@ describe('labware module dispatches', () => { ({ ...duplicate, filename: '/duplicate2.json' }: DuplicateLabwareFile), ] const mockAfterDeletes = [CustomLabwareFixtures.mockValidLabware] - const expectedAction = CustomLabware.customLabwareList(mockAfterDeletes) + const expectedAction = CustomLabware.customLabwareList( + mockAfterDeletes, + 'overwriteLabware' + ) // validation of existing definitions validateLabwareFiles.mockReturnValueOnce(mockExisting) diff --git a/app-shell/src/labware/index.js b/app-shell/src/labware/index.js index 39aa2b8f560..bb68f87ed5a 100644 --- a/app-shell/src/labware/index.js +++ b/app-shell/src/labware/index.js @@ -12,6 +12,7 @@ import * as ConfigActions from '@opentrons/app/src/config' import type { UncheckedLabwareFile, DuplicateLabwareFile, + CustomLabwareListActionSource as ListSource, } from '@opentrons/app/src/custom-labware/types' import type { Action, Dispatch } from '../types' @@ -25,14 +26,17 @@ const fetchCustomLabware = (): Promise> => { .then(Definitions.parseLabwareFiles) } -const fetchAndValidateCustomLabware = (dispatch: Dispatch): Promise => { +const fetchAndValidateCustomLabware = ( + dispatch: Dispatch, + source: ListSource +): Promise => { return fetchCustomLabware() .then(files => { const payload = validateLabwareFiles(files) - dispatch(CustomLabware.customLabwareList(payload)) + dispatch(CustomLabware.customLabwareList(payload, source)) }) .catch((error: Error) => { - dispatch(CustomLabware.customLabwareListFailure(error.message)) + dispatch(CustomLabware.customLabwareListFailure(error.message, source)) }) } @@ -54,7 +58,9 @@ const overwriteLabware = ( const dir = getFullConfig().labware.directory return Definitions.addLabwareFile(next.filename, dir) }) - .then(() => fetchAndValidateCustomLabware(dispatch)) + .then(() => + fetchAndValidateCustomLabware(dispatch, CustomLabware.OVERWRITE_LABWARE) + ) } const copyLabware = ( @@ -74,21 +80,25 @@ const copyLabware = ( } return Definitions.addLabwareFile(next.filename, dir).then(() => - fetchAndValidateCustomLabware(dispatch) + fetchAndValidateCustomLabware(dispatch, CustomLabware.ADD_LABWARE) ) }) } export function registerLabware(dispatch: Dispatch, mainWindow: {}) { handleConfigChange('labware.directory', () => { - fetchAndValidateCustomLabware(dispatch) + fetchAndValidateCustomLabware(dispatch, CustomLabware.CHANGE_DIRECTORY) }) return function handleActionForLabware(action: Action) { switch (action.type) { case CustomLabware.FETCH_CUSTOM_LABWARE: case 'shell:CHECK_UPDATE': { - fetchAndValidateCustomLabware(dispatch) + const source = + action.type === CustomLabware.FETCH_CUSTOM_LABWARE + ? CustomLabware.POLL + : CustomLabware.INITIAL + fetchAndValidateCustomLabware(dispatch, source) break } diff --git a/app/src/analytics/__tests__/custom-labware-events.test.js b/app/src/analytics/__tests__/custom-labware-events.test.js new file mode 100644 index 00000000000..581ecbc9c1e --- /dev/null +++ b/app/src/analytics/__tests__/custom-labware-events.test.js @@ -0,0 +1,114 @@ +// @flow + +import { makeEvent } from '../make-event' + +import * as CustomLabware from '../../custom-labware' +import * as LabwareFixtures from '../../custom-labware/__fixtures__' + +import type { State, Action } from '../../types' +import type { AnalyticsEvent } from '../types' + +type EventSpec = {| + name: string, + action: Action, + expected: AnalyticsEvent, +|} + +const SPECS: Array = [ + { + name: 'addCustomLabware success', + action: CustomLabware.customLabwareList([], CustomLabware.ADD_LABWARE), + expected: { + name: 'addCustomLabware', + properties: { success: true, overwrite: false, error: '' }, + superProperties: { customLabwareCount: 0 }, + }, + }, + { + name: 'addCustomLabware overwrite success', + action: CustomLabware.customLabwareList( + [], + CustomLabware.OVERWRITE_LABWARE + ), + expected: { + name: 'addCustomLabware', + properties: { success: true, overwrite: true, error: '' }, + superProperties: { customLabwareCount: 0 }, + }, + }, + { + name: 'addCustomLabware failure with bad labware', + action: CustomLabware.addCustomLabwareFailure( + LabwareFixtures.mockInvalidLabware + ), + expected: { + name: 'addCustomLabware', + properties: { + success: false, + overwrite: false, + error: 'INVALID_LABWARE_FILE', + }, + }, + }, + { + name: 'addCustomLabware failure with system error', + action: CustomLabware.addCustomLabwareFailure(null, 'AH'), + expected: { + name: 'addCustomLabware', + properties: { success: false, overwrite: false, error: 'AH' }, + }, + }, + { + name: 'changeLabwareSourceDirectory success', + action: CustomLabware.customLabwareList([], CustomLabware.CHANGE_DIRECTORY), + expected: { + name: 'changeLabwareSourceDirectory', + properties: { success: true, error: '' }, + superProperties: { customLabwareCount: 0 }, + }, + }, + { + name: 'changeLabwareSourceDirectory failure', + action: CustomLabware.customLabwareListFailure( + 'AH', + CustomLabware.CHANGE_DIRECTORY + ), + expected: { + name: 'changeLabwareSourceDirectory', + properties: { success: false, error: 'AH' }, + }, + }, + { + name: 'customLabwareListError on application boot', + action: CustomLabware.customLabwareListFailure('AH', CustomLabware.INITIAL), + expected: { + name: 'customLabwareListError', + properties: { error: 'AH' }, + }, + }, + { + name: 'labware:CUSTOM_LABWARE_LIST sets labware count super property', + action: CustomLabware.customLabwareList( + [ + LabwareFixtures.mockValidLabware, + LabwareFixtures.mockValidLabware, + LabwareFixtures.mockInvalidLabware, + ], + CustomLabware.INITIAL + ), + expected: expect.objectContaining({ + superProperties: { customLabwareCount: 2 }, + }), + }, +] + +const MOCK_STATE: State = ({ mockState: true }: any) + +describe('custom labware analytics events', () => { + SPECS.forEach(spec => { + const { name, action, expected } = spec + test(name, () => { + return expect(makeEvent(action, MOCK_STATE)).resolves.toEqual(expected) + }) + }) +}) diff --git a/app/src/analytics/__tests__/epics.test.js b/app/src/analytics/__tests__/epics.test.js index 0d51cee0da1..c56624f609a 100644 --- a/app/src/analytics/__tests__/epics.test.js +++ b/app/src/analytics/__tests__/epics.test.js @@ -2,7 +2,7 @@ import { TestScheduler } from 'rxjs/testing' import { trackEvent, setMixpanelTracking } from '../mixpanel' -import makeEvent from '../make-event' +import { makeEvent } from '../make-event' import * as epics from '../epics' jest.mock('../make-event') diff --git a/app/src/analytics/__tests__/make-event.test.js b/app/src/analytics/__tests__/make-event.test.js index 8f27822da2d..ef16cc2e4c9 100644 --- a/app/src/analytics/__tests__/make-event.test.js +++ b/app/src/analytics/__tests__/make-event.test.js @@ -1,5 +1,5 @@ // events map tests -import makeEvent from '../make-event' +import { makeEvent } from '../make-event' import { actions as robotActions, selectors as robotSelectors, diff --git a/app/src/analytics/epics.js b/app/src/analytics/epics.js index 745bc3e40c5..7326cca78bf 100644 --- a/app/src/analytics/epics.js +++ b/app/src/analytics/epics.js @@ -13,7 +13,7 @@ import { } from 'rxjs/operators' import { setMixpanelTracking, trackEvent } from './mixpanel' -import makeEvent from './make-event' +import { makeEvent } from './make-event' import type { State, Action, Epic } from '../types' import type { TrackEventArgs, AnalyticsConfig } from './types' diff --git a/app/src/analytics/make-event.js b/app/src/analytics/make-event.js index 26ec07d6e7e..a59fef1e83d 100644 --- a/app/src/analytics/make-event.js +++ b/app/src/analytics/make-event.js @@ -3,6 +3,7 @@ import createLogger from '../logger' import { selectors as robotSelectors } from '../robot' import { getConnectedRobot } from '../discovery' +import * as CustomLabware from '../custom-labware' import * as brActions from '../shell/buildroot/actions' import { getProtocolAnalyticsData, @@ -15,7 +16,7 @@ import type { AnalyticsEvent } from './types' const log = createLogger(__filename) -export default function makeEvent( +export function makeEvent( action: Action, state: State ): Promise { @@ -180,6 +181,77 @@ export default function makeEvent( properties: { ...data }, }) } + + case CustomLabware.CUSTOM_LABWARE_LIST: { + const { payload: labware, meta } = action + const { source } = meta + const customLabwareCount = labware.filter( + lw => lw.type === CustomLabware.VALID_LABWARE_FILE + ).length + const superProperties = { customLabwareCount } + + if ( + source === CustomLabware.ADD_LABWARE || + source === CustomLabware.OVERWRITE_LABWARE + ) { + return Promise.resolve({ + name: 'addCustomLabware', + properties: { + success: true, + overwrite: source === CustomLabware.OVERWRITE_LABWARE, + error: '', + }, + superProperties, + }) + } + + if (source === CustomLabware.CHANGE_DIRECTORY) { + return Promise.resolve({ + name: 'changeLabwareSourceDirectory', + properties: { success: true, error: '' }, + superProperties, + }) + } + + return Promise.resolve({ superProperties }) + } + + case CustomLabware.CUSTOM_LABWARE_LIST_FAILURE: { + const { message: error } = action.payload + const { source } = action.meta + + if (source === CustomLabware.CHANGE_DIRECTORY) { + return Promise.resolve({ + name: 'changeLabwareSourceDirectory', + properties: { success: false, error }, + }) + } + + if (source === CustomLabware.INITIAL) { + return Promise.resolve({ + name: 'customLabwareListError', + properties: { error }, + }) + } + + break + } + + case CustomLabware.ADD_CUSTOM_LABWARE_FAILURE: { + const { labware, message } = action.payload + let error = '' + + if (labware !== null) { + error = labware.type + } else if (message !== null) { + error = message + } + + return Promise.resolve({ + name: 'addCustomLabware', + properties: { success: false, overwrite: false, error }, + }) + } } return Promise.resolve(null) diff --git a/app/src/analytics/mixpanel.js b/app/src/analytics/mixpanel.js index c86f4c165dc..4a20cd4f3ce 100644 --- a/app/src/analytics/mixpanel.js +++ b/app/src/analytics/mixpanel.js @@ -37,7 +37,10 @@ export function trackEvent(event: AnalyticsEvent, config: AnalyticsConfig) { const { optedIn } = config log.debug('Trackable event', { event, optedIn }) - if (MIXPANEL_ID && optedIn) mixpanel.track(event.name, event.properties) + if (MIXPANEL_ID && optedIn) { + if (event.superProperties) mixpanel.register(event.superProperties) + if (event.name) mixpanel.track(event.name, event.properties) + } } export function setMixpanelTracking(config: AnalyticsConfig) { diff --git a/app/src/analytics/types.js b/app/src/analytics/types.js index c17bf5d29b8..46b7523fe88 100644 --- a/app/src/analytics/types.js +++ b/app/src/analytics/types.js @@ -32,9 +32,12 @@ export type BuildrootAnalyticsData = {| error: string | null, |} -export type AnalyticsEvent = {| - name: string, - properties: {}, -|} +export type AnalyticsEvent = + | {| + name: string, + properties: {}, + superProperties?: {}, + |} + | {| superProperties: {} |} export type TrackEventArgs = [AnalyticsEvent, AnalyticsConfig] diff --git a/app/src/components/AddLabwareCard/index.js b/app/src/components/AddLabwareCard/index.js index f650dbb4051..df04b47f478 100644 --- a/app/src/components/AddLabwareCard/index.js +++ b/app/src/components/AddLabwareCard/index.js @@ -11,6 +11,7 @@ import { getAddLabwareFailure, } from '../../custom-labware' +import { CardCopy } from '../layout' import { ManagePath } from './ManagePath' import { AddLabware } from './AddLabware' import { PortaledAddLabwareFailureModal } from './AddLabwareFailureModal' @@ -19,6 +20,8 @@ import type { Dispatch } from '../../types' // TODO(mc, 2019-10-17): i18n const LABWARE_MANAGEMENT = 'Labware Management' +const MANAGE_CUSTOM_LABWARE_DEFINITIONS = + 'Manage custom labware definitions for use in your Python Protocol API Version 2 protocols.' export function AddLabwareCard() { const dispatch = useDispatch() @@ -31,6 +34,7 @@ export function AddLabwareCard() { return ( + {MANAGE_CUSTOM_LABWARE_DEFINITIONS} {showAddFailure && ( diff --git a/app/src/components/layout/CardCopy.js b/app/src/components/layout/CardCopy.js new file mode 100644 index 00000000000..254649baad7 --- /dev/null +++ b/app/src/components/layout/CardCopy.js @@ -0,0 +1,12 @@ +// @flow +import * as React from 'react' + +import styles from './styles.css' + +export type CardCopyProps = {| + children: React.Node, +|} + +export function CardCopy(props: CardCopyProps) { + return

{props.children}

+} diff --git a/app/src/components/layout/index.js b/app/src/components/layout/index.js index 94d82e39b08..3c2230b45c9 100644 --- a/app/src/components/layout/index.js +++ b/app/src/components/layout/index.js @@ -1,6 +1,7 @@ // @flow // TODO(mc, 2019-11-26): flex box these and also no default exports export { default as CardContainer } from './CardContainer' +export * from './CardCopy' export { default as CardRow } from './CardRow' export { default as CardColumn } from './CardColumn' export { default as CardContentFull } from './CardContentFull' diff --git a/app/src/components/layout/styles.css b/app/src/components/layout/styles.css index e56cfeb4751..3199d8dfd44 100644 --- a/app/src/components/layout/styles.css +++ b/app/src/components/layout/styles.css @@ -11,6 +11,13 @@ padding-bottom: 0; } +.card_copy { + @apply --font-body-1-dark; + + line-height: var(--lh-copy); + margin: 0.5rem 1rem 0; +} + .row { width: 100%; margin-bottom: var(--card_spacing); diff --git a/app/src/custom-labware/__tests__/actions.test.js b/app/src/custom-labware/__tests__/actions.test.js index 580bd0cb674..53c44587ca3 100644 --- a/app/src/custom-labware/__tests__/actions.test.js +++ b/app/src/custom-labware/__tests__/actions.test.js @@ -19,7 +19,7 @@ describe('custom labware actions', () => { expected: { type: 'labware:FETCH_CUSTOM_LABWARE', meta: { shell: true } }, }, { - name: 'customLabwareList', + name: 'customLabwareList from poll', creator: actions.customLabwareList, args: [ [ @@ -33,15 +33,42 @@ describe('custom labware actions', () => { { type: 'INVALID_LABWARE_FILE', filename: 'a.json', created: 0 }, { type: 'INVALID_LABWARE_FILE', filename: 'b.json', created: 1 }, ], + meta: { source: 'poll' }, }, }, { - name: 'customLabwareListFailure', + name: 'customLabwareList from source', + creator: actions.customLabwareList, + args: [ + [{ type: 'INVALID_LABWARE_FILE', filename: 'a.json', created: 0 }], + 'changeDirectory', + ], + expected: { + type: 'labware:CUSTOM_LABWARE_LIST', + payload: [ + { type: 'INVALID_LABWARE_FILE', filename: 'a.json', created: 0 }, + ], + meta: { source: 'changeDirectory' }, + }, + }, + { + name: 'customLabwareListFailure from poll', creator: actions.customLabwareListFailure, args: ['AH!'], expected: { type: 'labware:CUSTOM_LABWARE_LIST_FAILURE', payload: { message: 'AH!' }, + meta: { source: 'poll' }, + }, + }, + { + name: 'customLabwareListFailure from source', + creator: actions.customLabwareListFailure, + args: ['AH!', 'changeDirectory'], + expected: { + type: 'labware:CUSTOM_LABWARE_LIST_FAILURE', + payload: { message: 'AH!' }, + meta: { source: 'changeDirectory' }, }, }, { diff --git a/app/src/custom-labware/__tests__/reducer.test.js b/app/src/custom-labware/__tests__/reducer.test.js index 7c7a405c4df..cb41c1a7252 100644 --- a/app/src/custom-labware/__tests__/reducer.test.js +++ b/app/src/custom-labware/__tests__/reducer.test.js @@ -20,6 +20,7 @@ describe('customLabwareReducer', () => { action: { type: 'labware:CUSTOM_LABWARE_LIST', payload: [Fixtures.mockInvalidLabware, Fixtures.mockValidLabware], + meta: { source: 'poll' }, }, expected: { ...INITIAL_STATE, @@ -50,6 +51,7 @@ describe('customLabwareReducer', () => { action: { type: 'labware:CUSTOM_LABWARE_LIST', payload: [Fixtures.mockInvalidLabware], + meta: { source: 'poll' }, }, expected: { filenames: [Fixtures.mockInvalidLabware.filename], @@ -65,6 +67,7 @@ describe('customLabwareReducer', () => { action: { type: 'labware:CUSTOM_LABWARE_LIST_FAILURE', payload: { message: 'AH' }, + meta: { source: 'poll' }, }, expected: { ...INITIAL_STATE, listFailureMessage: 'AH' }, }, diff --git a/app/src/custom-labware/actions.js b/app/src/custom-labware/actions.js index 3a7de3858a0..f739647773e 100644 --- a/app/src/custom-labware/actions.js +++ b/app/src/custom-labware/actions.js @@ -25,6 +25,14 @@ export const ADD_CUSTOM_LABWARE_FAILURE: 'labware:ADD_CUSTOM_LABWARE_FAILURE' = export const CLEAR_ADD_CUSTOM_LABWARE_FAILURE: 'labware:CLEAR_ADD_CUSTOM_LABWARE_FAILURE' = 'labware:CLEAR_ADD_CUSTOM_LABWARE_FAILURE' +// action meta literals + +export const POLL: 'poll' = 'poll' +export const INITIAL: 'initial' = 'initial' +export const ADD_LABWARE: 'addLabware' = 'addLabware' +export const OVERWRITE_LABWARE: 'overwriteLabware' = 'overwriteLabware' +export const CHANGE_DIRECTORY: 'changeDirectory' = 'changeDirectory' + // action creators export const fetchCustomLabware = (): Types.FetchCustomLabwareAction => ({ @@ -33,14 +41,21 @@ export const fetchCustomLabware = (): Types.FetchCustomLabwareAction => ({ }) export const customLabwareList = ( - payload: Array -): Types.CustomLabwareListAction => ({ type: CUSTOM_LABWARE_LIST, payload }) + payload: Array, + source: Types.CustomLabwareListActionSource = POLL +): Types.CustomLabwareListAction => ({ + type: CUSTOM_LABWARE_LIST, + payload, + meta: { source }, +}) export const customLabwareListFailure = ( - message: string + message: string, + source: Types.CustomLabwareListActionSource = POLL ): Types.CustomLabwareListFailureAction => ({ type: CUSTOM_LABWARE_LIST_FAILURE, payload: { message }, + meta: { source }, }) export const changeCustomLabwareDirectory = (): Types.ChangeCustomLabwareDirectoryAction => ({ diff --git a/app/src/custom-labware/types.js b/app/src/custom-labware/types.js index 86411abe897..07e2dfd2350 100644 --- a/app/src/custom-labware/types.js +++ b/app/src/custom-labware/types.js @@ -62,6 +62,13 @@ export type CustomLabwareState = $ReadOnly<{| // action types +export type CustomLabwareListActionSource = + | 'poll' + | 'initial' + | 'addLabware' + | 'overwriteLabware' + | 'changeDirectory' + export type FetchCustomLabwareAction = {| type: 'labware:FETCH_CUSTOM_LABWARE', meta: {| shell: true |}, @@ -70,11 +77,13 @@ export type FetchCustomLabwareAction = {| export type CustomLabwareListAction = {| type: 'labware:CUSTOM_LABWARE_LIST', payload: Array, + meta: {| source: CustomLabwareListActionSource |}, |} export type CustomLabwareListFailureAction = {| type: 'labware:CUSTOM_LABWARE_LIST_FAILURE', payload: {| message: string |}, + meta: {| source: CustomLabwareListActionSource |}, |} export type ChangeCustomLabwareDirectoryAction = {|