Repository: learningequality/ricecooker

Issue:
TreeNode to_dict is incompatible with Studio unstable branch
* ricecooker version: v0.7.0-beta6
* Python version: 3.7
* Operating System: Fedora Silverblue 36

### Description

I am creating a ricecooker script which makes use of the new structured metadata fields in the `develop` branch. This script creates a tree of TopicNode and VideoNode objects.

This is all working almost correctly.

The video nodes end up with attributes like `node.grade_levels = [levels.LOWER_PRIMARY]`, and the [`ContentNode.to_dict()`](https://github.com/learningequality/ricecooker/blob/aa857d102fc19a066931a8fe97e4d869e6d0d606/ricecooker/classes/nodes.py#L797-L832) function for each video node produces the expected schema, with something like `{…, "grade_levels": "wnarlxKo", …}`.

The topic nodes do not have the structured metadata attributes. However, the [`TreeNode.to_dict()`](https://github.com/learningequality/ricecooker/blob/aa857d102fc19a066931a8fe97e4d869e6d0d606/ricecooker/classes/nodes.py#L545-L578) function produces output like `{…, "grade_levels": null, …}`.

This is incompatible with the code in Studio which validates attributes for nodes: https://github.com/learningequality/studio/blob/269e7e0b677c569c3a68c0a30a0d0fa342f190c0/contentcuration/contentcuration/views/internal.py#L667-L675. If a structured metadata attribute is included in the body, it _must_ be a list.

The result is a failed upload, with an error like the following:

```
(319)add_nodes()
    318             ]
--> 319             for chunk in chunks:
    320                 payload_children = []

ipdb> response.content
b"['grade_levels must pass a list of values']"
ipdb> response.request.body
'{"root_id": "db6de12407f24f1c9e36752abd49ef2f", "content_data": [{"title": "Basketball", "language": "en", "description": "", "node_id": "8ed9e0083dc45373a385d2ab2d001132", "content_id": "ce34868e07ec5404a35de380ccd502dd", "source_domain": "379151f326fd5a0080473baf3c4c396e", "source_id": "basketball", "author": "", "aggregator": "", "provider": "", "files": [], "tags": [], "kind": "topic", "license": null, "license_description": null, "copyright_holder": "", "questions": [], "extra_fields": "{}", "grade_levels": null, "resource_types": null, "learning_activities": null, "accessibility_categories": null, "subjects": null, "needs": null}, {"title": "Soccer", "language": "en", "description": "", "node_id": "357293c10e8b59e5b383da07d62c6c90", "content_id": "030cbf9bbc8353bbbbb91d641b6318e9", "source_domain": "379151f326fd5a0080473baf3c4c396e", "source_id": "soccer", "author": "", "aggregator": "", "provider": "", "files": [], "tags": [], "kind": "topic", "license": null, "license_description": null, "copyright_holder": "", "questions": [], "extra_fields": "{}", "grade_levels": null, "resource_types": null, "learning_activities": null, "accessibility_categories": null, "subjects": null, "needs": null}, {"title": "Juggling", "language": "en", "description": "", "node_id": "a5f005393b7b5841a6b067a99779d7b5", "content_id": "ef04f09ef399570aba145495ddcce80a", "source_domain": "379151f326fd5a0080473baf3c4c396e", "source_id": "juggling", "author": "", "aggregator": "", "provider": "", "files": [], "tags": [], "kind": "topic", "license": null, "license_description": null, "copyright_holder": "", "questions": [], "extra_fields": "{}", "grade_levels": null, "resource_types": null, "learning_activities": null, "accessibility_categories": null, "subjects": null, "needs": null}]}'
```


Discussion:

Patch:
diff --git a/Makefile b/Makefile
index aabbbbd..4a0a854 100644
--- a/Makefile
+++ b/Makefile
@@ -59,6 +59,14 @@ test: clean-test ## run tests quickly with the default Python
 test-all: clean-test ## run tests on every Python version with tox
 	tox
 
+integration-test:
+	echo "Testing against hotfixes"
+	CONTENTWORKSHOP_URL=https://hotfixes.studio.learningequality.org python tests/test_chef_integration.py
+	echo "Testing against unstable"
+	CONTENTWORKSHOP_URL=https://unstable.studio.learningequality.org python tests/test_chef_integration.py
+	echo "Testing against production"
+	CONTENTWORKSHOP_URL=https://studio.learningequality.org python tests/test_chef_integration.py
+
 coverage: ## check code coverage quickly with the default Python
 	pip install coverage pytest
 	coverage run --source ricecooker -m pytest
diff --git a/ricecooker/classes/nodes.py b/ricecooker/classes/nodes.py
index 3f6e794..2f28609 100644
--- a/ricecooker/classes/nodes.py
+++ b/ricecooker/classes/nodes.py
@@ -463,6 +463,12 @@ class TreeNode(Node):
         provider="",
         tags=None,
         domain_ns=None,
+        grade_levels=None,
+        resource_types=None,
+        learning_activities=None,
+        accessibility_labels=None,
+        categories=None,
+        learner_needs=None,
         **kwargs
     ):
         # Map parameters to model variables
@@ -478,6 +484,13 @@ class TreeNode(Node):
             self.questions if hasattr(self, "questions") else []
         )  # Needed for to_dict method
 
