Allow cross-dataset-authored quizzes and lessons to sync

rtibbles · claude · rtibbles · commit 91ca5b32ac81 · 2026-05-26T20:56:51.000-07:00
Exam, Lesson and their assignments declared their author foreign key
null=True but blank=False, and required a creator in pre_save on add. When
a record authored by a superuser from a different dataset (whose author FK
is nulled by pre_save) was deserialized on a receiving device, clean_fields
rejected the null (blank=False) and the pre_save add-check fired, leaving
the record dirty in the Store and absent from the device.

Make the author foreign keys blank=True and route the pre_save checks
through enforce_authoring_user_field, which permits a null author during
deserialization. AttendanceSession had the same pre_save gap and is fixed
too. Adds a serialize/deserialize regression test covering all three.

Co-Authored-By: Claude Opus 4.7 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/kolibri/core/attendance/models.py b/kolibri/core/attendance/models.py
@@ -1,5 +1,4 @@
 from django.db import models
-from django.db.utils import IntegrityError
 
 from kolibri.core.auth.constants import role_kinds
 from kolibri.core.auth.models import AbstractFacilityDataModel
@@ -50,14 +49,7 @@ def __str__(self):
 
     def pre_save(self, **kwargs):
         super().pre_save(**kwargs)
-        if self._state.adding and self.created_by is None:
-            raise IntegrityError("AttendanceSession must be saved with a creator")
-        if self.created_by and self.created_by.dataset_id != self.dataset_id:
-            if not self.created_by.is_superuser:
-                raise IntegrityError(
-                    "AttendanceSession must have creator in the same dataset"
-                )
-            self.created_by = None
+        self.enforce_authoring_user_field("created_by", **kwargs)
 
     def infer_dataset(self, *args, **kwargs):
         return self.cached_related_dataset_lookup("collection")
diff --git a/kolibri/core/auth/test/test_morango_integration.py b/kolibri/core/auth/test/test_morango_integration.py
@@ -33,8 +33,11 @@
 from ..models import LearnerGroup
 from ..models import Membership
 from ..models import Role
+from .helpers import create_superuser
 from .helpers import DUMMY_PASSWORD
+from .helpers import provision_device
 from .sync_utils import multiple_kolibri_servers
+from kolibri.core.attendance.models import AttendanceSession
 from kolibri.core.auth.constants import role_kinds
 from kolibri.core.auth.management.utils import get_client_and_server_certs
 from kolibri.core.auth.utils.sync import find_soud_sync_session_for_resume
@@ -92,6 +95,106 @@ def test_deserializing_field(self):
             self.fail(e.message)
 
 
