Add Csvbox integration with import event trigger and sample row functionality #18982
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 centralized CSVBox API constants and request helpers, adds a new webhook-driven csvbox-new-row source (activate/deactivate/deploy/run), removes the submit-spreadsheet action and the old new-import source and its test fixtures, and removes the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Runtime as Pipedream Runtime
participant CSVBoxAPI as CSVBox API
participant Storage
rect rgba(200,220,255,0.12)
Note over Runtime: Activation
User->>Runtime: Enable csvbox-new-row
Runtime->>CSVBoxAPI: POST /register-webhook (createHook)
CSVBoxAPI-->>Runtime: { hookId }
Runtime->>Storage: persist hookId & sampleRow
end
rect rgba(200,255,220,0.12)
Note over Runtime: Webhook handling
CSVBoxAPI->>Runtime: Webhook callback (new rows)
Runtime->>Runtime: run(event) -> emit rows (id, summary)
Runtime->>User: Event emitted
end
rect rgba(255,240,200,0.12)
Note over Runtime: Deactivation
User->>Runtime: Disable csvbox-new-row
Runtime->>CSVBoxAPI: DELETE /delete-webhook (deleteHook)
Runtime->>Storage: clear hookId & sampleRow
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📚 Learning: 2024-10-10T19:18:27.998ZApplied to files:
📚 Learning: 2025-01-23T03:55:15.166ZApplied to files:
📚 Learning: 2024-10-30T15:24:39.294ZApplied to files:
🔇 Additional comments (7)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
components/csvbox/actions/submit-spreadsheet/submit-spreadsheet.mjs(0 hunks)components/csvbox/common/constants.mjs(1 hunks)components/csvbox/csvbox-new-row/csvbox-new-row.mjs(1 hunks)components/csvbox/csvbox.app.mjs(1 hunks)components/csvbox/package.json(1 hunks)components/csvbox/sources/new-import/new-import.mjs(0 hunks)components/csvbox/sources/new-import/test-event.mjs(0 hunks)
💤 Files with no reviewable changes (3)
- components/csvbox/sources/new-import/test-event.mjs
- components/csvbox/sources/new-import/new-import.mjs
- components/csvbox/actions/submit-spreadsheet/submit-spreadsheet.mjs
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-07-24T02:06:47.016Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-07-24T02:06:47.016Z
Learning: The `common-webhook-methods.mjs` object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like `generateWebhookMeta` and `getEventType` to enforce implementation in subclasses.
Applied to files:
components/csvbox/csvbox-new-row/csvbox-new-row.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
📚 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
🧬 Code graph analysis (1)
components/csvbox/csvbox.app.mjs (1)
components/csvbox/csvbox-new-row/csvbox-new-row.mjs (1)
data(33-33)
🔇 Additional comments (8)
components/csvbox/package.json (1)
15-17: LGTM! Dependency cleanup looks good.Removing the unused
form-datadependency aligns with the refactoring to use centralized request handling in the app module.components/csvbox/common/constants.mjs (1)
1-5: LGTM! Good practice to centralize API configuration.Defining the BASE_URL as a constant improves maintainability and makes it easier to update the API endpoint if needed.
components/csvbox/csvbox-new-row/csvbox-new-row.mjs (1)
98-110: LGTM! Helper methods follow standard patterns.The storage helper methods are well-structured and provide a clean abstraction over the db operations.
components/csvbox/csvbox.app.mjs (5)
54-61: LGTM! Webhook creation method is well-structured.The method correctly delegates to
_makeRequestwith the appropriate HTTP method and path.
63-70: LGTM! Webhook deletion method is correctly implemented.The method uses the appropriate HTTP DELETE method and path for webhook deletion.
72-79: LGTM! List sheets method is functional.The method correctly retrieves the sheet list. Minor note: line 78 could be simplified to
return this._makeRequest(...)without the intermediate variable, but the current implementation is acceptable.
33-41: LGTM! Header construction is correct.The method properly merges default headers with custom API key headers required for CSVBox authentication.
13-19: Verify the API response structure for sheet options.Line 17 assumes the API returns objects with a
valuefield. However, this is inconsistent with other similar integrations in the codebase:
- Smartsheet uses
sheet.id- Ragic uses
sheet.id- Orca Scan uses
sheet._idBased on the reference to
sheet_slugin webhook registration (csvbox-new-row.mjs), verify whether the CSVBox API actually returnsvalue, or if it should beid,slug, or another field. Consult the CSVBox API documentation for the/list-sheetsendpoint response structure to confirm the correct field name.
SumitYewale-Thalia
left a comment
SumitYewale-Thalia
left a comment
There was a problem hiding this comment.
Code improvements done
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/csvbox/csvbox-new-row/csvbox-new-row.mjs(1 hunks)components/csvbox/csvbox.app.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-10T19:18:27.998Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 14265
File: components/the_magic_drip/sources/common.mjs:35-43
Timestamp: 2024-10-10T19:18:27.998Z
Learning: In `components/the_magic_drip/sources/common.mjs`, when processing items in `getAndProcessData`, `savedIds` is intentionally updated with IDs of both emitted and non-emitted items to avoid emitting retroactive events upon first deployment and ensure only new events are emitted as they occur.
Applied to files:
components/csvbox/csvbox-new-row/csvbox-new-row.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/csvbox/csvbox.app.mjs
🧬 Code graph analysis (1)
components/csvbox/csvbox.app.mjs (1)
components/csvbox/csvbox-new-row/csvbox-new-row.mjs (1)
data(33-33)
🔇 Additional comments (10)
components/csvbox/csvbox-new-row/csvbox-new-row.mjs (6)
1-18: LGTM!The component metadata, imports, and props are properly structured. The use of
dedupe: "unique"is appropriate for a webhook source that emits events with unique IDs.
65-81: LGTM!The deactivation hook correctly cleans up the webhook and clears stored state using the helper methods. Error handling is appropriate—logging errors without throwing ensures deactivation doesn't fail if the webhook was already deleted.
97-109: LGTM!The helper methods correctly abstract database operations and follow consistent naming conventions.
111-126: LGTM!The run method correctly validates the webhook payload and emits events for each row. The fallback ID generation at line 121 includes
sheet_id,row_number, and timestamp, which provides good uniqueness. The primaryrow.import_idshould normally be present from the CSVBox API.
127-148: LGTM!The sample events array provides a clear example of the expected webhook payload structure, helping users understand the data format they'll receive.
40-56: Sample data structure aligns with documented CSVBox API fields—remove request, noteimport_descriptionconcern.The CSVBox API's webhook sample_response includes the fields used in your code: sheet_name, row_number, row_data, total_rows, env_name, custom_fields, and original_filename. Your defensive fallback values are appropriate for handling optional fields.
However,
import_descriptionis hardcoded as "This is a sample test import" but does not appear in CSVBox's documented sample_response fields. Either clarify this is intentional synthetic metadata for the sample workflow, or derive it from the API response if available.components/csvbox/csvbox.app.mjs (4)
23-41: LGTM!The authentication and header methods correctly construct the necessary authentication headers using custom API key headers (
x-csvbox-api-keyandx-csvbox-secret-api-key) as required by the CSVBox API.
43-51: LGTM!The
_makeRequestmethod correctly constructs the axios configuration without the previously flagged incorrectauthproperty. Authentication now properly relies on custom headers set via_getHeaders().
53-78: LGTM!The webhook management and sheet listing methods are correctly implemented with appropriate HTTP methods and endpoints. The consistent use of
_makeRequestensures all API calls benefit from centralized authentication and error handling.
8-20: <web_search>CSVBox API list-sheets endpoint response format</web_search>
SumitYewale-Thalia
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/csvbox/csvbox-new-row/csvbox-new-row.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-10T19:18:27.998Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 14265
File: components/the_magic_drip/sources/common.mjs:35-43
Timestamp: 2024-10-10T19:18:27.998Z
Learning: In `components/the_magic_drip/sources/common.mjs`, when processing items in `getAndProcessData`, `savedIds` is intentionally updated with IDs of both emitted and non-emitted items to avoid emitting retroactive events upon first deployment and ensure only new events are emitted as they occur.
Applied to files:
components/csvbox/csvbox-new-row/csvbox-new-row.mjs
🔇 Additional comments (4)
components/csvbox/csvbox-new-row/csvbox-new-row.mjs (4)
41-54: Well done fixing the ID consistency issue.The code now generates a single
sampleIdand uses it for both theimport_idfield and the event metadataid, ensuring consistency and proper deduplication. This addresses the previous review concern about inconsistent IDs.
76-77: Good fix using helper methods.The code now uses
_setHookID(null)and_setSampleRow(null)instead of directly callingdb.set(), maintaining consistency with the rest of the codebase. This addresses a previous review comment.
83-89: Excellent fix to avoid duplicate sample emissions.The deploy hook now correctly skips emitting the sample row since it was already emitted during activation. This prevents duplicate events and the ID collision issues flagged in previous reviews. The comment clearly documents the reasoning.
115-115: Good improvement to fallback ID generation.The fallback ID now includes
row.row_number, which significantly reduces the likelihood of collisions compared to the previous implementation that only usedsheet_idandDate.now(). While rare collisions are still theoretically possible in edge cases (identical sheet_id and row_number arriving in the same millisecond), this is an acceptable trade-off for practical use.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/csvbox/csvbox-new-row/csvbox-new-row.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-10-10T19:18:27.998Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 14265
File: components/the_magic_drip/sources/common.mjs:35-43
Timestamp: 2024-10-10T19:18:27.998Z
Learning: In `components/the_magic_drip/sources/common.mjs`, when processing items in `getAndProcessData`, `savedIds` is intentionally updated with IDs of both emitted and non-emitted items to avoid emitting retroactive events upon first deployment and ensure only new events are emitted as they occur.
Applied to files:
components/csvbox/csvbox-new-row/csvbox-new-row.mjs
📚 Learning: 2025-01-23T03:55:15.166Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.
Applied to files:
components/csvbox/csvbox-new-row/csvbox-new-row.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/csvbox/csvbox-new-row/csvbox-new-row.mjs
🔇 Additional comments (8)
components/csvbox/csvbox-new-row/csvbox-new-row.mjs (8)
1-9: LGTM! Component metadata follows best practices.The component key, name, and description are clear and follow Pipedream conventions. The
dedupe: "unique"strategy is appropriate for preventing duplicate import events.
10-18: LGTM! Props configuration is well-structured.Good use of
propDefinitionfor thesheetIdprop, which promotes reusability and consistency with other components.
20-64: Excellent implementation of activate hook.The activation flow properly:
- Validates required configuration before proceeding
- Creates the webhook and stores the ID
- Generates a consistent ID for the sample event (used in both the event data and metadata)
- Handles errors gracefully with informative messages
This addresses all past review concerns about ID consistency and collision risks.
65-81: LGTM! Proper cleanup in deactivate hook.The deactivation flow correctly:
- Deletes the webhook using the stored ID
- Clears all persisted state using helper methods
- Handles errors gracefully (logging without throwing is appropriate for cleanup operations)
82-88: LGTM! Deploy hook correctly avoids duplicate sample emission.The approach of skipping emission in
deploy()is appropriate here since:
- The sample event was already emitted during
activate()- Webhook sources don't have historical events to backfill
- With
dedupe: "unique", re-emitting would be redundantThis addresses the past concern about inconsistent sample data structure between hooks.
90-118: LGTM! Methods are well-documented.All helper methods now include proper JSDoc comments with parameter and return type documentation, improving maintainability and developer experience.
120-135: LGTM! Run method properly processes webhook events.The implementation correctly:
- Validates the webhook payload structure
- Handles invalid payloads gracefully without throwing (appropriate for webhook handlers)
- Emits each row with a proper ID (preferring
import_idfrom the API, with a robust fallback)- Provides descriptive event summaries
The fallback ID generation at line 130 includes
row_number, which addresses the past concern about collision risks.
136-157: LGTM! Sample events provide helpful documentation.The sample event structure clearly illustrates the expected payload format, which helps developers understand what data they'll receive from this source.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/csvbox/csvbox-new-row/csvbox-new-row.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-10-10T19:18:27.998Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 14265
File: components/the_magic_drip/sources/common.mjs:35-43
Timestamp: 2024-10-10T19:18:27.998Z
Learning: In `components/the_magic_drip/sources/common.mjs`, when processing items in `getAndProcessData`, `savedIds` is intentionally updated with IDs of both emitted and non-emitted items to avoid emitting retroactive events upon first deployment and ensure only new events are emitted as they occur.
Applied to files:
components/csvbox/csvbox-new-row/csvbox-new-row.mjs
📚 Learning: 2025-01-23T03:55:15.166Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.
Applied to files:
components/csvbox/csvbox-new-row/csvbox-new-row.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/csvbox/csvbox-new-row/csvbox-new-row.mjs
🔇 Additional comments (7)
components/csvbox/csvbox-new-row/csvbox-new-row.mjs (7)
1-18: Component structure and metadata look good!The component metadata, props, and dedupe strategy are well-configured for a webhook source. The component key follows naming conventions, and the prop definitions are appropriate.
20-57: Excellent improvements to activate hook!The activate hook has been significantly improved based on past feedback:
- Redundant
hookIdvariable eliminated (line 34 now useswebhookIddirectly)- Single
sampleIdgenerated once and used for bothimport_idandmeta.id(line 41), ensuring consistency- Proper validation of prerequisites before webhook creation
The logic is sound and the error handling is appropriate.
65-81: Deactivate hook properly implemented!The deactivate hook correctly:
- Uses helper methods
_setHookIDand_setSampleRowconsistently (addresses past review feedback)- Handles errors gracefully without throwing (appropriate for cleanup operations)
- Clears all stored state after webhook deletion
82-88: Deploy hook correctly avoids duplicate emission!The deploy hook properly addresses past review concerns by not re-emitting the sample row that was already emitted during activation. The comment on line 87 clearly documents this design decision.
90-118: Methods properly documented with JSDoc!All helper methods now include comprehensive JSDoc comments with parameter types and return types, addressing previous review feedback. This improves maintainability and developer experience.
Based on coding guidelines.
120-135: Run method significantly improved!The webhook handler now includes
row_numberin the fallback ID generation (line 130), addressing past review concerns. The primary path usesrow.import_idfrom the API, and the fallback composite key${row.sheet_id}_${row.row_number}_${Date.now()}is much more collision-resistant.While there's a theoretical collision risk if multiple rows with identical
sheet_idandrow_numberarrive within the same millisecond, this is extremely unlikely in practice, and the API-providedimport_idshould handle the primary use case.
136-157: Sample events properly structured!The sample events array provides a complete, realistic example that matches the structure of events emitted by the component. This is helpful for testing and user understanding.
3 participants
WHY
Resolves #18697
Summary
This PR introduces a new Csvbox component to the Pipedream components library.
Csvbox allows users to collect CSV data through hosted importers and send it directly into their applications.
This component connects to a user’s Csvbox account, retrieves available sheets, provides sample data for mapping with subsequent components, and listens for import events once deployed.
Functionality
Removed existing code
Removed some code that was present because it was unauthorized and doesn't relate to our app.
Files removed, namely:
Summary by CodeRabbit
New Features
Improvements
Removed