-
Notifications
You must be signed in to change notification settings - Fork 3.3k
fix(inputs): canonical params + manual validations + params resolution cleanups #3141
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.
|
Greptile OverviewGreptile SummaryFixed canonical parameter validation for OneDrive and Google Drive blocks by ensuring operation-specific canonical param IDs are used throughout. The changes address a bug where required field validation failed when using canonical parameters (basic/advanced mode pairs) because the serializer was checking the wrong field names. Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant UI as Block Editor UI
participant SubBlock as SubBlock Component
participant Serializer
participant BlockConfig as OneDrive/GoogleDrive Block
User->>UI: Select operation (e.g., "download")
UI->>SubBlock: Render file-selector with canonicalParamId="downloadFileId"
SubBlock->>SubBlock: buildCanonicalIndex() maps subBlock.id -> canonicalId
alt Basic Mode
User->>SubBlock: Select file from dropdown
SubBlock->>SubBlock: Store value in downloadFileSelector
else Advanced Mode
User->>SubBlock: Enter file ID manually
SubBlock->>SubBlock: Store value in downloadManualFileId
end
User->>UI: Execute workflow
UI->>Serializer: serialize(block, params)
Serializer->>Serializer: validateRequiredFields()
Note over Serializer: Check if subBlock is required
Serializer->>Serializer: Get canonicalId from canonicalIdBySubBlockId[subBlock.id]
Serializer->>Serializer: Look up params[canonicalId] instead of params[subBlock.id]
alt Value exists
Serializer->>BlockConfig: params.downloadFileId exists
BlockConfig->>BlockConfig: Resolve operation-specific fileId
BlockConfig-->>Serializer: Valid params with fileId
else Value missing
Serializer-->>UI: Error: "Missing required fields"
end
|
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.
3 files reviewed, 5 comments
Additional Comments (5)
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/sim/blocks/blocks/onedrive.ts
Line: 183:183
Comment:
should be `'createFolderParentId'` to match the input declaration on line 411 and follow the same pattern as Google Drive
```suggestion
canonicalParamId: 'createFolderParentId',
```
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/sim/blocks/blocks/onedrive.ts
Line: 204:204
Comment:
should be `'createFolderParentId'` to match the input declaration on line 411 and follow the same pattern as Google Drive
```suggestion
canonicalParamId: 'createFolderParentId',
```
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/sim/blocks/blocks/onedrive.ts
Line: 261:261
Comment:
should be `'downloadFileId'` to match the input declaration on line 414 and follow the same pattern as Google Drive
```suggestion
canonicalParamId: 'downloadFileId',
```
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/sim/blocks/blocks/onedrive.ts
Line: 283:283
Comment:
should be `'downloadFileId'` to match the input declaration on line 414 and follow the same pattern as Google Drive
```suggestion
canonicalParamId: 'downloadFileId',
```
How can I resolve this? If you propose a fix, please make it concise.
The function should:
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/sim/blocks/blocks/onedrive.ts
Line: 356:396
Comment:
this params function uses the old field IDs (`folderSelector`, `manualFolderId`, `fileSelector`, `manualFileId`) that no longer exist after the canonical param refactoring. It needs to be updated to destructure and handle the new canonical param IDs per-operation, similar to how Google Drive was fixed.
The function should:
1. Destructure the new canonical params like `uploadFolderId`, `listFolderId`, `createFolderParentId`, `downloadFileId`, `deleteFileId`
2. Resolve `folderId` based on the operation type (upload/create_file → uploadFolderId, list → listFolderId, create_folder → createFolderParentId)
3. Resolve `fileId` based on the operation type (download → downloadFileId, delete → deleteFileId)
How can I resolve this? If you propose a fix, please make it concise. |
|
@greptile |
|
@cursor review |
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.
3 files reviewed, no comments
...mponents/panel/components/editor/components/sub-block/components/file-upload/file-upload.tsx
Show resolved
Hide resolved
|
@cursor review |
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.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Summary
Type of Change
Testing
Tested manually
Checklist