Prevent duplicate committees

backend/clubs/management/commands/merge_duplicate_committees.py

@@ -0,0 +1,128 @@
+from django.core.management.base import BaseCommand
+from django.db import models, transaction
+from django.db.models import Count
+
+from clubs.models import ApplicationCommittee
+
+
+class Command(BaseCommand):
+    help = "Merges committees with duplicate names within the same application"
+
+    def add_arguments(self, parser):
+        parser.add_argument(
+            "--dry-run",
+            action="store_true",
+            help="Run without making any changes",
+        )
+        parser.add_argument(
+            "--club",
+            help="Only merge committees for this club code",
+        )
+        parser.set_defaults(dry_run=True, club=None)
+
+    def handle(self, *args, **kwargs):
+        dry_run = kwargs["dry_run"]
+        club_code = kwargs["club"]
+
+        if dry_run:
+            self.stdout.write("Running in dry run mode, no changes will be made.")
+
+        try:
+            with transaction.atomic():
+                # Find committees that have the same name within an application
+                committees_query = ApplicationCommittee.objects.annotate(
+                    normalized_name=models.functions.Lower(
+                        models.functions.Trim("name")
+                    )
+                ).values("normalized_name", "application")
+                if club_code is not None:
+                    committees_query = committees_query.filter(
+                        application__club__code=club_code
+                    )
+
+                duplicate_committees = committees_query.annotate(
+                    count=Count("id")
+                ).filter(count__gt=1)
+
+                total_merged = 0
+                for duplicate in duplicate_committees:
+                    normalized_name = duplicate["normalized_name"]
+                    application_id = duplicate["application"]
+
+                    committees = (
+                        ApplicationCommittee.objects.filter(application=application_id)
+                        .annotate(
+                            normalized_name=models.functions.Lower(
+                                models.functions.Trim("name")
+                            )
+                        )
+                        .filter(normalized_name=normalized_name)
+                        .order_by("id")
+                    )
+
+                    primary_committee = committees.first()
+                    if not primary_committee:
+                        continue
+
+                    self.stdout.write(
+                        f"Processing duplicate committees with name "
+                        f"'{normalized_name}' in application {application_id}"
+                    )
+
+                    # Merge all other committees into the primary one
+                    for committee in committees[1:]:
+                        try:
+                            if not dry_run:
+                                submissions_moved = 0
+                                for submission in committee.submissions.all():
+                                    submission.committee = primary_committee
+                                    submission.save()
+                                    submissions_moved += 1
+
+                                for question in committee.applicationquestion_set.all():
+                                    question.committees.remove(committee)
+                                    question.committees.add(primary_committee)
+
+                                committee.delete()
+
+                                self.stdout.write(
+                                    f"Moved {submissions_moved} submissions and "
+                                    f"updated questions from committee {committee.id} "
+                                    f"to {primary_committee.id}"
+                                )
+                                total_merged += 1
+                            else:
+                                submission_count = committee.submissions.count()
+                                question_count = (
+                                    committee.applicationquestion_set.count()
+                                )
+
+                                self.stdout.write(
+                                    f"Would move {submission_count} "
+                                    f"submissions and update {question_count} "
+                                    f"questions from committee {committee.id} to "
+                                    f"{primary_committee.id}"
+                                )
+
+                        except Exception as e:
+                            self.stderr.write(
+                                self.style.ERROR(
+                                    f"Failed to merge committee {committee.id} into "
+                                    f"{primary_committee.id}: {str(e)}"
+                                )
+                            )
+                            raise
+
+                if dry_run:
+                    return
+
+                self.stdout.write(
+                    self.style.SUCCESS(
+                        f"Successfully merged {total_merged} duplicate committees"
+                    )
+                )
+
+        except Exception as e:
+            self.stderr.write(
+                self.style.ERROR(f"{'(Dry run) ' if dry_run else ''}Error: {str(e)}")
+            )

backend/clubs/migrations/0118_alter_applicationcommittee_unique_together.py

@@ -0,0 +1,16 @@
+# Generated by Django 5.0.4 on 2025-01-29 04:33
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("clubs", "0117_clubapprovalresponsetemplate"),
+    ]
+
+    operations = [
+        migrations.AlterUniqueTogether(
+            name="applicationcommittee", unique_together={("name", "application")},
+        ),
+    ]

backend/clubs/migrations/0119_alter_applicationcommittee_name.py

@@ -0,0 +1,18 @@
+# Generated by Django 5.0.4 on 2025-01-29 18:46
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("clubs", "0118_alter_applicationcommittee_unique_together"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="applicationcommittee",
+            name="name",
+            field=models.CharField(max_length=255),
+        ),
+    ]

backend/clubs/models.py

@@ -1674,7 +1674,7 @@ class ApplicationCommittee(models.Model):
     may have multiple committees to which students can apply.
     """
 
-    name = models.TextField(blank=True)
+    name = models.CharField(max_length=255)
     application = models.ForeignKey(
         ClubApplication,
         related_name="committees",
@@ -1690,6 +1690,9 @@ def get_word_limit(self):
     def __str__(self):
         return "<ApplicationCommittee: {} in {}>".format(self.name, self.application.pk)
 
+    class Meta:
+        unique_together = (("name", "application"),)
+
 
 class ApplicationQuestion(CloneModel):
     """

backend/clubs/serializers.py

@@ -2801,6 +2801,7 @@ def get_image_url(self, obj):
             return image.url
 
     def validate(self, data):
+        # Validate acceptance and rejection emails
         acceptance_template = data.get("acceptance_email", "")
         rejection_template = data.get("rejection_email", "")
 
