New Components - dolibarr #17030
New Components - dolibarr #17030
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
|
""" WalkthroughThis update introduces a comprehensive Dolibarr integration, adding new actions for creating users, third parties, and orders, as well as sources for emitting events on new invoices, orders, and third parties. It includes utility functions, a robust API client, and base classes for event polling, along with sample event data for testing. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action
participant DolibarrApp
participant DolibarrAPI
User->>Action: Trigger (e.g., Create Order)
Action->>DolibarrApp: Call createOrder() with props
DolibarrApp->>DolibarrAPI: POST /orders (with data)
DolibarrAPI-->>DolibarrApp: API response
DolibarrApp-->>Action: Return response
Action-->>User: Output result
sequenceDiagram
participant Source
participant DolibarrApp
participant DolibarrAPI
participant EventStream
loop Polling Interval
Source->>DolibarrApp: list<Resource>() (e.g., listOrders)
DolibarrApp->>DolibarrAPI: GET /<resource>
DolibarrAPI-->>DolibarrApp: Resource list
DolibarrApp-->>Source: Return items
Source->>EventStream: Emit new events for new items
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/dolibarr/dolibarr.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 9
🧹 Nitpick comments (8)
components/dolibarr/common/utils.mjs (1)
1-27: Consider adding safeguards against circular references and improving error handling.The
parseObjectfunction is well-structured but could benefit from additional robustness:
- Circular reference protection: The recursive nature could cause infinite loops with circular object references
- Input validation: Consider adding type checking for edge cases
- More descriptive naming:
parseObjectcould be more specific, likeparseAndNormalizeInputorrecursiveJsonParseConsider this enhanced version with circular reference protection:
-export function parseObject(obj) { +export function parseObject(obj, seen = new WeakSet()) { if (!obj) { return {}; } if (typeof obj === "string") { try { return JSON.parse(obj); } catch (e) { return obj; } } if (Array.isArray(obj)) { - return obj.map(parseObject); + return obj.map(item => parseObject(item, seen)); } if (typeof obj === "object") { + if (seen.has(obj)) { + return obj; // Return original object to prevent infinite recursion + } + seen.add(obj); return Object.fromEntries( Object.entries(obj).map(([ key, value, ]) => [ key, - parseObject(value), + parseObject(value, seen), ]), ); } return obj; }components/dolibarr/sources/new-order-created/test-event.mjs (1)
1-231: Well-structured test data with comprehensive coverage.The test event object provides excellent coverage of the Dolibarr order data structure. The nested line items and comprehensive field mapping will be valuable for testing and development.
Consider these minor enhancements for maintainability:
- Dynamic timestamps: Some hardcoded timestamps (lines 177, 179, 218-220) might benefit from being relative to
Date.now()for long-term test reliability- Documentation: Adding JSDoc comments to explain key sections could help developers understand the data structure
components/dolibarr/actions/create-order/create-order.mjs (2)
26-26: Fix typo in description.There's a typo in the description: "expecteddelivery" should be "expected delivery".
- description: "The expecteddelivery date of the order in YYYY-MM-DD format", + description: "The expected delivery date of the order in YYYY-MM-DD format",
76-76: Improve summary message format.The summary message interpolates the entire response object, which may not display meaningful information. Consider using a specific field like order ID.
- $.export("$summary", `Successfully created order ${response}`); + $.export("$summary", `Successfully created order with ID: ${response.id || 'N/A'}`);components/dolibarr/actions/create-user/create-user.mjs (1)
102-102: Improve summary message format.Similar to the create-order action, the summary message interpolates the entire response object. Consider using a specific field like user ID or login.
- $.export("$summary", `Successfully created user ${response}`); + $.export("$summary", `Successfully created user: ${response.login || response.id || 'N/A'}`);components/dolibarr/dolibarr.app.mjs (3)
25-29: Fix label inconsistency for paymentTermCode.The label says "Payment Term ID" but the description and value field use "code". Consider updating for consistency:
- label: "Payment Term ID", + label: "Payment Term Code",
72-82: Minor: Remove unnecessary space in axios function call.There's an extra space between
axiosand the opening parenthesis.- return axios ($, { + return axios($, {
68-139: Consider adding JSDoc comments for better API documentation.Adding JSDoc comments to document method parameters and return types would improve maintainability and developer experience.
Example for one method:
+ /** + * List all third parties + * @param {Object} opts - Request options + * @param {Object} opts.params - Query parameters + * @param {number} opts.params.limit - Number of results per page + * @param {number} opts.params.page - Page number + * @returns {Promise<Array>} Array of third party objects + */ listThirdParties(opts = {}) {
📜 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 (13)
components/dolibarr/actions/create-order/create-order.mjs(1 hunks)components/dolibarr/actions/create-thirdparty/create-thirdparty.mjs(1 hunks)components/dolibarr/actions/create-user/create-user.mjs(1 hunks)components/dolibarr/common/utils.mjs(1 hunks)components/dolibarr/dolibarr.app.mjs(1 hunks)components/dolibarr/package.json(2 hunks)components/dolibarr/sources/common/base.mjs(1 hunks)components/dolibarr/sources/new-invoice-created/new-invoice-created.mjs(1 hunks)components/dolibarr/sources/new-invoice-created/test-event.mjs(1 hunks)components/dolibarr/sources/new-order-created/new-order-created.mjs(1 hunks)components/dolibarr/sources/new-order-created/test-event.mjs(1 hunks)components/dolibarr/sources/new-thirdparty-created/new-thirdparty-created.mjs(1 hunks)components/dolibarr/sources/new-thirdparty-created/test-event.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
🔇 Additional comments (13)
components/dolibarr/package.json (1)
3-3: Appropriate version bump for new feature additions.The minor version increment from
0.0.1to0.1.0correctly reflects the addition of new functionality described in the PR.components/dolibarr/sources/new-thirdparty-created/new-thirdparty-created.mjs (2)
1-22: Well-structured source module following Pipedream conventions.The module correctly extends the common base and implements the required methods. The structure follows Pipedream best practices with proper metadata and deduplication strategy.
Key strengths:
- Clear separation of concerns with base class inheritance
- Descriptive method implementations
- Proper use of template literals in
getSummary- Standard Pipedream source configuration
14-16:✅ Verification successful
Verify the base class provides the listThirdParties method.
Ensure that
this.dolibarr.listThirdPartiesis properly implemented in the Dolibarr app client to avoid runtime errors.
🏁 Script executed:
#!/bin/bash # Description: Verify that listThirdParties method exists in the Dolibarr app client # Expected: Method definition should be found in the app client ast-grep --pattern $'listThirdParties($$$) { $$$ }' # Also search for method references rg -A 3 "listThirdParties"Length of output: 1062
Confirmed:
listThirdPartiesExists in Dolibarr ClientThe
listThirdPartiesmethod is implemented in the Dolibarr app client:• File:
components/dolibarr/dolibarr.app.mjslistThirdParties(opts = {}) { return this._makeRequest({ path: "/thirdparties", ...opts, }); }No changes are required.
components/dolibarr/sources/new-thirdparty-created/test-event.mjs (1)
1-161: LGTM - Comprehensive test event data structure.The test event object provides a thorough representation of a third party entity with all the necessary fields for testing the new-thirdparty-created source. The data structure is well-organized and includes appropriate default values.
components/dolibarr/sources/new-invoice-created/test-event.mjs (1)
1-134: LGTM - Well-structured invoice test event data.The test event object comprehensively covers all invoice-related fields with appropriate default values for testing the new-invoice-created source functionality.
components/dolibarr/sources/new-order-created/new-order-created.mjs (1)
1-23: LGTM - Well-implemented source component following established patterns.The source component properly extends the common base module and implements the required methods. The structure is clean and follows Pipedream source component best practices.
components/dolibarr/sources/new-invoice-created/new-invoice-created.mjs (1)
1-22: LGTM! Clean source implementation.The source component correctly extends the base class and implements the required abstract methods. The implementation follows Pipedream patterns and integrates well with the common base.
components/dolibarr/actions/create-user/create-user.mjs (1)
93-95: LGTM! Proper boolean to integer conversion.The admin boolean conversion to 1/0 is correctly implemented for the Dolibarr API requirements.
components/dolibarr/sources/common/base.mjs (4)
54-58: Verify timestamp comparison logic.The logic breaks when
item.date_creation <= lastTs, but the condition checksitem.date_creation > lastTs. This assumes items are sorted in descending order by creation date, which should be correct based on thegetArgs()method. The implementation looks correct.
65-66: LGTM! Proper chronological event emission.The code correctly updates the timestamp to the most recent item's creation date and reverses the array to emit events in chronological order (oldest first), which is the expected behavior for Pipedream sources.
68-73: Good use of abstract method pattern.The abstract methods properly throw
ConfigurationErrorwhen not implemented, providing clear guidance to developers extending this base class.
47-51:✅ Verification successful
Verify paginate method exists in dolibarr app.
The code calls
this.dolibarr.paginate()which should be implemented in the dolibarr app component. Ensure this method exists and handles the pagination correctly.
🏁 Script executed:
#!/bin/bash # Description: Verify that the dolibarr app component has a paginate method # Expected: Find the paginate method implementation in the dolibarr app # Search for paginate method in dolibarr app files fd -e mjs -e js dolibarr.app | xargs rg -A 10 "paginate"Length of output: 271
Verified paginate method implementation
Thedolibarr.appcomponent defines an async generatorpaginate({ fn, params = {}, max })that initializes pagination parameters and iterates through pages until the limit is reached. This satisfies the call incomponents/dolibarr/sources/common/base.mjs, so no further action is needed.components/dolibarr/dolibarr.app.mjs (1)
1-3: LGTM!Good use of a constant for the default pagination limit.
components/dolibarr/actions/create-thirdparty/create-thirdparty.mjs
Outdated
Show resolved
Hide resolved
components/dolibarr/actions/create-thirdparty/create-thirdparty.mjs
Outdated
Show resolved
Hide resolved
components/dolibarr/actions/create-thirdparty/create-thirdparty.mjs
Outdated
Show resolved
Hide resolved
components/dolibarr/actions/create-thirdparty/create-thirdparty.mjs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
components/dolibarr/dolibarr.app.mjs (1)
140-164:⚠️ Potential issueCritical: The pagination logic still has the infinite loop issue flagged in previous review.
The pagination method still contains the same critical flaw identified in the previous review - the page number is never incremented, which will cause an infinite loop when results.length equals params.limit.
The fix from the previous review still needs to be applied:
async *paginate({ fn, params = {}, max, }) { params = { ...params, limit: 100, page: 0, }; - let total, count = 0; + let count = 0; do { const results = await fn({ params, }); if (!results?.length) { return; } for (const item of results) { yield item; if (max && ++count >= max) { return; } } - total = results?.length; - } while (total === params.limit); + params.page++; + } while (results.length === params.limit);
🧹 Nitpick comments (1)
components/dolibarr/dolibarr.app.mjs (1)
69-82: Minor formatting issue in _makeRequest method.The implementation is correct, but there's unusual spacing in the axios call.
- return axios ($, { + return axios($, {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/dolibarr/actions/create-thirdparty/create-thirdparty.mjs(1 hunks)components/dolibarr/actions/create-user/create-user.mjs(1 hunks)components/dolibarr/dolibarr.app.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- components/dolibarr/actions/create-user/create-user.mjs
- components/dolibarr/actions/create-thirdparty/create-thirdparty.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
🔇 Additional comments (3)
components/dolibarr/dolibarr.app.mjs (3)
1-2: LGTM: Clean imports and constants.The axios import and default limit constant are appropriately defined.
7-67: LGTM: Well-structured property definitions with proper async options.The propDefinitions are well-implemented with async option loaders that properly utilize pagination. The structure follows Pipedream conventions and provides good user experience with label-value mappings.
83-139: LGTM: Consistent and well-structured API methods.All list and create methods follow a consistent pattern and are properly implemented. The method signatures and endpoint paths appear correct for the Dolibarr API.
Resolves #16920
Summary by CodeRabbit