Skip to content

Commit

Permalink
refactor(app): refactor protocol event analytics hooks
Browse files Browse the repository at this point in the history
Recent implementation of analytics hooks for protocol events broke a rule of hooks, resulting in
broken functionality when trying to proceed to protocol setup. Here, I refactor the use of
'useRobot' into 'useTrackProtocolRunEvent' to follow the rule of hooks and avoid the use of
'useRobot' in the parsing utility function 'parseProtocolRunAnalyticsData'.
  • Loading branch information
ncdiehl11 committed Feb 12, 2024
1 parent 416d823 commit b3380d0
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { useStoredProtocolAnalysis, useProtocolDetailsForRun } from '../'
import { useProtocolMetadata } from '../useProtocolMetadata'
import { useRunTimestamps } from '../../../RunTimeControl/hooks'
import { formatInterval } from '../../../RunTimeControl/utils'
import { mockConnectableRobot } from '../../../../redux/discovery/__fixtures__'

jest.mock('../../../../redux/analytics/hash')
jest.mock('../../../../redux/protocol-storage')
Expand Down Expand Up @@ -45,7 +46,6 @@ let store: Store<any> = createStore(jest.fn(), {})

const RUN_ID = '1'
const RUN_ID_2 = '2'
const ROBOT_NAME = 'otie'

