fix: unify order summary #6773
fix: unify order summary #6773
Conversation
TODO: decompose useRecentActivity.ts file
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughRefactors order summary rendering from strings to React components, adds a typed OrderStatusEvents emitter, replaces CowWidget event wiring with ORDER_STATUS_EVENT_EMITTER, and restructures order notification components to use useUltimateOrder and OrderNotificationContent. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor EmitterUser as Producer
participant ORDER_EMIT as ORDER_STATUS_EVENT_EMITTER
participant HANDLER as OrdersNotificationsHandler
participant POSTED as PostedOrderNotification
participant NOTIF as OrderNotificationContent
Participant UI as UserToast/UI
EmitterUser->>ORDER_EMIT: emit OrderStatusEvents.ON_POSTED_ORDER(payload)
ORDER_EMIT->>HANDLER: deliver OnPostedOrderPayload
HANDLER->>POSTED: render PostedOrderNotification(payload)
POSTED->>NOTIF: map payload → OrderNotificationInfo
NOTIF->>UI: render title + <OrderSummary/> (ReactNode) + ReceiverInfo?
Note right of NOTIF: getToastMessageCallback → WIDGET_EVENT_EMITTER
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🧰 Additional context used🧠 Learnings (5)📚 Learning: 2025-10-10T20:28:16.565ZApplied to files:
📚 Learning: 2025-10-06T15:15:04.913ZApplied to files:
📚 Learning: 2025-07-24T16:42:53.154ZApplied to files:
📚 Learning: 2025-07-24T16:43:47.639ZApplied to files:
📚 Learning: 2025-07-24T10:00:45.353ZApplied to files:
⏰ 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)
🔇 Additional comments (6)
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 |
shoom3301
force-pushed
the
refactor/order-summary-2
branch
from
7f1b3c7 to
374b0b3
Compare
| * currencyAmountToTokenAmount converts native token to wrapped | ||
| * but here we don't want to convert wrapped token to native | ||
| */ | ||
| function currencyAmountToTokenAmountIfNeeded(amount: CurrencyAmount<Currency>): CurrencyAmount<Token> { |
There was a problem hiding this comment.
We should not do that, it was causing #6676.
| return <BridgingSuccessNotification payload={payload} /> | ||
| }, | ||
| }, | ||
| [CowWidgetEvents.ON_TOAST_MESSAGE]: { |
There was a problem hiding this comment.
Because we split extracted ORDER_STATUS_EVENT_EMITTER, we don't need those mocks anymore
|
|
||
| import { WIDGET_EVENT_EMITTER } from 'widgetEventEmitter' | ||
|
|
||
| export function getToastMessageCallback( |
There was a problem hiding this comment.
Not a new code, just moved it from another directory
| // Transaction | ||
| (activityDerivedState.summary ?? | ||
| // Order | ||
| (ultimateOrder ? computeOrderSummary(ultimateOrder) : null) ?? |
There was a problem hiding this comment.
computeOrderSummary was never called here, lol
| ) | ||
| } | ||
|
|
||
| function summaryTemplate({ |
There was a problem hiding this comment.
Maybe we can improve this component or its documentation so that it's clear if srcChainData or dstChainData is ever undefined.
…ol/cowswap into refactor/order-summary-1
… refactor/order-summary-1 # Conflicts: # apps/cowswap-frontend/src/common/containers/CancellationModal/index.tsx # apps/cowswap-frontend/src/common/pure/CancellationModal/RequestCancellationModal.tsx # apps/cowswap-frontend/src/common/updaters/orders/utils.ts # apps/cowswap-frontend/src/modules/account/containers/Transaction/ActivityDetails.tsx
…ocol/cowswap into refactor/order-summary-2
… refactor/order-summary-2 # Conflicts: # apps/cowswap-frontend/src/common/containers/CancellationModal/index.tsx # apps/cowswap-frontend/src/common/pure/CancellationModal/types.ts # apps/cowswap-frontend/src/common/updaters/orders/utils.ts # apps/cowswap-frontend/src/modules/account/containers/Transaction/ActivityDetails.tsx
![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 (4)
apps/cowswap-frontend/src/common/pure/OrderSummary/index.tsx (1)
35-49: Consider more granularuseMemodependencies for stability.Using the entire
propsobject as a dependency means these memos will recompute whenever any prop changes (or if the parent passes a new object reference). Since the computation only depends on specific fields, consider destructuring the relevant props:🔎 Suggested refinement
const inputAmount = useMemo(() => { if ('inputAmount' in props) { return props.inputAmount } return CurrencyAmount.fromRawAmount(TokenWithLogo.fromToken(props.inputToken), props.sellAmount) - }, [props]) + }, ['inputAmount' in props ? props.inputAmount : null, 'inputToken' in props ? props.inputToken : null, 'sellAmount' in props ? props.sellAmount : null]) const outputAmount = useMemo(() => { if ('outputAmount' in props) { return props.outputAmount } return CurrencyAmount.fromRawAmount(TokenWithLogo.fromToken(props.outputToken), props.buyAmount) - }, [props]) + }, ['outputAmount' in props ? props.outputAmount : null, 'outputToken' in props ? props.outputToken : null, 'buyAmount' in props ? props.buyAmount : null])Alternatively, extract individual props at the top of the function with type narrowing, then use those specific values in dependencies.
apps/cowswap-frontend/src/modules/orders/utils/emitPostedOrderEvent.ts (1)
52-61: Type assertion oncurrencyToTokenInforeturn.The
as TokenInfocast on line 60 assumes that either:
- The native token object (with overwritten
address) matchesTokenInfo, or- The non-native currency already conforms to
TokenInfoThis works because
Currencyfrom Uniswap SDK includes all fields needed forTokenInfo, but consider adding a more explicit type guard or mapping ifTokenInfoshape diverges in the future.🔎 Alternative with explicit mapping
function currencyToTokenInfo(currency: Currency): TokenInfo { - return ( - getIsNativeToken(currency) - ? { - ...currency, - address: NATIVE_CURRENCY_ADDRESS, - } - : currency - ) as TokenInfo + const address = getIsNativeToken(currency) ? NATIVE_CURRENCY_ADDRESS : currency.address + return { + chainId: currency.chainId, + address, + decimals: currency.decimals, + symbol: currency.symbol, + name: currency.name, + } as TokenInfo }apps/cowswap-frontend/src/modules/orders/containers/OrderNotification/index.tsx (1)
50-50: Missingnullin implicit return.Line 50 has an implicit return without a value (
return), which returnsundefined. For consistency with React patterns and the component's implicitReactNodereturn type, consider returningnullexplicitly.🔎 Proposed fix
- if (!orderInfo) return + if (!orderInfo) return nullapps/cowswap-frontend/src/modules/orders/containers/BridgingSuccessNotification/index.tsx (1)
43-60: Consider documenting chain data presence and fallback behavior for bridge orders.The
summaryTemplatefunction defensively handles missingsrcChainDataanddstChainData, but for a bridge success notification, the fallback rendering—showing only "Sell {inputAmount}" without output amount or destination chain—may be incomplete.The code design shows this is intentional (OrderSummary only passes chain data when
isBridgeOrderis true, requiring bothsrcChainDataanddstChainData). However, it would be helpful to add JSDoc documenting:
- When/why chain data might be undefined for bridge orders
- What the fallback behavior represents (incomplete order summary)
This clarification would make it explicit whether the fallback is an expected edge case or a potential data flow issue.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
apps/cowswap-frontend/src/common/containers/CancellationModal/index.tsxapps/cowswap-frontend/src/common/pure/CancellationModal/index.tsxapps/cowswap-frontend/src/common/pure/CancellationModal/types.tsapps/cowswap-frontend/src/common/pure/OrderSummary/index.tsxapps/cowswap-frontend/src/common/pure/OrderSummary/summaryTemplates.tsxapps/cowswap-frontend/src/common/updaters/orders/utils.tsapps/cowswap-frontend/src/legacy/state/enhancedTransactions/hooks/index.tsapps/cowswap-frontend/src/modules/account/containers/Transaction/ActivityDetails.tsxapps/cowswap-frontend/src/modules/orders/containers/BridgingSuccessNotification/index.tsxapps/cowswap-frontend/src/modules/orders/containers/FulfilledOrderInfo/index.tsxapps/cowswap-frontend/src/modules/orders/containers/OrderNotification/index.tsxapps/cowswap-frontend/src/modules/orders/containers/OrderNotification/utils.tsapps/cowswap-frontend/src/modules/orders/containers/PostedOrderNotification/index.tsxapps/cowswap-frontend/src/modules/orders/events/events.tsapps/cowswap-frontend/src/modules/orders/events/orderStatusEventEmitter.tsapps/cowswap-frontend/src/modules/orders/pure/OrderNotificationContent/index.tsxapps/cowswap-frontend/src/modules/orders/pure/OrderNotificationContent/utils.tsapps/cowswap-frontend/src/modules/orders/pure/PendingOrderNotification/index.tsxapps/cowswap-frontend/src/modules/orders/types.tsapps/cowswap-frontend/src/modules/orders/updaters/OrdersNotificationsUpdater/handlers.tsxapps/cowswap-frontend/src/modules/orders/updaters/OrdersNotificationsUpdater/index.tsxapps/cowswap-frontend/src/modules/orders/utils/emitBridgingSuccessEvent.tsapps/cowswap-frontend/src/modules/orders/utils/emitCancelledOrderEvent.tsapps/cowswap-frontend/src/modules/orders/utils/emitExpiredOrderEvent.tsapps/cowswap-frontend/src/modules/orders/utils/emitFulfilledOrderEvent.tsapps/cowswap-frontend/src/modules/orders/utils/emitPostedOrderEvent.tsapps/cowswap-frontend/src/modules/orders/utils/emitPresignedOrderEvent.ts
💤 Files with no reviewable changes (2)
- apps/cowswap-frontend/src/modules/orders/pure/PendingOrderNotification/index.tsx
- apps/cowswap-frontend/src/modules/orders/containers/OrderNotification/utils.ts
🧰 Additional context used
🧠 Learnings (7)
📚 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/common/pure/CancellationModal/index.tsxapps/cowswap-frontend/src/modules/orders/containers/PostedOrderNotification/index.tsxapps/cowswap-frontend/src/legacy/state/enhancedTransactions/hooks/index.tsapps/cowswap-frontend/src/modules/orders/types.tsapps/cowswap-frontend/src/modules/orders/containers/FulfilledOrderInfo/index.tsxapps/cowswap-frontend/src/common/pure/OrderSummary/index.tsxapps/cowswap-frontend/src/modules/account/containers/Transaction/ActivityDetails.tsxapps/cowswap-frontend/src/common/pure/CancellationModal/types.tsapps/cowswap-frontend/src/common/updaters/orders/utils.tsapps/cowswap-frontend/src/modules/orders/pure/OrderNotificationContent/index.tsxapps/cowswap-frontend/src/modules/orders/utils/emitPostedOrderEvent.tsapps/cowswap-frontend/src/modules/orders/containers/OrderNotification/index.tsxapps/cowswap-frontend/src/modules/orders/containers/BridgingSuccessNotification/index.tsxapps/cowswap-frontend/src/common/containers/CancellationModal/index.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/legacy/state/enhancedTransactions/hooks/index.tsapps/cowswap-frontend/src/modules/account/containers/Transaction/ActivityDetails.tsx
📚 Learning: 2025-08-12T05:57:08.021Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6138
File: libs/hook-dapp-lib/src/hookDappsRegistry.ts:1-1
Timestamp: 2025-08-12T05:57:08.021Z
Learning: The matchHooksToDapps function in libs/hook-dapp-lib/src/utils.ts provides backward compatibility for permit hooks through function selector detection (EIP_2612_PERMIT_SELECTOR and DAI_PERMIT_SELECTOR) rather than dappId matching, making it robust against dappId changes.
Applied to files:
apps/cowswap-frontend/src/legacy/state/enhancedTransactions/hooks/index.ts
📚 Learning: 2025-10-06T15:15:04.913Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6338
File: apps/cowswap-frontend/src/modules/ordersTable/pure/OrdersTableContainer/ConnectWalletContent.tsx:39-41
Timestamp: 2025-10-06T15:15:04.913Z
Learning: In the OrdersTable module (apps/cowswap-frontend/src/modules/ordersTable/), ConnectWalletContent uses `<h4>` while NoOrdersContent and warning sections use `<h3>` because they represent different styled titles with different visual treatments. This heading level difference is intentional.
Applied to files:
apps/cowswap-frontend/src/modules/orders/pure/OrderNotificationContent/index.tsx
📚 Learning: 2025-05-26T12:39:29.009Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 5715
File: libs/common-const/src/tokens.ts:539-555
Timestamp: 2025-05-26T12:39:29.009Z
Learning: The team accepts using NATIVE_CURRENCY_ADDRESS as a temporary placeholder for COW token contract addresses on new networks (Polygon, Avalanche) until actual COW contracts are deployed.
Applied to files:
apps/cowswap-frontend/src/modules/orders/utils/emitPostedOrderEvent.ts
📚 Learning: 2025-09-19T11:38:59.206Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6232
File: apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx:199-200
Timestamp: 2025-09-19T11:38:59.206Z
Learning: The makeBuildClickEvent function in apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx takes five parameters: defaultChainId, contextLabel, mode, isSwapMode, and chainsCount. The chainsCount parameter is used to determine the CrossChain flag in analytics events.
Applied to files:
apps/cowswap-frontend/src/modules/orders/utils/emitPostedOrderEvent.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/orders/containers/BridgingSuccessNotification/index.tsx
🧬 Code graph analysis (17)
apps/cowswap-frontend/src/modules/orders/utils/emitPresignedOrderEvent.ts (4)
apps/cowswap-frontend/src/modules/orders/index.ts (1)
emitPresignedOrderEvent(19-19)libs/events/src/types/orders.ts (1)
OnPresignedOrderPayload(35-37)apps/cowswap-frontend/src/widgetEventEmitter.ts (1)
WIDGET_EVENT_EMITTER(4-4)apps/cowswap-frontend/src/modules/orders/events/orderStatusEventEmitter.ts (1)
ORDER_STATUS_EVENT_EMITTER(5-5)
apps/cowswap-frontend/src/modules/orders/utils/emitBridgingSuccessEvent.ts (4)
apps/cowswap-frontend/src/modules/orders/index.ts (1)
emitBridgingSuccessEvent(14-14)libs/events/src/types/orders.ts (1)
OnBridgingSuccessPayload(39-39)apps/cowswap-frontend/src/widgetEventEmitter.ts (1)
WIDGET_EVENT_EMITTER(4-4)apps/cowswap-frontend/src/modules/orders/events/orderStatusEventEmitter.ts (1)
ORDER_STATUS_EVENT_EMITTER(5-5)
apps/cowswap-frontend/src/modules/orders/events/orderStatusEventEmitter.ts (2)
libs/events/src/CowEventEmitter.ts (1)
SimpleCowEventEmitter(17-48)apps/cowswap-frontend/src/modules/orders/events/events.ts (1)
OrderStatusEventPayloadMap(28-35)
apps/cowswap-frontend/src/modules/orders/events/events.ts (2)
libs/events/src/types/orders.ts (5)
OnFulfilledOrderPayload(25-27)OnCancelledOrderPayload(29-31)OnExpiredOrderPayload(33-33)OnPresignedOrderPayload(35-37)OnBridgingSuccessPayload(39-39)libs/events/src/CowEventEmitter.ts (1)
CowEventListener(7-9)
apps/cowswap-frontend/src/modules/orders/containers/PostedOrderNotification/index.tsx (3)
apps/cowswap-frontend/src/modules/orders/events/events.ts (1)
OnPostedOrderPayload(21-25)apps/cowswap-frontend/src/modules/orders/containers/OrderNotification/index.tsx (1)
OrderNotification(21-55)apps/cowswap-frontend/src/modules/orders/types.ts (1)
OrderNotificationInfo(6-13)
apps/cowswap-frontend/src/modules/orders/pure/OrderNotificationContent/utils.ts (2)
libs/events/src/types/toastMessages.ts (2)
ToastMessagePayloads(15-59)OnToastMessagePayload(91-93)apps/cowswap-frontend/src/widgetEventEmitter.ts (1)
WIDGET_EVENT_EMITTER(4-4)
apps/cowswap-frontend/src/modules/orders/utils/emitCancelledOrderEvent.ts (4)
apps/cowswap-frontend/src/modules/orders/index.ts (1)
emitCancelledOrderEvent(15-15)libs/events/src/types/orders.ts (1)
OnCancelledOrderPayload(29-31)apps/cowswap-frontend/src/widgetEventEmitter.ts (1)
WIDGET_EVENT_EMITTER(4-4)apps/cowswap-frontend/src/modules/orders/events/orderStatusEventEmitter.ts (1)
ORDER_STATUS_EVENT_EMITTER(5-5)
apps/cowswap-frontend/src/modules/orders/types.ts (1)
apps/cowswap-frontend/src/common/types.ts (1)
TradeAmounts(33-36)
apps/cowswap-frontend/src/modules/orders/utils/emitFulfilledOrderEvent.ts (5)
apps/cowswap-frontend/src/modules/orders/index.ts (1)
emitFulfilledOrderEvent(17-17)libs/widget-lib/src/types.ts (1)
SupportedChainId(4-4)libs/types/src/bridge.ts (1)
BridgeOrderDataSerialized(33-33)apps/cowswap-frontend/src/widgetEventEmitter.ts (1)
WIDGET_EVENT_EMITTER(4-4)apps/cowswap-frontend/src/modules/orders/events/orderStatusEventEmitter.ts (1)
ORDER_STATUS_EVENT_EMITTER(5-5)
apps/cowswap-frontend/src/common/pure/OrderSummary/index.tsx (3)
apps/cowswap-frontend/src/common/pure/OrderSummary/summaryTemplates.tsx (1)
SellForAtLeastTemplate(15-29)apps/cowswap-frontend/src/common/types.ts (1)
TradeAmounts(33-36)libs/common-const/src/types.ts (1)
TokenWithLogo(6-36)
apps/cowswap-frontend/src/modules/orders/updaters/OrdersNotificationsUpdater/index.tsx (3)
apps/cowswap-frontend/src/modules/orders/updaters/OrdersNotificationsUpdater/handlers.tsx (1)
ORDERS_NOTIFICATION_HANDLERS(26-111)apps/cowswap-frontend/src/modules/orders/events/events.ts (2)
OrderStatusEventPayloadMap(28-35)OrderStatusEventListener(39-39)apps/cowswap-frontend/src/modules/orders/events/orderStatusEventEmitter.ts (1)
ORDER_STATUS_EVENT_EMITTER(5-5)
apps/cowswap-frontend/src/modules/orders/utils/emitExpiredOrderEvent.ts (4)
apps/cowswap-frontend/src/modules/orders/index.ts (1)
emitExpiredOrderEvent(16-16)libs/events/src/types/orders.ts (1)
OnExpiredOrderPayload(33-33)apps/cowswap-frontend/src/widgetEventEmitter.ts (1)
WIDGET_EVENT_EMITTER(4-4)apps/cowswap-frontend/src/modules/orders/events/orderStatusEventEmitter.ts (1)
ORDER_STATUS_EVENT_EMITTER(5-5)
apps/cowswap-frontend/src/modules/orders/pure/OrderNotificationContent/index.tsx (6)
apps/cowswap-frontend/src/modules/orders/types.ts (1)
OrderNotificationInfo(6-13)apps/cowswap-frontend/src/common/pure/OrderSummary/summaryTemplates.tsx (1)
SellForAtLeastTemplate(15-29)libs/common-utils/src/legacyAddressUtils.ts (1)
shortenOrderId(129-131)apps/cowswap-frontend/src/common/pure/OrderSummary/index.tsx (1)
OrderSummary(30-76)apps/cowswap-frontend/src/modules/orders/pure/ReceiverInfo/index.tsx (1)
ReceiverInfo(15-27)apps/cowswap-frontend/src/modules/orders/pure/OrderNotificationContent/utils.ts (1)
getToastMessageCallback(5-18)
apps/cowswap-frontend/src/modules/orders/utils/emitPostedOrderEvent.ts (4)
apps/cowswap-frontend/src/widgetEventEmitter.ts (1)
WIDGET_EVENT_EMITTER(4-4)apps/cowswap-frontend/src/modules/orders/events/orderStatusEventEmitter.ts (1)
ORDER_STATUS_EVENT_EMITTER(5-5)libs/common-utils/src/getIsNativeToken.ts (1)
getIsNativeToken(13-33)libs/common-const/src/nativeAndWrappedTokens.ts (1)
NATIVE_CURRENCY_ADDRESS(10-10)
apps/cowswap-frontend/src/modules/orders/containers/OrderNotification/index.tsx (7)
apps/cowswap-frontend/src/modules/orders/pure/OrderNotificationContent/index.tsx (2)
OrderNotificationContentProps(19-32)OrderNotificationContent(34-86)apps/cowswap-frontend/src/modules/orders/types.ts (1)
OrderNotificationInfo(6-13)apps/cowswap-frontend/src/common/hooks/useUltimateOrder.ts (1)
useUltimateOrder(18-34)apps/cowswap-frontend/src/common/updaters/orders/utils.ts (1)
getUltimateOrderTradeAmounts(13-62)libs/common-utils/src/getIsNativeToken.ts (1)
getIsNativeToken(13-33)libs/common-const/src/chainInfo.ts (1)
getChainInfo(162-164)apps/cowswap-frontend/src/entities/bridgeProvider/index.ts (1)
useBridgeSupportedNetwork(1-1)
apps/cowswap-frontend/src/modules/orders/containers/BridgingSuccessNotification/index.tsx (1)
apps/cowswap-frontend/src/common/pure/OrderSummary/summaryTemplates.tsx (1)
OrderSummaryTemplateProps(7-13)
apps/cowswap-frontend/src/common/containers/CancellationModal/index.tsx (2)
apps/cowswap-frontend/src/common/updaters/orders/utils.ts (1)
getUltimateOrderTradeAmounts(13-62)apps/cowswap-frontend/src/common/pure/OrderSummary/index.tsx (1)
OrderSummary(30-76)
⏰ 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: Setup
- GitHub Check: Cypress
🔇 Additional comments (27)
apps/cowswap-frontend/src/legacy/state/enhancedTransactions/hooks/index.ts (1)
41-55: LGTM!The trailing comma additions are consistent with the codebase style and improve diff readability for future changes.
apps/cowswap-frontend/src/common/pure/OrderSummary/index.tsx (1)
14-28: LGTM!The polymorphic props pattern with a union type is well-designed. It allows callers to provide either pre-computed
CurrencyAmountvalues viaTradeAmountsor raw token/amount data viaTokensAndAmounts, giving flexibility while maintaining type safety.apps/cowswap-frontend/src/modules/orders/containers/FulfilledOrderInfo/index.tsx (1)
13-14: LGTM!The switch to absolute imports (
common/pure/OrderSummary) is consistent with the codebase conventions and improves maintainability.apps/cowswap-frontend/src/modules/orders/utils/emitBridgingSuccessEvent.ts (1)
5-10: LGTM!The dual emission pattern correctly separates concerns:
WIDGET_EVENT_EMITTERfor external widget consumers andORDER_STATUS_EVENT_EMITTERfor internal order status notifications. This aligns with the PR objective to decouple widget events from order status handling.apps/cowswap-frontend/src/modules/orders/utils/emitPresignedOrderEvent.ts (1)
5-10: LGTM!Consistent with the dual emission pattern applied to other event utilities in this PR.
apps/cowswap-frontend/src/common/pure/CancellationModal/types.ts (1)
9-10: LGTM!Expanding
summaryfromstringtoReactNodeenables richer order summary content (e.g., theOrderSummarycomponent with proper token formatting) in the cancellation modal. This aligns with the PR goal of unifying order summary rendering across the application.apps/cowswap-frontend/src/common/pure/CancellationModal/index.tsx (1)
17-22: LGTM!The
orderSummarytype expansion toReactNode | undefinedis properly propagated from the types file and correctly used in both the pending confirmation display and theRequestCancellationModal.apps/cowswap-frontend/src/modules/orders/types.ts (1)
6-13: LGTM!The
OrderNotificationInfointerface is well-designed:
- Extends
TradeAmountsfor consistent amount handling across the order notification system- Includes all necessary identity and type information for rendering notifications
- Uses
Nullish<string>forreceiverappropriately, since the receiver may be absent or explicitly nullThis type centralizes the data contract for order notifications, supporting the PR's goal of unifying order summary rendering.
apps/cowswap-frontend/src/modules/orders/containers/PostedOrderNotification/index.tsx (1)
1-42: LGTM! Clean component implementation.The PostedOrderNotification component is well-structured:
- Props are properly typed with ToastMessageType and OnPostedOrderPayload
- The mapper function cleanly transforms the payload into OrderNotificationInfo
- Safe default (
?? false) forisEthFlowOrderhandles optional fields correctly- Follows React best practices with proper typing
apps/cowswap-frontend/src/modules/orders/pure/OrderNotificationContent/utils.ts (1)
5-18: LGTM! Well-typed toast callback utility.The function correctly:
- Uses generic constraints to ensure type safety between messageType and data
- Emits to WIDGET_EVENT_EMITTER (appropriate for toast messages, separate from order status events)
- Returns a properly typed callback function
The type assertion on line 16 is necessary and safe given the generic constraints.
apps/cowswap-frontend/src/common/containers/CancellationModal/index.tsx (1)
24-32: LGTM! Clean refactor to component-based rendering.The migration from
computeOrderSummaryto theOrderSummarycomponent is well-executed:
- Uses
getUltimateOrderTradeAmountsto derive trade amounts from ultimate order data- Renders
<OrderSummary />component with proper props (inputAmount, outputAmount, kind)- Returns ReactNode instead of computed summary object, aligning with the Pure component's updated signature
This change centralizes order summary rendering and improves consistency across the codebase.
apps/cowswap-frontend/src/modules/orders/events/orderStatusEventEmitter.ts (1)
1-5: LGTM! Proper emitter separation.The ORDER_STATUS_EVENT_EMITTER singleton is correctly implemented:
- Properly typed with OrderStatusEventPayloadMap and OrderStatusEvents
- Follows the same pattern as WIDGET_EVENT_EMITTER
- Supports the PR objective of separating order status events from widget events for clearer responsibilities
apps/cowswap-frontend/src/modules/account/containers/Transaction/ActivityDetails.tsx (1)
591-591: LGTM! Correct removal of unreachable code.The simplification from
activityDerivedState.summary ?? (ultimateOrder ? computeOrderSummary(ultimateOrder) : null) ?? idtoactivityDerivedState.summary ?? idis correct. As noted in the past review comment,computeOrderSummarywas never actually called in this code path since it's in the transaction (non-order) branch.This cleanup removes dead code and improves readability.
apps/cowswap-frontend/src/modules/orders/utils/emitExpiredOrderEvent.ts (1)
8-11: LGTM! Consistent dual-emission pattern.The function now emits to both emitters:
- WIDGET_EVENT_EMITTER (line 9) - maintains backward compatibility
- ORDER_STATUS_EVENT_EMITTER (line 10) - new order status event system
This pattern is consistently applied across all emit* utilities in the PR, supporting the objective of separating widget events from order status events.
apps/cowswap-frontend/src/modules/orders/utils/emitFulfilledOrderEvent.ts (1)
15-23: LGTM! Clean dual-emission implementation.The refactor correctly:
- Extracts the payload to a variable (lines 15-19) for reuse
- Emits to both WIDGET_EVENT_EMITTER and ORDER_STATUS_EVENT_EMITTER (lines 21-22)
- Maintains the same function signature and behavior
Consistent with the dual-emission pattern applied throughout the PR.
apps/cowswap-frontend/src/modules/orders/utils/emitCancelledOrderEvent.ts (1)
8-11: LGTM! Consistent implementation.The function follows the same dual-emission pattern as the other emit* utilities:
- Maintains backward compatibility with WIDGET_EVENT_EMITTER (line 9)
- Adds ORDER_STATUS_EVENT_EMITTER emission (line 10) for the new order status event system
- Proper void return type annotation
apps/cowswap-frontend/src/modules/orders/updaters/OrdersNotificationsUpdater/index.tsx (1)
7-41: Clean migration to the new OrderStatusEvents system.The event system migration is well-structured:
- Properly imports from the new events module
- Listener registration/cleanup in useEffect follows React best practices
- Type casting is necessary due to TypeScript's handling of
Object.keys()One minor observation: using
eventTypedas the snackbarid(line 26) means all notifications of the same event type share an ID. This is likely intentional to prevent duplicate toasts for the same event type.apps/cowswap-frontend/src/common/updaters/orders/utils.ts (1)
13-62: Well-structured trade amounts derivation with clear branching logic.The function correctly handles three distinct cases:
- Bridge orders (using quote amounts)
- Fulfilled swap orders (using executed amounts)
- Pending/other swap orders (using sell + fee amounts)
The logic properly includes
feeAmountin the input for non-fulfilled orders (line 59), which represents the total amount the user is selling.apps/cowswap-frontend/src/modules/orders/pure/OrderNotificationContent/index.tsx (2)
34-86: Clean separation of pure presentation component.The component follows the container/pure pattern well:
- Accepts fully-resolved
orderInfoand chain data as props- Conditionally renders
OrderSummaryor customchildren- Properly handles
hideReceiverandskipExplorerLinkflags
88-110: Consider potential timing issues withinnerTextextraction.The
useToastRenderRefhook extractsnode.innerTextwhen the ref callback fires. This approach depends on the DOM being fully rendered with all text content when the ref is attached.If child components render asynchronously or use effects to populate text,
innerTextmight be incomplete. However, given that this renders static content (title, order ID, summary), this should work reliably in practice.Verify that toast messages display the expected content during testing of order notifications.
apps/cowswap-frontend/src/modules/orders/events/events.ts (1)
1-40: Well-designed typed event system.The event type definitions provide strong type safety:
OrderStatusEventPayloadMapenables type-safe event handlingOnPostedOrderPayloadcorrectly extends the generic payload withCurrencyAmountfields for the new status-event channel- Listener types leverage the existing
CowEventListenerpatternThis separation from
CowWidgetEventsallows the internal status updates to carry richer type information (likeCurrencyAmount) that wouldn't be appropriate for the external widget API.apps/cowswap-frontend/src/modules/orders/utils/emitPostedOrderEvent.ts (1)
44-49: Dual emission pattern enables richer internal event handling.Emitting to both
WIDGET_EVENT_EMITTER(for external widget consumers with serializable payloads) andORDER_STATUS_EVENT_EMITTER(for internal consumers withCurrencyAmountobjects) is a clean separation of concerns.apps/cowswap-frontend/src/modules/orders/updaters/OrdersNotificationsUpdater/handlers.tsx (2)
26-38:PostedOrderNotificationreceives full payload for richer rendering.This change enables the "Order submitted" notification to access
inputAmountandoutputAmountasCurrencyAmountobjects directly, addressing the PR objective to display consistent amounts on the order-submitted popup.
57-88:hideReceivercorrectly added for cancelled/expired orders.Per the PR objectives (issue #6748), the receiver should not be displayed on cancelled/expired modals unless it differs from the order creator. By setting
hideReceivertotrue, these notifications align with the expected behavior.apps/cowswap-frontend/src/modules/orders/containers/OrderNotification/index.tsx (2)
25-43: Good fallback pattern fororderInfoderivation.The logic correctly prioritizes deriving
orderInfofromultimateOrderwhen available, then falls back to the provided_orderInfoprop. This enables the component to work both:
- When order data is fetched (most cases)
- When pre-computed order info is passed directly (e.g., from
PostedOrderNotification)
57-65: Asymmetric chain info retrieval may cause subtle issues.
srcChainDatais retrieved viagetChainInfo()(synchronous lookup), whiledstChainDatausesuseBridgeSupportedNetwork()(likely a hook with state/async behavior). This asymmetry could cause:
srcChainDatato always be defined for valid chainsdstChainDatato potentially be undefined during loading statesThis is likely intentional for bridge orders where the destination might be a non-supported network, but worth noting for debugging if chain info appears inconsistent.
apps/cowswap-frontend/src/modules/orders/containers/BridgingSuccessNotification/index.tsx (1)
9-9: LGTM: Type-only import is appropriate.Using
import typeforOrderSummaryTemplatePropsis the correct approach since it's only used for type annotations, improving tree-shaking and making the intent clear.
Summary
Fixes #6748
This is a final PR in the series of 1, 2.
Changes:
computeOrderSummarywas not actually in use inActivityDetailscomponent. If was added to! isOrderblock, so I deleted it.computeOrderSummaryis only used inCancellationModal. I remembered that we haveOrderSummarycomponent, which is almost the same withcomputeOrderSummary, so I decided to replace it inCancellationModal.<OrderSummary>is uses is<OrderNotification>. I never liked that component because of it's complexity, so I decided to refactor it. One of the goals is to useuseUltimateOrder()in this component in order to have all necessary data to display a summary of any kind of order (Swap, Bridge, Twap, etc.). It's also a part of fixing Near: handle amounts on cancelled/expired swap and bridge orders #6748
<OrderNotification>is "Order submitted", because only in that case we don't have an order in store at the moment (because we just created it). To simplify that, I splitWIDGET_EVENT_EMITTERandORDER_STATUS_EVENT_EMITTER. Before we were usingWIDGET_EVENT_EMITTERfor several responsibilities, which is not good.To Test
Summary by CodeRabbit
New Features
Refactor
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.