-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Be amenities and rentable types #19387
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
base: master
Are you sure you want to change the base?
Be amenities and rentable types #19387
Conversation
- Add listAmenities() method to fetch all amenities - Add getAmenity() method to fetch single amenity by ID - Add listRentableTypesForAdmin() method to list rentable types for administration - Create get_amenity action - Create list_amenities action - Create list_rentable_types action Co-authored-by: Ona <no-reply@ona.com>
- Fix parameter name typo in listAmenities (opt -> opts) - Fix import path in list_amenities action - Fix parameter name in getAmenity (id -> amenityId) - Fix method name in list_rentable_types (listRentableTypes -> listRentableTypesForAdmin) - Fix parameter name in list_rentable_types (query -> params) - Rename misspelled file list_amennities.mjs to list_amenities.mjs - Add missing implementation to get_amenity and list_rentable_types actions Co-authored-by: Ona <no-reply@ona.com>
- Rename directories to use hyphens (get-amenity, list-amenities, list-rentable-types) - Add documentation links to get-amenity and list-rentable-types descriptions - Update list-amenities to use propDefinitions for page and perPage props - Fix property order in get-amenity (move annotations before type) - Fix indentation in list-amenities (use 2-space indentation) - Standardize summary formatting in list-rentable-types All actions now follow established codebase conventions. Co-authored-by: Ona <no-reply@ona.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
|
Thanks for submitting this PR! When we review PRs, we follow the Pipedream component guidelines. If you're not familiar, here's a quick checklist:
|
WalkthroughThis PR adds three new action modules to the Booking Experts integration: Get Amenity, List Amenities, and List Rentable Types. Corresponding methods are added to the booking_experts app client to support these actions, enabling retrieval of amenity and rentable type data from the BookingExperts API. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes The additions follow a consistent, homogeneous pattern across all files. New action modules are straightforward implementations with minimal logic, and app client methods are single-purpose wrappers. All changes are additive with no modifications to existing functionality. Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. 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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
components/booking_experts/actions/get-amenity/get_amenity.mjs(1 hunks)components/booking_experts/actions/list-amenities/list_amenities.mjs(1 hunks)components/booking_experts/actions/list-rentable-types/list_rentable_types.mjs(1 hunks)components/booking_experts/booking_experts.app.mjs(1 hunks)
| import bookingExperts from "../../booking_experts.app.mjs"; | ||
|
|
||
| export default { | ||
| key: "booking_experts-get-amenity", | ||
| name: "Get Amenity", | ||
| description: "Retrieve a single amenity by ID. [See the documentation](https://developers.bookingexperts.com/reference/amenities-show)", | ||
| version: "0.0.1", | ||
| annotations: { | ||
| destructiveHint: false, | ||
| openWorldHint: true, | ||
| readOnlyHint: true, | ||
| }, | ||
| type: "action", | ||
| props: { | ||
| bookingExperts, | ||
|
|
||
| amenityId: { | ||
| type: "string", | ||
| label: "Amenity ID", | ||
| description: "The ID of the amenity to retrieve.", | ||
| }, | ||
| }, | ||
|
|
||
| async run({ $ }) { | ||
| const { data } = await this.bookingExperts.getAmenity({ | ||
| $, | ||
| amenityId: this.amenityId, | ||
| }); | ||
|
|
||
| $.export("$summary", `Successfully retrieved amenity ${this.amenityId}`); | ||
| return data; | ||
| }, |
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.
🧹 Nitpick | 🔵 Trivial
Get Amenity action is clean and aligned with existing patterns
The action wiring looks good: correct import, metadata, annotations, and use of this.bookingExperts.getAmenity({ $, amenityId }), with a concise $summary and returning data.
If you later introduce a reusable amenityId prop definition (e.g., sourced from a list-amenities step), you could switch this prop to a propDefinition to give users a dropdown instead of a raw ID string, but that’s purely ergonomic and not required for this PR.
🤖 Prompt for AI Agents
In components/booking_experts/actions/get-amenity/get_amenity.mjs around lines
1–32 the action is correct and requires no functional changes; keep the current
prop as amenityId:string and the existing run logic returning data and exporting
the $summary as-is; optionally, if you later add a reusable amenityId
propDefinition (e.g., sourced from a list-amenities step) replace the amenityId
prop with propDefinition: "amenityId" to surface a dropdown to users instead of
a raw string.
| import bookingExperts from "../../booking_experts.app.mjs"; | ||
|
|
||
| export default { | ||
| key: "booking_experts-list-amenities", | ||
| name: "List Amenities", | ||
| description: "List amenities from BookingExperts. [See the documentation](https://developers.bookingexperts.com/reference/amenities-index)", | ||
| version: "0.0.1", | ||
| annotations: { | ||
| destructiveHint: false, | ||
| openWorldHint: true, | ||
| readOnlyHint: true, | ||
| }, | ||
| type: "action", | ||
| props: { | ||
| bookingExperts, | ||
| page: { | ||
| propDefinition: [ | ||
| bookingExperts, | ||
| "page", | ||
| ], | ||
| }, | ||
| perPage: { | ||
| propDefinition: [ | ||
| bookingExperts, | ||
| "perPage", | ||
| ], | ||
| }, | ||
| filters: { | ||
| type: "object", | ||
| label: "Filters", | ||
| optional: true, | ||
| description: "Additional query params to filter amenities", | ||
| }, | ||
| }, | ||
| async run({ $ }) { | ||
| const params = { | ||
| "page[number]": this.page, | ||
| "page[size]": this.perPage, | ||
| ...this.filters, | ||
| }; | ||
|
|
||
| const { data } = await this.bookingExperts.listAmenities({ | ||
| $, | ||
| params, | ||
| }); | ||
|
|
||
| $.export("$summary", `Successfully retrieved ${data?.length ?? 0} amenities`); | ||
|
|
||
| return data; | ||
| }, |
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.
🧹 Nitpick | 🔵 Trivial
List Amenities action matches existing pagination/filter conventions
This action is consistent with the rest of the integration: it reuses the shared page / perPage propDefinitions, supports arbitrary filter params, calls listAmenities with { $, params }, and reports a clear count-based $summary.
If you’d like to avoid emitting page[number] / page[size] when they’re unset, you could optionally guard them:
- const params = {
- "page[number]": this.page,
- "page[size]": this.perPage,
- ...this.filters,
- };
+ const params = {
+ ...this.filters,
+ ...(this.page != null && { "page[number]": this.page }),
+ ...(this.perPage != null && { "page[size]": this.perPage }),
+ };Not required for correctness, but can keep the outgoing query string a bit cleaner.
📝 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.
| import bookingExperts from "../../booking_experts.app.mjs"; | |
| export default { | |
| key: "booking_experts-list-amenities", | |
| name: "List Amenities", | |
| description: "List amenities from BookingExperts. [See the documentation](https://developers.bookingexperts.com/reference/amenities-index)", | |
| version: "0.0.1", | |
| annotations: { | |
| destructiveHint: false, | |
| openWorldHint: true, | |
| readOnlyHint: true, | |
| }, | |
| type: "action", | |
| props: { | |
| bookingExperts, | |
| page: { | |
| propDefinition: [ | |
| bookingExperts, | |
| "page", | |
| ], | |
| }, | |
| perPage: { | |
| propDefinition: [ | |
| bookingExperts, | |
| "perPage", | |
| ], | |
| }, | |
| filters: { | |
| type: "object", | |
| label: "Filters", | |
| optional: true, | |
| description: "Additional query params to filter amenities", | |
| }, | |
| }, | |
| async run({ $ }) { | |
| const params = { | |
| "page[number]": this.page, | |
| "page[size]": this.perPage, | |
| ...this.filters, | |
| }; | |
| const { data } = await this.bookingExperts.listAmenities({ | |
| $, | |
| params, | |
| }); | |
| $.export("$summary", `Successfully retrieved ${data?.length ?? 0} amenities`); | |
| return data; | |
| }, | |
| import bookingExperts from "../../booking_experts.app.mjs"; | |
| export default { | |
| key: "booking_experts-list-amenities", | |
| name: "List Amenities", | |
| description: "List amenities from BookingExperts. [See the documentation](https://developers.bookingexperts.com/reference/amenities-index)", | |
| version: "0.0.1", | |
| annotations: { | |
| destructiveHint: false, | |
| openWorldHint: true, | |
| readOnlyHint: true, | |
| }, | |
| type: "action", | |
| props: { | |
| bookingExperts, | |
| page: { | |
| propDefinition: [ | |
| bookingExperts, | |
| "page", | |
| ], | |
| }, | |
| perPage: { | |
| propDefinition: [ | |
| bookingExperts, | |
| "perPage", | |
| ], | |
| }, | |
| filters: { | |
| type: "object", | |
| label: "Filters", | |
| optional: true, | |
| description: "Additional query params to filter amenities", | |
| }, | |
| }, | |
| async run({ $ }) { | |
| const params = { | |
| ...this.filters, | |
| ...(this.page != null && { "page[number]": this.page }), | |
| ...(this.perPage != null && { "page[size]": this.perPage }), | |
| }; | |
| const { data } = await this.bookingExperts.listAmenities({ | |
| $, | |
| params, | |
| }); | |
| $.export("$summary", `Successfully retrieved ${data?.length ?? 0} amenities`); | |
| return data; | |
| }, |
🤖 Prompt for AI Agents
In components/booking_experts/actions/list-amenities/list_amenities.mjs around
lines 1 to 50, avoid emitting page[number] and page[size] when
this.page/this.perPage are unset by constructing params from the filters first
(e.g., start with a shallow copy of this.filters), then conditionally add
"page[number]" only if this.page is not null/undefined and "page[size]" only if
this.perPage is not null/undefined; keep passing { $, params } to listAmenities
and preserve the summary behavior.
| import bookingExperts from "../../booking_experts.app.mjs"; | ||
|
|
||
| export default { | ||
| key: "booking_experts-list-rentable-types", | ||
| name: "List Rentable Types", | ||
| description: "List all rentable types for a given administration. [See the documentation](https://developers.bookingexperts.com/reference/administration-rentabletypes-index)", | ||
| version: "0.0.1", | ||
| type: "action", | ||
|
|
||
| annotations: { | ||
| destructiveHint: false, | ||
| openWorldHint: true, | ||
| readOnlyHint: true, | ||
| }, | ||
|
|
||
| props: { | ||
| bookingExperts, | ||
|
|
||
| administrationId: { | ||
| propDefinition: [ | ||
| bookingExperts, | ||
| "administrationId", | ||
| ], | ||
| }, | ||
|
|
||
| query: { | ||
| type: "object", | ||
| label: "Query Filters", | ||
| description: "Optional filters passed to the API. Leave empty to fetch everything.", | ||
| optional: true, | ||
| }, | ||
| }, | ||
|
|
||
| async run({ $ }) { | ||
| const { data } = await this.bookingExperts.listRentableTypesForAdmin({ | ||
| $, | ||
| administrationId: this.administrationId, | ||
| params: this.query, | ||
| }); | ||
|
|
||
| $.export("$summary", `Successfully retrieved ${data?.length ?? 0} rentable types for Administration ${this.administrationId}`); | ||
|
|
||
| return data; | ||
| }, |
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.
🧹 Nitpick | 🔵 Trivial
List Rentable Types action correctly leverages the new client method
This action is well-structured: it reuses the administrationId propDefinition, calls listRentableTypesForAdmin with the right arguments, and exports a helpful summary based on data?.length.
If you want to normalize the axios config a bit, you could ensure params is always an object so callers don’t have to think about it, e.g.:
- const { data } = await this.bookingExperts.listRentableTypesForAdmin({
- $,
- administrationId: this.administrationId,
- params: this.query,
- });
+ const { data } = await this.bookingExperts.listRentableTypesForAdmin({
+ $,
+ administrationId: this.administrationId,
+ params: this.query ?? {},
+ });Functionally it should work as-is; this is just a small consistency tweak.
📝 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.
| import bookingExperts from "../../booking_experts.app.mjs"; | |
| export default { | |
| key: "booking_experts-list-rentable-types", | |
| name: "List Rentable Types", | |
| description: "List all rentable types for a given administration. [See the documentation](https://developers.bookingexperts.com/reference/administration-rentabletypes-index)", | |
| version: "0.0.1", | |
| type: "action", | |
| annotations: { | |
| destructiveHint: false, | |
| openWorldHint: true, | |
| readOnlyHint: true, | |
| }, | |
| props: { | |
| bookingExperts, | |
| administrationId: { | |
| propDefinition: [ | |
| bookingExperts, | |
| "administrationId", | |
| ], | |
| }, | |
| query: { | |
| type: "object", | |
| label: "Query Filters", | |
| description: "Optional filters passed to the API. Leave empty to fetch everything.", | |
| optional: true, | |
| }, | |
| }, | |
| async run({ $ }) { | |
| const { data } = await this.bookingExperts.listRentableTypesForAdmin({ | |
| $, | |
| administrationId: this.administrationId, | |
| params: this.query, | |
| }); | |
| $.export("$summary", `Successfully retrieved ${data?.length ?? 0} rentable types for Administration ${this.administrationId}`); | |
| return data; | |
| }, | |
| import bookingExperts from "../../booking_experts.app.mjs"; | |
| export default { | |
| key: "booking_experts-list-rentable-types", | |
| name: "List Rentable Types", | |
| description: "List all rentable types for a given administration. [See the documentation](https://developers.bookingexperts.com/reference/administration-rentabletypes-index)", | |
| version: "0.0.1", | |
| type: "action", | |
| annotations: { | |
| destructiveHint: false, | |
| openWorldHint: true, | |
| readOnlyHint: true, | |
| }, | |
| props: { | |
| bookingExperts, | |
| administrationId: { | |
| propDefinition: [ | |
| bookingExperts, | |
| "administrationId", | |
| ], | |
| }, | |
| query: { | |
| type: "object", | |
| label: "Query Filters", | |
| description: "Optional filters passed to the API. Leave empty to fetch everything.", | |
| optional: true, | |
| }, | |
| }, | |
| async run({ $ }) { | |
| const { data } = await this.bookingExperts.listRentableTypesForAdmin({ | |
| $, | |
| administrationId: this.administrationId, | |
| params: this.query ?? {}, | |
| }); | |
| $.export("$summary", `Successfully retrieved ${data?.length ?? 0} rentable types for Administration ${this.administrationId}`); | |
| return data; | |
| }, |
🤖 Prompt for AI Agents
In
components/booking_experts/actions/list-rentable-types/list_rentable_types.mjs
around lines 1 to 44, ensure the axios params passed to
listRentableTypesForAdmin is always an object to avoid callers needing to handle
undefined: when building the call, coerce this.query to an object (e.g., use a
nullish fallback) so params is never undefined; keep the rest of the call and
summary unchanged.
| listAmenities(opts = {}) { | ||
| return this._makeRequest({ | ||
| path: "/amenities", | ||
| ...opts, | ||
| }); | ||
| }, | ||
| getAmenity({ | ||
| amenityId, ...opts | ||
| }) { | ||
| return this._makeRequest({ | ||
| path: `/amenities/${amenityId}`, | ||
| ...opts, | ||
| }); | ||
| }, | ||
| listRentableTypesForAdmin({ | ||
| administrationId, ...opts | ||
| }) { | ||
| return this._makeRequest({ | ||
| path: `/administrations/${administrationId}/rentable_types`, | ||
| ...opts, | ||
| }); | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
New amenities / rentable-type client methods are consistent and correctly wired
These three methods follow the existing client pattern: they delegate to _makeRequest, use clear REST paths, and preserve flexible options via ...opts. Naming listRentableTypesForAdmin also nicely distinguishes it from the channel-scoped listRentableTypes.
If you want a bit more defensive behavior on the client surface, you could optionally default the argument objects and assert required IDs, e.g.:
- getAmenity({
- amenityId, ...opts
- }) {
+ getAmenity({
+ amenityId, ...opts
+ } = {}) {
+ if (!amenityId) {
+ throw new Error("amenityId is required");
+ }
return this._makeRequest({
path: `/amenities/${amenityId}`,
...opts,
});
},
- listRentableTypesForAdmin({
- administrationId, ...opts
- }) {
+ listRentableTypesForAdmin({
+ administrationId, ...opts
+ } = {}) {
+ if (!administrationId) {
+ throw new Error("administrationId is required");
+ }
return this._makeRequest({
path: `/administrations/${administrationId}/rentable_types`,
...opts,
});
},Not required for correctness (the actions already validate inputs), but it hardens the app client against accidental misuse elsewhere.
📝 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.
| listAmenities(opts = {}) { | |
| return this._makeRequest({ | |
| path: "/amenities", | |
| ...opts, | |
| }); | |
| }, | |
| getAmenity({ | |
| amenityId, ...opts | |
| }) { | |
| return this._makeRequest({ | |
| path: `/amenities/${amenityId}`, | |
| ...opts, | |
| }); | |
| }, | |
| listRentableTypesForAdmin({ | |
| administrationId, ...opts | |
| }) { | |
| return this._makeRequest({ | |
| path: `/administrations/${administrationId}/rentable_types`, | |
| ...opts, | |
| }); | |
| } | |
| listAmenities(opts = {}) { | |
| return this._makeRequest({ | |
| path: "/amenities", | |
| ...opts, | |
| }); | |
| }, | |
| getAmenity({ | |
| amenityId, ...opts | |
| } = {}) { | |
| if (!amenityId) { | |
| throw new Error("amenityId is required"); | |
| } | |
| return this._makeRequest({ | |
| path: `/amenities/${amenityId}`, | |
| ...opts, | |
| }); | |
| }, | |
| listRentableTypesForAdmin({ | |
| administrationId, ...opts | |
| } = {}) { | |
| if (!administrationId) { | |
| throw new Error("administrationId is required"); | |
| } | |
| return this._makeRequest({ | |
| path: `/administrations/${administrationId}/rentable_types`, | |
| ...opts, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In components/booking_experts/booking_experts.app.mjs around lines 487 to 508,
the new client methods accept destructured parameter objects but do not default
those parameters or validate required IDs; update each method signature to
default the parameter object (e.g. opts = {}) and add a short runtime check that
the required id (amenityId or administrationId) is present, throwing a clear
Error if missing, before calling this._makeRequest so the client surface fails
fast on accidental misuse.
WHY
issue #19368
The Booking Experts integration was missing actions to manage amenities and rentable types
, which are core features of the Booking Experts API. These actions enable users to:
This addition allows Pipedream users to build more complete automation workflows with Booking Experts, particularly for property management and booking systems that need to work with amenities and accommodation types.
WHAT
Added three new actions to the Booking Experts component:
list-amenities) - Retrieves all amenities with pagination and filtering supportget-amenity) - Fetches a single amenity by IDlist-rentable-types) - Lists all rentable types for a given administrationChanges Made:
listAmenities()method tobooking_experts.app.mjsgetAmenity()method tobooking_experts.app.mjslistRentableTypesForAdmin()method tobooking_experts.app.mjsAPI Documentation:
HOW TO TEST
All actions follow the existing Booking Experts component patterns and conventions.
Summary by CodeRabbit
New Features
✏️ Tip: You can customize this high-level summary in your review settings.