const PIPETTES = [
{ id: '1', pipetteName: 'testModelLeft' },
Expand Down Expand Up @@ -113,7 +113,7 @@ describe('useProtocolAnalysisErrors hook', () => {

it('returns getProtocolRunAnalyticsData function', () => {
const { result } = renderHook(
() => useProtocolRunAnalyticsData(RUN_ID, ROBOT_NAME),
() => useProtocolRunAnalyticsData(RUN_ID, mockConnectableRobot),
{
wrapper,
}
Expand All @@ -128,7 +128,7 @@ describe('useProtocolAnalysisErrors hook', () => {
.calledWith(RUN_ID_2)
.mockReturnValue({ protocolData: ROBOT_PROTOCOL_ANALYSIS } as any)
const { result } = renderHook(
() => useProtocolRunAnalyticsData(RUN_ID_2, ROBOT_NAME),
() => useProtocolRunAnalyticsData(RUN_ID_2, mockConnectableRobot),
{
wrapper,
}
Expand Down Expand Up @@ -157,7 +157,7 @@ describe('useProtocolAnalysisErrors hook', () => {

it('getProtocolRunAnalyticsData returns fallback stored data when robot data unavailable', async () => {
const { result } = renderHook(
() => useProtocolRunAnalyticsData(RUN_ID, ROBOT_NAME),
() => useProtocolRunAnalyticsData(RUN_ID, mockConnectableRobot),
{
wrapper,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,21 @@ import {
useTrackEvent,
ANALYTICS_PROTOCOL_RUN_START,
} from '../../../../redux/analytics'
import { mockConnectableRobot } from '../../../../redux/discovery/__fixtures__'
import { useRobot } from '../useRobot'

jest.mock('../../hooks')
jest.mock('../useProtocolRunAnalyticsData')
jest.mock('../../../../redux/discovery')
jest.mock('../../../../redux/pipettes')
jest.mock('../../../../redux/analytics')
jest.mock('../../../../redux/robot-settings')
jest.mock('../useRobot')

const mockUseTrackEvent = useTrackEvent as jest.MockedFunction<
typeof useTrackEvent
>
const mockUseRobot = useRobot as jest.MockedFunction<typeof useRobot>
const mockUseProtocolRunAnalyticsData = useProtocolRunAnalyticsData as jest.MockedFunction<
typeof useProtocolRunAnalyticsData
>
Expand Down Expand Up @@ -55,8 +59,9 @@ describe('useTrackProtocolRunEvent hook', () => {
)
)
mockUseTrackEvent.mockReturnValue(mockTrackEvent)
mockUseRobot.mockReturnValue(mockConnectableRobot)
when(mockUseProtocolRunAnalyticsData)
.calledWith(RUN_ID, ROBOT_NAME)
.calledWith(RUN_ID, mockConnectableRobot)
.mockReturnValue({
getProtocolRunAnalyticsData: mockGetProtocolRunAnalyticsData,
})
Expand Down Expand Up @@ -98,7 +103,7 @@ describe('useTrackProtocolRunEvent hook', () => {

it('trackProtocolRunEvent calls trackEvent without props when error is thrown in getProtocolRunAnalyticsData', async () => {
when(mockUseProtocolRunAnalyticsData)
.calledWith('errorId', ROBOT_NAME)
.calledWith('errorId', mockConnectableRobot)
.mockReturnValue({
getProtocolRunAnalyticsData: () =>
new Promise(() => {
Expand Down
14 changes: 5 additions & 9 deletions app/src/organisms/Devices/hooks/useProtocolRunAnalyticsData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,7 @@ import { useSelector } from 'react-redux'
import { hash } from '../../../redux/analytics/hash'
import { getStoredProtocol } from '../../../redux/protocol-storage'
import { getRobotSerialNumber } from '../../../redux/discovery'
import {
useRobot,
useStoredProtocolAnalysis,
useProtocolDetailsForRun,
} from './'
import { useStoredProtocolAnalysis, useProtocolDetailsForRun } from './'
import { useProtocolMetadata } from './useProtocolMetadata'
import { useRunTimestamps } from '../../RunTimeControl/hooks'
import { formatInterval } from '../../RunTimeControl/utils'
Expand All @@ -17,19 +13,19 @@ import type { ProtocolAnalyticsData } from '../../../redux/analytics/types'
import type { StoredProtocolData } from '../../../redux/protocol-storage/types'
import type { ProtocolAnalysisOutput } from '@opentrons/shared-data'
import type { State } from '../../../redux/types'
import { DiscoveredRobot } from '../../../redux/discovery/types'

export const parseProtocolRunAnalyticsData = (
protocolAnalysis: ProtocolAnalysisOutput | null,
storedProtocol: StoredProtocolData | null,
startedAt: string | null,
robotName: string
robot: DiscoveredRobot | null
) => () => {
const hashTasks = [
hash(protocolAnalysis?.metadata?.author) ?? '',
hash(storedProtocol?.srcFiles?.toString() ?? '') ?? '',
]

const robot = useRobot(robotName)
const serialNumber =
robot?.status != null ? getRobotSerialNumber(robot) : null

Expand Down Expand Up @@ -80,7 +76,7 @@ type GetProtocolRunAnalyticsData = () => Promise<{
*/
export function useProtocolRunAnalyticsData(
runId: string | null,
robotName: string
robot: DiscoveredRobot | null
): {
getProtocolRunAnalyticsData: GetProtocolRunAnalyticsData
} {
Expand Down Expand Up @@ -109,7 +105,7 @@ export function useProtocolRunAnalyticsData(
protocolAnalysis as ProtocolAnalysisOutput | null,
storedProtocol,
startedAt,
robotName
robot
)

return { getProtocolRunAnalyticsData }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { parseProtocolRunAnalyticsData } from './useProtocolRunAnalyticsData'
import { parseProtocolAnalysisOutput } from './useStoredProtocolAnalysis'

import type { StoredProtocolData } from '../../../redux/protocol-storage'
import { useRobot } from './useRobot'

type CreateProtocolRunEventName =
| 'createProtocolRecordRequest'
Expand All @@ -23,6 +24,8 @@ export function useTrackCreateProtocolRunEvent(
): { trackCreateProtocolRunEvent: TrackCreateProtocolRunEvent } {
const trackEvent = useTrackEvent()

const robot = useRobot(robotName)

const storedProtocolAnalysis = parseProtocolAnalysisOutput(
protocol?.mostRecentAnalysis ?? null
)
Expand All @@ -31,7 +34,7 @@ export function useTrackCreateProtocolRunEvent(
storedProtocolAnalysis,
protocol,
null,
robotName
robot
)

const trackCreateProtocolRunEvent: TrackCreateProtocolRunEvent = ({
Expand Down
4 changes: 3 additions & 1 deletion app/src/organisms/Devices/hooks/useTrackProtocolRunEvent.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { useTrackEvent } from '../../../redux/analytics'
import { useProtocolRunAnalyticsData } from './useProtocolRunAnalyticsData'
import { useRobot } from './useRobot'

interface ProtocolRunAnalyticsEvent {
name: string
Expand All @@ -15,9 +16,10 @@ export function useTrackProtocolRunEvent(
robotName: string
): { trackProtocolRunEvent: TrackProtocolRunEvent } {
const trackEvent = useTrackEvent()
const robot = useRobot(robotName)
const { getProtocolRunAnalyticsData } = useProtocolRunAnalyticsData(
runId,
robotName
robot
)

const trackProtocolRunEvent: TrackProtocolRunEvent = ({
Expand Down

0 comments on commit b3380d0

Please sign in to comment.