Fix: Make candidate account creation case insensitive and check for d… #1
Conversation
…uplicate emails This addresses issue compserv#500 by: 1. Making username comparison case-insensitive during candidate account provisioning 2. Adding duplicate email validation to prevent accounts with existing emails Fixes compserv#500
WalkthroughThe provisioning process in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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}
|
|
||
| # Get existing emails | ||
| emails = [] | ||
| for row in rows: | ||
| email = row["Berkeley email"] | ||
| if email: | ||
| emails.append(email) | ||
|
|
||
| existing_emails = set( | ||
| User.objects.filter(email__in=emails).values_list( | ||
| "email", flat=True | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Make email comparison case-insensitive.
Email addresses should be treated as case-insensitive to prevent duplicate accounts with emails that differ only in case (e.g., "[email protected]" vs "[email protected]").
Apply this diff to handle emails case-insensitively:
+ from django.db.models.functions import Lower
+
# Get existing emails
emails = []
for row in rows:
email = row["Berkeley email"]
if email:
emails.append(email)
- existing_emails = set(
- User.objects.filter(email__in=emails).values_list(
- "email", flat=True
+ existing_emails = set(
+ User.objects.annotate(email_lower=Lower('email'))
+ .filter(email_lower__in=[e.lower() for e in emails])
+ .values_list('email_lower', flat=True)
- )
)Then update the skip condition on line 257 to compare lowercased emails:
- if (row["username"] is None) or (row["username"].lower() in existing_usernames) or (row["Berkeley email"] in existing_emails):
+ if (row["username"] is None) or (row["username"].lower() in existing_usernames) or (row["Berkeley email"].lower() in existing_emails):Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In hknweb/forms.py around lines 228-240, the code collects emails without
normalizing case so lookups can miss duplicates; change it to normalize emails
to lowercase before querying and use a case-insensitive query against the DB
(e.g., annotate the User queryset with Lower('email') and filter on that
lowercase field using the list of lowercased emails), then build existing_emails
as a set of lowercased addresses; finally update the skip condition at line 257
to compare row_email.lower() against existing_emails.
| if (row["username"] is None) or (row["username"].lower() in existing_usernames) or (row["Berkeley email"] in existing_emails): | ||
| continue |
There was a problem hiding this comment.
Fix indentation error and broken case-insensitive username check.
This code segment has two critical issues:
-
Indentation Error: Line 257 has excessive leading whitespace that will cause a Python
IndentationError. -
Broken Case-Insensitive Check: The username case-insensitive comparison is incorrect.
existing_usernames(lines 223-227) contains usernames with their original casing from the database (e.g., "Alice"), but line 257 compares the lowercased username (e.g., "alice"). This means"alice" in {"Alice"}returnsFalse, allowing duplicate accounts to be created.
Apply this diff to fix both issues. First, update the existing_usernames query (around lines 223-227) to store lowercased usernames:
+ from django.db.models.functions import Lower
+
- existing_usernames = set(
- User.objects.filter(username__in=usernames).values_list(
- "username", flat=True
+ existing_usernames = set(
+ User.objects.annotate(username_lower=Lower('username'))
+ .filter(username_lower__in=[u.lower() for u in usernames])
+ .values_list('username_lower', flat=True)
- )
)Then fix the indentation and condition on lines 257-258:
- if (row["username"] is None) or (row["username"].lower() in existing_usernames) or (row["Berkeley email"] in existing_emails):
- continue
+ if (row["username"] is None) or (row["username"].lower() in existing_usernames) or (row["Berkeley email"] in existing_emails):
+ continue🧰 Tools
🪛 Ruff (0.14.5)
258-258: unindent does not match any outer indentation level
(invalid-syntax)
🤖 Prompt for AI Agents
In hknweb/forms.py around lines 223-227 and 257-258, the code builds
existing_usernames with original casing and has a misindented condition that
compares a lowercased input username against those original-cased names (plus
extra leading whitespace causing an IndentationError). Update the query that
builds existing_usernames to store usernames lowercased (e.g., .lower() when
collecting from the DB) and then remove the extra leading whitespace on line 257
and change the condition to compare the input username lowercased against the
lowercased existing_usernames set (i.e., if row["username"] is None or
row["username"].lower() in existing_usernames or row["Berkeley email"] in
existing_emails: continue).
…uplicate emails
This addresses issue compserv#500 by:
Fixes compserv#500
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.