[Components] CsvBox new components #18942
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds a CSVBox integration: a submit-spreadsheet action (URL or local file upload), an app module with HTTP helpers and auth handling, a webhook source that validates JSON payloads and emits events, version bump and dependency additions, and a test-event dataset for the source. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Action as Submit Spreadsheet Action
participant App as CSVBox App
participant API as CSVBox API
User->>Action: Invoke action with file input & config
activate Action
alt File is a public URL
Action->>Action: Build JSON import payload (public_file_url, sheet_license_key, optional user/headers)
Action->>App: submitFile(payload)
else File is local
Action->>Action: Build multipart/form-data (file stream + import JSON)
Action->>App: submitFile(formData, multipart headers)
end
deactivate Action
activate App
App->>API: POST /file with auth headers
API-->>App: Response
App-->>Action: Response
Action-->>User: Return success summary
deactivate App
sequenceDiagram
participant CSVBox as CSVBox (webhook sender)
participant Source as New Import Source
participant System as Pipedream Event System
CSVBox->>Source: POST /webhook (Content-Type header + body)
activate Source
Source->>Source: Validate Content-Type == application/json
alt Valid JSON
Source->>Source: Capture timestamp
Source->>System: Emit event (body, id=ts, summary)
System-->>CSVBox: 200 OK
else Invalid Content-Type
Source-->>CSVBox: Throw ConfigurationError / Error response
end
deactivate Source
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ 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 |
jcortes
force-pushed
the
csvbox-new-components
branch
from
7816ce4 to
7ce61c6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 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 (5)
components/csvbox/actions/submit-spreadsheet/submit-spreadsheet.mjs(1 hunks)components/csvbox/csvbox.app.mjs(1 hunks)components/csvbox/package.json(2 hunks)components/csvbox/sources/new-import/new-import.mjs(1 hunks)components/csvbox/sources/new-import/test-event.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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/csvbox/csvbox.app.mjs
📚 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/csvbox/package.json
⏰ 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). (3)
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (10)
components/csvbox/package.json (1)
3-18: LGTM!The version bump and dependency additions are appropriate. Both
@pipedream/platformandform-dataare external packages used by the new components.components/csvbox/sources/new-import/new-import.mjs (1)
24-31: LGTM!The event emission logic correctly timestamps and emits the processed import data.
components/csvbox/csvbox.app.mjs (4)
6-24: LGTM!The prop definitions are well-structured with clear labels and helpful descriptions referencing where users can find the required values.
26-28: LGTM!URL construction is straightforward and correct.
29-36: LGTM!The header construction correctly includes authentication and allows overrides via the spread operator.
37-53: LGTM!The HTTP request methods follow a clean pattern with centralized request handling and debugging enabled.
components/csvbox/actions/submit-spreadsheet/submit-spreadsheet.mjs (4)
1-10: LGTM!Imports and component metadata are properly structured.
11-42: LGTM!Props are well-defined, properly referencing app propDefinitions where appropriate.
48-53: LGTM!The
booleanToNumbermethod comprehensively handles various representations of boolean values.
87-103: LGTM!The branching logic correctly handles both URL and file path inputs, constructing appropriate payloads for each case.
| ...(hasHeaders | ||
| ? { | ||
| options: { | ||
| has_header: booleanToNumber(hasHeaders), | ||
| }, | ||
| } | ||
| : {} | ||
| ), |
There was a problem hiding this comment.
Handle explicit false value for hasHeaders.
The current truthiness check means that when hasHeaders is explicitly set to false, the options object won't be included in the request. This prevents users from explicitly indicating that the spreadsheet has no headers.
Apply this diff to handle explicit false values:
- ...(hasHeaders
+ ...(hasHeaders !== undefined
? {
options: {
has_header: booleanToNumber(hasHeaders),
},
}
: {}
),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ...(hasHeaders | |
| ? { | |
| options: { | |
| has_header: booleanToNumber(hasHeaders), | |
| }, | |
| } | |
| : {} | |
| ), | |
| ...(hasHeaders !== undefined | |
| ? { | |
| options: { | |
| has_header: booleanToNumber(hasHeaders), | |
| }, | |
| } | |
| : {} | |
| ), |
🤖 Prompt for AI Agents
In components/csvbox/actions/submit-spreadsheet/submit-spreadsheet.mjs around
lines 77 to 84, the current truthiness check removes the options object when
hasHeaders is explicitly false; change the condition to detect an explicit
boolean (e.g., hasHeaders !== undefined or typeof hasHeaders === 'boolean') so
options is included for both true and false, keeping options: { has_header:
booleanToNumber(hasHeaders) } when hasHeaders is provided.
| headers: !isUrl | ||
| ? { | ||
| "Content-Type": "multipart/form-data", | ||
| } | ||
| : undefined, |
There was a problem hiding this comment.
Fix multipart Content-Type to include boundary.
Manually setting Content-Type: multipart/form-data without the boundary parameter will cause the request to fail. The form-data package's FormData instance has a getHeaders() method that returns the correct headers including the boundary.
Apply this diff:
headers: !isUrl
- ? {
- "Content-Type": "multipart/form-data",
- }
+ ? data.getHeaders()
: undefined,This will properly include the boundary parameter in the Content-Type header (e.g., multipart/form-data; boundary=----WebKitFormBoundary...), which is required for the server to parse the multipart request correctly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| headers: !isUrl | |
| ? { | |
| "Content-Type": "multipart/form-data", | |
| } | |
| : undefined, | |
| headers: !isUrl | |
| ? data.getHeaders() | |
| : undefined, |
🤖 Prompt for AI Agents
In components/csvbox/actions/submit-spreadsheet/submit-spreadsheet.mjs around
lines 107 to 111, the code manually sets "Content-Type": "multipart/form-data"
which omits the required boundary; replace that manual header with the FormData
instance's getHeaders() (e.g., const formHeaders = form.getHeaders()) and merge
those headers into the request when !isUrl so the Content-Type includes the
boundary parameter returned by getHeaders().
| if (!headers["content-type"] || headers["content-type"] !== "application/json") { | ||
| throw new ConfigurationError("Invalid content type. Please check your CSVBox webhook configuration so that the content type is set to `JSON`."); | ||
| } |
There was a problem hiding this comment.
Content-Type validation is too strict.
The exact equality check headers["content-type"] !== "application/json" will reject valid Content-Type headers that include charset or other parameters (e.g., "application/json; charset=utf-8").
Apply this diff to use a more flexible check:
- if (!headers["content-type"] || headers["content-type"] !== "application/json") {
+ if (!headers["content-type"]?.startsWith("application/json")) {
throw new ConfigurationError("Invalid content type. Please check your CSVBox webhook configuration so that the content type is set to `JSON`.");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!headers["content-type"] || headers["content-type"] !== "application/json") { | |
| throw new ConfigurationError("Invalid content type. Please check your CSVBox webhook configuration so that the content type is set to `JSON`."); | |
| } | |
| if (!headers["content-type"]?.startsWith("application/json")) { | |
| throw new ConfigurationError("Invalid content type. Please check your CSVBox webhook configuration so that the content type is set to `JSON`."); | |
| } |
🤖 Prompt for AI Agents
In components/csvbox/sources/new-import/new-import.mjs around lines 20 to 22,
the Content-Type check uses strict equality against "application/json" which
rejects valid headers with parameters (e.g., "application/json; charset=utf-8");
change the validation to parse or prefix-match the header value (e.g., check
that headers["content-type"] startsWith "application/json" or split on ";" and
trim before comparing) and ensure you handle missing header case as before and
normalize casing when comparing.
| "Question": "What is the fourth state of matter after solid and liquid and gas?", | ||
| "Answer": "Plasma", | ||
| "Option B": "Ground State", | ||
| "Option C": "Metal ", |
There was a problem hiding this comment.
Remove trailing whitespace.
"Metal " contains trailing spaces, which is likely unintentional.
Apply this diff:
- "Option C": "Metal ",
+ "Option C": "Metal",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Option C": "Metal ", | |
| "Option C": "Metal", |
🤖 Prompt for AI Agents
In components/csvbox/sources/new-import/test-event.mjs around line 73, the value
"Metal " contains unintended trailing whitespace; remove the extra spaces so
the string becomes "Metal" (update the literal in that line and run a quick
check/lint to ensure no other entries have trailing spaces).
For Integration QA: |
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
WHY
Resolves #18697
Summary by CodeRabbit
New Features
Chores