Conversation
📝 WalkthroughWalkthroughThis pull request adds global currency awareness to wallet queries and UI: currency is read from a global store and propagated into wallet query options and vision portfolio calls, query keys now include currency, FormattedCurrency gains a skipConversion prop, and some portfolio invalidation keys were simplified. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
![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 (2)
apps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/_components/profile-wallet-token-summary.tsx (2)
139-151: Add missing return statement.The loading skeleton JSX will never be rendered because the
returnkeyword is missing.🔧 Proposed fix
if (isFetching) { - <div className="bg-white/80 dark:bg-dark-200/90 glass-box rounded-xl p-3 flex flex-col justify-between gap-4"> + return <div className="bg-white/80 dark:bg-dark-200/90 glass-box rounded-xl p-3 flex flex-col justify-between gap-4"> <div className="flex justify-between"> <div className="w-[90px] rounded-lg animate-pulse h-[44px] bg-blue-dark-sky-040 dark:bg-blue-dark-grey" /> <div className="w-[56px] rounded-lg animate-pulse h-[24px] bg-blue-dark-sky-040 dark:bg-blue-dark-grey" /> </div> <div className="grid grid-cols-3 gap-2 md:gap-4"> <div className="w-[56px] rounded-lg animate-pulse h-[64px] bg-blue-dark-sky-040 dark:bg-blue-dark-grey" /> <div className="w-full rounded-lg animate-pulse h-[64px] bg-blue-dark-sky-040 dark:bg-blue-dark-grey" /> <div className="w-full rounded-lg animate-pulse h-[64px] bg-blue-dark-sky-040 dark:bg-blue-dark-grey" /> </div> </div>; }
93-93: AddskipConversionto theFormattedCurrencycomponent displayingfiatBalanceat line 120.Since the query passes the
currencyparameter (getAccountWalletAssetInfoQueryOptions(..., { currency })),data?.priceis returned in the selected currency. ThefiatBalancecalculation at line 93 (liquidBalance * data?.price) is therefore already in the selected currency. WithoutskipConversionat line 120, the value will be multiplied bycurrencyRateagain, causing incorrect fiat balance display values.Use
skipConversionto match the pattern at line 174, which correctly handlesdata?.pricethe same way:value: <FormattedCurrency value={fiatBalance} skipConversion />,
🤖 Fix all issues with AI agents
In
@packages/wallets/src/modules/wallets/queries/get-tokens-operations-query-options.ts:
- Around line 21-28: The queryKey built in getTokenOperationsQueryOptions omits
the currency parameter, causing cache collisions when currency changes; update
the queryKey passed to queryOptions inside getTokenOperationsQueryOptions to
include the currency value (same currency param forwarded to
getVisionPortfolioQueryOptions) so the key becomes unique per currency, ensuring
cache entries are segregated by currency.
🧹 Nitpick comments (3)
packages/wallets/src/modules/wallets/mutations/wallet-operation.ts (1)
145-145: Consider making the cache invalidation strategy more explicit.The hardcoded query key uses prefix matching to invalidate all portfolio queries regardless of currency. While this works, it's less maintainable than using a helper or constant. Consider:
- Creating a constant for the portfolio base key to ensure consistency
- Adding a comment explaining that this invalidates all currency variants
- Or, if the mutation context has access to currency, using
getVisionPortfolioQueryOptions(username, currency).queryKeyfor exact matching♻️ Example: Extract base key to a constant
In a shared constants file:
export const PORTFOLIO_QUERY_KEY_BASE = ["ecency-wallets", "portfolio", "v2"] as const;Then use it here:
queryKey: ["ecency-wallets", "portfolio", "v2", username], + // Invalidates all portfolio queries for this user (all currencies) + queryKey: [...PORTFOLIO_QUERY_KEY_BASE, username],packages/wallets/src/modules/assets/hive/mutations/claim-rewards.ts (1)
56-56: Consider consolidating the portfolio cache invalidation logic.The hardcoded query key appears in two places (lines 56 and 79) and is duplicated across multiple mutation files. This creates maintenance risk if the portfolio query key structure changes.
Consider:
- Extracting a shared constant or helper function for portfolio invalidation
- Adding a comment explaining the prefix-based invalidation strategy
- Consolidating the repeated invalidation logic into a reusable function
♻️ Example: Create a shared invalidation helper
// In a shared utils file export function invalidatePortfolioQueries(queryClient: QueryClient, username: string) { return queryClient.invalidateQueries({ // Invalidates all portfolio queries for this user (all currencies) queryKey: ["ecency-wallets", "portfolio", "v2", username], }); }Then use it:
- queryClient.invalidateQueries({ - queryKey: ["ecency-wallets", "portfolio", "v2", username], - }); + invalidatePortfolioQueries(queryClient, username);Also applies to: 79-79
packages/wallets/src/modules/wallets/queries/get-account-wallet-asset-info-query-options.ts (1)
64-98: Consider documenting the fallback currency behavior.When
portfolioAssetInfois unavailable, the code falls back to individual asset queries (HIVE, HBD, HP, etc.) that likely return USD prices. This means users may see USD pricing as a fallback even if they selected a different currency. While this is reasonable fallback behavior, it would be helpful to document this in a comment.📝 Suggested documentation
const portfolioAssetInfo = await getPortfolioAssetInfo(); if (portfolioAssetInfo) { return portfolioAssetInfo; } + // Fallback to individual asset queries when portfolio API is unavailable. + // Note: These queries return USD prices regardless of the requested currency. if (asset === "HIVE") {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
packages/wallets/dist/browser/index.d.tsis excluded by!**/dist/**packages/wallets/dist/browser/index.jsis excluded by!**/dist/**packages/wallets/dist/browser/index.js.mapis excluded by!**/dist/**,!**/*.mappackages/wallets/dist/node/index.cjsis excluded by!**/dist/**packages/wallets/dist/node/index.cjs.mapis excluded by!**/dist/**,!**/*.mappackages/wallets/dist/node/index.mjsis excluded by!**/dist/**packages/wallets/dist/node/index.mjs.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (13)
apps/web/public/sw.jsapps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/_components/profile-wallet-token-summary.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-summary.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-tokens-list-item.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-tokens-list.tsxapps/web/src/features/shared/formatted-currency.tsxpackages/wallets/package.jsonpackages/wallets/src/modules/assets/hive/mutations/claim-rewards.tspackages/wallets/src/modules/wallets/mutations/wallet-operation.tspackages/wallets/src/modules/wallets/queries/get-account-wallet-asset-info-query-options.tspackages/wallets/src/modules/wallets/queries/get-tokens-operations-query-options.tspackages/wallets/src/modules/wallets/queries/get-vision-portfolio-query-options.tspackages/wallets/src/modules/wallets/queries/use-get-account-wallet-list-query.ts
🧰 Additional context used
🧬 Code graph analysis (8)
packages/wallets/src/modules/wallets/queries/get-tokens-operations-query-options.ts (4)
packages/wallets/dist/node/index.cjs (1)
currency(3785-3785)packages/wallets/dist/browser/index.js (1)
currency(3760-3760)packages/wallets/dist/node/index.mjs (1)
currency(3758-3758)packages/wallets/src/modules/wallets/queries/get-vision-portfolio-query-options.ts (1)
getVisionPortfolioQueryOptions(487-548)
packages/wallets/src/modules/wallets/queries/use-get-account-wallet-list-query.ts (1)
packages/wallets/src/modules/wallets/queries/get-vision-portfolio-query-options.ts (1)
getVisionPortfolioQueryOptions(487-548)
apps/web/src/features/shared/formatted-currency.tsx (1)
apps/web/src/core/global-store/index.ts (1)
useGlobalStore(13-22)
apps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/_components/profile-wallet-token-summary.tsx (2)
packages/wallets/src/modules/wallets/queries/get-account-wallet-asset-info-query-options.ts (1)
getAccountWalletAssetInfoQueryOptions(31-115)apps/web/src/features/shared/formatted-currency.tsx (1)
FormattedCurrency(13-22)
apps/web/src/app/(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-summary.tsx (4)
apps/web/src/core/global-store/index.ts (1)
useGlobalStore(13-22)packages/wallets/src/modules/wallets/queries/use-get-account-wallet-list-query.ts (1)
getAccountWalletListQueryOptions(35-118)packages/wallets/src/modules/wallets/queries/get-account-wallet-asset-info-query-options.ts (1)
getAccountWalletAssetInfoQueryOptions(31-115)apps/web/src/features/shared/formatted-currency.tsx (1)
FormattedCurrency(13-22)
packages/wallets/src/modules/wallets/queries/get-account-wallet-asset-info-query-options.ts (1)
packages/wallets/src/modules/wallets/queries/get-vision-portfolio-query-options.ts (1)
getVisionPortfolioQueryOptions(487-548)
packages/wallets/src/modules/wallets/queries/get-vision-portfolio-query-options.ts (1)
packages/wallets/dist/browser/index.js (4)
price(3128-3128)token(1243-1243)token(3121-3121)currency(3760-3760)
apps/web/src/app/(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-tokens-list-item.tsx (4)
packages/wallets/dist/browser/index.js (1)
currency(3760-3760)apps/web/src/core/global-store/index.ts (1)
useGlobalStore(13-22)packages/wallets/src/modules/wallets/queries/get-account-wallet-asset-info-query-options.ts (1)
getAccountWalletAssetInfoQueryOptions(31-115)apps/web/src/features/shared/formatted-currency.tsx (1)
FormattedCurrency(13-22)
🔇 Additional comments (18)
packages/wallets/package.json (1)
4-4: Verify version bump strategy.The version has been bumped to
1.5.8(patch). However, based on the PR summary, global currency support is being introduced to the wallet system, including new parameters to several public API functions (getAccountWalletAssetInfoQueryOptions,getVisionPortfolioQueryOptions, etc.) and new props to components (FormattedCurrency).This typically warrants a minor version bump (
1.6.0) rather than a patch, as it represents an additive API change. Confirm whether:
- The new currency parameters are backward-compatible (optional, with sensible defaults)
- No breaking changes were introduced to existing public exports
- This aligns with your semver strategy
apps/web/src/app/(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-summary.tsx (2)
21-27: LGTM! Currency is properly propagated through the query chain.The currency from the global store is correctly passed to both wallet list and asset info queries, ensuring currency-aware data fetching.
106-106: Correct use of skipConversion for portfolio totals.Since the portfolio API now returns prices in the user's selected currency, using
skipConversionhere prevents double conversion of already-converted values.apps/web/src/features/shared/formatted-currency.tsx (2)
7-11: Excellent documentation for the skipConversion prop.The JSDoc clearly explains the use case with a concrete example, making it easy for other developers to understand when to use this feature.
13-20: LGTM! Clean implementation with backward compatibility.The conditional logic is straightforward, and the default value of
falseensures existing usage continues to work without changes.packages/wallets/src/modules/wallets/queries/get-account-wallet-asset-info-query-options.ts (2)
28-28: LGTM! Currency parameter properly added with sensible defaults.The optional
currencyfield with "usd" default maintains backward compatibility while enabling currency-aware queries.Also applies to: 38-38
47-47: Currency correctly propagated through portfolio queries and cache keys.Including currency in both the portfolio query call and the cache key ensures proper cache isolation for different currency requests.
Also applies to: 62-62
apps/web/src/app/(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-tokens-list.tsx (2)
3-3: LGTM! Currency integration looks clean.The global store integration for currency is implemented correctly and follows the established pattern used in other wallet components.
Also applies to: 20-20
27-27: LGTM! Currency correctly propagated to wallet list query.The currency parameter is properly passed to
getAccountWalletListQueryOptions, ensuring currency-aware data fetching.packages/wallets/src/modules/wallets/queries/use-get-account-wallet-list-query.ts (1)
35-40: LGTM! Currency parameter properly integrated.The currency parameter is correctly added to the function signature with a sensible default ("usd"), included in the queryKey for proper cache segregation, and propagated to the vision portfolio query.
packages/wallets/src/modules/wallets/queries/get-vision-portfolio-query-options.ts (2)
487-495: LGTM! Currency parameter properly integrated into portfolio query.The currency parameter is correctly:
- Added to the function signature with a sensible default ("usd")
- Included in the queryKey for proper cache segregation
- Passed in the POST request body to the API
Also applies to: 518-518
366-366: Clarify the price field fallback strategy.The price calculation omits the multiple fallback options used elsewhere in this function (title, apr, baseParts). Confirm whether
fiatRateis guaranteed by the backend for all tokens, or if fallback sources (likeprice,lastPrice, or other price fields if available) should be added for resilience.apps/web/src/app/(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-tokens-list-item.tsx (4)
4-4: LGTM! Currency from global store properly integrated.The currency is correctly retrieved from the global store and will be used to fetch currency-specific asset information.
Also applies to: 60-60
69-70: LGTM! Currency propagated to asset info query.The currency is correctly passed to
getAccountWalletAssetInfoQueryOptionsin the options object, ensuring currency-aware asset information is fetched.
74-81: LGTM! Currency propagated to token operations query.The currency parameter is correctly passed to
getTokenOperationsQueryOptions, ensuring currency-aware operations are retrieved.
314-314: LGTM! Correct use of skipConversion for asset prices.The
skipConversionprop is appropriately used here since the price from the API is already in the user's selected currency (the API receives the currency parameter). This prevents double conversion of the price value.apps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/_components/profile-wallet-token-summary.tsx (2)
43-43: LGTM!Currency extraction from the global store is correctly implemented and follows the established pattern in this component.
69-71: LGTM!The currency parameter is correctly passed to the query options, ensuring proper cache keying and currency-aware data fetching.
1 participant
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.