Always require an actor_id to identify the user who caused a specific event in a channel's history.

Author: rtibbles

contentcuration/contentcuration/models.py

@@ -716,6 +716,32 @@ def exists(self, *filters):
         return Exists(self.queryset().filter(*filters).values("user_id"))
 
 
+class ChannelModelQuerySet(models.QuerySet):
+    def create(self, **kwargs):
+        """
+        Create a new object with the given kwargs, saving it to the database
+        and returning the created object.
+        Overriding the Django default here to allow passing through the actor_id
+        to register this event in the channel history.
+        """
+        # Either allow the actor_id to be passed in, or read from a special attribute
+        # on the queryset, this makes super calls to other methods easier to handle
+        # without having to reimplement the entire method.
+        actor_id = kwargs.pop("actor_id", getattr(self, "_actor_id", None))
+        obj = self.model(**kwargs)
+        self._for_write = True
+        obj.save(force_insert=True, using=self.db, actor_id=actor_id)
+        return obj
+
+    def get_or_create(self, defaults=None, **kwargs):
+        self._actor_id = kwargs.pop("actor_id", None)
+        return super().get_or_create(defaults, **kwargs)
+
+    def update_or_create(self, defaults=None, **kwargs):
+        self._actor_id = kwargs.pop("actor_id", None)
+        return super().update_or_create(defaults, **kwargs)
+
+
 class Channel(models.Model):
     """ Permissions come from association with organizations """
     id = UUIDField(primary_key=True, default=uuid.uuid4)
@@ -800,6 +826,8 @@ class Channel(models.Model):
         "version",
     ])
 
+    objects = ChannelModelQuerySet.as_manager()
+
     @classmethod
     def get_editable(cls, user, channel_id):
         return cls.filter_edit_queryset(cls.objects.all(), user).get(id=channel_id)
@@ -874,6 +902,10 @@ def get_resource_size(self):
         return files['resource_size'] or 0
 
     def on_create(self):
+        actor_id = getattr(self, "_actor_id", None)
+        if actor_id is None:
+            raise ValueError("No actor_id passed to save method")
+
         if not self.content_defaults:
             self.content_defaults = DEFAULT_CONTENT_DEFAULTS
 
@@ -905,7 +937,7 @@ def on_create(self):
         if self.public and (self.main_tree and self.main_tree.published):
             delete_public_channel_cache_keys()
 
-    def on_update(self):
+    def on_update(self):  # noqa C901
         from contentcuration.utils.user import calculate_user_storage
         original_values = self._field_updates.changed()
 
@@ -929,14 +961,23 @@ def on_update(self):
             for editor in self.editors.all():
                 calculate_user_storage(editor.pk)
 
-        # Delete db if channel has been deleted and mark as unpublished
         if "deleted" in original_values and not original_values["deleted"]:
             self.pending_editors.all().delete()
+            # Delete db if channel has been deleted and mark as unpublished
             export_db_storage_path = os.path.join(settings.DB_ROOT, "{channel_id}.sqlite3".format(channel_id=self.id))
             if default_storage.exists(export_db_storage_path):
                 default_storage.delete(export_db_storage_path)
                 if self.main_tree:
                     self.main_tree.published = False
+        # mark the instance as deleted or recovered, if requested
+        if "deleted" in original_values:
+            user_id = getattr(self, "_actor_id", None)
+            if user_id is None:
+                raise ValueError("No actor_id passed to save method")
+            if original_values["deleted"]:
+                self.history.create(actor_id=user_id, action=channel_history.RECOVERY)
+            else:
+                self.history.create(actor_id=user_id, action=channel_history.DELETION)
 
         if self.main_tree and self.main_tree._field_updates.changed():
             self.main_tree.save()
@@ -946,13 +987,20 @@ def on_update(self):
             delete_public_channel_cache_keys()
 
     def save(self, *args, **kwargs):