@@ -2811,68 +2812,73 @@ def validate(self, data):
                 "Your application email templates contain invalid variables!"
             )
 
-        if all(
-            field in data
-            for field in [
-                "application_start_time",
-                "application_end_time",
-                "result_release_time",
-            ]
-        ):
-            application_start_time = data["application_start_time"]
-            application_end_time = data["application_end_time"]
-            result_release_time = data["result_release_time"]
+        # Check that application times make sense
+        application_times = [
+            "application_start_time",
+            "application_end_time",
+            "result_release_time",
+        ]
+        if all(field in data for field in application_times):
+            start = data["application_start_time"]
+            end = data["application_end_time"]
+            release = data["result_release_time"]
 
-            if application_start_time > application_end_time:
+            if start > end:
                 raise serializers.ValidationError(
-                    "Your application start time must be less than the end time!"
+                    "Your application's start time must be less than its end time!"
                 )
 
-            if application_end_time > result_release_time:
+            if end > release:
                 raise serializers.ValidationError(
-                    """Your application end time must be less than
-                    the result release time!"""
+                    "Your application's end time must be less than the result release "
+                    "time!"
                 )
 
         return data
 
+    def validate_committees(self, value):
+        """Validate committee names are unique"""
+        if not value:
+            return []
+
+        normalized_committees = [name.strip().lower() for name in value]
+        if len(set(normalized_committees)) != len(normalized_committees):
+            raise serializers.ValidationError("Committee names must be unique")
+
+        # Strip whitespace from names
+        return [name.strip() for name in value]
+
     def save(self):
         application_obj = super().save()
-        # manually create committee objects as Django does
-        # not support nested serializers out of the box
-        request = self.context["request"].data
-        # only allow modifications to committees if the application is not yet open
-        now = timezone.now()
-        prev_committees = ApplicationCommittee.objects.filter(
+
+        if application_obj.application_start_time < timezone.now():
+            raise serializers.ValidationError(
+                "You cannot edit committees once the application is open"
+            )
+
+        existing_committees = ApplicationCommittee.objects.filter(
             application=application_obj
-        )
-        prev_committee_names = sorted(list(map(lambda x: x.name, prev_committees)))
-        committees = sorted(
-            list(
-                map(
-                    lambda x: x["value"] if "value" in x else x["name"],
-                    request["committees"] if "committees" in request else [],
-                )
+        ).values("name", flat=True)
+        existing_committees = set(existing_committees)
+
+        committees = self.validated_data.get("committees", [])
+        committees = set(committees)
+
+        if existing_committees == committees:
+            return application_obj
+
+        for committee_name in committees:
+            if committee_name in existing_committees:
+                continue
+            ApplicationCommittee.objects.create(
+                name=committee_name, application=application_obj
             )
-        )
-        if prev_committee_names != committees:
-            if application_obj.application_start_time < now:
-                raise serializers.ValidationError(
-                    "You cannot edit committees once the application is open"
-                )
-            # nasty hack for idempotency
-            prev_committee_names = prev_committees.values("name")
-            for prev_committee in prev_committees:
-                if prev_committee.name not in committees:
-                    prev_committee.delete()
-
-            for name in committees:
-                if name not in prev_committee_names:
-                    ApplicationCommittee.objects.create(
-                        name=name,
-                        application=application_obj,
-                    )
-            cache.delete(f"clubapplication:{application_obj.id}")
+
+        ApplicationCommittee.objects.filter(application=application_obj).exclude(
+            name__in=committees
+        ).delete()
+
+        cache.delete(f"clubapplication:{application_obj.id}")
 
         return application_obj
 

backend/tests/clubs/test_views.py

@@ -19,6 +19,7 @@
 from clubs.filters import DEFAULT_PAGE_SIZE
 from clubs.models import (
     Advisor,
+    ApplicationCommittee,
     ApplicationSubmission,
     Asset,
     Badge,
@@ -2937,6 +2938,54 @@ def test_club_approval_response_templates(self):
         )
         self.assertEqual(resp.status_code, 403)
 
+    def test_application_duplicate_committees(self):
+        """
+        Test that duplicate committees are rejected when creating/updating applications.
+        """
+        date = datetime.datetime.now() + datetime.timedelta(days=1)
+
+        self.client.login(username=self.user5.username, password="test")
+
+        # Test creating new application with duplicates
+        response = self.client.post(
+            reverse("club-applications-list", args=(self.club1.code,)),
+            data={
+                "name": "Test Application",
+                "application_start_time": date,
+                "application_end_time": date + datetime.timedelta(days=7),
+                "result_release_time": date + datetime.timedelta(days=14),
+                "committees": [{"name": "General Member"}, {"name": "General Member"}],
+            },
+            content_type="application/json",
+        )
+
+        self.assertEqual(response.status_code, 400)
+        self.assertIn("Committee names must be unique", str(response.content))
+
+        # Create valid application for patch test
+        application = ClubApplication.objects.create(
+            name="Test Application",
+            club=self.club1,
+            application_start_time=date,
+            application_end_time=date + datetime.timedelta(days=7),
+            result_release_time=date + datetime.timedelta(days=14),
+        )
+        ApplicationCommittee.objects.create(
+            name="General Member", application=application
+        )
+
+        # Try to update with duplicate committees
+        response = self.client.patch(
+            reverse("club-applications-detail", args=(self.club1.code, application.id)),
+            data={
+                "committees": [{"name": "General Member"}, {"name": "General Member"}]
+            },
+            content_type="application/json",
+        )
+
+        self.assertEqual(response.status_code, 400)
+        self.assertIn("Committee names must be unique", str(response.content))
+
 
 class HealthTestCase(TestCase):
     def test_health(self):