add more validations

Author: asadali145

src/ol_openedx_course_sync/migrations/0001_initial.py

@@ -1,4 +1,4 @@
-# Generated by Django 4.2.20 on 2025-05-13 08:28
+# Generated by Django 4.2.20 on 2025-05-14 09:11
 
 import opaque_keys.edx.django.models
 from django.db import migrations, models
@@ -24,7 +24,9 @@ class Migration(migrations.Migration):
                 ),
                 (
                     "source_course",
-                    opaque_keys.edx.django.models.CourseKeyField(max_length=255),
+                    opaque_keys.edx.django.models.CourseKeyField(
+                        max_length=255, unique=True
+                    ),
                 ),
                 (
                     "target_courses",

src/ol_openedx_course_sync/models.py

@@ -2,6 +2,7 @@
 Models for ol-openedx-course-sync plugin
 """
 
+from django.core.exceptions import ValidationError
 from django.db import models
 from opaque_keys.edx.django.models import (
     CourseKeyField,
@@ -33,10 +34,69 @@ class CourseSyncMap(models.Model):
     courses that are part of any organization added in `CourseSyncParentOrg`.
     """
 
-    source_course = CourseKeyField(max_length=255)
+    source_course = CourseKeyField(max_length=255, unique=True)
     target_courses = models.TextField(
         blank=True, help_text="Comma separated list of target course keys"
     )
 
     def __str__(self):
         return f"{self.source_course} Course Sync Map"
+
+    def save(self, *args, **kwargs):
+        """
+        Override save method to perform custom validations.
+        """
+        self.full_clean()
+        super().save(*args, **kwargs)
+
+    def clean(self):
+        """
+        Override clean method to perform custom validations.
+        """
+        super().clean()
+
+        conflicting_targets = CourseSyncMap.objects.filter(
+            target_courses__contains=self.source_course
+        )
+        if conflicting_targets:
+            raise ValidationError(
+                {
+                    "source_course": f"This course is already used as target course of: "  # noqa: E501
+                    f"{', '.join(str(ct.source_course) for ct in conflicting_targets)}"
+                }
+            )
+
+        conflicting_sources = CourseSyncMap.objects.filter(
+            source_course__in=self.target_course_keys
+        )
+        if conflicting_sources:
+            raise ValidationError(
+                {
+                    "target_courses": f"These course(s) are already used as source courses: "  # noqa:E501
+                    f"{', '.join(str(cs.source_course) for cs in conflicting_sources)}"
+                }
+            )
+
+        if self.target_course_keys:
+            query = models.Q()
+            for key in self.target_course_keys:
+                query |= models.Q(**{"target_courses__contains": key})
+            duplicate_targets = CourseSyncMap.objects.filter(query)
+
+            if self.pk:
+                duplicate_targets = duplicate_targets.exclude(pk=self.pk)
+
+            if duplicate_targets:
+                raise ValidationError(
+                    {
+                        "target_courses": f"Some of these course(s) are already used as target course(s) for: "  # noqa:E501
+                        f"{', '.join(str(dt.source_course) for dt in duplicate_targets)}"  # noqa:E501
+                    }
+                )
+
+    @property
+    def target_course_keys(self):
+        """
+        Returns a list of target course keys.
+        """
+        return [key for key in self.target_courses.strip().split(",") if key]

src/ol_openedx_course_sync/signals.py

@@ -5,6 +5,7 @@
 import logging
 
 from common.djangoapps.course_action_state.models import CourseRerunState
+from django.core.exceptions import ValidationError
 from django.db.models.signals import post_save
 from django.dispatch import receiver
 from ol_openedx_course_sync.constants import COURSE_RERUN_STATE_SUCCEEDED
@@ -29,12 +30,15 @@ def listen_for_course_publish(
 
     course_sync_map = CourseSyncMap.objects.filter(source_course=course_key).first()
     if not (course_sync_map and course_sync_map.target_courses):
-        log.info("No mapping found for course %s. Skipping copy.", course_key.id)
+        log.info("No mapping found for course %s. Skipping copy.", str(course_key))
         return
 
     source_course = str(course_sync_map.source_course)
     user_id = None
-    for target_course_key in course_sync_map.target_courses.split(","):
+    target_keys = [
+        key for key in course_sync_map.target_courses.strip().split(",") if key
+    ]
+    for target_course_key in target_keys:
         log.info(
             "Initializing async course content sync from %s to %s",
             source_course,
@@ -57,30 +61,31 @@ def listen_for_course_rerun_state_post_save(sender, instance, **kwargs):  # noqa
     ).exists():
         return
 
-    # If source course is a target/child of any other course, we won't make it
-    # a source/parent to avoid loops in course sync
-    if CourseSyncMap.objects.filter(
-        target_courses__contains=str(instance.source_course_key)
-    ).exists():
-        log.warning(
-            "Course %s is already a target course. Skipping.",
+    try:
+        course_sync_map, _ = CourseSyncMap.objects.get_or_create(
+            source_course=instance.source_course_key
+        )
+    except ValidationError:
+        log.exception(
+            "Failed to create CourseSyncMap for %s",
             instance.source_course_key,
         )
         return
 
-    course_sync_map, _ = CourseSyncMap.objects.get_or_create(
-        source_course=instance.source_course_key
-    )
-    target_courses = (
-        course_sync_map.target_courses.split(",")
-        if course_sync_map.target_courses
-        else []
-    )
+    target_courses = course_sync_map.target_course_keys
     target_courses.append(str(instance.course_key))
     course_sync_map.target_courses = ",".join(target_courses)
-    course_sync_map.save()
-    log.info(
-        "Added course %s to target courses for %s",
-        instance.course_key,
-        instance.source_course_key,
-    )
+
+    try:
+        course_sync_map.save()
+    except ValidationError:
+        log.exception(
+            "Failed to update CourseSyncMap for %s",
+            instance.source_course_key,
+        )
+    else:
+        log.info(
+            "Added course %s to target courses for %s",
+            instance.course_key,
+            instance.source_course_key,
+        )