-        if self._state.adding:
+        self._actor_id = kwargs.pop("actor_id", None)
+        creating = self._state.adding
+        if creating:
+            if self._actor_id is None:
+                raise ValueError("No actor_id passed to save method")
             self.on_create()
         else:
             self.on_update()
 
         super(Channel, self).save(*args, **kwargs)
 
+        if creating:
+            self.history.create(actor_id=self._actor_id, action=channel_history.CREATION)
+
     def get_thumbnail(self):
         return get_channel_thumbnail(self)
 
@@ -996,24 +1044,11 @@ def make_public(self, bypass_signals=False):
 
         return self
 
-    def mark_created(self, user):
-        self.history.create(actor_id=to_pk(user), action=channel_history.CREATION)
-
     def mark_publishing(self, user):
         self.history.create(actor_id=to_pk(user), action=channel_history.PUBLICATION)
         self.main_tree.publishing = True
         self.main_tree.save()
 
-    def mark_deleted(self, user):
-        self.history.create(actor_id=to_pk(user), action=channel_history.DELETION)
-        self.deleted = True
-        self.save()
-
-    def mark_recovered(self, user):
-        self.history.create(actor_id=to_pk(user), action=channel_history.RECOVERY)
-        self.deleted = False
-        self.save()
-
     @property
     def deletion_history(self):
         return self.history.filter(action=channel_history.DELETION)

contentcuration/contentcuration/tests/test_channel_model.py

@@ -145,7 +145,7 @@ def test_returns_date_newer_when_node_added(self):
         # add a new node
         node(
             parent=self.channel.main_tree,
-            data={"node_id": "nodez", "title": "new child", "kind_id": "video",},
+            data={"node_id": "nodez", "title": "new child", "kind_id": "video"},
         )
         # check that the returned date is newer
         assert self.channel.get_date_modified() > old_date
@@ -160,7 +160,7 @@ def setUp(self):
         super(GetAllChannelsTestCase, self).setUp()
 
         # create 10 channels for comparison
