-
Notifications
You must be signed in to change notification settings - Fork 3.3k
feat(create-excel): onedrive create excel #1745
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. |
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.
Greptile Overview
Greptile Summary
This PR adds Excel file creation capability to the OneDrive integration, implementing a three-step process: creating a blank workbook, establishing a session lock, and writing content via Microsoft Graph API.
Key Changes
- Added Excel workbook creation using the
xlsxlibrary to generate blank.xlsxfiles - Implemented workbook session management to lock files during editing
- Added content writing via Microsoft Graph Excel API with support for 2D arrays and object arrays
- Extended UI with file type selector (text/Excel) and values input field
- Added proper error handling and graceful degradation when session creation fails
Issues Found
- Critical: Column letter calculation has an off-by-one error that will produce incorrect Excel ranges for datasets with more than 26 columns
- Column ordering from object arrays may be non-deterministic due to
Setusage
Confidence Score: 3/5
- This PR has one critical bug that will break Excel files with >26 columns
- The implementation is well-structured with good error handling, but the
indexToColLettersfunction has a mathematical error in base-26 conversion that will cause incorrect Excel range addresses for larger datasets. This is a critical bug that needs fixing before merge. - Pay close attention to
apps/sim/app/api/tools/onedrive/upload/route.ts- specifically the column letter conversion logic around line 319
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/app/api/tools/onedrive/upload/route.ts | 3/5 | Added Excel file creation with workbook session management and content writing via Microsoft Graph API; contains several logical issues that need attention |
| apps/sim/blocks/blocks/onedrive.ts | 5/5 | Added UI controls for Excel file creation with mimeType selector and values input field with JSON parsing |
| apps/sim/tools/onedrive/types.ts | 5/5 | Added type definitions for Excel write results and values parameter |
| apps/sim/tools/onedrive/upload.ts | 5/5 | Updated routing logic to handle Excel file creation and route to custom API |
Sequence Diagram
sequenceDiagram
participant User
participant UI as OneDrive Block UI
participant API as /api/tools/onedrive/upload
participant XLSX as XLSX Library
participant Graph as Microsoft Graph API
User->>UI: Select "Create File" + Excel type
User->>UI: Enter filename & values (optional)
UI->>API: POST with mimeType, fileName, values
alt Excel File Creation
API->>API: Check isExcelCreation flag
API->>XLSX: Create blank workbook
XLSX-->>API: Return .xlsx buffer
API->>Graph: PUT /drive/items/.../content
Graph-->>API: Return file metadata
alt Values Provided
API->>Graph: POST /workbook/createSession
Graph-->>API: Return session ID
API->>Graph: GET /workbook/worksheets (get first sheet)
Graph-->>API: Return sheet name
API->>API: Process values & compute range
API->>API: indexToColLetters() calculation
API->>Graph: PATCH /worksheets/{sheet}/range
Graph-->>API: Return updated range
API->>Graph: POST /workbook/closeSession
end
API-->>UI: Return file + excelWriteResult
else Text File Creation
API->>Graph: PUT /drive/root:/{filename}:/content
Graph-->>API: Return file metadata
API-->>UI: Return file metadata
end
UI-->>User: Display success
4 files reviewed, 3 comments
| const allKeys = new Set<string>() | ||
| processedValues.forEach((obj: any) => { | ||
| if (obj && typeof obj === 'object') { | ||
| Object.keys(obj).forEach((key) => allKeys.add(key)) | ||
| } | ||
| }) |
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.
style: Using Set for collecting object keys may result in non-deterministic column ordering. Consider using a more predictable approach like collecting keys in first-seen order to ensure consistent Excel column layout across runs.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/tools/onedrive/upload/route.ts
Line: 281:286
Comment:
**style:** Using `Set` for collecting object keys may result in non-deterministic column ordering. Consider using a more predictable approach like collecting keys in first-seen order to ensure consistent Excel column layout across runs.
How can I resolve this? If you propose a fix, please make it concise.
Summary
Creates a excel file in onedrive. Onedrive does not support directly adding new excel sheets with content, so we have to make a call to create an empty excel sheet, create a session to create a lock on workbook edits, and then call the excel endpoint to write content to the new workbook.
Type of Change
Testing
This has been tested with and without content present in the excel
Checklist
Screenshots/Videos
Screen.Recording.2025-10-27.at.3.17.25.PM.mov