-
Notifications
You must be signed in to change notification settings - Fork 5.6k
[Components] crowdpower #13454 #19471
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?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
WalkthroughAdds three Crowdpower action modules (create charge, create event, upsert customer), extends the Crowdpower app with HTTP request helpers and API wrapper methods, and updates package metadata (version and dependency). Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
components/crowdpower/actions/create-customer-charge/create-customer-charge.mjs(1 hunks)components/crowdpower/actions/create-customer-event/create-customer-event.mjs(1 hunks)components/crowdpower/actions/upsert-customer/upsert-customer.mjs(1 hunks)components/crowdpower/crowdpower.app.mjs(1 hunks)components/crowdpower/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/crowdpower/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/crowdpower/actions/create-customer-charge/create-customer-charge.mjscomponents/crowdpower/actions/create-customer-event/create-customer-event.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/crowdpower/crowdpower.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/crowdpower/crowdpower.app.mjs
🧬 Code graph analysis (2)
components/crowdpower/actions/upsert-customer/upsert-customer.mjs (2)
components/crowdpower/actions/create-customer-charge/create-customer-charge.mjs (1)
response(30-36)components/crowdpower/actions/create-customer-event/create-customer-event.mjs (1)
response(37-44)
components/crowdpower/actions/create-customer-charge/create-customer-charge.mjs (2)
components/crowdpower/actions/create-customer-event/create-customer-event.mjs (1)
response(37-44)components/crowdpower/actions/upsert-customer/upsert-customer.mjs (1)
response(43-51)
🪛 GitHub Actions: Components Checks
components/crowdpower/package.json
[error] 1-1: Lockfile specifiers do not match package.json: the lockfile has {} while package.json requires "@pipedream/platform":"^1.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). (1)
- GitHub Check: pnpm publish
🔇 Additional comments (3)
components/crowdpower/package.json (1)
3-3: Version bump OK, but ensure it matches your release intent (0.0.4 → 0.1.0).Confirm this is intended as a minor bump (new actions + app surface). Also confirm any internal component registry/versioning expectations for Pipedream components.
components/crowdpower/crowdpower.app.mjs (1)
42-57: Confirm@pipedream/platformaxios behavior (return shape + non-2xx errors) matches action expectations.Actions rely on
response.success/response.code. Please verifyaxios($, config)returns the API JSON body (not an AxiosResponse wrapper), and whether it throws on non-2xx (in which case action summaries won’t run unless wrapped).components/crowdpower/actions/upsert-customer/upsert-customer.mjs (1)
16-22: The current implementation is correct as-is. TheuserIdparameter cannot be optional because it is embedded in the API endpoint path:PUT /customers/${user_id}. WithoutuserId, the API call would fail with an invalid endpoint (e.g.,/customers/undefined).The review comment misunderstands the action's purpose. This action is designed for updating/upserting an existing customer identified by their
userId. If the Crowdpower API supports true create-or-update semantics via thePUT /customers/{userId}endpoint, thenuserIdremains required. If the intended use case is to create customers without pre-existing IDs, a separate endpoint or action logic would be needed, but the current implementation correctly uses the endpoint as designed.
components/crowdpower/actions/create-customer-charge/create-customer-charge.mjs
Show resolved
Hide resolved
components/crowdpower/actions/create-customer-event/create-customer-event.mjs
Show resolved
Hide resolved
7246007 to
1bec5cc
Compare
GTFalcao
left a 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.
Looks good to me. But I did leave one comment regarding summaries that may need to be improved.
components/crowdpower/actions/create-customer-charge/create-customer-charge.mjs
Show resolved
Hide resolved
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
c8fb2d5 to
7a01ccd
Compare
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
♻️ Duplicate comments (2)
components/crowdpower/crowdpower.app.mjs (2)
12-16: Amount type should benumber, notstring.For better input UX and type consistency, the
amountprop should usetype: "number".🔎 Apply this diff:
amount: { - type: "string", + type: "number", label: "Amount", description: "Amount of the charge", },
39-57: Usenew URL()for safer URL construction.While the current string concatenation may work, it's fragile. If the base URL or path format changes, it could break. Use the standard
new URL()constructor for robust URL joining, as confirmed by maintainer feedback.🔎 Apply this diff:
async _makeRequest(opts = {}) { const { $ = this, path, headers, ...otherOpts } = opts; return axios($, { ...otherOpts, - url: this._baseUrl() + path, + url: new URL(path, this._baseUrl()).toString(), headers: { "Authorization": `Bearer ${this.$auth.application_key}`, ...headers, }, }); },
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/crowdpower/actions/create-customer-charge/create-customer-charge.mjs(1 hunks)components/crowdpower/crowdpower.app.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 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/crowdpower/actions/create-customer-charge/create-customer-charge.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/crowdpower/crowdpower.app.mjs
📚 Learning: 2025-10-20T01:01:02.970Z
Learnt from: js07
Repo: PipedreamHQ/pipedream PR: 18744
File: components/slack_v2/actions/send-large-message/send-large-message.mjs:49-64
Timestamp: 2025-10-20T01:01:02.970Z
Learning: In components/slack_v2/actions/send-large-message/send-large-message.mjs, the metadata_event_payload prop is typed as string, so the code only needs to handle string-to-JSON parsing and does not need to handle object inputs.
Applied to files:
components/crowdpower/crowdpower.app.mjs
📚 Learning: 2025-02-05T21:58:03.118Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 15479
File: packages/connect-react/src/components/ComponentForm.tsx:23-24
Timestamp: 2025-02-05T21:58:03.118Z
Learning: In the connect-react package, the `enableDebugging` property should be of type `boolean` as it's used for toggling debugging features and conditional rendering.
Applied to files:
components/crowdpower/crowdpower.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/crowdpower/crowdpower.app.mjs
🧬 Code graph analysis (1)
components/crowdpower/actions/create-customer-charge/create-customer-charge.mjs (2)
components/crowdpower/actions/upsert-customer/upsert-customer.mjs (1)
response(43-51)components/crowdpower/actions/create-customer-event/create-customer-event.mjs (1)
response(37-44)
🪛 GitHub Actions: Pull Request Checks
components/crowdpower/actions/create-customer-charge/create-customer-charge.mjs
[error] 41-41: Trailing spaces not allowed. (no-trailing-spaces)
🪛 GitHub Check: Lint Code Base
components/crowdpower/actions/create-customer-charge/create-customer-charge.mjs
[failure] 41-41:
Trailing spaces not allowed
⏰ 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). (2)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (6)
components/crowdpower/actions/create-customer-charge/create-customer-charge.mjs (2)
1-13: LGTM: Action metadata is well-structured.The import, key, naming, versioning, and annotations follow Pipedream conventions. Documentation link is provided.
14-27: LGTM: Props correctly reference app propDefinitions.The prop structure follows Pipedream conventions for reusing definitions from the app file.
components/crowdpower/crowdpower.app.mjs (4)
1-1: LGTM: Correct axios import.Using
@pipedream/platformaxios is the standard approach for Pipedream integrations.
6-26: LGTM: PropDefinitions for userId, email, and name are properly structured.Type, label, and description fields are appropriate for these string-based properties.
27-37: LGTM: CustomAttributes and action props are appropriately typed.The
objecttype for custom attributes andstringtype for action are suitable for their respective use cases.
58-78: LGTM: Public API methods are well-structured.All three methods (
createCustomerCharge,upsertCustomer,createCustomerEvent) follow a consistent pattern, correctly delegate to_makeRequest, and use appropriate HTTP methods and endpoints.
components/crowdpower/actions/create-customer-charge/create-customer-charge.mjs
Show resolved
Hide resolved
michelle0927
left a 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.
This is failing the Lint Code Base PR check. Please remember to run npx eslint --fix on any updates before pushing the changes, and make sure the PR is passing checks before moving to "Ready for PR Review".
| user_id: this.userId, | ||
| custom_attributes: this.customAttributes, | ||
| action: this.action, |
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.
The docs show this endpoint accepting properties user_id, action, and properties. Are you sure custom_attributes is correct?
If correct, add an example to the description of prop customAttributes.
If not, add prop properties with an example in the prop description.
| customAttributes: { | ||
| propDefinition: [ | ||
| app, | ||
| "customAttributes", | ||
| ], | ||
| }, |
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.
Should include an example of valid input in the prop description.
| userId: { | ||
| type: "string", | ||
| label: "User ID", | ||
| description: "ID of the user", | ||
| }, |
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.
Can you use the get customers endpoint to list the ids?
WHY
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.