+class CrossDatasetSuperuserDeserializationTestCase(TransactionTestCase):
+    """
+    Regression test for quizzes and lessons authored by a superuser that belongs to a
+    different dataset than the facility the content lives in (e.g. a device's own super
+    admin creating content in a facility that was synced onto the device).
+
+    In that case the models' ``pre_save`` deliberately nulls the creator/assigned_by
+    foreign key. The record must still deserialize on a receiving device: previously the
+    fields were ``blank=False``, so ``clean_fields()`` (which Morango runs during
+    deserialization) rejected the null value and the record stayed dirty in the Store,
+    never appearing on the receiving device.
+    """
+
+    def setUp(self):
+        self.controller = MorangoProfileController(PROFILE_FACILITY_DATA)
+        InstanceIDModel.get_or_create_current_instance()
+        provision_device()
+        # the facility the content lives in
+        self.facility = Facility.objects.create(name="Synced")
+        self.classroom = Classroom.objects.create(name="Class", parent=self.facility)
+        # a superuser belonging to a different dataset
+        self.other_facility = Facility.objects.create(name="Device")
+        self.superuser = create_superuser(self.other_facility, username="deviceadmin")
+
+    def _assert_deserializes_cleanly(self, *instance_ids):
+        self.controller.serialize_into_store()
+        Store.objects.update(dirty_bit=True)
+        self.controller.deserialize_from_store()
+        for instance_id in instance_ids:
+            store = Store.objects.get(id=instance_id)
+            self.assertIsNone(
+                store.deserialization_error,
+                msg="{} failed to deserialize: {}".format(
+                    store.model_name, store.deserialization_error
+                ),
+            )
+            self.assertFalse(store.dirty_bit)
+
+    def test_exam_authored_by_cross_dataset_superuser_deserializes(self):
+        exam = Exam.objects.create(
+            title="Quiz",
+            question_count=1,
+            question_sources=[
+                {
+                    "exercise_id": uuid.uuid4().hex,
+                    "question_id": uuid.uuid4().hex,
+                    "title": "a",
+                }
+            ],
+            collection=self.classroom,
+            creator=self.superuser,
+            active=True,
+        )
+        assignment = ExamAssignment.objects.create(
+            exam=exam,
+            collection=self.classroom,
+            assigned_by=self.superuser,
+        )
+        # pre_save nulls the cross-dataset superuser foreign keys
+        self.assertIsNone(Exam.objects.get(id=exam.id).creator_id)
+        self.assertIsNone(ExamAssignment.objects.get(id=assignment.id).assigned_by_id)
+
+        self._assert_deserializes_cleanly(exam.id, assignment.id)
+
+    def test_lesson_authored_by_cross_dataset_superuser_deserializes(self):
+        lesson = Lesson.objects.create(
+            title="Lesson",
+            resources=[
+                {
+                    "contentnode_id": uuid.uuid4().hex,
+                    "content_id": uuid.uuid4().hex,
+                    "channel_id": uuid.uuid4().hex,
+                }
+            ],
+            collection=self.classroom,
+            created_by=self.superuser,
+            is_active=True,
+        )
+        assignment = LessonAssignment.objects.create(
+            lesson=lesson,
+            collection=self.classroom,
+            assigned_by=self.superuser,
+        )
+        # pre_save nulls the cross-dataset superuser foreign keys
+        self.assertIsNone(Lesson.objects.get(id=lesson.id).created_by_id)
+        self.assertIsNone(LessonAssignment.objects.get(id=assignment.id).assigned_by_id)
+
+        self._assert_deserializes_cleanly(lesson.id, assignment.id)
+
+    def test_attendance_session_authored_by_cross_dataset_superuser_deserializes(self):
+        session = AttendanceSession.objects.create(
+            collection=self.classroom,
+            created_by=self.superuser,
+        )
+        # pre_save nulls the cross-dataset superuser foreign key
+        self.assertIsNone(AttendanceSession.objects.get(id=session.id).created_by_id)
+
+        self._assert_deserializes_cleanly(session.id)
+
+
 class MultipleServerTestCase(TestCase):
     """
     A test case to do special teardown handling to prevent errors from our additional databases.
diff --git a/kolibri/core/exams/migrations/0011_allow_blank_creator_and_assigned_by.py b/kolibri/core/exams/migrations/0011_allow_blank_creator_and_assigned_by.py
@@ -0,0 +1,47 @@
+# Generated by Django 3.2.25 on 2026-05-27 03:04
+import django.db.models.deletion
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("exams", "0010_add_exam_report_visibility_field"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="draftexam",
+            name="creator",
+            field=models.ForeignKey(
+                blank=True,
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name="draftexams",
+                to="kolibriauth.FacilityUser",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="exam",
+            name="creator",
+            field=models.ForeignKey(
+                blank=True,
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name="exams",
+                to="kolibriauth.FacilityUser",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="examassignment",
+            name="assigned_by",
+            field=models.ForeignKey(
+                blank=True,
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name="assigned_exams",
+                to="kolibriauth.FacilityUser",
+            ),
+        ),
+    ]
diff --git a/kolibri/core/exams/models.py b/kolibri/core/exams/models.py
@@ -141,7 +141,7 @@ class Meta:
     creator = models.ForeignKey(
         FacilityUser,
         related_name="%(class)ss",
-        blank=False,
+        blank=True,
         null=True,
         on_delete=models.CASCADE,
     )
@@ -267,18 +267,7 @@ def delete(self, using=None, keep_parents=False):
 
     def pre_save(self, **kwargs):
         super().pre_save(**kwargs)
-
-        # maintain stricter enforcement on when creator is allowed to be null
-        if self._state.adding and self.creator is None:
-            raise IntegrityError("Exam must be saved with an creator")
-
-        # validate that datasets match so this would be syncable
-        if self.creator and self.creator.dataset_id != self.dataset_id:
-            # the only time creator can be null is if it's a superuser
-            # and if we set it to none HERE
-            if not self.creator.is_superuser:
-                raise IntegrityError("Exam must have creator in the same dataset")
-            self.creator = None
+        self.enforce_authoring_user_field("creator", **kwargs)
 
     def save(self, *args, **kwargs):
         # If archive is True during the save op, but there is no date_archived then
@@ -342,7 +331,7 @@ class ExamAssignment(AbstractFacilityDataModel):
     assigned_by = models.ForeignKey(
         FacilityUser,
         related_name="assigned_exams",
-        blank=False,
+        blank=True,
         null=True,
         on_delete=models.CASCADE,
     )
@@ -360,21 +349,7 @@ def pre_save(self, **kwargs):
                 "Exam assignment foreign models must be in same dataset"
             )
 
-        # maintain stricter enforcement on when assigned_by is allowed to be null
-        # assignments aren't usually updated, but ensure only during creation
-        if self._state.adding and self.assigned_by is None:
-            raise IntegrityError("Exam assignment must be saved with an assigner")
-
-        # validate that datasets match so this would be syncable
-        if self.assigned_by and self.assigned_by.dataset_id != self.dataset_id:
-            # the only time assigned_by can be null is if it's a superuser
-            # and if we set it to none HERE
-            if not self.assigned_by.is_superuser:
-                # maintain stricter enforcement on when assigned_by is allowed to be null
-                raise IntegrityError(
-                    "Exam assignment must have assigner in the same dataset"
-                )
-            self.assigned_by = None
+        self.enforce_authoring_user_field("assigned_by", **kwargs)
 
     def infer_dataset(self, *args, **kwargs):
         # infer from exam so assignments align with exams
diff --git a/kolibri/core/lessons/migrations/0005_allow_blank_creator_and_assigned_by.py b/kolibri/core/lessons/migrations/0005_allow_blank_creator_and_assigned_by.py
@@ -0,0 +1,36 @@
+# Generated by Django 3.2.25 on 2026-05-27 03:04
+import django.db.models.deletion
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("lessons", "0004_nullable_created_by_assigned_by"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="lesson",
+            name="created_by",
+            field=models.ForeignKey(
+                blank=True,
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name="lessons_created",
+                to="kolibriauth.FacilityUser",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="lessonassignment",
+            name="assigned_by",
+            field=models.ForeignKey(
+                blank=True,
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name="assigned_lessons",
+                to="kolibriauth.FacilityUser",
+            ),
+        ),
+    ]
diff --git a/kolibri/core/lessons/models.py b/kolibri/core/lessons/models.py
@@ -65,7 +65,7 @@ class Lesson(AbstractFacilityDataModel):
     created_by = models.ForeignKey(
         FacilityUser,
         related_name="lessons_created",
-        blank=False,
+        blank=True,
         null=True,
         on_delete=models.CASCADE,
     )
@@ -96,17 +96,7 @@ def __str__(self):
 
     def pre_save(self, **kwargs):
         super().pre_save(**kwargs)
-
-        # maintain stricter enforcement on when assigned_by is allowed to be null
-        if self._state.adding and self.created_by is None:
-            raise IntegrityError("Lesson must be saved with a creator")
-
-        # if the created_by user is in a different dataset than the dataset we've determined, then
-        # we'll unset it to prevent deserialization errors when syncing
-        if self.created_by and self.created_by.dataset_id != self.dataset_id:
-            if not self.created_by.is_superuser:
-                raise IntegrityError("Lesson must have creator in the same dataset")
-            self.created_by = None
+        self.enforce_authoring_user_field("created_by", **kwargs)
 
     def delete(self, using=None, keep_parents=False):
         """
@@ -152,7 +142,7 @@ class LessonAssignment(AbstractFacilityDataModel):
     assigned_by = models.ForeignKey(
         FacilityUser,
         related_name="assigned_lessons",
-        blank=False,
+        blank=True,
         null=True,
         on_delete=models.CASCADE,
     )
@@ -178,20 +168,7 @@ def pre_save(self, **kwargs):
                 "Lesson assignment foreign models must be in same dataset"
             )
 
-        # maintain stricter enforcement on when assigned_by is allowed to be null
-        # assignments aren't usually updated, but ensure only during creation
-        if self._state.adding and self.assigned_by is None:
-            raise IntegrityError("Lesson assignment must be saved with an assigner")
-
-        # validate that datasets match so this would be syncable
-        if self.assigned_by and self.assigned_by.dataset_id != self.dataset_id:
-            # the only time assigned_by can be null is if it's a superuser
-            # and if we set it to none HERE
-            if not self.assigned_by.is_superuser:
-                raise IntegrityError(
-                    "Lesson assignment must have assigner in the same dataset"
-                )
-            self.assigned_by = None
+        self.enforce_authoring_user_field("assigned_by", **kwargs)
 
     def infer_dataset(self, *args, **kwargs):
         # infer from lesson so assignments align with lessons
