-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Zoho Workdrive - updates to sources #19647
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 GitHub. 1 Skipped Deployment
|
WalkthroughAdds manual folder ID entry, validates/normalizes lastDate parsing, makes folderId props required in two sources, refactors additionalFolderProps to accept existingProps and handle manual-entry early-exit, and bumps package/action/source versions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~23 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
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: 2
🤖 Fix all issues with AI agents
In @components/zoho_workdrive/common/additionalFolderProps.mjs:
- Around line 76-79: In the catch block inside the additionalFolderProps
function (file additionalFolderProps.mjs) replace
console.log(JSON.stringify(error, null, 2)) with
console.error(JSON.stringify(error, null, 2)) so errors are logged at the proper
severity; keep the existing return props behavior unchanged.
In @components/zoho_workdrive/sources/common/base.mjs:
- Around line 42-52: The _getValidatedLastDate method incorrectly assumes
Date.parse throws; replace the try/catch with explicit validation: call
Date.parse(lastDate), check Number.isNaN(result) and return 0 on invalid parse,
otherwise return the numeric timestamp; keep the existing fast-path for integer
lastDate (Number.isInteger(lastDate)) and ensure the method always returns a
number (0 or a valid timestamp).
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
components/zoho_workdrive/actions/download-file/download-file.mjscomponents/zoho_workdrive/actions/upload-file/upload-file.mjscomponents/zoho_workdrive/common/additionalFolderProps.mjscomponents/zoho_workdrive/package.jsoncomponents/zoho_workdrive/sources/common/base.mjscomponents/zoho_workdrive/sources/new-file-in-folder/new-file-in-folder.mjscomponents/zoho_workdrive/sources/new-folder/new-folder.mjs
🧰 Additional context used
🧬 Code graph analysis (1)
components/zoho_workdrive/sources/common/base.mjs (2)
components/zoho_workdrive/sources/new-file-in-folder/new-file-in-folder.mjs (1)
lastDate(85-85)components/zoho_workdrive/sources/new-folder/new-folder.mjs (1)
lastDate(55-55)
⏰ 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). (3)
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
🔇 Additional comments (15)
components/zoho_workdrive/actions/download-file/download-file.mjs (1)
13-13: LGTM: Version bump is appropriate.This version bump aligns with the broader package version update and the fixes introduced in related components.
components/zoho_workdrive/package.json (1)
3-3: LGTM: Package version bump is consistent with component updates.components/zoho_workdrive/actions/upload-file/upload-file.mjs (1)
11-11: LGTM: Version bump is coordinated with package release.components/zoho_workdrive/common/additionalFolderProps.mjs (2)
1-15: LGTM: Manual folder ID entry logic is well-implemented.The early-exit pattern cleanly separates the manual entry flow from the dynamic folder selection flow, addressing the PR objective to handle custom expressions gracefully.
17-75: LGTM: Folder listing refactor is well-structured.The refactored logic correctly handles root folder resolution, subfolder fetching, and dynamic prop generation. The error handling ensures the function won't throw on invalid folder IDs.
components/zoho_workdrive/sources/common/base.mjs (2)
23-30: LGTM: Manual folder ID prop is well-defined.The prop is properly configured with appropriate defaults and reloadProps to trigger dynamic UI updates.
32-34: LGTM: additionalProps correctly passes existingProps.The updated signature properly integrates with the refactored additionalFolderProps function.
components/zoho_workdrive/sources/new-file-in-folder/new-file-in-folder.mjs (4)
12-12: Version bump looks appropriate.The version increment from 0.2.1 to 0.2.2 correctly reflects the addition of manual folder ID entry and timestamp validation fixes.
85-85: Good defensive programming for timestamp handling.Switching to
_getValidatedLastDate()directly addresses the PR objective of ensuring reliable date filtering. This validates that the stored timestamp is in the correct format (milliseconds) before using it for comparison.
71-79: This concern is not applicable. The code correctly handles manual folder entry. WhenmanuallyEnterFolderIdis true, theadditionalFolderPropsfunction automatically hides thefolderIdfield and marks it asoptional: true, while showingtypedFolderIdas a text input instead. There is no UX conflict—users are never forced to select from the dropdown when using manual entry.Likely an incorrect or invalid review comment.
98-105: No action needed. The field namecreated_time_in_millisecondis correct per Zoho Workdrive API documentation and is properly formatted as milliseconds since epoch (confirmed by test data). The timestamp filtering and comparison logic is sound.components/zoho_workdrive/sources/new-folder/new-folder.mjs (4)
10-10: Version bump is appropriate.The version increment from 0.1.1 to 0.1.2 correctly reflects the changes made.
41-49: Manual folder ID logic is consistent.This implementation mirrors the logic in
new-file-in-folder.mjs, providing a consistent experience across sources. The same concern about thefolderIdrequirement potentially conflicting with manual entry applies here (see comment in the other file).
55-55: Consistent timestamp validation across sources.The switch to
_getValidatedLastDate()is consistent with the changes innew-file-in-folder.mjsand improves timestamp handling reliability.
68-75: Same timestamp field verification needed.This source uses the same
item.attributes.created_time_in_millisecondfield as the file source. The API documentation verification requested in the other file applies here as well to ensure this field name and format are correct for the Zoho Workdrive API.
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: 2
🤖 Fix all issues with AI agents
In @components/zoho_workdrive/sources/common/base.mjs:
- Around line 42-51: The _getValidatedLastDate method uses isNaN(parsed) to
detect invalid Date.parse results; change that call to Number.isNaN(parsed) for
consistency with Number.isInteger() and to avoid type-coercion semantics. Update
the ternary condition in _getValidatedLastDate so it uses Number.isNaN(parsed)
instead of isNaN(parsed), leaving the rest of the logic (returning 0 on invalid
parse) intact.
- Around line 32-34: The additionalProps wrapper calls additionalFolderProps
which assumes existingProps and its keys exist; update additionalFolderProps to
first verify existingProps is defined and only modify existingProps.folderId and
existingProps.folderType (and any other accessed keys) if those properties
exist. Specifically, when checking this.manuallyEnterFolderId, guard each
mutation with checks like "if (existingProps && existingProps.folderId) { ... }"
and similarly for folderType, so you don't attempt to set .hidden/.optional on
undefined objects; also ensure additionalProps gracefully handles a missing or
empty existingProps input before delegating to additionalFolderProps.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/zoho_workdrive/sources/common/base.mjs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-09T18:07:12.426Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 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/zoho_workdrive/sources/common/base.mjs
🧬 Code graph analysis (1)
components/zoho_workdrive/sources/common/base.mjs (2)
components/zoho_workdrive/sources/new-file-in-folder/new-file-in-folder.mjs (1)
lastDate(85-85)components/zoho_workdrive/sources/new-folder/new-folder.mjs (1)
lastDate(55-55)
⏰ 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 (1)
components/zoho_workdrive/sources/common/base.mjs (1)
23-30: LGTM! Prop structure supports manual folder ID entry.The new
manuallyEnterFolderIdprop is well-structured with appropriate defaults andreloadProps: trueto trigger dynamic prop reloading. This addresses the PR objective to prevent additionalProps from throwing when a custom expression is provided for the Folder ID.
GTFalcao
left a comment
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!
For Integration QA: |
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
Resolves #19644
Summary by CodeRabbit
New Features
Changes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.