mirror of
https://github.com/suitenumerique/meet.git
synced 2026-07-26 11:58:53 +00:00
🩹(backend) fix case-insensitive email deduplication in merge command
The management command that merges users with duplicate emails was comparing emails in a case-insensitive manner, which left some duplicates in the database when their emails only differed by case.
This commit is contained in:
committed by
aleb_the_flash
parent
dca24a1b25
commit
a98dc1484a
@@ -12,6 +12,7 @@ and this project adheres to
|
|||||||
|
|
||||||
- 🚀(front) fix frontend build failure
|
- 🚀(front) fix frontend build failure
|
||||||
- 🐛(makefile) fix args in make test
|
- 🐛(makefile) fix args in make test
|
||||||
|
- 🩹(backend) fix case-insensitive email deduplication in merge command
|
||||||
|
|
||||||
## [1.22.0] - 2026-07-03
|
## [1.22.0] - 2026-07-03
|
||||||
|
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ from django.contrib.auth import get_user_model
|
|||||||
from django.core.management.base import BaseCommand, CommandError
|
from django.core.management.base import BaseCommand, CommandError
|
||||||
from django.db import transaction
|
from django.db import transaction
|
||||||
from django.db.models import Count
|
from django.db.models import Count
|
||||||
|
from django.db.models.functions import Lower
|
||||||
|
|
||||||
from core.models import File, RecordingAccess, ResourceAccess, RoleChoices
|
from core.models import File, RecordingAccess, ResourceAccess, RoleChoices
|
||||||
|
|
||||||
@@ -20,11 +21,14 @@ ROLE_PRIORITY = {
|
|||||||
|
|
||||||
class Command(BaseCommand):
|
class Command(BaseCommand):
|
||||||
"""
|
"""
|
||||||
Merge duplicate users sharing the same email into the most recently created one.
|
Merge duplicate users sharing the same email (case-insensitive) into the
|
||||||
|
most recently created one.
|
||||||
|
|
||||||
The KEPT user is the most recently created. All room memberships, recording
|
Emails are compared case-insensitively, so 'John@Example.com' and
|
||||||
accesses and files are transferred to it. When a conflict exists, the
|
'john@example.com' are treated as duplicates. The KEPT user is the most
|
||||||
higher-privilege role wins. Stale users are then deleted.
|
recently created. All room memberships, recording accesses and files are
|
||||||
|
transferred to it. When a conflict exists, the higher-privilege role wins.
|
||||||
|
Stale users are then deleted.
|
||||||
Each email group is processed inside a single database transaction.
|
Each email group is processed inside a single database transaction.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
@@ -56,13 +60,16 @@ class Command(BaseCommand):
|
|||||||
users_qs = users_qs.filter(email__icontains=email_filter)
|
users_qs = users_qs.filter(email__icontains=email_filter)
|
||||||
self.stdout.write(f"[INFO] Filtering emails containing '{email_filter}'.\n")
|
self.stdout.write(f"[INFO] Filtering emails containing '{email_filter}'.\n")
|
||||||
|
|
||||||
|
# Group emails case-insensitively so 'John@X.com' and 'john@x.com'
|
||||||
|
# are detected as duplicates of each other.
|
||||||
duplicate_emails = (
|
duplicate_emails = (
|
||||||
users_qs.exclude(email__isnull=True)
|
users_qs.exclude(email__isnull=True)
|
||||||
.exclude(email="")
|
.exclude(email="")
|
||||||
.values("email")
|
.annotate(email_lower=Lower("email"))
|
||||||
|
.values("email_lower")
|
||||||
.annotate(cnt=Count("id"))
|
.annotate(cnt=Count("id"))
|
||||||
.filter(cnt__gt=1)
|
.filter(cnt__gt=1)
|
||||||
.values_list("email", flat=True)
|
.values_list("email_lower", flat=True)
|
||||||
)
|
)
|
||||||
|
|
||||||
if not duplicate_emails:
|
if not duplicate_emails:
|
||||||
@@ -78,9 +85,12 @@ class Command(BaseCommand):
|
|||||||
failed_emails = []
|
failed_emails = []
|
||||||
|
|
||||||
for email in duplicate_emails:
|
for email in duplicate_emails:
|
||||||
|
# Case-insensitive lookup to fetch every casing variant of the email.
|
||||||
# Secondary sort by id ensures a stable, deterministic order when
|
# Secondary sort by id ensures a stable, deterministic order when
|
||||||
# created_at timestamps are equal (common in tests and bulk imports).
|
# created_at timestamps are equal (common in tests and bulk imports).
|
||||||
users = list(User.objects.filter(email=email).order_by("created_at", "id"))
|
users = list(
|
||||||
|
User.objects.filter(email__iexact=email).order_by("created_at", "id")
|
||||||
|
)
|
||||||
kept_user = users[-1]
|
kept_user = users[-1]
|
||||||
stale_users = users[:-1]
|
stale_users = users[:-1]
|
||||||
|
|
||||||
@@ -120,6 +130,10 @@ class Command(BaseCommand):
|
|||||||
failed_emails.append(email)
|
failed_emails.append(email)
|
||||||
self.stderr.write(f"[ERROR] Failed to merge '{email}': {exc}")
|
self.stderr.write(f"[ERROR] Failed to merge '{email}': {exc}")
|
||||||
|
|
||||||
|
if not kept_user.email.islower():
|
||||||
|
kept_user.email = kept_user.email.lower()
|
||||||
|
kept_user.save(update_fields=["email"])
|
||||||
|
|
||||||
if failed_emails:
|
if failed_emails:
|
||||||
raise CommandError(
|
raise CommandError(
|
||||||
f"Failed to merge {len(failed_emails)} email group(s): {', '.join(failed_emails)}"
|
f"Failed to merge {len(failed_emails)} email group(s): {', '.join(failed_emails)}"
|
||||||
|
|||||||
@@ -36,6 +36,26 @@ def test_merge_keeps_most_recently_created_user():
|
|||||||
assert User.objects.filter(id=user2.id).exists()
|
assert User.objects.filter(id=user2.id).exists()
|
||||||
|
|
||||||
|
|
||||||
|
def test_merge_user_case_insensitive():
|
||||||
|
"""Emails differing only by case should be treated as duplicates and merged,
|
||||||
|
keeping the most recently created user."""
|
||||||
|
user1 = UserFactory(email="Dup@example.com")
|
||||||
|
user2 = UserFactory(email="dup@example.com")
|
||||||
|
call_command("merge_duplicate_users")
|
||||||
|
|
||||||
|
assert not User.objects.filter(id=user1.id).exists()
|
||||||
|
assert User.objects.filter(id=user2.id).exists()
|
||||||
|
|
||||||
|
user3 = UserFactory(email="joe@example.com")
|
||||||
|
user4 = UserFactory(email="Joe@example.com")
|
||||||
|
call_command("merge_duplicate_users")
|
||||||
|
assert not User.objects.filter(id=user3.id).exists()
|
||||||
|
assert User.objects.filter(id=user4.id).exists()
|
||||||
|
|
||||||
|
user4.refresh_from_db()
|
||||||
|
assert user4.email.islower()
|
||||||
|
|
||||||
|
|
||||||
def test_merge_deletes_all_stale_users():
|
def test_merge_deletes_all_stale_users():
|
||||||
"""Command should delete all stale users and keep only the most recently created one."""
|
"""Command should delete all stale users and keep only the most recently created one."""
|
||||||
email = "many@example.com"
|
email = "many@example.com"
|
||||||
@@ -457,4 +477,4 @@ def test_merge_email_filter_is_case_insensitive():
|
|||||||
UserFactory(email="user1@Example.com")
|
UserFactory(email="user1@Example.com")
|
||||||
UserFactory(email="user1@Example.com")
|
UserFactory(email="user1@Example.com")
|
||||||
call_command("merge_duplicate_users", email_filter="@example.com")
|
call_command("merge_duplicate_users", email_filter="@example.com")
|
||||||
assert User.objects.filter(email="user1@Example.com").count() == 1
|
assert User.objects.filter(email="user1@example.com").count() == 1
|
||||||
|
|||||||
Reference in New Issue
Block a user