Skip to content

Conversation

@luancazarine
Copy link
Collaborator

@luancazarine luancazarine commented Nov 28, 2025

Resolves #19270

Summary by CodeRabbit

  • New Features

    • Significantly enhanced availability search: many new filters (dates, LOS, guest counts, amenities, pricing, property features, countries, discounts), expanded sorting and limiter options, and support for complex/array inputs.
    • New selectable options and endpoints to populate rentable segments, amenities and discount campaign pickers.
  • Chores

    • Version bumps across multiple actions, sources and package metadata.

✏️ Tip: You can customize this high-level summary in your review settings.

…types

- Introduced new fields for filtering availabilities and rentable types, including `fields`, `include`, and `multibookSafetyMargin`.
- Added methods for listing rentable segments, amenities, and discount campaigns in the booking experts app.
- Updated version numbers for actions related to availabilities and rentable type availabilities.
- Created constants for fields, limiters, sorters, and weekdays to enhance filtering capabilities.
@luancazarine luancazarine linked an issue Nov 28, 2025 that may be closed by this pull request
@vercel
Copy link

vercel bot commented Nov 28, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
pipedream-docs Ignored Ignored Nov 29, 2025 1:50am
pipedream-docs-redirect-do-not-edit Ignored Ignored Nov 29, 2025 1:50am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 28, 2025

Walkthrough

Adds extensive new availability filters, sorters, limiters, and include props to two actions; introduces UI constants and a parseObject utility; adds propDefinitions and list endpoints to the booking_experts app; and updates many action/source versions and the package version.

Changes

