- Notifications
You must be signed in to change notification settings - Fork 5.5k
[Enhancement] - Add Support for Fetching All HubSpot Contact Properties #18664
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?
[Enhancement] - Add Support for Fetching All HubSpot Contact Properties #18664
Conversation
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. |
WalkthroughAdds two helper methods to fetch contacts with all properties in the HubSpot app, updates package version, and introduces an optional fetchAllProperties path in two sources to batch-retrieve full contact data. Multiple other HubSpot sources receive version-only bumps without logic changes. Changes
Sequence Diagram(s)sequenceDiagram autonumber actor User participant Source as Source (new-contact-added-to-list) participant App as HubSpot App participant HS as HubSpot API User->>Source: Run with fetchAllProperties = true Source->>App: getContactProperties() App->>HS: List contact properties HS-->>App: Property list Source->>App: batchGetContactsWithAllProperties(contactIds, props) App->>HS: Batch read contacts (all properties) HS-->>App: Contacts with full properties App-->>Source: Results Source-->>User: Contacts map (all properties) sequenceDiagram autonumber actor User participant Source as Source (new-contact-property-change) participant App as HubSpot App participant HS as HubSpot API User->>Source: Poll for changes (fetchAllProperties = true) Source->>App: batchGetContactsWithAllProperties(ids) App->>HS: Batch read contacts (all properties) HS-->>App: Contacts Source->>App: batchGetObjects(ids, { properties, propertiesWithHistory }) App->>HS: Batch read properties + history HS-->>App: Properties + history App-->>Source: Results Source-->>User: Merged contacts with propertiesWithHistory Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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: 0
🧹 Nitpick comments (3)
components/hubspot/hubspot.app.mjs (1)
1617-1648
: Memoize property names to avoid extra API calls; consider long-URL risk on single GET; add empty-IDs guard.
- Cache the contact property names once per app instance to reduce latency and rate-limit pressure.
- Single-contact path builds a very long query string when orgs have many properties. Verify this won’t hit proxy/gateway URL limits; if risky, consider a batch-read fallback for single IDs later.
- Optional: short‑circuit when contactIds is empty.
Apply this diff to add memoization within the changed methods:
async getContactWithAllProperties({ contactId, ...opts }) { - const properties = await this.getContactProperties(); - const allPropertyNames = properties.map((prop) => prop.name); + if (!this._allContactPropertyNames) { + const properties = await this.getContactProperties(); + this._allContactPropertyNames = properties.map((prop) => prop.name); + } + const allPropertyNames = this._allContactPropertyNames; return this.makeRequest({ api: API_PATH.CRMV3, endpoint: `/objects/contacts/${contactId}`, params: { properties: allPropertyNames.join(","), }, ...opts, }); }, async batchGetContactsWithAllProperties({ contactIds, ...opts }) { - const properties = await this.getContactProperties(); - const allPropertyNames = properties.map((prop) => prop.name); + if (!this._allContactPropertyNames) { + const properties = await this.getContactProperties(); + this._allContactPropertyNames = properties.map((prop) => prop.name); + } + const allPropertyNames = this._allContactPropertyNames; + if (!contactIds?.length) { + return { results: [] }; + } return this.batchGetObjects({ objectType: "contacts", data: { inputs: contactIds.map((id) => ({ id, })), properties: allPropertyNames, }, ...opts, }); },If you want, we can later extract a small helper (e.g., getAllContactPropertyNames) and add an optional includeArchived flag. Based on PR objectives.
components/hubspot/sources/new-contact-property-change/new-contact-property-change.mjs (1)
122-152
: Parallelize the two batch requests and merge in O(n) with a Map.Reduces latency and avoids O(n^2) lookups on each chunk.
Apply this refactor within batchGetContacts:
- if (this.fetchAllProperties) { - const { results } = await this.hubspot.batchGetContactsWithAllProperties({ - contactIds: inputs.map((input) => input.id), - }); - - const { results: contactsWithHistory } = await this.hubspot.batchGetObjects({ - objectType: "contacts", - data: { - properties: [ - this.property, - ], - propertiesWithHistory: [ - this.property, - ], - inputs, - }, - }); - - const mergedResults = results.map((contact) => { - const contactWithHistory = contactsWithHistory.find((c) => c.id === contact.id); - return { - ...contact, - propertiesWithHistory: contactWithHistory?.propertiesWithHistory || {}, - }; - }); - - return { - results: mergedResults, - }; - } + if (this.fetchAllProperties) { + const ids = inputs.map((input) => input.id); + const [allPropsResp, historyResp] = await Promise.all([ + this.hubspot.batchGetContactsWithAllProperties({ contactIds: ids }), + this.hubspot.batchGetObjects({ + objectType: "contacts", + data: { + properties: [ this.property ], + propertiesWithHistory: [ this.property ], + inputs, + }, + }), + ]); + + const historyById = new Map( + (historyResp.results || []).map((c) => [c.id, c]), + ); + const mergedResults = (allPropsResp.results || []).map((contact) => ({ + ...contact, + propertiesWithHistory: historyById.get(contact.id)?.propertiesWithHistory || {}, + })); + return { results: mergedResults }; + }components/hubspot/sources/new-contact-added-to-list/new-contact-added-to-list.mjs (1)
106-138
: Deduplicate contact IDs before chunking
- Use
Array.from(new Set(contactIds))
to prevent duplicate API calls.- Optional: leverage existing
processChunks
helper for unified error handling/backoff.Minimal diff:
if (this.fetchAllProperties) { - const chunks = []; + const uniqueIds = Array.from(new Set(contactIds)); + const chunks = []; const chunkSize = 100; - for (let i = 0; i < contactIds.length; i += chunkSize) { - chunks.push(contactIds.slice(i, i + chunkSize)); + for (let i = 0; i < uniqueIds.length; i += chunkSize) { + chunks.push(uniqueIds.slice(i, i + chunkSize)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
components/hubspot/hubspot.app.mjs
(1 hunks)components/hubspot/package.json
(1 hunks)components/hubspot/sources/delete-blog-article/delete-blog-article.mjs
(1 hunks)components/hubspot/sources/new-company-property-change/new-company-property-change.mjs
(1 hunks)components/hubspot/sources/new-contact-added-to-list/new-contact-added-to-list.mjs
(3 hunks)components/hubspot/sources/new-contact-property-change/new-contact-property-change.mjs
(3 hunks)components/hubspot/sources/new-custom-object-property-change/new-custom-object-property-change.mjs
(1 hunks)components/hubspot/sources/new-deal-in-stage/new-deal-in-stage.mjs
(1 hunks)components/hubspot/sources/new-deal-property-change/new-deal-property-change.mjs
(1 hunks)components/hubspot/sources/new-email-event/new-email-event.mjs
(1 hunks)components/hubspot/sources/new-email-subscriptions-timeline/new-email-subscriptions-timeline.mjs
(1 hunks)components/hubspot/sources/new-engagement/new-engagement.mjs
(1 hunks)components/hubspot/sources/new-event/new-event.mjs
(1 hunks)components/hubspot/sources/new-form-submission/new-form-submission.mjs
(1 hunks)components/hubspot/sources/new-note/new-note.mjs
(1 hunks)components/hubspot/sources/new-or-updated-blog-article/new-or-updated-blog-article.mjs
(1 hunks)components/hubspot/sources/new-or-updated-company/new-or-updated-company.mjs
(1 hunks)components/hubspot/sources/new-or-updated-contact/new-or-updated-contact.mjs
(1 hunks)components/hubspot/sources/new-or-updated-crm-object/new-or-updated-crm-object.mjs
(1 hunks)components/hubspot/sources/new-or-updated-custom-object/new-or-updated-custom-object.mjs
(1 hunks)components/hubspot/sources/new-or-updated-deal/new-or-updated-deal.mjs
(1 hunks)components/hubspot/sources/new-or-updated-line-item/new-or-updated-line-item.mjs
(1 hunks)components/hubspot/sources/new-or-updated-product/new-or-updated-product.mjs
(1 hunks)components/hubspot/sources/new-social-media-message/new-social-media-message.mjs
(1 hunks)components/hubspot/sources/new-task/new-task.mjs
(1 hunks)components/hubspot/sources/new-ticket-property-change/new-ticket-property-change.mjs
(1 hunks)components/hubspot/sources/new-ticket/new-ticket.mjs
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
components/hubspot/sources/new-contact-property-change/new-contact-property-change.mjs (4)
components/hubspot/sources/new-company-property-change/new-company-property-change.mjs (1)
results
(117-120)components/hubspot/sources/new-custom-object-property-change/new-custom-object-property-change.mjs (1)
results
(123-126)components/hubspot/sources/new-deal-property-change/new-deal-property-change.mjs (1)
results
(114-117)components/hubspot/sources/new-ticket-property-change/new-ticket-property-change.mjs (1)
results
(118-121)
components/hubspot/hubspot.app.mjs (1)
components/hubspot/sources/new-contact-property-change/new-contact-property-change.mjs (2)
properties
(21-21)properties
(173-173)
components/hubspot/sources/new-contact-added-to-list/new-contact-added-to-list.mjs (1)
components/hubspot/sources/new-contact-property-change/new-contact-property-change.mjs (2)
results
(191-194)contact
(60-62)
⏰ 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 (19)
components/hubspot/sources/new-or-updated-company/new-or-updated-company.mjs (1)
13-13
: Version bump LGTMIncrementing the source version to
0.0.22
keeps the metadata aligned with the release cadence. No other changes observed.components/hubspot/sources/new-task/new-task.mjs (1)
12-12
: Verify the necessity of this version bump.This file only receives a version bump from "1.0.15" to "1.0.16" with no functional changes. Since the PR's main functionality (fetching all contact properties) targets contact-related sources and not task-related sources, clarify whether this version increment is necessary or if it's part of a broader version alignment strategy across all HubSpot sources.
components/hubspot/sources/new-or-updated-deal/new-or-updated-deal.mjs (1)
13-13
: Clarify the rationale for this version bump.This source has no functional changes, yet its version is incremented from "0.0.21" to "0.0.22". The PR focuses on adding contact property fetching capabilities to specific contact-related sources, which does not appear to affect this deals source.
Version increments typically signal functional changes, bug fixes, or dependency updates. Bumping versions without corresponding changes can confuse users reviewing version history and may indicate unnecessary coupling across independent modules.
Please clarify:
- Is there a policy requiring version synchronization across all HubSpot sources?
- If so, what is the rationale for this approach?
- If not, should this version bump be reverted since there are no material changes to this source?
If version synchronization is intentional, consider documenting this practice in the repository to help contributors understand the versioning strategy.
components/hubspot/sources/new-company-property-change/new-company-property-change.mjs (1)
10-10
: LGTM!The version bump aligns with the broader metadata update pattern across HubSpot source modules in this PR.
components/hubspot/sources/new-or-updated-blog-article/new-or-updated-blog-article.mjs (1)
10-10
: LGTM!The version bump aligns with the broader metadata update pattern across HubSpot source modules in this PR.
components/hubspot/sources/new-deal-in-stage/new-deal-in-stage.mjs (1)
16-16
: LGTM!The version bump aligns with the broader metadata update pattern across HubSpot source modules in this PR.
components/hubspot/sources/new-deal-property-change/new-deal-property-change.mjs (1)
10-10
: Verify the version jump.The version increment from 0.0.28 to 0.0.30 skips 0.0.29. This could be intentional if 0.0.29 was used in another branch or release, but please confirm that this is correct to ensure version consistency.
components/hubspot/sources/new-engagement/new-engagement.mjs (1)
11-11
: LGTM!The version bump aligns with the broader metadata update pattern across HubSpot source modules in this PR.
components/hubspot/sources/new-or-updated-custom-object/new-or-updated-custom-object.mjs (1)
10-10
: LGTM!The version bump aligns with the broader metadata update pattern across HubSpot source modules in this PR.
components/hubspot/sources/new-event/new-event.mjs (1)
11-11
: LGTM!The version bump aligns with the broader metadata update pattern across HubSpot source modules in this PR.
components/hubspot/sources/new-ticket-property-change/new-ticket-property-change.mjs (1)
11-11
: LGTM!The version bump aligns with the broader metadata update pattern across HubSpot source modules in this PR.
components/hubspot/sources/new-social-media-message/new-social-media-message.mjs (1)
10-10
: Version bump looks good.No logic changes; safe to publish.
components/hubspot/sources/new-email-event/new-email-event.mjs (1)
11-11
: Version bump LGTM.No behavioral changes detected.
components/hubspot/sources/new-or-updated-crm-object/new-or-updated-crm-object.mjs (1)
10-10
: Version bump LGTM.No code path changes.
components/hubspot/package.json (1)
3-3
: Package version bump OK.Ensure release notes/changelog reflect the new helpers.
components/hubspot/sources/new-contact-added-to-list/new-contact-added-to-list.mjs (2)
15-15
: Version bump looks good.
58-65
: Good addition: opt‑in fetchAllProperties with sane default.Consistent with other sources; no breaking changes.
components/hubspot/sources/new-contact-property-change/new-contact-property-change.mjs (2)
11-11
: Version bump looks good.
41-48
: Prop design LGTM.Opt‑in flag, clear description, default false. Matches PR objective.
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.
Thanks for the contribution @choeqq , looks good, just one comment that might need to be changed
async getContactWithAllProperties({ | ||
contactId, ...opts | ||
}) { | ||
const properties = await this.getContactProperties(); | ||
const allPropertyNames = properties.map((prop) => prop.name); | ||
| ||
return this.makeRequest({ | ||
api: API_PATH.CRMV3, | ||
endpoint: `/objects/contacts/${contactId}`, | ||
params: { | ||
properties: allPropertyNames.join(","), | ||
}, | ||
...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.
Is this one being called anywhere? It seems only the batch method is being used
Summary
Added optional
fetchAllProperties
prop tonew-contact-property-change
andnew-contact-added-to-list
sources. When enabled, fetches all available contact properties instead of justdefault/selected ones.
Changes
getContactWithAllProperties()
andbatchGetContactsWithAllProperties()
methods tohubspot.app.mjs
fetchAllProperties
boolean prop (default:false
) to both sourcesUse Case
Enables syncing complete contact records (including custom properties) without manual configuration, useful for external system integrations and workflows requiring dynamic fields.
Summary by CodeRabbit