-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New Components - boldsign #15013
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
New Components - boldsign #15013
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis pull request introduces comprehensive support for BoldSign, a document e-signature platform. The changes include the addition of a new action for sending documents using templates, enhancements to the BoldSign application component with new properties and methods, and the introduction of polling sources for document events (completed, sent, and declined). Supporting utility functions and constants have also been added, improving the overall interaction with the BoldSign API and document management capabilities. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Sources - New Document Completed - New Document Sent - New Document Declined Actions - Send Document Template
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: 9
🧹 Nitpick comments (15)
components/boldsign/sources/new-document-declined/test-event.mjs (3)
1-8: Use obvious test data identifiersConsider using clearly fake test data to prevent confusion and accidental misuse in production:
- Use
test-doc-idinstead of UUID-like strings- Use
example.comdomain instead ofcubeflakes.com- Use obvious test names like
test-senderexport default { - "documentId": "755195d8-xxxx-xxxx-xxxx-88ff77d35419", + "documentId": "test-doc-id", "senderDetail": { - "name": "Richard", + "name": "test-sender", "privateMessage": null, - "emailAddress": "richard@cubeflakes.com", + "emailAddress": "sender@example.com", "isViewed": false },
9-14: Maintain consistent test data patternsApply the same test data conventions to CC recipients for consistency.
"ccDetails": [ { - "emailAddress": "alexgayle@cubeflakes.com", + "emailAddress": "cc-recipient@example.com", "isViewed": false } ],
42-47: Maintain consistent timestamp handlingApply the same timestamp recommendations to
expiryDateandnextCursor. Also, consider makingnextCursordifferent fromactivityDateto better represent real-world scenarios.- "expiryDate": 1670178599, + "expiryDate": 1000432000, // createdDate + 5 days in seconds "enableSigningOrder": false, "isDeleted": false, "labels": [], - "nextCursor": 1665989290 + "nextCursor": 1000086401 // slightly after activityDatecomponents/boldsign/sources/new-document-completed/test-event.mjs (2)
15-19: Improve timestamp format and document status values.
- Consider using ISO 8601 timestamp format for better readability in tests
- Replace PII in activityBy
- Consider adding a comment documenting possible status values
Apply this diff:
- "createdDate": 1664961706, - "activityDate": 1665989290, - "activityBy": "alexgayle@cubeflakes.com", + // Unix timestamps shown in ISO 8601 format for readability + "createdDate": "2022-10-05T12:00:00Z", + "activityDate": "2022-10-17T12:00:00Z", + "activityBy": "test.user@example.com", "messageTitle": "565", + // Possible values: "Completed", "Pending", "Declined", etc. "status": "Completed",
42-47: Improve timestamp format and add documentation for pagination.
- Use consistent timestamp format for expiryDate
- Document the purpose and format of nextCursor
Apply this diff:
- "expiryDate": 1670178599, + "expiryDate": "2022-12-04T12:00:00Z", "enableSigningOrder": false, "isDeleted": false, "labels": [], + // Timestamp used for pagination in polling requests "nextCursor": 1665989290components/boldsign/sources/common/base.mjs (2)
53-55: Consider supporting a custom initial value.
Currently, _getLastDate() defaults to 0 if the value is not set in the database. This baseline choice works, but you may want to support user-defined initial values or ingestion points (e.g., a specific date).
71-96: Potential large memory usage when collecting data in an array.
The code accumulates all items in the responseArray before filtering older items or applying maxResults. If the BoldSign API returns a large result set, it could lead to high memory usage. Consider a streaming approach or an early stop condition to break out of the loop once you’ve retrieved enough items.components/boldsign/boldsign.app.mjs (1)
90-98: Consolidate request logic.
This chunk of code in _makeRequest() is repeated across multiple list methods. While it's well-structured, consider storing shared parameters or default error handling logic in a single location if additional calls are required.components/boldsign/actions/send-document-template/send-document-template.mjs (2)
74-79: Change from boolean to strict enum for enableAutoReminder.
Using a boolean here is convenient, but you might consider an enum or a more descriptive property if the logic for auto reminders becomes more complex in future expansions (e.g., “daily”, “weekly”, etc.).
202-210: Remove redundant definitions of reminder settings.
You set reminderSettings in additionalData and in data at lines 228-232. Consolidate them into a single approach.components/boldsign/package.json (1)
3-3: Version bump indicates significant enhancements.
Ensure you update the changelog or release notes to highlight new features, breaking changes, or relevant migrations for version 0.1.0.components/boldsign/common/utils.mjs (1)
8-31: Add JSDoc documentation and consider performance optimizationThe function handles JSON parsing well, but could benefit from:
- Clear documentation of its behavior and edge cases
- Performance optimization for large arrays
- More specific type checking
Consider this enhanced version:
+/** + * Attempts to parse JSON strings within an object or array + * @param {any} obj - The input to parse + * @returns {any} Parsed object/array or original input if parsing fails + */ export const parseObject = (obj) => { if (!obj) return undefined; + // For large arrays, consider using for...of loop instead of map if (Array.isArray(obj)) { return obj.map((item) => { - if (typeof item === "string") { + if (typeof item === "string" && (item.startsWith("{") || item.startsWith("["))) { try { return JSON.parse(item); } catch (e) { return item; } } return item; }); } - if (typeof obj === "string") { + if (typeof obj === "string" && (obj.startsWith("{") || obj.startsWith("["))) { try { return JSON.parse(obj); } catch (e) { return obj; } } return obj; };components/boldsign/sources/new-document-declined/new-document-declined.mjs (1)
9-11: Consider semantic versioning for initial releaseFor a new component, consider starting with version "1.0.0" instead of "0.0.1" if this is intended for production use.
components/boldsign/sources/new-document-completed/new-document-completed.mjs (1)
4-27: Consider extracting common status constantsThe status strings ("Completed", "Declined") are used across multiple source components. Consider extracting these into shared constants to maintain consistency and ease maintenance.
Create a new constants file:
// components/boldsign/common/constants.mjs export const DocumentStatus = { COMPLETED: "Completed", DECLINED: "Declined", // Add other statuses as needed };Then update the source components to use these constants:
+import { DocumentStatus } from "../common/constants.mjs"; export default { // ... getParams() { return { - status: "Completed", + status: DocumentStatus.COMPLETED, }; }, };components/boldsign/sources/new-document-sent/new-document-sent.mjs (1)
4-11: Consider enhancing the component description.While the current description is clear, it could be more informative by including details about:
- What triggers the event (e.g., immediate or delayed emission)
- What data is included in the event
- Any filtering capabilities
Suggested enhancement:
- description: "Emit new event when a document is sent to a signer.", + description: "Emit new event when a document is sent to a signer. The event includes document details and is emitted when BoldSign registers the document as sent.",
📜 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 (12)
components/boldsign/actions/send-document-template/send-document-template.mjs(1 hunks)components/boldsign/boldsign.app.mjs(1 hunks)components/boldsign/common/constants.mjs(1 hunks)components/boldsign/common/utils.mjs(1 hunks)components/boldsign/package.json(2 hunks)components/boldsign/sources/common/base.mjs(1 hunks)components/boldsign/sources/new-document-completed/new-document-completed.mjs(1 hunks)components/boldsign/sources/new-document-completed/test-event.mjs(1 hunks)components/boldsign/sources/new-document-declined/new-document-declined.mjs(1 hunks)components/boldsign/sources/new-document-declined/test-event.mjs(1 hunks)components/boldsign/sources/new-document-sent/new-document-sent.mjs(1 hunks)components/boldsign/sources/new-document-sent/test-event.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- components/boldsign/common/constants.mjs
- components/boldsign/sources/new-document-sent/test-event.mjs
🔇 Additional comments (11)
components/boldsign/sources/new-document-declined/test-event.mjs (1)
1-47: Verify test event structure against API documentation
Please ensure this test event structure matches the actual BoldSign API response format for declined documents.
✅ Verification successful
Let me search for BoldSign-specific API response handling to verify the test event structure.
Let me examine the actual test events for comparison to verify the structure consistency.
Test event structure is consistent with BoldSign's webhook payload format
The test event structure matches the format used across all BoldSign webhook events in the codebase. The structure is consistent across different event types (completed, declined, sent) with the only variation being the status field which correctly reflects the event type. All essential fields like documentId, senderDetail, ccDetails, signerDetails, and metadata are present with the correct types and structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other BoldSign API response handling code to verify consistency
rg -t js -t ts "documentId|senderDetail|ccDetails|signerDetails" --type-add 'ts:*.{ts,mjs}'
Length of output: 30833
Script:
#!/bin/bash
# Search for BoldSign API response handling code and webhook handlers
rg -t js -t ts "new-document-declined|BoldSign.*webhook|BoldSign.*response" --type-add 'ts:*.{ts,mjs}'
# Search for other BoldSign test events for comparison
fd -e mjs -e js test-event components/boldsign/
Length of output: 463
Script:
#!/bin/bash
# Compare the structure of all BoldSign test events
cat components/boldsign/sources/new-document-completed/test-event.mjs components/boldsign/sources/new-document-sent/test-event.mjs
Length of output: 2424
components/boldsign/sources/new-document-completed/test-event.mjs (1)
1-47: Verify test event structure against BoldSign API documentation.
Please ensure that this test event structure matches the latest BoldSign API documentation for document completion events.
✅ Verification successful
Let me analyze the test event structure and generate additional verification scripts to check the BoldSign API integration.
Let me try a different search strategy to verify the BoldSign integration.
Test event structure matches BoldSign API integration
The test event structure is correctly aligned with the BoldSign integration implementation:
- The file is located in the correct path
components/boldsign/sources/new-document-completed/test-event.mjs - The event structure matches the BoldSign document completion webhook payload with all required fields:
documentIdfor document identificationsenderDetailcontaining sender informationsignerDetailswith signing status and authentication detailsstatusindicating document completion state- All other essential fields like
ccDetails,createdDate, etc.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for BoldSign API types or interfaces in the codebase
ast-grep --pattern 'interface $INTERFACE {
documentId?: string;
$$$
}'
# Search for other test events to ensure consistency
fd test-event.mjs --exec cat {}
Length of output: 70548
Script:
#!/bin/bash
# Search for BoldSign API response handling and types
rg -l "BoldSign" --type ts --type js
# Search for document completion event handling
rg -A 5 "documentId|senderDetail|signerDetails" --type ts --type js
# Look for BoldSign related documentation or schema files
fd -e json -e md -e ts "boldsign|BoldSign"
Length of output: 151
Script:
#!/bin/bash
# Search for any files in the boldsign directory
fd . components/boldsign/
# Look for webhook or event handling files
fd "webhook|event" components/boldsign/
# Search for API response handling code
rg -A 10 "export default" components/boldsign/sources/new-document-completed/
Length of output: 3528
components/boldsign/sources/common/base.mjs (1)
59-70: Validate parameters for pagination and filtering.
You treat various inputs (recipients, labels, brandIds, etc.) by parsing objects. Consider gracefully handling malformed or empty inputs with fallback logic or warnings to avoid runtime exceptions or incomplete queries.
components/boldsign/boldsign.app.mjs (2)
50-60: Clarify error handling for empty contacts.
When listing contacts at lines 50–60, consider clarifying how you handle cases where no contacts are returned. Return an empty array instead of potentially an undefined result.
136-164: Check for empty or null returns in paginated endpoints.
When using the paginate function, handle scenarios where the API might return an empty result array, or page details might be missing if the API changes. This can prevent potential runtime errors.
components/boldsign/actions/send-document-template/send-document-template.mjs (2)
256-268: Avoid partial JSON parsing on error.
When trying to parse the error message as JSON, consider fallback logic in case the message is invalid JSON or is a string that doesn’t parse correctly.
156-161:
Support file type inference.
Manually extracting the file extension from the last three characters can fail if the extension is more than three letters (like ‘.jpeg’). Consider parsing the MIME type in a more reliable way, or verifying file types more thoroughly.
- const filePath = fs.readFileSync(checkTmp(file), "base64");
- files.push(`data:application/${file.substr(file.length - 3)};base64,${filePath}`);
+ const extension = file.split('.').pop();
+ const mimeType = inferMimeType(extension); // implement a helper to map extension to MIME
+ files.push(`data:${mimeType};base64,${filePath}`);Likely invalid or redundant comment.
components/boldsign/package.json (1)
15-16: Match dependency version with pipeline environment.
You pinned @pipedream/platform to ^3.0.3. Verify that all existing references to older versions within the codebase have been updated to avoid potential conflicts.
components/boldsign/sources/new-document-declined/new-document-declined.mjs (1)
14-16: Verify error handling in listDocuments method
The getFunction returns this.boldsign.listDocuments directly. Ensure that the base implementation includes proper error handling for API failures.
components/boldsign/sources/new-document-sent/new-document-sent.mjs (2)
1-2: LGTM! Clean imports following ES module conventions.
The imports are well-structured, leveraging common functionality through base module inheritance.
14-25: Verify BoldSign API integration parameters.
The status and transmitType parameters in getParams() are crucial for correct API filtering.
✅ Verification successful
Based on the gathered information from examining the BoldSign component's codebase, I can now provide the final response:
The API parameters in new-document-sent.mjs appear to be correctly implemented
The implementation is consistent with other BoldSign document sources in the codebase. The parameters are properly handled:
- The
status: "None"parameter differentiates from other states like "Completed" and "Declined" used in other sources - The
transmitType: "Sent"parameter correctly filters for sent documents - The common base implementation in
common/base.mjsshows these parameters are properly passed to the API along with optional filters
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other BoldSign API usages to verify parameter consistency
rg -t js -t mjs "status.*None.*transmitType.*Sent" --no-filename
# Check if these parameters are documented in the BoldSign app file
rg -t js -t mjs "listDocuments.*parameters" components/boldsign/boldsign.app.mjs
Length of output: 205
Script:
#!/bin/bash
# Search for BoldSign API usages with correct file type
rg "status.*None.*transmitType.*Sent"
# Check the app file for API documentation and parameters
cat components/boldsign/boldsign.app.mjs
# Search for any other usage of listDocuments method
rg "listDocuments"
Length of output: 10040
Script:
#!/bin/bash
# Check BoldSign API documentation for document list parameters
curl -s https://api.boldsign.com/swagger/index.html | grep -A 10 "document/list"
# Search for other BoldSign document sources to compare parameters
cat components/boldsign/sources/new-document-completed/new-document-completed.mjs
cat components/boldsign/sources/new-document-declined/new-document-declined.mjs
Length of output: 1476
Script:
#!/bin/bash
# Check the common base file for BoldSign sources
cat components/boldsign/sources/common/base.mjs
# Check if there are any other files that might contain API documentation
rg -l "BoldSign.*API" --type md
Length of output: 2719
components/boldsign/sources/new-document-completed/new-document-completed.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.
LGTM!
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
🧹 Nitpick comments (2)
components/boldsign/boldsign.app.mjs (2)
40-44: Add validation for labels propertyThe labels property should include validation to ensure consistent formatting and prevent invalid input.
labels: { type: "string[]", label: "Labels", description: "Labels for categorizing documents.", + validate: (labels) => { + if (!labels.every((label) => typeof label === 'string' && label.trim())) { + throw new Error('All labels must be non-empty strings'); + } + }, },
131-159: Enhance pagination robustness and efficiencyThe pagination implementation could benefit from additional safeguards and efficiency improvements.
async *paginate({ - fn, params = {}, maxResults = null, ...opts + fn, params = {}, maxResults = null, maxPages = 100, ...opts }) { let hasMore = false; let count = 0; let page = 0; do { + if (page >= maxPages) { + console.warn(`Reached maximum page limit of ${maxPages}`); + return count; + } + params.page = ++page; const { result, pageDetails: { page: currentPage, totalPages, }, - } = await fn({ - params, - ...opts, - }); + } = await fn({ params, ...opts }) + .catch((error) => { + console.error(`Pagination error on page ${page}:`, error); + return { result: [], pageDetails: { page: 0, totalPages: 0 }}; + }); + + if (!Array.isArray(result)) { + console.error(`Expected array result on page ${page}`); + return count; + } for (const d of result) { yield d;
📜 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 (2)
components/boldsign/actions/send-document-template/send-document-template.mjs(1 hunks)components/boldsign/boldsign.app.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/boldsign/actions/send-document-template/send-document-template.mjs
🔇 Additional comments (2)
components/boldsign/boldsign.app.mjs (2)
1-6: LGTM!
The import and app definition follow standard patterns.
75-77: Verify API endpoint compatibility
Let's ensure the base URL and endpoints match BoldSign's API documentation.
✅ Verification successful
Base URL and endpoints are correctly implemented
The verification confirms that:
- The base URL
https://api.boldsign.com/v1matches BoldSign's official API endpoint - All API paths are correctly structured and match BoldSign's documented endpoints:
/template/list/brand/list/contacts/list/document/list/senderIdentities/list/template/send
- The URL construction is properly implemented using template literals with
_baseUrl() - No hardcoded URLs that bypass the
_baseUrl()method were found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all API endpoints used in the code match BoldSign's documentation
# Search for all API endpoint paths used in the code
echo "API endpoints used in the code:"
rg --no-filename "path: \".*\"" | sort -u
# Search for any hardcoded URLs that might bypass _baseUrl
echo -e "\nChecking for hardcoded URLs:"
rg "https?://" --no-filename | grep -v "_baseUrl"
Length of output: 81284
Script:
#!/bin/bash
# Search for API endpoints and URLs specifically in the boldsign app file
echo "Checking BoldSign API endpoints and URLs in the component:"
rg -A 3 "path:|url:|https?://" components/boldsign/boldsign.app.mjs
# Search for any BoldSign API related methods
echo -e "\nChecking BoldSign API methods:"
ast-grep --pattern 'async $METHODNAME($$$) { $$$}' components/boldsign/boldsign.app.mjs
Length of output: 990
|
/approve |
Resolves #14976.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores