feat(widget): filter eligible tokens by yield category#541
Conversation
|
📝 WalkthroughWalkthroughReplaces single-page legacy token fetching with an infinite-paginated ChangesPaginated Yield-Category Token Fetching
KYC authorizeUrl Migration
Withdrawal Action Types and Translations
Sequence Diagram(s)sequenceDiagram
participant SelectToken
participant EarnPageContext
participant useDefaultTokens
participant fetchDefaultTokens
participant YieldAPI as GET /v1/tokens (yield)
participant LegacyAPI as GET /v1/tokens (legacy)
SelectToken->>EarnPageContext: onLoadMoreTokens()
EarnPageContext->>useDefaultTokens: fetchNextPage()
useDefaultTokens->>fetchDefaultTokens: offset, limit, yieldTypes
alt yieldTypes present (DashboardYieldCategory set)
fetchDefaultTokens->>YieldAPI: GET /v1/tokens?yieldTypes=...&offset=N
YieldAPI-->>fetchDefaultTokens: { items[], nextOffset }
else no yield filter
fetchDefaultTokens->>LegacyAPI: GET /v1/tokens?enabledYieldsOnly
LegacyAPI-->>fetchDefaultTokens: single page
end
fetchDefaultTokens-->>useDefaultTokens: DefaultTokensPage + nextOffset
useDefaultTokens-->>EarnPageContext: flattened pages[].tokens
EarnPageContext-->>SelectToken: updated hasMoreTokens / isLoadingMoreTokens
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
petar-omni
force-pushed
the
feat/eligible-tokens-only
branch
from
7ae18ad to
f0cde9c
Compare
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/widget/tests/use-cases/rwa-kyc-flow.test.tsx (1)
48-52:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
authorizeUrlin the KYC status mock response.
mockKycStatusstill returnskycUrl(Line 51), but the production mapping now readsstatus.authorizeUrl. This mock no longer matches the API contract and can hide regressions in status-URL handling.Suggested fix
return HttpResponse.json({ kycStatus: calls > 1 && statusAfterRefresh ? statusAfterRefresh : status, - kycUrl: "https://issuer.example/verify", + authorizeUrl: "https://issuer.example/verify", });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/tests/use-cases/rwa-kyc-flow.test.tsx` around lines 48 - 52, The mock response in the rwa-kyc-flow.test.tsx file is using an outdated field name that no longer matches the production API contract. In the HttpResponse.json return statement within the mockKycStatus mock handler, change the field name from kycUrl to authorizeUrl to align with the current status mapping in production code. This ensures the mock accurately reflects the actual API response structure and prevents hiding regressions related to status-URL handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/widget/src/pages/details/earn-page/state/earn-page-context.tsx`:
- Around line 227-236: The categoryTokenSet is being derived from the paginated
defTb data, which only includes currently loaded pages and filters out scanned
balances for tokens from not-yet-loaded pages. Instead of using val.defTb to
determine which tokens belong to the selected dashboard yield category, retrieve
the full category membership information separately from the paginated subset.
Then use this complete category membership set (not just the currently loaded
page subset) to filter val.tb, ensuring tokens from all pages are properly
evaluated regardless of pagination state.
---
Outside diff comments:
In `@packages/widget/tests/use-cases/rwa-kyc-flow.test.tsx`:
- Around line 48-52: The mock response in the rwa-kyc-flow.test.tsx file is
using an outdated field name that no longer matches the production API contract.
In the HttpResponse.json return statement within the mockKycStatus mock handler,
change the field name from kycUrl to authorizeUrl to align with the current
status mapping in production code. This ensures the mock accurately reflects the
actual API response structure and prevents hiding regressions related to
status-URL handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 70785469-c32f-48f8-b413-f05e48e10f52
⛔ Files ignored due to path filters (2)
packages/widget/src/generated/api/legacy.tsis excluded by!**/generated/**packages/widget/src/generated/api/yield.tsis excluded by!**/generated/**
📒 Files selected for processing (22)
packages/widget/src/common/get-token-balances.tspackages/widget/src/domain/types/action.tspackages/widget/src/domain/types/kyc.tspackages/widget/src/domain/types/token-balance.tspackages/widget/src/hooks/api/use-default-tokens.tspackages/widget/src/pages/details/earn-page/components/select-token-section/select-token.tsxpackages/widget/src/pages/details/earn-page/state/earn-page-context.tsxpackages/widget/src/pages/details/earn-page/state/earn-page-state-context.tsxpackages/widget/src/pages/details/earn-page/state/types.tspackages/widget/src/pages/details/earn-page/state/use-get-init-yield.tspackages/widget/src/pages/details/earn-page/state/use-init-yield.tspackages/widget/src/pages/details/earn-page/state/use-token-balance.tspackages/widget/src/pages/details/earn-page/state/use-token-balances-map.tspackages/widget/src/providers/api/api-client.tspackages/widget/src/translation/English/translations.jsonpackages/widget/src/translation/French/translations.jsonpackages/widget/tests/domain/kyc.test.tspackages/widget/tests/hooks/default-tokens.test.tspackages/widget/tests/mocks/yield-api-handlers.tspackages/widget/tests/pages-dashboard/earn-details-model.test.tsxpackages/widget/tests/providers/api-client.test.tsxpackages/widget/tests/use-cases/rwa-kyc-flow.test.tsx
| const categoryTokenSet = | ||
| dashboardYieldCategoryGroupingEnabled && | ||
| selectedDashboardYieldCategory | ||
| ? new Set(val.defTb.map((item) => tokenString(item.token))) | ||
| : null; | ||
| const tokenBalancesScanData = categoryTokenSet | ||
| ? val.tb.filter((item) => | ||
| categoryTokenSet.has(tokenString(item.token)) | ||
| ) | ||
| : val.tb; |
There was a problem hiding this comment.
Paginated default-token set is being used as a hard filter for scanned balances.
On Line 227–236, categoryTokenSet is derived from defaultTokens.data (incremental pages). While more pages exist, scanned balances for not-yet-loaded category tokens are filtered out, so users can miss/selectably lose eligible tokens until they manually load further pages. Please decouple category membership from the currently loaded page subset before filtering val.tb.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/widget/src/pages/details/earn-page/state/earn-page-context.tsx`
around lines 227 - 236, The categoryTokenSet is being derived from the paginated
defTb data, which only includes currently loaded pages and filters out scanned
balances for tokens from not-yet-loaded pages. Instead of using val.defTb to
determine which tokens belong to the selected dashboard yield category, retrieve
the full category membership information separately from the paginated subset.
Then use this complete category membership set (not just the currently loaded
page subset) to filter val.tb, ensuring tokens from all pages are properly
evaluated regardless of pagination state.
|
|
||
| return useCallback( | ||
| ({ | ||
| availableYields, |
There was a problem hiding this comment.
nit: availableYields are threaded into the cached path here, but on a cache miss we still fall back to useInitYield ---> getTokenBalances, which re-fetches token discovery to rebuild the same yieldIds...could we carry availableYields through the async path too?
| ? val.tb.filter((item) => | ||
| categoryTokenSet.has(tokenString(item.token)) | ||
| ) | ||
| : val.tb; |
There was a problem hiding this comment.
same as bot: it looks like category membership is derived from the currently loaded defaultTokens pages, so eligible balance tokens from later pages can be filtered out and become unsearchable...
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/widget/src/hooks/api/use-default-tokens.ts (2)
145-152: 💤 Low valueNon-null assertions on potentially undefined pagination fields.
The fields
offset,limit, andtotalare optional inDefaultTokensPagetype (lines 37-41), yet this code uses non-null assertions. While the yield API should always return these values, if the API response is malformed or the contract changes, this will throw at runtime.Consider using default values or early validation to improve robustness:
💡 Suggested defensive approach
+ const offset = firstPage.offset ?? firstOffset; + const limit = firstPage.limit ?? DEFAULT_TOKENS_PAGE_LIMIT; + const total = firstPage.total ?? 0; + const remainingOffsets: number[] = []; for ( - let offset = firstPage.offset! + firstPage.limit!; - offset < firstPage.total!; - offset += firstPage.limit! + let nextOff = offset + limit; + nextOff < total; + nextOff += limit ) { - remainingOffsets.push(offset); + remainingOffsets.push(nextOff); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/hooks/api/use-default-tokens.ts` around lines 145 - 152, The loop in the remainingOffsets section uses non-null assertions on the optional fields offset, limit, and total from firstPage, which will throw at runtime if these fields are undefined due to a malformed API response. Instead of relying on non-null assertions, add defensive validation before the loop to ensure these fields have values (either by checking they exist and returning early, or by providing sensible defaults), then use the validated values in the loop condition and increment expression without the non-null assertion operators.
226-251: ⚖️ Poor tradeoffDuplicated yield API call construction.
The yield API request construction and response mapping (params building,
TokensControllerGetTokenscall, response transformation toDefaultTokensPage) is duplicated betweenfetchDefaultTokens(lines 109-136) and thisinfiniteQuery.queryFn. This increases maintenance burden if the API contract changes.Consider extracting a shared helper for the yield API page fetch:
💡 Possible extraction approach
Extract
fetchYieldTokensPagefromfetchDefaultTokensto module scope and reuse it here:// At module scope const fetchYieldTokensPage = async ( apiClient: ApiClient, params: { networks?: ...; yieldTypes?: ...; offset: number; limit: number }, signal?: AbortSignal ): Promise<DefaultTokensPage> => { const page = await apiClient .withOptions({ signal }) .yield.TokensControllerGetTokens({ params }); // ... mapping logic };Then reuse in both
fetchDefaultTokensandinfiniteQuery.queryFn.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/hooks/api/use-default-tokens.ts` around lines 226 - 251, The yield API request construction and response mapping logic is duplicated in both fetchDefaultTokens and the infiniteQuery.queryFn where TokensControllerGetTokens is called and responses are transformed to DefaultTokensPage. Extract this shared logic into a new module-scope helper function called fetchYieldTokensPage that accepts the apiClient, params object containing networks, yieldTypes, offset, and limit, and an optional AbortSignal. Move the API call, response mapping via toTokenBalanceScanResponse, and the return value construction into this helper, then replace both duplicate implementations with calls to this new helper function.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/widget/src/hooks/api/use-default-tokens.ts`:
- Around line 145-152: The loop in the remainingOffsets section uses non-null
assertions on the optional fields offset, limit, and total from firstPage, which
will throw at runtime if these fields are undefined due to a malformed API
response. Instead of relying on non-null assertions, add defensive validation
before the loop to ensure these fields have values (either by checking they
exist and returning early, or by providing sensible defaults), then use the
validated values in the loop condition and increment expression without the
non-null assertion operators.
- Around line 226-251: The yield API request construction and response mapping
logic is duplicated in both fetchDefaultTokens and the infiniteQuery.queryFn
where TokensControllerGetTokens is called and responses are transformed to
DefaultTokensPage. Extract this shared logic into a new module-scope helper
function called fetchYieldTokensPage that accepts the apiClient, params object
containing networks, yieldTypes, offset, and limit, and an optional AbortSignal.
Move the API call, response mapping via toTokenBalanceScanResponse, and the
return value construction into this helper, then replace both duplicate
implementations with calls to this new helper function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 84a527d4-d8df-4550-8f4b-8f4f0bcf6fee
📒 Files selected for processing (2)
packages/widget/src/hooks/api/use-default-tokens.tspackages/widget/tests/hooks/default-tokens.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/widget/tests/hooks/default-tokens.test.ts
3 participants
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Localization