diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d801a02..f8134581 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,10 @@ and this project adheres to ## [Unreleased] -## Added +### Added - 🔧(backend) backport logging configuration from docs +- 🧑‍💻(backend) add management command to merge duplicate users ### Fixed diff --git a/src/backend/core/management/__init__.py b/src/backend/core/management/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/backend/core/management/commands/__init__.py b/src/backend/core/management/commands/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/backend/core/management/commands/merge_duplicate_users.py b/src/backend/core/management/commands/merge_duplicate_users.py new file mode 100644 index 00000000..cc7750d4 --- /dev/null +++ b/src/backend/core/management/commands/merge_duplicate_users.py @@ -0,0 +1,157 @@ +"""Management command to merge duplicate users based on their email address.""" + +# pylint: disable=too-many-locals + +from django.contrib.auth import get_user_model +from django.core.management.base import BaseCommand, CommandError +from django.db import transaction +from django.db.models import Count + +from core.models import File, RecordingAccess, ResourceAccess, RoleChoices + +User = get_user_model() + +ROLE_PRIORITY = { + RoleChoices.OWNER: 3, + RoleChoices.ADMIN: 2, + RoleChoices.MEMBER: 1, +} + + +class Command(BaseCommand): + """ + Merge duplicate users sharing the same email into the most recently created one. + + The KEPT user is the most 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. + """ + + help = __doc__ + + def add_arguments(self, parser): + parser.add_argument( + "--dry-run", + action="store_true", + help="Simulate the merge without writing any changes to the database.", + ) + + def handle(self, *args, **options): + """Execute the management command.""" + dry_run = options["dry_run"] + + if dry_run: + self.stdout.write("[DRY-RUN] No changes will be written.\n") + + duplicate_emails = ( + User.objects.all() + .exclude(email__isnull=True) + .exclude(email="") + .values("email") + .annotate(cnt=Count("id")) + .filter(cnt__gt=1) + .values_list("email", flat=True) + ) + + if not duplicate_emails: + self.stdout.write("[INFO] No duplicate users found. Nothing to do.") + return + + self.stdout.write( + f"[INFO] Found {len(duplicate_emails)} email(s) with duplicate users." + ) + + total_merged = 0 + total_deleted = 0 + failed_emails = [] + + for email in duplicate_emails: + # Secondary sort by id ensures a stable, deterministic order when + # created_at timestamps are equal (common in tests and bulk imports). + users = list(User.objects.filter(email=email).order_by("created_at", "id")) + kept_user = users[-1] + stale_users = users[:-1] + + self.stdout.write( + f"\n[INFO] Email '{email}': {len(users)} users — " + f"keeping {kept_user.id} (created {kept_user.created_at.date()})." + ) + for u in stale_users: + self.stdout.write( + f" stale: {u.id} (created {u.created_at.date()})" + ) + + if dry_run: + ra_count = ResourceAccess.objects.filter(user__in=stale_users).count() + rca_count = RecordingAccess.objects.filter(user__in=stale_users).count() + f_count = File.objects.filter(creator__in=stale_users).count() + self.stdout.write( + f" [DRY-RUN] Would migrate: {ra_count} ResourceAccess, " + f"{rca_count} RecordingAccess, {f_count} File(s)." + ) + continue + + try: + group_deleted = 0 + with transaction.atomic(): + for stale_user in stale_users: + self._merge_resource_accesses(stale_user, kept_user) + self._merge_recording_accesses(stale_user, kept_user) + self._merge_files(stale_user, kept_user) + stale_user.delete() + group_deleted += 1 + + total_deleted += group_deleted + total_merged += 1 + + except Exception as exc: # noqa: BLE001 #pylint: disable=broad-exception-caught + failed_emails.append(email) + self.stderr.write(f"[ERROR] Failed to merge '{email}': {exc}") + + if failed_emails: + raise CommandError( + f"Failed to merge {len(failed_emails)} email group(s): {', '.join(failed_emails)}" + ) + + self.stdout.write( + self.style.SUCCESS( + f"\n[DONE] Merged {total_merged} group(s), deleted {total_deleted} user(s)." + ) + ) + + def _merge_resource_accesses(self, stale_user, kept_user): + """Transfer room memberships from stale_user to kept_user.""" + for ra in ResourceAccess.objects.filter(user=stale_user): + existing = ResourceAccess.objects.filter( + user=kept_user, resource=ra.resource + ).first() + + if existing is None: + ra.user = kept_user + ra.save(update_fields=["user"]) + else: + if ROLE_PRIORITY.get(ra.role, 0) > ROLE_PRIORITY.get(existing.role, 0): + existing.role = ra.role + existing.save(update_fields=["role"]) + ra.delete() + + def _merge_recording_accesses(self, stale_user, kept_user): + """Transfer recording accesses from stale_user to kept_user.""" + for rca in RecordingAccess.objects.filter(user=stale_user): + existing = RecordingAccess.objects.filter( + user=kept_user, recording=rca.recording + ).first() + + if existing is None: + rca.user = kept_user + rca.save(update_fields=["user"]) + else: + if ROLE_PRIORITY.get(rca.role, 0) > ROLE_PRIORITY.get(existing.role, 0): + existing.role = rca.role + existing.save(update_fields=["role"]) + rca.delete() + + def _merge_files(self, stale_user, kept_user): + """Re-assign files created by stale_user to kept_user.""" + File.objects.filter(creator=stale_user).update(creator=kept_user) diff --git a/src/backend/core/tests/management/__init__.py b/src/backend/core/tests/management/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/backend/core/tests/management/test_management_merge_duplicate_users.py b/src/backend/core/tests/management/test_management_merge_duplicate_users.py new file mode 100644 index 00000000..fe1d6a6d --- /dev/null +++ b/src/backend/core/tests/management/test_management_merge_duplicate_users.py @@ -0,0 +1,429 @@ +"""Tests for the merge_duplicate_users management command.""" + +from unittest import mock + +from django.core.management import base, call_command + +import pytest + +from core.factories import ( + FileFactory, + UserFactory, + UserRecordingAccessFactory, + UserResourceAccessFactory, +) +from core.models import RecordingAccess, ResourceAccess, RoleChoices, User + +pytestmark = pytest.mark.django_db + +# pylint: disable=W0613 + + +def test_merge_no_duplicates_does_nothing(): + """Command should do nothing when no duplicate users exist.""" + user = UserFactory(email="unique@example.com") + call_command("merge_duplicate_users") + assert User.objects.count() == 1 + assert User.objects.filter(id=user.id).exists() + + +def test_merge_keeps_most_recently_created_user(): + """Command should keep the most recently created user when duplicates exist.""" + 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() + + +def test_merge_deletes_all_stale_users(): + """Command should delete all stale users and keep only the most recently created one.""" + email = "many@example.com" + UserFactory(email=email) + UserFactory(email=email) + user_kept = UserFactory(email=email) + call_command("merge_duplicate_users") + assert User.objects.filter(email=email).count() == 1 + assert User.objects.filter(id=user_kept.id).exists() + + +# ── ResourceAccess ───────────────────────────────────────────────────────────── + + +def test_merge_transfers_resource_access_to_kept_user(): + """ResourceAccess should be transferred to the kept user when stale user is merged.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + ra = UserResourceAccessFactory(user=user1) + call_command("merge_duplicate_users") + ra.refresh_from_db() + assert ra.user == user2 + + +def test_merge_transfers_multiple_room_accesses(): + """All ResourceAccesses should be transferred to the kept user when stale user is merged.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + accesses = UserResourceAccessFactory.create_batch(3, user=user1) + call_command("merge_duplicate_users") + assert not ResourceAccess.objects.filter(user=user1).exists() + for ra in accesses: + assert ResourceAccess.objects.filter(user=user2, resource=ra.resource).exists() + + +def test_merge_all_resource_accesses_owned_by_kept_user_nothing_changes(): + """ResourceAccesses should remain unchanged when all are already owned by the kept user.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + accesses = UserResourceAccessFactory.create_batch(3, user=user2) + call_command("merge_duplicate_users") + assert not ResourceAccess.objects.filter(user=user1).exists() + for ra in accesses: + ra.refresh_from_db() + assert ra.user == user2 + + +def test_merge_resource_access_conflict_upgrades_to_owner(): + """ResourceAccess role should be upgraded to owner when stale user has a higher role.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + ra1 = UserResourceAccessFactory(user=user1, role=RoleChoices.OWNER) + ra2 = UserResourceAccessFactory( + user=user2, resource=ra1.resource, role=RoleChoices.MEMBER + ) + other_accesses = UserResourceAccessFactory.create_batch( + 3, user=user1, role=RoleChoices.MEMBER + ) + call_command("merge_duplicate_users") + ra2.refresh_from_db() + assert ra2.role == RoleChoices.OWNER + assert not ResourceAccess.objects.filter(user=user1).exists() + for ra in other_accesses: + assert ResourceAccess.objects.filter( + user=user2, resource=ra.resource, role=RoleChoices.MEMBER + ).exists() + + +def test_merge_resource_access_conflict_upgrades_to_admin(): + """ResourceAccess role should be upgraded to admin when stale user has a higher role.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + ra1 = UserResourceAccessFactory(user=user1, role=RoleChoices.ADMIN) + ra2 = UserResourceAccessFactory( + user=user2, resource=ra1.resource, role=RoleChoices.MEMBER + ) + other_accesses = UserResourceAccessFactory.create_batch( + 3, user=user1, role=RoleChoices.MEMBER + ) + call_command("merge_duplicate_users") + ra2.refresh_from_db() + assert ra2.role == RoleChoices.ADMIN + assert not ResourceAccess.objects.filter(user=user1).exists() + for ra in other_accesses: + assert ResourceAccess.objects.filter( + user=user2, resource=ra.resource, role=RoleChoices.MEMBER + ).exists() + + +def test_merge_resource_access_conflict_does_not_downgrade_role(): + """ResourceAccess role should not be downgraded when stale user has a lower role.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + ra1 = UserResourceAccessFactory(user=user1, role=RoleChoices.MEMBER) + ra2 = UserResourceAccessFactory( + user=user2, resource=ra1.resource, role=RoleChoices.OWNER + ) + other_accesses = UserResourceAccessFactory.create_batch( + 3, user=user1, role=RoleChoices.MEMBER + ) + call_command("merge_duplicate_users") + ra2.refresh_from_db() + assert ra2.role == RoleChoices.OWNER + assert not ResourceAccess.objects.filter(user=user1).exists() + for ra in other_accesses: + assert ResourceAccess.objects.filter( + user=user2, resource=ra.resource, role=RoleChoices.MEMBER + ).exists() + + +def test_merge_resource_access_conflict_equal_role_keeps_single_access(): + """ResourceAccess should keep one entry for the kept user when both ones have the same role.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + ra1 = UserResourceAccessFactory(user=user1, role=RoleChoices.MEMBER) + UserResourceAccessFactory( + user=user2, resource=ra1.resource, role=RoleChoices.MEMBER + ) + call_command("merge_duplicate_users") + accesses = ResourceAccess.objects.filter(resource=ra1.resource) + assert accesses.count() == 1 + assert accesses.first().user == user2 + assert accesses.first().role == RoleChoices.MEMBER + + +# ── RecordingAccess ──────────────────────────────────────────────────────────── + + +def test_merge_transfers_recording_access_to_kept_user(): + """RecordingAccess should be transferred to the kept user when stale user is merged.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + rca = UserRecordingAccessFactory(user=user1) + call_command("merge_duplicate_users") + rca.refresh_from_db() + assert rca.user == user2 + + +def test_merge_transfers_multiple_recording_accesses(): + """All RecordingAccesses should be transferred to the kept user when stale user is merged.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + accesses = UserRecordingAccessFactory.create_batch(3, user=user1) + call_command("merge_duplicate_users") + assert not RecordingAccess.objects.filter(user=user1).exists() + for rca in accesses: + assert RecordingAccess.objects.filter( + user=user2, recording=rca.recording + ).exists() + + +def test_merge_all_recording_accesses_owned_by_kept_user_nothing_changes(): + """RecordingAccesses should remain unchanged when all are already owned by the kept user.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + accesses = UserRecordingAccessFactory.create_batch(3, user=user2) + call_command("merge_duplicate_users") + assert not RecordingAccess.objects.filter(user=user1).exists() + for rca in accesses: + rca.refresh_from_db() + assert rca.user == user2 + + +def test_merge_recording_access_conflict_upgrades_to_owner(): + """RecordingAccess role should be upgraded to owner when stale user has a higher role.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + rca1 = UserRecordingAccessFactory(user=user1, role=RoleChoices.OWNER) + rca2 = UserRecordingAccessFactory( + user=user2, recording=rca1.recording, role=RoleChoices.MEMBER + ) + other_accesses = UserRecordingAccessFactory.create_batch( + 3, user=user1, role=RoleChoices.MEMBER + ) + call_command("merge_duplicate_users") + rca2.refresh_from_db() + assert rca2.role == RoleChoices.OWNER + assert not RecordingAccess.objects.filter(user=user1).exists() + for rca in other_accesses: + assert RecordingAccess.objects.filter( + user=user2, recording=rca.recording, role=RoleChoices.MEMBER + ).exists() + + +def test_merge_recording_access_conflict_upgrades_to_admin(): + """RecordingAccess role should be upgraded to admin when stale user has a higher role.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + rca1 = UserRecordingAccessFactory(user=user1, role=RoleChoices.ADMIN) + rca2 = UserRecordingAccessFactory( + user=user2, recording=rca1.recording, role=RoleChoices.MEMBER + ) + other_accesses = UserRecordingAccessFactory.create_batch( + 3, user=user1, role=RoleChoices.MEMBER + ) + call_command("merge_duplicate_users") + rca2.refresh_from_db() + assert rca2.role == RoleChoices.ADMIN + assert not RecordingAccess.objects.filter(user=user1).exists() + for rca in other_accesses: + assert RecordingAccess.objects.filter( + user=user2, recording=rca.recording, role=RoleChoices.MEMBER + ).exists() + + +def test_merge_recording_access_conflict_does_not_downgrade_role(): + """RecordingAccess role should not be downgraded when stale user has a lower role.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + rca1 = UserRecordingAccessFactory(user=user1, role=RoleChoices.MEMBER) + rca2 = UserRecordingAccessFactory( + user=user2, recording=rca1.recording, role=RoleChoices.OWNER + ) + other_accesses = UserRecordingAccessFactory.create_batch( + 3, user=user1, role=RoleChoices.MEMBER + ) + call_command("merge_duplicate_users") + rca2.refresh_from_db() + assert rca2.role == RoleChoices.OWNER + assert not RecordingAccess.objects.filter(user=user1).exists() + for rca in other_accesses: + assert RecordingAccess.objects.filter( + user=user2, recording=rca.recording, role=RoleChoices.MEMBER + ).exists() + + +def test_merge_recording_access_conflict_equal_role_keeps_single_access(): + """RecordingAccess should keep one entry for the user when both users have the same role.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + rca1 = UserRecordingAccessFactory(user=user1, role=RoleChoices.MEMBER) + UserRecordingAccessFactory( + user=user2, recording=rca1.recording, role=RoleChoices.MEMBER + ) + call_command("merge_duplicate_users") + accesses = RecordingAccess.objects.filter(recording=rca1.recording) + assert accesses.count() == 1 + assert accesses.first().user == user2 + assert accesses.first().role == RoleChoices.MEMBER + + +# ── Files ────────────────────────────────────────────────────────────────────── + + +def test_merge_reassigns_files_to_kept_user(): + """Files should be reassigned to the kept user when stale user is merged.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + files = FileFactory.create_batch(3, creator=user1) + call_command("merge_duplicate_users") + for f in files: + f.refresh_from_db() + assert f.creator == user2 + + +def test_merge_kept_user_own_files_untouched(): + """Files already owned by the kept user should remain unchanged after merge.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + FileFactory(creator=user1) + kept_file = FileFactory(creator=user2) + call_command("merge_duplicate_users") + kept_file.refresh_from_db() + assert kept_file.creator == user2 + + +# ── Dry-run ──────────────────────────────────────────────────────────────────── + + +def test_merge_dry_run_does_not_delete_users(): + """Command should not delete any users when dry-run is enabled.""" + UserFactory(email="dup@example.com") + UserFactory(email="dup@example.com") + call_command("merge_duplicate_users", dry_run=True) + assert User.objects.filter(email="dup@example.com").count() == 2 + + +def test_merge_dry_run_does_not_move_resource_access(): + """Command should not move resource accesses when dry-run is enabled.""" + user1 = UserFactory(email="dup@example.com") + UserFactory(email="dup@example.com") + ra = UserResourceAccessFactory(user=user1) + call_command("merge_duplicate_users", dry_run=True) + ra.refresh_from_db() + assert ra.user == user1 + + +def test_merge_dry_run_does_not_move_recording_access(): + """Command should not move recording accesses when dry-run is enabled.""" + user1 = UserFactory(email="dup@example.com") + UserFactory(email="dup@example.com") + ra = UserRecordingAccessFactory(user=user1) + call_command("merge_duplicate_users", dry_run=True) + ra.refresh_from_db() + assert ra.user == user1 + + +def test_merge_dry_run_does_not_move_files(): + """Command should not reassign files when dry-run is enabled.""" + user1 = UserFactory(email="dup@example.com") + UserFactory(email="dup@example.com") + f = FileFactory(creator=user1) + call_command("merge_duplicate_users", dry_run=True) + f.refresh_from_db() + assert f.creator == user1 + + +# ── Isolation ────────────────────────────────────────────────────────────────── + + +def test_merge_non_duplicate_users_untouched(): + """Non-duplicate users should remain untouched when other duplicates are merged.""" + unique = UserFactory(email="unique@example.com") + UserFactory(email="dup@example.com") + UserFactory(email="dup@example.com") + call_command("merge_duplicate_users") + assert User.objects.filter(id=unique.id).exists() + + +def test_merge_non_duplicate_resource_access_untouched(): + """ResourceAccess of non-duplicate users should remain untouched.""" + unique = UserFactory(email="unique@example.com") + ra = UserResourceAccessFactory(user=unique) + UserFactory(email="dup@example.com") + UserFactory(email="dup@example.com") + call_command("merge_duplicate_users") + ra.refresh_from_db() + assert ra.user == unique + + +def test_merge_multiple_email_groups_all_merged(): + """Command should merge all duplicate email groups in a single run.""" + for i in range(3): + UserFactory(email=f"group{i}@example.com") + UserFactory(email=f"group{i}@example.com") + call_command("merge_duplicate_users") + for i in range(3): + assert User.objects.filter(email=f"group{i}@example.com").count() == 1 + assert User.objects.count() == 3 + + +# ── NULL / blank email guard ─────────────────────────────────────────────────── + + +def test_merge_does_not_merge_users_with_null_email(): + """Users with NULL email must never be merged together, even if multiple exist.""" + user1 = UserFactory(email=None) + user2 = UserFactory(email=None) + call_command("merge_duplicate_users") + assert User.objects.filter(id=user1.id).exists() + assert User.objects.filter(id=user2.id).exists() + + +def test_merge_does_not_merge_users_with_blank_email(): + """Users with empty-string email must never be merged together, even if multiple exist.""" + user1 = UserFactory(email="") + user2 = UserFactory(email="") + call_command("merge_duplicate_users") + assert User.objects.filter(id=user1.id).exists() + assert User.objects.filter(id=user2.id).exists() + + +# ── Atomicity ────────────────────────────────────────────────────────────────── + + +@mock.patch( + "core.management.commands.merge_duplicate_users.Command._merge_recording_accesses", + side_effect=Exception("forced failure"), +) +def test_merge_is_atomic_rolls_back_all_on_any_failure(mock_reassign_files): + """Merge should be fully rolled back when any step fails.""" + user1 = UserFactory(email="dup@example.com") + user2 = UserFactory(email="dup@example.com") + resource_accesses = UserResourceAccessFactory.create_batch(3, user=user1) + recording_accesses = UserRecordingAccessFactory.create_batch(3, user=user1) + files = FileFactory.create_batch(3, creator=user1) + + with pytest.raises(base.CommandError): + call_command("merge_duplicate_users") + + assert User.objects.filter(id=user1.id).exists() + assert User.objects.filter(id=user2.id).exists() + for ra in resource_accesses: + ra.refresh_from_db() + assert ra.user == user1 + for rca in recording_accesses: + rca.refresh_from_db() + assert rca.user == user1 + for f in files: + f.refresh_from_db() + assert f.creator == user1