-
Notifications
You must be signed in to change notification settings - Fork 159
refactor: decompose UnfillableOrdersUpdater #5909
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… fix/order-price-to-fill
…x/order-price-to-fill
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
WalkthroughThis change removes legacy quote fetching logic and the UnfillableOrdersUpdater component from the frontend, replacing them with new hooks and utilities for managing order fillability and navigation. Navigation hooks for the orders table are consolidated and generalized, and account-specific filtering is added for pending orders. A utility for address comparison is introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UnfillableOrdersUpdater
participant useUpdatePendingOrders
participant tradingSdk
participant State
Note over User,State: Periodic check for pending orders fillability
UnfillableOrdersUpdater->>useUpdatePendingOrders: Trigger update with pending orders
useUpdatePendingOrders->>tradingSdk: Fetch quote for each order
tradingSdk-->>useUpdatePendingOrders: Return quote or error
useUpdatePendingOrders->>State: Update price/unfillable status in state
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
yarn install v1.22.22 ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
… fix/order-price-to-fill
…col/cowswap into fix/order-price-to-fill-1
...tend/src/common/updaters/orders/UnfillableOrdersUpdater/hooks/usePriceOutOfRangeAnalytics.ts
Outdated
Show resolved
Hide resolved
apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater/index.ts
Outdated
Show resolved
Hide resolved
elena-zh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, works fine!
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater/hooks/useUpdatePendingOrders.ts (1)
17-38: Simplify error handling pattern.The function has redundant error handling with both try/catch and .catch() on the promise chain. Consider simplifying to use only one approach.
async function getOrderPrice(chainId: SupportedChainId, order: Order): Promise<QuoteResults | null> { const amount = getRemainderAmount(order.kind, order) - try { - return tradingSdk + return tradingSdk .getQuote({ chainId, kind: order.kind, owner: order.owner as AccountAddress, sellToken: order.sellToken, sellTokenDecimals: order.inputToken.decimals, buyToken: order.buyToken, buyTokenDecimals: order.outputToken.decimals, amount, receiver: order.receiver, partiallyFillable: order.partiallyFillable, }) .then((res) => res.quoteResults) - } catch { - return null - } + .catch(() => null)apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater/hooks/useUpdateIsUnfillableFlag.ts (1)
40-52: Minor optimization: redundant condition check.The condition
order.isUnfillable !== isUnfillable && isSwapincludes a redundantisSwapcheck sinceisUnfillableis already computed asisSwap && isOrderUnfillable(...). WhenisSwapis false,isUnfillablewill always be false.- // Only trigger state update if flag changed - if (order.isUnfillable !== isUnfillable && isSwap) { + // Only trigger state update if flag changed for swap orders + if (order.isUnfillable !== isUnfillable && isUnfillable !== false) {Or more clearly:
- if (order.isUnfillable !== isUnfillable && isSwap) { + if (isSwap && order.isUnfillable !== isUnfillable) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
apps/cowswap-frontend/src/api/cowProtocol/api.ts(0 hunks)apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater.ts(0 hunks)apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater/hooks/usePriceOutOfRangeAnalytics.ts(1 hunks)apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater/hooks/useUpdateIsUnfillableFlag.ts(1 hunks)apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater/hooks/useUpdatePendingOrders.ts(1 hunks)apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater/index.ts(1 hunks)apps/cowswap-frontend/src/legacy/state/orders/hooks.ts(2 hunks)apps/cowswap-frontend/src/modules/limitOrders/hooks/useHandleOrderPlacement.test.ts(2 hunks)apps/cowswap-frontend/src/modules/limitOrders/hooks/useHandleOrderPlacement.ts(5 hunks)apps/cowswap-frontend/src/modules/limitOrders/updaters/TriggerAppziUpdater.ts(1 hunks)apps/cowswap-frontend/src/modules/ordersTable/hooks/useNavigateToAllOrdersTable.ts(0 hunks)apps/cowswap-frontend/src/modules/ordersTable/hooks/useNavigateToOrdersTableTab.ts(1 hunks)apps/cowswap-frontend/src/modules/ordersTable/index.tsx(1 hunks)apps/cowswap-frontend/src/modules/twap/hooks/useCreateTwapOrder.tsx(5 hunks)apps/cowswap-frontend/src/types/index.ts(0 hunks)libs/common-utils/src/areAddressesEqual.ts(1 hunks)libs/common-utils/src/index.ts(1 hunks)
💤 Files with no reviewable changes (4)
- apps/cowswap-frontend/src/types/index.ts
- apps/cowswap-frontend/src/api/cowProtocol/api.ts
- apps/cowswap-frontend/src/modules/ordersTable/hooks/useNavigateToAllOrdersTable.ts
- apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater.ts
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: shoom3301
PR: cowprotocol/cowswap#5443
File: apps/cowswap-frontend/src/modules/swap/containers/ConfirmSwapModalSetup/index.tsx:71-71
Timestamp: 2025-02-20T15:59:33.749Z
Learning: The swap module in apps/cowswap-frontend/src/modules/swap/ is marked for deletion in PR #5444 as part of the swap widget unification effort.
Learnt from: shoom3301
PR: cowprotocol/cowswap#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.
apps/cowswap-frontend/src/modules/limitOrders/hooks/useHandleOrderPlacement.test.ts (1)
Learnt from: shoom3301
PR: cowprotocol/cowswap#5549
File: apps/cowswap-frontend/src/modules/tradeFlow/services/safeBundleFlow/safeBundleEthFlow.ts:152-152
Timestamp: 2025-04-02T09:58:29.374Z
Learning: In the `safeBundleEthFlow` function, `account` is guaranteed to be truthy based on the type system (`PostOrderParams` defines it as a required string) and the context in which the function is called, so additional runtime checks are unnecessary.
apps/cowswap-frontend/src/modules/twap/hooks/useCreateTwapOrder.tsx (4)
Learnt from: shoom3301
PR: cowprotocol/cowswap#5443
File: apps/cowswap-frontend/src/modules/swap/containers/ConfirmSwapModalSetup/index.tsx:71-71
Timestamp: 2025-02-20T15:59:33.749Z
Learning: The swap module in apps/cowswap-frontend/src/modules/swap/ is marked for deletion in PR #5444 as part of the swap widget unification effort.
Learnt from: alfetopito
PR: cowprotocol/cowswap#5831
File: apps/cowswap-frontend/src/modules/application/containers/AppMenu/index.tsx:7-9
Timestamp: 2025-06-16T16:01:46.729Z
Learning: React Router v7 restructured packages - NavLink and other core routing components should be imported from 'react-router' (not 'react-router-dom'). In v7, 'react-router-dom' mainly re-exports for backward compatibility, while 'react-router' is the new preferred import path for most components.
Learnt from: shoom3301
PR: cowprotocol/cowswap#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.
Learnt from: shoom3301
PR: cowprotocol/cowswap#5549
File: apps/cowswap-frontend/src/modules/tradeFlow/services/safeBundleFlow/safeBundleEthFlow.ts:152-152
Timestamp: 2025-04-02T09:58:29.374Z
Learning: In the `safeBundleEthFlow` function, `account` is guaranteed to be truthy based on the type system (`PostOrderParams` defines it as a required string) and the context in which the function is called, so additional runtime checks are unnecessary.
apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater/hooks/usePriceOutOfRangeAnalytics.ts (1)
Learnt from: shoom3301
PR: cowprotocol/cowswap#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.
apps/cowswap-frontend/src/modules/ordersTable/hooks/useNavigateToOrdersTableTab.ts (1)
Learnt from: fairlighteth
PR: cowprotocol/cowswap#5768
File: apps/cow-fi/components/LearnPageComponent.tsx:184-185
Timestamp: 2025-05-28T16:50:12.273Z
Learning: In apps/cow-fi/components/LearnPageComponent.tsx, the user prefers to keep the inconsistent link behavior where featured articles always open in new tabs (target="_blank") while media coverage links conditionally open in new tabs based on the linkExternal flag. This inconsistency should not be flagged as an issue in future reviews.
apps/cowswap-frontend/src/modules/limitOrders/updaters/TriggerAppziUpdater.ts (2)
Learnt from: shoom3301
PR: cowprotocol/cowswap#5549
File: apps/cowswap-frontend/src/modules/tradeFlow/services/safeBundleFlow/safeBundleEthFlow.ts:152-152
Timestamp: 2025-04-02T09:58:29.374Z
Learning: In the `safeBundleEthFlow` function, `account` is guaranteed to be truthy based on the type system (`PostOrderParams` defines it as a required string) and the context in which the function is called, so additional runtime checks are unnecessary.
Learnt from: shoom3301
PR: cowprotocol/cowswap#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.
apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater/index.ts (2)
Learnt from: shoom3301
PR: cowprotocol/cowswap#5443
File: apps/cowswap-frontend/src/modules/swap/containers/ConfirmSwapModalSetup/index.tsx:71-71
Timestamp: 2025-02-20T15:59:33.749Z
Learning: The swap module in apps/cowswap-frontend/src/modules/swap/ is marked for deletion in PR #5444 as part of the swap widget unification effort.
Learnt from: shoom3301
PR: cowprotocol/cowswap#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.
apps/cowswap-frontend/src/modules/ordersTable/index.tsx (3)
Learnt from: shoom3301
PR: cowprotocol/cowswap#5443
File: apps/cowswap-frontend/src/modules/swap/containers/ConfirmSwapModalSetup/index.tsx:71-71
Timestamp: 2025-02-20T15:59:33.749Z
Learning: The swap module in apps/cowswap-frontend/src/modules/swap/ is marked for deletion in PR #5444 as part of the swap widget unification effort.
Learnt from: alfetopito
PR: cowprotocol/cowswap#5831
File: apps/cowswap-frontend/src/modules/application/containers/AppMenu/index.tsx:7-9
Timestamp: 2025-06-16T16:01:46.729Z
Learning: React Router v7 restructured packages - NavLink and other core routing components should be imported from 'react-router' (not 'react-router-dom'). In v7, 'react-router-dom' mainly re-exports for backward compatibility, while 'react-router' is the new preferred import path for most components.
Learnt from: fairlighteth
PR: cowprotocol/cowswap#5768
File: apps/cow-fi/components/LearnPageComponent.tsx:184-185
Timestamp: 2025-05-28T16:50:12.273Z
Learning: In apps/cow-fi/components/LearnPageComponent.tsx, the user prefers to keep the inconsistent link behavior where featured articles always open in new tabs (target="_blank") while media coverage links conditionally open in new tabs based on the linkExternal flag. This inconsistency should not be flagged as an issue in future reviews.
apps/cowswap-frontend/src/legacy/state/orders/hooks.ts (3)
Learnt from: shoom3301
PR: cowprotocol/cowswap#5549
File: apps/cowswap-frontend/src/modules/tradeFlow/services/safeBundleFlow/safeBundleEthFlow.ts:152-152
Timestamp: 2025-04-02T09:58:29.374Z
Learning: In the `safeBundleEthFlow` function, `account` is guaranteed to be truthy based on the type system (`PostOrderParams` defines it as a required string) and the context in which the function is called, so additional runtime checks are unnecessary.
Learnt from: alfetopito
PR: cowprotocol/cowswap#5532
File: libs/common-hooks/src/useOnScroll.tsx:22-38
Timestamp: 2025-03-25T10:03:35.157Z
Learning: In React useEffect hooks, refs (RefObjects) should generally be included in dependency arrays even though changes to ref.current don't trigger effects. This is because the ref object itself could change between renders (especially in custom hooks where refs are passed as parameters), and it follows React's best practice of including all external values used in the effect.
Learnt from: fairlighteth
PR: cowprotocol/cowswap#5782
File: apps/cow-fi/app/(learn)/learn/topics/page.tsx:1-1
Timestamp: 2025-06-05T08:31:01.284Z
Learning: Next.js App Router official documentation states that Client Components marked with 'use client' should NOT be declared as async functions, as this can lead to infinite loops or application hangs. The recommended pattern for async operations in Client Components is to use useEffect hooks. Server Components (without 'use client') can be async, which might be a source of confusion.
apps/cowswap-frontend/src/modules/limitOrders/hooks/useHandleOrderPlacement.ts (2)
Learnt from: shoom3301
PR: cowprotocol/cowswap#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.
Learnt from: shoom3301
PR: cowprotocol/cowswap#5443
File: apps/cowswap-frontend/src/modules/swap/containers/ConfirmSwapModalSetup/index.tsx:71-71
Timestamp: 2025-02-20T15:59:33.749Z
Learning: The swap module in apps/cowswap-frontend/src/modules/swap/ is marked for deletion in PR #5444 as part of the swap widget unification effort.
apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater/hooks/useUpdatePendingOrders.ts (1)
Learnt from: shoom3301
PR: cowprotocol/cowswap#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.
apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater/hooks/useUpdateIsUnfillableFlag.ts (1)
Learnt from: shoom3301
PR: cowprotocol/cowswap#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.
🧬 Code Graph Analysis (5)
apps/cowswap-frontend/src/modules/limitOrders/hooks/useHandleOrderPlacement.test.ts (1)
apps/cowswap-frontend/src/modules/ordersTable/hooks/useNavigateToOrdersTableTab.ts (1)
useNavigateToOrdersTableTab(9-19)
apps/cowswap-frontend/src/modules/twap/hooks/useCreateTwapOrder.tsx (1)
apps/cowswap-frontend/src/modules/ordersTable/hooks/useNavigateToOrdersTableTab.ts (1)
useNavigateToOrdersTableTab(9-19)
apps/cowswap-frontend/src/modules/ordersTable/hooks/useNavigateToOrdersTableTab.ts (3)
apps/cowswap-frontend/src/common/hooks/useNavigate.ts (1)
useNavigate(10-23)apps/cowswap-frontend/src/modules/ordersTable/utils/buildOrdersTableUrl.ts (1)
buildOrdersTableUrl(4-19)apps/cowswap-frontend/src/modules/ordersTable/hooks/useGetBuildOrdersTableUrl.ts (1)
useGetBuildOrdersTableUrl(9-16)
apps/cowswap-frontend/src/modules/limitOrders/updaters/TriggerAppziUpdater.ts (1)
apps/cowswap-frontend/src/legacy/state/orders/hooks.ts (1)
useOnlyPendingOrders(262-278)
apps/cowswap-frontend/src/modules/limitOrders/hooks/useHandleOrderPlacement.ts (1)
apps/cowswap-frontend/src/modules/ordersTable/hooks/useNavigateToOrdersTableTab.ts (1)
useNavigateToOrdersTableTab(9-19)
🔇 Additional comments (16)
apps/cowswap-frontend/src/modules/limitOrders/hooks/useHandleOrderPlacement.test.ts (1)
11-11: LGTM! Test properly updated for navigation hook refactoring.The test file correctly adapts to the consolidated navigation hook with proper import, mock variable naming, and implementation setup.
Also applies to: 48-50, 92-92
apps/cowswap-frontend/src/modules/ordersTable/index.tsx (1)
4-4: LGTM! Clean consolidation of navigation hook exports.The export update properly consolidates the navigation functionality into a single, more flexible hook.
apps/cowswap-frontend/src/modules/twap/hooks/useCreateTwapOrder.tsx (1)
15-15: LGTM! Consistent navigation hook updates for TWAP orders.The changes properly adapt to the consolidated navigation hook with explicit tab specification (
OrderTabId.all), improving code clarity and maintainability.Also applies to: 60-60, 191-191, 219-219
apps/cowswap-frontend/src/modules/ordersTable/hooks/useNavigateToOrdersTableTab.ts (1)
9-19: LGTM! Well-implemented navigation hook consolidation.The refactored hook successfully consolidates navigation logic with:
- Clear type-safe signature accepting
OrderTabId- Consistent page reset to 1 on navigation
- Proper dependency management in useCallback
This improves maintainability while providing flexible tab navigation.
apps/cowswap-frontend/src/modules/limitOrders/hooks/useHandleOrderPlacement.ts (2)
29-29: Good improvement! Added explicit return type annotation.The explicit return type
(wasPlaced: boolean) => voidimproves type safety and code clarity.
16-16: LGTM! Consistent navigation hook updates for limit orders.The changes properly adapt to the consolidated navigation hook with explicit tab specification (
OrderTabId.all), maintaining consistency with the refactoring across the codebase.Also applies to: 57-57, 140-140, 164-164
libs/common-utils/src/areAddressesEqual.ts (1)
3-7: Well-implemented address comparison utility.The function correctly handles all edge cases:
- Returns
falseif exactly one address is nullish- Returns
trueif both addresses are nullish (viaundefined?.toLowerCase() === undefined?.toLowerCase())- Performs case-insensitive comparison for truthy addresses
The logic is sound and the use of optional chaining is appropriate.
libs/common-utils/src/index.ts (1)
66-66: LGTM!The export statement correctly exposes the new
areAddressesEqualutility following the established pattern.apps/cowswap-frontend/src/modules/limitOrders/updaters/TriggerAppziUpdater.ts (1)
20-20: Correctly updated to use the new hook signature.The addition of the
accountparameter aligns with the updateduseOnlyPendingOrdershook signature and ensures that only the current user's pending orders are considered for triggering the Appzi survey.apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater/hooks/usePriceOutOfRangeAnalytics.ts (1)
7-20: Well-structured analytics hook with proper memoization.The implementation follows React best practices:
- Proper use of
useCallbackwith correct dependency array- Clear separation of concerns for analytics tracking
- Good TypeScript typing with explicit return type
- Clean API design with dynamic label parameter
This is a good example of extracting focused functionality from a larger component.
apps/cowswap-frontend/src/legacy/state/orders/hooks.ts (2)
4-4: LGTM!Properly imported the new address comparison utility that will be used for filtering orders by account.
262-278: Excellent refactoring with improved efficiency and account filtering.The updated hook implementation provides several improvements:
- Account-specific filtering: Now filters orders by the connected account using the new
areAddressesEqualutility- Performance optimization: Only deserializes orders that match the account, avoiding unnecessary work
- Proper edge case handling: Early return when state or account is missing
- Correct memoization: Updated dependency array includes both
stateandaccountThe logic flow is correct and the use of
reducewith selective processing is more efficient than the previous approach.apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater/index.ts (1)
1-48: Well-structured refactor with clear separation of concerns.The component effectively uses SWR for periodic polling and properly handles the background updater pattern. The use of refs to capture current values in the SWR fetcher is correct and prevents stale closures.
Note: The SWR key includes
pendingOrders.length, which means any change in the count of pending orders will trigger revalidation for all orders - this aligns with the PR objective of updating quotes for all pending orders rather than selectively.apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater/hooks/useUpdatePendingOrders.ts (1)
49-73: Consider the implications of parallel order processing.The
forEachloop processes all orders in parallel without waiting for completion. While this provides better performance, it means there's no way to handle overall completion or implement batch error handling if needed.This appears to be intentional for performance reasons, but consider documenting this behavior or adding error aggregation if needed for debugging.
apps/cowswap-frontend/src/common/updaters/orders/UnfillableOrdersUpdater/hooks/useUpdateIsUnfillableFlag.ts (2)
47-51: Good defensive programming for analytics.The check
typeof order.isUnfillable !== 'undefined'prevents analytics events from being sent on initial state, which is appropriate since the default undefined value doesn't represent a meaningful state change.
54-62: Comprehensive price data update.The pending order prices update includes all necessary data points: timestamp, market price, estimated execution price, and fee amount. The conversion of fee to
CurrencyAmountusing the market price base currency is correct.
alfetopito
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
… fix/order-price-to-fill-1 # Conflicts: # apps/cowswap-frontend/src/modules/ordersTable/hooks/useNavigateToAllOrdersTable.ts # apps/cowswap-frontend/src/modules/ordersTable/hooks/useNavigateToOpenOrdersTable.ts

Summary
UnfillableOrdersUpdatergetQuotefunction and usedtradingSDKinsteadTo Test
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores