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 <[email protected]>
Opentrons · Oct 14, 2020 · 7124bd9 · 7124bd9
1 parent 00fe420
commit 7124bd9
Show file tree

Hide file tree

Showing 4 changed files with 180 additions and 81 deletions.
diff --git a/...tion/pipette-offset/epic/__tests__/fetchPipetteOffsetCalibrationsOnCalibrationEnd.test.js b/...tion/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
@@ -1,44 +1,62 @@
 // @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,
     SESSION_TYPE_DECK_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 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/.../calibration/tip-length/epic/__tests__/fetchTipLengthCalibrationsOnCalibrationEnd.test.js b/.../calibration/tip-length/epic/__tests__/fetchTipLengthCalibrationsOnCalibrationEnd.test.js
@@ -8,39 +8,53 @@ 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,
+    },
   },
 ]
 
 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('---')
       })
     })
   })