Fix mastery criterion publishing

contentcuration/contentcuration/tests/test_exportchannel.py

@@ -12,6 +12,7 @@
 from kolibri_content import models as kolibri_models
 from kolibri_content.router import get_active_content_database
 from kolibri_content.router import set_active_content_database
+from le_utils.constants import exercises
 from le_utils.constants.labels import accessibility_categories
 from le_utils.constants.labels import learning_activities
 from le_utils.constants.labels import levels
@@ -96,15 +97,39 @@ def setUp(self):
         new_video.parent = self.content_channel.main_tree
         new_video.save()
 
+        # Add a node to test new style mastery models.
+        extra_fields = {
+            "options": {
+                "completion_criteria": {
+                    "model": "mastery",
+                    "threshold": {
+                        "m": 1,
+                        "n": 2,
+                        "mastery_model": exercises.M_OF_N,
+                    }
+                }
+            }
+        }
+        current_exercise = cc.ContentNode.objects.filter(kind_id="exercise").first()
+
+        new_exercise = create_node({'kind_id': 'exercise', 'title': 'Mastery test', 'extra_fields': extra_fields})
+        new_exercise.complete = True
+        new_exercise.parent = current_exercise.parent
+        new_exercise.save()
+        for ai in current_exercise.assessment_items.all():
+            ai.id = None
+            ai.contentnode = new_exercise
+            ai.save()
+
         first_topic = self.content_channel.main_tree.get_descendants().first()
 
         # Add a publishable topic to ensure it does not inherit but that its children do
-        new_node = create_node({'kind_id': 'topic', 'title': 'Disinherited topic', 'children': []})
+        new_node = create_node({'kind_id': 'topic', 'title': 'Disinherited topic'})
         new_node.complete = True
         new_node.parent = first_topic
         new_node.save()
 
-        new_video = create_node({'kind_id': 'video', 'title': 'Inheriting video', 'children': []})
+        new_video = create_node({'kind_id': 'video', 'title': 'Inheriting video'})
         new_video.complete = True
         new_video.parent = new_node
         new_video.save()
@@ -241,9 +266,14 @@ def test_channel_icon_encoding(self):
         self.assertIsNotNone(self.content_channel.icon_encoding)
 
     def test_assessment_metadata(self):
-        asm = kolibri_models.AssessmentMetaData.objects.first()
-        self.assertTrue(isinstance(json.loads(asm.assessment_item_ids), list))
-        self.assertTrue(isinstance(json.loads(asm.mastery_model), dict))
+        for i, exercise in enumerate(kolibri_models.ContentNode.objects.filter(kind="exercise")):
+            asm = exercise.assessmentmetadata.first()
+            self.assertTrue(isinstance(json.loads(asm.assessment_item_ids), list))
+            mastery = json.loads(asm.mastery_model)
+            self.assertTrue(isinstance(mastery, dict))
+            self.assertEqual(mastery["type"], exercises.DO_ALL if i == 0 else exercises.M_OF_N)
+            self.assertEqual(mastery["m"], 3 if i == 0 else 1)
+            self.assertEqual(mastery["n"], 3 if i == 0 else 2)
 
     def test_inherited_language(self):
         first_topic_node_id = self.content_channel.main_tree.get_descendants().first().node_id

contentcuration/contentcuration/tests/testdata.py

@@ -112,7 +112,7 @@ def node_json(data):
     return node_data
 
 
-def node(data, parent=None):
+def node(data, parent=None):  # noqa: C901
     new_node = None
     # Create topics
     if 'node_id' not in data:
@@ -155,12 +155,15 @@ def node(data, parent=None):
 
     # Create exercises
     elif data['kind_id'] == "exercise":
-        extra_fields = {
-            'mastery_model': data['mastery_model'],
-            'randomize': True,
-            'm': data.get('m') or 0,
-            'n': data.get('n') or 0
-        }
+        if "extra_fields" in data:
+            extra_fields = data["extra_fields"]
+        else:
+            extra_fields = {
+                'mastery_model': data['mastery_model'],
+                'randomize': True,
+                'm': data.get('m') or 0,
+                'n': data.get('n') or 0
+            }
         new_node = cc.ContentNode(
             kind=exercise(),
             parent=parent,
@@ -174,7 +177,7 @@ def node(data, parent=None):
         )
 
         new_node.save()