+        self.grade_levels = grade_levels or []
+        self.resource_types = resource_types or []
+        self.learning_activities = learning_activities or []
+        self.accessibility_labels = accessibility_labels or []
+        self.categories = categories or []
+        self.learner_needs = learner_needs or []
+
         super(TreeNode, self).__init__(title, **kwargs)
 
     def get_domain_namespace(self):
@@ -569,12 +582,12 @@ class TreeNode(Node):
             "copyright_holder": "",
             "questions": [],
             "extra_fields": json.dumps(self.extra_fields),
-            "grade_levels": None,
-            "resource_types": None,
-            "learning_activities": None,
-            "accessibility_categories": None,
-            "subjects": None,
-            "needs": None,
+            "grade_levels": self.grade_levels,
+            "resource_types": self.resource_types,
+            "learning_activities": self.learning_activities,
+            "accessibility_labels": self.accessibility_labels,
+            "categories": self.categories,
+            "learner_needs": self.learner_needs,
         }
 
     def validate(self):
@@ -686,12 +699,6 @@ class ContentNode(TreeNode):
         **kwargs
     ):
         self.role = role
-        self.grade_levels = grade_levels
-        self.resource_types = resource_types
-        self.learning_activities = learning_activities
-        self.accessibility_labels = accessibility_labels
-        self.categories = categories
-        self.learner_needs = learner_needs
 
         self.set_license(
             license, copyright_holder=copyright_holder, description=license_description
@@ -826,9 +833,9 @@ class ContentNode(TreeNode):
             "grade_levels": self.grade_levels,
             "resource_types": self.resource_types,
             "learning_activities": self.learning_activities,
-            "accessibility_categories": self.accessibility_labels,
-            "subjects": self.categories,
-            "needs": self.learner_needs,
+            "accessibility_labels": self.accessibility_labels,
+            "categories": self.categories,
+            "learner_needs": self.learner_needs,
         }
 
 


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index aa15b77..a0c5f30 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -171,6 +171,12 @@ def base_data(channel_domain_namespace, title):
         "license_description": None,
         "aggregator": "",  # New in ricecooker 0.6.20
         "provider": "",  # New in ricecooker 0.6.20
+        "grade_levels": [],
+        "resource_types": [],
+        "learning_activities": [],
+        "accessibility_labels": [],
+        "categories": [],
+        "learner_needs": [],
     }
 
 
@@ -259,6 +265,12 @@ def contentnode_base_data(base_data):
             "copyright_holder": "Copyright Holder",
             "license_description": None,
             "role": roles.LEARNER,
+            "grade_levels": [],
+            "resource_types": [],
+            "learning_activities": [],
+            "accessibility_labels": [],
+            "categories": [],
+            "learner_needs": [],
         }
     )
     return data
