fix: keep displayed orders in progress state and dedupe favicon completions #6510
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a derived atom and public hook to expose surplus-modal order IDs, refactors the favicon animation controller to track completed/in-progress orders and queue completion animations safely, makes orderId optional across several order-progress hooks and updaters, and updates order-progress pruning to preserve orders from surplus queue, trade confirmation, and cancellation tracking with tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Updater as OrderProgressStateUpdater
participant Surplus as useSurplusQueueOrderIds()
participant Trade as useTradeConfirmState()
participant Cancellations as cancellationTrackedOrderIdsAtom
participant Pruner as pruneOrdersProgressBarState
Updater->>Surplus: read surplus queue IDs
Updater->>Trade: read active transaction hash
Updater->>Cancellations: read cancellation-tracked IDs
Updater->>Updater: combine tracked IDs (market ∪ surplus ∪ trade ∪ cancellations)
Updater->>Pruner: pass combined IDs to prune/preserve state
Note over Pruner,Updater: deduplicate and preserve tracked orders
sequenceDiagram
participant State as OrderProgressBarState
participant Controller as FaviconAnimationController
participant Animator as FaviconAnimator
State->>Controller: update(state)
Controller->>Controller: compute hasInProgress, seenOrderIds
Controller->>Controller: updateCompletionCache(orderId, step)
alt success and conditions met (no in-progress, not completed, recent change)
Controller->>Animator: enqueue completion animation (tryQueueCompletion)
Controller->>Controller: mark order completed
else suppress or ignore enqueue
end
Controller->>Controller: cleanupCompletedOrders(seenOrderIds)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hey @fairlighteth , thank you! It works good in terms of solvers, but is not good in terms of favicon: it keeps shimmering with green when an order is filled (3-4 times instead of 1 time). Could you please take a look at it? |
|
Pushed a fix. Previously, every finished order sat in the queue, so you’d get multiple green flashes once the backlog cleared. Now, if order 1 finishes while order 2 is still solving, the favicon stays in the “progress” state for order 2, and when order 2 completes you get a single green flash for that order only. In other words, completion animations are skipped for any order that finishes while another is still in progress. |
fairlighteth
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.ts (1)
17-18: Consider documenting the completion deduplication behavior.The new
completedOrdersSet andhasInProgressflag implement sophisticated logic to prevent duplicate favicon animations, as described in the PR comments. This addresses the issue where multiple green flashes occurred when clearing a backlog of finished orders.Consider adding JSDoc comments explaining:
completedOrders: tracks which orders have shown a completion animation in their current success statehasInProgress: suppresses completion animations while any order is still solving+ /** + * Tracks orders that have already shown a completion animation in their current success state. + * Cleared when an order transitions out of success state, allowing re-animation if it succeeds again. + */ private completedOrders = new Set<string>() + /** + * True if any order is currently in a solving/in-progress state. + * Suppresses completion animations until all orders finish to avoid multiple green flashes. + */ private hasInProgress = falseAlso applies to: 29-29
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/cowswap-frontend/src/entities/surplusModal/index.ts(2 hunks)apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.test.ts(1 hunks)apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.ts(3 hunks)apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.test.tsx(5 hunks)apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-10-10T20:28:16.565Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6347
File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49
Timestamp: 2025-10-10T20:28:16.565Z
Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior.
<!-- [add_learning]
When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import th...
Applied to files:
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsxapps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.test.tsxapps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.ts
📚 Learning: 2025-08-05T14:27:05.023Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 5992
File: libs/wallet/src/web3-react/utils/switchChain.ts:36-38
Timestamp: 2025-08-05T14:27:05.023Z
Learning: In libs/wallet/src/web3-react/utils/switchChain.ts, the team prefers using Record<SupportedChainId, string | null> over Partial<Record<SupportedChainId, string>> for WALLET_RPC_SUGGESTION to enforce that all supported chain IDs have explicit values set, even if some might be null. This ensures compile-time completeness checking.
Applied to files:
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsx
📚 Learning: 2025-06-23T07:03:50.760Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5859
File: apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuotePolling.ts:76-82
Timestamp: 2025-06-23T07:03:50.760Z
Learning: In the useTradeQuotePolling hook, there are two useLayoutEffect hooks that work together: one resets the counter to 0 when the confirmation modal closes, and another automatically triggers pollQuote(false, true) whenever the counter reaches 0. This creates an intentional chain reaction for immediate quote updates.
Applied to files:
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsxapps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.test.tsx
📚 Learning: 2025-09-25T08:49:32.256Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Applied to files:
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.test.tsx
📚 Learning: 2025-09-25T08:48:53.495Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:58-60
Timestamp: 2025-09-25T08:48:53.495Z
Learning: In the cowswap-frontend codebase, when writing SWR tests, the team prefers maximum test isolation by using `provider: () => new Map()` in SWRConfig wrappers, even if it recreates cache on every render, to ensure tests don't share any state.
Applied to files:
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.test.tsx
🧬 Code graph analysis (4)
apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.test.ts (2)
apps/cowswap-frontend/src/modules/orderProgressBar/types.ts (2)
OrderProgressBarState(15-24)OrderProgressBarStepName(13-13)apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.ts (1)
FaviconAnimationController(9-218)
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsx (2)
apps/cowswap-frontend/src/modules/trade/hooks/useTradeConfirmState.ts (1)
useTradeConfirmState(7-9)apps/cowswap-frontend/src/entities/surplusModal/index.ts (1)
useSurplusQueueOrderIds(58-60)
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.test.tsx (3)
apps/cowswap-frontend/src/entities/surplusModal/index.ts (1)
useSurplusQueueOrderIds(58-60)apps/cowswap-frontend/src/modules/trade/hooks/useTradeConfirmState.ts (1)
useTradeConfirmState(7-9)apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsx (1)
OrderProgressStateUpdater(22-61)
apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.ts (2)
apps/cowswap-frontend/src/modules/orderProgressBar/types.ts (2)
OrderProgressBarStepName(13-13)OrderProgressBarState(15-24)apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/logic.ts (2)
isSuccess(50-52)isRecentStateChange(40-48)
🪛 GitHub Actions: CI
apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.test.ts
[error] 92-92: Test failure: FaviconAnimationController queues another completion when an order returns to solving and finishes again - Expected: 2, Received: 1.
🔇 Additional comments (5)
apps/cowswap-frontend/src/entities/surplusModal/index.ts (1)
14-15: LGTM! Clean Jotai pattern for derived state.The derived atom and hook follow standard Jotai patterns, providing a clean public API for accessing surplus queue order IDs. This enables other modules to reactively track which orders are queued for the surplus modal.
Also applies to: 58-60
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsx (1)
34-48: LGTM! Effective deduplication using Set.The refactored effect correctly merges order IDs from three sources (pending market orders, surplus queue, and trade confirmation modal) using a Set for automatic deduplication. The logic properly handles the conditional inclusion of marketOrders (only when account/chainId present) while always including surplus queue IDs and transactionHash when available. The dependency array is complete and accurate.
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.test.tsx (1)
115-156: LGTM! Comprehensive test coverage for new tracking sources.The three new test cases effectively validate the integration of surplus queue IDs and trade confirmation state:
- Orders queued for surplus modal are tracked even without wallet info
- The currently displayed order in the confirmation modal is tracked
- All three sources (pending, queued, displayed) are merged and deduplicated correctly
The test setup properly initializes mocks in beforeEach, following the team's preference for test isolation.
apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.test.ts (1)
85-93: Test setup issue: Mock animator callbacks never fire, preventing mode transitions.The completion cache logic itself is correct. However, the test fails because the
mockPlaymock never invokes theonCompletecallback. This leaves the controller stuck in'completing'mode after the first animation, causingprocessAnimationMode()to return early (line 36) for subsequent updates. This blocks both the queue processing and prevents the second completion from being queued or played.The test needs to mock the animator such that
onCompletecallbacks execute synchronously, or use jest timers to simulate the animation sequence. The review comment's analysis of the cache logic is incorrect—the real issue is test infrastructure, not the controller's state management.Likely an incorrect or invalid review comment.
apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.ts (1)
101-126: Fix suppressQueue logic to not suppress success-state completions on first update.The root cause is different from what the review comment hypothesizes. The issue is that
suppressQueue=trueon the firstupdate()call (when!isInitialized) prevents the initial FINISHED state from being queued in Update 1, not any blocking guard in Update 3.Trace:
- Update 1 (FINISHED, suppressQueue=true): Line 114 guard triggers, returns early → no queue
- Update 2 (SOLVING): Deletes from
completedOrders→ no queue- Update 3 (FINISHED, suppressQueue=false): All guards pass → queues successfully
Result: 1 completion play (only Update 3), but test expects 2.
The
suppressQueuelogic suppresses ALL completions on app initialization to avoid spurious animations during state hydration. However, this also suppresses legitimate success-state completions. Either the test expectations are incorrect (should expect 1), or thesuppressQueuelogic should exclude success states that represent actual state transitions (not just initial load artifact).
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsx
Show resolved
Hide resolved
…gressStateUpdater
#6511) * refactor: streamline order progress state management and enhance tests * feat: add cancellation tracking for order progress and improve related hooks and tests * fix: update order ID handling to allow optional values in progress bar hooks * fix: ensure consistent order ID usage in progress bar step name updates
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/modules/orderProgressBar/hooks/useOrderProgressBarProps.ts (1)
363-371: Consider simplifying the orderId capture pattern.After the early return on lines 363-365, TypeScript already knows that
orderIdis defined (not undefined). TheensuredOrderIdvariable on line 367 doesn't provide additional type safety—it's just a copy. Consider usingorderIddirectly in theupdateStepNamefunction to reduce indirection.Apply this diff if you prefer a more direct approach:
useEffect(() => { if (!orderId) { return } - const ensuredOrderId = orderId - function updateStepName(name: OrderProgressBarStepName): void { - setProgressBarStepName(ensuredOrderId, name || DEFAULT_STEP_NAME) + setProgressBarStepName(orderId, name || DEFAULT_STEP_NAME) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/cowswap-frontend/src/modules/orderProgressBar/hooks/useOrderProgressBarProps.ts(7 hunks)apps/cowswap-frontend/src/modules/orderProgressBar/state/atoms.test.ts(3 hunks)apps/cowswap-frontend/src/modules/orderProgressBar/state/atoms.ts(2 hunks)apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.test.tsx(5 hunks)apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-10-10T20:28:16.565Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6347
File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49
Timestamp: 2025-10-10T20:28:16.565Z
Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior.
<!-- [add_learning]
When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import th...
Applied to files:
apps/cowswap-frontend/src/modules/orderProgressBar/state/atoms.test.tsapps/cowswap-frontend/src/modules/orderProgressBar/state/atoms.tsapps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.test.tsxapps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsx
📚 Learning: 2025-08-08T13:56:18.009Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6125
File: libs/tokens/src/updaters/TokensListsUpdater/index.tsx:29-31
Timestamp: 2025-08-08T13:56:18.009Z
Learning: In libs/tokens/src/updaters/TokensListsUpdater/index.tsx, the project’s current Jotai version requires using `unstable_getOnInit` (not `getOnInit`) in atomWithStorage options; keep `{ unstable_getOnInit: true }` until Jotai is upgraded.
Applied to files:
apps/cowswap-frontend/src/modules/orderProgressBar/state/atoms.test.tsapps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsx
📚 Learning: 2025-06-23T07:03:50.760Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5859
File: apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuotePolling.ts:76-82
Timestamp: 2025-06-23T07:03:50.760Z
Learning: In the useTradeQuotePolling hook, there are two useLayoutEffect hooks that work together: one resets the counter to 0 when the confirmation modal closes, and another automatically triggers pollQuote(false, true) whenever the counter reaches 0. This creates an intentional chain reaction for immediate quote updates.
Applied to files:
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.test.tsxapps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsx
📚 Learning: 2025-09-25T08:48:53.495Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:58-60
Timestamp: 2025-09-25T08:48:53.495Z
Learning: In the cowswap-frontend codebase, when writing SWR tests, the team prefers maximum test isolation by using `provider: () => new Map()` in SWRConfig wrappers, even if it recreates cache on every render, to ensure tests don't share any state.
Applied to files:
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.test.tsx
📚 Learning: 2025-09-25T08:49:32.256Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Applied to files:
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.test.tsx
📚 Learning: 2025-08-08T13:55:17.528Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6125
File: libs/tokens/src/state/tokens/allTokensAtom.ts:78-78
Timestamp: 2025-08-08T13:55:17.528Z
Learning: In libs/tokens/src/state/tokens/allTokensAtom.ts (TypeScript/Jotai), the team prefers to wait for token lists to initialize (listsStatesListAtom non-empty) before returning tokens. No fallback to favorites/user-added/native tokens should be used when listsStatesList is empty.
Applied to files:
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsx
📚 Learning: 2025-08-05T14:27:05.023Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 5992
File: libs/wallet/src/web3-react/utils/switchChain.ts:36-38
Timestamp: 2025-08-05T14:27:05.023Z
Learning: In libs/wallet/src/web3-react/utils/switchChain.ts, the team prefers using Record<SupportedChainId, string | null> over Partial<Record<SupportedChainId, string>> for WALLET_RPC_SUGGESTION to enforce that all supported chain IDs have explicit values set, even if some might be null. This ensures compile-time completeness checking.
Applied to files:
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsx
📚 Learning: 2025-10-13T19:41:31.440Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6351
File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121
Timestamp: 2025-10-13T19:41:31.440Z
Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
Applied to files:
apps/cowswap-frontend/src/modules/orderProgressBar/hooks/useOrderProgressBarProps.ts
🧬 Code graph analysis (4)
apps/cowswap-frontend/src/modules/orderProgressBar/state/atoms.test.ts (1)
apps/cowswap-frontend/src/modules/orderProgressBar/state/atoms.ts (2)
ordersProgressBarStateAtom(15-15)cancellationTrackedOrderIdsAtom(217-223)
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.test.tsx (6)
apps/cowswap-frontend/src/modules/orderProgressBar/state/atoms.ts (1)
cancellationTrackedOrderIdsAtom(217-223)libs/wallet/src/api/hooks.ts (1)
useWalletInfo(24-26)apps/cowswap-frontend/src/legacy/state/orders/hooks.ts (1)
useOnlyPendingOrders(262-278)apps/cowswap-frontend/src/entities/surplusModal/index.ts (1)
useSurplusQueueOrderIds(58-60)apps/cowswap-frontend/src/modules/trade/hooks/useTradeConfirmState.ts (1)
useTradeConfirmState(7-9)apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsx (1)
OrderProgressStateUpdater(22-74)
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsx (3)
apps/cowswap-frontend/src/modules/trade/hooks/useTradeConfirmState.ts (1)
useTradeConfirmState(7-9)apps/cowswap-frontend/src/entities/surplusModal/index.ts (1)
useSurplusQueueOrderIds(58-60)apps/cowswap-frontend/src/modules/orderProgressBar/state/atoms.ts (1)
cancellationTrackedOrderIdsAtom(217-223)
apps/cowswap-frontend/src/modules/orderProgressBar/hooks/useOrderProgressBarProps.ts (3)
apps/cowswap-frontend/src/modules/orderProgressBar/types.ts (2)
OrderProgressBarState(15-24)OrderProgressBarStepName(13-13)apps/cowswap-frontend/src/modules/orderProgressBar/state/atoms.ts (2)
ordersProgressBarStateAtom(15-15)setOrderProgressBarCancellationTriggered(202-215)apps/cowswap-frontend/src/modules/orderProgressBar/constants.ts (1)
DEFAULT_STEP_NAME(35-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cypress
- GitHub Check: Setup
🔇 Additional comments (12)
apps/cowswap-frontend/src/modules/orderProgressBar/state/atoms.test.ts (1)
130-141: LGTM! Comprehensive test coverage for cancellation tracking.The test correctly validates that
cancellationTrackedOrderIdsAtomfilters orders with thecancellationTriggeredflag set and returns their IDs.apps/cowswap-frontend/src/modules/orderProgressBar/state/atoms.ts (2)
35-37: LGTM! Simplified pruning logic.The refactored early-return condition correctly skips the state update only when no changes were made, removing the unnecessary check for tracked IDs existence.
217-223: LGTM! Clean derived atom for cancellation tracking.The atom correctly derives the list of order IDs with
cancellationTriggeredset, enabling other components to reference cancellation-tracked orders for state retention.apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.test.tsx (1)
126-180: LGTM! Excellent test coverage for multi-source tracking.The four new test cases comprehensively validate:
- Surplus queue IDs are retained even without wallet context
- Transaction hash from confirmation modal is tracked
- Deduplication works correctly across pending, queued, and displayed orders
- Cancellation-triggered orders persist even after leaving pending state
This test coverage should prevent regressions and aligns perfectly with the PR objectives.
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressStateUpdater.tsx (2)
36-52: LGTM! Clean Set-based deduplication of tracked IDs.The refactored logic elegantly builds a union of order IDs from four sources (pending market orders, surplus queue, confirmation modal transaction hash, and cancellation-tracked orders) using a Set for automatic deduplication. The comment on lines 38-39 helpfully explains why pruning continues even without wallet context, directly addressing the previous review question.
53-61: LGTM! Comprehensive dependency array.All tracked state sources are correctly included in the effect dependencies, ensuring pruning runs whenever any source changes.
apps/cowswap-frontend/src/modules/orderProgressBar/hooks/useOrderProgressBarProps.ts (6)
136-136: LGTM! Safe optional chaining for orderId.The change to
order?.idcorrectly handles cases where order may be undefined, aligning with the broader refactoring to support optional orderIds throughout the progress bar subsystem.
257-261: LGTM! Safe optional orderId handling.The function correctly handles undefined
orderIdby conditionally accessing state and returning a default empty state.
284-313: LGTM! Proper guard for countdown updates.The early return on line 292-294 correctly prevents countdown updates when orderId is undefined, ensuring safe access to the state atom.
315-325: LGTM! Clean cancellation guard.The combined guard for both
orderIdandisCancellingcorrectly prevents unnecessary atom updates.
504-538: LGTM! Safe backend status updates with optional orderId.The function correctly handles optional
orderIdby checking its presence before calling the atom setter on line 529.
544-554: LGTM! Proper conditional SWR key for optional orderId.The SWR hook correctly returns
nullas the key whenorderIdis undefined ordoNotQueryis true, preventing unnecessary API calls.
Summary
Fixes #6508
OrderProgressStateUpdater’s tracked IDs.useSurplusQueueOrderIdsso other modules can reference the queued modal orders.To Test
Place two market orders back-to-back in the Trade widget.
Queue an order in the Surplus modal (e.g., via a fulfilled limit order) while leaving another order pending.
Background
OrderProgressStateUpdater pruned solver data as soon as an order left the pending list, which happens before the Surplus modal finishes rendering. The new tracked-ID union ensures state lives until all relevant UIs release it.
Summary by CodeRabbit
New Features
Bug Fixes
Tests