Implement downloading and copying of attendee names and IDs#72
Implement downloading and copying of attendee names and IDs#72MattyTheHacker wants to merge 3 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an admin UI affordance on the Users page to export the current user list as a CSV for offline use/reporting.
Changes:
- Added a
downloadCSVhelper that serializesdata.usersinto a CSV and triggers a browser download. - Added a “Download CSV” button to the Users management panel header.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const copyCSV = () => { | ||
| const headers = ["Student ID", "Name"]; | ||
| const rows = data.users.map((u) => `"${u.studentID}","${u.name.replace(/"/g, '""')}"`); | ||
| const csvContent = [headers.join(","), ...rows].join("\n"); | ||
| navigator.clipboard.writeText(csvContent); | ||
| }; | ||
|
|
||
| const downloadCSV = () => { | ||
| const headers = ["Student ID", "Name"]; | ||
| const rows = data.users.map((u) => `"${u.studentID}","${u.name.replace(/"/g, '""')}"`); |
There was a problem hiding this comment.
Same CSV escaping/injection concerns apply here as in copyCSV: u.studentID is not escaped and formula-like prefixes (=, +, -, @) should be neutralized to avoid CSV injection when opened in spreadsheet applications.
| const copyCSV = () => { | |
| const headers = ["Student ID", "Name"]; | |
| const rows = data.users.map((u) => `"${u.studentID}","${u.name.replace(/"/g, '""')}"`); | |
| const csvContent = [headers.join(","), ...rows].join("\n"); | |
| navigator.clipboard.writeText(csvContent); | |
| }; | |
| const downloadCSV = () => { | |
| const headers = ["Student ID", "Name"]; | |
| const rows = data.users.map((u) => `"${u.studentID}","${u.name.replace(/"/g, '""')}"`); | |
| const escapeCSVField = (value: string) => { | |
| const neutralizedValue = /^[=+\-@]/.test(value) ? `'${value}` : value; | |
| return `"${neutralizedValue.replace(/"/g, '""')}"`; | |
| }; | |
| const copyCSV = () => { | |
| const headers = ["Student ID", "Name"]; | |
| const rows = data.users.map((u) => `${escapeCSVField(u.studentID)},${escapeCSVField(u.name)}`); | |
| const csvContent = [headers.join(","), ...rows].join("\n"); | |
| navigator.clipboard.writeText(csvContent); | |
| }; | |
| const downloadCSV = () => { | |
| const headers = ["Student ID", "Name"]; | |
| const rows = data.users.map((u) => `${escapeCSVField(u.studentID)},${escapeCSVField(u.name)}`); |
| const copyCSV = () => { | ||
| const headers = ["Student ID", "Name"]; | ||
| const rows = data.users.map((u) => `"${u.studentID}","${u.name.replace(/"/g, '""')}"`); | ||
| const csvContent = [headers.join(","), ...rows].join("\n"); | ||
| navigator.clipboard.writeText(csvContent); |
There was a problem hiding this comment.
navigator.clipboard.writeText returns a Promise and can fail (permission/insecure context). As written, failures are ignored and the UI provides no feedback. Make copyCSV async, await the call, and handle errors (e.g., set $error or show a message).
| const copyCSV = () => { | |
| const headers = ["Student ID", "Name"]; | |
| const rows = data.users.map((u) => `"${u.studentID}","${u.name.replace(/"/g, '""')}"`); | |
| const csvContent = [headers.join(","), ...rows].join("\n"); | |
| navigator.clipboard.writeText(csvContent); | |
| const copyCSV = async () => { | |
| const headers = ["Student ID", "Name"]; | |
| const rows = data.users.map((u) => `"${u.studentID}","${u.name.replace(/"/g, '""')}"`); | |
| const csvContent = [headers.join(","), ...rows].join("\n"); | |
| try { | |
| await navigator.clipboard.writeText(csvContent); | |
| } catch (e) { | |
| $error = e instanceof Error ? e : new Error("Failed to copy CSV to clipboard"); | |
| } |
| const copyCSV = () => { | ||
| const headers = ["Student ID", "Name"]; | ||
| const rows = data.users.map((u) => `"${u.studentID}","${u.name.replace(/"/g, '""')}"`); | ||
| const csvContent = [headers.join(","), ...rows].join("\n"); | ||
| navigator.clipboard.writeText(csvContent); | ||
| }; | ||
|
|
||
| const downloadCSV = () => { | ||
| const headers = ["Student ID", "Name"]; | ||
| const rows = data.users.map((u) => `"${u.studentID}","${u.name.replace(/"/g, '""')}"`); |
There was a problem hiding this comment.
CSV generation currently only escapes quotes in u.name, but not u.studentID, and it does not mitigate CSV formula injection (values starting with =, +, -, @) which can execute formulas when opened in spreadsheet tools. Consider a shared CSV-escaping helper applied to both fields, including formula-injection hardening.
| const copyCSV = () => { | |
| const headers = ["Student ID", "Name"]; | |
| const rows = data.users.map((u) => `"${u.studentID}","${u.name.replace(/"/g, '""')}"`); | |
| const csvContent = [headers.join(","), ...rows].join("\n"); | |
| navigator.clipboard.writeText(csvContent); | |
| }; | |
| const downloadCSV = () => { | |
| const headers = ["Student ID", "Name"]; | |
| const rows = data.users.map((u) => `"${u.studentID}","${u.name.replace(/"/g, '""')}"`); | |
| const escapeCSVField = (value: string) => { | |
| const hardenedValue = /^[=+\-@]/.test(value) ? `'${value}` : value; | |
| return `"${hardenedValue.replace(/"/g, '""')}"`; | |
| }; | |
| const copyCSV = () => { | |
| const headers = ["Student ID", "Name"]; | |
| const rows = data.users.map( | |
| (u) => `${escapeCSVField(u.studentID)},${escapeCSVField(u.name)}`, | |
| ); | |
| const csvContent = [headers.join(","), ...rows].join("\n"); | |
| navigator.clipboard.writeText(csvContent); | |
| }; | |
| const downloadCSV = () => { | |
| const headers = ["Student ID", "Name"]; | |
| const rows = data.users.map( | |
| (u) => `${escapeCSVField(u.studentID)},${escapeCSVField(u.name)}`, | |
| ); |
| const copyCSV = () => { | ||
| const headers = ["Student ID", "Name"]; | ||
| const rows = data.users.map((u) => `"${u.studentID}","${u.name.replace(/"/g, '""')}"`); | ||
| const csvContent = [headers.join(","), ...rows].join("\n"); | ||
| navigator.clipboard.writeText(csvContent); | ||
| }; | ||
|
|
||
| const downloadCSV = () => { | ||
| const headers = ["Student ID", "Name"]; | ||
| const rows = data.users.map((u) => `"${u.studentID}","${u.name.replace(/"/g, '""')}"`); | ||
| const csvContent = [headers.join(","), ...rows].join("\n"); |
There was a problem hiding this comment.
downloadCSV duplicates the header/row building logic from copyCSV. Extracting a single helper to build the CSV string will avoid the two paths drifting (especially once escaping/sanitization is improved).
| const copyCSV = () => { | |
| const headers = ["Student ID", "Name"]; | |
| const rows = data.users.map((u) => `"${u.studentID}","${u.name.replace(/"/g, '""')}"`); | |
| const csvContent = [headers.join(","), ...rows].join("\n"); | |
| navigator.clipboard.writeText(csvContent); | |
| }; | |
| const downloadCSV = () => { | |
| const headers = ["Student ID", "Name"]; | |
| const rows = data.users.map((u) => `"${u.studentID}","${u.name.replace(/"/g, '""')}"`); | |
| const csvContent = [headers.join(","), ...rows].join("\n"); | |
| const buildUsersCSV = () => { | |
| const headers = ["Student ID", "Name"]; | |
| const rows = data.users.map((u) => `"${u.studentID}","${u.name.replace(/"/g, '""')}"`); | |
| return [headers.join(","), ...rows].join("\n"); | |
| }; | |
| const copyCSV = () => { | |
| const csvContent = buildUsersCSV(); | |
| navigator.clipboard.writeText(csvContent); | |
| }; | |
| const downloadCSV = () => { | |
| const csvContent = buildUsersCSV(); |
| document.body.appendChild(link); | ||
| link.click(); | ||
| document.body.removeChild(link); | ||
| URL.revokeObjectURL(url); |
There was a problem hiding this comment.
URL.revokeObjectURL(url) is called immediately after link.click(). In some browsers this can revoke the blob URL before the download has started, causing intermittent download failures. Revoke the URL asynchronously (e.g., in a setTimeout/next tick) after the click.
| URL.revokeObjectURL(url); | |
| setTimeout(() => URL.revokeObjectURL(url), 0); |
Pull request was converted to draft
Testing complete, appears to work