feat: add receipt export and viewing on request scan #142
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes involve modifications to several Svelte components and utility files related to invoice management and payment processing. Key updates include the relocation of utility function imports to a shared module, the introduction of new exported properties for handling PDF receipts and request links, and adjustments to the PDF generation logic. The updates enhance the functionality of the payment widget, allowing users to download receipts and manage request links more effectively. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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: 16
🧹 Outside diff range comments (3)
packages/payment-widget/src/lib/components/payment-confirmation.svelte (1)
Line range hint
1-589: Summary: Good progress towards PR objectives, consider implementing receipt functionality.The changes in this file contribute to the PR objectives by introducing the ability to store the created request object after successful payment processing. This lays the groundwork for implementing the receipt export and viewing functionality.
Next steps to consider:
- Implement the receipt generation functionality using the
createdRequestdata.- Add UI elements for users to choose how they want to receive the receipt (print, download PDF, email, or link).
- Implement the chosen receipt delivery methods.
- Update the component to guide users through the receipt options after successful payment.
These steps will fully address the objectives outlined in issue #113 and complete the enhancement of the payment widget functionality.
packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte (2)
Line range hint
44-45: Address the FIXME comment forgetCurrencyFromManager.There's a FIXME comment indicating that
getCurrencyFromManagermight be using a deprecated function.Please investigate the deprecation status of
getCurrencyFromManagerand update it to use a non-deprecated alternative if available. If no alternative exists, consider creating a task to address this technical debt in the future.
Line range hint
1-669: Consider addressing TODO and FIXME comments in future iterations.While reviewing the file, I noticed several TODO and FIXME comments unrelated to the current changes. These comments indicate potential technical debt or incomplete features.
To improve code quality and completeness, consider creating tasks to address these comments in future iterations. Would you like me to compile a list of these comments and open a GitHub issue to track them?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (9)
- packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte (1 hunks)
- packages/invoice-dashboard/src/lib/view-requests.svelte (2 hunks)
- packages/invoice-dashboard/src/utils/index.ts (0 hunks)
- packages/payment-widget/src/lib/components/payment-complete.svelte (3 hunks)
- packages/payment-widget/src/lib/components/payment-confirmation.svelte (2 hunks)
- packages/payment-widget/src/lib/payment-widget.svelte (3 hunks)
- packages/payment-widget/src/lib/react/PaymentWidget.d.ts (1 hunks)
- packages/payment-widget/src/lib/utils/request.ts (2 hunks)
- shared/utils/generateInvoice.ts (6 hunks)
💤 Files not reviewed due to no reviewable changes (1)
- packages/invoice-dashboard/src/utils/index.ts
🧰 Additional context used
🪛 Biome
packages/payment-widget/src/lib/utils/request.ts
[error] 279-279: This let declares a variable that is only assigned once.
'inMemoryRequest' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
🔇 Additional comments (8)
packages/payment-widget/src/lib/utils/request.ts (1)
Line range hint
279-337: LGTM! Enhancing request data consistency.The changes improve the
handleRequestPaymentfunction by ensuring that theinMemoryRequestcontains accurate payer and payee information. This enhancement aligns with the PR objectives of improving the payment widget functionality.A few observations:
- The use of
letforinMemoryRequestis appropriate, as the variable is potentially modified in the new conditional block.- The conditional block (lines 331-337) updates the
requestDatawith payer and payee information fromrequestParameters, ensuring consistency.- The use of optional chaining (
?.) in the condition is a good practice to prevent potential runtime errors.These changes contribute to a more robust and consistent handling of request data.
packages/payment-widget/src/lib/payment-widget.svelte (2)
251-251: LGTM! BindingcreatedRequesttoPaymentConfirmation.The binding of
createdRequestto thePaymentConfirmationcomponent is appropriate. This allows the component to update the request information after a successful payment confirmation, which is consistent with its purpose.
257-262: LGTM! New props forPaymentCompletecomponent.The addition of new props to the
PaymentCompletecomponent aligns well with the PR objectives for enhancing receipt management options. ThecreatedRequest,enablePdfReceipt, andenableRequestScanLinkprops provide the necessary data and control for implementing the new features.Please ensure that the
PaymentCompletecomponent correctly implements these new features:packages/invoice-dashboard/src/lib/view-requests.svelte (3)
28-29: Improved code organization with shared utilitiesThe changes to the import statements are beneficial:
- Added imports from
@requestnetwork/shared-utilsforexportToPDFandgetCurrencyFromManager.- Removed the local import of
exportToPDFfrom../utils.These changes promote code reuse and maintainability by utilizing shared utility functions instead of local implementations.
Also applies to: 36-37
520-521: Improved readability with better formattingThe adjustment to the formatting of the conditional expression within the
formatAddressfunction call enhances code readability. The ternary operator is now split across multiple lines, making it easier to understand the condition and its outcomes.
28-29: Verify receipt export functionality with new shared utilityWhile the shift to using the shared
exportToPDFutility is a good practice, it's important to ensure that this change doesn't affect the new receipt export functionality mentioned in the PR objectives.Please verify that the shared
exportToPDFfunction fully supports all the required features for exporting receipts, including any new functionality added as part of this PR.To assist with this verification, you can run the following script:
This will help ensure that the shared utility meets all the requirements for the new receipt export feature.
✅ Verification successful
Receipt export functionality with new shared utility is verified and functions as expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the implementation of exportToPDF in the shared utils ast-grep --lang typescript --pattern 'function exportToPDF($_) { $$$ }'Length of output: 75
Script:
#!/bin/bash # Search for all occurrences of exportToPDF in the codebase rg 'exportToPDF' --type typescript --context 2Length of output: 81
Script:
#!/bin/bash # List all exported functions in the shared-utils package ast-grep --lang typescript --pattern 'export function $_' --print-matchLength of output: 288
Script:
#!/bin/bash # Search for all occurrences of exportToPDF in the codebase with correct file type rg 'exportToPDF' --type ts --context 2Length of output: 269
Script:
#!/bin/bash # List all exported functions in the shared-utils package without invalid flags ast-grep --lang typescript --pattern 'export function $_'Length of output: 404
Script:
#!/bin/bash # Extract the implementation of exportToPDF exported as a constant ast-grep --lang typescript --pattern 'export const exportToPDF = async ($_) { $$$ }'Length of output: 86
Script:
#!/bin/bash # Retrieve the entire content of generateInvoice.ts to manually inspect exportToPDF cat shared/utils/generateInvoice.tsLength of output: 7184
packages/payment-widget/src/lib/components/payment-complete.svelte (1)
15-15: Verify initialization of CurrencyManager with an empty arrayYou are initializing the currency manager with an empty array:
initializeCurrencyManager([]). Ensure that this does not lead to missing currency data during PDF generation.Run the following script to check how
initializeCurrencyManageris used elsewhere:✅ Verification successful
Initialization of
CurrencyManagerwith an empty array is verified to be safe and does not lead to missing currency data during PDF generation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find usages of initializeCurrencyManager to verify proper initialization rg --type ts --type js 'initializeCurrencyManager\([^\)]*\)'Length of output: 62
Script:
#!/bin/bash # Description: Comprehensive search for usages and definition of initializeCurrencyManager # Search for all usages with any arguments rg 'initializeCurrencyManager\s*\(([^)]*)\)' --type ts --type js --type svelte # Additionally, find the definition of initializeCurrencyManager rg 'function\s+initializeCurrencyManager\s*\(' --type ts --type js --type svelteLength of output: 225
Script:
#!/bin/bash # Description: Comprehensive search for usages and definition of initializeCurrencyManager across .ts, .js, and .svelte files # Search for all usages with any arguments in .ts, .js, and .svelte files rg 'initializeCurrencyManager\s*\(([^)]*)\)' --glob '*.{ts,js,svelte}' # Additionally, find the definition of initializeCurrencyManager in .ts, .js, and .svelte files rg 'function\s+initializeCurrencyManager\s*\(' --glob '*.{ts,js,svelte}'Length of output: 600
Script:
#!/bin/bash # Description: Retrieve the implementation of initializeCurrencyManager cat shared/utils/initCurrencyManager.tsLength of output: 2296
shared/utils/generateInvoice.ts (1)
31-31: Optionallogoparameter is properly handled.Making the
logoparameter optional in theexportToPDFfunction enhances flexibility and is correctly implemented:export const exportToPDF = async ( invoice: any, currency: any, logo: string | undefined ) => { /* ... */ }The conditional rendering in the HTML template ensures that the absence of a logo doesn't cause errors.
| enablePdfReceipt?: boolean; | ||
| enableRequestScanLink?: boolean; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM! Consider updating documentation.
The new properties enablePdfReceipt and enableRequestScanLink are well-aligned with the PR objectives and the linked issue #113. They provide the necessary configuration options for the new receipt export and viewing functionality.
Consider updating the JSDoc comment for the PaymentWidget component to include these new properties in the example usage. This will help developers understand how to use these new features. Here's a suggested addition to the example:
* @example
* <PaymentWidget
* // ... other props ...
* enablePdfReceipt={true}
* enableRequestScanLink={true}
* />| export let enablePdfReceipt: boolean = true; | ||
| export let enableRequestScanLink: boolean = true; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM! Consider adding JSDoc comments.
The new exported properties enablePdfReceipt and enableRequestScanLink align well with the PR objectives for enhancing receipt management options. The default value of true ensures backward compatibility.
Consider adding JSDoc comments to describe these new properties and their purpose. For example:
/** Enable the option to generate and download a PDF receipt. Default is true. */
export let enablePdfReceipt: boolean = true;
/** Enable the option to view the request on RequestScan. Default is true. */
export let enableRequestScanLink: boolean = true;| let connectionCheckInterval: ReturnType<typeof setInterval> | null = null; | ||
| let currentPaymentStep: PaymentStep = "currency"; | ||
| let createdRequest: any; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM! Consider adding a type annotation.
The new createdRequest variable is appropriately placed and its purpose can be inferred from the context. It's correctly bound to the PaymentConfirmation component where it's likely to be updated.
Consider adding a type annotation to createdRequest for better type safety and code readability. For example:
let createdRequest: PaymentRequest | null = null;Replace PaymentRequest with the actual type of the created request object.
| export let invoiceNumber: string | undefined; | ||
| export let feeAddress: string; | ||
| export let feeAmountInUSD: number; | ||
| export let createdRequest: any; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a more specific type for createdRequest.
The use of any type for createdRequest bypasses TypeScript's type checking, which may lead to potential runtime errors. Consider using a more specific type that accurately represents the structure of the request object. Additionally, it would be helpful to add a comment explaining the purpose and usage of this variable within the component.
Here's a suggested improvement:
// Assuming there's a Request type defined elsewhere
import type { Request } from '../types';
// The created request object after successful payment processing
export let createdRequest: Request | null = null;This change provides type safety and sets a default value, making the code more robust and self-documenting.
| createdRequest = request; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM! Consider adding type assertion for createdRequest.
The assignment of the request to createdRequest is correctly placed within the try block, ensuring it only occurs on successful payment processing. This change aligns well with the PR objectives of enhancing the payment widget functionality.
To further improve type safety and error handling, consider adding a type assertion and a null check:
if (request) {
createdRequest = request as Request; // Assuming Request is the correct type
} else {
console.warn('Request object is null or undefined after payment processing');
}This change ensures that createdRequest is only assigned when request is not null or undefined, and it provides a type assertion for better type checking.
| <meta charset="UTF-8"> | ||
| <style> | ||
| body { font-family: 'Urbanist', sans-serif; } | ||
| @import url('https://fonts.googleapis.com/css2?family=Urbanist:ital,wght@0,100..900;1,100..900&display=swap'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider hosting fonts locally to improve reliability and privacy.
The stylesheet imports a font from an external source:
@import url('https://fonts.googleapis.com/css2?family=Urbanist:ital,wght@0,100..900;1,100..900&display=swap');Potential Issues:
- Privacy Concerns: Loading fonts from external servers can expose user data to third parties.
- Reliability: If the user is offline or the Google Fonts service is unavailable, the font won't load, potentially affecting the PDF's appearance.
Recommendation:
- Host Fonts Locally: Download the required font files and include them in your project's assets. Update the CSS to load the fonts from local paths.
| <p style="font-size: 10px; margin: 3px 0; word-break: break-all;">${invoice.payee?.value || "-"}</p> | ||
| <p style="font-size: 10px; margin: 3px 0;">${invoice.contentData?.sellerInfo?.firstName || ""} ${invoice.contentData?.sellerInfo?.lastName || ""}</p> | ||
| <p style="font-size: 10px; margin: 3px 0;">${renderAddress(invoice.contentData?.sellerInfo)}</p> | ||
| ${invoice.contentData?.sellerInfo?.taxRegistration ? `<p style="font-size: 10px; margin: 3px 0;">VAT: ${invoice.contentData.sellerInfo.taxRegistration}</p>` : ""} |
There was a problem hiding this comment.
Sanitize user input to prevent XSS vulnerabilities.
User-supplied data is injected directly into the HTML without sanitization:
- Seller Information (Lines 92-95)
- Buyer Information (Lines 100-103)
Examples:
<p style="font-size: 10px; margin: 3px 0; word-break: break-all;">
${invoice.payee?.value || "-"}
</p>Risks:
- Cross-Site Scripting (XSS): Malicious users could inject scripts that execute when the HTML is rendered to PDF.
Recommendations:
- Sanitize Inputs: Use a sanitization library or function to escape special characters in user data.
- Validate Data: Ensure that the data conforms to expected formats before rendering.
Also applies to: 100-103
| ? `<div style="margin-top: 20px; font-size: 10px;"> | ||
| <h3>Note:</h3> | ||
| <p>${invoice.contentData.note}</p> | ||
| </div>` |
There was a problem hiding this comment.
Sanitize the invoice note content to prevent XSS vulnerabilities.
The invoice note is directly embedded into the HTML:
<p>${invoice.contentData.note}</p>Risk:
- XSS Vulnerabilities: If the note contains malicious content, it could lead to script execution during PDF generation.
Recommendation:
- Sanitize the Note Content: Escape or remove any HTML tags and special characters from
invoice.contentData.notebefore insertion.
| <td style="text-align: right;">${ | ||
| item.unitPrice | ||
| ? formatUnits(BigInt(item.unitPrice), currency?.decimals || 0) | ||
| : "-" | ||
| }</td> | ||
| <td style="border: 1px solid #ddd; padding: 8px; text-align: right;">${ | ||
| <td style="text-align: right;">${ | ||
| item.discount | ||
| ? formatUnits(BigInt(item.discount), currency?.decimals || 0) | ||
| : "-" | ||
| }</td> | ||
| <td style="border: 1px solid #ddd; padding: 8px; text-align: right;">${ | ||
| <td style="text-align: right;">${ | ||
| item.tax?.amount ? `${item.tax.amount}%` : "-" | ||
| }</td> | ||
| <td style="border: 1px solid #ddd; padding: 8px; text-align: right;">${ | ||
| <td style="text-align: right;">${ |
There was a problem hiding this comment.
Handle potential errors when converting strings to BigInt.
Converting user-provided strings to BigInt without validation can cause exceptions:
item.unitPrice
? formatUnits(BigInt(item.unitPrice), currency?.decimals || 0)
: "-"Risk:
- Exceptions: Non-numeric or improperly formatted strings will cause
BigIntto throw an error.
Recommendations:
- Validate Numeric Strings: Ensure that
item.unitPrice,item.discount, and other monetary values are valid numeric strings before conversion. - Error Handling: Implement try-catch blocks or use conditional checks to handle invalid inputs gracefully.
| <td colspan="5" style="text-align: right;"><strong>Due:</strong></td> | ||
| <td style="text-align: right;"><strong>${ |
There was a problem hiding this comment.
Validate invoice.expectedAmount before conversion to BigInt.
When formatting the Due amount:
invoice.expectedAmount
? `${formatUnits(BigInt(invoice.expectedAmount), currency?.decimals || 0)} ${invoice.currency || ""}`
: "-"Risk:
- Uncaught Exceptions: If
invoice.expectedAmountis null, undefined, or not a valid numeric string, the conversion toBigIntwill fail.
Recommendations:
- Input Validation: Check that
invoice.expectedAmountis a valid number. - Graceful Degradation: Provide a default value or error message if the amount is invalid.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- packages/payment-widget/src/lib/components/payment-complete.svelte (3 hunks)
🔇 Additional comments (3)
packages/payment-widget/src/lib/components/payment-complete.svelte (3)
7-7: Use specific TypeScript types instead of 'any' for 'createdRequest'Using
anynegates the benefits of TypeScript's type safety. It's recommended to define a specific interface forcreatedRequestto enhance maintainability and catch potential type-related errors during development.
22-22: Handle possible undefined 'sellerLogo' to prevent runtime errorsThe
sellerLogovariable is asserted as non-null withsellerLogo!, but it can beundefinedbased on its type. This could lead to runtime errors ifsellerLogois not provided.Consider providing a default value or checking if
sellerLogois defined before using it.
24-27: Provide user feedback when receipt download failsCurrently, if an error occurs during the receipt download, it is only logged to the console. This might leave users unaware of the failure.
Consider displaying an error message to the user:
} catch (error) { console.error("Error downloading receipt:", error); + alert("Failed to download receipt. Please try again later."); }
| const currencyData = createdRequest?.inMemoryInfo?.requestData; | ||
| await exportToPDF( | ||
| currencyData, | ||
| getCurrencyFromManager(currencyData.currencyInfo, currencyManager), | ||
| sellerLogo! |
There was a problem hiding this comment.
Handle possible undefined 'currencyData' to prevent runtime errors
currencyData is assigned using optional chaining and might be undefined if createdRequest.inMemoryInfo.requestData doesn't exist. However, currencyData is used without checking for undefined, specifically when accessing currencyData.currencyInfo. This could lead to runtime errors.
Consider adding a check to ensure currencyData is defined before using it:
async function handleDownloadReceipt() {
if (createdRequest) {
try {
const currencyManager = initializeCurrencyManager([]);
- const currencyData = createdRequest?.inMemoryInfo?.requestData;
+ const currencyData = createdRequest?.inMemoryInfo?.requestData;
+ if (!currencyData) {
+ throw new Error("Currency data is unavailable.");
+ }
await exportToPDF(
currencyData,
getCurrencyFromManager(currencyData.currencyInfo, currencyManager),
sellerLogo!
);
} catch (error) {
console.error("Error downloading receipt:", error);
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const currencyData = createdRequest?.inMemoryInfo?.requestData; | |
| await exportToPDF( | |
| currencyData, | |
| getCurrencyFromManager(currencyData.currencyInfo, currencyManager), | |
| sellerLogo! | |
| const currencyData = createdRequest?.inMemoryInfo?.requestData; | |
| if (!currencyData) { | |
| throw new Error("Currency data is unavailable."); | |
| } | |
| await exportToPDF( | |
| currencyData, | |
| getCurrencyFromManager(currencyData.currencyInfo, currencyManager), | |
| sellerLogo! |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range comments (1)
shared/utils/generateInvoice.ts (1)
Line range hint
34-36: Enhance date parsing informatDateto handle invalid inputs.The
formatDatefunction may encounter invalid date strings, resulting inInvalid Datebeing displayed.const formatDate = (date: string | undefined) => { return date ? new Date(date).toLocaleDateString() : "-"; };Recommendation:
- Input Validation: Check if the date string is valid before formatting.
- Graceful Fallback: Provide a default value or error message for invalid dates.
Apply this diff to improve date handling:
const formatDate = (date: string | undefined) => { - return date ? new Date(date).toLocaleDateString() : "-"; + if (date) { + const parsedDate = new Date(date); + return isNaN(parsedDate.getTime()) ? "-" : parsedDate.toLocaleDateString(); + } else { + return "-"; + } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- packages/payment-widget/src/lib/components/payment-complete.svelte (3 hunks)
- shared/utils/generateInvoice.ts (5 hunks)
🔇 Additional comments (7)
packages/payment-widget/src/lib/components/payment-complete.svelte (6)
7-7: Use specific TypeScript types instead ofanyforcreatedRequestUsing
anynegates the benefits of TypeScript's type safety. It's recommended to define a specific interface or type forcreatedRequestto enhance maintainability and catch potential type-related errors during development.
17-17: Handle possible undefinedcurrencyDatato prevent runtime errors
currencyDatamight beundefinedifcreatedRequest.inMemoryInfo.requestDatadoesn't exist. However, it's used without checking forundefined, specifically when accessingcurrencyData.currencyInfo. This could lead to runtime errors.
19-27: Provide user feedback when receipt download failsCurrently, if an error occurs during the receipt download, it is only logged to the console. This might leave users unaware of the failure.
Consider displaying an error message to the user:
} catch (error) { console.error("Error downloading receipt:", error); + alert("Failed to download receipt. Please try again later."); }
22-22: Handle possible undefinedsellerLogoto prevent runtime errorsThe
sellerLogovariable can be an empty string based on its initialization. Using it directly inexportToPDFmight lead to unexpected results if a logo is required.Consider providing a default value or checking if
sellerLogois defined before using it:const logo = sellerLogo || 'default-logo.png'; await exportToPDF( currencyData, getCurrencyFromManager(currencyData.currencyInfo, currencyManager), - sellerLogo + logo );
58-58: EnsurecreatedRequest.requestIdis defined before using it in the URLUsing
createdRequest.requestIdwithout validation might result in an incorrect URL ifrequestIdisundefined.
52-52: EnsurehandleDownloadReceiptis properly bound in the templateThe
handleDownloadReceiptfunction is used in the button'son:clickevent handler. Make sure that the function does not rely on any incorrect context or bindings.Run the following script to check for correct usage:
shared/utils/generateInvoice.ts (1)
31-31: Verify all usages ofexportToPDFafter function signature change.The
exportToPDFfunction's signature has been updated to make thelogoparameter optional (logo: string | undefined). Please ensure that all calls toexportToPDFthroughout the codebase have been updated accordingly and handle the possibility oflogobeingundefined.Run the following script to find all usages of
exportToPDFand check for correct parameters:
| {#if enablePdfReceipt || (enableRequestScanLink && createdRequest)} | ||
| <div class="buttons-container"> | ||
| {#if enablePdfReceipt} | ||
| <button on:click={handleDownloadReceipt}>Download Receipt</button> | ||
| {/if} | ||
| {#if enableRequestScanLink && createdRequest} | ||
| <a | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| href={`https://scan.request.network/request/${createdRequest.requestId}`} | ||
| > | ||
| View on Request Scan | ||
| </a> | ||
| {/if} | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve accessibility by adding ARIA attributes to interactive elements
The buttons and links added do not include ARIA attributes or additional accessibility considerations. This might affect users who rely on assistive technologies.
Consider adding ARIA labels and roles to improve accessibility:
<button
+ aria-label="Download your receipt"
on:click={handleDownloadReceipt}
>
Download Receipt
</button>
<a
+ aria-label="View your transaction on Request Scan"
target="_blank"
rel="noopener noreferrer"
href={`https://scan.request.network/request/${createdRequest.requestId}`}
>
View on Request Scan
</a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {#if enablePdfReceipt || (enableRequestScanLink && createdRequest)} | |
| <div class="buttons-container"> | |
| {#if enablePdfReceipt} | |
| <button on:click={handleDownloadReceipt}>Download Receipt</button> | |
| {/if} | |
| {#if enableRequestScanLink && createdRequest} | |
| <a | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| href={`https://scan.request.network/request/${createdRequest.requestId}`} | |
| > | |
| View on Request Scan | |
| </a> | |
| {/if} | |
| </div> | |
| {#if enablePdfReceipt || (enableRequestScanLink && createdRequest)} | |
| <div class="buttons-container"> | |
| {#if enablePdfReceipt} | |
| <button | |
| aria-label="Download your receipt" | |
| on:click={handleDownloadReceipt} | |
| > | |
| Download Receipt | |
| </button> | |
| {/if} | |
| {#if enableRequestScanLink && createdRequest} | |
| <a | |
| aria-label="View your transaction on Request Scan" | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| href={`https://scan.request.network/request/${createdRequest.requestId}`} | |
| > | |
| View on Request Scan | |
| </a> | |
| {/if} | |
| </div> | |
| {/if} |
| .buttons-container { | ||
| display: flex; | ||
| gap: 16px; | ||
| margin-top: 24px; | ||
| button, | ||
| a { | ||
| padding: 10px 20px; | ||
| border-radius: 6px; | ||
| font-size: 14px; | ||
| font-weight: 500; | ||
| text-decoration: none; | ||
| transition: background-color 0.3s ease; | ||
| } | ||
| button { | ||
| background-color: #0bb489; | ||
| color: white; | ||
| border: none; | ||
| cursor: pointer; | ||
| &:hover { | ||
| background-color: darken(#0bb489, 10%); | ||
| } | ||
| } | ||
| a { | ||
| background-color: #f5f5f5; | ||
| color: #333; | ||
| &:hover { | ||
| background-color: darken(#f5f5f5, 10%); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize CSS by reducing redundancy in button styles
The styles for button and a elements share common properties. To improve maintainability, consider combining shared styles.
Apply this diff to merge common styles:
.buttons-container {
display: flex;
gap: 16px;
margin-top: 24px;
- button,
- a {
- padding: 10px 20px;
- border-radius: 6px;
- font-size: 14px;
- font-weight: 500;
- text-decoration: none;
- transition: background-color 0.3s ease;
- }
+ .button-style {
+ padding: 10px 20px;
+ border-radius: 6px;
+ font-size: 14px;
+ font-weight: 500;
+ text-decoration: none;
+ transition: background-color 0.3s ease;
+ }
+
+ button,
+ a {
+ @extend .button-style;
+ }
button {
background-color: #0bb489;
color: white;
border: none;
cursor: pointer;
&:hover {
background-color: darken(#0bb489, 10%);
}
}
a {
background-color: #f5f5f5;
color: #333;
&:hover {
background-color: darken(#f5f5f5, 10%);
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .buttons-container { | |
| display: flex; | |
| gap: 16px; | |
| margin-top: 24px; | |
| button, | |
| a { | |
| padding: 10px 20px; | |
| border-radius: 6px; | |
| font-size: 14px; | |
| font-weight: 500; | |
| text-decoration: none; | |
| transition: background-color 0.3s ease; | |
| } | |
| button { | |
| background-color: #0bb489; | |
| color: white; | |
| border: none; | |
| cursor: pointer; | |
| &:hover { | |
| background-color: darken(#0bb489, 10%); | |
| } | |
| } | |
| a { | |
| background-color: #f5f5f5; | |
| color: #333; | |
| &:hover { | |
| background-color: darken(#f5f5f5, 10%); | |
| } | |
| } | |
| } | |
| .buttons-container { | |
| display: flex; | |
| gap: 16px; | |
| margin-top: 24px; | |
| .button-style { | |
| padding: 10px 20px; | |
| border-radius: 6px; | |
| font-size: 14px; | |
| font-weight: 500; | |
| text-decoration: none; | |
| transition: background-color 0.3s ease; | |
| } | |
| button, | |
| a { | |
| @extend .button-style; | |
| } | |
| button { | |
| background-color: #0bb489; | |
| color: white; | |
| border: none; | |
| cursor: pointer; | |
| &:hover { | |
| background-color: darken(#0bb489, 10%); | |
| } | |
| } | |
| a { | |
| background-color: #f5f5f5; | |
| color: #333; | |
| &:hover { | |
| background-color: darken(#f5f5f5, 10%); | |
| } | |
| } | |
| } |
| <div id="invoice" style="max-width: 680px; margin: 0 auto; padding: 20px;"> | ||
| <div style="display: flex; justify-content: space-between; align-items: start;"> | ||
| ${logo ? `<img src="${logo}" alt="Logo" style="width: 50px; height: 50px;">` : ""} | ||
| ${logo && logo.length > 0 ? `<img src="${logo}" alt="Logo" style="width: 50px; height: 50px;">` : ""} |
There was a problem hiding this comment.
Validate the logo source to prevent potential security risks.
The logo is directly injected into the HTML without validation:
${logo && logo.length > 0 ? `<img src="${logo}" alt="Logo" style="width: 50px; height: 50px;">` : ""}Risks:
- Security Vulnerabilities: If
logocomes from an untrusted source, it could lead to security issues like phishing or malicious content injection.
Recommendation:
- Source Validation: Ensure that
logooriginates from a trusted source. - Content Security Policy (CSP): Implement CSP headers to restrict where images can be loaded from.
- Data Validation: Sanitize or validate the
logoURL before usage.
| sellerLogo | ||
| ); | ||
| } catch (error) { | ||
| console.error("Error downloading receipt:", error); |
There was a problem hiding this comment.
I think we need a sonner or alert when this error occurs so the user is informed.
aimensahnoun
deleted the
113-payment-widget-after-request-is-persisted-buyer-has-option-on-how-to-receive-receipt-print-download-pdf-email-link-add-step-9
branch
Resolves #113
Summary by CodeRabbit
Release Notes
New Features
Improvements
payment-completecomponent to conditionally display buttons for downloading receipts and viewing requests.Bug Fixes