Zoho Workdrive - update folderId props to fix pagination issue #18495
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughIntroduces dynamic folder selection helpers and shared source base. Refactors actions and sources to use additional folder props, max folder resolution, and conditional subfolder loading. Updates pagination logic and timestamps, adds syncDir, and bumps versions across modules, including package.json. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Action as Action (Download/Upload)
participant Helpers as additionalFolderProps/findMaxFolderId
participant App as Zoho WorkDrive App
User->>Action: Open config UI
Action->>Helpers: additionalProps(context)
Helpers->>App: listRootFolders(includeSubfolders?)
App-->>Helpers: Root folders (paged/options)
loop For each selected folderIdN
Helpers->>App: list subfolders of folderIdN
App-->>Helpers: Subfolders (paged/options)
Helpers-->>Action: Append folderIdN+1 prop (reloadProps)
end
User->>Action: Select Folder ID(+N), File ID/Name
Action->>Helpers: findMaxFolderId(context)
Helpers-->>Action: max index (N)
Action->>App: listFiles(folderIdN) (paginated)
App-->>Action: File options/results
sequenceDiagram
autonumber
participant Source as Source (New File/Folder)
participant Common as Common Base
participant Helpers as findMaxFolderId
participant App as Zoho WorkDrive App
participant DB as Source DB
Common->>DB: _getLastDate()
Source->>Helpers: findMaxFolderId(props)
Helpers-->>Source: folderId index (N or 0)
Source->>App: paginate(listFiles, { folderIdN })
App-->>Source: Items (created_time_in_millisecond)
Source->>Source: Filter items > lastDate
Source->>DB: _setLastDate(max created_time_in_millisecond)
Source-->>Source: Emit events (summary uses ID, ts=ms)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ 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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/zoho_workdrive/sources/new-file-in-folder/new-file-in-folder.mjs (1)
81-102: Useattributes.created_time(epoch ms) instead ofcreated_time_in_millisecond.Change
item.attributes.created_time_in_millisecondtoitem.attributes.created_timeto match the WorkDrive API’s v1 response. Numeric comparison andmaxDatelogic remain valid.components/zoho_workdrive/actions/upload-file/upload-file.mjs (1)
58-63: Remove unused syncDir propThe
syncDirprop incomponents/zoho_workdrive/actions/upload-file/upload-file.mjs(lines 58–63) isn’t referenced in therun()method or elsewhere—delete it to avoid dead code.
🧹 Nitpick comments (2)
components/zoho_workdrive/common/additionalFolderProps.mjs (1)
63-79: Consider returning 0 instead of -Infinity for clarity.The function initializes
maxNumto-Infinity(line 64) as a sentinel value when no folder props are found. While the logic is correct and callers handle this properly (e.g.,num < 1checks), returning0or-1would be more conventional and clearer as a "not found" indicator.Apply this diff to improve clarity:
function findMaxFolderId(obj) { - let maxNum = -Infinity; + let maxNum = 0; Object.keys(obj).forEach((key) => { const match = key.match(/^(folderId(\d*)|parentId)$/); if (match) { const num = match[2] === undefined || match[2] === "" ? 0 : parseInt(match[2], 10); if (num > maxNum) { maxNum = num; } } }); return maxNum; }components/zoho_workdrive/actions/download-file/download-file.mjs (1)
89-93: Consider consistency with upload-file.mjs usage.This file assigns
findMaxFolderIdas a method (line 90) and calls it asthis.findMaxFolderId(this)(line 61), whereasupload-file.mjs(line 80) calls the imported function directly asfindMaxFolderId(this). Both approaches work, but for consistency across the codebase, consider using the same pattern—either assign to methods or call the import directly.Option 1: Call the import directly (matching upload-file.mjs):
Remove from methods and call directly in additionalProps:
async additionalProps() { const folderProps = await additionalFolderProps.call(this); const props = { ...folderProps, }; props.fileId = { type: "string", label: "File ID", description: "The unique ID of the file to download.", withLabel: true, options: async ({ page }) => { - const num = this.findMaxFolderId(this); + const num = findMaxFolderId(this); const limit = this.getLimit(); // ... rest of options }, }; // ... rest of additionalProps }, methods: { - findMaxFolderId, getLimit() { return LIMIT; }, },Option 2: Keep as method and update upload-file.mjs to match (would require changes outside this PR).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
components/zoho_workdrive/actions/download-file/download-file.mjs(3 hunks)components/zoho_workdrive/actions/upload-file/upload-file.mjs(4 hunks)components/zoho_workdrive/common/additionalFolderProps.mjs(1 hunks)components/zoho_workdrive/package.json(1 hunks)components/zoho_workdrive/sources/common/base.mjs(1 hunks)components/zoho_workdrive/sources/new-file-in-folder/new-file-in-folder.mjs(5 hunks)components/zoho_workdrive/sources/new-folder/new-folder.mjs(3 hunks)components/zoho_workdrive/zoho_workdrive.app.mjs(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
components/zoho_workdrive/sources/common/base.mjs (2)
components/zoho_workdrive/sources/new-file-in-folder/new-file-in-folder.mjs (1)
lastDate(81-81)components/zoho_workdrive/sources/new-folder/new-folder.mjs (1)
lastDate(51-51)
components/zoho_workdrive/actions/upload-file/upload-file.mjs (4)
components/zoho_workdrive/actions/download-file/download-file.mjs (2)
num(61-61)num(63-72)components/zoho_workdrive/common/additionalFolderProps.mjs (2)
num(30-30)num(69-71)components/zoho_workdrive/sources/new-file-in-folder/new-file-in-folder.mjs (1)
num(72-72)components/zoho_workdrive/sources/new-folder/new-folder.mjs (1)
num(42-42)
components/zoho_workdrive/common/additionalFolderProps.mjs (4)
components/zoho_workdrive/actions/download-file/download-file.mjs (3)
props(52-54)num(61-61)num(63-72)components/zoho_workdrive/actions/upload-file/upload-file.mjs (1)
num(80-80)components/zoho_workdrive/sources/new-file-in-folder/new-file-in-folder.mjs (1)
num(72-72)components/zoho_workdrive/sources/new-folder/new-folder.mjs (1)
num(42-42)
components/zoho_workdrive/actions/download-file/download-file.mjs (1)
components/zoho_workdrive/common/additionalFolderProps.mjs (3)
props(2-2)num(30-30)num(69-71)
components/zoho_workdrive/sources/new-file-in-folder/new-file-in-folder.mjs (4)
components/zoho_workdrive/actions/download-file/download-file.mjs (2)
num(61-61)num(63-72)components/zoho_workdrive/actions/upload-file/upload-file.mjs (1)
num(80-80)components/zoho_workdrive/common/additionalFolderProps.mjs (2)
num(30-30)num(69-71)components/zoho_workdrive/sources/new-folder/new-folder.mjs (6)
num(42-42)folderId(43-45)lastDate(51-51)maxDate(52-52)items(53-59)createdTime(64-64)
components/zoho_workdrive/sources/new-folder/new-folder.mjs (4)
components/zoho_workdrive/actions/download-file/download-file.mjs (2)
num(61-61)num(63-72)components/zoho_workdrive/actions/upload-file/upload-file.mjs (1)
num(80-80)components/zoho_workdrive/common/additionalFolderProps.mjs (2)
num(30-30)num(69-71)components/zoho_workdrive/sources/new-file-in-folder/new-file-in-folder.mjs (7)
num(72-72)folderId(73-75)lastDate(81-81)maxDate(82-82)items(83-89)responseArray(91-91)createdTime(94-94)
⏰ 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: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (23)
components/zoho_workdrive/package.json (1)
3-3: LGTM! Version bump is appropriate for the pagination bug fix.The patch version increment from 0.2.6 to 0.2.7 correctly follows semantic versioning for a bug fix that addresses the HTTP 429 rate-limit errors.
components/zoho_workdrive/zoho_workdrive.app.mjs (2)
142-148: LGTM! Core fix for 429 rate-limit issue.The conditional subfolder loading prevents excessive pagination when
includeSubfoldersisfalse, directly addressing the 429 rate-limit errors described in issue #18457. The logic correctly skips the expensivepaginateFolderscall when not needed.
31-31: No external callers oflistRootFolders—default mismatch is intentional and safe.components/zoho_workdrive/actions/download-file/download-file.mjs (1)
60-71: LGTM! Dynamic folder selection and pagination logic.The dynamic folder selection logic (lines 64-66) correctly identifies the deepest selected folder, mirroring the approach in
upload-file.mjs. The pagination parameters (lines 69-70) properly use thegetLimit()method, maintaining consistency with the LIMIT constant.components/zoho_workdrive/sources/common/base.mjs (1)
35-37: All extending sources definestartEvent(). No further action required.components/zoho_workdrive/common/additionalFolderProps.mjs (1)
8-10: Verify pagination behavior of listFiles
The defaultLIMITis 50, but I couldn’t find thelistFilesdefinition to confirm if it supports overriding the page size or paginating through multiple pages. At lines 8–10 and 37–39, you fetch subfolders without pagination parameters—please verify whetherthis.app.listFilesaccepts options likelimit,page, oroffsetto ensure all subfolders are retrieved.components/zoho_workdrive/sources/new-file-in-folder/new-file-in-folder.mjs (5)
1-9: LGTM! Clean refactoring to shared base.The migration to a common base with spread syntax promotes consistency across sources and reduces duplication.
16-39: LGTM! Dynamic folder prop pattern aligns with pagination fix.The
reloadProps: trueflag enables dynamic subfolder loading, which is central to the pagination fix described in the PR objectives. The updated description clearly communicates this behavior.
53-54: LGTM! Shared methods reduce duplication.Spreading common methods eliminates local state management duplication across sources.
104-117: LGTM! Event payload consistent with timestamp changes.The emitted event uses the same
created_time_in_millisecondfield for thetsproperty, maintaining consistency with the filtering logic.
71-79: Dynamic folder props mechanism verified. TheadditionalFolderPropsfunction correctly generatesfolderId{n}properties andfindMaxFolderIdis properly imported and used—no further changes needed.components/zoho_workdrive/sources/new-folder/new-folder.mjs (6)
1-7: LGTM! Consistent with new-file-in-folder.mjs.The shared base pattern is applied consistently across sources.
14-37: LGTM! Dynamic folder props match pattern.Consistent implementation of
reloadProps: truefor subfolder navigation.
40-49: Core pagination fix applied consistently.The folder resolution logic matches new-file-in-folder.mjs exactly, ensuring consistent behavior across sources.
51-72: LGTM! Correct filter for folder detection.The
filter: "folder"parameter correctly scopes the pagination to folders only, distinguishing this source from the file detection source.
74-84: LGTM! Event payload consistent.Correct use of "folder" terminology and consistent timestamp handling.
86-90: Verify deploy limit difference is intentional.This source fetches 25 items on deploy while new-file-in-folder.mjs fetches 10. Is this difference intentional, or should they use the same limit for consistency?
components/zoho_workdrive/actions/upload-file/upload-file.mjs (6)
4-6: LGTM!The imports are correctly structured and the functions are properly utilized in the
additionalProps()method and therun()method.
11-11: LGTM!Version bump is appropriate for the changes made.
65-67: LGTM!The
additionalProps()method correctly delegates toadditionalFolderPropswith the component context, enabling dynamic folder selection props.
28-40: LGTM!The addition of
reloadProps: truecorrectly triggers dynamic prop re-evaluation when the parent folder changes, enabling the cascading folder selection mechanism.
80-83: LGTM!The
findMaxFolderIdlogic correctly determines the deepest selected folder level and falls back toparentIdwhen no dynamic folders are selected. This pattern is consistent with the implementation in other files across the component.
85-85: LGTM!The computed
parentIdis correctly applied to the form data. This change enables users to select deeply nested folders without triggering the pagination issue that caused HTTP 429 errors.
Resolves #18457
Summary by CodeRabbit
New Features
Chores