Cohort / File(s) Summary
Availabilities action
components/booking_experts/actions/list-availabilities/list-availabilities.mjs
Version → 0.0.3. Imports LIMITERS_OPTIONS, SORTERS_OPTIONS, WEEKDAY_OPTIONS, and parseObject. Expands public props with many filter/sorter/limiter/include fields. run() now constructs a comprehensive params object mapping those props (including parsed comma-separated arrays and nested filter[...], sorter[...], limiter[...] keys) and calls this.bookingExperts.listAvailabilities.
Rentable-type availabilities action
components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs
Version → 0.0.3. Imports FIELDS_OPTIONS and parseObject. Adds fields, include, and multibookSafetyMargin props. run() passes fields[rentable_type_availability], include, and multibook_safety_margin (parsed/joined as needed) to the API call.
App propDefinitions & API methods
components/booking_experts/booking_experts.app.mjs
Adds rentableSegmentId, amenityId, discountCampaignId propDefinitions with async options. Adds methods listDiscountCampaigns(administrationId,...opts), listRentableSegments(opts), and listAmenities(opts) that call respective API endpoints.
Constants
components/booking_experts/common/constants.mjs
New exports: FIELDS_OPTIONS, LIMITERS_OPTIONS, SORTERS_OPTIONS, WEEKDAY_OPTIONS for UI option lists.
Utility
components/booking_experts/common/utils.mjs
Adds parseObject(obj) to safely parse string/array inputs (tries JSON.parse with try/catch); returns parsed arrays/values or original input when parsing fails.
Version bumps (metadata-only)
components/booking_experts/actions/*, components/booking_experts/sources/*, components/booking_experts/package.json
Many modules had only their exported version incremented (examples: add-guest-to-reservation, create-agenda-period, delete-guest, get-booking, get-reservation, list-bookings, list-inventory-objects, list-reservations, search-contacts, update-guest, several sources). Package version bumped to 0.4.0.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Focus review on list-availabilities mappings of props → params (snake_case keys, nested filter[...]/sorter[...]/limiter[...] structures).
  • Verify parseObject behavior for array/string inputs used across new props.
  • Check constants (FIELDS_OPTIONS, SORTERS_OPTIONS, LIMITERS_OPTIONS, WEEKDAY_OPTIONS) for completeness and naming.
  • Inspect new app methods and async prop options for correct endpoint usage and pagination/error handling.

Possibly related PRs

Suggested labels

User submitted

Suggested reviewers

  • michelle0927
  • jcortes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes version bumps across multiple actions and sources that are outside the scope of expanding filters for the two specified endpoints. Remove or isolate the version bump changes to a separate PR focused only on versioning, keeping this PR scoped to filter expansion for the two targeted endpoints.
Description check ❓ Inconclusive The PR description only contains a minimal reference to the linked issue (Resolves #19270) without following the template structure that requires a WHY section. Expand the PR description to include the WHY section from the template explaining the rationale and impact of expanding filters for these endpoints.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately describes the main change: adding new fields and filtering options for availabilities and rentable type endpoints.
Linked Issues check ✅ Passed The PR successfully implements all required optional filters for both GET availabilities and GET rentable_type availabilities endpoints as requested in issue #19270.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 19270-action-booking-experts---expand-filters

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

…s to reflect recent changes. Actions updated include adding guests, creating agenda periods, deleting guests, fetching bookings, and listing reservations. Sources updated include booking updates, inventory object updates, and new booking events.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ce189ad and ce7dde5.

📒 Files selected for processing (5)
  • components/booking_experts/actions/list-availabilities/list-availabilities.mjs (2 hunks)
  • components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs (3 hunks)
  • components/booking_experts/booking_experts.app.mjs (2 hunks)
  • components/booking_experts/common/constants.mjs (1 hunks)
  • components/booking_experts/common/utils.mjs (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs (3)
components/booking_experts/common/constants.mjs (2)
  • FIELDS_OPTIONS (1-12)
  • FIELDS_OPTIONS (1-12)
components/booking_experts/actions/list-availabilities/list-availabilities.mjs (1)
  • parseObject (464-536)
components/booking_experts/common/utils.mjs (2)
  • parseObject (1-24)
  • parseObject (1-24)
components/booking_experts/actions/list-availabilities/list-availabilities.mjs (2)
components/booking_experts/common/constants.mjs (6)
  • WEEKDAY_OPTIONS (80-109)
  • WEEKDAY_OPTIONS (80-109)
  • SORTERS_OPTIONS (37-78)
  • SORTERS_OPTIONS (37-78)
  • LIMITERS_OPTIONS (14-35)
  • LIMITERS_OPTIONS (14-35)
components/booking_experts/common/utils.mjs (2)
  • parseObject (1-24)
  • parseObject (1-24)
⏰ 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: Lint Code Base
  • GitHub Check: pnpm publish
  • GitHub Check: Publish TypeScript components
  • GitHub Check: Verify TypeScript components
🔇 Additional comments (6)
components/booking_experts/common/utils.mjs (1)

1-24: LGTM!

The parseObject utility handles various input types robustly with appropriate try/catch guards for JSON parsing failures. The function gracefully falls back to the original value when parsing fails, which is the correct behavior for this use case.

components/booking_experts/booking_experts.app.mjs (1)

462-481: LGTM!

The new API methods follow the established patterns consistently and correctly delegate to _makeRequest with appropriate paths.

components/booking_experts/common/constants.mjs (1)

1-109: LGTM!

Constants are well-structured with clear labels. The weekday values correctly follow the standard convention (0=Sunday through 6=Saturday).

components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs (2)

72-92: LGTM!

The run method correctly builds the params object with optional fields. The use of parseObject and optional chaining ensures graceful handling of undefined values.


52-58: Remove this review comment — the core premise is incorrect.

The FIELDS_OPTIONS constant is not shared with the list-availabilities action. The list-availabilities.mjs file does not import or use FIELDS_OPTIONS at all; its fields parameter has no predefined options. These are separate endpoints with different implementations: list-availabilities queries the general availabilities endpoint, while list-rentabletype-availabilities queries the rentable_type_availability endpoint and appropriately provides field options specific to that context.

Likely an incorrect or invalid review comment.

components/booking_experts/actions/list-availabilities/list-availabilities.mjs (1)

463-539: Overall implementation approach is sound.

The comprehensive params object construction with parseObject and optional chaining handles the many optional filters appropriately. The structure aligns well with the PR objective of expanding filter support.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
components/booking_experts/actions/list-inventory-objects/list-inventory-objects.mjs (1)

53-55: Consider using parseObject utility for labels serialization.

The PR context mentions a new parseObject utility introduced in shared constants. Currently, the code manually joins labels with a comma. If parseObject is the standard way to serialize complex fields in this PR, apply it here for consistency.

Please confirm whether labels should use:

-"filter[labels]": this.labels
-  ? this.labels.join(",")
-  : undefined,
+"filter[labels]": this.labels
+  ? parseObject(this.labels)
+  : undefined,

(or similar, depending on the intended signature of parseObject)

components/booking_experts/actions/list-bookings/list-bookings.mjs (1)

67-77: Channel filter uses undefined prop; created/updated filters have no props

In the params object:

  • "filter[channel]" uses this.listAdministrationChannels, but there is no listAdministrationChannels prop defined. The available prop is administrationChannelId (Line 32). As written, the channel filter is never actually applied.
  • "filter[created_at]" and "filter[updated_at]" reference this.createdAt and this.updatedAt, but there are no corresponding props, so these filters are effectively dead code.

This breaks the advertised “Filter by channel” behavior and leaves the created/updated filters in a half‑wired state.

Consider updating the action like this:

    const { data } = await this.bookingExperts.listBookings({
      $,
      administrationId: this.administrationId,
      params: {
        "filter[owner]": this.ownerId,
-        "filter[channel]": this.listAdministrationChannels,
-        "filter[reservations]": this.reservationId,
-        "filter[created_at]": this.createdAt,
-        "filter[updated_at]": this.updatedAt,
+        "filter[channel]": this.administrationChannelId,
+        "filter[reservations]": this.reservationId,
+        // If you intend to support these, add corresponding props to `props`:
+        // "filter[created_at]": this.createdAt,
+        // "filter[updated_at]": this.updatedAt,
        "page[number]": this.page,
        "page[size]": this.perPage,
      },
    });

If createdAt / updatedAt filters should be user-configurable, also add explicit props for them to the props section; otherwise, remove those params entirely to avoid confusion.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ce7dde5 and 31fbd5e.

📒 Files selected for processing (15)
  • components/booking_experts/actions/add-guest-to-reservation/add-guest-to-reservation.mjs (1 hunks)
  • components/booking_experts/actions/create-agenda-period/create-agenda-period.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/get-complex-prices/get-complex-prices.mjs (1 hunks)
  • components/booking_experts/actions/get-reservation/get-reservation.mjs (1 hunks)
  • components/booking_experts/actions/list-bookings/list-bookings.mjs (1 hunks)
  • components/booking_experts/actions/list-inventory-objects/list-inventory-objects.mjs (1 hunks)
  • components/booking_experts/actions/list-reservations/list-reservations.mjs (1 hunks)
  • components/booking_experts/actions/search-contacts/search-contacts.mjs (1 hunks)
  • components/booking_experts/actions/update-guest/update-guest.mjs (1 hunks)
  • components/booking_experts/sources/booking-updated/booking-updated.mjs (1 hunks)
  • components/booking_experts/sources/inventory-object-updated/inventory-object-updated.mjs (1 hunks)
  • components/booking_experts/sources/new-booking-created/new-booking-created.mjs (1 hunks)
  • components/booking_experts/sources/new-inventory-object-created/new-inventory-object-created.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). (3)
  • GitHub Check: Verify TypeScript components
  • GitHub Check: Publish TypeScript components
  • GitHub Check: Lint Code Base
🔇 Additional comments (14)
components/booking_experts/sources/new-booking-created/new-booking-created.mjs (1)

8-8: Version bump looks consistent with broader Booking Experts changes

The version increment to 0.0.4 matches the pattern of PR-wide version bumps for Booking Experts components and doesn’t alter runtime behavior for this source. As long as any external references (docs, templates, or catalog metadata) expect the new version, this is good to go.

components/booking_experts/sources/inventory-object-updated/inventory-object-updated.mjs (1)

8-8: Verify scope: version bump appears misaligned with PR objectives.

This file handles inventory object updates, but the PR objectives specifically request expanding filters for availability endpoints (GET availabilities and GET rentable_type availabilities). The version bump to "0.0.4" lacks accompanying functional changes and appears out of scope.

Please clarify whether this version bump is intentional, part of a broader versioning strategy across multiple files, or should be removed from this PR.

components/booking_experts/actions/add-guest-to-reservation/add-guest-to-reservation.mjs (1)

7-7: Consider whether version bump is warranted without functional changes.

The version has been bumped from "0.0.4" to "0.0.5", but according to the enriched summary and the provided code, this action has no behavioral changes, error handling modifications, or control flow updates. Semantic versioning typically reserves patch version bumps for bug fixes or minor functional improvements.

If this is a systematic version bump across all related actions in the PR, this may be appropriate for consistency. However, if this action is unchanged while related actions are being enhanced, the version may remain at "0.0.4".

Could you clarify whether this version bump is intentional? Is this part of a systematic versioning strategy across all Booking Experts actions, or should it remain at "0.0.4" given no functional changes?

components/booking_experts/actions/create-agenda-period/create-agenda-period.mjs (1)

7-7: Clarify the rationale for the version bump.

This file has no functional changes—only a version increment from 0.0.4 to 0.0.5. Since the PR objectives focus on expanding availability filters (in list-availabilities and list-rentabletype-availabilities), it's unclear why this unrelated action should be versioned.

Is this a collateral bump across all Booking Experts actions, or should this change be excluded from the PR?

components/booking_experts/actions/search-contacts/search-contacts.mjs (1)

4-13: Version bump to 0.0.5 is appropriate and non-breaking

Patch version increment with no logic changes is consistent with current behavior; this action’s runtime behavior remains unchanged.

components/booking_experts/actions/delete-guest/delete-guest.mjs (1)

7-7: Clarify the rationale for the version bump without functional changes.

The version is incremented from 0.0.2 to 0.0.3, but the enriched summary notes that this action "does not incorporate these changes directly, it only reflects the version bump." This is inconsistent with semantic versioning, where a patch bump (0.0.2 → 0.0.3) should accompany a bug fix or compatible change, but the action has no functional modifications.

Either this is part of a coordinated versioning strategy across all Booking Experts actions (which should be documented), or the version bump should not be applied to actions without functional changes. Per the PR objectives, this PR is focused on expanding filters for availabilities; it's unclear whether delete-guest should be included.

components/booking_experts/actions/get-reservation/get-reservation.mjs (1)

7-7: Clarify version bump rationale.

The version is bumped from 0.0.1 to 0.0.2, but per the AI summary, there are no changes to logic, behavior, or control flow in this file. The PR objectives (#19270) specifically target expanding GET availabilities and GET rentable_type availabilities, not the get-reservation endpoint.

Is this version bump part of a coordinated release pattern across multiple actions, or should it be reverted if no functional changes are intended?

components/booking_experts/actions/update-guest/update-guest.mjs (1)

7-7: Version bump to 0.0.3 looks correct

This is a non-functional metadata update and is consistent with bumping action versions alongside broader Booking Experts changes.

components/booking_experts/actions/list-inventory-objects/list-inventory-objects.mjs (1)

7-7: Verify version bump aligns with PR scope and actual changes.

The version was bumped from 0.0.4 to 0.0.5, but the AI summary indicates "no changes to logic, arguments, or runtime behavior." Additionally, PR #19270 targets availability actions, not inventory objects. Clarify whether:

  1. This file is in scope for the PR (or if this version bump should be reverted)
  2. What substantive change warrants the version bump if no logic/args changed
components/booking_experts/sources/booking-updated/booking-updated.mjs (1)

8-8: Verify necessity of version bump.

The file contains only a version change (0.0.3 → 0.0.4) with no functional or logic modifications. Based on the AI summary, no behavior changes are present.

Confirm whether this version bump is necessary given that:

  1. This source handles booking updates, not availabilities (which are the focus of the PR).
  2. No usage of the new utilities (parseObject) or constants (FIELDS_OPTIONS, etc.) introduced in this PR appears in this file.

If this source doesn't directly consume the new capabilities being added in the PR, consider whether the version bump should be deferred or consolidated with other changes.

components/booking_experts/sources/new-inventory-object-created/new-inventory-object-created.mjs (1)

8-8: Verify the version bump rationale—no logic changes in this source.

The version is incremented from 0.0.3 to 0.0.4, but no functional changes, new features, or bug fixes are present in this source file. The PR's primary objective (expanding filters for availabilities) does not affect this source, and it does not use the new utilities or constants mentioned in the broader PR summary.

Semantic versioning typically ties version increments to actual changes. If this is a blanket version bump across the integration, clarify the rationale. If it was unintentional, consider reverting it.

components/booking_experts/actions/get-booking/get-booking.mjs (1)

7-7: Clarify the rationale for this version bump.

This file shows only a version bump without any functional changes to logic, props, or API calls. The PR objectives focus on expanding filters for GET availabilities and GET rentable_type availabilities endpoints, which are unrelated to the "Get Booking" action.

Verify whether:

  • This version bump is part of a coordinated versioning strategy across all actions in the PR
  • This action requires updates to use new utilities or constants introduced in the PR
  • This file should be included in this PR at all, or if it was inadvertently modified
components/booking_experts/actions/list-bookings/list-bookings.mjs (1)

7-7: Version bump-only change looks consistent

The version bump to "0.0.5" is consistent with the PR pattern of bumping Booking Experts action versions and does not introduce behavioral changes in this file.

components/booking_experts/actions/list-reservations/list-reservations.mjs (1)

7-7: No version bump exists in this file within the current PR.

The version field (line 7) shows "0.0.2", which was already set in a previous commit. Git diff returns no changes for this file, confirming it was not modified in the current PR. The review comment's claim that the version was "bumped from '0.0.1' to '0.0.2'" is incorrect—no version change occurred in this PR for this file.

While the commit message references updating version numbers for booking experts actions, the list-reservations action was not part of the current diff under review.

…k safety margin naming in availabilities actions

- Introduced `includeSemanticSegmentCounts` option to enhance metadata results.
- Renamed `multibookSsafetyMargin` to `multibookSafetyMargin` for consistency in rentable type availabilities.
- Updated parsing logic for `wday` filter to handle undefined values correctly.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (2)
components/booking_experts/actions/list-availabilities/list-availabilities.mjs (2)

27-32: Missing options for fields prop.

Unlike list-rentabletype-availabilities.mjs which uses FIELDS_OPTIONS, this prop has no options defined. Consider adding FIELDS_OPTIONS to provide a consistent user experience with selectable field values.


156-177: discountCampaignId and inventoryObjectId reference non-existent administrationId prop.

The prop definitions reference c.administrationId, but this action only has administrationIds (plural, array type) defined at lines 104-113. The callbacks will receive undefined, causing the dropdown options to fail loading.

Consider one of these approaches:

  1. Add a singular administrationId prop for these dependent fields
  2. Change the dependency to use the first element of administrationIds:
    discountCampaignId: {
      propDefinition: [
        bookingExperts,
        "discountCampaignId",
        (c) => ({
-          administrationId: c.administrationId,
+          administrationId: c.administrationIds?.[0],
        }),
      ],
      description: "Filter on discount campaign ID.",
      optional: true,
    },
    inventoryObjectId: {
      propDefinition: [
        bookingExperts,
        "inventoryObjectId",
        (c) => ({
-          administrationId: c.administrationId,
+          administrationId: c.administrationIds?.[0],
        }),
      ],
      description: "Filter on inventory object ID.",
      optional: true,
    },

Note: Option 2 may not provide the best UX since it only considers the first administration.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9f6c1d8 and 8164387.

📒 Files selected for processing (2)
  • components/booking_experts/actions/list-availabilities/list-availabilities.mjs (2 hunks)
  • components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs (3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-12T07:49:36.125Z
Learnt from: matyascimbulka
Repo: PipedreamHQ/pipedream PR: 18308
File: components/apify/actions/run-task-synchronously/run-task-synchronously.mjs:70-0
Timestamp: 2025-09-12T07:49:36.125Z
Learning: The Apify Task object always contains the `options` field according to the official API documentation, making nested destructuring like `options: { build }` safe to use without additional checks.

Applied to files:

  • components/booking_experts/actions/list-availabilities/list-availabilities.mjs
📚 Learning: 2025-09-11T01:53:51.070Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 18334
File: components/buddee/actions/list-employees/list-employees.mjs:147-155
Timestamp: 2025-09-11T01:53:51.070Z
Learning: In Buddee list-employees action, the "manager" prop should be a boolean type to filter employees who have direct reports, not an employeeId propDefinition which would send an employee ID instead of the expected boolean value.

Applied to files:

  • components/booking_experts/actions/list-availabilities/list-availabilities.mjs
🧬 Code graph analysis (1)
components/booking_experts/actions/list-availabilities/list-availabilities.mjs (2)
components/booking_experts/common/constants.mjs (6)
  • WEEKDAY_OPTIONS (80-109)
  • WEEKDAY_OPTIONS (80-109)
  • SORTERS_OPTIONS (37-78)
  • SORTERS_OPTIONS (37-78)
  • LIMITERS_OPTIONS (14-35)
  • LIMITERS_OPTIONS (14-35)
components/booking_experts/common/utils.mjs (2)
  • parseObject (1-24)
  • parseObject (1-24)
⏰ 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 (2)
components/booking_experts/actions/list-availabilities/list-availabilities.mjs (1)

49-54: Previously flagged issues have been fixed.

Good work addressing the past review feedback:

  • includeSemanticSegmentCounts is now properly defined as a prop (lines 49-54)
  • wday parsing now safely handles undefined values with a conditional check (lines 484-486)

Also applies to: 484-486

components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs (1)

10-10: Version bump to 0.0.3 is appropriate for the expanded public props

Given the new public inputs (fields, include, multibookSafetyMargin) and behavior changes, incrementing the action version to 0.0.3 is a reasonable, backwards‑compatible update.

Comment on lines +3 to +4
import { FIELDS_OPTIONS } from "../../common/constants.mjs";
import { parseObject } from "../../common/utils.mjs";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Conditionally include fields / include params to avoid undefined or empty values

parseObject(this.fields)?.join(",") and parseObject(this.include)?.join(",") will set the query keys unconditionally, which can result in undefined (if parseObject returns undefined) or an empty string (for an empty array). Depending on how listRentableTypeAvailabilities builds the request, these may still end up in the query string and differ from “parameter not sent at all”.

For consistency with the existing date_range handling and to keep the wire format clean, consider only adding these params when there’s at least one value:

   async run({ $ }) {
-    if ((!this.startDate && this.endDate) || (this.startDate && !this.endDate)) {
-      throw new ConfigurationError("You should provide both the start and end date parameters.");
-    }
-    const { data } = await this.bookingExperts.listRentableTypeAvailabilities({
+    if ((!this.startDate && this.endDate) || (this.startDate && !this.endDate)) {
+      throw new ConfigurationError("You should provide both the start and end date parameters.");
+    }
+
+    const fields = parseObject(this.fields);
+    const includes = parseObject(this.include);
+
+    const { data } = await this.bookingExperts.listRentableTypeAvailabilities({
       $,
       channelId: this.channelId,
       rentableTypeId: this.rentableTypeId,
       params: {
-        ...(this.startDate && this.endDate
-          && {
-            "date_range": `${this.startDate}..${this.endDate}`,
-          }),
-        "fields[rentable_type_availability]": parseObject(this.fields)?.join(","),
-        "include": parseObject(this.include)?.join(","),
-        "multibook_safety_margin": this.multibookSafetyMargin,
+        ...(this.startDate && this.endDate && {
+          "date_range": `${this.startDate}..${this.endDate}`,
+        }),
+        ...(fields?.length && {
+          "fields[rentable_type_availability]": fields.join(","),
+        }),
+        ...(includes?.length && {
+          "include": includes.join(","),
+        }),
+        ...(this.multibookSafetyMargin != null && {
+          "multibook_safety_margin": this.multibookSafetyMargin,
+        }),
       },
     });

This keeps the request closer to the API’s documented semantics and avoids relying on client‑side behavior around undefined values. Please double‑check that this matches how other Booking Experts actions in the repo handle optional list/array params for consistency.

Also applies to: 52-64, 85-86

🤖 Prompt for AI Agents
In
components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs
around lines 3-4 (and also apply same change at 52-64 and 85-86), the code
unconditionally sets query keys using parseObject(this.fields)?.join(",") and
parseObject(this.include)?.join(",") which can produce undefined or empty
strings; change the logic so you first call parseObject(...) into a local
variable, check that it is an array with length > 0, and only then set the
corresponding query param to the joined string (otherwise omit the key
entirely), mirroring the existing date_range handling and matching other Booking
Experts actions for consistent wire format.

Comment on lines +65 to +70
multibookSafetyMargin: {
type: "integer",
label: "Multi-book Safety Margin",
description: "Specifies a custom multibook safety margin (must be a positive number). A common problem that occurs when dealing with accommodations instead of hotelrooms is that a single accommodation must be available for all consecutive days for a given start date and LOS. The safety margin helps to prevent overbookings by transforming the available stock. When specified, the safety margin is subtracted from the actual stock. It is only applied to RentableType availabilities with capacity of 3 or more, as this issue cannot occur otherwise.",
optional: true,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Enforce “positive number” constraint for multibookSafetyMargin

The description says the safety margin “must be a positive number”, but the prop currently accepts any integer (including 0 or negative), passing it straight through to the API. To align behavior with the description and fail early with a clearer error, consider validating this before making the request, similar to the start/end date validation, for example:

   async run({ $ }) {
-    if ((!this.startDate && this.endDate) || (this.startDate && !this.endDate)) {
-      throw new ConfigurationError("You should provide both the start and end date parameters.");
-    }
+    if ((!this.startDate && this.endDate) || (this.startDate && !this.endDate)) {
+      throw new ConfigurationError("You should provide both the start and end date parameters.");
+    }
+
+    if (this.multibookSafetyMargin != null && this.multibookSafetyMargin <= 0) {
+      throw new ConfigurationError("Multi-book Safety Margin must be a positive integer when provided.");
+    }

This keeps invalid values from reaching the external API and matches the contract implied by the prop description.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
components/booking_experts/actions/list-rentabletype-availabilities/list-rentabletype-availabilities.mjs
around lines 65 to 70, the multibookSafetyMargin prop is declared as an integer
but the description requires a positive number; add an input validation step
(like the start/end date checks) that, when multibookSafetyMargin is provided,
ensures it is an integer > 0 and rejects/throws a clear error if it is 0 or
negative so invalid values are caught locally before calling the external API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ACTION] Booking Experts - expand filters

2 participants