-        for assessment_item in data['assessment_items']:
+        for assessment_item in data.get('assessment_items', []):
             ai = cc.AssessmentItem(
                 contentnode=new_node,
                 assessment_id=assessment_item['assessment_id'],

contentcuration/contentcuration/utils/nodes.py

@@ -15,6 +15,7 @@
 from django.db.models import Count
 from django.db.models import Sum
 from django.utils import timezone
+from le_utils.constants import completion_criteria
 from le_utils.constants import content_kinds
 from le_utils.constants import format_presets
 
@@ -427,3 +428,22 @@ def calculate_resource_size(node, force=False):
             report_exception(e)
 
     return size, False
+
+
+def migrate_extra_fields(extra_fields):
+    if not isinstance(extra_fields, dict):
+        return extra_fields
+    m = extra_fields.pop("m", None)
+    n = extra_fields.pop("n", None)
+    mastery_model = extra_fields.pop("mastery_model", None)
+    if not extra_fields.get("options", {}).get("completion_criteria", {}) and mastery_model is not None:
+        extra_fields["options"] = extra_fields.get("options", {})
+        extra_fields["options"]["completion_criteria"] = {
+            "threshold": {
+                "m": m,
+                "n": n,
+                "mastery_model": mastery_model,
+            },
+            "model": completion_criteria.MASTERY,
+        }
+    return extra_fields

contentcuration/contentcuration/utils/publish.py

@@ -44,6 +44,7 @@
 from contentcuration.utils.cache import delete_public_channel_cache_keys
 from contentcuration.utils.files import create_thumbnail_from_base64
 from contentcuration.utils.files import get_thumbnail_encoding
+from contentcuration.utils.nodes import migrate_extra_fields
 from contentcuration.utils.parser import extract_value
 from contentcuration.utils.parser import load_json_string
 from contentcuration.utils.sentry import report_exception
@@ -471,21 +472,16 @@ def create_perseus_exercise(ccnode, kolibrinode, exercise_data, user_id=None):
 def process_assessment_metadata(ccnode, kolibrinode):
     # Get mastery model information, set to default if none provided
     assessment_items = ccnode.assessment_items.all().order_by('order')
-    exercise_data = ccnode.extra_fields if ccnode.extra_fields else {}
-    if isinstance(exercise_data, basestring):
-        exercise_data = json.loads(exercise_data)
-    randomize = exercise_data.get('randomize') if exercise_data.get('randomize') is not None else True
+    extra_fields = ccnode.extra_fields
+    if isinstance(extra_fields, basestring):
+        extra_fields = json.loads(extra_fields)
+    extra_fields = migrate_extra_fields(extra_fields) or {}
+    randomize = extra_fields.get('randomize') if extra_fields.get('randomize') is not None else True
     assessment_item_ids = [a.assessment_id for a in assessment_items]
 
-    exercise_data_type = ""
-    if exercise_data.get('mastery_model'):
-        exercise_data_type = exercise_data.get('mastery_model')
-    if (
-        exercise_data.get('option') and
-        exercise_data.get('option').get('completion_criteria') and
-        exercise_data.get('option').get('completion_criteria').get('mastery_model')
-    ):
-        exercise_data_type = exercise_data.get('option').get('completion_criteria').get('mastery_model')
+    exercise_data = extra_fields.get('options').get('completion_criteria').get('threshold')
+
+    exercise_data_type = exercise_data.get('mastery_model', "")
 
     mastery_model = {'type': exercise_data_type or exercises.M_OF_N}
     if mastery_model['type'] == exercises.M_OF_N:

contentcuration/contentcuration/viewsets/contentnode.py

@@ -53,6 +53,7 @@
 from contentcuration.models import UUIDField
 from contentcuration.tasks import calculate_resource_size_task
 from contentcuration.utils.nodes import calculate_resource_size
+from contentcuration.utils.nodes import migrate_extra_fields
 from contentcuration.viewsets.base import BulkListSerializer
 from contentcuration.viewsets.base import BulkModelSerializer
 from contentcuration.viewsets.base import BulkUpdateMixin
@@ -257,7 +258,7 @@ def to_representation(self, value):
     def to_internal_value(self, data):
         try:
             data = int(data)
-        except(ValueError, TypeError):
+        except (ValueError, TypeError):
             if not isinstance(data, (str, dict)):
                 self.fail("Must be either an integer, string or dictionary")
         return data
@@ -283,25 +284,6 @@ def update(self, instance, validated_data):
         return instance
 
 
-def _migrate_extra_fields(extra_fields):
-    if not isinstance(extra_fields, dict):
-        return extra_fields
-    m = extra_fields.pop("m", None)
-    n = extra_fields.pop("n", None)
-    mastery_model = extra_fields.pop("mastery_model", None)
-    if not extra_fields.get("options", {}).get("completion_criteria", {}) and mastery_model is not None:
-        extra_fields["options"] = extra_fields.get("options", {})
-        extra_fields["options"]["completion_criteria"] = {
-            "threshold": {
-                "m": m,
-                "n": n,
-                "mastery_model": mastery_model,
-            },
-            "model": completion_criteria.MASTERY,
-        }
-    return extra_fields
-
-
 class ExtraFieldsOptionsSerializer(JSONFieldDictSerializer):
     modality = ChoiceField(choices=(("QUIZ", "Quiz"),), allow_null=True, required=False)
     completion_criteria = CompletionCriteriaSerializer(required=False)
@@ -313,7 +295,7 @@ class ExtraFieldsSerializer(JSONFieldDictSerializer):
     suggested_duration_type = ChoiceField(choices=[completion_criteria.TIME, completion_criteria.APPROX_TIME], allow_null=True, required=False)
 
     def update(self, instance, validated_data):
-        instance = _migrate_extra_fields(instance)
+        instance = migrate_extra_fields(instance)
         return super(ExtraFieldsSerializer, self).update(instance, validated_data)
 
 
@@ -472,7 +454,7 @@ def retrieve_thumbail_src(item):
 def consolidate_extra_fields(item):
     extra_fields = item.get("extra_fields")
     if item["kind"] == content_kinds.EXERCISE:
-        extra_fields = _migrate_extra_fields(extra_fields)
+        extra_fields = migrate_extra_fields(extra_fields)
 
     return extra_fields