-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New Components - domain_group #15932
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThis PR introduces new modules for creating business, commercial, and residential listings within the domain group. It adds detailed action modules with asynchronous run methods that interface with the domain group's API, along with a shared module for property type constants. The domain group application's property definitions and API methods have been expanded to support listing creation, pagination, and agency operations. Several new source modules have been added to handle agency listing updates and new agency events. The package version has been bumped and a dependency was added. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User/Context
participant AM as Action Module (Create Listing)
participant DG as DomainGroup API
U->>AM: Invoke run(context)
AM->>DG: Prepare and send listing data
DG-->>AM: Return created listing ID
AM->>U: Return summary with listing ID
sequenceDiagram
participant S as Scheduler
participant SM as Source Module
participant DG as DomainGroup API
S->>SM: Trigger getResults
SM->>DG: Fetch updated listings or new agencies
DG-->>SM: Return listings data
SM->>SM: Generate metadata for each item
SM->>S: Emit event with metadata
Possibly related PRs
Suggested labels
Suggested reviewers
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
Scope: all 2 workspace projects Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ 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:
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 (
|
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
🧹 Nitpick comments (12)
components/domain_group/sources/new-agency-created/new-agency-created.mjs (1)
13-20: Simplify the getResults method.The current implementation creates an array, loops through agencies, and pushes each one to the array. This can be simplified since the agencies variable is likely already an array.
async getResults() { const agencies = await this.domainGroup.listAgencies(); - const results = []; - for (const agency of agencies) { - results.push(agency); - } - return results; + return agencies; }components/domain_group/sources/common/base.mjs (4)
17-23: Consider default fallback for no stored timestamp
IflastTsdoesn't exist in the database (e.g., first run),_getLastTs()will returnundefined. Ensure that your downstream logic gracefully handles this scenario (e.g., treatundefinedas a zero or minimal timestamp for retrieval).
27-56: Validate item timestamps before parsing
The code callsDate.parse(ts), but doesn't verify thatitem[tsField]exists or is valid. IftsFieldis undefined or null,Date.parse()might yieldNaN. Consider adding a check to ensure valid timestamps and gracefully handle any malformed data.
57-66: Clarify or implement configuration methods
ThegetResourceFn(),getTsField(), andgenerateMeta()methods are not implemented yet and raiseConfigurationError. Confirm that child classes override and implement these methods; otherwise, this source will fail at runtime.
67-78: Potentially expose run results
Insiderun(), you emit items and then return if empty. If debugging or chaining is required later, consider returning the processed results or logging them. This can help when investigating issues or verifying outputs in subsequent steps.components/domain_group/actions/create-commercial-listing/create-commercial-listing.mjs (2)
4-138: Validate required props and handle missing data
Your props are defined with multiple required fields (agencyId, providerAdId, streetNumber, etc.). If users fail to provide them, the call tocreateCommercialListing()might result in errors. Consider adding form-level or runtime validations to provide clearer error messages before making the API call.
139-182: Handle unexpected listingAction values
listingActioncan be “sale”, “rent”, or “saleAndLease”; you account for these cases. Consider adding a default scenario or explicit validation if a user somehow provides a value outside these three enumerations.components/domain_group/actions/create-residential-listing/create-residential-listing.mjs (1)
116-149: Consider adding error handling to the run methodThe current implementation doesn't include explicit error handling. Adding try/catch blocks would improve user experience by providing more specific error messages when API calls fail.
async run({ $ }) { + try { const response = await this.domainGroup.createResidentialListing({ $, data: { domainAgencyID: this.agencyId, providerAdId: this.providerAdId, listingAction: this.listingAction, underOfferOrContract: this.underOfferOrContract, price: { from: this.fromPrice, to: this.toPrice, }, description: this.description, features: this.features, propertyDetails: { propertyType: [ this.propertyType, ], address: { streetNumber: this.streetNumber, unitNumber: this.unitNumber, street: this.street, state: this.state, suburb: this.suburb, postcode: this.postcode, }, }, receiveEmailsToDefaultAddress: this.receiveEmailsToDefaultAddress, isRural: this.isRural, }, }); $.export("$summary", `Created residential listing with ID: ${response.id}`); return response; + } catch (error) { + $.export("$summary", "Failed to create residential listing"); + throw error; + } },components/domain_group/actions/create-business-listing/create-business-listing.mjs (2)
130-130: Make type conversion more explicitUsing the unary plus operator (
+) works but is less explicit than alternatives likeNumber(). Consider using a more explicit conversion method for better readability.- nabers: this.nabers && +this.nabers, + nabers: this.nabers && Number(this.nabers),
122-156: Add error handling to the run methodSimilar to the residential listing component, consider adding explicit error handling for better user experience.
async run({ $ }) { + try { const response = await this.domainGroup.createBusinessListing({ $, data: { domainAgencyID: this.agencyId, providerAdId: this.providerAdId, listingAction: this.listingAction, underOfferOrContract: this.underOfferOrContract, nabers: this.nabers && +this.nabers, price: { from: this.fromPrice, to: this.toPrice, }, description: this.description, features: this.features, propertyDetails: { propertyType: [ this.propertyType, ], address: { streetNumber: this.streetNumber, unitNumber: this.unitNumber, street: this.street, state: this.state, suburb: this.suburb, postcode: this.postcode, }, }, receiveEmailsToDefaultAddress: this.receiveEmailsToDefaultAddress, isRural: this.isRural, }, }); $.export("$summary", `Created business listing with ID: ${response.id}`); return response; + } catch (error) { + $.export("$summary", "Failed to create business listing"); + throw error; + } },components/domain_group/domain_group.app.mjs (2)
45-50: Consider type consistency for nabers propertyThe
nabersproperty is defined as a string here but is converted to a number in the business listing component. It might be clearer to define it as a number type to begin with, to ensure type consistency across the application.nabers: { - type: "string", + type: "integer", label: "NABERS", description: "The NABERS Rating is the energy efficiency rating that the property has been measured to have. This rating is measured in increments of .5 and can range from 0 to 6. The NABERS rating is required for spaces within office buildings of 1000 square metres or more. For more information on the NABERS rating system please visit [http://www.nabers.gov.au](http://www.nabers.gov.au)", optional: true, },
157-177: Note API version difference for commercial listingsThe commercial listing endpoint uses API v2 while business and residential use v1. This appears intentional based on the documentation URL shown in the residential listing component, but it's worth documenting why there's a version difference.
Consider adding a comment above the createCommercialListing method explaining why it uses v2 instead of v1.
createCommercialListing(opts = {}) { + // Using v2 endpoint as required by Domain Group API for commercial listings return this._makeRequest({ method: "PUT", path: "/v2/listings/commercial", ...opts, }); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
components/domain_group/actions/create-business-listing/create-business-listing.mjs(1 hunks)components/domain_group/actions/create-commercial-listing/create-commercial-listing.mjs(1 hunks)components/domain_group/actions/create-residential-listing/create-residential-listing.mjs(1 hunks)components/domain_group/common/property-types.mjs(1 hunks)components/domain_group/domain_group.app.mjs(1 hunks)components/domain_group/package.json(2 hunks)components/domain_group/sources/agency-listing-updated/agency-listing-updated.mjs(1 hunks)components/domain_group/sources/common/base.mjs(1 hunks)components/domain_group/sources/new-agency-created/new-agency-created.mjs(1 hunks)components/domain_group/sources/new-agency-listing-created/new-agency-listing-created.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (16)
components/domain_group/package.json (2)
3-3: Version bump looks appropriate.The version update from 0.0.1 to 0.1.0 correctly follows semantic versioning principles for introducing new features without breaking existing functionality.
15-16: Dependencies addition is appropriate.Adding the @pipedream/platform dependency is necessary for the new components being introduced in this PR. The version constraint (^3.0.3) follows best practices by allowing compatible updates.
components/domain_group/sources/new-agency-created/new-agency-created.mjs (1)
3-29: Module structure looks good.The source module is well-structured with appropriate metadata, version, and description. The documentation link provides helpful context for users.
components/domain_group/common/property-types.mjs (4)
1-214: Comprehensive business types list.The BUSINESS_TYPES constant provides a thorough list of business categories for validation purposes, which will help ensure data consistency when creating business listings.
216-253: Well-defined residential property types.The RESIDENTIAL_TYPES constant appropriately defines the various residential property categories, which will be useful for creating residential listings.
255-282: Comprehensive commercial property types.The COMMERCIAL_TYPES constant includes a thorough list of commercial property categories, which provides good validation options for commercial listings.
284-288: Clean export structure.The exports are well-organized, making it easy for other modules to import specific constants as needed.
components/domain_group/sources/agency-listing-updated/agency-listing-updated.mjs (3)
3-11: Module metadata is well-defined.The source module has appropriate key, name, description, version, and type definitions. The documentation link is helpful for users who need more information.
11-19: Props structure looks good.The component properly extends the common props and adds the specific agencyId property that's needed for this particular source.
21-47: Well-implemented methods for tracking updates.The methods are well-structured for tracking listing updates:
- getResourceFn returns the correct API function
- getArgs properly handles filtering by update date
- getTsField specifies the correct timestamp field
- generateMeta creates a unique ID that includes both the listing ID and timestamp
This implementation will effectively detect and emit events for updated listings.
components/domain_group/sources/common/base.mjs (1)
24-26: Confirm usage of an empty args object
getArgs()currently always returns an empty object. If you plan to differentiate fetching logic based on user inputs or config in the future, consider adding placeholders or optional parameters to ensure future scalability.components/domain_group/sources/new-agency-listing-created/new-agency-listing-created.mjs (2)
3-19: Good reuse of the base module
Merging...commonprops and methods ensures DRY code. Confirm that you only override or extend where necessary, avoiding duplication.
20-46: Check parameter vs. timestamp field consistency
getTsField()returns "dateCreated", butgetArgs(lastTs)appliesdateUpdatedSinceinargs.params. If you intend to detect newly created listings by their creation timestamp, confirm that “dateUpdatedSince” is the correct query param for listing creation events.components/domain_group/actions/create-residential-listing/create-residential-listing.mjs (1)
1-150: Well-structured component with good organizationThe residential listing creation component follows good practices with proper separation of concerns, clear property definitions, and a straightforward implementation.
components/domain_group/domain_group.app.mjs (2)
6-126: Well-defined property definitions with clear labels and descriptionsThe property definitions are comprehensive with appropriate types, labels, and descriptions that will provide good guidance for users.
178-202:❓ Verification inconclusive
Verify the pagination logic works as expected with the API
The current pagination implementation assumes that when the returned items length is 0, there are no more pages. Verify that this is how the Domain Group API indicates the end of pagination.
🏁 Script executed:
#!/bin/bash # Check the Domain Group API documentation or sample responses to confirm pagination behavior # Use a single example endpoint to see if we can find documentation about their pagination approach curl -s "https://developer.domain.com.au/docs/latest" | grep -A 5 -B 5 "pagination"Length of output: 81
Manual verification required: Confirm API pagination behavior
The current pagination implementation in
components/domain_group/domain_group.app.mjs(lines 178‑202) assumes that an empty array returned by the API indicates there are no more pages. Since our initial attempt to retrieve documentation via the curl command produced no output, it’s unclear whether this assumption holds true for the Domain Group API.
- Please manually verify that the API returns an empty list (or another suitable indicator) when there are no additional items/pages.
- If possible, check actual API responses (e.g., via API manual testing) or consult updated API documentation to confirm that pagination ends when items length is 0.
Resolves #15747
Summary by CodeRabbit
New Features
Chores