From 7124bd91718711b218c465b95c71bfd1330d5bd8 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Wed, 14 Oct 2020 10:29:37 -0400 Subject: [PATCH] fix(app): fix fetch-on-session-end epics for POC, TLC (#6757) The epics to fetch pipette offset calibration and tip length calibration were using the wrong rxjs operators to emit their actions, and consequently would fire at the right times but not actually incur a fetch from the robot. In addition, the UI works much better if the data is fetched when the session delete _starts_ rather than completes successfully, since most session wizards go away when the delete begins and therefore let the user see the places where the data goes. Closes #6747 Co-authored-by: ahiuchingau <20424172+ahiuchingau@users.noreply.github.com> --- ...OffsetCalibrationsOnCalibrationEnd.test.js | 82 ++++++++++------ ...petteOffsetCalibrationsOnCalibrationEnd.js | 42 +++++--- ...LengthCalibrationsOnCalibrationEnd.test.js | 95 +++++++++++++------ ...chTipLengthCalibrationsOnCalibrationEnd.js | 42 +++++--- 4 files changed, 180 insertions(+), 81 deletions(-) diff --git a/app/src/calibration/pipette-offset/epic/__tests__/fetchPipetteOffsetCalibrationsOnCalibrationEnd.test.js b/app/src/calibration/pipette-offset/epic/__tests__/fetchPipetteOffsetCalibrationsOnCalibrationEnd.test.js index e45351bcab1..d962c200060 100644 --- a/app/src/calibration/pipette-offset/epic/__tests__/fetchPipetteOffsetCalibrationsOnCalibrationEnd.test.js +++ b/app/src/calibration/pipette-offset/epic/__tests__/fetchPipetteOffsetCalibrationsOnCalibrationEnd.test.js @@ -8,35 +8,52 @@ import { pipetteOffsetCalibrationsEpic } from '..' import * as SessionFixtures from '../../../../sessions/__fixtures__' import * as SessionTypes from '../../../../sessions/types' import * as SessionActions from '../../../../sessions/actions' +import * as SessionSelectors from '../../../../sessions/selectors' jest.mock('../../actions') jest.mock('../../../../robot/selectors') +jest.mock('../../../../sessions/selectors') const mockState = { state: true } +const mockRobotName = 'robot-name' const mockGetConnectedRobotName: JestMockFn<[any], ?string> = RobotSelectors.getConnectedRobotName +const mockGetRobotSessionById: JestMockFn<[any, string, string], mixed> = + SessionSelectors.getRobotSessionById + const SPECS: Array<{| describe: string, - response: SessionTypes.SessionResponse, + robotSession: SessionTypes.Session, |}> = [ { describe: 'tip length calibration', - response: SessionFixtures.mockDeleteTipLengthCalibrationSessionSuccess.body, + robotSession: { + ...SessionFixtures.mockTipLengthCalibrationSessionAttributes, + id: SessionFixtures.mockSessionId, + }, }, { describe: 'check calibration', - response: SessionFixtures.mockDeleteSessionSuccess.body, + robotSession: { + ...SessionFixtures.mockCalibrationCheckSessionAttributes, + id: SessionFixtures.mockSessionId, + }, }, { describe: 'deck calibration', - response: SessionFixtures.mockDeleteDeckCalibrationSessionSuccess.body, + robotSession: { + ...SessionFixtures.mockDeckCalibrationSessionAttributes, + id: SessionFixtures.mockSessionId, + }, }, { describe: 'pipette offset calibration', - response: - SessionFixtures.mockDeletePipetteOffsetCalibrationSessionSuccess.body, + robotSession: { + ...SessionFixtures.mockTipLengthCalibrationSessionAttributes, + id: SessionFixtures.mockSessionId, + }, }, ] @@ -55,39 +72,44 @@ describe('fetchPipetteOffsetCalibrationsOnCalibrationEndEpic', () => { jest.resetAllMocks() }) - it('dispatches nothing on calibration session delete failure', () => { - const action = SessionActions.deleteSessionFailure( - 'robot-api', - 'some-session-id', - SessionFixtures.mockDeleteSessionFailure.body, - {} - ) + SPECS.forEach(({ describe, robotSession }) => { + it(`dispatches FETCH_PIPETTE_OFFSET_CALIBRATIONS on ${describe} delete`, () => { + mockGetConnectedRobotName.mockReturnValue(mockRobotName) + mockGetRobotSessionById.mockReturnValue(robotSession) - mockGetConnectedRobotName.mockReturnValue(null) + const action = SessionActions.deleteSession( + mockRobotName, + SessionFixtures.mockSessionId + ) - testScheduler.run(({ hot, cold, expectObservable, flush }) => { - const action$ = hot('--a', { a: action }) - const state$ = hot('a--', { a: mockState }) - const output$ = pipetteOffsetCalibrationsEpic(action$, state$) + testScheduler.run(({ hot, expectObservable, flush }) => { + const action$ = hot('--a', { a: action }) + const state$ = hot('s-s', { s: mockState }) + const output$ = pipetteOffsetCalibrationsEpic(action$, state$) - expectObservable(output$).toBe('---') + expectObservable(output$).toBe('--a', { + a: Actions.fetchPipetteOffsetCalibrations(mockRobotName), + }) + }) }) }) - SPECS.forEach(({ describe, response }) => { - it(`dispatches FETCH_PIPETTE_OFFSET_CALIBRATIONS on ${describe} delete success`, () => { - mockGetConnectedRobotName.mockReturnValue('robot-name') + SPECS.forEach(({ describe, robotSession }) => { + it(`dispatches nothing on ${describe} fetch`, () => { + mockGetConnectedRobotName.mockReturnValue(mockRobotName) + mockGetRobotSessionById.mockReturnValue(robotSession) - testScheduler.run(({ hot, cold, expectObservable, flush }) => { - const action$ = hot('--a', { - a: SessionActions.deleteSessionSuccess('robot-name', response, {}), - }) - const state$ = hot('a--', { a: mockState }) + const action = SessionActions.fetchSession( + mockRobotName, + SessionFixtures.mockSessionId + ) + + testScheduler.run(({ hot, expectObservable, flush }) => { + const action$ = hot('--a', { a: action }) + const state$ = hot('s-s', { s: mockState }) const output$ = pipetteOffsetCalibrationsEpic(action$, state$) - expectObservable(output$).toBe('--a', { - a: Actions.fetchPipetteOffsetCalibrations('robot-name'), - }) + expectObservable(output$).toBe('---') }) }) }) diff --git a/app/src/calibration/pipette-offset/epic/fetchPipetteOffsetCalibrationsOnCalibrationEnd.js b/app/src/calibration/pipette-offset/epic/fetchPipetteOffsetCalibrationsOnCalibrationEnd.js index b9b863eb5a0..1148e8ac27e 100644 --- a/app/src/calibration/pipette-offset/epic/fetchPipetteOffsetCalibrationsOnCalibrationEnd.js +++ b/app/src/calibration/pipette-offset/epic/fetchPipetteOffsetCalibrationsOnCalibrationEnd.js @@ -1,23 +1,23 @@ // @flow -import { of } from 'rxjs' -import { filter, withLatestFrom, mergeMap } from 'rxjs/operators' +import { filter, withLatestFrom, map } from 'rxjs/operators' import { ofType } from 'redux-observable' import { getConnectedRobotName } from '../../../robot/selectors' import * as Actions from '../actions' -import type { Epic } from '../../../types' +import type { Epic, State } from '../../../types' import type { SessionType } from '../../../sessions/types' import { - DELETE_SESSION_SUCCESS, + DELETE_SESSION, SESSION_TYPE_CALIBRATION_CHECK, SESSION_TYPE_TIP_LENGTH_CALIBRATION, SESSION_TYPE_DECK_CALIBRATION, SESSION_TYPE_PIPETTE_OFFSET_CALIBRATION, } from '../../../sessions/constants' +import { getRobotSessionById } from '../../../sessions/selectors' -const sessionIncursRefetch: SessionType => boolean = sessionType => +const isTargetSessionType: SessionType => boolean = sessionType => [ SESSION_TYPE_CALIBRATION_CHECK, SESSION_TYPE_TIP_LENGTH_CALIBRATION, @@ -25,20 +25,38 @@ const sessionIncursRefetch: SessionType => boolean = sessionType => SESSION_TYPE_PIPETTE_OFFSET_CALIBRATION, ].includes(sessionType) +const sessionIncursRefetch: ( + state: State, + robotName: string, + sessionId: string +) => boolean = (state, robotName, sessionId) => { + const sessionType = + getRobotSessionById(state, robotName, sessionId)?.sessionType || null + if (sessionType) { + return isTargetSessionType(sessionType) + } else { + return false + } +} + export const fetchPipetteOffsetCalibrationsOnCalibrationEndEpic: Epic = ( action$, state$ ) => { return action$.pipe( - ofType(DELETE_SESSION_SUCCESS), - withLatestFrom(state$, (a, s) => [a, getConnectedRobotName(s)]), + ofType(DELETE_SESSION), + withLatestFrom(state$, (a, s) => [ + a, + s, + getConnectedRobotName(s), + a.payload.sessionId, + ]), filter( - ([action, robotName]) => - sessionIncursRefetch(action.payload.data.attributes.sessionType) && - robotName != null + ([action, state, robotName, sessionId]) => + robotName != null && sessionIncursRefetch(state, robotName, sessionId) ), - mergeMap(robotName => { - return of(Actions.fetchPipetteOffsetCalibrations(robotName)) + map(robotName => { + return Actions.fetchPipetteOffsetCalibrations(robotName) }) ) } diff --git a/app/src/calibration/tip-length/epic/__tests__/fetchTipLengthCalibrationsOnCalibrationEnd.test.js b/app/src/calibration/tip-length/epic/__tests__/fetchTipLengthCalibrationsOnCalibrationEnd.test.js index b2d6d1a45bc..55d508c2464 100644 --- a/app/src/calibration/tip-length/epic/__tests__/fetchTipLengthCalibrationsOnCalibrationEnd.test.js +++ b/app/src/calibration/tip-length/epic/__tests__/fetchTipLengthCalibrationsOnCalibrationEnd.test.js @@ -8,31 +8,45 @@ import { tipLengthCalibrationsEpic } from '..' import * as SessionFixtures from '../../../../sessions/__fixtures__' import * as SessionTypes from '../../../../sessions/types' import * as SessionActions from '../../../../sessions/actions' +import * as SessionSelectors from '../../../../sessions/selectors' jest.mock('../../actions') jest.mock('../../../../robot/selectors') +jest.mock('../../../../sessions/selectors') const mockState = { state: true } +const mockRobotName = 'robot-name' const mockGetConnectedRobotName: JestMockFn<[any], ?string> = RobotSelectors.getConnectedRobotName +const mockGetRobotSessionById: JestMockFn<[any, string, string], mixed> = + SessionSelectors.getRobotSessionById + const SPECS: Array<{| describe: string, - response: SessionTypes.SessionResponse, + robotSession: SessionTypes.Session, |}> = [ { describe: 'tip length calibration', - response: SessionFixtures.mockDeleteTipLengthCalibrationSessionSuccess.body, + robotSession: { + ...SessionFixtures.mockTipLengthCalibrationSessionAttributes, + id: SessionFixtures.mockSessionId, + }, }, { describe: 'check calibration', - response: SessionFixtures.mockDeleteSessionSuccess.body, + robotSession: { + ...SessionFixtures.mockCalibrationCheckSessionAttributes, + id: SessionFixtures.mockSessionId, + }, }, { describe: 'pipette offset calibration', - response: - SessionFixtures.mockDeletePipetteOffsetCalibrationSessionSuccess.body, + robotSession: { + ...SessionFixtures.mockTipLengthCalibrationSessionAttributes, + id: SessionFixtures.mockSessionId, + }, }, ] @@ -40,7 +54,7 @@ describe('fetchTipLengthCalibrationsOnCalibrationEndEpic', () => { let testScheduler beforeEach(() => { - mockGetConnectedRobotName.mockReturnValue('robot-name') + mockGetConnectedRobotName.mockReturnValue(mockRobotName) testScheduler = new TestScheduler((actual, expected) => { expect(actual).toEqual(expected) @@ -51,39 +65,66 @@ describe('fetchTipLengthCalibrationsOnCalibrationEndEpic', () => { jest.resetAllMocks() }) - it('dispatches nothing on calibration session delete failure', () => { - const action = SessionActions.deleteSessionFailure( - 'robot-api', - 'some-session-id', - SessionFixtures.mockDeleteSessionFailure.body, - {} - ) + SPECS.forEach(({ describe, robotSession }) => { + it(`dispatches FETCH_TIP_LENGTH_CALIBRATIONS on ${describe} delete`, () => { + mockGetConnectedRobotName.mockReturnValue(mockRobotName) + mockGetRobotSessionById.mockReturnValue(robotSession) + + const action = SessionActions.deleteSession( + mockRobotName, + SessionFixtures.mockSessionId + ) + + testScheduler.run(({ hot, expectObservable, flush }) => { + const action$ = hot('--a', { a: action }) + const state$ = hot('s-s', { s: mockState }) + const output$ = tipLengthCalibrationsEpic(action$, state$) + + expectObservable(output$).toBe('--a', { + a: Actions.fetchTipLengthCalibrations(mockRobotName), + }) + }) + }) + }) - mockGetConnectedRobotName.mockReturnValue(null) + it('dispatches nothing on other session (i.e. deck calibration) delete', () => { + const mockDeckCalSession = { + ...SessionFixtures.mockDeckCalibrationSessionAttributes, + id: SessionFixtures.mockSessionId, + } + mockGetConnectedRobotName.mockReturnValue(mockRobotName) + mockGetRobotSessionById.mockReturnValue(mockDeckCalSession) + + const action = SessionActions.deleteSession( + mockRobotName, + SessionFixtures.mockSessionId + ) - testScheduler.run(({ hot, cold, expectObservable, flush }) => { + testScheduler.run(({ hot, expectObservable, flush }) => { const action$ = hot('--a', { a: action }) - const state$ = hot('a--', { a: mockState }) + const state$ = hot('s-s', { s: mockState }) const output$ = tipLengthCalibrationsEpic(action$, state$) expectObservable(output$).toBe('---') }) }) - SPECS.forEach(({ describe, response }) => { - it(`dispatches FETCH_TIP_LENGTH_CALIBRATIONS on ${describe} delete success`, () => { - mockGetConnectedRobotName.mockReturnValue('robot-name') + SPECS.forEach(({ describe, robotSession }) => { + it(`dispatches nothing on ${describe} fetch`, () => { + mockGetConnectedRobotName.mockReturnValue(mockRobotName) + mockGetRobotSessionById.mockReturnValue(robotSession) - testScheduler.run(({ hot, cold, expectObservable, flush }) => { - const action$ = hot('--a', { - a: SessionActions.deleteSessionSuccess('robot-name', response, {}), - }) - const state$ = hot('a--', { a: mockState }) + const action = SessionActions.fetchSession( + mockRobotName, + SessionFixtures.mockSessionId + ) + + testScheduler.run(({ hot, expectObservable, flush }) => { + const action$ = hot('--a', { a: action }) + const state$ = hot('s-s', { s: mockState }) const output$ = tipLengthCalibrationsEpic(action$, state$) - expectObservable(output$).toBe('--a', { - a: Actions.fetchTipLengthCalibrations('robot-name'), - }) + expectObservable(output$).toBe('---') }) }) }) diff --git a/app/src/calibration/tip-length/epic/fetchTipLengthCalibrationsOnCalibrationEnd.js b/app/src/calibration/tip-length/epic/fetchTipLengthCalibrationsOnCalibrationEnd.js index eaf181fab5b..8da2f6baf30 100644 --- a/app/src/calibration/tip-length/epic/fetchTipLengthCalibrationsOnCalibrationEnd.js +++ b/app/src/calibration/tip-length/epic/fetchTipLengthCalibrationsOnCalibrationEnd.js @@ -1,42 +1,60 @@ // @flow -import { of } from 'rxjs' -import { filter, withLatestFrom, mergeMap } from 'rxjs/operators' +import { filter, withLatestFrom, map } from 'rxjs/operators' import { ofType } from 'redux-observable' import { getConnectedRobotName } from '../../../robot/selectors' import * as Actions from '../actions' -import type { Epic } from '../../../types' +import type { Epic, State } from '../../../types' import type { SessionType } from '../../../sessions/types' import { - DELETE_SESSION_SUCCESS, + DELETE_SESSION, SESSION_TYPE_CALIBRATION_CHECK, SESSION_TYPE_TIP_LENGTH_CALIBRATION, SESSION_TYPE_PIPETTE_OFFSET_CALIBRATION, } from '../../../sessions/constants' +import { getRobotSessionById } from '../../../sessions/selectors' -const sessionIncursRefetch: SessionType => boolean = sessionType => +const isTargetSessionType: SessionType => boolean = sessionType => [ SESSION_TYPE_CALIBRATION_CHECK, SESSION_TYPE_TIP_LENGTH_CALIBRATION, SESSION_TYPE_PIPETTE_OFFSET_CALIBRATION, ].includes(sessionType) +const sessionIncursRefetch: ( + state: State, + robotName: string, + sessionId: string +) => boolean = (state, robotName, sessionId) => { + const sessionType = + getRobotSessionById(state, robotName, sessionId)?.sessionType || null + if (sessionType) { + return isTargetSessionType(sessionType) + } else { + return false + } +} + export const fetchTipLengthCalibrationsOnCalibrationEndEpic: Epic = ( action$, state$ ) => { return action$.pipe( - ofType(DELETE_SESSION_SUCCESS), - withLatestFrom(state$, (a, s) => [a, getConnectedRobotName(s)]), + ofType(DELETE_SESSION), + withLatestFrom(state$, (a, s) => [ + a, + s, + getConnectedRobotName(s), + a.payload.sessionId, + ]), filter( - ([action, robotName]) => - sessionIncursRefetch(action.payload.data.attributes.sessionType) && - robotName != null + ([action, state, robotName, sessionId]) => + robotName != null && sessionIncursRefetch(state, robotName, sessionId) ), - mergeMap(robotName => { - return of(Actions.fetchTipLengthCalibrations(robotName)) + map(([_action, robotName]) => { + return Actions.fetchTipLengthCalibrations(robotName) }) ) }