Skip to content

feat: add subjectID as page name for excel download #1219

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.

Sign up for GitHub

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

Merged
joshunrau merged 2 commits into from
Oct 23, 2025
Merged

feat: add subjectID as page name for excel download #1219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
78c8ef2
feat: add subjectID as page name for excel download
david-roper Oct 23, 2025
b4afa29
feat: add code rabbit sheet name filter
david-roper Oct 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/web/src/hooks/useInstrumentVisualization.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,12 @@ export function useInstrumentVisualization({ params }: UseInstrumentVisualizatio
}
case 'Excel': {
const rows = makeWideRows();
downloadSubjectTableExcel(`${baseFilename}.xlsx`, rows);
downloadSubjectTableExcel(`${baseFilename}.xlsx`, rows, removeSubjectIdScope(params.subjectId));
break;
}
case 'Excel Long': {
const rows = makeLongRows();
downloadSubjectTableExcel(`${baseFilename}.xlsx`, rows);
downloadSubjectTableExcel(`${baseFilename}.xlsx`, rows, removeSubjectIdScope(params.subjectId));
break;
}
case 'JSON': {
Expand Down
11 changes: 9 additions & 2 deletions apps/web/src/utils/excel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,15 @@ export function downloadExcel(filename: string, recordsExport: InstrumentRecords
writeFileXLSX(workbook, filename);
}

export function downloadSubjectTableExcel(filename: string, records: { [key: string]: any }[]) {
export function downloadSubjectTableExcel(filename: string, records: { [key: string]: any }[], name: string) {
const sanitizedName =
name
.replace(/[\\/?*[\]:]/g, '_') // Replace invalid chars
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Suggested change
.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.

.slice(0, 31) // Max 31 chars
.replace(/^'|'$/g, '') // Remove leading/trailing apostrophes
.trim() || 'Subject'; // Fallback if empty
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), sanitizedName);
utils.book_append_sheet(workbook, utils.json_to_sheet(records), name);
Comment on lines +18 to +19
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

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.

Suggested change
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).

writeFileXLSX(workbook, filename);
}