-        self.channels = mixer.cycle(10).blend(Channel)
+        self.channels = [Channel.objects.create(actor_id=self.admin_user.id) for _ in range(10)]
 
     def test_returns_all_channels_in_the_db(self):
         """
@@ -179,9 +179,10 @@ class ChannelSetTestCase(BaseAPITestCase):
     def setUp(self):
         super(ChannelSetTestCase, self).setUp()
         self.channelset = mixer.blend(ChannelSet, editors=[self.user])
-        self.channels = mixer.cycle(10).blend(
-            Channel, secret_tokens=[self.channelset.secret_token], editors=[self.user]
-        )
+        self.channels = [Channel.objects.create(actor_id=self.user.id) for _ in range(10)]
+        for chann in self.channels:
+            chann.secret_tokens.add(self.channelset.secret_token)
+            chann.editors.add(self.user)
 
     def test_get_user_channel_sets(self):
         """ Make sure get_user_channel_sets returns the correct sets """
@@ -215,7 +216,7 @@ def test_channelset_deletion(self):
     def test_save_channels_to_token(self):
         """ Check endpoint will assign token to channels """
         token = self.channelset.secret_token
-        channels = mixer.cycle(5).blend(Channel)
+        channels = [Channel.objects.create(actor_id=self.user.id) for _ in range(5)]
         channels = Channel.objects.filter(
             pk__in=[c.pk for c in channels]
         )  # Make this a queryset
@@ -274,7 +275,7 @@ class ChannelMetadataSaveTestCase(StudioTestCase):
 
     def setUp(self):
         super(ChannelMetadataSaveTestCase, self).setUp()
-        self.channels = mixer.cycle(5).blend(Channel)
+        self.channels = [Channel.objects.create(actor_id=self.admin_user.id) for _ in range(5)]
         for c in self.channels:
             c.main_tree.changed = False
             c.main_tree.save()

contentcuration/contentcuration/tests/test_contentnodes.py

@@ -876,7 +876,7 @@ def _setup_original_and_deriative_nodes(self):
 
         # Setup derivative channel
         self.new_channel = Channel.objects.create(
-            name="derivative of teschannel", source_id="lkajs"
+            name="derivative of teschannel", source_id="lkajs", actor_id=self.admin_user.id
         )
         self.new_channel.save()
         self.new_channel.main_tree = self._create_empty_tree()

contentcuration/contentcuration/tests/test_exportchannel.py

@@ -493,20 +493,21 @@ def tearDown(self):
         clear_tasks()
 
     def test_convert_channel_thumbnail_empty_thumbnail(self):
-        channel = cc.Channel.objects.create()
+        channel = cc.Channel.objects.create(actor_id=self.admin_user.id)
         self.assertEqual("", convert_channel_thumbnail(channel))
 
     def test_convert_channel_thumbnail_static_thumbnail(self):
-        channel = cc.Channel.objects.create(thumbnail="/static/kolibri_flapping_bird.png")
+        channel = cc.Channel.objects.create(thumbnail="/static/kolibri_flapping_bird.png", actor_id=self.admin_user.id)
         self.assertEqual("", convert_channel_thumbnail(channel))
 
     def test_convert_channel_thumbnail_encoding_valid(self):
-        channel = cc.Channel.objects.create(thumbnail="/content/kolibri_flapping_bird.png", thumbnail_encoding={"base64": "flappy_bird"})
+        channel = cc.Channel.objects.create(
+            thumbnail="/content/kolibri_flapping_bird.png", thumbnail_encoding={"base64": "flappy_bird"}, actor_id=self.admin_user.id)
         self.assertEqual("flappy_bird", convert_channel_thumbnail(channel))
 
     def test_convert_channel_thumbnail_encoding_invalid(self):
         with patch("contentcuration.utils.publish.get_thumbnail_encoding", return_value="this is a test"):
-            channel = cc.Channel.objects.create(thumbnail="/content/kolibri_flapping_bird.png", thumbnail_encoding={})
+            channel = cc.Channel.objects.create(thumbnail="/content/kolibri_flapping_bird.png", thumbnail_encoding={}, actor_id=self.admin_user.id)
             self.assertEqual("this is a test", convert_channel_thumbnail(channel))
 
     def test_create_slideshow_manifest(self):
@@ -543,7 +544,7 @@ def tearDown(self):
             os.remove(self.output_db)
 
     def test_nonexistent_prerequisites(self):
-        channel = cc.Channel.objects.create()
+        channel = cc.Channel.objects.create(actor_id=self.admin_user.id)
         node1 = cc.ContentNode.objects.create(kind_id="exercise", parent_id=channel.main_tree.pk, complete=True)
         exercise = cc.ContentNode.objects.create(kind_id="exercise", complete=True)
 
@@ -554,7 +555,7 @@ def test_nonexistent_prerequisites(self):
 class ChannelExportPublishedData(StudioTestCase):
     def test_fill_published_fields(self):
         version_notes = description()
-        channel = cc.Channel.objects.create()
+        channel = cc.Channel.objects.create(actor_id=self.admin_user.id)
         channel.last_published
         fill_published_fields(channel, version_notes)
         self.assertTrue(channel.published_data)

contentcuration/contentcuration/tests/test_models.py

@@ -165,7 +165,7 @@ def test_filter_view_queryset__public_channel(self):
     def test_filter_view_queryset__public_channel__deleted(self):
         channel = self.public_channel
         channel.deleted = True
-        channel.save()
+        channel.save(actor_id=self.admin_user.id)
 
         queryset = Channel.filter_view_queryset(self.base_queryset, user=self.forbidden_user)
         self.assertQuerysetDoesNotContain(queryset, pk=channel.id)
@@ -816,7 +816,7 @@ def _setup_user_related_data(self):
         user_b = self._create_user("b@tester.com")
 
         # Create a sole editor non-public channel.
-        sole_editor_channel = Channel.objects.create(name="sole-editor")
+        sole_editor_channel = Channel.objects.create(name="sole-editor", actor_id=user_a.id)
         sole_editor_channel.editors.add(user_a)
 
         # Create sole-editor channel nodes.
@@ -944,18 +944,20 @@ def setUp(self):
         self.channel = testdata.channel()
 
     def test_mark_channel_created(self):
-        self.assertEqual(0, self.channel.history.filter(action=channel_history.CREATION).count())
-        self.channel.mark_created(self.admin_user)
-        self.assertEqual(1, self.channel.history.filter(actor=self.admin_user, action=channel_history.CREATION).count())
+        self.assertEqual(1, self.channel.history.filter(action=channel_history.CREATION).count())
 
     def test_mark_channel_deleted(self):
         self.assertEqual(0, self.channel.deletion_history.count())
-        self.channel.mark_deleted(self.admin_user)
+        self.channel.deleted = True
+        self.channel.save(actor_id=self.admin_user.id)
         self.assertEqual(1, self.channel.deletion_history.filter(actor=self.admin_user).count())
 
     def test_mark_channel_recovered(self):
         self.assertEqual(0, self.channel.history.filter(actor=self.admin_user, action=channel_history.RECOVERY).count())
-        self.channel.mark_recovered(self.admin_user)
+        self.channel.deleted = True
+        self.channel.save(actor_id=self.admin_user.id)
+        self.channel.deleted = False
+        self.channel.save(actor_id=self.admin_user.id)
         self.assertEqual(1, self.channel.history.filter(actor=self.admin_user, action=channel_history.RECOVERY).count())
 
     def test_prune(self):
@@ -966,6 +968,7 @@ def test_prune(self):
             testdata.channel()
         ]
         last_history_ids = []
+        ChannelHistory.objects.all().delete()
 
         self.assertEqual(0, ChannelHistory.objects.count())
 

contentcuration/contentcuration/tests/test_restore_channel.py

@@ -93,7 +93,7 @@ def setUp(self, thumb_mock):
             self.version,
             self.last_updated,
         )
-        self.channel, _ = create_channel(self.cursor_mock, self.id)
+        self.channel, _ = create_channel(self.cursor_mock, self.id, self.admin_user)
 
     def test_restore_channel_id(self):
         self.assertEqual(self.channel.id, self.id)

contentcuration/contentcuration/tests/test_serializers.py

@@ -2,6 +2,7 @@
 
 from django.db.models.query import QuerySet
 from le_utils.constants import content_kinds
+from mock import Mock
 from rest_framework import serializers
 
 from .base import BaseAPITestCase
@@ -146,12 +147,15 @@ class Meta:
             nested_writes = True
 
     def test_save__create(self):
+        request = Mock()
+        request.user = self.user
         s = self.ChannelSerializer(
             data=dict(
                 name="New test channel",
                 description="This is the best test channel",
                 content_defaults=dict(author="Buster"),
-            )
+            ),
+            context=dict(request=request),
         )
 
         self.assertTrue(s.is_valid())
@@ -167,7 +171,7 @@ def test_save__update(self):
             description="This is the best test channel",
             content_defaults=dict(author="Buster"),
         )
-        c.save()
+        c.save(actor_id=self.user.id)
 
         s = self.ChannelSerializer(
             c, data=dict(content_defaults=dict(license="Special Permissions"))

contentcuration/contentcuration/tests/test_sync.py

@@ -37,7 +37,7 @@ class SyncTestCase(StudioTestCase):
 
     def setUp(self):
         super(SyncTestCase, self).setUpBase()
-        self.derivative_channel = Channel.objects.create(name="testchannel")
+        self.derivative_channel = Channel.objects.create(name="testchannel", actor_id=self.admin_user.id)
         self.channel.main_tree.copy_to(self.derivative_channel.main_tree)
         self.derivative_channel.main_tree.refresh_from_db()
         self.derivative_channel.save()

contentcuration/contentcuration/tests/testdata.py

@@ -211,7 +211,8 @@ def tree(parent=None):
 
 
 def channel(name="testchannel"):
-    channel = cc.Channel.objects.create(name=name)
+    channel_creator = user()
+    channel = cc.Channel.objects.create(name=name, actor_id=channel_creator.id)
     channel.save()
 
     channel.main_tree = tree()