Allow for failures in the batch quotes function#2301
Merged
Conversation
Closed
Contributor
WalkthroughTyped multicall handling was introduced in the RPC path, replacing untyped dynamic calls with typed Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller (rpc.rs)
participant Multicall as Multicall.Aggregate
participant Registry as ABI Registry
participant Mapper as Result Mapper
Caller->>Multicall: build per-target calls (.into_call(true))\nadd_call_dynamic(call)
Caller->>Multicall: call dynamic::<quote2Call>() and aggregate()
alt aggregate succeeds
Multicall-->>Caller: Vec<Result<quote2Return, Failure>>
Caller->>Mapper: map successes -> QuoteResult(value)
Mapper-->>Caller: success QuoteResult list
else aggregate returns MulticallError::CallFailed
Multicall-->>Caller: MulticallError::CallFailed(payload)
Caller->>Registry: decode payload via AbiDecodedErrorType
Registry-->>Caller: decoded error / decode failure
Caller->>Mapper: map decoded failures -> FailedQuote/QuoteResult
Mapper-->>Caller: failure QuoteResult list
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
hardyjosh
approved these changes
Nov 6, 2025
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts(4 hunks)packages/ui-components/src/lib/components/detail/TanstackOrderQuote.svelte(6 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
packages/ui-components/**/*.{svelte,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
UI components must pass Svelte lint/format checks (npm run svelte-lint-format-check -w @rainlanguage/ui-components)
Files:
packages/ui-components/src/lib/components/detail/TanstackOrderQuote.sveltepackages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
packages/{webapp,ui-components}/**/*.{svelte,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
If you modify frontend code, you must include a screenshot of the built webapp reflecting the change
Files:
packages/ui-components/src/lib/components/detail/TanstackOrderQuote.sveltepackages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
packages/**
📄 CodeRabbit inference engine (AGENTS.md)
JavaScript/Svelte projects live under packages/* (webapp, ui-components, orderbook)
Files:
packages/ui-components/src/lib/components/detail/TanstackOrderQuote.sveltepackages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
packages/ui-components/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
UI components tests must pass (npm run test -w @rainlanguage/ui-components)
Files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
packages/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name TypeScript test files as *.test.ts or *.spec.ts (Vitest)
Files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
🧠 Learnings (21)
📓 Common learnings
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1512
File: packages/ui-components/src/__tests__/OrderDetail.test.ts:125-143
Timestamp: 2025-04-08T12:56:03.272Z
Learning: The OrderDetail component in the Rain orderbook UI doesn't currently have error handling tests, but issue #1605 has been created to address this in the future.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2117
File: crates/js_api/src/gui/select_tokens.rs:421-424
Timestamp: 2025-09-02T08:04:34.729Z
Learning: User findolor in the rainlanguage/rain.orderbook codebase prefers not to change test code when tests are already passing, even if there appear to be type mismatches or potential issues. Working tests should be left alone unless there are actual errors.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2044
File: tauri-app/src/routes/orders/[chainId]-[orderbook]-[orderHash]/+page.svelte:76-78
Timestamp: 2025-07-31T19:03:56.594Z
Learning: In the rainlanguage/rain.orderbook project, when the Tauri app has issues preventing proper testing, the team prefers to defer Tauri-specific implementations to separate PRs rather than including untested code, especially for financial operations like withdrawals.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
📚 Learning: 2025-07-26T06:56:28.207Z
Learnt from: hardyjosh
Repo: rainlanguage/rain.orderbook PR: 2035
File: packages/ui-components/src/lib/components/tables/OrdersListTable.svelte:167-174
Timestamp: 2025-07-26T06:56:28.207Z
Learning: In OrdersListTable.svelte, maintainer hardyjosh considers keyed each-blocks unnecessary for item.inputs and item.outputs arrays because the entire query result updates at once during refresh, making DOM diffing optimizations negligible when the whole dataset is replaced rather than individual items being modified.
Applied to files:
packages/ui-components/src/lib/components/detail/TanstackOrderQuote.svelte
📚 Learning: 2025-04-09T09:28:05.097Z
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1512
File: packages/ui-components/src/__tests__/OrderDetail.test.ts:0-0
Timestamp: 2025-04-09T09:28:05.097Z
Learning: The OrderDetail component has been refactored to use an `onRemove` callback approach instead of the previous `handleOrderRemoveModal` pattern for order removal functionality, as part of PR #1512.
Applied to files:
packages/ui-components/src/lib/components/detail/TanstackOrderQuote.svelte
📚 Learning: 2025-04-08T12:31:23.761Z
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1597
File: packages/ui-components/src/__tests__/OrderDetail.test.ts:263-275
Timestamp: 2025-04-08T12:31:23.761Z
Learning: The OrderDetail component's refresh functionality doesn't currently have error handling, but issue #1604 has been created to address this in the future.
Applied to files:
packages/ui-components/src/lib/components/detail/TanstackOrderQuote.svelte
📚 Learning: 2025-07-17T13:33:08.771Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2004
File: packages/ui-components/src/lib/components/deployment/InvalidStrategiesSection.svelte:10-12
Timestamp: 2025-07-17T13:33:08.771Z
Learning: In packages/ui-components/src/lib/components/deployment/InvalidStrategiesSection.svelte, maintainer findolor is comfortable with mixed terminology during the strategy-to-order rename, where user-facing text is updated to "orders" but internal identifiers like test IDs ("invalid-strategies") and prop names ("strategiesWithErrors") can remain using "strategy" terminology.
Applied to files:
packages/ui-components/src/lib/components/detail/TanstackOrderQuote.svelte
📚 Learning: 2025-08-14T18:29:32.933Z
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2083
File: packages/ui-components/src/__tests__/VaultsListTable.test.ts:16-19
Timestamp: 2025-08-14T18:29:32.933Z
Learning: In the rain.orderbook project's UI components tests, mocking hooks like useToasts is often required as infrastructure even when not directly asserting on their calls, because components internally depend on these hooks. Removing such mocks would break component rendering in tests and require more complex test setup with providers.
Applied to files:
packages/ui-components/src/lib/components/detail/TanstackOrderQuote.sveltepackages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
📚 Learning: 2025-07-09T12:35:45.699Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1974
File: packages/ui-components/src/__tests__/DeploymentSteps.test.ts:123-126
Timestamp: 2025-07-09T12:35:45.699Z
Learning: In packages/ui-components/src/__tests__/DeploymentSteps.test.ts, findolor prefers to keep mock initializations (like setSelectToken) in individual test cases rather than consolidating them into shared beforeEach blocks, even when it results in duplication.
Applied to files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
📚 Learning: 2025-09-02T08:04:44.814Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2117
File: packages/ui-components/src/__tests__/VaultIdInformation.test.ts:9-13
Timestamp: 2025-09-02T08:04:44.814Z
Learning: In packages/ui-components/src/__tests__/VaultIdInformation.test.ts and similar test files in the rain.orderbook project, the passthrough vi.mock('rainlanguage/orderbook', async (importOriginal) => { return { ...(await importOriginal()) }; }); block is required for tests to run properly, even when not overriding any exports. This is needed due to the specific Vitest configuration or test environment setup in the project.
Applied to files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
📚 Learning: 2025-06-04T10:21:01.388Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1907
File: packages/orderbook/test/common/test.test.ts:75-77
Timestamp: 2025-06-04T10:21:01.388Z
Learning: The DotrainOrder.create API in packages/orderbook/test/common/test.test.ts is internal and not used directly in consumer applications, so API changes here don't require external breaking change documentation.
Applied to files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
📚 Learning: 2025-08-04T09:07:00.160Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2048
File: packages/webapp/src/__tests__/WithdrawModal.test.ts:36-37
Timestamp: 2025-08-04T09:07:00.160Z
Learning: In packages/webapp/src/__tests__/WithdrawModal.test.ts and similar test files, findolor considers it acceptable to directly unwrap .value from Float.fromFixedDecimal() and Float.parse() calls without explicit error handling, preferring to keep test fixtures simple rather than adding comprehensive error checking.
Applied to files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
📚 Learning: 2025-10-06T08:05:02.438Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-06T08:05:02.438Z
Learning: Applies to packages/orderbook/**/*.{test,spec}.{ts,tsx} : Orderbook tests must pass (npm run test -w rainlanguage/orderbook)
Applied to files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
📚 Learning: 2025-07-11T08:46:07.606Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1925
File: packages/ui-components/src/__tests__/DeploymentSteps.test.ts:519-554
Timestamp: 2025-07-11T08:46:07.606Z
Learning: In packages/ui-components/src/__tests__/DeploymentSteps.test.ts, findolor prefers to keep hardcoded timeout values (like 50ms and 100ms) inline in test cases rather than extracting them to named constants, viewing such refactoring as unnecessary for test maintainability.
Applied to files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
📚 Learning: 2025-04-07T08:18:36.473Z
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1565
File: packages/webapp/src/routes/deploy/layout.test.ts:12-29
Timestamp: 2025-04-07T08:18:36.473Z
Learning: In test files for this project, hardingjam prefers to use custom mocks (such as for localStorage) rather than relying on environment-provided implementations, as this allows for spying on individual methods and having precise control over implementation details for more robust testing.
Applied to files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
📚 Learning: 2025-06-08T18:43:51.842Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1891
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/page.test.ts:66-80
Timestamp: 2025-06-08T18:43:51.842Z
Learning: In the rain.orderbook webapp test files, when mocking objects like the transaction manager, it's acceptable to use simple empty objects with ts-expect-error comments rather than providing complete mock implementations with all properties and methods.
Applied to files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
📚 Learning: 2025-05-27T12:01:51.647Z
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1896
File: tauri-app/src/lib/services/executeWalletConnectOrder.ts:41-45
Timestamp: 2025-05-27T12:01:51.647Z
Learning: In the tauri-app codebase, the formatEthersTransactionErrorFn function already handles error formatting appropriately for toast messages in executeWalletConnectOrder, so additional granular error handling based on error types is not needed.
Applied to files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
📚 Learning: 2025-06-30T14:17:16.626Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1926
File: packages/ui-components/src/lib/__mocks__/stores.ts:13-17
Timestamp: 2025-06-30T14:17:16.626Z
Learning: User findolor reports that vi.mock(import('rainlanguage/orderbook'), async (importOriginal) => { ... }) syntax works in their testing environment, despite official Vitest documentation indicating the first argument should be a string. This suggests there may be specific Vitest versions or configurations that support dynamic import() as the first argument to vi.mock().
Applied to files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
📚 Learning: 2025-04-08T09:18:46.653Z
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1597
File: packages/ui-components/src/__tests__/OrderDetail.test.ts:120-120
Timestamp: 2025-04-08T09:18:46.653Z
Learning: In test files for the Rain Orderbook project, it's acceptable to bypass TypeScript's strict typing using constructs like `as unknown as [Type]` to create simplified mock objects that don't need to implement the entire interface.
Applied to files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
📚 Learning: 2025-06-17T14:55:22.914Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1936
File: packages/ui-components/vite.config.ts:21-23
Timestamp: 2025-06-17T14:55:22.914Z
Learning: In the rain.orderbook project, the Vite configuration uses `'import.meta.vitest': 'undefined'` (as a string) combined with conditional `if (import.meta.vitest)` checks for in-source testing. The mock files are excluded from test execution using `exclude: ['src/lib/__mocks__/**/*.ts']`. This configuration successfully allows dev builds to work without `vi` undefined errors, despite the theoretical expectation that the string "undefined" would be truthy and cause issues.
Applied to files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
📚 Learning: 2025-07-04T09:14:35.748Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: packages/ui-components/src/__tests__/VaultDetail.test.ts:177-181
Timestamp: 2025-07-04T09:14:35.748Z
Learning: In the rain.orderbook project, maintainer findolor is fine with mutating mock data directly in test files using ts-expect-error comments, preferring this pragmatic approach over immutable patterns like spread operators for test data updates.
Applied to files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
📚 Learning: 2025-04-09T09:40:03.131Z
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1565
File: packages/webapp/src/__tests__/registryManager.test.ts:20-21
Timestamp: 2025-04-09T09:40:03.131Z
Learning: When mocking browser APIs like localStorage for tests, it's not always necessary to fully implement all interface properties/methods with dynamic behavior. A hardcoded value (like setting length: 0) can be sufficient when the tests don't rely on that specific property.
Applied to files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
📚 Learning: 2025-04-08T12:53:12.526Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1588
File: packages/orderbook/test/js_api/gui.test.ts:2037-2057
Timestamp: 2025-04-08T12:53:12.526Z
Learning: In the remote network test for gui.test.ts, asserting that getCurrentDeployment() succeeds is sufficient validation that remote networks were properly fetched, as this operation would fail if the networks weren't correctly processed.
Applied to files:
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts
🧬 Code graph analysis (1)
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts (1)
packages/ui-components/src/lib/index.ts (1)
useToasts(146-146)
🔇 Additional comments (7)
packages/ui-components/src/lib/components/detail/TanstackOrderQuote.svelte (4)
18-18: LGTM! Clean integration of clipboard functionality.The ClipboardOutline icon import and addToast extraction are properly added to support the new copy-to-clipboard feature.
Also applies to: 35-35
101-101: Good addition for testability.The test ID enables reliable test automation for the refresh functionality.
134-134: Index parameter correctly added.The index is properly used to generate unique element IDs for per-item error display. Keyed each-blocks are not necessary here since the entire query result updates atomically.
165-187: Excellent per-item error UX implementation.The error display provides good user experience with:
- Truncated error text with full version in tooltip (lines 166-186)
- Easy copy-to-clipboard action (lines 173-180)
- Proper accessibility with aria-label (line 176)
- Correct colspan spanning the data columns (line 165)
The tooltip-before-target ordering is handled correctly by Flowbite Svelte's reactive attachment.
packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts (3)
25-26: Well-structured test infrastructure.The clipboard and toast mocks are properly initialized and reset between tests. The
configurable: trueflag at line 42 ensures clean test isolation.Based on learnings
Also applies to: 37-43, 47-47
200-202: Updated test correctly handles per-item error rendering.The assertion now properly accounts for the error appearing multiple times in the DOM (tooltip content and truncated display), which matches the new UI structure.
1-316: Reminder: Run test and lint checks before merging.As per coding guidelines, please ensure:
- Tests pass:
npm run test -w @rainlanguage/ui-components- Lint/format checks pass:
npm run svelte-lint-format-check -w @rainlanguage/ui-componentsThe PR description mentions a screenshot is included, which satisfies the frontend change documentation requirement.
As per coding guidelines
Comment on lines
+280
to
+315
| it('copies the quote error to the clipboard and notifies the user', async () => { | ||
| (mockOrder.getQuotes as Mock).mockResolvedValueOnce({ | ||
| value: [ | ||
| { | ||
| success: false, | ||
| block_number: '0x123', | ||
| pair: { pairName: 'ETH/USDT', inputIndex: 0, outputIndex: 1 }, | ||
| data: undefined, | ||
| error: 'Failed to calculate quote' | ||
| } | ||
| ] | ||
| }); | ||
|
|
||
| const queryClient = new QueryClient(); | ||
|
|
||
| render(TanstackOrderQuote, { | ||
| props: { | ||
| order: mockOrder, | ||
| handleQuoteDebugModal: vi.fn() | ||
| }, | ||
| context: new Map([['$$_queryClient', queryClient]]) | ||
| }); | ||
|
|
||
| const copyButton = await screen.findByRole('button', { name: 'Copy quote error' }); | ||
| await fireEvent.click(copyButton); | ||
|
|
||
| await waitFor(() => { | ||
| expect(clipboardWriteText).toHaveBeenCalledWith('Failed to calculate quote'); | ||
| expect(mockAddToast).toHaveBeenCalledWith({ | ||
| message: 'Copied quote error', | ||
| type: 'success', | ||
| color: 'green' | ||
| }); | ||
| expect(mockErrToast).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Comprehensive test for clipboard functionality.
The test properly validates the copy workflow using accessible selectors (findByRole) and verifies both success toast and clipboard API interaction.
Consider adding test coverage for error scenarios:
- Clipboard API unavailable (line 51-53 in component)
- Empty/undefined error parameter (line 46-49 in component)
- Clipboard write failure (line 60-64 in component)
These would increase confidence in the error handling paths. Would you like me to generate these additional test cases?
🤖 Prompt for AI Agents
In packages/ui-components/src/__tests__/TanstackOrderQuote.test.ts around lines
280 to 315, add three additional tests to cover clipboard error paths: (1)
simulate navigator.clipboard being undefined and assert the component falls back
to error toast (mockErrToast called) and does not call clipboardWriteText; (2)
simulate a quote with error === undefined or empty string and assert no
clipboard write occurs and an appropriate error toast is shown; (3) simulate
navigator.clipboard.writeText rejecting (throwing) and assert mockErrToast is
called with the failure message while mockAddToast is not called; use the same
render/queryClient setup, mock getQuotes responses per case, and await waitFor
assertions for asynchronous behavior.
Contributor
There was a problem hiding this comment.
<scratchpad_start>
@findolor, understood! I'll skip the additional error scenario tests for the clipboard functionality.
🧠 Learnings used
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2083
File: packages/ui-components/src/__tests__/VaultsListTable.test.ts:16-19
Timestamp: 2025-08-14T18:29:32.933Z
Learning: In the rain.orderbook project's UI components tests, mocking hooks like useToasts is often required as infrastructure even when not directly asserting on their calls, because components internally depend on these hooks. Removing such mocks would break component rendering in tests and require more complex test setup with providers.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1974
File: packages/ui-components/src/__tests__/DeploymentSteps.test.ts:123-126
Timestamp: 2025-07-09T12:35:45.699Z
Learning: In packages/ui-components/src/__tests__/DeploymentSteps.test.ts, findolor prefers to keep mock initializations (like setSelectToken) in individual test cases rather than consolidating them into shared beforeEach blocks, even when it results in duplication.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2117
File: packages/ui-components/src/__tests__/VaultIdInformation.test.ts:9-13
Timestamp: 2025-09-02T08:04:44.814Z
Learning: In packages/ui-components/src/__tests__/VaultIdInformation.test.ts and similar test files in the rain.orderbook project, the passthrough vi.mock('rainlanguage/orderbook', async (importOriginal) => { return { ...(await importOriginal()) }; }); block is required for tests to run properly, even when not overriding any exports. This is needed due to the specific Vitest configuration or test environment setup in the project.
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1512
File: packages/ui-components/src/__tests__/OrderDetail.test.ts:125-143
Timestamp: 2025-04-08T12:56:03.272Z
Learning: The OrderDetail component in the Rain orderbook UI doesn't currently have error handling tests, but issue #1605 has been created to address this in the future.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1996
File: packages/ui-components/src/__tests__/VaultIdInformation.test.ts:6-6
Timestamp: 2025-07-17T10:36:02.846Z
Learning: In packages/ui-components/src/__tests__/VaultIdInformation.test.ts, findolor prefers to keep exported type aliases like `VaultIdInformationComponentProps` in test files, even when static analysis tools flag this as discouraged practice.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1925
File: packages/ui-components/src/__tests__/DeploymentSteps.test.ts:519-554
Timestamp: 2025-07-11T08:46:07.606Z
Learning: In packages/ui-components/src/__tests__/DeploymentSteps.test.ts, findolor prefers to keep hardcoded timeout values (like 50ms and 100ms) inline in test cases rather than extracting them to named constants, viewing such refactoring as unnecessary for test maintainability.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2048
File: packages/webapp/src/__tests__/WithdrawModal.test.ts:36-37
Timestamp: 2025-08-04T09:07:00.160Z
Learning: In packages/webapp/src/__tests__/WithdrawModal.test.ts and similar test files, findolor considers it acceptable to directly unwrap .value from Float.fromFixedDecimal() and Float.parse() calls without explicit error handling, preferring to keep test fixtures simple rather than adding comprehensive error checking.
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-06T08:05:02.438Z
Learning: Applies to packages/ui-components/**/*.{test,spec}.{ts,tsx} : UI components tests must pass (npm run test -w rainlanguage/ui-components)
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2297
File: tauri-app/src/lib/components/MetricChart.test.ts:68-85
Timestamp: 2025-11-06T15:31:40.782Z
Learning: In tauri-app/src/lib/components/MetricChart.test.ts, findolor considers it acceptable to duplicate Float parsing and formatting logic within test helper functions like buildPlotData, rather than extracting it to shared utilities or reusing production code like decodeRainFloat from chartData.ts.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1831
File: packages/ui-components/src/lib/components/transactions/getStatusEmoji.ts:13-22
Timestamp: 2025-06-05T12:20:20.455Z
Learning: The user findolor prefers not to add test coverage for fallback cases in the getStatusEmoji function when using ts-pattern's .otherwise() method.
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1565
File: packages/webapp/src/routes/deploy/layout.test.ts:12-29
Timestamp: 2025-04-07T08:18:36.473Z
Learning: In test files for this project, hardingjam prefers to use custom mocks (such as for localStorage) rather than relying on environment-provided implementations, as this allows for spying on individual methods and having precise control over implementation details for more robust testing.
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1738
File: packages/ui-components/src/lib/providers/toasts/useToasts.ts:46-52
Timestamp: 2025-05-07T16:51:13.418Z
Learning: UX enhancements for toast notifications, such as pausing timeout on hover, were considered out of scope for the toast refactoring PR.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2117
File: crates/js_api/src/gui/select_tokens.rs:421-424
Timestamp: 2025-09-02T08:04:34.729Z
Learning: User findolor in the rainlanguage/rain.orderbook codebase prefers not to change test code when tests are already passing, even if there appear to be type mismatches or potential issues. Working tests should be left alone unless there are actual errors.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2044
File: tauri-app/src/routes/orders/[chainId]-[orderbook]-[orderHash]/+page.svelte:76-78
Timestamp: 2025-07-31T19:03:56.594Z
Learning: In the rainlanguage/rain.orderbook project, when the Tauri app has issues preventing proper testing, the team prefers to defer Tauri-specific implementations to separate PRs rather than including untested code, especially for financial operations like withdrawals.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Comment on lines
+45
to
+66
| const copyQuoteError = async (error?: string) => { | ||
| if (!error) { | ||
| errToast('No quote error to copy'); | ||
| return; | ||
| } | ||
| try { | ||
| if (typeof navigator === 'undefined' || !navigator.clipboard?.writeText) { | ||
| throw new Error('Clipboard API unavailable'); | ||
| } | ||
| await navigator.clipboard.writeText(error); | ||
| addToast({ | ||
| message: 'Copied quote error', | ||
| type: 'success', | ||
| color: 'green' | ||
| }); | ||
| } catch (copyError) { | ||
| errToast( | ||
| 'Failed to copy quote error', | ||
| copyError instanceof Error ? copyError.message : undefined | ||
| ); | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Solid implementation with good error handling.
The clipboard copy function properly checks for API availability and handles errors gracefully. The navigator check at line 51 is good defensive programming for SSR scenarios.
Consider adding test coverage for edge cases:
- When
errorparameter is undefined/empty (line 46-49 path) - When clipboard API is unavailable (line 51-53 path)
Do you want me to generate additional test cases for these scenarios?
Contributor
There was a problem hiding this comment.
@findolor, understood! I'll skip the additional test coverage for those edge cases.
✏️ Learnings added
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2301
File: packages/ui-components/src/lib/components/detail/TanstackOrderQuote.svelte:45-66
Timestamp: 2025-11-06T19:46:38.595Z
Learning: User findolor in the rainlanguage/rain.orderbook repository prefers not to add test coverage for clipboard API edge cases (e.g., undefined error parameter, clipboard API unavailability) in the TanstackOrderQuote component.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1831
File: packages/ui-components/src/lib/components/transactions/getStatusEmoji.ts:13-22
Timestamp: 2025-06-05T12:20:20.455Z
Learning: The user findolor prefers not to add test coverage for fallback cases in the getStatusEmoji function when using ts-pattern's .otherwise() method.
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1512
File: packages/ui-components/src/__tests__/OrderDetail.test.ts:125-143
Timestamp: 2025-04-08T12:56:03.272Z
Learning: The OrderDetail component in the Rain orderbook UI doesn't currently have error handling tests, but issue #1605 has been created to address this in the future.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2117
File: crates/js_api/src/gui/select_tokens.rs:421-424
Timestamp: 2025-09-02T08:04:34.729Z
Learning: User findolor in the rainlanguage/rain.orderbook codebase prefers not to change test code when tests are already passing, even if there appear to be type mismatches or potential issues. Working tests should be left alone unless there are actual errors.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2044
File: tauri-app/src/routes/orders/[chainId]-[orderbook]-[orderHash]/+page.svelte:76-78
Timestamp: 2025-07-31T19:03:56.594Z
Learning: In the rainlanguage/rain.orderbook project, when the Tauri app has issues preventing proper testing, the team prefers to defer Tauri-specific implementations to separate PRs rather than including untested code, especially for financial operations like withdrawals.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
0xgleb
approved these changes
Nov 6, 2025
Contributor
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
Contributor
|
Tip For best results, initiate chat on the files or code changes. SIZE=M 🧠 Learnings used |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
See issues:
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit