-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Adjust hubspot actions reloadProps #18470
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
Removing reloadProps from the app file for actions for which this is irrelevant
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughRefactors HubSpot actions to use a direct hubspot prop instead of appProp.props, updates propDefinitions accordingly, and adjusts common create logic to call methods via this.hubspot. Adds getObjectType in create-custom-object. Multiple action files increment version strings. Bumps package version in components/hubspot/package.json. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Action as Action (Create/Update)
participant HSApp as HubSpot App (hubspot prop)
participant API as HubSpot API
User->>Action: Trigger run()
Note over Action: Access props via `hubspot`<br/>Resolve object type/id
Action->>HSApp: createObject / updateObject(props)
HSApp->>API: REST call with payload
API-->>HSApp: Response (success/error)
HSApp-->>Action: Result
Action-->>User: Emit outcome
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/hubspot/actions/update-custom-object/update-custom-object.mjs (1)
19-34: Replace remaining appProp references and audit reloadProps
- components/hubspot/actions/common/common-create.mjs: drop the
import appPropand spread ofappProp.props, referencehubspotdirectly.- components/hubspot/actions/create-communication/create-communication.mjs: replace
appProp.props.hubspotwithhubspotand remove theappPropimport.- Audit all
reloadPropsusages to confirm they’re only set on props that gate downstream options.
🧹 Nitpick comments (5)
components/hubspot/actions/common/common-create-object.mjs (2)
48-59: Avoid name shadowing ofhubspot; usethis.hubspotconsistently.Reduces confusion between the imported prop def and the runtime instance.
Apply:
@@ async run({ $ }) { const { - hubspot, /* eslint-disable no-unused-vars */ propertyGroups, customObjectType, contactId, $db, updateIfExists, objectProperties, ...otherProperties } = this; @@ - const objectName = hubspot.getObjectTypeName(objectType); + const objectName = this.hubspot.getObjectTypeName(objectType); $.export("$summary", `Successfully created ${objectName}`); @@ - const response = await hubspot.updateObject({ + const response = await this.hubspot.updateObject({ $, objectType, objectId, data: { properties, }, }); - const objectName = hubspot.getObjectTypeName(objectType); + const objectName = this.hubspot.getObjectTypeName(objectType); $.export("$summary", `Successfully updated ${objectName}`);Also applies to: 85-86, 94-101, 102-104
89-107: Harden error parsing and guard objectId extraction in update-on-conflict path.Avoids losing the original error on non‑JSON messages and prevents invalid update calls.
Apply:
- if (updateIfExists && err?.message) { - const errorObj = JSON.parse(err?.message); - if (errorObj?.category === "CONFLICT" || errorObj?.category === "OBJECT_ALREADY_EXISTS") { - const objectId = parseInt(errorObj.message.replace(/[^\d]/g, "")); - const response = await this.hubspot.updateObject({ + if (updateIfExists && err?.message) { + try { + const errorObj = JSON.parse(err.message); + const isConflict = ["CONFLICT", "OBJECT_ALREADY_EXISTS"].includes(errorObj?.category); + const objectIdFromMsg = parseInt(String(errorObj?.message || "").replace(/[^\d]/g, ""), 10); + const objectId = Number.isFinite(objectIdFromMsg) ? objectIdFromMsg : undefined; + if (isConflict && objectId) { + const response = await this.hubspot.updateObject({ $, objectType, objectId, data: { properties, }, }); - const objectName = this.hubspot.getObjectTypeName(objectType); - $.export("$summary", `Successfully updated ${objectName}`); - return response; - } + const objectName = this.hubspot.getObjectTypeName(objectType); + $.export("$summary", `Successfully updated ${objectName}`); + return response; + } + } catch (_) { + // fall through to throw original error + } }components/hubspot/actions/create-ticket/create-ticket.mjs (1)
20-33: Align propDefinition to the new hubspot prop pattern.Other updated actions now pass the local
hubspotprop (notcommon.props.hubspot) topropDefinition. For consistency and to simplify future refactors, adopt the same pattern used in create-lead.Example pattern (outside this range):
- Destructure once:
const { hubspot, ...otherProps } = common.props- In props:
props: { hubspot, ...otherProps, ... }- Then update:
propDefinition: [hubspot, "ticketPipeline"]
propDefinition: [hubspot, "ticketStage", (c) => ({ pipelineId: c.hs_pipeline }))components/hubspot/actions/update-contact/update-contact.mjs (1)
20-29: Use the existing constant instead of a string for objectTypeFor consistency and to avoid magic strings, prefer OBJECT_TYPE.CONTACT in the objectId propDefinition.
Apply:
propDefinition: [ hubspot, "objectId", - () => ({ - objectType: "contact", - }), + () => ({ + objectType: OBJECT_TYPE.CONTACT, + }), ],components/hubspot/actions/create-custom-object/create-custom-object.mjs (1)
16-24: Props arrangement looks good; minor consistency note
- Including hubspot before ...otherProps avoids collisions.
- Optional: for cross-file consistency, consider standardizing on one pattern (destructuring from common.props vs. direct hubspot import) across HubSpot actions in a follow-up sweep.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
components/hubspot/actions/common/common-create-object.mjs(1 hunks)components/hubspot/actions/create-company/create-company.mjs(1 hunks)components/hubspot/actions/create-custom-object/create-custom-object.mjs(1 hunks)components/hubspot/actions/create-deal/create-deal.mjs(1 hunks)components/hubspot/actions/create-lead/create-lead.mjs(2 hunks)components/hubspot/actions/create-or-update-contact/create-or-update-contact.mjs(1 hunks)components/hubspot/actions/create-ticket/create-ticket.mjs(1 hunks)components/hubspot/actions/update-company/update-company.mjs(3 hunks)components/hubspot/actions/update-contact/update-contact.mjs(3 hunks)components/hubspot/actions/update-custom-object/update-custom-object.mjs(3 hunks)components/hubspot/actions/update-deal/update-deal.mjs(3 hunks)components/hubspot/actions/update-lead/update-lead.mjs(3 hunks)components/hubspot/package.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
components/hubspot/actions/create-lead/create-lead.mjs (1)
components/hubspot/actions/create-custom-object/create-custom-object.mjs (1)
otherProps(3-5)
components/hubspot/actions/create-custom-object/create-custom-object.mjs (1)
components/hubspot/actions/create-lead/create-lead.mjs (1)
otherProps(6-8)
⏰ 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: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
🔇 Additional comments (15)
components/hubspot/actions/create-deal/create-deal.mjs (1)
10-10: Version bump looks good.Consistent with the package bump and other actions in this PR.
components/hubspot/package.json (1)
3-3: Package version bump acknowledged.Aligned with action version increments.
components/hubspot/actions/create-or-update-contact/create-or-update-contact.mjs (1)
10-10: Version bump looks good.No functional changes introduced here.
components/hubspot/actions/create-company/create-company.mjs (1)
10-10: Version bump looks good.In step with related action modules.
components/hubspot/actions/create-ticket/create-ticket.mjs (1)
10-10: Version bump LGTM.No functional changes here.
components/hubspot/actions/update-deal/update-deal.mjs (1)
2-2: HubSpot prop migration + version bump: LGTM.
hubspotimport/prop usage andobjectIdpropDefinition with{ objectType: "deal" }are correct and consistent.Optionally verify consistency with a quick scan (same script as in update-company comment).
Also applies to: 11-11, 20-29
components/hubspot/actions/create-lead/create-lead.mjs (2)
6-8: Destructuring hubspot from common.props: LGTM.This matches the new standardized pattern used across actions.
16-16: Prop wiring to hubspot + objectId for Contact: LGTM.
propDefinition: [hubspot, "objectId", { objectType: "contact" }]and inclusion ofhubspotinpropsare correct.Also applies to: 19-31
components/hubspot/actions/update-lead/update-lead.mjs (1)
2-2: HubSpot prop migration + version bump: LGTM
hubspot.app.mjsdefines bothleadId(line 422) andobjectId, sopropDefinition: [hubspot, "leadId"]is correct.components/hubspot/actions/update-custom-object/update-custom-object.mjs (2)
1-1: Direct hubspot app import — LGTMSwitching from appProp to a direct hubspot import aligns with the newer pattern and simplifies prop definitions.
10-10: Version bump acknowledgedVersion updated to 1.0.12.
components/hubspot/actions/update-contact/update-contact.mjs (2)
2-2: Direct hubspot app import — LGTMConsistent with the refactor across actions.
11-11: Version bump acknowledgedVersion updated to 0.0.27.
components/hubspot/actions/create-custom-object/create-custom-object.mjs (2)
3-5: Destructuring hubspot from common.props — LGTMCleanly avoids duplicating the hubspot prop while reusing the rest of common props.
13-13: Version bump acknowledgedVersion updated to 1.0.12.
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 @GTFalcao, LGTM! Ready for QA!
|
Hi everyone, all test cases are passed! Ready for release! Test report |
These actions only show additional props that were selected in "propertyGroups", so calling reloadProps in the app file (when propertyGroups has not been selected yet) is irrelevant
Summary by CodeRabbit
New Features
Refactor
Chores