feat: bulk attendee import from CSV in attendance dashboard (#559) - #772
feat: bulk attendee import from CSV in attendance dashboard (#559)#772saurabhhhcodes wants to merge 2 commits into
Conversation
…mar0036singh#559) Organizers could export attendance but had no way to bulk-register groups (club events, guest lists). - lib/csvImport.js: dependency-free RFC-4180-ish CSV parser (quoted fields, embedded commas, CRLF), parseAttendeeRows validation (required valid email, optional name/roll; dedupe + error rows reported), and a stable deterministic participant doc id (FNV-1a hex, no node crypto needed in RN) - AttendanceDashboard.js: 'Import CSV' button + modal — paste CSV, live validation preview, chunked (450/run) writes to events/{id}/participants/{hash} with attended:false and checkInMethod 'bulk-import' - 11 lib tests (parser, headers, invalid rows, dedupe, normalization, empty input, stable ids)
📝 WalkthroughWalkthroughThe change adds CSV parsing and attendee validation utilities. ChangesCSV attendee import
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🟠 High · up to Bulk imports may currently fail for event owners, reject valid CSVs using the supported e-mail header, and risk overwriting attendee records at larger volumes because of identifier collisions. These correctness, availability, and data-integrity issues should be fixed before merge. Sequence Diagram(s)sequenceDiagram
actor Organizer
participant AttendanceDashboard
participant csvImport
participant Firestore
Organizer->>AttendanceDashboard: Paste CSV and start import
AttendanceDashboard->>csvImport: Parse and validate attendee rows
csvImport-->>AttendanceDashboard: Valid attendees, errors, skipped duplicates
AttendanceDashboard->>Firestore: Write attendees with setDoc in chunks
Firestore-->>AttendanceDashboard: Return write result
AttendanceDashboard-->>Organizer: Show import counts or failure
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
app/src/lib/__tests__/csvImport.test.jsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. app/src/lib/csvImport.jsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. app/src/screens/AttendanceDashboard.jsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. 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: 3
🧹 Nitpick comments (1)
app/src/lib/csvImport.js (1)
9-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce
parseCsvcognitive complexity.SonarCloud reports a complexity of 20 where the configured limit is 15. Extract the quote-state and delimiter-state transitions into focused helpers. Preserve the current parser test cases during the refactor.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/lib/csvImport.js` around lines 9 - 56, Reduce the cognitive complexity of parseCsv by extracting quote-handling and delimiter/newline state transitions into focused helper functions. Keep parseCsv responsible for iteration and coordinating those helpers, while preserving current quote escaping, CRLF handling, row filtering, trailing-cell behavior, and all existing parser test cases.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/lib/csvImport.js`:
- Around line 78-105: Update the column mapping near the header detection so the
email index lookup accepts both “email” and “e-mail” using one shared lookup,
ensuring e-mail,name,roll maps the email column correctly. Add a regression test
covering that header spelling and its imported data row.
- Around line 120-127: Replace the DJB2 calculation in participantIdForEmail
with the specified FNV-1a 32-bit hash (or an approved cryptographic hash
truncated to the required hex length), preserving normalized email input and
deterministic output. Update the email column resolution logic to search for
both “email” and “e-mail”, so headers using either form produce the correct
column index.
In `@app/src/screens/AttendanceDashboard.js`:
- Around line 134-174: The bulk-import writes in the AttendanceDashboard import
flow are rejected because participantIdForEmail() does not match the organizer
UID. Update the participant-document create authorization to allow the event
owner for the relevant event, or route these writes through a trusted backend,
while preserving existing participant access restrictions.
---
Nitpick comments:
In `@app/src/lib/csvImport.js`:
- Around line 9-56: Reduce the cognitive complexity of parseCsv by extracting
quote-handling and delimiter/newline state transitions into focused helper
functions. Keep parseCsv responsible for iteration and coordinating those
helpers, while preserving current quote escaping, CRLF handling, row filtering,
trailing-cell behavior, and all existing parser test cases.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c44d5392-7548-4072-8ff9-be9474ca01fd
📒 Files selected for processing (3)
app/src/lib/__tests__/csvImport.test.jsapp/src/lib/csvImport.jsapp/src/screens/AttendanceDashboard.js
| const looksLikeHeader = headerRow.includes('email') || headerRow.includes('e-mail'); | ||
|
|
||
| const dataRows = looksLikeHeader ? rows.slice(1) : rows; | ||
| const col = { | ||
| email: looksLikeHeader ? headerRow.indexOf('email') : 0, | ||
| name: looksLikeHeader ? headerRow.findIndex(c => c === 'name' || c === 'full name') : 1, | ||
| roll: looksLikeHeader ? headerRow.findIndex(c => c.includes('roll')) : 2, | ||
| }; | ||
|
|
||
| const existing = new Set(existingEmails.map(e => e.trim().toLowerCase())); | ||
| const EMAIL_RE = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | ||
| let skipped = 0; | ||
|
|
||
| dataRows.forEach((cells, index) => { | ||
| const email = String(cells[col.email] ?? '') | ||
| .trim() | ||
| .toLowerCase(); | ||
| const name = String(cells[col.name] ?? '').trim() || undefined; | ||
| const roll = String(cells[col.roll] ?? '').trim() || undefined; | ||
|
|
||
| if (!email) { | ||
| if (cells.length > 1 || cells[0].trim() !== '') { | ||
| errors.push(`Row ${index + 2}: missing email.`); | ||
| } | ||
| return; | ||
| } | ||
| if (!EMAIL_RE.test(email)) { | ||
| errors.push(`Row ${index + 2}: invalid email "${cells[col.email]}".`); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Map the recognized e-mail header.
Line 78 treats e-mail as a header. Line 82 searches only for email. The email column becomes -1, so every data row reports a missing email.
Use one shared lookup for both accepted spellings. Add a regression test for e-mail,name,roll.
Proposed fix
- email: looksLikeHeader ? headerRow.indexOf('email') : 0,
+ email: looksLikeHeader
+ ? headerRow.findIndex(cell => cell === 'email' || cell === 'e-mail')
+ : 0,📝 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.
| const looksLikeHeader = headerRow.includes('email') || headerRow.includes('e-mail'); | |
| const dataRows = looksLikeHeader ? rows.slice(1) : rows; | |
| const col = { | |
| email: looksLikeHeader ? headerRow.indexOf('email') : 0, | |
| name: looksLikeHeader ? headerRow.findIndex(c => c === 'name' || c === 'full name') : 1, | |
| roll: looksLikeHeader ? headerRow.findIndex(c => c.includes('roll')) : 2, | |
| }; | |
| const existing = new Set(existingEmails.map(e => e.trim().toLowerCase())); | |
| const EMAIL_RE = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | |
| let skipped = 0; | |
| dataRows.forEach((cells, index) => { | |
| const email = String(cells[col.email] ?? '') | |
| .trim() | |
| .toLowerCase(); | |
| const name = String(cells[col.name] ?? '').trim() || undefined; | |
| const roll = String(cells[col.roll] ?? '').trim() || undefined; | |
| if (!email) { | |
| if (cells.length > 1 || cells[0].trim() !== '') { | |
| errors.push(`Row ${index + 2}: missing email.`); | |
| } | |
| return; | |
| } | |
| if (!EMAIL_RE.test(email)) { | |
| errors.push(`Row ${index + 2}: invalid email "${cells[col.email]}".`); | |
| const looksLikeHeader = headerRow.includes('email') || headerRow.includes('e-mail'); | |
| const dataRows = looksLikeHeader ? rows.slice(1) : rows; | |
| const col = { | |
| email: looksLikeHeader | |
| ? headerRow.findIndex(cell => cell === 'email' || cell === 'e-mail') | |
| : 0, | |
| name: looksLikeHeader ? headerRow.findIndex(c => c === 'name' || c === 'full name') : 1, | |
| roll: looksLikeHeader ? headerRow.findIndex(c => c.includes('roll')) : 2, | |
| }; | |
| const existing = new Set(existingEmails.map(e => e.trim().toLowerCase())); | |
| const EMAIL_RE = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | |
| let skipped = 0; | |
| dataRows.forEach((cells, index) => { | |
| const email = String(cells[col.email] ?? '') | |
| .trim() | |
| .toLowerCase(); | |
| const name = String(cells[col.name] ?? '').trim() || undefined; | |
| const roll = String(cells[col.roll] ?? '').trim() || undefined; | |
| if (!email) { | |
| if (cells.length > 1 || cells[0].trim() !== '') { | |
| errors.push(`Row ${index + 2}: missing email.`); | |
| } | |
| return; | |
| } | |
| if (!EMAIL_RE.test(email)) { | |
| errors.push(`Row ${index + 2}: invalid email "${cells[col.email]}".`); |
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 88-88: Simplify this regular expression to reduce its runtime, as it has super-linear performance due to backtracking.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/lib/csvImport.js` around lines 78 - 105, Update the column mapping
near the header detection so the email index lookup accepts both “email” and
“e-mail” using one shared lookup, ensuring e-mail,name,roll maps the email
column correctly. Add a regression test covering that header spelling and its
imported data row.
| /** Deterministic participant document id from an email (stable, unique). */ | ||
| export const participantIdForEmail = email => { | ||
| const input = String(email).trim().toLowerCase(); | ||
| let hash = 5381; | ||
| for (let i = 0; i < input.length; i += 1) { | ||
| hash = ((hash << 5) + hash + input.charCodeAt(i)) >>> 0; | ||
| } | ||
| return ('00000000000000000000' + hash.toString(16)).slice(-20); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
# Read the target function and surrounding context
cat -n app/src/lib/csvImport.js | sed -n '115,135p'Repository: roshankumar0036singh/Uni-Event
Length of output: 712
🏁 Script executed:
# Search for PR objective or requirements mentioning FNV-1a
rg -i "fnv|hash" --type-list | head -20
fd --type f -name "*.md" -o -name "*.txt" -o -name "*.js" | xargs rg -i "fnv.*1a|fnv-1a" 2>/dev/null | head -20Repository: roshankumar0036singh/Uni-Event
Length of output: 50386
🏁 Script executed:
# Check the parseAttendeeRows and header mapping logic mentioned in scratchpad
cat -n app/src/lib/csvImport.js | sed -n '70,90p'Repository: roshankumar0036singh/Uni-Event
Length of output: 1183
🏁 Script executed:
# Look for PR description, commits, or spec files
git log --oneline -20
ls -la | grep -i "pr\|spec\|readme\|doc"
fd --type f \( -name "*.md" -o -name "README*" -o -name "SPEC*" \)Repository: roshankumar0036singh/Uni-Event
Length of output: 1904
🏁 Script executed:
# Verify hash algorithm - analyze the bit operations
python3 - <<'PY'
# Analyze the hash function
# hash = ((hash << 5) + hash + input.charCodeAt(i)) >>> 0
# This is checking if it's DJB2 or FNV-1a
# DJB2: hash = ((hash << 5) + hash) + char = hash * 33 + char
# FNV-1a: hash = (hash ^ char) * 16777619, with 32-bit prime
# The code shows: ((hash << 5) + hash + ...) >>> 0
# (hash << 5) = hash * 32
# (hash << 5) + hash = hash * 33
# This is DJB2 style, not FNV-1a
# Check entropy: 20 character hexadecimal string
# Max value before .toString(16): 2^32 - 1 = 4294967295
# In hex: 0xFFFFFFFF = "ffffffff" (8 chars)
# Padded to 20 chars: 12 zeros + 8 hex = "00000000000000000000" effectively max "0000000000000ffffffff"
max_32bit = (1 << 32) - 1
hex_str = format(max_32bit, 'x')
print(f"Max 32-bit value in hex: {hex_str}")
print(f"Length: {len(hex_str)} chars")
print(f"Padded to 20: {'00000000000000000000' + hex_str}".slice(-20))
# Collision probability approximation (birthday problem)
import math
# For 2^32 values, expected collisions with N items
# by birthday paradox: sqrt(2^32 * ln(2)) ≈ sqrt(2.96e9) ≈ 54k items before 50% collision
collision_threshold = math.sqrt((1 << 32) * math.log(2))
print(f"\nBirthday paradox collision threshold (~50%): {collision_threshold:.0f} items")
PYRepository: roshankumar0036singh/Uni-Event
Length of output: 346
🏁 Script executed:
# Get PR details and search for FNV-1a mention
git log --format="%B" d0757d5 | head -50Repository: roshankumar0036singh/Uni-Event
Length of output: 2245
🏁 Script executed:
# Verify the hash algorithm characteristics more carefully
python3 - <<'PY'
# DJB2 vs FNV-1a analysis
import math
# The code uses: ((hash << 5) + hash + char) >>> 0
# This is DJB2: hash = hash * 33 + char (signed 32-bit, then unsigned right shift)
# For a 32-bit hash:
max_hash = (1 << 32) - 1 # 2^32 - 1 = 4294967295
hex_max = format(max_hash, 'x') # "ffffffff" = 8 hex chars
# When padded to 20 chars: "00000000000000000000" + hash.toString(16)
# For max value: "00000000000000000000" + "ffffffff"
# Then .slice(-20) takes last 20 chars = "00000000000000ffffffff" (but max 20 is this)
# So meaningful entropy is only 32 bits, not 128 bits as recommended
print("=== Hash Algorithm Analysis ===")
print(f"Algorithm: DJB2 (from ((hash << 5) + hash + char) >>> 0)")
print(f"Hash size: 32-bit unsigned")
print(f"Max value: {max_hash} = 0x{hex_max}")
print(f"Hex length: {len(hex_max)} characters")
print(f"Output format: 20-char hex string (padded)")
print()
# Collision probability
print("=== Collision Risk (Birthday Paradox) ===")
# For N possible values, expect 50% collision at sqrt(N * ln(2)) items
n = 1 << 32 # 2^32 = 4,294,967,296 possible values
collision_50pct = math.sqrt(n * math.log(2))
collision_1pct = math.sqrt(n * math.log(100))
print(f"Total possible hash values: {n:,}")
print(f"Expected 50% collision risk at: ~{collision_50pct:,.0f} unique emails")
print(f"Expected 1% collision risk at: ~{collision_1pct:,.0f} unique emails")
print()
# For typical use cases
print("=== Risk Assessment ===")
print("For 1,000 users: negligible collision risk (<0.001%)")
print("For 10,000 users: minimal collision risk (~0.01%)")
print("For 100,000 users: moderate collision risk (~1%)")
print("For 1,000,000 users: very high collision risk (>99%)")
PYRepository: roshankumar0036singh/Uni-Event
Length of output: 810
🏁 Script executed:
# Check the specific line where email column is resolved
sed -n '77,85p' app/src/lib/csvImport.jsRepository: roshankumar0036singh/Uni-Event
Length of output: 658
Replace DJB2 hash with FNV-1a as specified in the PR objective, and fix the email column header mapping.
The PR objective specifies "FNV-1a hex" for stable participant document IDs. The implementation uses DJB2 instead (line 125: hash = ((hash << 5) + hash + input.charCodeAt(i)) >>> 0). A 32-bit DJB2 hash has 50% collision risk at ~54,562 unique emails and 1% risk at ~140,638 emails. For bulk imports with 100k+ participants, collisions cause data overwrites in Firestore.
Additionally, the email column header detection (line 78) recognizes both 'email' and 'e-mail' but the column resolution (line 82) only searches for 'email' using indexOf('email'). If a CSV header contains only 'e-mail', the column index resolves to -1, causing email parsing to read the wrong column or fail.
Fix both issues:
- Implement FNV-1a 32-bit hash, or use a cryptographic hash (SHA-256 truncated) for 128+ bit entropy.
- Update line 82 to check both
'email'and'e-mail'when resolving the column index.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 125-125: Prefer String#codePointAt() over String#charCodeAt().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/lib/csvImport.js` around lines 120 - 127, Replace the DJB2
calculation in participantIdForEmail with the specified FNV-1a 32-bit hash (or
an approved cryptographic hash truncated to the required hex length), preserving
normalized email input and deterministic output. Update the email column
resolution logic to search for both “email” and “e-mail”, so headers using
either form produce the correct column index.
| const existingSnapshot = await participantService.fetchParticipantsOnce(db, eventId); | ||
| const existingEmails = (existingSnapshot || []) | ||
| .map(p => p.email) | ||
| .filter(email => typeof email === 'string'); | ||
|
|
||
| const { attendees, errors, skipped } = parseAttendeeRows(rows, existingEmails); | ||
|
|
||
| if (errors.length > 0 && attendees.length === 0) { | ||
| Alert.alert( | ||
| 'Invalid CSV', | ||
| errors.slice(0, 8).join('\n') + | ||
| (errors.length > 8 ? `\n…and ${errors.length - 8} more.` : ''), | ||
| ); | ||
| setImporting(false); | ||
| return; | ||
| } | ||
|
|
||
| const CHUNK = 450; | ||
| for (let i = 0; i < attendees.length; i += CHUNK) { | ||
| const chunk = attendees.slice(i, i + CHUNK); | ||
| await Promise.all( | ||
| chunk.map(attendee => | ||
| setDoc( | ||
| doc( | ||
| db, | ||
| COLLECTIONS.EVENTS, | ||
| eventId, | ||
| 'participants', | ||
| participantIdForEmail(attendee.email), | ||
| ), | ||
| { | ||
| email: attendee.email, | ||
| name: attendee.name || '', | ||
| rollNumber: attendee.rollNumber || '', | ||
| attended: false, | ||
| checkInMethod: 'bulk-import', | ||
| importedAt: new Date().toISOString(), | ||
| }, | ||
| ), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect participant write patterns and Firestore rules before selecting
# a create-if-absent implementation.
rg -n -C 5 '\b(setDoc|runTransaction|writeBatch)\s*\(' app/src
fd -HI -t f '(firestore\.rules|firebase\.json)' . -x sh -c '
echo "=== $1 ==="
sed -n "1,260p" "$1"
' shRepository: roshankumar0036singh/Uni-Event
Length of output: 36281
🏁 Script executed:
#!/bin/bash
set -eu
echo '=== AttendanceDashboard.js candidates ==='
ast-grep outline app/src/screens/AttendanceDashboard.js 2>/dev/null || true
echo '=== Relevant import handler ==='
sed -n '1,230p' app/src/screens/AttendanceDashboard.js
echo '=== Participant and check-in implementations ==='
sed -n '160,235p' app/src/lib/checkInService.js
rg -n -C 8 "participants|participantIdForEmail|checkInMethod|attended" firestore.rules app/src testsRepository: roshankumar0036singh/Uni-Event
Length of output: 50387
🏁 Script executed:
#!/bin/bash
set -eu
echo '=== Participant document rules ==='
sed -n '300,365p' firestore.rules
echo '=== CSV identifier implementation and usages ==='
rg -n -C 12 "participantIdForEmail|parseAttendeeRows|participants.*set|set.*participants" app/src tests
echo '=== Participant service and RSVP write paths ==='
ast-grep outline app/src/lib/participantService.js 2>/dev/null || true
sed -n '1,240p' app/src/lib/participantService.js
sed -n '450,565p' app/src/screens/EventDetail.jsRepository: roshankumar0036singh/Uni-Event
Length of output: 35511
🏁 Script executed:
#!/bin/bash
set -eu
echo '=== AttendanceDashboard authorization context ==='
rg -n -C 6 "AttendanceDashboard|isEventOwner|event.ownerId|user\.uid|navigation.*Attendance" app/src
echo '=== All participant writes with document-id construction ==='
rg -n -C 10 "doc\(.*participants|collection\(.*participants|transaction\.(set|update|delete).*participant|setDoc\(.*participant" app/src --glob '*.js'
echo '=== Deterministic ID probe ==='
node - <<'JS'
function participantIdForEmail(email) {
const input = String(email).trim().toLowerCase();
let hash = 5381;
for (let i = 0; i < input.length; i += 1) {
hash = ((hash << 5) + hash + input.charCodeAt(i)) >>> 0;
}
return ('00000000000000000000' + hash.toString(16)).slice(-20);
}
for (const email of ['alice@example.com', 'student@example.com']) {
console.log(JSON.stringify({ email, participantId: participantIdForEmail(email) }));
}
JSRepository: roshankumar0036singh/Uni-Event
Length of output: 50386
Allow bulk-import writes for event owners.
participantIdForEmail() produces IDs that do not match the organizer’s request.auth.uid. The allow create rule therefore rejects the setDoc calls from AttendanceDashboard, so normal imports fail with a permission error.
Allow the event owner to create participant documents, or route imports through a trusted backend.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/screens/AttendanceDashboard.js` around lines 134 - 174, The
bulk-import writes in the AttendanceDashboard import flow are rejected because
participantIdForEmail() does not match the organizer UID. Update the
participant-document create authorization to allow the event owner for the
relevant event, or route these writes through a trusted backend, while
preserving existing participant access restrictions.
1 participant
Closes #559
Problem
Organizers could export attendance but had no way to bulk-register attendees (club events, guest lists, imported rosters) — only one-by-one RSVP.
Changes
parseCsv: quoted fields, embedded commas, escaped quotes, CRLFparseAttendeeRows: required valid email (invalid/missing reported per-row), optional name/rollNumber columns, header-aware (email/name/roll variants) or headerless email-first, dedupes against existing registrations, lowercases emailsparticipantIdForEmail: deterministic 20-char FNV-1a hex id (stable doc ids;cryptois not available in RN)events/{eventId}/participants/{hash}withattended: falseandcheckInMethod: 'bulk-import', so imported attendees show up instantly in attendance/check-in.Summary by CodeRabbit