17949 app luminpdf #18023
17949 app luminpdf #18023
Conversation
Actions - Get Signature Request - Send Signature Request - Cancel Signature Request - Download File as File URL - Download File - Get User Information
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
WalkthroughAdds an Axios-based Lumin PDF app client, six new actions for signature-request and file operations, a parseObject utility, and package metadata changes; actions call the app client's methods, export brief summaries via $, and return API responses or saved files. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action
participant LuminPdfApp
participant LuminAPI
User->>Action: provide inputs (e.g., signatureRequestId, files, params)
Action->>LuminPdfApp: call method({ $, params })
LuminPdfApp->>LuminAPI: HTTP request with x-api-key
LuminAPI-->>LuminPdfApp: response (JSON or stream)
LuminPdfApp-->>Action: response
Action-->>User: $.export("$summary") + returned data
sequenceDiagram
participant User
participant DownloadFileAction
participant LuminPdfApp
participant LuminAPI
participant FS as Filesystem
User->>DownloadFileAction: signatureRequestId, filePath
DownloadFileAction->>LuminPdfApp: downloadFile({ responseType: stream })
LuminPdfApp->>LuminAPI: GET /signature_request/files/{id}
LuminAPI-->>LuminPdfApp: Stream
LuminPdfApp-->>DownloadFileAction: Stream
DownloadFileAction->>FS: pipe stream to filePath
DownloadFileAction-->>User: { filePath }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 11
🧹 Nitpick comments (7)
components/lumin_pdf/actions/cancel-signature-request/cancel-signature-request.mjs (1)
29-29: Minor copy/style nit: standardize US English “canceled”Use “canceled” (single L) to align with US English and the action name.
Apply this diff:
- $.export("$summary", `Successfully cancelled signature request with ID: ${this.signatureRequestId}`); + $.export("$summary", `Successfully canceled signature request with ID: ${this.signatureRequestId}`);components/lumin_pdf/actions/download-file/download-file.mjs (2)
25-30: syncDir is unusedThe
syncDirprop is declared but not used. Either remove it to reduce surface area or use it to provide a defaultfilePath(e.g.,${syncDir}/<signatureRequestId>.pdf) whenfilePathis not provided.Example approach (requires making
filePathoptional):- filePath: { - type: "string", - label: "File Path", - description: "The path to the file you'd like to download eg. `/tmp/file.pdf`", - }, + filePath: { + type: "string", + label: "File Path", + description: "The path to write the downloaded file, e.g. `/tmp/file.pdf`. If not provided, a file is created under the selected directory.", + optional: true, + },And inside
run:- const response = await this.luminPdf.downloadFile({ + const response = await this.luminPdf.downloadFile({ $, signatureRequestId: this.signatureRequestId, responseType: "stream", }); - const pipeline = promisify(stream.pipeline); - const readable = response?.data ?? response; - await pipeline(readable, fs.createWriteStream(this.filePath)); + const outPath = this.filePath || `${this.syncDir}/luminpdf-${this.signatureRequestId}.pdf`; + const pipeline = promisify(stream.pipeline); + const readable = response?.data ?? response; + await pipeline(readable, fs.createWriteStream(outPath)); - $.export("$summary", `Successfully downloaded file for signature request with ID: ${this.signatureRequestId}`); + $.export("$summary", `Successfully downloaded file for signature request with ID: ${this.signatureRequestId}`); return { - filePath: this.filePath, + filePath: outPath, };
1-4: Optional: use native promise-based pipelineYou can simplify by importing
pipelinefromnode:stream/promisesand droppromisify+streamimports.Apply this diff:
-import fs from "fs"; -import stream from "stream"; -import { promisify } from "util"; +import fs from "fs"; +import { pipeline } from "node:stream/promises"; import luminPdf from "../../lumin_pdf.app.mjs";And update usage:
- const pipeline = promisify(stream.pipeline); - const readable = response?.data ?? response; - await pipeline(readable, fs.createWriteStream(this.filePath)); + const readable = response?.data ?? response; + await pipeline(readable, fs.createWriteStream(this.filePath));components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (4)
61-66: Allow explicit false for useTextTagsCurrent check only appends when truthy. If users explicitly set false (to override a default true), it won’t be sent. The diff above adjusts to append whenever defined and stringifies the boolean, which is safer for multipart form fields.
35-40: Typos and clarity infilesprop descriptionMinor grammar/clarity issues and a self-reference:
- “An array of path” → “An array of paths”
- Mutually exclusive list should reference “File”, “File URL”, and “File URLs” (not “Files” itself)
Apply this diff:
- description: "An array of path to files in the `/tmp` directory (for example, `/tmp/myFile.html`) to be sent for signature. This field is mutually exclusive with `File URL`, `Files`, and `File URLs`. Only one of these fields should be provided in the request.", + description: "An array of paths to files in the `/tmp` directory (e.g., `/tmp/myFile.html`) to be sent for signature. This field is mutually exclusive with `File`, `File URL`, and `File URLs`. Only one of these fields should be provided in the request.",
47-50: Typo inviewersexampleThe example array is missing a closing brace before the last bracket.
Apply this diff:
- description: "A list of objects of viewers to add to your Signature Request. Format: `[{'email_address': '[email protected]', 'name': 'John Doe'}, {'email_address': '[email protected]', 'name': 'Jane Doe']`. [See the documentation](https://developers.luminpdf.com/api/send-signature-request/) for more information.", + description: "A list of objects of viewers to add to your Signature Request. Format: `[{'email_address': '[email protected]', 'name': 'John Doe'}, {'email_address': '[email protected]', 'name': 'Jane Doe'}]`. [See the documentation](https://developers.luminpdf.com/api/send-signature-request/) for more information.",
90-94: Clarify label to avoid confusion with request titleThe “Title” label here refers to the custom email title, which can be confused with the Signature Request “Title”. Consider renaming the label for clarity.
Apply this diff:
- label: "Title", + label: "Custom Email Title",
📜 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 (9)
components/lumin_pdf/actions/cancel-signature-request/cancel-signature-request.mjs(1 hunks)components/lumin_pdf/actions/download-file-as-file-url/download-file-as-file-url.mjs(1 hunks)components/lumin_pdf/actions/download-file/download-file.mjs(1 hunks)components/lumin_pdf/actions/get-signature-request/get-signature-request.mjs(1 hunks)components/lumin_pdf/actions/get-user-information/get-user-information.mjs(1 hunks)components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs(1 hunks)components/lumin_pdf/common/utils.mjs(1 hunks)components/lumin_pdf/lumin_pdf.app.mjs(1 hunks)components/lumin_pdf/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
PR: PipedreamHQ/pipedream#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/lumin_pdf/package.json
🧬 Code Graph Analysis (6)
components/lumin_pdf/actions/download-file-as-file-url/download-file-as-file-url.mjs (5)
components/lumin_pdf/actions/cancel-signature-request/cancel-signature-request.mjs (1)
response(24-27)components/lumin_pdf/actions/get-user-information/get-user-information.mjs (1)
response(13-15)components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (1)
response(182-186)components/lumin_pdf/actions/get-signature-request/get-signature-request.mjs (1)
response(19-22)components/lumin_pdf/actions/download-file/download-file.mjs (1)
response(32-36)
components/lumin_pdf/actions/cancel-signature-request/cancel-signature-request.mjs (2)
components/lumin_pdf/actions/get-signature-request/get-signature-request.mjs (1)
response(19-22)components/lumin_pdf/actions/download-file-as-file-url/download-file-as-file-url.mjs (1)
response(19-22)
components/lumin_pdf/actions/get-user-information/get-user-information.mjs (5)
components/lumin_pdf/actions/cancel-signature-request/cancel-signature-request.mjs (1)
response(24-27)components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (1)
response(182-186)components/lumin_pdf/actions/get-signature-request/get-signature-request.mjs (1)
response(19-22)components/lumin_pdf/actions/download-file/download-file.mjs (1)
response(32-36)components/lumin_pdf/actions/download-file-as-file-url/download-file-as-file-url.mjs (1)
response(19-22)
components/lumin_pdf/actions/get-signature-request/get-signature-request.mjs (5)
components/lumin_pdf/actions/cancel-signature-request/cancel-signature-request.mjs (1)
response(24-27)components/lumin_pdf/actions/get-user-information/get-user-information.mjs (1)
response(13-15)components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (1)
response(182-186)components/lumin_pdf/actions/download-file/download-file.mjs (1)
response(32-36)components/lumin_pdf/actions/download-file-as-file-url/download-file-as-file-url.mjs (1)
response(19-22)
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (6)
components/lumin_pdf/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)components/lumin_pdf/actions/cancel-signature-request/cancel-signature-request.mjs (1)
response(24-27)components/lumin_pdf/actions/get-user-information/get-user-information.mjs (1)
response(13-15)components/lumin_pdf/actions/get-signature-request/get-signature-request.mjs (1)
response(19-22)components/lumin_pdf/actions/download-file/download-file.mjs (1)
response(32-36)components/lumin_pdf/actions/download-file-as-file-url/download-file-as-file-url.mjs (1)
response(19-22)
components/lumin_pdf/actions/download-file/download-file.mjs (1)
components/lumin_pdf/actions/download-file-as-file-url/download-file-as-file-url.mjs (1)
response(19-22)
⏰ 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). (4)
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
🔇 Additional comments (7)
components/lumin_pdf/package.json (2)
3-3: Version bump looks appropriate.Minor version bump reflects new features. No concerns here.
15-17: Dependency choice aligns with platform standards.Adding “@pipedream/platform” is consistent with using the platform’s axios wrapper. Also validated against org learning: no built-in Node modules added as deps.
components/lumin_pdf/actions/download-file-as-file-url/download-file-as-file-url.mjs (1)
3-27: LGTM — action wiring and summary look good.Action correctly uses the app’s propDefinition and method, and exports a useful summary.
components/lumin_pdf/actions/cancel-signature-request/cancel-signature-request.mjs (1)
23-31: Straightforward action wiring looks goodThe action cleanly delegates to the app client and exports a clear summary. No functional issues spotted here.
components/lumin_pdf/actions/get-signature-request/get-signature-request.mjs (1)
18-26: LGTM: Correct use of app client and summaryThe implementation aligns with the established action pattern and uses the app client properly. Looks good.
components/lumin_pdf/actions/download-file/download-file.mjs (1)
31-39: We need to confirm what_makeRequestreturns to know whether it yields a stream directly or wraps it insideresponse.data. Let's inspect its implementation.components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (1)
176-181: Verifycustom_emailpayload field namesIt looks like the
custom_emailobject is usingsubject_name, but most Lumin API examples usesubject. Please confirm against the official Lumin API spec and align these keys accordingly. For example, if the API expectssubjectyou can update:- if (this.subject) customEmail.subject_name = this.subject; + if (this.subject) customEmail.subject = this.subject;– Ensure all
custom_emailfields (sender_email,subject,title, etc.) exactly match the API documentation.
components/lumin_pdf/actions/get-user-information/get-user-information.mjs
Show resolved
Hide resolved
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs
Show resolved
Hide resolved
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs
Show resolved
Hide resolved
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs
Outdated
Show resolved
Hide resolved
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs
Show resolved
Hide resolved
michelle0927
left a comment
michelle0927
left a comment
There was a problem hiding this comment.
Just one comment: The form-data dependency should be added to package.json.
Ready for QA!
Hi @michelle0927 form-data is native, so there is no need to include it |
… file type and ensure useTextTags is appended as a string
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (5)
116-128: Mutually exclusive file inputs: empty arrays/strings are treated as present[] is truthy, so the current presence check can accept empty arrays/strings and bypass validation. Count only non-empty values.
- const formData = new FormData(); - const checkFiles = {}; - if (this.file) checkFiles.file = this.file; - if (this.files) checkFiles.files = this.files; - if (this.fileUrl) checkFiles.fileUrl = this.fileUrl; - if (this.fileUrls) checkFiles.fileUrls = this.fileUrls; - - if (Object.keys(checkFiles).length > 1) { + const formData = new FormData(); + const hasFile = typeof this.file === "string" && this.file.trim().length > 0; + const hasFiles = Array.isArray(this.files) && this.files.length > 0; + const hasFileUrl = typeof this.fileUrl === "string" && this.fileUrl.trim().length > 0; + const hasFileUrls = Array.isArray(this.fileUrls) && this.fileUrls.length > 0; + const providedCount = [hasFile, hasFiles, hasFileUrl, hasFileUrls].filter(Boolean).length; + + if (providedCount > 1) { throw new ConfigurationError("Only one of `File URL`, `File`, `File URLs`, or `Files` should be provided in the request."); } - if (Object.keys(checkFiles).length === 0) { + if (providedCount === 0) { throw new ConfigurationError("At least one of `File URL`, `File`, `File URLs`, or `Files` should be provided in the request."); }
152-161: Validate signer objects before iterating keysparseObject may yield non-object entries (e.g., unparsed strings). Guard before Object.keys to avoid malformed fields.
- if (this.signers) { - for (const [ - index, - signer, - ] of parseObject(this.signers).entries()) { - for (const item of Object.keys(signer)) { - formData.append(`signers[${index}][${item}]`, signer[item]); - } - } - } + if (this.signers) { + const signers = parseObject(this.signers); + for (const [index, signer] of signers.entries()) { + if (typeof signer !== "object" || signer === null || Array.isArray(signer)) { + throw new ConfigurationError(`Invalid signer at index ${index}: expected an object, got ${typeof signer}`); + } + for (const key of Object.keys(signer)) { + formData.append(`signers[${index}][${key}]`, signer[key]); + } + } + }
162-171: Validate viewer objects before iterating keysSame issue as signers: ensure each viewer is a plain object before iterating.
- if (this.viewers) { - for (const [ - index, - viewer, - ] of parseObject(this.viewers).entries()) { - for (const item of Object.keys(viewer)) { - formData.append(`viewers[${index}][${item}]`, viewer[item]); - } - } - } + if (this.viewers) { + const viewers = parseObject(this.viewers); + for (const [index, viewer] of viewers.entries()) { + if (typeof viewer !== "object" || viewer === null || Array.isArray(viewer)) { + throw new ConfigurationError(`Invalid viewer at index ${index}: expected an object, got ${typeof viewer}`); + } + for (const key of Object.keys(viewer)) { + formData.append(`viewers[${index}][${key}]`, viewer[key]); + } + } + }
172-176: expiresAt should be ISO 8601 string, not milliseconds; append boolean even when falseDocs and your prop description specify ISO 8601. Date.parse produces a number and may break the API. Also, append use_text_tags when defined so false is transmitted.
- if (this.title) formData.append("title", this.title); - if (this.expiresAt) formData.append("expires_at", Date.parse(this.expiresAt)); - if (this.useTextTags) formData.append("use_text_tags", `${this.useTextTags}`); + if (this.title) formData.append("title", this.title); + if (this.expiresAt) { + if (Number.isNaN(Date.parse(this.expiresAt))) { + throw new ConfigurationError("Invalid `Expires At` value. Provide an ISO 8601 datetime, e.g., 2025-01-31T23:59:59Z."); + } + formData.append("expires_at", this.expiresAt); + } + if (this.useTextTags !== undefined) formData.append("use_text_tags", String(this.useTextTags)); if (this.signingType) formData.append("signing_type", this.signingType);
188-190: Make the summary resilient to response shapeAvoid throwing if signature_request or signature_request_id is missing; fallback gracefully.
- if (response) { - $.export("$summary", `Successfully sent signature request ${response.signature_request.signature_request_id}`); - } + if (response) { + const id = response?.signature_request?.signature_request_id ?? response?.id ?? "<unknown>"; + $.export("$summary", `Successfully sent signature request ${id}`); + }
🧹 Nitpick comments (2)
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (2)
17-40: Mutually exclusive file inputs: fix wording and typos in prop descriptionsMinor copy issues and inconsistencies:
- “Files” description says “mutually exclusive with File URL, Files, and File URLs” (includes itself).
- “An array of path” → “An array of paths”.
- Inconsistent capitalization of “File URL(s)”.
Apply this diff to polish copy:
@@ - description: "The URL of a single file to be downloaded and signed. This field is mutually exclusive with `file`, `files`, and `File URLs`. Only one of these fields should be provided in the request.", + description: "The URL of a single file to be downloaded and signed. This field is mutually exclusive with `File`, `Files`, and `File URLs`. Only one of these fields should be provided in the request.", @@ - description: "A single path to a file in the `/tmp` directory (for example, `/tmp/myFile.pdf`) to be sent for signature. This field is mutually exclusive with `File URL`, `Files`, and `File URLs`. Only one of these fields should be provided in the request.", + description: "A single path to a file in the `/tmp` directory (for example, `/tmp/myFile.pdf`) to be sent for signature. This field is mutually exclusive with `File URL`, `Files`, and `File URLs`. Only one of these fields should be provided in the request.", @@ - description: "An array of URLs of files to be downloaded and signed. This field is mutually exclusive with `File`, `Files`, and `File URL`. Only one of these fields should be provided in the request.", + description: "An array of URLs of files to be downloaded and signed. This field is mutually exclusive with `File`, `Files`, and `File URL`. Only one of these fields should be provided in the request.", @@ - description: "An array of path to files in the `/tmp` directory (for example, `/tmp/myFile.pdf`) to be sent for signature. This field is mutually exclusive with `File URL`, `Files`, and `File URLs`. Only one of these fields should be provided in the request.", + description: "An array of paths to files in the `/tmp` directory (for example, `/tmp/myFile.pdf`) to be sent for signature. This field is mutually exclusive with `File URL`, `File`, and `File URLs`. Only one of these fields should be provided in the request.",
89-94: Disambiguate label to avoid duplicate “Title” propsThere are two “Title” labels (for request title and custom email title). Rename this one for clarity.
customEmailTitle: { type: "string", - label: "Title", + label: "Custom Email Title", description: "The title of the email.", optional: true, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-09T18:07:12.426Z
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#17538
File: components/aircall/sources/new-sms/new-sms.mjs:19-25
Timestamp: 2025-07-09T18:07:12.426Z
Learning: In Aircall API webhook payloads, the `created_at` field is returned as an ISO 8601 string format (e.g., "2020-02-18T20:52:22.000Z"), not as milliseconds since epoch. For Pipedream components, this needs to be converted to milliseconds using `Date.parse()` before assigning to the `ts` field in `generateMeta()`.
Applied to files:
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs
🧬 Code Graph Analysis (1)
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (6)
components/lumin_pdf/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)components/lumin_pdf/actions/download-file-as-file-url/download-file-as-file-url.mjs (1)
response(19-22)components/lumin_pdf/actions/cancel-signature-request/cancel-signature-request.mjs (1)
response(24-27)components/lumin_pdf/actions/download-file/download-file.mjs (1)
response(32-36)components/lumin_pdf/actions/get-signature-request/get-signature-request.mjs (1)
response(19-22)components/lumin_pdf/actions/get-user-information/get-user-information.mjs (1)
response(13-15)
⏰ 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). (4)
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
🔇 Additional comments (2)
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (2)
95-100: syncDir is unusedsyncDir is declared but never referenced. If it’s intended to grant read access to /tmp for getFileStreamAndMetadata, it’s fine to keep. Otherwise, remove for clarity.
Do you want to keep syncDir for consistency with other file-based actions, or should I remove it and update the description of File/Files accordingly?
176-180: Confirm custom_email shape matches APIYou’re sending custom_email as a JSON string with keys sender_email, subject_name, and title. Please confirm these exact field names and that JSON stringification is expected by the endpoint (some APIs expect form fields like custom_email[sender_email]).
I can check the luminpdf client implementation and docs for the exact payload shape and generate the correct mapping. Want me to proceed?
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs
Show resolved
Hide resolved
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs
Show resolved
Hide resolved
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs
Show resolved
Hide resolved
…d object structure for better clarity and update label for custom email title
…ture-request.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
/approve |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (7)
126-147: Follow-up: append only validated non-empty file inputsUse the validated flags to decide which payload sections to append and avoid empty entries.
- if (this.file) { + if (hasFile) { await this.appendFile(formData, "file", this.file); } - if (this.files) { + if (hasFiles) { for (const [ index, file, ] of this.files.entries()) { await this.appendFile(formData, `files[${index}]`, file); } } - if (this.fileUrl) { + if (hasFileUrl) { formData.append("file_url", this.fileUrl); } - if (this.fileUrls) { + if (hasFileUrls) { for (const [ index, fileUrl, ] of this.fileUrls.entries()) { formData.append(`file_urls[${index}]`, fileUrl); } }
182-184: Make summary resilient to response shape changesAvoid runtime errors if
signature_requestorsignature_request_idis missing; use optional chaining with a fallback.- if (response) { - $.export("$summary", `Successfully sent signature request ${response.signature_request.signature_request_id}`); - } + if (response) { + const id = response?.signature_request?.signature_request_id ?? response?.id ?? "<unknown>"; + $.export("$summary", `Successfully sent signature request ${id}`); + }
5-5: FormData import requires a declared dependency or switch to native Web FormData
form-datais not native to Node. Either add it to package.json or switch to the global Web FormData (Node 18+), and don’t callgetHeaders()in that case.Option A — keep current code (recommended for consistency with getHeaders):
- Add
"form-data"to dependencies.Option B — use Web FormData:
- Remove the import.
- Stop calling
formData.getHeaders().Diff (Option B) within this file:
-import FormData from "form-data";- const response = await this.luminPdf.sendSignatureRequest({ - $, - headers: formData.getHeaders(), - data: formData, - }); + const response = await this.luminPdf.sendSignatureRequest({ + $, + data: formData, + });
112-124: Mutually exclusive file inputs: empty strings/arrays currently pass validation
[]is truthy and empty strings are truthy after current checks, so empty values can be treated as “provided”, breaking exclusivity and minimum-one validation.Use explicit non-empty checks and a precise count:
- const formData = new FormData(); - const checkFiles = {}; - if (this.file) checkFiles.file = this.file; - if (this.files) checkFiles.files = this.files; - if (this.fileUrl) checkFiles.fileUrl = this.fileUrl; - if (this.fileUrls) checkFiles.fileUrls = this.fileUrls; - - if (Object.keys(checkFiles).length > 1) { + const formData = new FormData(); + const hasFile = typeof this.file === "string" && this.file.trim().length > 0; + const hasFiles = Array.isArray(this.files) && this.files.length > 0 && this.files.every((p) => typeof p === "string" && p.trim().length > 0); + const hasFileUrl = typeof this.fileUrl === "string" && this.fileUrl.trim().length > 0; + const hasFileUrls = Array.isArray(this.fileUrls) && this.fileUrls.length > 0 && this.fileUrls.every((u) => typeof u === "string" && u.trim().length > 0); + const providedCount = [hasFile, hasFiles, hasFileUrl, hasFileUrls].filter(Boolean).length; + + if (providedCount > 1) { throw new ConfigurationError("Only one of `File URL`, `File`, `File URLs`, or `Files` should be provided in the request."); } - if (Object.keys(checkFiles).length === 0) { + if (providedCount === 0) { throw new ConfigurationError("At least one of `File URL`, `File`, `File URLs`, or `Files` should be provided in the request."); }
148-157: Guard signer entries: ensure each parsed signer is an objectIf a signer is an unparseable string,
parseObjectmay return the raw string, andObject.keys(signer)will iterate its indices, corrupting the payload. Validate before iterating.- if (this.signers) { - for (const [ - index, - signer, - ] of parseObject(this.signers).entries()) { - for (const item of Object.keys(signer)) { - formData.append(`signers[${index}][${item}]`, signer[item]); - } - } - } + if (this.signers) { + const signers = parseObject(this.signers); + for (const [index, signer] of signers.entries()) { + if (typeof signer !== "object" || signer === null || Array.isArray(signer)) { + throw new ConfigurationError(`Invalid signer at index ${index}: expected an object, got ${typeof signer}`); + } + for (const key of Object.keys(signer)) { + formData.append(`signers[${index}][${key}]`, signer[key]); + } + } + }
158-167: Guard viewer entries: ensure each parsed viewer is an objectSame concern as signers. Validate to avoid malformed payload.
- if (this.viewers) { - for (const [ - index, - viewer, - ] of parseObject(this.viewers).entries()) { - for (const item of Object.keys(viewer)) { - formData.append(`viewers[${index}][${item}]`, viewer[item]); - } - } - } + if (this.viewers) { + const viewers = parseObject(this.viewers); + for (const [index, viewer] of viewers.entries()) { + if (typeof viewer !== "object" || viewer === null || Array.isArray(viewer)) { + throw new ConfigurationError(`Invalid viewer at index ${index}: expected an object, got ${typeof viewer}`); + } + for (const key of Object.keys(viewer)) { + formData.append(`viewers[${index}][${key}]`, viewer[key]); + } + } + }
168-171: expiresAt should remain ISO string; boolean handling should include falseThe docs specify ISO 8601, but you convert to milliseconds. Also, only appending
use_text_tagswhen truthy drops explicitfalse.- if (this.title) formData.append("title", this.title); - if (this.expiresAt) formData.append("expires_at", Date.parse(this.expiresAt)); - if (this.useTextTags) formData.append("use_text_tags", `${this.useTextTags}`); + if (this.title) formData.append("title", this.title); + if (this.expiresAt) { + const ms = Date.parse(this.expiresAt); + if (Number.isNaN(ms)) { + throw new ConfigurationError("Invalid `Expires At` value. Provide an ISO 8601 datetime, e.g., 2025-01-31T23:59:59Z."); + } + // Optional: enforce future date + // if (ms <= Date.now()) throw new ConfigurationError("`Expires At` must be a future datetime."); + formData.append("expires_at", this.expiresAt); + } + if (this.useTextTags !== undefined) formData.append("use_text_tags", String(this.useTextTags));
🧹 Nitpick comments (4)
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (4)
47-56: Title and Expires At are marked required; make them optionalThese fields don’t appear to be required by the API. Mark them optional to avoid forcing input in the UI.
title: { type: "string", label: "Title", description: "The title you want to give the Signature Request.", + optional: true, }, expiresAt: { type: "string", label: "Expires At", description: "When the Signature Request will expire. Should be later than today. In ISO 8601 format (YYYY-MM-DDTHH:MM:SSZ).", + optional: true, },
38-38: Grammar nit: pluralize “path”Minor copy fix for clarity.
- description: "An array of path to files in the `/tmp` directory (for example, `/tmp/myFile.pdf`) to be sent for signature. This field is mutually exclusive with `File URL`, `Files`, and `File URLs`. Only one of these fields should be provided in the request.", + description: "An array of paths to files in the `/tmp` directory (for example, `/tmp/myFile.pdf`) to be sent for signature. This field is mutually exclusive with `File URL`, `Files`, and `File URLs`. Only one of these fields should be provided in the request.",
172-175: Don’t couple subject/title to senderEmail; append each field independentlyCurrently
subjectandcustomEmailTitleare only sent ifsenderEmailis provided. Allow users to set any custom email subfields independently.- if (this.senderEmail) formData.append("custom_email[sender_email]", this.senderEmail); - if (this.senderEmail) formData.append("custom_email[subject_name]", this.subject); - if (this.senderEmail) formData.append("custom_email[title]", this.customEmailTitle); + if (this.senderEmail) formData.append("custom_email[sender_email]", this.senderEmail); + if (this.subject) formData.append("custom_email[subject_name]", this.subject); + if (this.customEmailTitle) formData.append("custom_email[title]", this.customEmailTitle);
148-157: Follow-up: ensure at least one valid signer is providedBeyond defining the prop, consider asserting at least one signer object exists after parsing, since the API generally requires it.
Example:
if (this.signers) { const signers = parseObject(this.signers); + if (!Array.isArray(signers) || signers.length === 0) { + throw new ConfigurationError("At least one signer is required."); + } for (const [index, signer] of signers.entries()) { if (typeof signer !== "object" || signer === null || Array.isArray(signer)) { throw new ConfigurationError(`Invalid signer at index ${index}: expected an object, got ${typeof signer}`); } for (const key of Object.keys(signer)) { formData.append(`signers[${index}][${key}]`, signer[key]); } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-09T18:07:12.426Z
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#17538
File: components/aircall/sources/new-sms/new-sms.mjs:19-25
Timestamp: 2025-07-09T18:07:12.426Z
Learning: In Aircall API webhook payloads, the `created_at` field is returned as an ISO 8601 string format (e.g., "2020-02-18T20:52:22.000Z"), not as milliseconds since epoch. For Pipedream components, this needs to be converted to milliseconds using `Date.parse()` before assigning to the `ts` field in `generateMeta()`.
Applied to files:
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs
⏰ 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). (4)
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
🔇 Additional comments (1)
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (1)
172-175: Confirmcustom_emailfield names with Lumin’s API
I searched the codebase and only found these threecustom_email[...]entries incomponents/lumin_pdf/actions/send-signature-request/send-signature-request.mjs—the client simply forwards whatever keys you append. Please verify in Lumin’s official API docs whether the subject field should be namedsubject(instead ofsubject_name), and update these lines if needed:
- components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs:172–174
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs
Show resolved
Hide resolved
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs
Show resolved
Hide resolved
…quest customization
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (4)
5-5: FormData is not native: add dependency or switch to Web FormDataYou're importing FormData from "form-data" and calling getHeaders(), which requires the npm package. If the dependency isn’t declared, this will fail at runtime. Either add "form-data" to the package’s dependencies (keep current code), or switch to the native Web FormData (Node 18+) and remove getHeaders().
Option A — keep current code: add "form-data" to package.json dependencies.
Option B — switch to Web FormData: remove the import and header usage.
Apply this diff for Option B (native Web FormData):
-import FormData from "form-data";- headers: formData.getHeaders(), data: formData,If you prefer Option A, add the dependency (outside this file):
{ "dependencies": { "form-data": "^4.0.0" } }Verification script to confirm dependency presence:
#!/usr/bin/env bash fd -H -a package.json | while read -r pj; do echo "Checking $pj" rg -n '"form-data"' "$pj" || echo "No form-data dependency found in $pj" doneAlso applies to: 184-185
154-163: Validate signer entries before iterating keysparseObject may yield non-object entries (e.g., unparseable strings). Object.keys(signer) would iterate characters and produce malformed fields. Validate each entry is a plain object.
Apply this diff:
- if (this.signers) { - for (const [ - index, - signer, - ] of parseObject(this.signers).entries()) { - for (const item of Object.keys(signer)) { - formData.append(`signers[${index}][${item}]`, signer[item]); - } - } - } + if (this.signers) { + const signers = parseObject(this.signers); + for (const [index, signer] of signers.entries()) { + if (typeof signer !== "object" || signer === null || Array.isArray(signer)) { + throw new ConfigurationError(`Invalid signer at index ${index}: expected an object, got ${typeof signer}`); + } + for (const key of Object.keys(signer)) { + formData.append(`signers[${index}][${key}]`, signer[key]); + } + } + }
164-173: Validate viewer entries before iterating keysSame issue as signers: ensure each viewer is a non-null object.
Apply this diff:
- if (this.viewers) { - for (const [ - index, - viewer, - ] of parseObject(this.viewers).entries()) { - for (const item of Object.keys(viewer)) { - formData.append(`viewers[${index}][${item}]`, viewer[item]); - } - } - } + if (this.viewers) { + const viewers = parseObject(this.viewers); + for (const [index, viewer] of viewers.entries()) { + if (typeof viewer !== "object" || viewer === null || Array.isArray(viewer)) { + throw new ConfigurationError(`Invalid viewer at index ${index}: expected an object, got ${typeof viewer}`); + } + for (const key of Object.keys(viewer)) { + formData.append(`viewers[${index}][${key}]`, viewer[key]); + } + } + }
174-177: expiresAt: keep ISO 8601 string (don’t convert to ms) and handle boolean flag explicitlyYour prop description specifies ISO 8601. Date.parse() converts to epoch ms and likely breaks the API. Also, append use_text_tags even when false if the user set it.
Apply this diff:
- if (this.title) formData.append("title", this.title); - if (this.expiresAt) formData.append("expires_at", Date.parse(this.expiresAt)); - if (this.useTextTags) formData.append("use_text_tags", `${this.useTextTags}`); + if (this.title) formData.append("title", this.title); + if (this.expiresAt) { + if (Number.isNaN(Date.parse(this.expiresAt))) { + throw new ConfigurationError("Invalid `Expires At` value. Provide an ISO 8601 datetime, e.g., 2025-01-31T23:59:59Z."); + } + formData.append("expires_at", this.expiresAt); + } + if (this.useTextTags !== undefined) formData.append("use_text_tags", String(this.useTextTags));
🧹 Nitpick comments (4)
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (4)
178-181: Avoid appending undefined custom email fieldsCurrently subject_name/title are appended whenever senderEmail is set, even if subject/title are undefined. Append each field only when provided.
Apply this diff:
- if (this.senderEmail) formData.append("custom_email[sender_email]", this.senderEmail); - if (this.senderEmail) formData.append("custom_email[subject_name]", this.subject); - if (this.senderEmail) formData.append("custom_email[title]", this.customEmailTitle); + if (this.senderEmail) formData.append("custom_email[sender_email]", this.senderEmail); + if (this.subject) formData.append("custom_email[subject_name]", this.subject); + if (this.customEmailTitle) formData.append("custom_email[title]", this.customEmailTitle);
188-190: Make $summary resilient to response shapeGuard against missing nested fields to avoid runtime errors.
Apply this diff:
- if (response) { - $.export("$summary", `Successfully sent signature request ${response.signature_request.signature_request_id}`); - } + if (response) { + const id = response?.signature_request?.signature_request_id ?? response?.id ?? "<unknown>"; + $.export("$summary", `Successfully sent signature request ${id}`); + }
53-62: Props mismatch: title/expiresAt are treated as optional in code but required in UIThe run() logic appends these fields only when provided, but the props lack optional: true, making them required in the UI. Align the UI with runtime behavior (verify API requirements).
Apply this diff:
title: { type: "string", label: "Title", description: "The title you want to give the Signature Request.", + optional: true, }, expiresAt: { type: "string", label: "Expires At", description: "When the Signature Request will expire. Should be later than today. In ISO 8601 format (YYYY-MM-DDTHH:MM:SSZ).", + optional: true, },
38-38: Nit: grammar and exclusivity list
- “An array of path to files” → “An array of paths to files”
- The exclusivity list mentions “Files” (itself) instead of “File”
Apply this diff:
- description: "An array of path to files in the `/tmp` directory (for example, `/tmp/myFile.pdf`) to be sent for signature. This field is mutually exclusive with `File URL`, `Files`, and `File URLs`. Only one of these fields should be provided in the request.", + description: "An array of paths to files in the `/tmp` directory (for example, `/tmp/myFile.pdf`) to be sent for signature. This field is mutually exclusive with `File URL`, `File`, and `File URLs`. Only one of these fields should be provided in the request.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-09T18:07:12.426Z
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#17538
File: components/aircall/sources/new-sms/new-sms.mjs:19-25
Timestamp: 2025-07-09T18:07:12.426Z
Learning: In Aircall API webhook payloads, the `created_at` field is returned as an ISO 8601 string format (e.g., "2020-02-18T20:52:22.000Z"), not as milliseconds since epoch. For Pipedream components, this needs to be converted to milliseconds using `Date.parse()` before assigning to the `ts` field in `generateMeta()`.
Applied to files:
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs
🧬 Code Graph Analysis (1)
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (6)
components/lumin_pdf/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)components/lumin_pdf/actions/cancel-signature-request/cancel-signature-request.mjs (1)
response(24-27)components/lumin_pdf/actions/download-file/download-file.mjs (1)
response(32-36)components/lumin_pdf/actions/get-user-information/get-user-information.mjs (1)
response(13-15)components/lumin_pdf/actions/get-signature-request/get-signature-request.mjs (1)
response(19-22)components/lumin_pdf/actions/download-file-as-file-url/download-file-as-file-url.mjs (1)
response(19-22)
🔇 Additional comments (2)
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs (2)
41-46: Good addition: signers prop definedDefining the signers prop unblocks users from providing required signer data. LGTM.
47-52: Viewers marked optional and example JSON fixedMarking viewers as optional and correcting the example JSON looks good.
components/lumin_pdf/actions/send-signature-request/send-signature-request.mjs
Show resolved
Hide resolved
|
/approve |
Resolves #17949
Summary by CodeRabbit
New Features
Chores
Refactor