diff --git a/contentcuration/contentcuration/tests/test_exportchannel.py b/contentcuration/contentcuration/tests/test_exportchannel.py index 29ce8d1c69..291dcb5679 100644 --- a/contentcuration/contentcuration/tests/test_exportchannel.py +++ b/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 diff --git a/contentcuration/contentcuration/tests/testdata.py b/contentcuration/contentcuration/tests/testdata.py index 50895337b1..6d91292154 100644 --- a/contentcuration/contentcuration/tests/testdata.py +++ b/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'], diff --git a/contentcuration/contentcuration/utils/nodes.py b/contentcuration/contentcuration/utils/nodes.py index 668d5ff81d..2a1c2dad1a 100644 --- a/contentcuration/contentcuration/utils/nodes.py +++ b/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 diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py index c82b959850..c00d13b639 100644 --- a/contentcuration/contentcuration/utils/publish.py +++ b/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: diff --git a/contentcuration/contentcuration/viewsets/contentnode.py b/contentcuration/contentcuration/viewsets/contentnode.py index cc6ad0727f..00c3bc5601 100644 --- a/contentcuration/contentcuration/viewsets/contentnode.py +++ b/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