New Components - invoice_ninja #15354
Conversation
luancazarine
added
the
ai-assisted
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis pull request introduces a comprehensive set of components for the Invoice Ninja integration, including actions for creating clients, invoices, and payments, along with webhook sources for tracking new clients, invoices, and payments. The implementation provides a robust set of methods in the application module to interact with the Invoice Ninja API, including creating, retrieving, and managing various entities. The changes establish a complete workflow for managing invoicing-related operations with detailed property definitions and error handling. Changes
Assessment against linked issues
Possibly related PRs
Poem
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 (
|
Sources - New Client (Instant) - New Invoice (Instant) - New Payment (Instant) Actions - Create Client - Create Invoice - Create Payment
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (16)
components/invoice_ninja/invoice_ninja.app.mjs (4)
76-107: Optional property consideration.
The newly introduced country and group settings properties look fine. Consider marking both as optional if the API allows. This helps avoid confusion when no value is provided.
127-154: Gateway config labeling.
When returning(id: value, config: label), consider clarifying user-facing labels, becauseconfigmight not always be user-friendly.
157-165: Hard-coded base URL.
The_baseUrl()method is returning a fixed URL. If your integration needs to support multiple regions or custom domains, consider making this a parameter or prop.
196-237: Broad coverage of retrieval and CRUD operations.
The range of helper methods (getClients,getUsers, etc.) is comprehensive. Adding unit tests for these endpoints would strengthen confidence in integration correctness.Would you like me to draft basic test examples for these methods?
components/invoice_ninja/sources/new-client-instant/new-client-instant.mjs (1)
1-22: Convenient event source definition.
This source correctly extendscommon/base.mjsand usessampleEmit. ThegetSummarymethod is straightforward, though consider using a safe default ifclient.nameis missing or null.components/invoice_ninja/sources/new-invoice-instant/new-invoice-instant.mjs (1)
14-16: Avoid magic numbers for event IDs.
Returning a hard-coded event ID (2) may cause confusion or maintenance issues later. Consider referencing a constant or an enum to document the meaning of this ID.getEvent() { - return 2; + return EVENT_IDS.INVOICE_CREATED; // Example constant }components/invoice_ninja/sources/new-payment-instant/new-payment-instant.mjs (2)
14-16: Centralize event ID definitions.
Similar to the invoice source, returning just4may be unclear to future maintainers. Replace the magic number with a clearly named reference.getEvent() { - return 4; + return EVENT_IDS.PAYMENT_CREATED; // Example constant }
17-19: Ensure robust summary generation.
If a payment object is missing or ifnumberis not defined, the string interpolation could be incomplete. Safeguard with a fallback.getSummary(payment) { - return `New payment created: ${payment.number}`; + const paymentNumber = payment?.number || "N/A"; + return `New payment created: ${paymentNumber}`; }components/invoice_ninja/sources/common/base.mjs (1)
35-38: Handle missing webhook ID on deactivation.
If a hook ID was never set or was lost due to DB resets,webhookIdmay be undefined. Consider a safeguard to avoid failing unexpectedly.async deactivate() { const webhookId = this._getHookId(); + if (!webhookId) { + console.warn("No webhook ID found to deactivate"); + return; + } await this.app.deleteWebhook(webhookId); }components/invoice_ninja/sources/new-invoice-instant/test-event.mjs (2)
2-5: Clarify repeated ID fields.
IDs (id,user_id,assigned_user_id, etc.) share the same placeholder values. While suitable for a test fixture, consider varying them to simulate a more realistic scenario and better test coverage.
58-58: Potential discount conflict.
discount: "10.00"andpartial: "10.00"may conflict withpaid_to_date: "10.00". Confirm the logic behind these fields to avoid confusion in test scenarios.Do you want me to help refactor these values or open an issue tracking the correct test data scenarios?
components/invoice_ninja/actions/create-payment/create-payment.mjs (2)
4-137: Consider using numeric types foramountandrefunded.
Since you're ultimately parsing them as floats at lines 153 and 154, consider changing the prop definitions fromstringtonumberfor clarity and type-consistency.- amount: { - type: "string", + amount: { + type: "number", ... - refunded: { - type: "string", + refunded: { + type: "number", ...
138-165: Add error handling aroundparseObjectcalls.
You may want to detect invalid JSON inputs forinvoicesandcredits, to provide a clearer error if users supply malformed JSON.+ try { invoices: parseObject(this.invoices), credits: parseObject(this.credits), + } catch (err) { + throw new Error("Invalid JSON format in invoices or credits property."); + }components/invoice_ninja/actions/create-invoice/create-invoice.mjs (2)
4-142: Validate numeric fields in the props definition.
Similar to “Create Payment,” consider using numeric field types for props liketotalTaxes,amount,balance,paidToDate, anddiscountrather than strings. This avoids confusion and possible runtime parsing errors.
143-174: WrapparseObjectcalls to handle incorrect input formats.
By gracefully catching JSON parse errors, you can provide more actionable errors to users.components/invoice_ninja/actions/create-client/create-client.mjs (1)
206-246: Consider enhanced error handling onparseObject.
Similar to the other actions, validating user-provided JSON forcontactsandsettingscan prevent unexpected runtime failures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
components/invoice_ninja/actions/create-client/create-client.mjs(1 hunks)components/invoice_ninja/actions/create-invoice/create-invoice.mjs(1 hunks)components/invoice_ninja/actions/create-payment/create-payment.mjs(1 hunks)components/invoice_ninja/common/constants.mjs(1 hunks)components/invoice_ninja/common/utils.mjs(1 hunks)components/invoice_ninja/invoice_ninja.app.mjs(1 hunks)components/invoice_ninja/package.json(1 hunks)components/invoice_ninja/sources/common/base.mjs(1 hunks)components/invoice_ninja/sources/new-client-instant/new-client-instant.mjs(1 hunks)components/invoice_ninja/sources/new-client-instant/test-event.mjs(1 hunks)components/invoice_ninja/sources/new-invoice-instant/new-invoice-instant.mjs(1 hunks)components/invoice_ninja/sources/new-invoice-instant/test-event.mjs(1 hunks)components/invoice_ninja/sources/new-payment-instant/new-payment-instant.mjs(1 hunks)components/invoice_ninja/sources/new-payment-instant/test-event.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- components/invoice_ninja/common/constants.mjs
- components/invoice_ninja/package.json
- components/invoice_ninja/sources/new-payment-instant/test-event.mjs
🧰 Additional context used
🪛 Gitleaks (8.21.2)
components/invoice_ninja/sources/new-client-instant/test-event.mjs
20-20: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (12)
components/invoice_ninja/invoice_ninja.app.mjs (4)
1-2: Imports look good.
The import from@pipedream/platformis standard practice and appears correct.
6-74: Potential off-by-one in pagination logic.
Withinasync options({ page }) { ... page: page + 1 }, ensure downstream code expects 1-based page indexing. If the API is zero-based, this could skip or repeat pages inadvertently.
109-126: Client contact retrieval logic.
Here, you correctly retrieve client contacts using the newly introducedshowClientmethod, ensuring you fetch the right contact data. This approach is consistent with the rest of the file.
175-195: POST creation methods look consistent.
YourcreateNewInvoice,createNewClient, andrecordPaymentmethods align well with the overall request pattern. Centralizing logic in_makeRequestensures consistency.components/invoice_ninja/common/utils.mjs (1)
1-24: Robust object parsing.
YourparseObjectfunction handles array and string inputs gracefully, returning the original data on JSON parse errors. This approach is clean and avoids unhandled exceptions.components/invoice_ninja/sources/new-invoice-instant/new-invoice-instant.mjs (2)
1-3: Proactive import usage check.
Imports forcommonandsampleEmitlook fine for this new source, ensuring consistent base methods and test event data are integrated.
17-19: Handle potential missing invoice data.
Ifinvoiceis undefined or missingnumber, string concatenation may fail silently. Consider adding a quick null check or fallback.getSummary(invoice) { - return `New invoice created: ${invoice.number}`; + const invoiceNumber = invoice?.number || "N/A"; + return `New invoice created: ${invoiceNumber}`; }components/invoice_ninja/sources/new-invoice-instant/test-event.mjs (1)
59-60: Questionable test data for deleted invoice.
is_deleted: truemight lead to confusion for a “new invoice” sample. If the goal is to test a normal creation flow, consider setting this to false or removing it.- "is_deleted": true, + "is_deleted": false,✅ Verification successful
Confirmed:
is_deleted: trueis inappropriate for new invoice test dataThe test event fixture is specifically for testing new invoice creation events (
new-invoice-instant), so marking the invoice as deleted contradicts its intended purpose and could lead to confusion during testing.🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # First, let's look at the full content of the test file cat components/invoice_ninja/sources/new-invoice-instant/test-event.mjs # Let's also look for similar test files in the invoice_ninja component fd test-event.mjs components/invoice_ninjaLength of output: 2544
components/invoice_ninja/actions/create-payment/create-payment.mjs (1)
1-3: Imports look good and maintain DRY coding practices.
No issues found referencing the sharedparseObjectutility and theappinstance.components/invoice_ninja/actions/create-invoice/create-invoice.mjs (1)
1-3: Shared utilities usage is consistent.
The imports maintain a consistent pattern for interacting with common utilities and theappmodule.components/invoice_ninja/actions/create-client/create-client.mjs (2)
1-4: Constants and utility imports are well-organized.
No issues with the import structure.
5-205: Ensure all prop definitions align with server requirements.
Confirm whether all fields labeled asstring(e.g., addresses, phone numbers) meet the Invoice Ninja API constraints. For example, if some numeric fields are typed as strings, parse or define them accordingly.
| settings: { | ||
| type: "object", | ||
| label: "Settings", | ||
| description: "An array of settings objects in JSON format. **Example: {\"currency_id\": 1, \"timezone_id\": 5, \"date_format_id\": 1, \"language_id\": 1}** [See the documentation](https://api-docs.invoicing.co/#tag/clients/POST/api/v1/clients) for further details", |
There was a problem hiding this comment.
I think settings takes an object instead of an array.
| description: "An array of settings objects in JSON format. **Example: {\"currency_id\": 1, \"timezone_id\": 5, \"date_format_id\": 1, \"language_id\": 1}** [See the documentation](https://api-docs.invoicing.co/#tag/clients/POST/api/v1/clients) for further details", | |
| description: "A settings object in JSON format. **Example: {\"currency_id\": 1, \"timezone_id\": 5, \"date_format_id\": 1, \"language_id\": 1}** [See the documentation](https://api-docs.invoicing.co/#tag/clients/POST/api/v1/clients) for further details", |
Resolves #15154.
Summary by CodeRabbit
New Features
Improvements
Documentation