Docusign - Download Documents #14831
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughA new module named Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
components/docusign/actions/download-documents/download-documents.mjs (3)
93-95: Simplify the file writing process by writing content directlyCurrently, the code converts
contentto a base64 string and then back to a buffer before writing. You can simplify this by writingcontentdirectly to the file since it's already in a buffer format.Apply this diff to streamline the code:
const content = await this.docusign._makeRequest({ // existing code }); - const rawcontent = content.toString("base64"); - const buffer = Buffer.from(rawcontent, "base64"); - fs.writeFileSync(filePath, buffer); + fs.writeFileSync(filePath, content);
71-75: Validate filename extension based on download typeTo prevent user errors, add validation to ensure the filename extension matches the selected download type. For example, if
downloadTypeis"combined"(PDF), the filename should end with"archive"(ZIP), it should end with.zip.Consider implementing a validator in the
filenameproperty:filename: { type: "string", label: "Filename", description: "The filename to save the file as in the `/tmp` directory including the file extension (.pdf or .zip)", + async validate(value) { + const expectedExtension = this.downloadType === "archive" ? ".zip" : ".pdf"; + if (!value.endsWith(expectedExtension)) { + throw new Error(`Filename must end with ${expectedExtension} for the selected download type.`); + } + }, },
98-109: Handle potential errors in therunmethodAdd error handling in the
runmethod to catch and handle any exceptions that might occur during the execution, such as network issues or API errors.Consider wrapping the code in a try-catch block:
async run({ $ }) { + try { const baseUri = await this.docusign.getBaseUri({ accountId: this.account, }); const envelope = await this.getEnvelope($, baseUri, this.envelopeId); const filePath = `/tmp/${this.filename}`; await this.downloadToTmp($, baseUri, envelope.documentsUri, filePath); $.export("$summary", `Successfully downloaded files to ${filePath}`); return filePath; + } catch (error) { + $.export("$summary", `Failed to download documents: ${error.message}`); + throw error; + } },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
components/docusign/actions/download-documents/download-documents.mjs(1 hunks)components/docusign/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/docusign/package.json
🔇 Additional comments (2)
components/docusign/actions/download-documents/download-documents.mjs (2)
89-89: Verify the construction of the download URL
Ensure that slicing the first character of documentsUri correctly handles the leading slash. If documentsUri doesn't start with a slash, this could lead to incorrect URLs.
Please confirm that documentsUri always starts with a /, and adjust the code if necessary.
22-46: 🛠️ Refactor suggestion
Handle pagination correctly when listing envelopes
In the options method for envelopeId, ensure that pagination is handled correctly. The current implementation may not properly fetch additional envelopes beyond the first page.
Consider updating the pagination logic to use nextUri or increment start_position. You might need to adjust how startPosition is updated:
const { startPosition = 0 } = prevContext || {};
// existing code
return {
options: envelopes.map(/* existing mapping */),
context: {
- startPosition: nextUri
+ startPosition: nextUri
? endPosition + 1
: undefined,
},
};Ensure that this logic correctly moves through the pages of results.
Likely invalid or redundant comment.
components/docusign/actions/download-documents/download-documents.mjs
Outdated
Show resolved
Hide resolved
Created new action "Download Documents" instead of adding a prop to New Change in Envelope Status because sources/triggers do not have access to the
/tmpdirectory for a workflow.Resolves #14805
Summary by CodeRabbit
New Features
Bug Fixes
Chores
@pipedream/docusignpackage from 0.2.0 to 0.2.1.