Feat: support same currencies as checkout and fix currency display #118
Conversation
WalkthroughThe changes across multiple files enhance the handling of different currency types, particularly focusing on ERC20 tokens and Ethereum. The logic for preparing request parameters has been refined, and new utility functions have been introduced to improve currency management. Additionally, the structure of components has been reorganized for better clarity, and new currency identifiers have been added to support transactions involving stablecoins on the Sepolia network. Changes
Assessment against linked issues
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 (
|
aimensahnoun
requested review from
MantisClone and
sstefdev
and removed request for
sstefdev
![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
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (7)
- packages/create-invoice-form/src/lib/utils/prepareRequest.ts (1 hunks)
- packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte (5 hunks)
- packages/invoice-dashboard/src/lib/view-requests.svelte (6 hunks)
- packages/invoice-dashboard/src/utils/getCurrency.ts (1 hunks)
- packages/payment-widget/src/lib/utils/currencies.ts (1 hunks)
- packages/payment-widget/src/lib/utils/request.ts (1 hunks)
- shared/utils/initCurrencyManager.ts (1 hunks)
Additional context used
Biome
packages/invoice-dashboard/src/utils/getCurrency.ts
[error] 10-10: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
Additional comments not posted (23)
shared/utils/initCurrencyManager.ts (2)
12-45: LGTM!The expansion of the
defaultCurrencyIdsarray with new currency identifiers is approved. This change enhances the functionality and flexibility of the currency management system by increasing the variety of currencies that can be initialized through theCurrencyManager.
53-60: LGTM!The restructuring of the logic for determining the list of currencies to use is approved. The changes streamline the logic, ensure that the default currencies are always included, and improve the clarity and efficiency of the currency initialization process.
packages/payment-widget/src/lib/utils/currencies.ts (1)
50-51: LGTM!The code changes are approved. The addition of new currency identifiers for stablecoins on the Sepolia network expands the supported currencies without altering the existing functionality or behavior.
packages/create-invoice-form/src/lib/utils/prepareRequest.ts (4)
21-23: LGTM!The code changes are approved.
24-54: LGTM!The code changes are approved.
57-105: LGTM!The code changes are approved.
106-129: LGTM!The code changes are approved.
packages/payment-widget/src/lib/utils/request.ts (1)
93-93: LGTM!The code changes are approved. The conditional logic ensures that the correct representation of the currency is used based on its type, improving the flexibility and accuracy of the request parameters.
packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte (10)
2-2: LGTM!The change in the import order improves code readability without affecting functionality.
6-11: LGTM!The reorganization of import statements improves code readability without affecting functionality.
15-15: LGTM!The import statement for the
Buttoncomponent is necessary for the component to function correctly.
27-29: LGTM!The import statements for the
formatUnits,exportToPDF,walletClientToSigner, andgetCurrencyFromManagerfunctions are necessary for the component to function correctly.
40-40: LGTM!Extracting the currency determination logic into a separate
getCurrencyFromManagerfunction improves code readability and maintainability.
101-101: LGTM!Updating the
currencyvariable using thegetCurrencyFromManagerfunction ensures that the currency is always determined based on the latestrequestandcurrencyManagervalues.
113-118: LGTM!The conditional approval check based on the currency type improves efficiency by only calling the
checkApprovalfunction for ERC20 tokens and automatically setting the approval totruefor non-ERC20 currencies.
200-209: LGTM!The addition of new network options in the
getNetworkIdFromNetworkNamefunction improves the flexibility and compatibility of the component with various blockchain networks.
462-462: LGTM!The simplified error message for unsupported networks improves the user experience by providing a clear and concise indication of the issue.
34-34: Verify the impact of making therequestprop required.The
requestprop is now always expected to be defined and not optional. Ensure that all usages of theinvoice-viewcomponent provide the requiredrequestprop to avoid runtime errors.Run the following script to verify the
requestprop usage:packages/invoice-dashboard/src/lib/view-requests.svelte (5)
9-9: LGTM!The reorganization of the import statements improves code clarity and consistency without affecting functionality.
Also applies to: 11-11, 13-13
54-59: LGTM!The updated type of
activeRequestenhances the data structure and allows for easier access to formatted currency information.
200-221: LGTM!The new reactive statement
$: processedRequestsimproves data handling and presentation by transformingpaginatedRequestsinto a more useful format with formatted currency information. This allows for easier access to these values in the template.
266-269: LGTM!The updated
requestparameter type in thehandleRequestSelectfunction signature matches the new structure ofactiveRequest, ensuring seamless handling of the additional properties.
453-454: LGTM!The template changes, such as iterating over
processedRequestsand displaying theformattedAmountandcurrencySymbolproperties, reflect the new data structure and ensure the correct display of formatted currency information in the UI.Also applies to: 513-514
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- shared/utils/initCurrencyManager.ts (1 hunks)
Additional comments not posted (4)
shared/utils/initCurrencyManager.ts (4)
12-45: LGTM!The expansion of the
defaultCurrencyIdsarray is a positive change that increases the range of supported currencies, making the system more versatile.
55-62: LGTM!The restructuring of the logic for determining the list of currencies to use improves the clarity and efficiency of the currency initialization process. The changes ensure that default currencies are consistently included, enhancing the robustness of the system.
65-79: LGTM!The updated deduplication logic is more comprehensive and handles both ETH and ERC20 currency types correctly, ensuring that duplicate currencies are effectively filtered out.
48-49: LGTM!The new import statement for
Typesfrom@requestnetwork/request-client.jsis necessary for the updated deduplication logic that handles both ETH and ERC20 currency types.
|
Tip For best results, initiate chat on the files or code changes. @MantisClone, you are correct. The objective "Add REQ on Ethereum as part of the default list" was indeed addressed in the PR, as "REQ-mainnet" is included in the default list of currencies. I will update the status accordingly. Thank you for pointing this out! Analysis chainScripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for the inclusion of REQ on Ethereum in the default list of currencies.
rg 'REQ' -A 5
Length of output: 3885 |
aimensahnoun
deleted the
108-invoice-dashboard-displays-incorrect-amount-when-token-is-unknown-and-has-less-than-18-decimals
branch
MantisClone
changed the title
Resolves #61 #108
This PR adds the following:
Fixes the use of deprecated
currencyManager.from()to use eitherfromAddress()orgetNativeCurrency()depending on the use-caseEnable the possibility to create and pay with native currencies by adding
pn-eth-fee-proxy-contractAdds all of the currencies that are supported by
Payment Widgetinto the invoicing template to enable full interoperabilityCustom currencies are now appended to the default currency array instead of overriding them.
Make sure that currencies are unique so we don't have duplicates in currency manager.
Enable switching to the newly added chains.
It might be worth to add a
overrideDefaultCurrenciesboolean as a prop in the future to allow users to override the default currencies still.Screenshots:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores