feat: add subjectID as page name for excel download #1219
Conversation
WalkthroughExcel export was updated to accept a worksheet name: Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as UI / Hook
participant Excel as utils/excel.downloadSubjectTableExcel
note over UI,Excel #D6EAF8: Excel/Excel Long path
UI->>Excel: downloadSubjectTableExcel(filename, rows, removeSubjectIdScope(subjectId))
Excel-->>UI: writes .xlsx with sanitized sheet name + original name sheet
note over UI,Excel #F8F3D4: Other export paths (CSV/JSON/TSV)
UI->>UI: exportCSV/exportJSON/exportTSV(rows)
UI-->>User: triggers download
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/hooks/useInstrumentVisualization.ts(1 hunks)apps/web/src/utils/excel.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/hooks/useInstrumentVisualization.ts (2)
apps/web/src/utils/excel.ts (1)
downloadSubjectTableExcel(10-14)packages/subject-utils/src/index.ts (1)
removeSubjectIdScope(34-39)
⏰ 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). (1)
- GitHub Check: lint-and-test
| export function downloadSubjectTableExcel(filename: string, records: { [key: string]: any }[], name: string) { | ||
| const workbook = utils.book_new(); | ||
| utils.book_append_sheet(workbook, utils.json_to_sheet(records), 'ULTRA_LONG'); | ||
| utils.book_append_sheet(workbook, utils.json_to_sheet(records), name); |
There was a problem hiding this comment.
Validate and sanitize the worksheet name.
Excel worksheet names have strict constraints: max 31 characters, cannot contain \ / ? * [ ] :, cannot be empty, and cannot start/end with apostrophe. Subject IDs may violate these restrictions, causing export failures.
Add validation before using the name:
export function downloadSubjectTableExcel(filename: string, records: { [key: string]: any }[], name: string) {
+ // Sanitize worksheet name for Excel constraints
+ const sanitizedName = name
+ .replace(/[\\\/\?\*\[\]:]/g, '_') // Replace invalid chars
+ .slice(0, 31) // Max 31 chars
+ .replace(/^'|'$/g, '') // Remove leading/trailing apostrophes
+ .trim() || 'Sheet1'; // Fallback if empty
+
const workbook = utils.book_new();
- utils.book_append_sheet(workbook, utils.json_to_sheet(records), name);
+ utils.book_append_sheet(workbook, utils.json_to_sheet(records), sanitizedName);
writeFileXLSX(workbook, filename);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function downloadSubjectTableExcel(filename: string, records: { [key: string]: any }[], name: string) { | |
| const workbook = utils.book_new(); | |
| utils.book_append_sheet(workbook, utils.json_to_sheet(records), 'ULTRA_LONG'); | |
| utils.book_append_sheet(workbook, utils.json_to_sheet(records), name); | |
| export function downloadSubjectTableExcel(filename: string, records: { [key: string]: any }[], name: string) { | |
| // Sanitize worksheet name for Excel constraints | |
| const sanitizedName = name | |
| .replace(/[\\\/\?\*\[\]:]/g, '_') // Replace invalid chars | |
| .slice(0, 31) // Max 31 chars | |
| .replace(/^'|'$/g, '') // Remove leading/trailing apostrophes | |
| .trim() || 'Sheet1'; // Fallback if empty | |
| const workbook = utils.book_new(); | |
| utils.book_append_sheet(workbook, utils.json_to_sheet(records), sanitizedName); |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/utils/excel.ts(1 hunks)
⏰ 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). (1)
- GitHub Check: lint-and-test
| export function downloadSubjectTableExcel(filename: string, records: { [key: string]: any }[], name: string) { | ||
| const sanitizedName = | ||
| name | ||
| .replace(/[\\/?*[\]:]/g, '_') // Replace invalid chars |
There was a problem hiding this comment.
Escape brackets in the regex.
The [ and ] characters inside the character class are not properly escaped. This may cause the regex to malfunction.
Apply this diff:
- .replace(/[\\/?*[\]:]/g, '_') // Replace invalid chars
+ .replace(/[\\/?*\[\]:]/g, '_') // Replace invalid chars📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .replace(/[\\/?*[\]:]/g, '_') // Replace invalid chars | |
| const sanitizedName = name | |
| .replace(/[\\/?*\[\]:]/g, '_') // Replace invalid chars | |
| .slice(0, 31) // Max 31 chars | |
| .replace(/^'|'$/g, '') // Remove leading/trailing apostrophes | |
| .trim() || 'Sheet1'; // Fallback if empty |
🤖 Prompt for AI Agents
In apps/web/src/utils/excel.ts around line 13, the regex used in
.replace(/[\\/?*[\]:]/g, '_') contains unescaped square brackets inside the
character class which can break the pattern; update the character class so the
literal brackets are escaped (for example replace with /[\\/?*\\[\\]:]/g or
place the ] immediately after the opening bracket) so the regex correctly
matches and replaces literal '[' and ']' along with the other invalid filename
characters.
| utils.book_append_sheet(workbook, utils.json_to_sheet(records), sanitizedName); | ||
| utils.book_append_sheet(workbook, utils.json_to_sheet(records), name); |
There was a problem hiding this comment.
Remove the second worksheet with unsanitized name.
Creating a worksheet with the unsanitized name (line 19) defeats the purpose of sanitization and will cause Excel export failures when subject IDs contain invalid characters or exceed 31 characters.
Apply this diff:
utils.book_append_sheet(workbook, utils.json_to_sheet(records), sanitizedName);
- utils.book_append_sheet(workbook, utils.json_to_sheet(records), name);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| utils.book_append_sheet(workbook, utils.json_to_sheet(records), sanitizedName); | |
| utils.book_append_sheet(workbook, utils.json_to_sheet(records), name); | |
| utils.book_append_sheet(workbook, utils.json_to_sheet(records), sanitizedName); |
🤖 Prompt for AI Agents
In apps/web/src/utils/excel.ts around lines 18 to 19, a second worksheet is
being appended using the unsanitized `name`, which contradicts the sanitization
and can cause Excel failures; remove the second utils.book_append_sheet call
that uses `name` so only the sanitizedName is appended (keep the existing
sanitized append, delete or omit the unsanitized one).
2 participants
Adds the subject ID as the page name for the subject table excel download
Summary by CodeRabbit
New Features
Bug Fixes