Skip to content

Commit bd3e6a4

Browse files
rtibblesclaude
andcommitted
Extract authoring-user-field enforcement into a shared helper
Move the repeated "require an author FK, allow null only during deserialization, and drop references to a cross-dataset superuser" logic out of the course models and into AbstractFacilityDataModel.enforce_authoring_user_field, routing CourseSession, CourseSessionAssignment and UnitTestAssignment through it. No behaviour change: enforcement still happens on every non-deserialization save, matching the semantics established in #14130. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d02d3b9 commit bd3e6a4

2 files changed

Lines changed: 39 additions & 44 deletions

File tree

kolibri/core/auth/models.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,42 @@ def pre_save(self, **kwargs):
397397
except KolibriValidationError as e:
398398
raise IntegrityError(str(e))
399399

400+
def enforce_authoring_user_field(self, field_name, **save_kwargs):
401+
"""
402+
Enforce and sanitize an "authoring" foreign key to a ``FacilityUser`` -- e.g.
403+
``creator``, ``created_by``, ``assigned_by`` or ``activated_by``.
404+
405+
The field is required on any save, so we always capture who authored the record.
406+
The exception is deserialization, where the value comes from the originating
407+
device and may legitimately be null (e.g. on a learner-only device where the
408+
author's ``FacilityUser`` has not synced); Morango signals deserialization by
409+
passing ``update_dirty_bit_to=False`` to ``save`` (and hence ``pre_save``).
410+
411+
When the field points at a superuser belonging to a different dataset -- e.g. a
412+
device's own super admin authoring content in a facility that was synced onto the
413+
device -- the reference is dropped (nulled) so the record can sync without a
414+
dangling cross-dataset foreign key. Any other cross-dataset user is rejected.
415+
416+
The related foreign key must be declared ``blank=True, null=True`` so that
417+
``clean_fields`` (run by Morango during deserialization) accepts the nulled value.
418+
"""
419+
user = getattr(self, field_name)
420+
is_deserialization = save_kwargs.get("update_dirty_bit_to") is False
421+
if not is_deserialization and user is None:
422+
raise IntegrityError(
423+
"{model}.{field} may not be null".format(
424+
model=type(self).__name__, field=field_name
425+
)
426+
)
427+
if user and user.dataset_id != self.dataset_id:
428+
if not user.is_superuser:
429+
raise IntegrityError(
430+
"{model}.{field} must belong to the same dataset".format(
431+
model=type(self).__name__, field=field_name
432+
)
433+
)
434+
setattr(self, field_name, None)
435+
400436
def save(self, *args, **kwargs):
401437
self.pre_save(**kwargs)
402438
super().save(*args, **kwargs)

kolibri/core/courses/models.py

Lines changed: 3 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -321,20 +321,7 @@ def get_course_content_download_priority(self, contentnode_id):
321321

322322
def pre_save(self, **kwargs):
323323
super().pre_save(**kwargs)
324-
325-
is_deserialization = kwargs.get("update_dirty_bit_to") is False
326-
327-
if not is_deserialization and self.created_by is None:
328-
raise IntegrityError(
329-
"CourseSession must be saved with a non None created_by field"
330-
)
331-
332-
if self.created_by and self.created_by.dataset_id != self.dataset_id:
333-
if not self.created_by.is_superuser:
334-
raise IntegrityError(
335-
"CourseSession must have creator in the same dataset"
336-
)
337-
self.created_by = None
324+
self.enforce_authoring_user_field("created_by", **kwargs)
338325

339326
def infer_dataset(self, *args, **kwargs):
340327
return self.cached_related_dataset_lookup("collection")
@@ -427,22 +414,7 @@ def pre_save(self, **kwargs):
427414
"CourseSession assignment foreign models must be in same dataset"
428415
)
429416

430-
is_deserialization = kwargs.get("update_dirty_bit_to") is False
431-
432-
if not is_deserialization and self.assigned_by is None:
433-
raise IntegrityError(
434-
"CourseSession assignment must be saved with a non None assigned_by field"
435-
)
436-
437-
# validate that datasets match so this would be syncable
438-
if self.assigned_by and self.assigned_by.dataset_id != self.dataset_id:
439-
# the only time assigned_by can be null is if it's a superuser
440-
# and if we set it to none HERE
441-
if not self.assigned_by.is_superuser:
442-
raise IntegrityError(
443-
"CourseSession assignment must have assigner in the same dataset"
444-
)
445-
self.assigned_by = None
417+
self.enforce_authoring_user_field("assigned_by", **kwargs)
446418

447419
def infer_dataset(self, *args, **kwargs):
448420
# infer from course_session so assignments align with course_sessions
@@ -614,20 +586,7 @@ def pre_save(self, **kwargs):
614586
"UnitTestAssignment collection must be the same as or a child of the CourseSession's collection"
615587
)
616588

617-
is_deserialization = kwargs.get("update_dirty_bit_to") is False
618-
619-
if not is_deserialization and self.activated_by is None:
620-
raise IntegrityError(
621-
"UnitTestAssignment must be saved with an non None activated_by field"
622-
)
623-
624-
if self.activated_by and self.activated_by.dataset_id != self.dataset_id:
625-
# Only allow null for superusers
626-
if not self.activated_by.is_superuser:
627-
raise IntegrityError(
628-
"UnitTestAssignment activated_by must be in the same dataset"
629-
)
630-
self.activated_by = None
589+
self.enforce_authoring_user_field("activated_by", **kwargs)
631590

632591
@classmethod
633592
def deserialize(cls, dict_model, sync_filter=None):

0 commit comments

Comments
 (0)