Fix : generate pdf invoices correctly #119
Conversation
WalkthroughThe changes encompass various enhancements across multiple components within the invoice management system. Key modifications include improved handling of currency types, refinements in request parameter preparation, and updates to the PDF generation process. Additionally, new utility functions have been introduced for better currency management, and the overall structure of components has been organized for clarity and maintainability. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (8)
- 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 (7 hunks)
- packages/invoice-dashboard/src/utils/generateInvoice.ts (4 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
Learnings (1)
packages/invoice-dashboard/src/utils/getCurrency.ts (1)
Learnt from: aimensahnoun PR: RequestNetwork/web-components#118 File: packages/invoice-dashboard/src/utils/getCurrency.ts:4-14 Timestamp: 2024-09-02T20:05:27.723Z Learning: The `network` property is always present in the currency object within the `getCurrencyFromManager` function, making the non-null assertion safe to use.
Biome
packages/invoice-dashboard/src/utils/getCurrency.ts
[error] 10-10: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
Additional comments not posted (23)
packages/invoice-dashboard/src/utils/getCurrency.ts (1)
1-14: LGTM!The code changes are approved.
The static analysis error can be safely ignored based on the learning:
The
networkproperty is always present in the currency object within thegetCurrencyFromManagerfunction, making the non-null assertion safe to use.Tools
Biome
[error] 10-10: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
packages/payment-widget/src/lib/utils/currencies.ts (1)
50-51: LGTM!The code changes are approved for the following reasons:
- The additions expand the set of supported currencies by introducing new currency identifiers for
FUSDT_SEPOLIAandFUSDC_SEPOLIA.- The entries follow the existing structure of the
CURRENCY_IDconstant and map to the appropriate string values.- The change is self-contained, does not affect the control flow, and aligns with the likely intention of accommodating new currency options.
shared/utils/initCurrencyManager.ts (4)
12-43: LGTM!The code changes are approved.
48-49: LGTM!The code changes are approved.
55-62: LGTM!The code changes are approved. The new implementation ensures that the default currencies are always included and custom currencies are appended only if present, which is an improvement over the previous implementation.
65-80: LGTM!The code changes are approved. The new filtering mechanism is robust and ensures that the final list of currencies used in the
CurrencyManageris unique. The logic for identifying duplicates is well-defined and differentiates between currency types (ETH and ERC20) based on network and address properties.packages/create-invoice-form/src/lib/utils/prepareRequest.ts (1)
21-129: LGTM!The changes in the
prepareRequestParamsfunction enhance the handling of currency types and streamline the construction of the request parameters. The key improvements include:
- Introduction of a conditional check to determine if the currency type is ERC20, which influences how the currency address is assigned within the
requestInfoandcontentDatastructures.- Dynamic assignment of the payment network ID based on the currency type in the
paymentNetworksection, ensuring that the correct network is utilized for both ETH and ERC20 currencies.- Refinement of the
contentDatasection, with the addition of thecurrencyfield in theinvoiceItemsmapping, which now reflects the appropriate currency symbol or address based on the currency type.These changes not only enhance the functionality of the function but also improve its robustness and adaptability to various currency types, leading to a more reliable invoice preparation process.
packages/invoice-dashboard/src/utils/generateInvoice.ts (6)
26-28: LGTM!The
formatDateutility function is a nice addition that improves code readability by abstracting date formatting logic. It also handles the case when the date is undefined.
30-38: LGTM!The
renderAddressutility function is a nice addition that improves code readability by abstracting address rendering logic. It also handles the case when some or all address fields are missing.
46-48: LGTM!Moving the font family declaration to the
<style>block is a good change that improves separation of concerns and reduces duplication.
Line range hint
51-139: LGTM!The changes to the HTML structure of the invoice are well-implemented and improve the presentation and robustness of the invoice:
- Using the
formatDateandrenderAddressutility functions for formatting improves readability and maintainability.- Handling potentially undefined properties using optional chaining and default values ensures that the invoice remains informative even when some data is missing.
- Formatting monetary values using the
formatUnitsfunction from theviemlibrary ensures that the values are displayed correctly.
Line range hint
145-151: LGTM!Renaming the "Memo" section to "Note" improves the clarity of the section. Conditionally rendering the section based on the presence of
invoice.contentData?.noteis a good practice to avoid rendering empty sections.
159-179: LGTM!The changes to the PDF generation process are well-implemented and improve the functionality:
- Including the invoice number in the filename (with a fallback to "unknown") makes it easier to identify the generated PDF.
- Adding the
useCORSandletterRenderingoptions to thehtml2canvasconfiguration improves the rendering quality and compatibility of the generated PDF.- Using a temporary
<div>element to hold the invoice content is a good practice to avoid modifying the existing DOM structure.packages/payment-widget/src/lib/utils/request.ts (1)
93-93: LGTM!The code change is approved. It enhances the flexibility of the
prepareRequestParametersfunction by allowing it to handle different types of currency representations based on the context of the request.packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte (6)
2-2: LGTM!The reorganization of import statements enhances clarity and consistency.
Also applies to: 8-11, 15-15
34-34: LGTM!Making the
requestproperty required is a good change that enforces stricter type safety and prevents runtime errors.
40-40: LGTM!The update to use the
getCurrencyFromManagerutility function is a good change that improves readability and addresses the deprecation warning.Also applies to: 101-101
113-118: LGTM!The refinement of the approval check logic based on the currency type is a good change that streamlines the approval process and reduces unnecessary checks for non-ERC20 currencies.
200-210: LGTM!The expansion of the
getNetworkIdFromNetworkNamefunction to include more network options is a good change that broadens the component's compatibility with various blockchain networks.
462-462: LGTM!The simplification of the unsupported network error message is a good change that improves user experience by providing a clearer indication of the issue without exposing technical details.
packages/invoice-dashboard/src/lib/view-requests.svelte (3)
29-31: LGTM!The changes to the import statements enhance readability and organization.
54-59: LGTM!The changes to the
activeRequestvariable's type indicate a shift towards a more enriched data structure that allows for easier access to formatted currency information directly within the request object.
200-221: LGTM!The changes to the
processedRequestsreactive statement centralize the logic for formatting currency and ensure that all requests are processed consistently before rendering.
…111-invoice-dashboard-download-pdf-does-not-work-on-requests-created-by-the-checkout
aimensahnoun
deleted the
111-invoice-dashboard-download-pdf-does-not-work-on-requests-created-by-the-checkout
branch
Resolves #111
Invoice generated from `Payment Widget (Checkout)
Invoice created by
Invoicingwith detailsSummary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores