fix(app): fix infinitely re-rendering/never rendering firmware success toasts

app/src/assets/localization/en/device_details.json

@@ -58,7 +58,7 @@
   "firmware_update_needed": "Instrument firmware update needed. Start the update on the robot's touchscreen.",
   "firmware_update_available": "Firmware update available.",
   "firmware_update_failed": "Failed to update module firmware",
-  "firmware_update_installation_successful": "Installation successful",
+  "firmware_updated_successfully": "Firmware updated successfully",
   "firmware_update_occurring": "Firmware update in progress...",
   "fixture": "Fixture",
   "have_not_run_description": "After you run some protocols, they will appear here.",

app/src/organisms/Devices/InstrumentsAndModules.tsx

@@ -33,6 +33,7 @@ import { PipetteCard } from './PipetteCard'
 import { FlexPipetteCard } from './PipetteCard/FlexPipetteCard'
 import { GripperCard } from '../GripperCard'
 import { useIsEstopNotDisengaged } from '../../resources/devices/hooks/useIsEstopNotDisengaged'
+import { useModuleApiRequests } from '../ModuleCard/utils'
 
 import type {
   BadGripper,
@@ -62,6 +63,7 @@ export function InstrumentsAndModules({
   const currentRunId = useCurrentRunId()
   const { isRunTerminal, isRunRunning } = useRunStatuses()
   const isEstopNotDisengaged = useIsEstopNotDisengaged(robotName)
+  const [getLatestRequestId, handleModuleApiRequests] = useModuleApiRequests()
 
   const { data: attachedInstruments } = useInstrumentsQuery({
     refetchInterval: EQUIPMENT_POLL_MS,
@@ -218,6 +220,8 @@ export function InstrumentsAndModules({
                   attachPipetteRequired={attachPipetteRequired}
                   calibratePipetteRequired={calibratePipetteRequired}
                   updatePipetteFWRequired={updatePipetteFWRequired}
+                  latestRequestId={getLatestRequestId(module.serialNumber)}
+                  handleModuleApiRequests={handleModuleApiRequests}
                 />
               ))}
             </Flex>
@@ -267,6 +271,8 @@ export function InstrumentsAndModules({
                   attachPipetteRequired={attachPipetteRequired}
                   calibratePipetteRequired={calibratePipetteRequired}
                   updatePipetteFWRequired={updatePipetteFWRequired}
+                  latestRequestId={getLatestRequestId(module.serialNumber)}
+                  handleModuleApiRequests={handleModuleApiRequests}
                 />
               ))}
             </Flex>

app/src/organisms/Devices/ProtocolRun/ProtocolRunModuleControls.tsx

@@ -10,6 +10,8 @@ import {
 } from '@opentrons/components'
 import { ModuleCard } from '../../ModuleCard'
 import { useModuleRenderInfoForProtocolById } from '../hooks'
+import { useModuleApiRequests } from '../../ModuleCard/utils'
+
 import type { BadPipette, PipetteData } from '@opentrons/api-client'
 
 interface PipetteStatus {
@@ -77,6 +79,7 @@ export const ProtocolRunModuleControls = ({
     calibratePipetteRequired,
     updatePipetteFWRequired,
   } = usePipetteIsReady()
+  const [getLatestRequestId, handleModuleApiRequests] = useModuleApiRequests()
 
   const moduleRenderInfoForProtocolById = useModuleRenderInfoForProtocolById(
     runId,
@@ -120,6 +123,10 @@ export const ProtocolRunModuleControls = ({
               attachPipetteRequired={attachPipetteRequired}
               calibratePipetteRequired={calibratePipetteRequired}
               updatePipetteFWRequired={updatePipetteFWRequired}
+              latestRequestId={getLatestRequestId(
+                module.attachedModuleMatch.serialNumber
+              )}
+              handleModuleApiRequests={handleModuleApiRequests}
             />
           ) : null
         )}
@@ -141,6 +148,10 @@ export const ProtocolRunModuleControls = ({
               attachPipetteRequired={attachPipetteRequired}
               calibratePipetteRequired={calibratePipetteRequired}
               updatePipetteFWRequired={updatePipetteFWRequired}
+              latestRequestId={getLatestRequestId(
+                module.attachedModuleMatch.serialNumber
+              )}
+              handleModuleApiRequests={handleModuleApiRequests}
             />
           ) : null
         )}

app/src/organisms/ModuleCard/__tests__/ModuleCard.test.tsx

@@ -24,7 +24,6 @@ import {
   getRequestById,
   PENDING,
   SUCCESS,
-  useDispatchApiRequest,
 } from '../../../redux/robot-api'
 import { useCurrentRunStatus } from '../../RunTimeControl/hooks'
 import { useToaster } from '../../ToasterOven'
@@ -43,7 +42,7 @@ import type {
   MagneticModule,
   ThermocyclerModule,
 } from '../../../redux/modules/types'
-import type { DispatchApiRequestType } from '../../../redux/robot-api'
+import type { Mock } from 'vitest'
 
 vi.mock('../ErrorInfo')
 vi.mock('../MagneticModuleData')
@@ -182,32 +181,33 @@ const mockMakeSnackbar = vi.fn()
 const mockMakeToast = vi.fn()
 const mockEatToast = vi.fn()
 
+const MOCK_LATEST_REQUEST_ID = '1234'
+
 const render = (props: React.ComponentProps<typeof ModuleCard>) => {
   return renderWithProviders(<ModuleCard {...props} />, {
     i18nInstance: i18n,
   })[0]
 }
 
 describe('ModuleCard', () => {
-  let dispatchApiRequest: DispatchApiRequestType
   let props: React.ComponentProps<typeof ModuleCard>
+  let mockHandleModuleApiRequests: Mock
 
   beforeEach(() => {
+    mockHandleModuleApiRequests = vi.fn()
+
     props = {
       module: mockMagneticModule,
       robotName: mockRobot.name,
       isLoadedInRun: false,
       attachPipetteRequired: false,
       calibratePipetteRequired: false,
       updatePipetteFWRequired: false,
+      handleModuleApiRequests: mockHandleModuleApiRequests,
+      latestRequestId: MOCK_LATEST_REQUEST_ID,
     }
 
-    dispatchApiRequest = vi.fn()
     vi.mocked(ErrorInfo).mockReturnValue(null)
-    vi.mocked(useDispatchApiRequest).mockReturnValue([
-      dispatchApiRequest,
-      ['id'],
-    ])
     vi.mocked(MagneticModuleData).mockReturnValue(
       <div>Mock Magnetic Module Data</div>
     )

app/src/organisms/ModuleCard/__tests__/utils.test.ts

@@ -1,4 +1,5 @@
-import { describe, expect, it } from 'vitest'
+import { describe, expect, it, vi, beforeEach } from 'vitest'
+import { renderHook, act } from '@testing-library/react'
 
 import {
   mockHeaterShaker,
@@ -9,7 +10,10 @@ import {
   mockThermocycler,
   mockThermocyclerGen2,
 } from '../../../redux/modules/__fixtures__'
-import { getModuleCardImage } from '../utils'
+import { getModuleCardImage, useModuleApiRequests } from '../utils'
+import { useDispatchApiRequest } from '../../../redux/robot-api'
+
+vi.mock('../../../redux/robot-api')
 
 const mockThermocyclerGen2ClosedLid = {
   id: 'thermocycler_id2',
@@ -83,3 +87,29 @@ describe('getModuleCardImage', () => {
     )
   })
 })
+
+const updateModuleAction = { meta: { requestId: '12345' } }
+const MOCK_ROBOT_NAME = 'MOCK_ROBOT'
+const MOCK_SERIAL_NUMBER = '1234'
+const mockDispatchApiRequest = () => updateModuleAction
+
+describe('useModuleApiRequests', () => {
+  beforeEach(() => {
+    vi.mocked(useDispatchApiRequest).mockReturnValue([
+      mockDispatchApiRequest,
+    ] as any)
+  })
+
+  it('should dispatch an API request and update requestIdsBySerial on handleModuleApiRequests', () => {
+    const { result } = renderHook(() => useModuleApiRequests())
+
+    act(() => {
+      result.current[1](MOCK_ROBOT_NAME, MOCK_SERIAL_NUMBER)
+    })
+
+    expect(result.current[0](MOCK_SERIAL_NUMBER)).toEqual(
+      updateModuleAction.meta.requestId
+    )
+    expect(result.current[0]('NON_EXISTENT_SERIAL')).toBeNull()
+  })
+})

app/src/organisms/ModuleCard/index.tsx

@@ -1,7 +1,6 @@
 import * as React from 'react'
 import { Trans, useTranslation } from 'react-i18next'
 import { useDispatch, useSelector } from 'react-redux'
-import last from 'lodash/last'
 import { useHistory } from 'react-router-dom'
 
 import {
@@ -31,9 +30,7 @@ import {
 import { RUN_STATUS_FINISHING, RUN_STATUS_RUNNING } from '@opentrons/api-client'
 
 import { OverflowBtn } from '../../atoms/MenuList/OverflowBtn'
-import { updateModule } from '../../redux/modules'
 import {
-  useDispatchApiRequest,
   getRequestById,
   PENDING,
   FAILURE,
@@ -85,6 +82,8 @@ interface ModuleCardProps {
   attachPipetteRequired: boolean
   calibratePipetteRequired: boolean
   updatePipetteFWRequired: boolean
+  latestRequestId: string | null
+  handleModuleApiRequests: (robotName: string, serialNumber: string) => void
   runId?: string
   slotName?: string
 }
@@ -100,6 +99,8 @@ export const ModuleCard = (props: ModuleCardProps): JSX.Element | null => {
     attachPipetteRequired,
     calibratePipetteRequired,
     updatePipetteFWRequired,
+    latestRequestId,
+    handleModuleApiRequests,
   } = props
   const dispatch = useDispatch<Dispatch>()
   const {
@@ -115,13 +116,12 @@ export const ModuleCard = (props: ModuleCardProps): JSX.Element | null => {
   const [hasSecondary, setHasSecondary] = React.useState(false)
   const [showAboutModule, setShowAboutModule] = React.useState(false)
   const [showTestShake, setShowTestShake] = React.useState(false)
-  const [showHSWizard, setShowHSWizard] = React.useState<boolean>(false)
-  const [showFWBanner, setShowFWBanner] = React.useState<boolean>(true)
-  const [showCalModal, setShowCalModal] = React.useState<boolean>(false)
+  const [showHSWizard, setShowHSWizard] = React.useState(false)
+  const [showFWBanner, setShowFWBanner] = React.useState(true)
+  const [showCalModal, setShowCalModal] = React.useState(false)
 
   const [targetProps, tooltipProps] = useHoverTooltip()
   const history = useHistory()
-  const [dispatchApiRequest, requestIds] = useDispatchApiRequest()
   const runStatus = useCurrentRunStatus({
     onSettled: data => {
       if (data == null) {
@@ -138,29 +138,31 @@ export const ModuleCard = (props: ModuleCardProps): JSX.Element | null => {
     (!attachPipetteRequired ?? false) &&
     (!calibratePipetteRequired ?? false) &&
     (!updatePipetteFWRequired ?? false)
-  const latestRequestId = last(requestIds)
+
   const latestRequest = useSelector<State, RequestState | null>(state =>
-    latestRequestId ? getRequestById(state, latestRequestId) : null
+    latestRequestId != null ? getRequestById(state, latestRequestId) : null
   )
-  const isEstopNotDisengaged = useIsEstopNotDisengaged(robotName)
 
-  const handleCloseErrorModal = (): void => {
-    if (latestRequestId != null) {
-      dispatch(dismissRequest(latestRequestId))
-    }
+  const hasUpdated =
+    !module.hasAvailableUpdate && latestRequest?.status === SUCCESS
+  const [showFirmwareToast, setShowFirmwareToast] = React.useState(hasUpdated)
+  const { makeToast } = useToaster()
+  if (showFirmwareToast) {
+    makeToast(t('firmware_updated_successfully'), SUCCESS_TOAST)
+    setShowFirmwareToast(false)
   }
 
   const handleFirmwareUpdateClick = (): void => {
-    robotName &&
-      dispatchApiRequest(updateModule(robotName, module.serialNumber))
+    robotName && handleModuleApiRequests(robotName, module.serialNumber)
   }
 
-  const { makeToast } = useToaster()
-  React.useEffect(() => {
-    if (!module.hasAvailableUpdate && latestRequest?.status === SUCCESS) {
-      makeToast(t('firmware_update_installation_successful'), SUCCESS_TOAST)
+  const isEstopNotDisengaged = useIsEstopNotDisengaged(robotName)
+
+  const handleCloseErrorModal = (): void => {
+    if (latestRequestId != null) {
+      dispatch(dismissRequest(latestRequestId))
     }
-  }, [module.hasAvailableUpdate, latestRequest?.status, makeToast, t])
+  }
 
   const isPending = latestRequest?.status === PENDING
   const hotToTouch: IconProps = { name: 'ot-hot-to-touch' }

app/src/organisms/ModuleCard/utils.ts

@@ -1,10 +1,17 @@
+import * as React from 'react'
+import last from 'lodash/last'
+
+import { useDispatchApiRequest } from '../../redux/robot-api'
+import { updateModule } from '../../redux/modules'
+
 import magneticModule from '../../assets/images/magnetic_module_gen_2_transparent.png'
 import temperatureModule from '../../assets/images/temp_deck_gen_2_transparent.png'
 import thermoModuleGen1Closed from '../../assets/images/thermocycler_closed.png'
 import thermoModuleGen1Opened from '../../assets/images/thermocycler_open_transparent.png'
 import heaterShakerModule from '../../assets/images/heater_shaker_module_transparent.png'
 import thermoModuleGen2Closed from '../../assets/images/thermocycler_gen_2_closed.png'
 import thermoModuleGen2Opened from '../../assets/images/thermocycler_gen_2_opened.png'
+
 import type { AttachedModule } from '../../redux/modules/types'
 
 export function getModuleCardImage(attachedModule: AttachedModule): string {
@@ -35,3 +42,58 @@ export function getModuleCardImage(attachedModule: AttachedModule): string {
       return 'unknown module model, this is an error'
   }
 }
+
+type RequestIdsBySerialNumber = Record<string, string[]>
+type HandleModuleApiRequestsType = (robotName: string, moduleId: string) => void
+type GetLatestRequestIdType = (moduleId: string) => string | null
+
+export function useModuleApiRequests(): [
+  GetLatestRequestIdType,
+  HandleModuleApiRequestsType
+] {
+  const [dispatchApiRequest] = useDispatchApiRequest()
+  const [
+    requestIdsBySerial,
+    setRequestIdsBySerial,
+  ] = React.useState<RequestIdsBySerialNumber>({})
+
+  const handleModuleApiRequests = (
+    robotName: string,
+    serialNumber: string
+  ): void => {
+    const action = dispatchApiRequest(updateModule(robotName, serialNumber))
+    const { requestId } = action.meta
+
+    if (requestId != null) {
+      if (serialNumber in requestIdsBySerial) {
+        setRequestIdsBySerial((prevState: RequestIdsBySerialNumber) => {
+          const existingRequestIds = prevState[serialNumber] || []
+          return {
+            ...prevState,
+            [serialNumber]: [...existingRequestIds, requestId],
+          }
+        })
+      } else {
+        setRequestIdsBySerial(prevState => {
+          return {
+            ...prevState,
+            [serialNumber]: [requestId],
+          }
+        })
+      }
+    }
+  }
+
+  const getLatestRequestId = React.useCallback(
+    (serialNumber: string): string | null => {
+      if (serialNumber in requestIdsBySerial) {
+        return last(requestIdsBySerial[serialNumber]) ?? null
+      } else {
+        return null
+      }
+    },
+    [requestIdsBySerial]
+  )
+
+  return [getLatestRequestId, handleModuleApiRequests]
+}