diff --git a/contentcuration/contentcuration/tests/testdata.py b/contentcuration/contentcuration/tests/testdata.py index 9f93c5ab79..962d2e0a5b 100644 --- a/contentcuration/contentcuration/tests/testdata.py +++ b/contentcuration/contentcuration/tests/testdata.py @@ -288,13 +288,16 @@ def create_temp_file(filebytes, preset="document", ext="pdf", original_filename= ) -def create_studio_file(filebytes, preset="document", ext="pdf", original_filename=None): +def create_studio_file( + filebytes, preset="document", ext="pdf", original_filename=None, uploaded_by=None +): """ Create a file with contents of `filebytes` and the associated cc.File object for it. :param filebytes: The data to be stored in the file (as bytes) :param preset: String identifying the format preset (defaults to ``document``) :param ext: File extension, omitting the initial period :param original_filename: Original filename (needed for exercise_images) + :param uploaded_by: User the file should appear to have been uploaded by Returns a dict containing the following: - name (str): the filename within the content storage system (= md5 hash of the contents + .ext ) - data (bytes): file content (echo of `filebytes`) @@ -335,6 +338,7 @@ def create_studio_file(filebytes, preset="document", ext="pdf", original_filenam preset=preset, original_filename=original_filename, file_on_disk=storage_file_path, + uploaded_by=uploaded_by, ) return { @@ -414,7 +418,7 @@ def create_test_file(filebytes, ext="pdf"): ] -def fileobj_exercise_image(size=(100, 100), color="red"): +def fileobj_exercise_image(size=(100, 100), color="red", uploaded_by=None): """ Create a generic exercise image file in storage and return a File model pointing to it. """ @@ -422,12 +426,15 @@ def fileobj_exercise_image(size=(100, 100), color="red"): buffer = BytesIO() image.save(buffer, "JPEG") temp_file_dict = create_studio_file( - buffer.getvalue(), preset=format_presets.EXERCISE_IMAGE, ext="jpg" + buffer.getvalue(), + preset=format_presets.EXERCISE_IMAGE, + ext="jpg", + uploaded_by=uploaded_by, ) return temp_file_dict["db_file"] -def fileobj_exercise_graphie(original_filename=None): +def fileobj_exercise_graphie(original_filename=None, uploaded_by=None): """ Create an graphi exercise image file in storage and return a File model pointing to it. """ @@ -439,6 +446,7 @@ def fileobj_exercise_graphie(original_filename=None): preset=format_presets.EXERCISE_GRAPHIE, ext="graphie", original_filename=original_filename or "theoriginalfilename", + uploaded_by=uploaded_by, ) return temp_file_dict["db_file"] diff --git a/contentcuration/contentcuration/tests/utils/qti/test_ingest.py b/contentcuration/contentcuration/tests/utils/qti/test_ingest.py new file mode 100644 index 0000000000..43eef6ffe9 --- /dev/null +++ b/contentcuration/contentcuration/tests/utils/qti/test_ingest.py @@ -0,0 +1,74 @@ +import unittest + +from contentcuration.tests.utils.qti.test_validation import _item_xml +from contentcuration.tests.utils.qti.test_validation import VALID_CHOICE_ITEM +from contentcuration.utils.assessment.qti.ingest import convert_legacy_question_to_qti +from contentcuration.utils.assessment.qti.ingest import ( + find_perseus_custom_interaction_path, +) +from contentcuration.utils.assessment.qti.ingest import ( + strip_content_storage_placeholder, +) +from contentcuration.utils.assessment.qti.media import get_qti_media_references +from contentcuration.utils.assessment.qti.validation import validate_qti_item + + +class StripContentStoragePlaceholderTests(unittest.TestCase): + def test_strips_placeholder_leaving_bare_filename(self): + text = "Look: ![](${☣ CONTENTSTORAGE}/abc123.png)" + self.assertEqual( + strip_content_storage_placeholder(text), "Look: ![](abc123.png)" + ) + + +class ConvertLegacyQuestionToQTITests(unittest.TestCase): + def test_convert_legacy_question_to_qti_strips_placeholder_from_question_and_answers( + self, + ): + question_data = { + "type": "multiple_selection", + "assessment_id": "abf45e8fd7f151adb1b3df2d751e945e", + "question": "Which is red? ![](${☣ CONTENTSTORAGE}/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.png)", # noqa + "answers": '[{"answer": "Apple ![](${☣ CONTENTSTORAGE}/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.png)", "correct": true, "order": 0}, {"answer": "Sky", "correct": false, "order": 1}]', # noqa + "randomize": False, + } + result = convert_legacy_question_to_qti(question_data) + validation_result = validate_qti_item(result.xml) + self.assertTrue(validation_result.is_valid, validation_result.errors) + self.assertEqual( + get_qti_media_references(result.xml), + { + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.png", + "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.png", + }, + ) + + +def _custom_interaction_item_xml(data_type, path_attr, path_value): + return _item_xml( + "item1", + "t", + "", + '' + % (data_type, path_attr, path_value), + ) + + +class FindPerseusCustomInteractionPathTests(unittest.TestCase): + def test_find_perseus_custom_interaction_path_detects_wrapper(self): + raw_data = _custom_interaction_item_xml( + "perseus", "data-perseus-path", "cccccccccccccccccccccccccccccccc.json" + ) + self.assertEqual( + find_perseus_custom_interaction_path(raw_data), + "cccccccccccccccccccccccccccccccc.json", + ) + + def test_find_perseus_custom_interaction_path_returns_none_when_absent(self): + self.assertIsNone(find_perseus_custom_interaction_path(VALID_CHOICE_ITEM)) + + def test_find_perseus_custom_interaction_path_ignores_other_vendor_types(self): + raw_data = _custom_interaction_item_xml( + "other-vendor", "data-other-path", "cccccccccccccccccccccccccccccccc.json" + ) + self.assertIsNone(find_perseus_custom_interaction_path(raw_data)) diff --git a/contentcuration/contentcuration/tests/utils/test_create_exercises.py b/contentcuration/contentcuration/tests/utils/test_create_exercises.py new file mode 100644 index 0000000000..5d081d3517 --- /dev/null +++ b/contentcuration/contentcuration/tests/utils/test_create_exercises.py @@ -0,0 +1,90 @@ +from django.core.exceptions import ValidationError as DjangoValidationError +from le_utils.constants import exercises + +from contentcuration.tests.base import StudioTestCase +from contentcuration.tests.testdata import channel +from contentcuration.tests.utils.qti.test_validation import _item_xml +from contentcuration.views.internal import create_exercises + + +class CreateExercisesInvalidQTITestCase(StudioTestCase): + def test_invalid_qti_raw_data_raises_node_validation_error(self): + node = channel().main_tree + questions = [ + { + "assessment_id": "dddddddddddddddddddddddddddddddd", + "type": exercises.QTI, + "raw_data": "this is not valid QTI", + "files": [], + "source_url": None, + "randomize": False, + } + ] + with self.assertRaises(DjangoValidationError): + create_exercises(self.admin_user, node, questions) + + def test_unsupported_legacy_question_type_raises_node_validation_error(self): + node = channel().main_tree + questions = [ + { + "assessment_id": "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee", + "type": "not_a_real_question_type", + "question": "What?", + "answers": "[]", + "files": [], + "source_url": None, + "randomize": False, + } + ] + with self.assertRaises(DjangoValidationError): + create_exercises(self.admin_user, node, questions) + + def test_qti_raw_data_referencing_missing_file_raises_node_validation_error(self): + node = channel().main_tree + raw_data = _item_xml( + "item_1", + "Sample Item", + '' + "choice_0" + "", + 'Pick. ' + 'diagram' + 'A' + 'B' + "", + ) + questions = [ + { + "assessment_id": "ffffffffffffffffffffffffffffffff", + "type": exercises.QTI, + "raw_data": raw_data, + "files": [], + "source_url": None, + "randomize": False, + } + ] + with self.assertRaises(DjangoValidationError): + create_exercises(self.admin_user, node, questions) + + def test_qti_wrapping_perseus_custom_interaction_rejected(self): + node = channel().main_tree + raw_data = _item_xml( + "item_2", + "Sample Item", + "", + '', + ) + questions = [ + { + "assessment_id": "cccccccccccccccccccccccccccccccc", + "type": exercises.QTI, + "raw_data": raw_data, + "files": [], + "source_url": None, + "randomize": False, + } + ] + with self.assertRaises(DjangoValidationError): + create_exercises(self.admin_user, node, questions) diff --git a/contentcuration/contentcuration/tests/views/test_views_internal.py b/contentcuration/contentcuration/tests/views/test_views_internal.py index f503ebe538..5d791f703e 100644 --- a/contentcuration/contentcuration/tests/views/test_views_internal.py +++ b/contentcuration/contentcuration/tests/views/test_views_internal.py @@ -33,12 +33,15 @@ from ..testdata import fileobj_video from ..testdata import thumbnail_bytes from ..testdata import user +from ..utils.qti.test_validation import _item_xml from contentcuration import ricecooker_versions as rc from contentcuration.constants import completion_criteria as studio_completion_criteria from contentcuration.db.models.manager import ALLOWED_OVERRIDES from contentcuration.db.models.manager import EDIT_ALLOWED_OVERRIDES from contentcuration.models import Channel from contentcuration.models import ContentNode +from contentcuration.models import File +from contentcuration.utils.assessment.qti.validation import validate_qti_item from contentcuration.views import internal @@ -293,10 +296,13 @@ def setUp(self): # get our random data from mixer random_data = mixer.blend(SampleContentNodeDataSchema) - # a vanilla image file associated with question - self.exercise_image = fileobj_exercise_image() + # a vanilla image file associated with question - uploaded_by matches + # self.admin_client() to mirror ricecooker's real upload_url flow, + # where the same authenticated user stages the file before referencing + # it in create_exercises + self.exercise_image = fileobj_exercise_image(uploaded_by=self.admin_user) # a perseus image file associated with question - self.exercise_graphie = fileobj_exercise_graphie() + self.exercise_graphie = fileobj_exercise_graphie(uploaded_by=self.admin_user) self.title = random_data.title self.sample_data = { "root_id": self.root_node.id, @@ -332,7 +338,7 @@ def setUp(self): "question": u"Which numbers are even?\n\nTest local image include: ![](${☣ CONTENTSTORAGE}/%s)" % self.exercise_image.filename(), "hints": "[]", - "answers": '[{"answer": "1", "correct": false, "order": 0}, {"answer": "2", "correct": True, "order": 1}, {"answer": "3", "correct": false, "order": 2}, {"answer": "4", "correct": true, "order": 3}, {"answer": "5", "correct": false, "order": 4}]', # noqa + "answers": '[{"answer": "1", "correct": false, "order": 0}, {"answer": "2", "correct": true, "order": 1}, {"answer": "3", "correct": false, "order": 2}, {"answer": "4", "correct": true, "order": 3}, {"answer": "5", "correct": false, "order": 4}]', # noqa "raw_data": "", "source_url": None, "randomize": False, @@ -473,6 +479,188 @@ def test_duplicate_assessment_item_returns_400_status_code(self): response.status_code ) + def test_native_qti_item_persisted_and_media_mapped(self): + """ + A native QTI question (raw_data XML + declared media) is persisted + verbatim as an exercises.QTI assessment item, and its media is mapped + by reusing the File row ricecooker's file_upload_url request already + created for it, without leaving stray/duplicate File rows behind. + """ + assessment_id = "1" * 32 + qti_exercise_image = fileobj_exercise_image( + size=(50, 50), color="blue", uploaded_by=self.admin_user + ) + qti_raw_data = _item_xml( + "item_1", + "Sample Item", + '' + "choice_0" + "", + 'Pick the diagram. ' + 'diagram' + 'Option A' + 'Option B' + "" % qti_exercise_image.filename(), + ) + self.sample_data["content_data"][0]["questions"].append( + { + "assessment_id": assessment_id, + "type": "QTI", + "files": [ + { + "size": qti_exercise_image.file_size, + "preset": "exercise_image", + "filename": qti_exercise_image.filename(), + "original_filename": None, + "language": None, + "source_url": None, + } + ], + "question": "", + "hints": "[]", + "answers": "[]", + "raw_data": qti_raw_data, + "source_url": None, + "randomize": False, + } + ) + + response = self._make_request() + assert response.status_code == 200, response.content + + c = ContentNode.objects.get(title=self.title) + qti_item = c.assessment_items.get(assessment_id=assessment_id) + self.assertEqual(qti_item.type, exercises.QTI) + self.assertEqual(qti_item.raw_data, qti_raw_data) + self.assertEqual(qti_item.files.count(), 1) + attached_file = qti_item.files.first() + self.assertEqual(attached_file.filename(), qti_exercise_image.filename()) + # The File row is the exact one created when the image was staged + # (i.e. by ricecooker's file_upload_url request), not a new row - so + # its id and original_filename are preserved rather than overwritten. + self.assertEqual(attached_file.id, qti_exercise_image.id) + self.assertEqual( + attached_file.original_filename, qti_exercise_image.original_filename + ) + # No stray/duplicate File row is left behind for this checksum. + self.assertEqual( + File.objects.filter(checksum=qti_exercise_image.checksum).count(), 1 + ) + + def test_legacy_multiple_selection_question_converted_to_qti_on_ingest(self): + """ + A legacy multiple_selection question is converted to schema-valid QTI + on ingest, and its media stays attached through the conversion. + """ + response = self._make_request() + assert response.status_code == 200, response.content + + c = ContentNode.objects.get(title=self.title) + item = c.assessment_items.get(assessment_id="abf45e8fd7f151adb1b3df2d751e945e") + self.assertEqual(item.type, exercises.QTI) + result = validate_qti_item(item.raw_data) + self.assertTrue(result.is_valid, result.errors) + self.assertIn("Which numbers are even?", item.raw_data) + self.assertEqual(item.files.count(), 1) + attached_file = item.files.first() + self.assertEqual(attached_file.filename(), self.exercise_image.filename()) + # The pre-staged setUp fixture File is reused directly, not duplicated. + self.assertEqual(attached_file.id, self.exercise_image.id) + self.assertEqual( + File.objects.filter(checksum=self.exercise_image.checksum).count(), 1 + ) + + def test_invalid_qti_raw_data_returns_400_and_creates_nothing(self): + """ + A schema-invalid QTI upload is rejected with a 400 and rolls back the + whole request - it must not partially create the node/other items. + """ + self.sample_data["content_data"][0]["questions"].append( + { + "assessment_id": "2" * 32, + "type": "QTI", + "files": [], + "question": "", + "hints": "[]", + "answers": "[]", + "raw_data": "not schema compliant", + "source_url": None, + "randomize": False, + } + ) + response = self._make_request() + assert response.status_code == 400, response.content + # whole-request creation is atomic: the otherwise-valid node/questions + # from setUp must not have been persisted alongside the invalid one + assert not ContentNode.objects.filter(title=self.title).exists() + + def test_perseus_custom_interaction_in_qti_rejected(self): + """ + A QTI item wrapping a Perseus custom interaction is a categorical + mistake on ricecooker's part (it should upload raw Perseus content as + a perseus_question instead) - reject it outright rather than trying + to unwrap it. + """ + assessment_id = "3" * 32 + wrapped_perseus_json = self.sample_data["content_data"][0]["questions"][1][ + "raw_data" + ] + perseus_payload_file = create_studio_file(wrapped_perseus_json, ext="json") + perseus_payload_filename = perseus_payload_file["name"] + raw_data = _item_xml( + "item_2", + "Wrapped Perseus Item", + "", + '' % perseus_payload_filename, + ) + self.sample_data["content_data"][0]["questions"].append( + { + "assessment_id": assessment_id, + "type": "QTI", + "files": [ + { + "size": len(wrapped_perseus_json.encode("utf-8")), + "preset": "document", + "filename": perseus_payload_filename, + "original_filename": None, + "language": None, + "source_url": None, + } + ], + "question": "", + "hints": "[]", + "answers": "[]", + "raw_data": raw_data, + "source_url": None, + "randomize": False, + } + ) + + response = self._make_request() + assert response.status_code == 400, response.content + # whole-request creation is atomic: the otherwise-valid node/questions + # from setUp must not have been persisted alongside the rejected one + assert not ContentNode.objects.filter(title=self.title).exists() + + def test_raw_perseus_question_passthrough_unchanged(self): + """ + Regression guard: a raw (non-wrapped) perseus_question is completely + unaffected by the QTI ingest refactor. + """ + response = self._make_request() + assert response.status_code == 200, response.content + + c = ContentNode.objects.get(title=self.title) + item = c.assessment_items.get(assessment_id="98856e24d53b57ea9023782ab6018767") + self.assertEqual(item.type, exercises.PERSEUS_QUESTION) + self.assertIn("radio 1", item.raw_data) + self.assertEqual(item.files.count(), 1) + self.assertEqual( + item.files.first().filename(), self.exercise_graphie.filename() + ) + class ApiAddExerciseExtraFieldsMigrationTestCase(StudioTestCase): """ diff --git a/contentcuration/contentcuration/tests/viewsets/test_assessmentitem.py b/contentcuration/contentcuration/tests/viewsets/test_assessmentitem.py index 891d66c2f8..1f3d1330f8 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_assessmentitem.py +++ b/contentcuration/contentcuration/tests/viewsets/test_assessmentitem.py @@ -84,6 +84,28 @@ def test_create_assessmentitem(self): except models.AssessmentItem.DoesNotExist: self.fail("AssessmentItem was not created") + def test_create_perseus_question_with_raw_data(self): + self.client.force_authenticate(user=self.user) + assessmentitem = self.assessmentitem_metadata + assessmentitem["type"] = exercises.PERSEUS_QUESTION + assessmentitem["raw_data"] = json.dumps({"question": {"content": "hi"}}) + response = self.sync_changes( + [ + generate_create_event( + [assessmentitem["contentnode"], assessmentitem["assessment_id"]], + ASSESSMENTITEM, + assessmentitem, + channel_id=self.channel.id, + ) + ], + ) + self.assertEqual(response.status_code, 200, response.content) + ai = models.AssessmentItem.objects.get( + assessment_id=assessmentitem["assessment_id"] + ) + self.assertEqual(ai.type, exercises.PERSEUS_QUESTION) + self.assertEqual(ai.raw_data, assessmentitem["raw_data"]) + def test_create_assessmentitem_no_node_permission(self): self.client.force_authenticate(user=self.user) new_channel = testdata.channel() diff --git a/contentcuration/contentcuration/utils/assessment/qti/ingest.py b/contentcuration/contentcuration/utils/assessment/qti/ingest.py new file mode 100644 index 0000000000..1a03e641fb --- /dev/null +++ b/contentcuration/contentcuration/utils/assessment/qti/ingest.py @@ -0,0 +1,54 @@ +import json +from typing import Optional + +from le_utils.constants import exercises +from lxml import etree + +from contentcuration.utils.assessment.qti.convert import ( + convert_legacy_assessment_item_to_qti, +) +from contentcuration.utils.assessment.qti.convert import LegacyAssessmentItem +from contentcuration.utils.assessment.qti.convert import QTIConversionResult +from contentcuration.utils.assessment.qti.validation import parse_qti_xml + + +CONTENT_STORAGE_PREFIX = exercises.CONTENT_STORAGE_FORMAT.format("") + +_PERSEUS_CUSTOM_INTERACTION_XPATH = etree.XPath( + "//*[local-name()='qti-custom-interaction' and @data-type='perseus']" +) + + +def strip_content_storage_placeholder(text): + return text.replace(CONTENT_STORAGE_PREFIX, "") + + +def convert_legacy_question_to_qti(question_data: dict) -> QTIConversionResult: + answers = json.loads(question_data.get("answers") or "[]") + for answer in answers: + if isinstance(answer.get("answer"), str): + answer["answer"] = strip_content_storage_placeholder(answer["answer"]) + + item = LegacyAssessmentItem( + type=question_data["type"], + question=strip_content_storage_placeholder(question_data.get("question") or ""), + answers=answers, + randomize=question_data.get("randomize") or False, + assessment_id=question_data["assessment_id"], + title=question_data.get("assessment_id"), + language=question_data.get("language") or "en", + ) + return convert_legacy_assessment_item_to_qti(item) + + +def find_perseus_custom_interaction_path(raw_data) -> Optional[str]: + if isinstance(raw_data, str): + raw_data = raw_data.encode("utf-8") + try: + doc = parse_qti_xml(raw_data) + except etree.XMLSyntaxError: + return None + matches = _PERSEUS_CUSTOM_INTERACTION_XPATH(doc) + if not matches: + return None + return matches[0].get("data-perseus-path") diff --git a/contentcuration/contentcuration/views/internal.py b/contentcuration/contentcuration/views/internal.py index 09893e9e0c..952032ed63 100644 --- a/contentcuration/contentcuration/views/internal.py +++ b/contentcuration/contentcuration/views/internal.py @@ -1,6 +1,7 @@ import json import logging from collections import namedtuple +from types import SimpleNamespace from distutils.version import LooseVersion from django.core.exceptions import ObjectDoesNotExist @@ -14,6 +15,7 @@ from django.http import HttpResponseServerError from django.http import JsonResponse from le_utils.constants import content_kinds +from le_utils.constants import exercises from le_utils.constants import roles from le_utils.constants.labels.accessibility_categories import ( ACCESSIBILITYCATEGORIESLIST, @@ -29,6 +31,7 @@ from rest_framework.decorators import api_view from rest_framework.decorators import authentication_classes from rest_framework.decorators import permission_classes +from rest_framework.exceptions import ValidationError as DRFValidationError from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from sentry_sdk import capture_exception @@ -37,7 +40,6 @@ from contentcuration.api import write_file_to_storage from contentcuration.constants import completion_criteria from contentcuration.decorators import delay_user_storage_calculation -from contentcuration.models import AssessmentItem from contentcuration.models import Change from contentcuration.models import Channel from contentcuration.models import ContentNode @@ -48,6 +50,10 @@ from contentcuration.serializers import GetTreeDataSerializer from contentcuration.tasks import apply_channel_changes_task from contentcuration.tasks import generatenodediff_task +from contentcuration.utils.assessment.qti.ingest import convert_legacy_question_to_qti +from contentcuration.utils.assessment.qti.ingest import ( + find_perseus_custom_interaction_path, +) from contentcuration.utils.files import get_file_diff from contentcuration.utils.garbage_collect import get_deleted_chefs_root from contentcuration.utils.nodes import map_files_to_assessment_item @@ -55,6 +61,7 @@ from contentcuration.utils.nodes import map_files_to_slideshow_slide_item from contentcuration.utils.nodes import migrate_extra_fields from contentcuration.utils.sentry import report_exception +from contentcuration.viewsets.assessmentitem import AssessmentItemSerializer from contentcuration.viewsets.sync.constants import CHANNEL from contentcuration.viewsets.sync.utils import generate_publish_event from contentcuration.viewsets.sync.utils import generate_update_event @@ -929,7 +936,8 @@ def create_node(node_data, parent_node, sort_order): # noqa: C901 def create_exercises(user, node, data): - """ Generate exercise from data """ + """Generate exercise from data, delegating item construction and QTI schema + validation to AssessmentItemSerializer.""" # First check that all assessment_ids are unique within the node assessment_ids = [question.get("assessment_id") for question in data] if len(assessment_ids) != len(set(assessment_ids)): @@ -937,25 +945,72 @@ def create_exercises(user, node, data): "Duplicate assessment_ids found in node {}".format(node.node_id) ) + item_dicts = [] + for order, question in enumerate(data): + try: + item_dicts.append(_build_assessment_item_dict(node, question, order)) + except ValueError as e: + raise NodeValidationError( + "Invalid question data for node {}: {}".format(node.node_id, e) + ) + with transaction.atomic(): - order = 0 - - for question in data: - question_obj = AssessmentItem( - type=question.get("type"), - question=question.get("question"), - hints=question.get("hints"), - answers=question.get("answers"), - order=order, - contentnode=node, - assessment_id=question.get("assessment_id"), - raw_data=question.get("raw_data"), - source_url=question.get("source_url"), - randomize=question.get("randomize") or False, + serializer = AssessmentItemSerializer( + data=item_dicts, + many=True, + context={"request": SimpleNamespace(user=user)}, + ) + try: + serializer.is_valid(raise_exception=True) + # AssessmentItemSerializer.set_files() attaches QTI media by + # reusing the File row ricecooker's file_upload_url request already + # created for each referenced checksum. A QTI item referencing a + # checksum nothing was ever uploaded for surfaces as a storage-layer + # error (FileNotFoundError locally, OSError on the S3 backend) here + # rather than as a DRF ValidationError. + instances = serializer.save() + except (DRFValidationError, OSError) as e: + detail = e.detail if isinstance(e, DRFValidationError) else e + raise NodeValidationError( + "Invalid assessment item data for node {}: {}".format( + node.node_id, detail + ) + ) + + # set_files() only scans raw_data for QTI items; perseus_question raw_data + # is opaque JSON, so its file (declared explicitly in question["files"]) + # is still attached here. + for question, instance in zip(data, instances): + if instance.type == exercises.PERSEUS_QUESTION: + map_files_to_assessment_item(user, instance, question["files"]) + + +def _build_assessment_item_dict(node, question, order): + common = { + "contentnode": node.pk, + "assessment_id": question.get("assessment_id"), + "order": order, + "source_url": question.get("source_url"), + "randomize": question.get("randomize") or False, + } + + if question.get("type") == exercises.QTI: + raw_data = question.get("raw_data") + if find_perseus_custom_interaction_path(raw_data): + raise ValueError( + "QTI item wraps a Perseus custom interaction; ricecooker must " + "upload raw Perseus content as a perseus_question instead of " + "wrapping it in QTI" ) - order += 1 - question_obj.save() - map_files_to_assessment_item(user, question_obj, question["files"]) + return dict(common, type=exercises.QTI, raw_data=raw_data) + + if question.get("type") == exercises.PERSEUS_QUESTION: + return dict( + common, type=exercises.PERSEUS_QUESTION, raw_data=question.get("raw_data") + ) + + result = convert_legacy_question_to_qti(question) + return dict(common, type=exercises.QTI, raw_data=result.xml) def create_slides(user, node, slideshow_data): diff --git a/contentcuration/contentcuration/viewsets/assessmentitem.py b/contentcuration/contentcuration/viewsets/assessmentitem.py index 30678b9570..e000c67371 100644 --- a/contentcuration/contentcuration/viewsets/assessmentitem.py +++ b/contentcuration/contentcuration/viewsets/assessmentitem.py @@ -144,11 +144,11 @@ def validate(self, data): raise ValidationError( {"raw_data": [error.message for error in result.errors]} ) - elif "raw_data" in data: + elif self._item_type != exercises.PERSEUS_QUESTION and "raw_data" in data: raise ValidationError( { "raw_data": [ - "This field can only be edited on a QTI assessment item." + "This field can only be edited on a QTI or Perseus assessment item." ] } )