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

contentcuration/contentcuration/frontend/shared/data/resources.js

@@ -821,33 +821,6 @@ class Resource extends mix(APIResource, IndexedDBResource) {
     });
   }
 
-  createModel(data) {
-    return client.post(this.collectionUrl(), data).then(response => {
-      const now = Date.now();
-      const data = response.data;
-      data[LAST_FETCHED] = now;
-      // Directly write to the table, rather than using the add method
-      // to avoid creating change events that we would sync back to the server.
-      return this.transaction({ mode: 'rw' }, () => {
-        return this.table.put(data).then(() => {
-          return data;
-        });
-      });
-    });
-  }
-
-  deleteModel(id) {
-    return client.delete(this.modelUrl(id)).then(() => {
-      // Directly write to the table, rather than using the delete method
-      // to avoid creating change events that we would sync back to the server.
-      return this.transaction({ mode: 'rw' }, () => {
-        return this.table.delete(id).then(() => {
-          return true;
-        });
-      });
-    });
-  }
-
   /**
    * @param {String} id
    * @param {Boolean} [doRefresh=true] -- Whether or not to refresh async from server
@@ -878,6 +851,27 @@ class Resource extends mix(APIResource, IndexedDBResource) {
   }
 }
 
+/**
+ * Resource that allows directly creating through the API,
+ * rather than through IndexedDB. API must explicitly support this.
+ */
+class CreateModelResource extends Resource {
+  createModel(data) {
+    return client.post(this.collectionUrl(), data).then(response => {
+      const now = Date.now();
+      const data = response.data;
+      data[LAST_FETCHED] = now;
+      // Directly write to the table, rather than using the add method
+      // to avoid creating change events that we would sync back to the server.
+      return this.transaction({ mode: 'rw' }, () => {
+        return this.table.put(data).then(() => {
+          return data;
+        });
+      });
+    });
+  }
+}
+
 /**
  * Tree resources mixin
  */
@@ -1018,7 +1012,7 @@ export const Bookmark = new Resource({
   getUserId: getUserIdFromStore,
 });
 
-export const Channel = new Resource({
+export const Channel = new CreateModelResource({
   tableName: TABLE_NAMES.CHANNEL,
   urlName: 'channel',
   indexFields: ['name', 'language'],
@@ -1719,7 +1713,7 @@ export const ContentNode = new TreeResource({
   },
 });
 
-export const ChannelSet = new Resource({
+export const ChannelSet = new CreateModelResource({
   tableName: TABLE_NAMES.CHANNELSET,
   urlName: 'channelset',
   getUserId: getUserIdFromStore,
@@ -1757,7 +1751,7 @@ export const User = new Resource({
   uuid: false,
 
   updateAsAdmin(id, changes) {
-    return client.patch(window.Urls.adminUsersDetail(id), changes).then(() => {
+    return client.post(window.Urls.adminUsersAccept(id)).then(() => {
       return this.transaction({ mode: 'rw' }, () => {
         return this.table.update(id, changes);
       });

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)