-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Enhance booking_experts component with new actions and properties #19060
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhance booking_experts component with new actions and properties #19060
Conversation
- Added new actions: `getBooking`, `deleteGuest`, `updateGuest`, `listAvailabilities`, and `listRentableTypeAvailabilities`. - Introduced new properties for administration and booking IDs, guest details, and rentable type availability. - Updated existing actions to improve functionality and consistency. - Incremented package version to 0.2.0 and updated action versions accordingly.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds multiple Booking Experts actions (get-booking, list-availabilities, list-rentabletype-availabilities, update-guest, delete-guest), extends the booking_experts app with new propDefinitions and API methods (getBooking, listAvailabilities, listRentableTypes, listGuests, updateGuest, deleteGuest, listAdministrationChannels), renames a prop in list-bookings, and bumps several versions including the package. Changes
Sequence Diagram(s)sequenceDiagram
participant Action as Action (user)
participant App as booking_experts.app
participant API as BookingExperts API
rect rgb(235,243,255)
note right of Action: Update guest
Action->>App: updateGuest(administrationId, reservationId, guestId, payload, $)
App->>API: PATCH /administrations/{administrationId}/reservations/{reservationId}/guests/{guestId}
API-->>App: 200 OK (guest)
App-->>Action: data & $summary
end
rect rgb(255,245,235)
note right of Action: Delete guest
Action->>App: deleteGuest(administrationId, reservationId, guestId, $)
App->>API: DELETE /administrations/{administrationId}/reservations/{reservationId}/guests/{guestId}
API-->>App: 204 / confirmation
App-->>Action: confirmation & $summary
end
rect rgb(235,255,240)
note right of Action: Booking & availabilities
Action->>App: getBooking(administrationId, bookingId, $)
App->>API: GET /administrations/{administrationId}/bookings/{bookingId}
API-->>App: 200 OK (booking)
App-->>Action: booking data & $summary
Action->>App: listRentableTypeAvailabilities(channelId, rentableTypeId, date_range?, $)
App->>API: GET /channels/{channelId}/rentable_types/{rentableTypeId}/availabilities?date_range=start..end
API-->>App: 200 OK (availabilities[])
App-->>Action: availabilities & $summary
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (7)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-01-29T22:59:38.825ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (10)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/booking_experts/actions/list-bookings/list-bookings.mjs (1)
32-41: Clarify the property name.The property name
listAdministrationChannelsis confusing because:
- It sounds like a method that lists channels rather than a channel filter parameter
- The plural form suggests multiple values, but line 72 uses it as a single filter value
- It's inconsistent with other singular-named props in this action (administrationId, ownerId, reservationId)
Consider renaming to
channelIdoradministrationChannelIdfor clarity and consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
components/booking_experts/actions/add-guest-to-reservation/add-guest-to-reservation.mjs(1 hunks)components/booking_experts/actions/delete-guest/delete-guest.mjs(1 hunks)components/booking_experts/actions/get-booking/get-booking.mjs(1 hunks)components/booking_experts/actions/list-availabilities/list-availabilities.mjs(1 hunks)components/booking_experts/actions/list-bookings/list-bookings.mjs(3 hunks)components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs(1 hunks)components/booking_experts/actions/update-guest/update-guest.mjs(1 hunks)components/booking_experts/booking_experts.app.mjs(5 hunks)components/booking_experts/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
Repo: PipedreamHQ/pipedream PR: 14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
Applied to files:
components/booking_experts/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (19)
components/booking_experts/package.json (1)
3-3: LGTM!The version bump from 0.1.0 to 0.2.0 is appropriate for the addition of new actions and API methods.
components/booking_experts/actions/add-guest-to-reservation/add-guest-to-reservation.mjs (1)
6-7: LGTM!The typo fix (removing the extra period) and version bump are appropriate.
components/booking_experts/actions/list-bookings/list-bookings.mjs (1)
72-72: Verify the filter parameter usage.Ensure that the
filter[channel]parameter accepts the value returned by thelistAdministrationChannelsprop definition. If it expects a single channel ID but the prop returns an array or different format, this will cause runtime errors.components/booking_experts/actions/delete-guest/delete-guest.mjs (1)
26-28: LGTM!The arrow function syntax for passing
administrationIdto the prop definition is clean and correct.components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs (1)
1-2: LGTM!Good use of ConfigurationError for validation.
components/booking_experts/actions/get-booking/get-booking.mjs (1)
22-30: LGTM!The prop definition correctly passes
administrationIdto scope the booking selection.components/booking_experts/actions/update-guest/update-guest.mjs (1)
97-112: The review comment is based on an incorrect assumption about how Pipedream's axios handles undefined values.The
booking_experts.app.mjsimports axios from@pipedream/platform(line 1), and theupdateGuestmethod uses Pipedream's axios implementation. As confirmed in the learnings, Pipedream's platform axios automatically excludes undefined values from HTTP requests, so the current code already handles partial updates correctly without manual filtering.The API requirement to send only changed fields (no undefined values) is already satisfied by Pipedream's axios behavior. No changes are needed.
Likely an incorrect or invalid review comment.
components/booking_experts/booking_experts.app.mjs (12)
47-68: LGTM!The
administrationChannelIdpropDefinition is well-structured and follows the established pattern for dependent dropdowns in the codebase.
69-89: LGTM!The
bookingIdpropDefinition correctly fetches bookings and displays the booking number as the label, which provides good UX.
111-131: LGTM!The
rentableTypeIdpropDefinition correctly depends onchannelIdand follows the established pattern for cascading dropdowns.
288-295: LGTM!The
getBookingmethod correctly implements the GET booking endpoint as specified in the PR objectives.
312-319: LGTM!The
listAdministrationChannelsmethod is correctly implemented and properly scoped to a specific administration.
320-325: LGTM!The
listAvailabilitiesmethod correctly implements the global availabilities endpoint as specified in the PR objectives.
326-331: LGTM!The
listChannelsmethod correctly implements the global channels endpoint.
332-339: LGTM!The
listGuestsmethod correctly implements the endpoint for fetching guests associated with a reservation.
340-347: LGTM!The
listRentableTypesmethod correctly implements the endpoint for fetching rentable types for a specific channel.
348-355: LGTM!The
listRentableTypeAvailabilitiesmethod correctly implements the rentable type availabilities endpoint as specified in the PR objectives.
412-420: LGTM!The
updateGuestmethod correctly implements the PATCH guests endpoint as specified in the PR objectives.
421-429: LGTM!The
deleteGuestmethod correctly implements the DELETE guest endpoint as specified in the PR objectives.
components/booking_experts/actions/list-availabilities/list-availabilities.mjs
Show resolved
Hide resolved
...ooking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs
Show resolved
Hide resolved
…0.0.2 respectively - Incremented version for actions: `create-agenda-period`, `get-complex-prices`, and `list-inventory-objects` to 0.0.3. - Updated version for sources: `booking-updated`, `inventory-object-updated`, `new-booking-created`, and `new-inventory-object-created` to 0.0.2.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/booking_experts/actions/delete-guest/delete-guest.mjs(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
components/booking_experts/actions/delete-guest/delete-guest.mjs
[error] 13-13: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 15-15: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 17-17: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 22-22: Expected an expression but instead found '}'.
Expected an expression here.
(parse)
[error] 22-23: Expected a statement but instead found ',
reservationId:'.
Expected a statement here.
(parse)
[error] 31-31: Expected an expression but instead found '}'.
Expected an expression here.
(parse)
[error] 31-32: Expected a statement but instead found ',
guestId:'.
Expected a statement here.
(parse)
[error] 43-43: Expected an expression but instead found '}'.
Expected an expression here.
(parse)
[error] 43-43: Expected a statement but instead found ','.
Expected a statement here.
(parse)
[error] 44-45: Expected a statement but instead found ',
async run('.
Expected a statement here.
(parse)
[error] 45-45: Expected a statement but instead found ')'.
Expected a statement here.
(parse)
[error] 53-53: Illegal return statement outside of a function
(parse)
[error] 54-55: Expected a statement but instead found ',
}'.
Expected a statement here.
(parse)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
components/booking_experts/actions/delete-guest/delete-guest.mjs
Outdated
Show resolved
Hide resolved
- Simplified options method in booking_experts.app.mjs by removing unused channelId parameter. - Updated description in get-booking action to point to the correct documentation link. - Enhanced list-rentabletype-availabilities action to conditionally include date_range parameter based on start and end dates. - Removed unnecessary closing brace in delete-guest action for cleaner code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
components/booking_experts/actions/delete-guest/delete-guest.mjs(1 hunks)components/booking_experts/actions/get-booking/get-booking.mjs(1 hunks)components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs(1 hunks)components/booking_experts/booking_experts.app.mjs(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (6)
components/booking_experts/actions/delete-guest/delete-guest.mjs (1)
1-54: LGTM! Past issues resolved.The previous concerns about
destructiveHintand syntax errors have been addressed. The action is properly configured withdestructiveHint: true(appropriate for a delete operation), and the code structure is correct.components/booking_experts/booking_experts.app.mjs (3)
90-107: LGTM! Circular dependency resolved.The previous circular logic issue has been fixed. The
channelIdpropDefinition now correctly uses onlypageas a parameter and calls the globallistChannelsendpoint without any circular reference.
150-175: LGTM! Label formatting corrected.The previous double-space issue has been resolved. The label now correctly formats as
"First Last (email)"or"First Last"without extra spaces.
47-68: LGTM! New propDefinitions and methods are well-implemented.The new additions follow consistent patterns:
- PropDefinitions properly handle dependencies and pagination
- API methods correctly construct paths and use appropriate HTTP verbs
- All endpoints align with the Booking Experts API structure
Also applies to: 69-89, 108-128, 285-292, 317-352, 409-426
components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs (1)
64-67: LGTM! Date range parameter handling corrected.The previous issue with undefined date_range has been resolved. The parameter is now only included when both dates are provided using a conditional spread operator.
components/booking_experts/actions/get-booking/get-booking.mjs (1)
1-41: LGTM! Documentation link corrected.The previous issue with the documentation URL has been resolved. The link now correctly points to
administration-bookings-showfor retrieving a single booking. The action is properly implemented with correct prop wiring and API call.
...ooking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs
Outdated
Show resolved
Hide resolved
- Removed the administrationId prop definition and its associated logic from the list-rentabletype-availabilities action for improved clarity and simplicity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs (1)
33-49: Consider adding date format validation.While the descriptions specify
YYYY-MM-DDformat for both dates, there's no validation to ensure users provide the correct format. Adding a regex check (e.g.,/^\d{4}-\d{2}-\d{2}$/) would improve user experience by catching format errors early rather than letting the API reject invalid dates.Example validation:
async run({ $ }) { + const dateRegex = /^\d{4}-\d{2}-\d{2}$/; + if (this.startDate && !dateRegex.test(this.startDate)) { + throw new ConfigurationError("Start date must be in YYYY-MM-DD format."); + } + if (this.endDate && !dateRegex.test(this.endDate)) { + throw new ConfigurationError("End date must be in YYYY-MM-DD format."); + } if ((!this.startDate && this.endDate) || (this.startDate && !this.endDate)) { throw new ConfigurationError("You should provide both the start and end date parameters."); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (2)
components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs (2)
47-60: Previous issue resolved: date_range now conditionally included.The previous concern about sending
date_range: "undefined..undefined"to the API has been successfully addressed. The conditional spreading pattern...(this.startDate && this.endDate && { "date_range": ... })ensures the parameter is only included when both dates are provided.
1-64: Overall implementation looks solid.The action correctly implements the rentable type availabilities endpoint with proper prop dependencies (rentableTypeId depends on channelId), clear validation logic, and appropriate error handling. The structure aligns well with other actions in the booking_experts component.
...ooking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs
Show resolved
Hide resolved
jcortes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @luancazarine lgtm! Ready for QA!
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
- Added warning alerts to listAvailabilities, deleteGuest, updateGuest, and listRentableTypeAvailabilities actions to inform users about API limitations. - Updated label formatting in booking_experts.app.mjs to include booking number for better identification. - Refactored administrationChannelId prop in list-bookings action for clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/booking_experts/actions/list-bookings/list-bookings.mjs (1)
32-41:filter[channel]is wired to the wrong valueIn the request params you’re sending:
"filter[channel]": this.listAdministrationChannels,
listAdministrationChannelsis a method on the app, not the selected channel value. This means the channel filter will be incorrect (or effectively omitted).You likely want to pass the selected
administrationChannelIdinstead:- "filter[channel]": this.listAdministrationChannels, + "filter[channel]": this.administrationChannelId,This will correctly filter bookings by the chosen administration channel.
Also applies to: 67-78
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
components/booking_experts/actions/delete-guest/delete-guest.mjs(1 hunks)components/booking_experts/actions/list-availabilities/list-availabilities.mjs(1 hunks)components/booking_experts/actions/list-bookings/list-bookings.mjs(3 hunks)components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs(1 hunks)components/booking_experts/actions/update-guest/update-guest.mjs(1 hunks)components/booking_experts/booking_experts.app.mjs(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs (1)
1-68: Validation and query construction look solidThe date-range validation and conditional
"date_range"param construction are correct, and the wiring tochannelId/rentableTypeIdmatches the app methods.readOnlyHint: trueis also appropriate here.No changes needed from my side.
components/booking_experts/actions/list-availabilities/list-availabilities.mjs
Show resolved
Hide resolved
…lity - Removed unnecessary console log from booking_experts.app.mjs for cleaner code. - Added missing `$` parameter to addGuestToReservation, deleteGuest, listInventoryObjects, searchContacts, and updateGuest actions for better integration. - Updated readOnlyHint in listAvailabilities action to true for improved user guidance.
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
|
/approve |
Resolves #19046
Summary by CodeRabbit
New Features
Bug Fixes
Chores