diff --git a/tests/test_chef_integration.py b/tests/test_chef_integration.py
new file mode 100644
index 0000000..1d4d9ad
--- /dev/null
+++ b/tests/test_chef_integration.py
@@ -0,0 +1,132 @@
+#!/usr/bin/env python
+import random
+import string
+
+from le_utils.constants import licenses
+
+from ricecooker.chefs import SushiChef
+from ricecooker.classes.files import AudioFile
+from ricecooker.classes.files import DocumentFile
+from ricecooker.classes.files import VideoFile
+from ricecooker.classes.licenses import get_license
+from ricecooker.classes.nodes import AudioNode
+from ricecooker.classes.nodes import DocumentNode
+from ricecooker.classes.nodes import TopicNode
+from ricecooker.classes.nodes import VideoNode
+
+
+class TestChef(SushiChef):
+    """
+    Used as an integration test by actually using Ricecooker to chef local test content into Studio.
+
+    For anything you need to test, add it to the channel created in the `construct_channel`.
+
+    Copied from examples/tutorial/sushichef.py
+    """
+
+    # Be sure we don't conflict with a channel someone else pushed before us when running this test
+    # as the channel source domain and ID determine which Channel is updated on Studio and since
+    # you'll run this with your own API key we can use this random (enough) string generator (thanks SO)
+    # to append a random set of characters to the two values.
+    def randomstring():
+        return "".join(
+            random.choice(string.ascii_uppercase + string.digits) for _ in range(8)
+        )
+
+    channel_info = {
+        "CHANNEL_SOURCE_DOMAIN": "RicecookerIntegrationTest.{}".format(
+            randomstring()
+        ),  # who is providing the content (e.g. learningequality.org)
+        "CHANNEL_SOURCE_ID": "RicecookerTests.{}".format(
+            randomstring()
+        ),  # channel's unique id
+        "CHANNEL_TITLE": "Ricecooker Testing!",
+        "CHANNEL_LANGUAGE": "en",
+    }
+
+    # CONSTRUCT CHANNEL
+    def construct_channel(self, *args, **kwargs):
+        """
+        This method is reponsible for creating a `ChannelNode` object and
+        populating it with `TopicNode` and `ContentNode` children.
+        """
+        # Create channel
+        ########################################################################
+        channel = self.get_channel(*args, **kwargs)  # uses self.channel_info
+
+        # Create topics to add to your channel
+        ########################################################################
+        # Here we are creating a topic named 'Example Topic'
+        exampletopic = TopicNode(source_id="topic-1", title="Example Topic")
+
+        # Now we are adding 'Example Topic' to our channel
+        channel.add_child(exampletopic)
+
+        # You can also add subtopics to topics
+        # Here we are creating a subtopic named 'Example Subtopic'
+        examplesubtopic = TopicNode(source_id="topic-1a", title="Example Subtopic")
+
+        # Now we are adding 'Example Subtopic' to our 'Example Topic'
+        exampletopic.add_child(examplesubtopic)
+
+        # Content
+        # You can add documents (pdfs and ePubs), videos, audios, and other content
+        # let's create a document file called 'Example PDF'
+        document_file = DocumentFile(path="http://www.pdf995.com/samples/pdf.pdf")
+        examplepdf = DocumentNode(
+            title="Example PDF",
+            source_id="example-pdf",
+            files=[document_file],
+            license=get_license(licenses.PUBLIC_DOMAIN),
+        )
+
+        # We are also going to add a video file called 'Example Video'
+        video_file = VideoFile(
+            path="https://ia600209.us.archive.org/27/items/RiceChef/Rice Chef.mp4"
+        )
+        fancy_license = get_license(
+            licenses.SPECIAL_PERMISSIONS,
+            description="Special license for ricecooker fans only.",
+            copyright_holder="The chef video makers",
+        )
+        examplevideo = VideoNode(
+            title="Example Video",
+            source_id="example-video",
+            files=[video_file],
+            license=fancy_license,
+        )
+
+        # Finally, we are creating an audio file called 'Example Audio'
+        audio_file = AudioFile(
+            path="https://ia802508.us.archive.org/5/items/testmp3testfile/mpthreetest.mp3"
+        )
+        exampleaudio = AudioNode(
+            title="Example Audio",
+            source_id="example-audio",
+            files=[audio_file],
+            license=get_license(licenses.PUBLIC_DOMAIN),
+        )
+
+        # Now that we have our files, let's add them to our channel
+        channel.add_child(examplepdf)  # Adding 'Example PDF' to your channel
+        exampletopic.add_child(
+            examplevideo
+        )  # Adding 'Example Video' to 'Example Topic'
+        examplesubtopic.add_child(
+            exampleaudio
+        )  # Adding 'Example Audio' to 'Example Subtopic'
+
+        # the `construct_channel` method returns a ChannelNode that will be
+        # processed by the ricecooker framework
+        return channel
+
+
+if __name__ == "__main__":
+    """
+    This code will run when the sushi chef is called from the command line.
+    """
+    chef = TestChef()
+    print(
+        "Note that you will need your Studio API key for this. It will upload to your account."
+    )
+    chef.main()
diff --git a/tests/test_data.py b/tests/test_data.py
index d50d03b..4816941 100644
--- a/tests/test_data.py
+++ b/tests/test_data.py
@@ -145,10 +145,26 @@ def test_video_to_dict(video, video_data):
     video_dict.pop("files")
     expected_files = video_data.pop("files")
     video_data["extra_fields"] = json.dumps(video_data["extra_fields"])
+
     assert video.files == expected_files, "Video files do not match"
+
     for key, _ in video_data.items():
         assert key in video_dict, "Key {} is not found in to_dict method".format(key)
+
+    list_type_keys = [
+        "grade_levels",
+        "learner_needs",
+        "accessibility_labels",
+        "categories",
+        "learning_activities",
+        "resource_types",
+    ]
     for key, value in video_dict.items():
+        if key in list_type_keys:
+            assert isinstance(value, list), "{} should be a list, but it's {}".format(
+                key, value
+            )
+
         assert value == video_data.get(key), "Mismatched {}: {} != {}".format(
             key, value, video_data[key]
         )