New Components - limoexpress #16933
New Components - limoexpress #16933
Conversation
luancazarine
added
the
ai-assisted
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis update introduces a new LimoExpress integration, including actions for creating bookings, creating clients, and marking invoices as paid. It adds a polling source for new bookings, utility and constants modules, and implements the core app logic with API methods, dynamic prop options, and pagination. The package metadata is updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action
participant App
participant LimoExpressAPI
User->>Action: Provide booking/client/invoice data
Action->>App: Call API method with data
App->>LimoExpressAPI: Send HTTP request (createBooking/createClient/markInvoiceAsPaid)
LimoExpressAPI-->>App: Return API response
App-->>Action: Return response or throw error
Action-->>User: Emit summary and response
sequenceDiagram
participant Source
participant App
participant LimoExpressAPI
participant DB
Source->>DB: Get last processed booking number
loop Polling interval
Source->>App: listBookings (descending by ID)
App->>LimoExpressAPI: GET /bookings
LimoExpressAPI-->>App: Return bookings
App-->>Source: Return bookings
Source->>DB: Update last booking number
Source-->>User: Emit new booking events
end
Assessment against linked issues
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/limoexpress/actions/create-booking/create-booking.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/limoexpress/limoexpress.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 ✨ 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 (
|
Sources - New Booking Actions - Create Booking - Create Client - Mark Invoice As Paid
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
components/limoexpress/actions/create-booking/create-booking.mjs (1)
104-135: Consider using number type for numeric string properties.Several properties like price, commissionAmount, and vatPercentage are defined as strings but require parseFloat conversion. Consider using number type directly for better type safety.
price: { - type: "string", + type: "number", label: "Price", description: "The price of the booking.", optional: true, }, commissionAmount: { - type: "string", + type: "number", label: "Commission Amount", description: "The commission amount for the booking.", optional: true, }, vatPercentage: { - type: "string", + type: "number", label: "VAT Percentage", description: "The VAT percentage for the booking.", optional: true, },This would eliminate the need for parseFloat conversions in the run method.
components/limoexpress/limoexpress.app.mjs (1)
142-147: Consider using a more specific Accept header.The Accept header "/" is too generic. Consider specifying the expected content type for clearer API communication.
Apply this diff to use a more specific Accept header:
- "Accept": "*/*", + "Accept": "application/json",
📜 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 (9)
components/limoexpress/actions/create-booking/create-booking.mjs(1 hunks)components/limoexpress/actions/create-client/create-client.mjs(1 hunks)components/limoexpress/actions/mark-invoice-paid/mark-invoice-paid.mjs(1 hunks)components/limoexpress/common/constants.mjs(1 hunks)components/limoexpress/common/utils.mjs(1 hunks)components/limoexpress/limoexpress.app.mjs(1 hunks)components/limoexpress/package.json(2 hunks)components/limoexpress/sources/new-booking/new-booking.mjs(1 hunks)components/limoexpress/sources/new-booking/test-event.mjs(1 hunks)
🔇 Additional comments (9)
components/limoexpress/package.json (2)
3-3: LGTM! Appropriate version bump for new features.The version increment from 0.0.1 to 0.1.0 correctly follows semantic versioning conventions for adding new functionality.
15-17: LGTM! Required dependency addition.The @pipedream/platform dependency is correctly added and needed for the new LimoExpress integration components.
components/limoexpress/common/constants.mjs (1)
1-15: LGTM! Well-structured constants for centralized configuration.The constants are well-organized and follow good practices:
- Clear, descriptive names
- Consistent structure for TYPE_OPTIONS with label/value pairs
- Centralized configuration that can be reused across components
components/limoexpress/actions/mark-invoice-paid/mark-invoice-paid.mjs (1)
3-29: LGTM! Well-implemented Pipedream action following best practices.The action implementation is excellent:
- Follows standard Pipedream action structure
- Uses propDefinition for reusable invoice ID selection
- Proper error handling delegation to the app method
- Clear summary export with meaningful message
- Correct API documentation reference
components/limoexpress/common/utils.mjs (2)
3-6: LGTM! Proper error handling with ConfigurationError.The error handling function correctly extracts error messages and uses optional chaining for safe property access.
8-12:❌ Incorrect review comment
Review the logic in checkNumbers function - it may be inverted.
According to the AI summary, this function should "determine if a booking number is newer than the last processed one to avoid duplicate event emissions." However, the current logic appears to return
truefor older or equal numbers, not newer ones.The function returns
truewhen:
numbers[1] < lastNumber[1](clearly older), ORnumbers[1] >= lastNumber[1] AND numbers[0] <= lastNumber[0](equal or older in the first part)This seems inverted for detecting "newer" bookings.
Consider renaming the function to reflect its actual behavior or inverting the logic if it should detect newer bookings:
-export const checkNumbers = (numbers, lastNumber) => { - return (parseInt(numbers[1]) < parseInt(lastNumber[1])) || - ((parseInt(numbers[1]) >= parseInt(lastNumber[1])) && - (parseInt(numbers[0]) <= parseInt(lastNumber[0]))); -}; +export const isBookingNewer = (numbers, lastNumber) => { + return (parseInt(numbers[1]) > parseInt(lastNumber[1])) || + ((parseInt(numbers[1]) === parseInt(lastNumber[1])) && + (parseInt(numbers[0]) > parseInt(lastNumber[0]))); +};Please verify the intended behavior by checking how this function is used in the polling source.
🏁 Script executed:
#!/bin/bash # Description: Find usage of checkNumbers function to understand the intended logic # Expected: Usage in polling source that shows whether it should detect newer or older bookings rg -A 10 -B 5 "checkNumbers"Length of output: 3475
The
checkNumbersfunction correctly flags older or equal bookings for breaking the loopThe usage in
new-booking.mjsmakes it clear thatcheckNumbersis meant to returntruewhen the current booking number is older than or the same as the last processed one, so that the loop stops as soon as it reaches already‐seen items:for await (const item of response) { const numbers = item.number.split("/"); // break out once we hit bookings ≤ lastNumber if (checkNumbers(numbers, lastNumber)) break; responseArray.push(item); }No inversion of the logic is needed. If you’d like clearer naming, you could rename it to something like:
isBookingOlderOrEqual(numbers, lastNumber)shouldStopIteration(numbers, lastNumber)Likely an incorrect or invalid review comment.
components/limoexpress/actions/create-client/create-client.mjs (1)
1-68: Well-structured action implementation.The action follows Pipedream's standard patterns and best practices:
- Proper imports and error handling
- Clear prop definitions with appropriate types and descriptions
- Good use of the throwError utility for consistent error handling
- Summary export for user feedback
components/limoexpress/sources/new-booking/test-event.mjs (1)
1-74: Comprehensive test event data.The test event provides a realistic and comprehensive sample of booking data that includes all necessary fields for testing the integration. The data structure appears consistent with the API documentation.
components/limoexpress/limoexpress.app.mjs (1)
211-224:❓ Verification inconclusive
Verify the use of PUT method for creating resources.
Both
createBookingandcreateClientuse PUT method instead of the conventional POST for creating new resources. While this might be intentional based on the API design, it's worth confirming this is correct.Run the following script to check if there are any API documentation references or similar patterns in the codebase:
🏁 Script executed:
#!/bin/bash # Description: Search for LimoExpress API documentation or similar PUT usage patterns # Search for any API documentation references rg -i "limoexpress.*api" --type md -A 5 # Check if other create methods in the codebase use PUT ast-grep --pattern 'create$_($$$) { return this._makeRequest({ method: "PUT", $$$ }); }'Length of output: 144
Confirm HTTP method for createBooking and createClient
I wasn’t able to find any LimoExpress API docs or othercreate…methods using PUT in the repository. Please verify with the official API specification whether these endpoints should indeed use PUT (or if POST is the correct verb).• File: components/limoexpress/limoexpress.app.mjs
Lines 211–224createBooking(opts = {}) { return this._makeRequest({ method: "PUT", path: "/bookings", ...opts, }); }, createClient(opts = {}) { return this._makeRequest({ method: "PUT", path: "/clients", ...opts, }); },• Action: Confirm with API docs/spec that PUT is intentional for these “create” operations, and update to POST if needed.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
components/limoexpress/limoexpress.app.mjs (1)
266-266: Fix the loose equality operator.The loose equality operator can cause unexpected type coercion issues. This was flagged in a previous review but hasn't been addressed yet.
Apply this diff:
- hasMore = !(current_page == last_page); + hasMore = !(current_page === last_page);
🧹 Nitpick comments (2)
components/limoexpress/limoexpress.app.mjs (2)
217-222: Consider documenting the unusual HTTP method choice.Using
PUTfor creating bookings is unconventional - typicallyPOSTis used for creation andPUTfor updates. While this may be required by the LimoExpress API, consider adding a comment to clarify this design choice.Add a comment to clarify:
createBooking(opts = {}) { + // Note: LimoExpress API uses PUT for booking creation return this._makeRequest({ method: "PUT", path: "/bookings", ...opts, }); },
223-229: Consider documenting the unusual HTTP method choice.Similar to
createBooking, usingPUTfor client creation is unconventional. Adding documentation would help future maintainers understand this API design.Add a comment to clarify:
createClient(opts = {}) { + // Note: LimoExpress API uses PUT for client creation return this._makeRequest({ method: "PUT", path: "/clients", ...opts, }); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/limoexpress/actions/create-booking/create-booking.mjs(1 hunks)components/limoexpress/limoexpress.app.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/limoexpress/actions/create-booking/create-booking.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (3)
components/limoexpress/limoexpress.app.mjs (3)
98-98: LGTM: Good filtering of hidden payment methods.The filtering logic to exclude hidden payment methods (
filter(({ hidden }) => !hidden)) is well-implemented and ensures only available options are presented to users.
247-248: Good pagination configuration.The pagination setup with
per_page: 100and incrementing page numbers follows best practices for efficient data retrieval.
57-65:❓ Verification inconclusive
Verify the search parameter for vehicle filtering.
The
vehicleIdoptions function usessearch_string: vehicleClassIdto filter vehicles by class. Ensure this parameter name is correct according to the LimoExpress API documentation, as using an ID value for a search string parameter seems unusual.
🏁 Script executed:
#!/bin/bash # Description: Check if there are any action files that use vehicleId to verify the parameter usage # Expected: Find usage patterns that might indicate the correct parameter name rg -A 5 -B 5 "vehicleId|search_string" --type jsLength of output: 50
🏁 Script executed:
#!/bin/bash # Search for any occurrence of `search_string` in .js and .mjs files rg -n "search_string" -g "*.{js,mjs}" # Locate the definition (or import) of `listVehicles` to inspect its expected parameters rg -n "listVehicles" -g "*.{js,mjs}"Length of output: 708
🏁 Script executed:
#!/bin/bash # Display the listVehicles method implementation in LimoExpress to verify supported parameters sed -n '170,240p' components/limoexpress/limoexpress.app.mjsLength of output: 1605
Verify vehicle class filter query parameter
The
optionsmethod in components/limoexpress/limoexpress.app.mjs (lines 57–65) currently calls:await this.listVehicles({ params: { page, search_string: vehicleClassId, }, });Please confirm in the LimoExpress API documentation whether the Vehicles endpoint supports filtering by class via a generic
search_stringparameter, or if it requires a dedicated parameter (e.g.,vehicle_class_id) instead.
|
/approve |
Resolves #16907.
Summary by CodeRabbit