Google Drive - Create File From Template - support templates in shared drives #19053
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds an optional Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Action as Create File from Template
participant GDrive as Google Drive API
User->>Action: Start (templateId, folderId, options, optional drive)
activate Action
rect rgb(230,248,255)
Note over Action: Copy template with supportsAllDrives (propagate drive)
Action->>GDrive: files.copy(templateId, name="template-copy", supportsAllDrives=true, drive=opt)
GDrive-->>Action: copiedTemplateId
end
rect rgb(240,255,230)
Note over Action: Interpolate replacements (parseObjectEntries) using copiedTemplateId
Action->>GDrive: files.update/createFromTemplate(copiedTemplateId, replacements..., parents=maybeUndefinedForSharedDrive)
GDrive-->>Action: googleDocId
end
rect rgb(255,250,230)
Note over Action: Optional export to PDF (supportsAllDrives)
alt PDF requested
Action->>GDrive: files.export(googleDocId, mimeType=application/pdf, supportsAllDrives=true)
GDrive-->>Action: pdfData / pdfId
end
end
rect rgb(240,255,240)
Note over Action: If destination is in shared drive, move doc/pdf into target folder (propagate drive)
Action->>GDrive: files.update(move googleDocId -> parents=targetFolder, supportsAllDrives=true)
opt pdf created
Action->>GDrive: files.update(move pdfId -> parents=targetFolder, supportsAllDrives=true)
end
GDrive-->>Action: moveAcks
end
rect rgb(255,240,240)
Note over Action: Cleanup — delete temporary template copy (supportsAllDrives)
Action->>GDrive: files.delete(copiedTemplateId, supportsAllDrives=true)
GDrive-->>Action: deleteAck
end
deactivate Action
Action-->>User: Return created file(s) and metadata
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
⏰ 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)
🔇 Additional comments (1)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs (2)
38-46: Add drive parameter to folderId prop for consistency.The
folderIdprop should receive thedriveparameter liketemplateIddoes, ensuring users can select folders from the chosen shared drive.Apply this diff to propagate the drive parameter:
folderId: { propDefinition: [ googleDrive, "folderId", + (c) => ({ + drive: c.drive, + }), ], description: "Select the folder of the newly created Google Doc and/or PDF, or use a custom expression to reference a folder ID from a previous step.", optional: true, },
82-142: Critical: Wrap template copy workflow in try-finally to prevent resource leaks.If any operation fails (interpolation, PDF creation, or Google Doc deletion), the copied template is never cleaned up, leaving orphaned "template-copy" files in the target folder. The cleanup must execute regardless of success or failure.
Apply this diff to ensure cleanup always happens:
+ let copiedTemplateId; + try { // COPY THE TEMPLATE const drive = this.googleDrive.drive(); const copiedTemplate = await drive.files.copy({ fileId: this.templateId, requestBody: { name: "template-copy", parents: [ this.folderId, ], }, supportsAllDrives: true, }); - const templateId = copiedTemplate.data.id; + copiedTemplateId = copiedTemplate.data.id; /* CREATE THE GOOGLE DOC */ let googleDocId; try { googleDocId = await client.interpolate({ - source: templateId, + source: copiedTemplateId, destination: this.folderId, name: this.name, data: this.replaceValues, }); } catch (e) { const { code, message, } = e.error.error; let errorMessage = `Status: ${code}, ${message} `; if (code == 404 || code == 400) { errorMessage += "Make sure that the template file selected contains placeholders in the format {{xyz}}."; } throw new Error(errorMessage); } result["googleDocId"] = googleDocId; /* CREATE THE PDF */ if (this.mode.includes(MODE_PDF)) { const pdfId = await client.export({ file: googleDocId, mimeType: "application/pdf", name: this.name, destination: this.folderId, }); result["pdfId"] = pdfId; } /* REMOVE THE GOOGLE DOC */ if (!this.mode.includes(MODE_GOOGLE_DOC)) { await this.googleDrive.deleteFile(googleDocId); } - - // REMOVE THE COPIED TEMPLATE - - await drive.files.delete({ - fileId: templateId, - supportsAllDrives: true, - }); + } finally { + // REMOVE THE COPIED TEMPLATE (always cleanup) + if (copiedTemplateId) { + try { + await drive.files.delete({ + fileId: copiedTemplateId, + supportsAllDrives: true, + }); + } catch (cleanupError) { + // Log but don't fail the action if cleanup fails + console.error("Failed to cleanup copied template:", cleanupError); + } + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs(4 hunks)components/google_drive/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-01T17:07:48.193Z
Learnt from: js07
Repo: PipedreamHQ/pipedream PR: 17375
File: components/zerobounce/actions/get-validation-results-file/get-validation-results-file.mjs:23-27
Timestamp: 2025-07-01T17:07:48.193Z
Learning: For "dir" props in Pipedream components, whether to mark them as optional depends on the action's file I/O behavior - if an action always writes files as output, the "dir" prop should not be marked as optional.
Applied to files:
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs
🧬 Code graph analysis (1)
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs (1)
components/google_drive/actions/create-file-from-text/create-file-from-text.mjs (1)
drive(93-93)
⏰ 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: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (5)
components/google_drive/package.json (1)
3-3: LGTM - version bump is appropriate.The patch version bump from 1.2.0 to 1.2.1 correctly reflects the bug fix and feature enhancement for shared drive support.
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs (4)
11-11: LGTM - version bump is appropriate.
20-26: LGTM - drive prop enables shared drive selection.The optional drive prop correctly uses the
watchedDrivepropDefinition, allowing users to select shared drives while maintaining backward compatibility.
31-33: LGTM - drive parameter correctly propagated to template selector.The templateId prop now receives the drive parameter, allowing templates to be selected from the chosen drive.
93-93: LGTM - correct use of supportsAllDrives for shared drive compatibility.The
supportsAllDrives: trueparameter is correctly applied to both copy and delete operations, enabling the action to work with shared drives.Also applies to: 141-141
luancazarine
left a comment
luancazarine
left a comment
There was a problem hiding this comment.
Hi @michelle0927, LGTM! Ready for QA!
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs (1)
101-117: Critical: Copied template not cleaned up when interpolation fails.If the
interpolatecall fails, the copied template (created at lines 86-96) is never deleted, causing orphaned files to accumulate in the folder.Apply this diff to ensure cleanup on failure:
let googleDocId; try { googleDocId = await client.interpolate({ source: templateId, destination: this.folderId, name: this.name, data: parseObjectEntries(this.replaceValues), }); } catch (e) { + // Clean up the copied template before throwing + try { + await drive.files.delete({ + fileId: templateId, + supportsAllDrives: true, + }); + } catch (deleteError) { + console.error("Failed to clean up template copy:", deleteError); + } const { code, message, } = e.error.error; let errorMessage = `Status: ${code}, ${message} `; if (code == 404 || code == 400) { errorMessage += "Make sure that the template file selected contains placeholders in the format {{xyz}}."; } throw new Error(errorMessage); }
♻️ Duplicate comments (1)
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs (1)
83-97: Critical: Hardcoded template copy name causes collisions and variable shadowing persists.The issues flagged in the previous review remain unaddressed:
- Name collision risk: The hardcoded
"template-copy"name will cause conflicts in concurrent executions and leave orphaned files when runs fail.- Variable shadowing: Using
templateIdas the variable name shadowsthis.templateId, reducing code clarity.Additionally, no error handling is present for the copy operation, which could leave the action in an inconsistent state.
Apply the previously suggested fix with error handling:
+ let copiedTemplateId; + try { // COPY THE TEMPLATE const drive = this.googleDrive.drive(); const copiedTemplate = await drive.files.copy({ fileId: this.templateId, requestBody: { - name: "template-copy", + name: `template-copy-${Date.now()}-${Math.random().toString(36).substring(7)}`, parents: [ this.folderId || "root", ], }, supportsAllDrives: true, }); - const templateId = copiedTemplate.data.id; + copiedTemplateId = copiedTemplate.data.id; + } catch (e) { + throw new Error(`Failed to copy template: ${e.message}`); + }Then update references on lines 103 and 141 from
templateIdtocopiedTemplateId.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: js07
Repo: PipedreamHQ/pipedream PR: 17375
File: components/zerobounce/actions/get-validation-results-file/get-validation-results-file.mjs:23-27
Timestamp: 2025-07-01T17:07:48.193Z
Learning: For "dir" props in Pipedream components, whether to mark them as optional depends on the action's file I/O behavior - if an action always writes files as output, the "dir" prop should not be marked as optional.
📚 Learning: 2025-07-01T17:07:48.193Z
Learnt from: js07
Repo: PipedreamHQ/pipedream PR: 17375
File: components/zerobounce/actions/get-validation-results-file/get-validation-results-file.mjs:23-27
Timestamp: 2025-07-01T17:07:48.193Z
Learning: For "dir" props in Pipedream components, whether to mark them as optional depends on the action's file I/O behavior - if an action always writes files as output, the "dir" prop should not be marked as optional.
Applied to files:
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs
🧬 Code graph analysis (1)
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs (1)
components/airtop/common/utils.mjs (1)
parseObjectEntries(9-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: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (5)
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs (5)
3-3: LGTM: Import is properly used.The
parseObjectEntriesimport is correctly used on line 106 to preprocessreplaceValuesbefore passing to the interpolation library.
12-12: LGTM: Appropriate version bump.Version increment reflects the feature addition for shared drive support.
21-27: LGTM: Drive prop enables shared drive support.The optional
driveprop allows users to select shared drives and is properly wired to thetemplateIdselector for filtering templates.
28-35: LGTM: Proper drive parameter propagation.The
templateIdpropDefinition correctly propagates the selected drive to filter templates, improving user experience.
106-106: No changes needed – parseObjectEntries is safe and backward compatible.The function correctly handles both simple key-value pairs and nested JSON values. Simple strings (like
"value") remain unchanged, while JSON strings (like'{"nested":"data"}') are parsed into objects. This is backward compatible and aligns with its usage across 35+ integrations in the codebase. ThereplaceValuesobject will work seamlessly with thegoogle-docs-mustacheslibrary'sinterpolate()method.
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs
Outdated
Show resolved
Hide resolved
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs
Show resolved
Hide resolved
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs (1)
75-181: Recommend: Add comprehensive error handling for the multi-step workflow.The current implementation performs multiple operations (copy, interpolate, export, move, cleanup) without comprehensive error handling. If any intermediate step fails after files are created, the action may leave orphaned files or incomplete state.
Consider wrapping the entire workflow in try-catch-finally blocks to ensure:
- Cleanup always runs (delete copied template) even on failure
- Partial successes are communicated to the user (e.g., "Google Doc created but PDF export failed")
- Resource leaks are prevented (orphaned template copies)
Example structure:
let copiedTemplateId; try { // Copy template copiedTemplateId = ...; // Create files // Move files if needed } catch (error) { // Handle and report specific errors } finally { // Always cleanup copied template if (copiedTemplateId) { try { await drive.files.delete({ fileId: copiedTemplateId, supportsAllDrives: true }); } catch (deleteError) { console.warn("Failed to delete template copy:", deleteError); } } }
♻️ Duplicate comments (1)
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs (1)
172-178: Previous comment still applies: Add error handling for template cleanup.The deletion of the copied template lacks error handling, which could cause the action to fail after successfully creating files. The previous review comment suggesting a try-catch or finally block remains valid.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-01T17:07:48.193Z
Learnt from: js07
Repo: PipedreamHQ/pipedream PR: 17375
File: components/zerobounce/actions/get-validation-results-file/get-validation-results-file.mjs:23-27
Timestamp: 2025-07-01T17:07:48.193Z
Learning: For "dir" props in Pipedream components, whether to mark them as optional depends on the action's file I/O behavior - if an action always writes files as output, the "dir" prop should not be marked as optional.
Applied to files:
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs
🧬 Code graph analysis (1)
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs (4)
components/google_drive/actions/create-file-from-text/create-file-from-text.mjs (2)
drive(93-93)file(90-92)components/google_drive/google_drive.app.mjs (16)
drive(396-396)drive(415-415)drive(443-443)drive(450-450)drive(498-498)drive(499-502)drive(594-594)drive(595-598)drive(719-719)drive(757-757)drive(766-766)drive(775-775)drive(783-783)drive(791-791)drive(800-800)file(121-121)components/airtop/common/utils.mjs (1)
parseObjectEntries(9-22)components/google_drive/actions/move-file/move-file.mjs (1)
file(47-47)
⏰ 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: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (5)
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs (5)
3-3: LGTM: Import addition is appropriate.The
parseObjectEntriesimport is correctly added and used later in the code for parsing replacement values.
21-27: LGTM: Drive prop correctly defined.The optional drive property is properly configured using the
watchedDrivepropDefinition, allowing users to select shared drives.
32-34: LGTM: Drive propagation correctly implemented.Both
templateIdandfolderIdprops correctly propagate the drive context, ensuring proper scoping for shared drives.Also applies to: 43-45
107-114: LGTM: Interpolation logic correctly implements shared drive workaround.The conditional destination (
undefinedfor shared drives) is a reasonable workaround for API limitations. The use ofparseObjectEntriescorrectly handles both string and object inputs for replacement values.
129-140: LGTM: PDF export logic is consistent with Google Doc creation.The conditional destination handling and
letdeclaration forpdfIdare appropriate and consistent with the interpolation logic.
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs
Outdated
Show resolved
Hide resolved
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs
Show resolved
Hide resolved
components/google_drive/actions/create-file-from-template/create-file-from-template.mjs
Show resolved
Hide resolved
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
|
/approve |
Resolves #18958
Summary by CodeRabbit
New Features
Chores