-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New Component - sharepoint #16489
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New Component - sharepoint #16489
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis update introduces a new "Download File" action for Microsoft SharePoint, enabling users to download files from a specified SharePoint site and drive to a local directory. Supporting this feature, the SharePoint app module is extended with new property definitions and methods for selecting drives and files, listing drives and drive items, searching drive items, and fetching file contents. Additionally, several existing action and source modules receive version number increments, and the package manifest is updated to reflect a new version and dependency update. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DownloadFileAction
participant SharePointApp
participant MicrosoftGraphAPI
participant FileSystem
User->>DownloadFileAction: Provide siteId, driveId, fileId, filename
DownloadFileAction->>SharePointApp: getFile({siteId, fileId})
SharePointApp->>MicrosoftGraphAPI: Request file content
MicrosoftGraphAPI-->>SharePointApp: Return file content (array buffer)
SharePointApp-->>DownloadFileAction: File content (array buffer)
DownloadFileAction->>FileSystem: Write file to /tmp/filename
DownloadFileAction-->>User: Return filename and path
Assessment against linked issues
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/sharepoint/sharepoint.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
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. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
components/sharepoint/actions/download-file/download-file.mjs (1)
1-62
: 🛠️ Refactor suggestionNew component implementation looks solid, with some improvement opportunities.
The new SharePoint Download File action provides a useful capability to download files from SharePoint to the local
/tmp
directory.Consider adding the following improvements:
- Add error handling for file operations and API calls:
async run({ $ }) { - const response = await this.sharepoint.getFile({ - $, - siteId: this.siteId, - fileId: this.fileId, - responseType: "arraybuffer", - }); + try { + const response = await this.sharepoint.getFile({ + $, + siteId: this.siteId, + fileId: this.fileId, + responseType: "arraybuffer", + }); + + const downloadedFilepath = `/tmp/${this.filename}`; + fs.writeFileSync(downloadedFilepath, response); + + $.export("$summary", `Successfully downloaded file to ${downloadedFilepath}`); + + return { + filename: this.filename, + downloadedFilepath, + }; + } catch (error) { + $.export("$summary", `Failed to download file: ${error.message}`); + throw error; + } },
- Simplify the buffer handling - the arraybuffer from the API can be written directly without base64 conversion:
- const rawcontent = response.toString("base64"); - const buffer = Buffer.from(rawcontent, "base64"); - const downloadedFilepath = `/tmp/${this.filename}`; - fs.writeFileSync(downloadedFilepath, buffer); + const downloadedFilepath = `/tmp/${this.filename}`; + fs.writeFileSync(downloadedFilepath, response);
- Add basic filename sanitization to prevent path traversal issues:
+ // Sanitize filename to prevent path traversal + const sanitizedFilename = this.filename.replace(/[/\\?%*:|"<>]/g, '-'); - const downloadedFilepath = `/tmp/${this.filename}`; + const downloadedFilepath = `/tmp/${sanitizedFilename}`;
🧹 Nitpick comments (2)
components/sharepoint/sharepoint.app.mjs (1)
152-154
: Typo in property descriptionThe description for the
fileId
property contains a grammatical error: "You can either search for the file here, provide a custom File ID." This sentence is incomplete or incorrectly structured.- description: "The file to download. You can either search for the file here, provide a custom *File ID*.", + description: "The file to download. You can either search for the file here or provide a custom *File ID*.",components/sharepoint/actions/download-file/download-file.mjs (1)
43-59
: Add success summary message for better user feedback.Consider adding a summary message to indicate successful download and file location.
fs.writeFileSync(downloadedFilepath, buffer); + $.export("$summary", `Successfully downloaded file to ${downloadedFilepath}`); + return { filename: this.filename, downloadedFilepath, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
components/sharepoint/actions/create-item/create-item.mjs
(1 hunks)components/sharepoint/actions/create-list/create-list.mjs
(1 hunks)components/sharepoint/actions/download-file/download-file.mjs
(1 hunks)components/sharepoint/actions/update-item/update-item.mjs
(1 hunks)components/sharepoint/package.json
(2 hunks)components/sharepoint/sharepoint.app.mjs
(2 hunks)components/sharepoint/sources/new-list-item/new-list-item.mjs
(1 hunks)components/sharepoint/sources/updated-list-item/updated-list-item.mjs
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
🔇 Additional comments (10)
components/sharepoint/sharepoint.app.mjs (3)
159-171
: Efficiently structured file search and listing optionsThe implementation elegantly handles both searching and listing scenarios for files, with conditional logic that determines which API to call based on whether a query is provided.
120-180
: Well-implemented property definitions with pagination supportBoth the
driveId
andfileId
property definitions are well-implemented with proper pagination support, consistent with the existing property definitions in the file.
232-263
: Well-structured API methods following established patternsThe new methods for working with SharePoint drives and files follow the same consistent patterns as the existing methods, reusing the
_makeRequest
helper appropriately.components/sharepoint/actions/create-list/create-list.mjs (1)
7-7
: Version bump is appropriateThe version increment from "0.0.2" to "0.0.3" is appropriate as part of this larger update to the SharePoint components.
components/sharepoint/actions/update-item/update-item.mjs (1)
8-8
: Version bump is appropriateThe version increment from "0.0.1" to "0.0.2" is appropriate as part of this larger update to the SharePoint components.
components/sharepoint/package.json (2)
3-3
: Appropriate package version incrementThe package version increment from 0.2.0 to 0.3.0 appropriately reflects the addition of new functionality (drive and file operations) to the SharePoint components.
16-16
:❓ Verification inconclusive
Significant platform dependency update
This update involves a major version change to the
@pipedream/platform
dependency (from ^1.5.1 to ^3.0.3). Ensure that this change is intentional and that all components remain compatible with the updated platform version.
🏁 Script executed:
#!/bin/bash # Check if other SharePoint components use features from the new platform version # Find import statements to see what's being used from the platform rg -A 2 "import.*from.*@pipedream/platform" --type js --type mjs components/sharepoint/Length of output: 117
🏁 Script executed:
#!/bin/bash # Search for all import and require usages of @pipedream/platform in sharepoint components rg -A2 "import .* from ['\"]@pipedream/platform['\"]" -g "*.[jt]s" -g "*.mjs" components/sharepoint/ rg -A2 "require\(['\"]@pipedream/platform['\"]\)" -g "*.[jt]s" -g "*.mjs" components/sharepoint/Length of output: 637
Major @pipedream/platform version bump – verify API compatibility
You’ve upgraded
@pipedream/platform
from^1.5.1
to^3.0.3
, which is a breaking change. We’ve confirmed that the SharePoint package imports platform APIs in two places:
components/sharepoint/sharepoint.app.mjs
import { axios } from "@pipedream/platform";components/sharepoint/sources/common/common.mjs
import { DEFAULT_POLLING_SOURCE_TIMER_INTERVAL } from "@pipedream/platform";Please ensure that both
axios
andDEFAULT_POLLING_SOURCE_TIMER_INTERVAL
are still exported and behave as expected in v3.x. Update usages as needed and run your integration tests to catch any regressions.components/sharepoint/sources/updated-list-item/updated-list-item.mjs (1)
8-8
: Version increment looks good.The version bump from "0.0.2" to "0.0.3" is consistent with the other SharePoint component updates in this PR. This coordinated version increment aligns with the addition of new SharePoint functionality for handling drives and files.
components/sharepoint/actions/create-item/create-item.mjs (1)
7-7
: Version increment looks good.The version bump from "0.0.2" to "0.0.3" is consistent with the other SharePoint component updates in this PR, maintaining version alignment across the component library.
components/sharepoint/sources/new-list-item/new-list-item.mjs (1)
8-8
: Version increment looks good.The version bump from "0.0.2" to "0.0.3" is consistent with the other SharePoint component updates in this PR, maintaining version alignment across related components.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, just made one minor grammar suggestion!
Co-authored-by: Guilherme Falcão <48412907+GTFalcao@users.noreply.github.com>
/approve |
Resolves #16465
Summary by CodeRabbit