-
Notifications
You must be signed in to change notification settings - Fork 5.5k
[TRIGGER] Add shared drive prop to relevant Onedrive triggers #17758
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThis update introduces support for selecting shared drives in Microsoft OneDrive triggers by adding a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Trigger (new-file-created/new-folder-created)
participant OneDrive App
participant Microsoft Graph API
User->>Trigger: Configure trigger (select drive and folder)
Trigger->>OneDrive App: Request available drives (drive.options)
OneDrive App->>Microsoft Graph API: GET /drives and /me/drive/sharedWithMe
Microsoft Graph API-->>OneDrive App: Return drives and shared files
OneDrive App-->>Trigger: Return drive options
User->>Trigger: Select drive and folder
Trigger->>OneDrive App: Request folders for selected drive (folder.options)
OneDrive App->>Microsoft Graph API: GET /drives/{driveId}/root/children
Microsoft Graph API-->>OneDrive App: Return folders
OneDrive App-->>Trigger: Return folder options
Trigger->>OneDrive App: Set up webhook with driveId and folderId
OneDrive App->>Microsoft Graph API: Create subscription with driveId/folderId
Microsoft Graph API-->>OneDrive App: Confirm subscription
Estimated code review effort3 (~30–60 minutes) 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/microsoft_onedrive/actions/create-link/create-link.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/microsoft_onedrive/actions/create-folder/create-folder.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/microsoft_onedrive/actions/find-file-by-name/find-file-by-name.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
✨ 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: 0
🧹 Nitpick comments (3)
components/microsoft_onedrive/actions/get-excel-table/get-excel-table.mjs (1)
64-64
: Minor copy tweak – capitalize “Excel” in the summary
Tiny polish for user–facing text.- $.export("$summary", "Successfully retrieved excel table."); + $.export("$summary", "Successfully retrieved Excel table.");components/microsoft_onedrive/actions/get-file-by-id/get-file-by-id.mjs (1)
20-23
: Fix typo in exported summary message
retreived
→retrieved
.- $.export("$summary", `Successfully retreived file with ID: ${this.fileId}`); + $.export("$summary", `Successfully retrieved file with ID: ${this.fileId}`);components/microsoft_onedrive/microsoft_onedrive.app.mjs (1)
163-205
: Review drive options filtering and potential duplicate handling.The drive prop implementation correctly fetches drives and shared files, but there are a few considerations:
- Filtering logic: The filter
drive.owner?.user?.email
may exclude valid drives that don't have this specific structure.- Potential duplicates: Shared drives from
listSharedFiles
could duplicate drives fromlistDrives
since the same drive might appear in both lists.Consider these improvements:
const { value: drives } = await this.listDrives(); options = drives - .filter((drive) => drive.owner?.user?.email) + .filter((drive) => drive.owner?.user || drive.owner?.application) .map(({ id, description, name, driveType, }) => ({ label: `${description || name} (${driveType})`, value: id, }));And add deduplication:
const sharedDriveOptions = sharedFiles .map(({ remoteItem }) => ({ label: `${remoteItem.parentReference.driveId} (${remoteItem.parentReference.driveType})`, value: remoteItem.parentReference.driveId, })); + // Remove duplicates by drive ID + const existingDriveIds = new Set(options.map(opt => opt.value)); + const uniqueSharedOptions = sharedDriveOptions.filter(opt => !existingDriveIds.has(opt.value)); options = [ ...options, - ...sharedDriveOptions, + ...uniqueSharedOptions, ];
📜 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 (13)
components/microsoft_onedrive/actions/create-folder/create-folder.mjs
(1 hunks)components/microsoft_onedrive/actions/create-link/create-link.mjs
(1 hunks)components/microsoft_onedrive/actions/download-file/download-file.mjs
(1 hunks)components/microsoft_onedrive/actions/find-file-by-name/find-file-by-name.mjs
(1 hunks)components/microsoft_onedrive/actions/get-excel-table/get-excel-table.mjs
(1 hunks)components/microsoft_onedrive/actions/get-file-by-id/get-file-by-id.mjs
(1 hunks)components/microsoft_onedrive/actions/list-files-in-folder/list-files-in-folder.mjs
(1 hunks)components/microsoft_onedrive/actions/upload-file/upload-file.mjs
(1 hunks)components/microsoft_onedrive/microsoft_onedrive.app.mjs
(3 hunks)components/microsoft_onedrive/package.json
(1 hunks)components/microsoft_onedrive/sources/common/base.mjs
(1 hunks)components/microsoft_onedrive/sources/new-file-created/new-file-created.mjs
(2 hunks)components/microsoft_onedrive/sources/new-folder-created/new-folder-created.mjs
(1 hunks)
🧠 Learnings (7)
components/microsoft_onedrive/package.json (1)
Learnt from: jcortes
PR: #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.
components/microsoft_onedrive/actions/list-files-in-folder/list-files-in-folder.mjs (1)
Learnt from: js07
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.
components/microsoft_onedrive/sources/common/base.mjs (2)
Learnt from: GTFalcao
PR: #12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-10-08T15:33:38.240Z
Learning: The common-webhook-methods.mjs
object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like generateWebhookMeta
and getEventType
to enforce implementation in subclasses.
Learnt from: GTFalcao
PR: #12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-07-24T02:06:47.016Z
Learning: The common-webhook-methods.mjs
object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like generateWebhookMeta
and getEventType
to enforce implementation in subclasses.
components/microsoft_onedrive/actions/find-file-by-name/find-file-by-name.mjs (1)
Learnt from: jcortes
PR: #14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In components/gainsight_px/actions/create-account/create-account.mjs
, the action name should be "Create Account" instead of "Create Memory".
components/microsoft_onedrive/actions/create-link/create-link.mjs (1)
Learnt from: jcortes
PR: #14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In components/gainsight_px/actions/create-account/create-account.mjs
, the action name should be "Create Account" instead of "Create Memory".
components/microsoft_onedrive/sources/new-file-created/new-file-created.mjs (3)
Learnt from: GTFalcao
PR: #15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.
Learnt from: js07
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.
Learnt from: GTFalcao
PR: #14265
File: components/the_magic_drip/sources/common.mjs:35-43
Timestamp: 2024-10-10T19:18:27.998Z
Learning: In components/the_magic_drip/sources/common.mjs
, when processing items in getAndProcessData
, savedIds
is intentionally updated with IDs of both emitted and non-emitted items to avoid emitting retroactive events upon first deployment and ensure only new events are emitted as they occur.
components/microsoft_onedrive/microsoft_onedrive.app.mjs (1)
Learnt from: js07
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.
🧰 Additional context used
🧠 Learnings (7)
components/microsoft_onedrive/package.json (1)
Learnt from: jcortes
PR: #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.
components/microsoft_onedrive/actions/list-files-in-folder/list-files-in-folder.mjs (1)
Learnt from: js07
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.
components/microsoft_onedrive/sources/common/base.mjs (2)
Learnt from: GTFalcao
PR: #12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-10-08T15:33:38.240Z
Learning: The common-webhook-methods.mjs
object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like generateWebhookMeta
and getEventType
to enforce implementation in subclasses.
Learnt from: GTFalcao
PR: #12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-07-24T02:06:47.016Z
Learning: The common-webhook-methods.mjs
object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like generateWebhookMeta
and getEventType
to enforce implementation in subclasses.
components/microsoft_onedrive/actions/find-file-by-name/find-file-by-name.mjs (1)
Learnt from: jcortes
PR: #14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In components/gainsight_px/actions/create-account/create-account.mjs
, the action name should be "Create Account" instead of "Create Memory".
components/microsoft_onedrive/actions/create-link/create-link.mjs (1)
Learnt from: jcortes
PR: #14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In components/gainsight_px/actions/create-account/create-account.mjs
, the action name should be "Create Account" instead of "Create Memory".
components/microsoft_onedrive/sources/new-file-created/new-file-created.mjs (3)
Learnt from: GTFalcao
PR: #15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.
Learnt from: js07
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.
Learnt from: GTFalcao
PR: #14265
File: components/the_magic_drip/sources/common.mjs:35-43
Timestamp: 2024-10-10T19:18:27.998Z
Learning: In components/the_magic_drip/sources/common.mjs
, when processing items in getAndProcessData
, savedIds
is intentionally updated with IDs of both emitted and non-emitted items to avoid emitting retroactive events upon first deployment and ensure only new events are emitted as they occur.
components/microsoft_onedrive/microsoft_onedrive.app.mjs (1)
Learnt from: js07
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.
⏰ 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 (21)
components/microsoft_onedrive/actions/get-excel-table/get-excel-table.mjs (1)
7-7
: Version bump is consistent with the rest of the suite
No issues spotted.components/microsoft_onedrive/package.json (1)
3-3
: Package version bump LGTM
1.7.2
aligns with the patch-level increments across all updated actions. Remember to add a matching entry to your changelog/release notes.components/microsoft_onedrive/actions/download-file/download-file.mjs (1)
12-12
: Version bump acknowledged
No functional changes in this diff. Implementation remains solid.components/microsoft_onedrive/actions/find-file-by-name/find-file-by-name.mjs (1)
28-33
: URL-encode query parameters in OneDrive actions
When interpolatingthis.name
into the search URL, wrap it inencodeURIComponent
to avoid invalid paths whenever the filename contains spaces, quotes, or other reserved characters:- const response = await this.httpRequest({ - url: `/search(q='${this.name}')`, - }); + const encoded = encodeURIComponent(this.name); + const response = await this.httpRequest({ + url: `/search(q='${encoded}')`, + });• Please review all other actions under
components/microsoft_onedrive/actions
—anytime user input is injected into a URL path, it should be wrapped withencodeURIComponent
.components/microsoft_onedrive/actions/create-link/create-link.mjs (1)
6-6
: LGTM - Appropriate version incrementThe patch version bump aligns with the coordinated release update across the Microsoft OneDrive component.
components/microsoft_onedrive/actions/list-files-in-folder/list-files-in-folder.mjs (1)
8-8
: LGTM - Consistent version incrementThe patch version bump maintains consistency with the coordinated component release.
components/microsoft_onedrive/actions/upload-file/upload-file.mjs (1)
11-11
: LGTM - Coordinated version updateThe patch version increment is appropriate for the component-wide release.
components/microsoft_onedrive/actions/create-folder/create-folder.mjs (1)
8-8
: LGTM - Standard version incrementThe patch version bump is consistent with the component-wide release strategy.
components/microsoft_onedrive/sources/common/base.mjs (2)
87-87
: Good implementation - Retrieves drive parameters for webhook subscriptionThe addition of
getDeltaLinkParams()
call enables child classes to specify which drive to monitor by overriding this method.
90-90
: createHook correctly handlesdriveId
fallbackWe’ve confirmed that
createHook
uses_getDrivePath(driveId)
, which returns/drives/{driveId}
when provided or defaults to/me/drive
ifdriveId
is undefined. This ensures webhook subscriptions are scoped correctly in all cases—no further changes needed.components/microsoft_onedrive/sources/new-folder-created/new-folder-created.mjs (4)
11-11
: LGTM: Version increment is appropriate.The version bump from 0.0.1 to 0.0.2 correctly reflects the addition of new functionality (drive selection feature).
15-22
: LGTM: Drive prop implementation is well-structured.The optional drive prop is properly configured with a clear description that explains the default behavior when not specified.
27-29
: LGTM: Folder prop enhancement enables drive-scoped selection.The modification correctly passes the drive ID to the folder prop definition, enabling folder selection to be scoped to the specified drive.
36-45
: LGTM: getDeltaLinkParams refactor improves consistency.The refactored method now returns a consistent object structure with conditional properties, which is cleaner than the previous approach and aligns with the enhanced drive/folder scoping functionality.
components/microsoft_onedrive/sources/new-file-created/new-file-created.mjs (4)
11-11
: LGTM: Consistent version increment.The version bump to 0.0.2 is consistent with the companion folder-created source component.
15-22
: LGTM: Drive prop implementation matches folder-created component.The drive prop implementation is consistent with the new-folder-created component, maintaining a uniform API across OneDrive source components.
27-29
: LGTM: Consistent folder prop enhancement.The folder prop modification correctly mirrors the implementation in the new-folder-created component, ensuring consistent behavior across OneDrive triggers.
44-51
: LGTM: getDeltaLinkParams refactor maintains consistency.The method refactor is identical to the folder-created component, ensuring consistent parameter handling across OneDrive source components.
components/microsoft_onedrive/microsoft_onedrive.app.mjs (3)
30-69
: LGTM: Excellent error handling addition.The try-catch block around the Microsoft Graph API call prevents unhandled exceptions and provides graceful degradation by returning empty options when folder listing fails.
476-482
: LGTM: listDrives method implementation is correct.The method correctly calls the
/drives
endpoint with appropriate field selection for the drive options functionality.
483-489
: LGTM: listSharedFiles method implementation is correct.The method properly calls the
/me/drive/sharedWithMe
endpoint with the necessary field selection to support shared drive detection.
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.
LGTM!
WHY
Resolves #17335
Summary by CodeRabbit
New Features
Improvements
Chores