fix: keep displayed orders in progress state and dedupe favicon completions

apps/cowswap-frontend/src/entities/surplusModal/index.ts

@@ -11,6 +11,8 @@ const initialState: OrdersToDisplayModal = {
 
 const surplusModalAtom = atom<OrdersToDisplayModal>(initialState)
 
+const surplusModalOrderIdsAtom = atom((get) => get(surplusModalAtom).orderIds)
+
 const addSurplusOrderAtom = atom(null, (get, set, orderId: string) =>
   set(surplusModalAtom, () => {
     const state = get(surplusModalAtom)
@@ -52,3 +54,7 @@ export function useOrderIdForSurplusModal(): string | undefined {
 export function useRemoveOrderFromSurplusQueue(): (orderId: string) => void {
   return useSetAtom(removeSurplusOrderAtom)
 }
+
+export function useSurplusQueueOrderIds(): string[] {
+  return useAtomValue(surplusModalOrderIdsAtom)
+}

apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.test.ts

@@ -0,0 +1,106 @@
+jest.mock('common/favicon', () => {
+  const mockPlay = jest.fn()
+
+  class MockFaviconAnimator {
+    play(_frames: string[], options?: { onComplete?: () => void }): void {
+      mockPlay(_frames)
+      options?.onComplete?.()
+    }
+    stop(): void {}
+    resetToDefault(): void {}
+    setDefaultFrame(): void {}
+    isAnimationRunning(): boolean {
+      return false
+    }
+  }
+
+  return {
+    FaviconAnimator: MockFaviconAnimator,
+    frameDurations: {
+      solving: 100,
+      completed: 100,
+      completedHold: 100,
+      backToDefault: 100,
+    },
+    __mockPlay__: mockPlay,
+  }
+})
+
+import { OrderProgressBarStepName } from 'modules/orderProgressBar'
+import { OrderProgressBarState } from 'modules/orderProgressBar/types'
+
+import { FaviconAnimationController } from './controller'
+
+const { __mockPlay__: mockPlay } = jest.requireMock('common/favicon') as { __mockPlay__: jest.Mock }
+
+describe('FaviconAnimationController', () => {
+  const frameSet = {
+    defaultFrame: 'default.ico',
+    completedFrames: ['completed-1', 'completed-2'],
+    completedHoldFrame: 'completed-hold',
+    backToDefaultFrames: ['back-1'],
+    solvingFrames: ['solving-1'],
+  }
+
+  const NOW = 10_000
+
+  function successState(): OrderProgressBarState {
+    return {
+      progressBarStepName: OrderProgressBarStepName.FINISHED,
+      lastTimeChangedSteps: NOW,
+    }
+  }
+
+  function solvingState(): OrderProgressBarState {
+    return {
+      progressBarStepName: OrderProgressBarStepName.SOLVING,
+      lastTimeChangedSteps: NOW,
+      countdown: 5,
+    }
+  }
+
+  beforeEach(() => {
+    jest.spyOn(Date, 'now').mockReturnValue(NOW)
+    mockPlay.mockClear()
+  })
+
+  afterEach(() => {
+    jest.restoreAllMocks()
+  })
+
+  function countCompletionPlays(): number {
+    return mockPlay.mock.calls.filter(([frames]) => frames === frameSet.completedFrames).length
+  }
+
+  it('only queues a completion animation once for the same finished state', () => {
+    const controller = new FaviconAnimationController(frameSet)
+
+    controller.update({ orderA: successState() })
+    controller.update({}) // state pruned temporarily
+    controller.update({ orderA: successState() }) // re-added while still success
+
+    expect(countCompletionPlays()).toBe(1)
+  })
+
+  it('queues another completion when an order returns to solving and finishes again', () => {
+    const controller = new FaviconAnimationController(frameSet)
+
+    controller.update({ orderA: successState() })
+    controller.update({ orderA: solvingState() })
+    controller.update({ orderA: successState() })
+
+    expect(countCompletionPlays()).toBe(2)
+  })
+
+  it('ignores completion animations triggered while another order is still in progress', () => {
+    const controller = new FaviconAnimationController(frameSet)
+
+    controller.update({ orderA: successState(), orderB: solvingState() })
+    // Completing while another order is still solving should not enqueue anything.
+    expect(countCompletionPlays()).toBe(0)
+
+    controller.update({ orderB: successState() })
+
+    expect(countCompletionPlays()).toBe(1)
+  })
+})

apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.ts

@@ -14,6 +14,8 @@ export class FaviconAnimationController {
   private currentCompletion: string | null = null
   private isInitialized = false
   private frameSet: FrameSet
+  private completedOrders = new Set<string>()
+  private hasInProgress = false
 
   constructor(frameSet: FrameSet) {
     this.frameSet = frameSet
@@ -24,6 +26,7 @@ export class FaviconAnimationController {
     const entries = Object.entries(state)
     const suppressQueue = !this.isInitialized
     const hasInProgress = entries.some(([, value]) => shouldAnimateInProgress(value))
+    this.hasInProgress = hasInProgress
     this.enqueueCompleted(entries, suppressQueue)
     this.isInitialized = true
     if (hasInProgress) {
@@ -80,23 +83,56 @@ export class FaviconAnimationController {
 
   private enqueueCompleted(entries: Array<[string, OrderProgressBarState]>, suppressQueue: boolean): void {
     const nextSteps: Record<string, OrderProgressBarStepName | undefined> = {}
+    const seenOrderIds = new Set<string>()
 
     for (const [orderId, state] of entries) {
       const currentStep = state.progressBarStepName
       nextSteps[orderId] = currentStep
+      seenOrderIds.add(orderId)
 
-      if (
-        !suppressQueue &&
-        isSuccess(currentStep) &&
-        this.previousSteps[orderId] !== currentStep &&
-        isRecentStateChange(state)
-      ) {
-        if (!this.completionQueue.includes(orderId)) this.completionQueue.push(orderId)
-      }
+      this.updateCompletionCache(orderId, currentStep)
+      this.tryQueueCompletion(orderId, state, currentStep, suppressQueue)
     }
+
+    this.cleanupCompletedOrders(seenOrderIds)
     this.previousSteps = nextSteps
   }
 
+  private updateCompletionCache(orderId: string, currentStep: OrderProgressBarStepName | undefined): void {
+    if (!isSuccess(currentStep)) {
+      this.completedOrders.delete(orderId)
+    }
+  }
+
+  private tryQueueCompletion(
+    orderId: string,
+    state: OrderProgressBarState,
+    currentStep: OrderProgressBarStepName | undefined,
+    suppressQueue: boolean,
+  ): void {
+    if (
+      suppressQueue ||
+      !isSuccess(currentStep) ||
+      this.completedOrders.has(orderId) ||
+      this.previousSteps[orderId] === currentStep ||
+      this.hasInProgress ||
+      !isRecentStateChange(state)
+    ) {
+      return
+    }
+
+    if (!this.completionQueue.includes(orderId)) this.completionQueue.push(orderId)
+    this.completedOrders.add(orderId)
+  }
+
+  private cleanupCompletedOrders(seenOrderIds: Set<string>): void {
+    for (const orderId of Array.from(this.completedOrders)) {
+      if (!seenOrderIds.has(orderId)) {
+        this.completedOrders.delete(orderId)
+      }
+    }
+  }
+
   private handleInProgress(): void {
     if (this.mode === 'completing' && this.currentCompletion) this.unshiftCurrentCompletion()
     if (!this.frameSet.solvingFrames.length) {

apps/cowswap-frontend/src/modules/orderProgressBar/hooks/useOrderProgressBarProps.ts

@@ -133,7 +133,7 @@ function useOrderBaseProgressBarProps(params: UseOrderProgressBarPropsParams): U
   // Do not build progress bar data when these conditions are set
   const disableProgressBar = widgetDisabled || isCreating || isFailed || isPresignaturePending || featureFlagDisabled
 
-  const orderId = order?.id || ''
+  const orderId = order?.id
 
   const getCancelOrder = useCancelOrder()
   const showCancellationModal = order && getCancelOrder ? getCancelOrder(order) : null
@@ -254,9 +254,9 @@ function getDoNotQueryStatusEndpoint(
 
 const DEFAULT_STATE = {}
 
-function useGetExecutingOrderState(orderId: string): OrderProgressBarState {
+function useGetExecutingOrderState(orderId?: string): OrderProgressBarState {
   const fullState = useAtomValue(ordersProgressBarStateAtom)
-  const singleState = fullState[orderId]
+  const singleState = orderId ? fullState[orderId] : undefined
 
   return useMemo(() => singleState || DEFAULT_STATE, [singleState])
 }
@@ -281,14 +281,18 @@ function useSetExecutingOrderProgressBarStepNameCallback(): (orderId: string, va
 // local updaters
 
 function useCountdownStartUpdater(
-  orderId: string,
+  orderId: string | undefined,
   countdown: OrderProgressBarState['countdown'],
   backendApiStatus: OrderProgressBarState['backendApiStatus'],
   shouldDisableCountdown: boolean,
 ): void {
   const setCountdown = useSetExecutingOrderCountdownCallback()
 
   useEffect(() => {
+    if (!orderId) {
+      return
+    }
+
     if (shouldDisableCountdown) {
       // Loose `!= null` on purpose: both null and undefined should reset the countdown, but 0 must stay; strict `!== null` would let undefined slip through
       if (countdown != null) {
@@ -308,17 +312,21 @@ function useCountdownStartUpdater(
   }, [backendApiStatus, setCountdown, countdown, orderId, shouldDisableCountdown])
 }
 
-function useCancellingOrderUpdater(orderId: string, isCancelling: boolean): void {
+function useCancellingOrderUpdater(orderId: string | undefined, isCancelling: boolean): void {
   const setCancellationTriggered = useSetAtom(setOrderProgressBarCancellationTriggered)
 
   useEffect(() => {
-    if (isCancelling) setCancellationTriggered(orderId)
+    if (!orderId || !isCancelling) {
+      return
+    }
+
+    setCancellationTriggered(orderId)
   }, [orderId, isCancelling, setCancellationTriggered])
 }
 
 // TODO: Break down this large function into smaller functions
 function useProgressBarStepNameUpdater(
-  orderId: string,
+  orderId: string | undefined,
   isUnfillable: boolean,
   isCancelled: boolean,
   isExpired: boolean,
@@ -352,8 +360,14 @@ function useProgressBarStepNameUpdater(
 
   // Update state with new step name
   useEffect(() => {
+    if (!orderId) {
+      return
+    }
+
+    const ensuredOrderId = orderId
+
     function updateStepName(name: OrderProgressBarStepName): void {
-      setProgressBarStepName(orderId, name || DEFAULT_STEP_NAME)
+      setProgressBarStepName(ensuredOrderId, name || DEFAULT_STEP_NAME)
     }
 
     let timer: NodeJS.Timeout | undefined
@@ -487,7 +501,11 @@ const BACKEND_TYPE_TO_PROGRESS_BAR_STEP_NAME: Record<CompetitionOrderStatus.type
   [CompetitionOrderStatus.type.CANCELLED]: OrderProgressBarStepName.INITIAL, // TODO: maybe add another state for finished with error?
 }
 
-function useBackendApiStatusUpdater(chainId: SupportedChainId, orderId: string, doNotQuery: boolean): void {
+function useBackendApiStatusUpdater(
+  chainId: SupportedChainId,
+  orderId: string | undefined,
+  doNotQuery: boolean,
+): void {
   const setAtom = useSetAtom(updateOrderProgressBarBackendInfo)
   const [stopQuerying, setStopQuerying] = useState(false)
   const { type: backendApiStatus, value } = usePendingOrderStatus(chainId, orderId, stopQuerying) || {}
@@ -525,7 +543,7 @@ const POOLING_SWR_OPTIONS = {
 
 function usePendingOrderStatus(
   chainId: SupportedChainId,
-  orderId: string,
+  orderId: string | undefined,
   doNotQuery?: boolean,
 ): CompetitionOrderStatus | undefined {
   return useSWR(

apps/cowswap-frontend/src/modules/orderProgressBar/state/atoms.test.ts

@@ -1,6 +1,7 @@
 import { createStore } from 'jotai'
 
 import {
+  cancellationTrackedOrderIdsAtom,
   ordersProgressBarStateAtom,
   pruneOrdersProgressBarState,
   updateOrderProgressBarCountdown,
@@ -49,6 +50,7 @@ describe('pruneOrdersProgressBarState', () => {
 
     expect(store.get(ordersProgressBarStateAtom)).toBe(initialState)
   })
+
 })
 
 describe('updateOrderProgressBarCountdown', () => {
@@ -124,3 +126,16 @@ describe('updateOrderProgressBarCountdown', () => {
     expect(store.get(ordersProgressBarStateAtom)).toEqual({})
   })
 })
+
+describe('cancellationTrackedOrderIdsAtom', () => {
+  it('returns ids with cancellationTriggered flag set', () => {
+    const store = createStore()
+    store.set(ordersProgressBarStateAtom, {
+      a: { cancellationTriggered: true },
+      b: {},
+      c: { cancellationTriggered: true },
+    })
+
+    expect(store.get(cancellationTrackedOrderIdsAtom)).toEqual(['a', 'c'])
+  })
+})

apps/cowswap-frontend/src/modules/orderProgressBar/state/atoms.ts

@@ -32,7 +32,7 @@ export const pruneOrdersProgressBarState = atom(null, (get, set, trackedOrderIds
     return acc
   }, {})
 
-  if (!changed && trackedOrderIds.every((orderId) => orderId in fullState)) {
+  if (!changed) {
     return
   }
 
@@ -213,3 +213,11 @@ export const setOrderProgressBarCancellationTriggered = atom(null, (get, set, or
 
   set(ordersProgressBarStateAtom, { ...fullState, [orderId]: singleState })
 })
+
+export const cancellationTrackedOrderIdsAtom = atom((get) => {
+  const fullState = get(ordersProgressBarStateAtom)
+
+  return Object.entries(fullState)
+    .filter(([, state]) => state?.cancellationTriggered)
+    .map(([orderId]) => orderId)
+})