-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Dealcloud components initial merge #19233
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 GitHub. 2 Skipped Deployments
|
WalkthroughAdds a DealCloud integration: CRUD action modules, a shared create/update helper, field-to-prop utilities and currency constants, an HTTP-backed app API layer using axios, and package metadata changes (version bump and dependency). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Action as Action (create/update/get/delete)
participant Base as common-create-update
participant App as dealcloud.app
participant API as DealCloud API
User->>Action: invoke action with props
alt create/update uses field metadata
Action->>Base: additionalProps()
Base->>App: getEntryTypeFields(entryTypeId)
App->>API: GET /entry-type/{id}/fields
API-->>App: fields
App-->>Base: fields
Base->>Base: convertFieldsToProps(fields)
Base-->>Action: augmented props
end
Action->>Base: getRequestData(props)
Base-->>Action: storeRequests payload
Action->>App: createEntry/updateEntry/queryEntries/deleteEntry(...)
App->>API: REST request (POST/PUT/GET/DELETE)
API-->>App: response
App-->>Action: response
Action->>User: return response and export $summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
components/dealcloud/actions/common/common-create-update.mjs(1 hunks)components/dealcloud/actions/create-record/create-record.mjs(1 hunks)components/dealcloud/actions/delete-records/delete-records.mjs(1 hunks)components/dealcloud/actions/get-records/get-records.mjs(1 hunks)components/dealcloud/actions/update-record/update-record.mjs(1 hunks)components/dealcloud/common/constants.mjs(1 hunks)components/dealcloud/common/utils.mjs(1 hunks)components/dealcloud/dealcloud.app.mjs(1 hunks)components/dealcloud/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
Repo: PipedreamHQ/pipedream PR: 14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
Applied to files:
components/dealcloud/package.json
📚 Learning: 2024-10-30T15:24:39.294Z
Learnt from: jcortes
Repo: PipedreamHQ/pipedream PR: 14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In `components/gainsight_px/actions/create-account/create-account.mjs`, the action name should be "Create Account" instead of "Create Memory".
Applied to files:
components/dealcloud/actions/create-record/create-record.mjs
📚 Learning: 2025-06-04T17:52:05.780Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 16954
File: components/salesloft/salesloft.app.mjs:14-23
Timestamp: 2025-06-04T17:52:05.780Z
Learning: In the Salesloft API integration (components/salesloft/salesloft.app.mjs), the _makeRequest method returns response.data which directly contains arrays for list endpoints like listPeople, listCadences, listUsers, and listAccounts. The propDefinitions correctly call .map() directly on these responses without needing to destructure a nested data property.
Applied to files:
components/dealcloud/dealcloud.app.mjs
📚 Learning: 2025-09-15T22:01:11.472Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 18362
File: components/leonardo_ai/actions/generate-image/generate-image.mjs:103-105
Timestamp: 2025-09-15T22:01:11.472Z
Learning: In Pipedream components, pipedream/platform's axios implementation automatically excludes undefined values from HTTP requests, so there's no need to manually check for truthiness before including properties in request payloads.
Applied to files:
components/dealcloud/dealcloud.app.mjs
🧬 Code graph analysis (6)
components/dealcloud/actions/update-record/update-record.mjs (3)
components/dealcloud/actions/common/common-create-update.mjs (1)
props(47-47)components/dealcloud/actions/create-record/create-record.mjs (2)
props(17-26)response(29-33)components/dealcloud/actions/delete-records/delete-records.mjs (1)
response(38-42)
components/dealcloud/common/utils.mjs (2)
components/dealcloud/dealcloud.app.mjs (1)
value(30-30)components/dealcloud/common/constants.mjs (2)
CURRENCY_OPTIONS(1-154)CURRENCY_OPTIONS(1-154)
components/dealcloud/actions/common/common-create-update.mjs (3)
components/dealcloud/actions/create-record/create-record.mjs (1)
props(17-26)components/dealcloud/actions/update-record/update-record.mjs (1)
props(46-56)components/dealcloud/dealcloud.app.mjs (1)
value(30-30)
components/dealcloud/actions/get-records/get-records.mjs (2)
components/dealcloud/actions/delete-records/delete-records.mjs (3)
entryIds(37-37)response(38-42)count(44-44)components/dealcloud/common/utils.mjs (1)
checkIdArray(4-12)
components/dealcloud/dealcloud.app.mjs (1)
components/gmail/gmail.app.mjs (1)
axios(415-423)
components/dealcloud/actions/delete-records/delete-records.mjs (4)
components/dealcloud/actions/get-records/get-records.mjs (3)
entryIds(37-37)response(38-46)count(48-48)components/dealcloud/common/utils.mjs (1)
checkIdArray(4-12)components/dealcloud/actions/create-record/create-record.mjs (1)
response(29-33)components/dealcloud/actions/update-record/update-record.mjs (1)
response(59-63)
⏰ 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: Verify TypeScript components
- GitHub Check: pnpm publish
🔇 Additional comments (15)
components/dealcloud/package.json (1)
3-17: LGTM!Version bump to 0.1.0 is appropriate for introducing new functionality. The
@pipedream/platformdependency is correctly declared and supports the axios-based API layer used throughout the new actions.components/dealcloud/common/constants.mjs (1)
1-154: LGTM - comprehensive currency list.The list includes some currencies that have been replaced by the Euro (EEK, LTL, LVL, NLG). If this mirrors DealCloud's supported currencies, that's fine. Otherwise, consider removing obsolete codes.
components/dealcloud/dealcloud.app.mjs (2)
6-46: Well-structured prop definitions.Good fallback chain in
entryIdoptions (EntryId || Id || idandName || value) to handle different response shapes gracefully.
75-117: LGTM - API methods are well implemented.The CRUD methods follow a consistent pattern, properly pass through additional args, and use correct HTTP methods.
components/dealcloud/actions/update-record/update-record.mjs (2)
1-43: Clean implementation extending the common base.The component correctly inherits from
commonCreateUpdate, overridesisUpdate()to returntrue, and properly sets up theentryIdprop with dependency onentryTypeId.
59-63: The review comment is incorrect.ignoreNearDupsis properly used.The
ignoreNearDupsprop flows to the API through thegetRequestData()method (defined in the common-create-update base module). This method constructs the storeRequests array, and each request includesignoreNearDupsas part of the payload sent via thedataparameter. The prop is not unused; it's accessed throughthis.ignoreNearDupsinsidegetRequestData(), which is why it's safe to destructure it and mark it witheslint-disable no-unused-vars.components/dealcloud/actions/delete-records/delete-records.mjs (1)
1-14: LGTM - appropriate metadata for a destructive action.The
destructiveHint: trueannotation correctly signals that this action deletes data.components/dealcloud/actions/get-records/get-records.mjs (2)
36-46: Potential type inconsistency with ID array.
checkIdArrayreturns strings when splitting a comma-separated input (line 6 in utils.mjs), but the prop is typed asinteger[]. This could cause issues if the DealCloud API expects numeric IDs in the query.Consider coercing to integers:
- const entryIds = checkIdArray(this.entryIds); + const entryIds = checkIdArray(this.entryIds).map(Number);
1-14: LGTM!The action metadata, imports, and annotations are well-structured and follow Pipedream conventions.
components/dealcloud/common/utils.mjs (2)
36-49: Boolean array type not handled.If
fieldTypeIdisBOOLEANandisArrayistrue, this returns"boolean"rather than"boolean[]". Verify if this is intentional (DealCloud doesn't support boolean arrays) or if it needs handling.
14-28: LGTM!The
FIELD_TYPESenum is well-defined and covers the necessary DealCloud field types.components/dealcloud/actions/create-record/create-record.mjs (2)
15-27: Consider extracting common destructuring pattern.The destructuring with eslint-disable comments is repeated in both create and update actions. This works but is somewhat verbose.
29-38: LGTM!The API call and response handling are well-structured. The TODO comment about enhancing the summary with the record ID is appropriate.
components/dealcloud/actions/common/common-create-update.mjs (2)
46-55: LGTM!The
additionalPropsmethod correctly handles the case whenentryTypeIdis not set and properly chains the field fetching with prop conversion.
5-12: LGTM!The base methods
isUpdate()andgetEntryId()provide sensible defaults that can be overridden by extending actions.
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
♻️ Duplicate comments (1)
components/dealcloud/actions/get-records/get-records.mjs (1)
48-51: Use the actual response length for the summary (not requested IDs)Right now the summary uses
entryIds.length, which counts requested IDs, not the number of records actually returned. If some IDs are missing, the summary will be misleading.You can base the count on the response array instead:
- const count = entryIds.length; - $.export("$summary", `Successfully retrieved ${count} record${count === 1 + const count = response?.length ?? 0; + $.export("$summary", `Successfully retrieved ${count} record${count === 1 ? "" : "s"}`);This keeps the summary format consistent with prior guidance while accurately reflecting retrieved records. Based on learnings, …
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
components/dealcloud/actions/create-record/create-record.mjs(1 hunks)components/dealcloud/actions/delete-records/delete-records.mjs(1 hunks)components/dealcloud/actions/get-records/get-records.mjs(1 hunks)components/dealcloud/actions/update-record/update-record.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-08T15:33:38.240Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-10-08T15:33:38.240Z
Learning: When exporting a summary message in the `run` method of an action, ensure the message is correctly formatted. For example, in the `hackerone-get-members` action, the correct format is `Successfully retrieved ${response.data.length} members`.
Applied to files:
components/dealcloud/actions/get-records/get-records.mjscomponents/dealcloud/actions/delete-records/delete-records.mjs
📚 Learning: 2024-10-30T15:24:39.294Z
Learnt from: jcortes
Repo: PipedreamHQ/pipedream PR: 14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In `components/gainsight_px/actions/create-account/create-account.mjs`, the action name should be "Create Account" instead of "Create Memory".
Applied to files:
components/dealcloud/actions/create-record/create-record.mjs
🧬 Code graph analysis (2)
components/dealcloud/actions/delete-records/delete-records.mjs (4)
components/dealcloud/actions/get-records/get-records.mjs (3)
entryIds(37-37)response(38-46)count(48-48)components/dealcloud/common/utils.mjs (1)
checkIdArray(4-12)components/dealcloud/actions/create-record/create-record.mjs (1)
response(29-33)components/dealcloud/actions/update-record/update-record.mjs (1)
response(59-63)
components/dealcloud/actions/create-record/create-record.mjs (4)
components/dealcloud/actions/update-record/update-record.mjs (3)
commonCreateUpdate(3-7)props(46-56)response(59-63)components/dealcloud/actions/common/common-create-update.mjs (1)
props(47-47)components/dealcloud/actions/delete-records/delete-records.mjs (1)
response(38-42)components/dealcloud/actions/get-records/get-records.mjs (1)
response(38-46)
⏰ 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: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (4)
components/dealcloud/actions/delete-records/delete-records.mjs (3)
8-8: Verify the version string change.The version is set to
"0.0.1", but based on past review comments, it was previously"0.0.{{ts}}". The template format with{{ts}}is a Pipedream convention that auto-replaces with a timestamp for versioning. Changing to a fixed version may break the automated versioning workflow.Confirm whether the fixed version
"0.0.1"is intentional for this initial merge, or if it should be reverted to the template format.
15-35: Props definition looks good.The props are well-structured with appropriate use of
propDefinitionand theinteger[]type override for multiple record selection. The dynamic parameter passing forentryTypeIddependency is correctly implemented.
44-47: Summary message format is correct.The summary message follows the correct formatting pattern with proper pluralization, consistent with the learnings from previous reviews.
Based on learnings, the format
Successfully deleted ${count} record${count === 1 ? "" : "s"}is appropriate.components/dealcloud/actions/update-record/update-record.mjs (1)
9-20: Overall update action structure and commonCreateUpdate integration look soundThe override of
key/name/description, theentryIdpropDefinition keyed offentryTypeId, and theisUpdate()/getEntryId()methods all align with the sharedcommonCreateUpdatepattern and should letgetRequestDataconstruct an appropriate update payload.No other functional issues spotted here.
Also applies to: 24-43, 59-67
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
♻️ Duplicate comments (2)
components/dealcloud/dealcloud.app.mjs (1)
61-66: Inconsistent URL path handling - leading slashes cause incorrect endpoints.URLs on lines 63, 71, 79, and 87 start with
/which will cause axios to resolve against the origin, ignoring thebaseURLpath. Lines 96, 105, and 114 correctly omit the leading slash.Remove leading slashes from all URL paths for consistency:
async listEntryTypes(args = {}) { return this._makeRequest({ - url: "/schema/entrytypes", + url: "schema/entrytypes", ...args, }); },components/dealcloud/common/utils.mjs (1)
73-83: Documentthisbinding requirement forconvertFieldsToProps.This function uses
this.isUpdate()(line 80), which requires it to be called as a method on an object that definesisUpdate(). While this works correctly when re-exported incommon-create-update.mjs, the implicit dependency isn't obvious.Consider adding a JSDoc note:
/** * Converts DealCloud fields to Pipedream prop format + * @this {Object} Must be called with an object context that provides isUpdate() method * @param {DealCloudField[]} fields - Array of DealCloud field objects * @returns Object to be returned in dynamic props */ export function convertFieldsToProps(fields) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
components/dealcloud/actions/common/common-create-update.mjs(1 hunks)components/dealcloud/actions/delete-records/delete-records.mjs(1 hunks)components/dealcloud/actions/get-records/get-records.mjs(1 hunks)components/dealcloud/common/utils.mjs(1 hunks)components/dealcloud/dealcloud.app.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-10-08T15:33:38.240Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-10-08T15:33:38.240Z
Learning: When exporting a summary message in the `run` method of an action, ensure the message is correctly formatted. For example, in the `hackerone-get-members` action, the correct format is `Successfully retrieved ${response.data.length} members`.
Applied to files:
components/dealcloud/actions/get-records/get-records.mjscomponents/dealcloud/actions/delete-records/delete-records.mjs
📚 Learning: 2025-06-04T17:52:05.780Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 16954
File: components/salesloft/salesloft.app.mjs:14-23
Timestamp: 2025-06-04T17:52:05.780Z
Learning: In the Salesloft API integration (components/salesloft/salesloft.app.mjs), the _makeRequest method returns response.data which directly contains arrays for list endpoints like listPeople, listCadences, listUsers, and listAccounts. The propDefinitions correctly call .map() directly on these responses without needing to destructure a nested data property.
Applied to files:
components/dealcloud/dealcloud.app.mjs
📚 Learning: 2025-09-15T22:01:11.472Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 18362
File: components/leonardo_ai/actions/generate-image/generate-image.mjs:103-105
Timestamp: 2025-09-15T22:01:11.472Z
Learning: In Pipedream components, pipedream/platform's axios implementation automatically excludes undefined values from HTTP requests, so there's no need to manually check for truthiness before including properties in request payloads.
Applied to files:
components/dealcloud/dealcloud.app.mjs
🧬 Code graph analysis (5)
components/dealcloud/actions/get-records/get-records.mjs (5)
components/dealcloud/actions/delete-records/delete-records.mjs (2)
entryIds(37-37)response(38-42)components/dealcloud/common/utils.mjs (1)
checkIdArray(4-13)components/dealcloud/dealcloud.app.mjs (1)
response(86-89)components/dealcloud/actions/update-record/update-record.mjs (1)
response(59-63)components/dealcloud/actions/create-record/create-record.mjs (1)
response(29-33)
components/dealcloud/dealcloud.app.mjs (5)
components/gmail/gmail.app.mjs (1)
axios(415-423)components/dealcloud/actions/delete-records/delete-records.mjs (1)
response(38-42)components/dealcloud/actions/get-records/get-records.mjs (1)
response(38-46)components/dealcloud/actions/update-record/update-record.mjs (1)
response(59-63)components/dealcloud/actions/create-record/create-record.mjs (1)
response(29-33)
components/dealcloud/common/utils.mjs (2)
components/dealcloud/actions/common/common-create-update.mjs (1)
fields(51-53)components/dealcloud/common/constants.mjs (2)
CURRENCY_OPTIONS(1-154)CURRENCY_OPTIONS(1-154)
components/dealcloud/actions/common/common-create-update.mjs (2)
components/dealcloud/actions/update-record/update-record.mjs (1)
props(46-56)components/dealcloud/actions/create-record/create-record.mjs (1)
props(17-26)
components/dealcloud/actions/delete-records/delete-records.mjs (5)
components/dealcloud/actions/get-records/get-records.mjs (3)
entryIds(37-37)response(38-46)count(48-48)components/dealcloud/common/utils.mjs (1)
checkIdArray(4-13)components/dealcloud/dealcloud.app.mjs (1)
response(86-89)components/dealcloud/actions/update-record/update-record.mjs (1)
response(59-63)components/dealcloud/actions/create-record/create-record.mjs (1)
response(29-33)
⏰ 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: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (6)
components/dealcloud/dealcloud.app.mjs (1)
48-60: LGTM - Clean HTTP wrapper implementation.The
_makeRequestmethod properly configures axios with baseURL, authorization header, and allows flexible argument spreading.components/dealcloud/actions/get-records/get-records.mjs (2)
48-52: LGTM - Summary uses actual response length.The summary correctly reflects the number of records actually returned by the API rather than the number of IDs requested.
37-46: Add.map(Number)for consistency with delete-records action.The
delete-recordsaction applies.map(Number)to coerce IDs, but this action doesn't. This inconsistency could cause issues ifcheckIdArrayreturns strings from a comma-separated input.Additionally, consider short-circuiting when
entryIdsis empty to avoid an unnecessary API call.async run({ $ }) { - const entryIds = checkIdArray(this.entryIds); + const entryIds = checkIdArray(this.entryIds).map(Number); + if (!entryIds.length) { + $.export("$summary", "No records to retrieve"); + return []; + } const response = await this.dealcloud.queryEntries({Likely an incorrect or invalid review comment.
components/dealcloud/common/utils.mjs (2)
4-13: LGTM - Properly handles string input with trim and filter.The function now correctly trims whitespace and filters empty values when processing comma-separated strings.
89-92: LGTM - Commented code replaced with TODO.The large commented block has been appropriately replaced with a brief TODO comment.
components/dealcloud/actions/common/common-create-update.mjs (1)
46-55: LGTM - Dynamic props generation is well-structured.The
additionalPropsmethod correctly guards against missingentryTypeIdand fetches fields dynamically to generate props. ThereloadProps: trueonentryTypeIdensures props refresh when the entry type changes.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.