Represent categories as a list after passing from changes

Author: Jakoma02

contentcuration/contentcuration/tests/viewsets/test_channel.py

@@ -615,7 +615,7 @@ def test_process_added_to_community_library_change(self, mock_export_func):
         )
         self.assertEqual(call_kwargs["channel_id"], channel.id)
         self.assertEqual(call_kwargs["channel_version"], 2)
-        self.assertEqual(call_kwargs["categories"], categories)
+        self.assertCountEqual(call_kwargs["categories"], categories.keys())
 
         # The countries argument used when creating the mapper is in fact
         # not a list, but a QuerySet, but it contains the same elements

contentcuration/contentcuration/viewsets/channel.py

@@ -795,6 +795,10 @@ def add_to_community_library_from_changes(self, changes):
     def add_to_community_library(
         self, channel_id, channel_version, categories, country_codes
     ):
+        # Note: The `categories` field should contain a _dict_, with the category IDs as keys
+        # and `True` as a value. This is to match the representation
+        # of sets in the changes architecture.
+
         # The change to add a channel to the community library can only
         # be created server-side, so in theory we should not be getting
         # malformed requests here. However, just to be safe, we still
@@ -810,11 +814,17 @@ def add_to_community_library(
         if channel_version <= 0 or channel_version > channel.version:
             raise ValidationError("Invalid channel version")
 
+        # Because of changes architecture, the categories are passed as a dict
+        # with the category IDs as keys and `True` as a value. At this point,
+        # we are no longer working with changes, so we switch to the more
+        # convenient representation as a list.
+        categories_list = [key for key, val in categories.items() if val]
+
         export_channel_to_kolibri_public(
             channel_id=channel_id,
             channel_version=channel_version,
             public=False,  # Community library
-            categories=categories,
+            categories=categories_list,
             countries=countries,
         )
 

contentcuration/kolibri_public/models.py

@@ -97,6 +97,8 @@ class AssessmentMetaData(base_models.AssessmentMetaData):
 
 
 class ChannelMetadata(base_models.ChannelMetadata):
+    # Note: The `categories` field should contain a _list_, NOT a _dict_.
+
     # precalculated fields during annotation/migration
     published_size = models.BigIntegerField(default=0, null=True, blank=True)
     total_resource_count = models.IntegerField(default=0, null=True, blank=True)

contentcuration/kolibri_public/tests/test_export_channels_to_kolibri_public.py

@@ -102,10 +102,7 @@ def tearDown(self):
     def test_export_channel_to_kolibri_public__existing_version(
         self, mock_channel_mapper
     ):
-        categories = {
-            "Category1": True,
-            "Category2": True,
-        }
+        categories = ["Category1", "Category2"]
         country1 = Country.objects.create(code="C1", name="Country 1")
         country2 = Country.objects.create(code="C2", name="Country 2")
         countries = [country1, country2]

contentcuration/kolibri_public/tests/test_mapper.py

@@ -181,15 +181,15 @@ def test_categories__none_provided(self):
             mapper = ChannelMapper(self.channel)
             mapper.run()
 
-            self.assertEqual(mapper.mapped_channel.categories, {})
+            self.assertEqual(mapper.mapped_channel.categories, [])
 
     def test_categories__only_provided(self):
         with using_content_database(self.tempdb):
             kolibri_content_models.ContentNode.objects.filter(
                 channel_id=self.channel.id,
             ).update(categories=None)
 
-            categories = {"Category1": True, "Category2": True}
+            categories = ["Category1", "Category2"]
             mapper = ChannelMapper(self.channel, categories=categories)
             mapper.run()
 
@@ -216,7 +216,7 @@ def test_categories__only_on_content_nodes(self):
 
             self.assertEqual(
                 mapper.mapped_channel.categories,
-                {"Category1": True, "Category2": True, "Category3": True},
+                ["Category1", "Category2", "Category3"],
             )
 
     def test_categories__both_provided_and_on_content_nodes(self):
@@ -235,18 +235,13 @@ def test_categories__both_provided_and_on_content_nodes(self):
             contentnode1.save()
             contentnode2.save()
 
-            categories = {"Category3": True, "Category4": True}
+            categories = ["Category3", "Category4"]
             mapper = ChannelMapper(self.channel, categories=categories)
             mapper.run()
 
             self.assertEqual(
                 mapper.mapped_channel.categories,
-                {
-                    "Category1": True,
-                    "Category2": True,
-                    "Category3": True,
-                    "Category4": True,
-                },
+                ["Category1", "Category2", "Category3", "Category4"],
             )
 
     def test_countries__none_provided(self):

contentcuration/kolibri_public/utils/annotation.py

@@ -21,6 +21,8 @@ def set_channel_metadata_fields(
     categories=None,
     countries=None,
 ):
+    # Note: The `categories` argument should be a _list_, NOT a _dict_.
+
     # Remove unneeded db_lock
     channel = ChannelMetadata.objects.get(id=channel_id)
     calculate_published_size(channel)
@@ -92,9 +94,9 @@ def calculate_included_categories(channel, categories):
             )
         )
     )
-    categories_dict = {category: True for category in contentnode_categories}
-    categories_dict.update(categories or {})
-    channel.categories = categories_dict
+
+    all_categories = sorted(set(categories or []).union(contentnode_categories))
+    channel.categories = all_categories
     channel.save()
 
 

contentcuration/kolibri_public/utils/export_channel_to_kolibri_public.py

@@ -23,6 +23,7 @@ def export_channel_to_kolibri_public(
     categories=None,
     countries=None,
 ):
+    # Note: The `categories` argument should be a _list_, NOT a _dict_.
     logger.info("Putting channel {} into kolibri_public".format(channel_id))
 
     if channel_version is not None:

contentcuration/kolibri_public/utils/mapper.py

@@ -29,6 +29,9 @@ def __init__(
         # Note: The argument `channel` is an instance of `kolibri_content.models.ChannelMetadata,`
         # which belongs to a specific channel version to be exported. Therefore, we do not
         # need to explicitly pass the channel version as an argument here.
+
+        # Note: The `categories` argument should be a _list_, NOT a _dict_.
+
         self.channel = channel
         self.public = public
         self.categories = categories