fix: asset requested issue while currency is something other than USD #1594
Conversation
WalkthroughThe changes in this pull request involve the introduction of a new custom hook, Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (5)
packages/extension-polkagate/src/fullscreen/governance/post/Metadata.tsx (1)
78-79: LGTM: Implemented currency-specific values using new hook.The introduction of
useReferendaRequestedand the use ofrDecimalandrTokenalign well with the PR objectives to fix currency display issues. This change should ensure that the correct currency sign is displayed for the referendum.Consider adding explicit type annotations for improved type safety:
const { rDecimal, rToken }: { rDecimal: number, rToken: string } = useReferendaRequested(address, referendum);Also applies to: 186-188
packages/extension-polkagate/src/util/types.ts (1)
617-618: LGTM! Consider adding JSDoc comments for clarity.The additions of
decimalandtokenproperties to thePriceinterface are appropriate and align well with the PR objectives. These changes will enable more accurate handling of different currencies.Consider adding JSDoc comments to explain the purpose of these new properties:
export interface Price { price: number; /** The number of decimal places for the price */ decimal: number; /** The currency symbol or identifier */ token: string; priceChainName: string; priceDate: number; }packages/extension-polkagate/src/fullscreen/governance/post/useReferendaRequested.tsx (1)
11-14: Enhance property names for better readabilityThe properties
rAssetInCurrency,rCurrencySign,rDecimal, andrTokencould be more descriptive without therprefix. Renaming them improves clarity and aligns with common naming conventions.Apply this diff to rename the properties:
interface ReferendaRequested { - rAssetInCurrency?: number, - rCurrencySign?: string, - rDecimal?: number, - rToken?: string + assetInCurrency?: number, + currencySign?: string, + decimal?: number, + token?: string }Remember to update all occurrences of these property names throughout the code to reflect the changes.
packages/extension-polkagate/src/hooks/useTokenPrice.ts (2)
28-29: Improve JSDoc comments forassetIdandassetChainNameparametersThe descriptions for the parameters can be clarified for better understanding.
Suggested changes:
* @param assetId : asset ID on multi-asset chains - * @param assetChainName : chain name to fetch asset id price from + * @param assetChainName : name of the chain to fetch the asset price from
68-68: Consider usingtoLowerCase()instead oftoLocaleLowerCase()forpriceChainNameUsing
toLowerCase()provides consistent lowercasing without locale-specific variations, which is suitable for chain names.Suggested change:
- priceChainName: _chainName?.toLocaleLowerCase(), + priceChainName: _chainName?.toLowerCase(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- packages/extension-polkagate/src/fullscreen/governance/post/Description.tsx (4 hunks)
- packages/extension-polkagate/src/fullscreen/governance/post/Metadata.tsx (3 hunks)
- packages/extension-polkagate/src/fullscreen/governance/post/useReferendaRequested.tsx (1 hunks)
- packages/extension-polkagate/src/hooks/useTokenPrice.ts (2 hunks)
- packages/extension-polkagate/src/util/types.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (19)
packages/extension-polkagate/src/fullscreen/governance/post/Metadata.tsx (3)
24-24: LGTM: New hook import added.The import of
useReferendaRequestedis correctly placed and aligns with the PR objectives to address currency display issues.
26-26: LGTM: Minor formatting improvement.The addition of a space after the function name in
hexAddressToFormattedimproves code readability and consistency.
24-24: Overall assessment: Changes effectively address the PR objectives.The introduction of the
useReferendaRequestedhook and the use ofrDecimalandrTokenin theMetadatacomponent successfully address the currency display issue mentioned in the PR objectives. These changes ensure that the correct currency sign is displayed for the referendum, improving the accuracy of financial representations in the application.The implementation is consistent with updates in other components, as mentioned in the AI summary, indicating a cohesive approach to solving the issue across the application.
Also applies to: 26-26, 78-79, 186-188
packages/extension-polkagate/src/util/types.ts (1)
617-618: Verify usage of updated Price interface across the codebaseWhile the changes to the
Priceinterface are non-breaking, it's important to ensure that other parts of the codebase are updated to utilize these new properties for improved currency handling.Let's check the usage of the
Priceinterface across the codebase:✅ Verification successful
Regenerate search scripts for Price interface usage
The previous search encountered issues recognizing the
typescriptfile type. Please use the following updated shell scripts to accurately search for thePriceinterface and its new properties:
Usage of updated Price interface is correctly handled
All instances of the
Priceinterface have been checked, and no existing code utilizes the newdecimalortokenproperties. This indicates that the changes are non-breaking and do not affect the current codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of the Price interface echo "Searching for Price interface usage:" rg --type typescript "Price(\s*[:{]|\[\])" -g '!**/types.ts' # Search for places where price.decimal or price.token might be needed echo "\nSearching for potential places to use new Price properties:" rg --type typescript "price\.(price|priceChainName|priceDate)" -g '!**/types.ts'Length of output: 431
Script:
# Search for usages of the Price interface in TypeScript files rg "Price(\s*[:{]|\[\])" -g "*.ts" -g "*.tsx" # Search for places where price.decimal or price.token might be needed in TypeScript files rg "price\.(decimal|token)" -g "*.ts" -g "*.tsx"Length of output: 1773
packages/extension-polkagate/src/hooks/useTokenPrice.ts (8)
16-20: Addition of new properties toDEFAULT_PRICEenhances clarityThe inclusion of
decimal,priceDate, andtokenin theDEFAULT_PRICEobject ensures that all expected properties are explicitly defined, improving code readability and preventing potential undefined property access.
32-32: Function signature updated appropriately to includeassetChainNameThe addition of the
assetChainNameparameter enhances the flexibility of theuseTokenPricehook by allowing price retrieval based on a specified chain name.
33-33: Destructuring ofuseInforesults is appropriateThe destructuring correctly retrieves
addressChainName,decimal,genesisHash, andtokenfromuseInfo(address).
36-36: Assignment of_chainNameprioritizesassetChainNameappropriatelySetting
_chainNametoassetChainNameif provided, otherwise falling back toaddressChainName, ensures that the correct chain name is used for price retrieval.
50-50: Inclusion of_chainNamein null-check condition ensures robustnessAdding
_chainNameto the condition prevents potential errors when chain information is missing, enhancing the reliability of the function.
59-59: Correct determination ofpriceIdThe logic prioritizes
maybeAssetInfo?.priceId, falling back touserAddedPriceIdorgetPriceIdByChainName(_chainName), which is appropriate for accurate price identification.
62-63: Appropriate assignment of_decimaland_tokenUsing
maybeAssetInfoto obtain_decimaland_tokenenhances accuracy when asset-specific information is available, while defaulting to existing values ensures reliability.
69-70: Inclusion ofpriceDateandtokenin returned objectAdding
priceDateandtokento the return value ensures that consumers have access to the most recent price information and the token symbol.packages/extension-polkagate/src/fullscreen/governance/post/Description.tsx (7)
18-18: Imports are appropriate and necessary.The
useInfoanduseTranslationhooks are correctly imported from'../../../hooks'for use within this component.
24-24: New hookuseReferendaRequestedimported successfully.The
useReferendaRequestedhook is imported from'./useReferendaRequested', which is necessary for retrieving referendum-related data.
39-39: Proper utilization ofuseInfohook.Destructuring
apiandchainfromuseInfo(address)ensures that the component has access to necessary blockchain information based on the provided address.
41-41: Destructuring values fromuseReferendaRequestedhook.The variables
rAssetInCurrency,rCurrencySign,rDecimal, andrTokenare correctly obtained from theuseReferendaRequestedhook, which streamlines data management for the referendum.
130-132: UpdatedShowBalancecomponent props for consistency.The
decimalandtokenprops now userDecimalandrToken, ensuring that the displayed balance aligns with the selected currency and decimal precision.
151-153: Validate status color mappings and display labels.Ensure that all possible values of
referendum.statushave corresponding entries inSTATUS_COLORand that the displayed status labels are correctly formatted for user clarity.Run the following script to verify that all statuses are properly handled:
#!/bin/bash # Description: Check that STATUS_COLOR includes all statuses used by referendum.status. # Test: List all keys in STATUS_COLOR. rg 'const STATUS_COLOR = {(.*)}' --type=tsx --only-matching --replace '$1' # List all unique statuses used in the codebase. rg 'referendum\?\.status' --type=tsx --only-matching --replace '$&' # Expected: All statuses from referendum?.status should be keys in STATUS_COLOR.
142-143: Ensure currency sign and value reflect the selected currency.The
FormatPricecomponent now usesrAssetInCurrencyandrCurrencySign, which should display the correct price and currency sign based on the user's selection. Please verify that these values update appropriately with different currency settings.Run the following script to confirm that
rCurrencySignandrAssetInCurrencyare correctly set according to the selected currency:
packages/extension-polkagate/src/fullscreen/governance/post/useReferendaRequested.tsx
Show resolved
Hide resolved
packages/extension-polkagate/src/fullscreen/governance/post/useReferendaRequested.tsx
Show resolved
Hide resolved
## [0.19.4](v0.19.3...v0.19.4) (2024-10-23) ### Bug Fixes * asset requested issue while currency is something other than USD ([#1594](#1594)) ([9083e0b](9083e0b))
closes #1588
Summary by CodeRabbit
New Features
useReferendaRequested, for improved management of referendum-related data.MetadataandReferendumDescriptioncomponents to utilize the new hook for better clarity in displaying asset and currency information.Bug Fixes
Documentation
Refactor