New Components - papersign #14423
New Components - papersign #14423
Conversation
luancazarine
added
the
ai-assisted
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThe pull request introduces multiple new modules and functionalities to the Papersign application. It includes actions for copying, retrieving, and sending documents, along with constants and utility functions. New event sources are also added to handle document completion and signer actions. Additionally, the package version is updated, and various properties and methods are defined to enhance API interactions and document management capabilities. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
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 Event (Instant) - New Document Completed (Instant) - New Signer Signed (Instant) Actions - Copy Document - Get Document - Send Document
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
🧹 Outside diff range and nitpick comments (13)
components/papersign/common/constants.mjs (2)
1-1: Add documentation for the LIMIT constant.The constant looks good, but adding a JSDoc comment would help clarify its purpose in pagination or other limiting contexts.
+/** Maximum number of items to return in paginated responses */ export const LIMIT = 100;
3-12: Add documentation and consider type definitions for SCOPE_OPTIONS.The scope options are well-structured for folder-based document organization. Consider adding documentation and type definitions for better maintainability.
+/** + * Options for configuring webhook scope in folder hierarchies + * @type {Array<{label: string, value: 'folder.direct_children' | 'folder.all_descendants'}>} + */ export const SCOPE_OPTIONS = [ { label: "Direct Children", value: "folder.direct_children", }, { label: "All Descendants", value: "folder.all_descendants", }, ];components/papersign/sources/new-document-completed-instant/test-event.mjs (1)
2-3: Consider using distinct IDs for event and document.The event
idanddocument_idare currently identical. In real scenarios, these would typically be different as they serve different purposes - one identifies the event itself while the other references the document."id": "671ba9030af55bff3b7a8c00", - "document_id": "671ba9030af55bff3b7a8c00", + "document_id": "5f9b2c1d4e3a2b1a0c8d9e7f",components/papersign/sources/new-signer-signed-instant/new-signer-signed-instant.mjs (2)
4-11: Add version update guidelines in the documentation.The metadata is well-structured and follows the component naming conventions. Consider adding documentation about version update guidelines to help maintain semantic versioning as the component evolves.
version: "0.0.1", +// Version guidelines: +// - Patch (0.0.x): Bug fixes and minor changes +// - Minor (0.x.0): New features in a backward compatible manner +// - Major (x.0.0): Breaking changes
1-24: Add JSDoc documentation for better maintainability.While the implementation satisfies the requirements, adding JSDoc documentation would improve maintainability and developer experience.
+/** + * @typedef {Object} SignerData + * @property {Object} by - Information about the signer + * @property {string} by.name - Name of the signer + */ +/** + * Webhook source that emits events when a signer signs a document. + * @extends {CommonBase} + */ export default { ...common,components/papersign/sources/new-document-completed-instant/new-document-completed-instant.mjs (1)
4-11: Consider starting with version 1.0.0The component configuration is well-structured and matches the PR objectives. However, since this is a production-ready feature (not experimental), consider starting with version 1.0.0 instead of 0.0.1.
- version: "0.0.1", + version: "1.0.0",components/papersign/common/utils.mjs (1)
1-1: Add documentation and consider renaming.The function would benefit from proper documentation and a more specific name that reflects its purpose in the papersign context.
Consider adding JSDoc documentation:
+/** + * Recursively parses JSON strings within an object or array structure. + * Used for processing papersign API responses that may contain stringified JSON. + * + * @param {any} obj - The input to parse + * @returns {any} The parsed object with any JSON strings converted to objects + * @throws {Error} If circular references are detected + * + * @example + * parseSignaturePayload('{"data": "{\\"key\\": \\"value\\"}"}') + * // Returns: {data: {key: "value"}} + */ -export const parseObject = (obj) => { +export const parseSignaturePayload = (obj) => {components/papersign/actions/get-document/get-document.mjs (1)
1-8: LGTM! Consider enhancing the documentation.The module structure and metadata are well-defined. The documentation link is helpful, but consider adding a brief example of the expected response format in the description.
- description: "Retrieve a document using a specified ID. [See the documentation](https://paperform.readme.io/reference/getpapersigndocument)", + description: "Retrieve a document using a specified ID. Returns the document details including status, signers, and metadata. [See the documentation](https://paperform.readme.io/reference/getpapersigndocument)",components/papersign/sources/common/base.mjs (1)
4-65: Add JSDoc documentation for the base module.This base module is meant to be extended by specific webhook source implementations. Adding documentation would help developers understand:
- Required methods to implement (getTriggers, getSummary)
- Available hooks and their lifecycle
- Props configuration
- Usage examples
Add documentation at the top of the file:
import { SCOPE_OPTIONS } from "../../common/constants.mjs"; import papersign from "../../papersign.app.mjs"; +/** + * @typedef {Object} WebhookBody + * @property {string} id - Unique identifier of the webhook event + * @property {string} created_at - ISO timestamp of the event + */ + +/** + * Base class for Papersign webhook sources + * @abstract + */ export default {components/papersign/papersign.app.mjs (4)
13-17: Remove unnecessaryawaitin thereturnstatement.In an async function, using
return awaitis redundant because the function will automatically return a promise. You can simplify the code by removingawait.Apply this diff to fix the issue:
async options({ page }) { - return await this.list({ + return this.list({ module: "documents", page, }); },
23-28: Remove unnecessaryawaitin thereturnstatement.Similar to the previous comment, you can remove the
awaitto streamline the code.Apply this diff to fix the issue:
async options({ page }) { - return await this.list({ + return this.list({ module: "spaces", page, }); },
34-39: Remove unnecessaryawaitin thereturnstatement.Again, removing
awaitsimplifies the function without affecting functionality.Apply this diff to fix the issue:
async options({ page }) { - return await this.list({ + return this.list({ module: "folders", page, }); },
115-119: Align parameter structure indeleteWebhookmethod for consistency.For consistency and future extensibility, consider destructuring the parameters and allowing additional options, similar to other methods.
Apply this diff to adjust the parameter structure:
-deleteWebhook(hookId) { +deleteWebhook({ + hookId, ...opts +}) { return this._makeRequest({ method: "DELETE", path: `/webhooks/${hookId}`, + ...opts, }); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
- components/papersign/actions/copy-document/copy-document.mjs (1 hunks)
- components/papersign/actions/get-document/get-document.mjs (1 hunks)
- components/papersign/actions/send-document/send-document.mjs (1 hunks)
- components/papersign/common/constants.mjs (1 hunks)
- components/papersign/common/utils.mjs (1 hunks)
- components/papersign/package.json (2 hunks)
- components/papersign/papersign.app.mjs (1 hunks)
- components/papersign/sources/common/base.mjs (1 hunks)
- components/papersign/sources/new-document-completed-instant/new-document-completed-instant.mjs (1 hunks)
- components/papersign/sources/new-document-completed-instant/test-event.mjs (1 hunks)
- components/papersign/sources/new-event-instant/new-event-instant.mjs (1 hunks)
- components/papersign/sources/new-event-instant/test-event.mjs (1 hunks)
- components/papersign/sources/new-signer-signed-instant/new-signer-signed-instant.mjs (1 hunks)
- components/papersign/sources/new-signer-signed-instant/test-event.mjs (1 hunks)
✅ Files skipped from review due to trivial changes (2)
- components/papersign/sources/new-event-instant/test-event.mjs
- components/papersign/sources/new-signer-signed-instant/test-event.mjs
🔇 Additional comments (17)
components/papersign/sources/new-document-completed-instant/test-event.mjs (1)
7-7: Review security implications of the test document URL.The test URL contains a signature parameter that, while fake, follows a pattern that might be similar to production. Consider using a more obviously fake signature pattern for test data to avoid any potential security implications.
components/papersign/package.json (2)
3-3: Version bump follows semantic versioning.The version increment from 0.0.1 to 0.1.0 appropriately reflects the addition of new features (webhook sources and document actions) without breaking changes.
15-17: Verify @pipedream/platform compatibility.The addition of @pipedream/platform dependency is correct for implementing Pipedream components. Let's verify this is the latest compatible version for the new features being added.
components/papersign/sources/new-signer-signed-instant/new-signer-signed-instant.mjs (1)
1-2: LGTM! Imports are properly structured.The imports correctly reference the common base module and test events, following the component organization pattern.
components/papersign/sources/new-document-completed-instant/new-document-completed-instant.mjs (2)
1-2: LGTM! Imports are well-structuredThe imports follow good practices by reusing common functionality and separating test events.
23-23: Verify test event structureLet's ensure the test event includes all necessary fields for proper testing and documentation.
✅ Verification successful
Test event structure is properly defined and consistent
The test event includes all essential fields for the
document.completedwebhook:
- Unique identifiers (
id,document_id)- Document metadata (
document_name,type)- Document access URL in the data object
- Timestamp information (
created_at)The structure is also consistent with other PaperSign document completion test events in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the test event structure matches the expected webhook payload # Test: Check the content of test event file cat "components/papersign/sources/new-document-completed-instant/test-event.mjs" # Test: Look for similar test events in other papersign components for consistency fd test-event.mjs -t f | grep papersign | xargs catLength of output: 1509
components/papersign/common/utils.mjs (2)
1-32: Verify handling of all papersign data types.Since this utility will be used across various papersign components (document copying, retrieval, and webhook processing), we should verify it handles all expected data types correctly.
Let's check the API response formats:
#!/bin/bash # Search for sample API responses or type definitions in the codebase rg -A 10 "interface.*Document|type.*Document" --type typescript rg -A 10 "interface.*Signature|type.*Signature" --type typescript
16-22:⚠️ Potential issueImprove error handling and add JSON validation.
The current implementation silently swallows JSON parsing errors and might process untrusted input without validation.
Consider these improvements:
if (typeof obj === "string") { try { - return JSON.parse(obj); + const parsed = JSON.parse(obj); + // Validate parsed result is an object or array + if (parsed && typeof parsed === 'object') { + return parseObject(parsed); + } + return parsed; } catch (e) { + // Log parsing errors for debugging + console.debug(`Failed to parse JSON string: ${e.message}`); return obj; } }Let's verify if this utility is used with untrusted input:
components/papersign/actions/get-document/get-document.mjs (1)
9-17: 🛠️ Refactor suggestionConsider adding input validation for documentId.
While the prop is correctly defined using the app's propDefinition, consider adding validation to ensure the documentId is in the correct format before making the API call.
Let's check how documentId is validated in the app file:
If validation is not already handled in the app, consider adding:
documentId: { propDefinition: [ papersign, "documentId", ], + validate: (value) => { + if (!/^[a-zA-Z0-9-]+$/.test(value)) { + throw new Error("Document ID must contain only alphanumeric characters and hyphens"); + } + }, },components/papersign/sources/new-event-instant/new-event-instant.mjs (3)
4-11: LGTM! Component metadata is well-definedThe component metadata is properly configured with:
- Descriptive name and description
- Appropriate deduplication strategy for webhook events
- Correct version numbering for a new component
1-2: Verify test event implementationLet's ensure the test event file exists and contains appropriate test data for this webhook source.
#!/bin/bash # Check if test-event.mjs exists and verify its contents rg -l "test-event.mjs" --type js cat $(fd -t f "test-event.mjs$")
14-27: Verify event types against Papersign API documentationLet's ensure we've covered all possible event types from the Papersign API.
✅ Verification successful
Let me search specifically for Papersign documentation and webhook event types.
Let me check the Papersign API documentation in the repository to verify the event types.
Based on the code analysis and search results, I can now provide a final response:
All Papersign webhook event types are properly implemented
The implementation in
new-event-instant.mjscovers all the standard Papersign document and signer events:Document events:
- document.sent
- document.completed
- document.cancelled
- document.rejected
- document.expired
Signer events:
- signer.notified
- signer.viewed
- signer.consent_accepted
- signer.nominated
- signer.signed
The component also includes dedicated sources for specific events (e.g.,
new-signer-signed-instant,new-document-completed-instant), showing consistent event type usage across the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any API documentation files that might list event types fd -t f -e md -e txt . | xargs rg -l "papersign.*event.*type|webhook.*event"Length of output: 2171
Script:
#!/bin/bash # Search for Papersign documentation and webhook events rg -A 10 -B 10 "papersign.*webhook|document\.(sent|completed|cancelled)" docs/Length of output: 80
Script:
#!/bin/bash # Search for any Papersign related files fd papersign . | xargs catLength of output: 2788
Script:
#!/bin/bash # Search for event types in the component code rg -A 10 "getTriggers|getEvents" components/papersign/Length of output: 4244
Script:
#!/bin/bash # Search for Papersign API documentation cat components/papersign/README.mdLength of output: 99
Script:
#!/bin/bash # Search for any documentation files containing Papersign webhook events rg -A 5 "Papersign.*webhook|Papersign.*event" components/papersign/Length of output: 69
Script:
#!/bin/bash # Look for test files that might contain event types fd -t f test components/papersign/ | xargs catLength of output: 1019
components/papersign/sources/common/base.mjs (1)
1-2: Verify the imported modules exist.The imports look correct, but let's verify they exist in the codebase.
components/papersign/actions/copy-document/copy-document.mjs (2)
1-8: LGTM! Component metadata is well-defined.The component follows Pipedream's best practices with proper naming, versioning, and documentation linking.
9-43: Verify destination specification handling.While the run method correctly prioritizes
folderIdoverspaceId+path, consider adding validation in the props to prevent users from specifying both destination methods simultaneously.Consider adding a custom validation function to ensure only one destination method is specified:
validate(configuredProps) { if (configuredProps.folderId && (configuredProps.spaceId || configuredProps.path)) { throw new Error("Please specify either folderId OR spaceId/path, not both"); } }components/papersign/actions/send-document/send-document.mjs (2)
1-10: LGTM! Module setup and imports are well-structured.The imports are appropriate, and the module metadata is properly defined with documentation links.
98-111: Verify integration with webhook components.Since this action is part of a larger Papersign integration that includes webhook components for document events, ensure that the document ID format in the response matches what the webhook components expect.
✅ Verification successful
Let me gather more information about the document ID format in the action component to compare with the webhook format.
Let me check the papersign app definition to see how the documentId prop is defined.
Let me try a different approach to check the app's property definitions.
Based on the results from our verification attempts, I can now provide a final response:
Document ID format is consistent across components
The webhook components (
new-document-completed-instantandnew-event-instant) expect document IDs in the format "671ba9030af55bff3b7a8c00" (24-character hexadecimal), and the action components consistently use the samedocumentIdparameter across all operations (send, get, copy), maintaining compatibility with the webhook format.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check document ID format consistency across components # Look for document ID handling in webhook components rg -A 3 "documentId|document_id" components/papersign/sources/Length of output: 865
Script:
#!/bin/bash # Check document ID handling in the action component and API client rg -A 3 "documentId" components/papersign/actions/Length of output: 3306
Script:
#!/bin/bash # Check documentId property definition in the app ast-grep --pattern 'documentId: { $$$ }'Length of output: 44
Script:
#!/bin/bash # Look for documentId property definition in the app rg -A 10 "documentId.*type|document_id.*type" components/papersign/Length of output: 69
components/papersign/sources/new-document-completed-instant/test-event.mjs
Show resolved
Hide resolved
components/papersign/sources/new-signer-signed-instant/new-signer-signed-instant.mjs
Show resolved
Hide resolved
components/papersign/sources/new-signer-signed-instant/new-signer-signed-instant.mjs
Show resolved
Hide resolved
components/papersign/sources/new-document-completed-instant/new-document-completed-instant.mjs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
components/papersign/sources/common/base.mjs (1)
24-29: Add a default value for the scope property.The scope property should have a default value to ensure predictable behavior when not explicitly set.
scope: { type: "string", label: "Scope", description: "The scope of the webhook", options: SCOPE_OPTIONS, + default: SCOPE_OPTIONS[0], },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
components/papersign/sources/common/base.mjs(1 hunks)
🔇 Additional comments (2)
components/papersign/sources/common/base.mjs (2)
1-4: LGTM! Imports and module structure are well-organized.
The imports are properly structured and the module follows the recommended patterns.
1-64: Verify base implementation supports all webhook types.
Please ensure this base implementation supports all three webhook types mentioned in the PR objectives:
- new-event-instant
- new-document-completed-instant
- new-signer-signed-instant
✅ Verification successful
Let me verify the implementation details of these webhook sources to ensure they properly extend and utilize the base implementation.
Base implementation successfully supports all webhook types
The verification confirms that the base implementation properly supports all three webhook sources:
- All sources correctly import and extend the base implementation
- Each source implements the required
getTriggers()method to specify its webhook triggers - Each source implements the required
getSummary()method with appropriate event-specific formatting - The inheritance pattern allows for proper code reuse while maintaining source-specific customizations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for webhook source implementations
echo "Searching for webhook source implementations..."
fd -e mjs -e js . components/papersign/sources/ --exec grep -l "new-.*-instant"
# Verify they extend this base implementation
echo "Verifying base implementation usage..."
ast-grep --pattern 'import $base from "../common/base.mjs"
export default {
...base,
$$$
}'
Length of output: 620
Script:
#!/bin/bash
# Check implementation of new-event-instant
echo "=== new-event-instant implementation ==="
cat components/papersign/sources/new-event-instant/new-event-instant.mjs
echo -e "\n=== new-document-completed-instant implementation ==="
cat components/papersign/sources/new-document-completed-instant/new-document-completed-instant.mjs
echo -e "\n=== new-signer-signed-instant implementation ==="
cat components/papersign/sources/new-signer-signed-instant/new-signer-signed-instant.mjs
Length of output: 2528
|
/approve |
* papersign init * [Components] papersign #14412 Sources - New Event (Instant) - New Document Completed (Instant) - New Signer Signed (Instant) Actions - Copy Document - Get Document - Send Document * pnpm update * fix method call * remove optional true from scope
Resolves #14412.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Version Update