From f5d9e1c1ad266b33d35ad4428529f6fdb4e205ab Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 1 Oct 2025 11:24:01 -0700 Subject: [PATCH 1/3] Add regression test for erroneous randomize validation on HTML5. Make randomize no longer required. Prevent contentnodes from having their kind updated after creation. Ensure randomize is set at exercise creation. Add tests for the above behaviours. --- .../tests/viewsets/test_contentnode.py | 130 +++++++++++++++++- .../contentcuration/viewsets/contentnode.py | 26 +++- 2 files changed, 153 insertions(+), 3 deletions(-) diff --git a/contentcuration/contentcuration/tests/viewsets/test_contentnode.py b/contentcuration/contentcuration/tests/viewsets/test_contentnode.py index d27b4304d9..184a1c544f 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_contentnode.py +++ b/contentcuration/contentcuration/tests/viewsets/test_contentnode.py @@ -210,7 +210,7 @@ def test_deadlock_move_and_rebuild(self): for (node_id, target_id) in zip( root_children_ids, first_child_node_children_ids ) - ) + ), ) for result in results: @@ -236,7 +236,7 @@ def test_deadlock_create_and_rebuild(self): *( (create_contentnode, {"parent_id": node_id}) for node_id in first_child_node_children_ids - ) + ), ) for result in results: @@ -1903,6 +1903,132 @@ def test_delete_no_permission_prerequisite(self): self.assertEqual(len(response.data["disallowed"]), 1) self.assertTrue(contentnode.prerequisite.filter(id=prereq.id).exists()) + def test_create_html5_contentnode_with_entry_validation(self): + """ + Regression test for HTML5 nodes validation failure when entry value is set in extra_fields. + + This test verifies that newly created HTML5 content nodes with an "entry" value + in extra_fields.options.entry can be successfully validated and created. + """ + contentnode_data = self.contentnode_metadata + contentnode_data["kind"] = content_kinds.HTML5 + contentnode_data["extra_fields"] = {"options": {"entry": "index.html"}} + + response = self.sync_changes( + [ + generate_create_event( + contentnode_data["id"], + CONTENTNODE, + contentnode_data, + channel_id=self.channel.id, + ) + ], + ) + self.assertEqual(response.status_code, 200, response.content) + self.assertEqual( + len(response.data.get("errors", [])), + 0, + f"Expected no validation errors, but got: {response.data.get('errors', [])}", + ) + + try: + new_node = models.ContentNode.objects.get(id=contentnode_data["id"]) + except models.ContentNode.DoesNotExist: + self.fail("HTML5 ContentNode with entry value was not created") + + self.assertEqual(new_node.parent_id, self.channel.main_tree_id) + self.assertEqual(new_node.kind_id, content_kinds.HTML5) + self.assertEqual(new_node.extra_fields["options"]["entry"], "index.html") + + def test_create_exercise_contentnode_requires_randomize(self): + """ + Test that exercise content nodes require the randomize field in extra_fields. + """ + contentnode_data = self.contentnode_metadata + contentnode_data["kind"] = content_kinds.EXERCISE + # Deliberately omit randomize field + contentnode_data["extra_fields"] = {"options": {}} + + response = self.sync_changes( + [ + generate_create_event( + contentnode_data["id"], + CONTENTNODE, + contentnode_data, + channel_id=self.channel.id, + ) + ], + ) + self.assertEqual(response.status_code, 200, response.content) + self.assertEqual(len(response.data.get("errors", [])), 1) + + error = response.data["errors"][0] + + self.assertIn("randomize", error["errors"]["extra_fields"]) + self.assertEqual( + error["errors"]["extra_fields"]["randomize"][0], + "This field is required for exercise content.", + ) + + def test_create_exercise_contentnode_with_randomize_succeeds(self): + """ + Test that exercise content nodes with randomize field are created successfully. + """ + contentnode_data = self.contentnode_metadata + contentnode_data["kind"] = content_kinds.EXERCISE + contentnode_data["extra_fields"] = {"randomize": True, "options": {}} + + response = self.sync_changes( + [ + generate_create_event( + contentnode_data["id"], + CONTENTNODE, + contentnode_data, + channel_id=self.channel.id, + ) + ], + ) + self.assertEqual(response.status_code, 200, response.content) + self.assertEqual(len(response.data.get("errors", [])), 0) + + try: + new_node = models.ContentNode.objects.get(id=contentnode_data["id"]) + except models.ContentNode.DoesNotExist: + self.fail("Exercise ContentNode with randomize field was not created") + + self.assertEqual(new_node.kind_id, content_kinds.EXERCISE) + self.assertTrue(new_node.extra_fields["randomize"]) + + def test_cannot_update_contentnode_kind(self): + """ + Test that content node kind cannot be changed after creation. + """ + contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata) + original_kind = contentnode.kind_id + + response = self.sync_changes( + [ + generate_update_event( + contentnode.id, + CONTENTNODE, + {"kind": content_kinds.HTML5}, + channel_id=self.channel.id, + ) + ], + ) + self.assertEqual(response.status_code, 200, response.content) + self.assertEqual(len(response.data.get("errors", [])), 1) + + error = response.data["errors"][0] + self.assertIn("kind", error["errors"]) + self.assertEqual( + error["errors"]["kind"][0], "Content kind cannot be changed after creation" + ) + + # Verify kind was not changed + contentnode.refresh_from_db() + self.assertEqual(contentnode.kind_id, original_kind) + class CRUDTestCase(StudioAPITestCase): def setUp(self): diff --git a/contentcuration/contentcuration/viewsets/contentnode.py b/contentcuration/contentcuration/viewsets/contentnode.py index 6a0e1fa829..6aaf179b46 100644 --- a/contentcuration/contentcuration/viewsets/contentnode.py +++ b/contentcuration/contentcuration/viewsets/contentnode.py @@ -297,6 +297,7 @@ class ExtraFieldsOptionsSerializer(JSONFieldDictSerializer): required=False, ) completion_criteria = CompletionCriteriaSerializer(required=False) + entry = CharField(required=False, allow_null=True) class InheritedMetadataSerializer(JSONFieldDictSerializer): @@ -307,7 +308,7 @@ class InheritedMetadataSerializer(JSONFieldDictSerializer): class ExtraFieldsSerializer(JSONFieldDictSerializer): - randomize = BooleanField() + randomize = BooleanField(required=False) options = ExtraFieldsOptionsSerializer(required=False) suggested_duration_type = ChoiceField( choices=[completion_criteria.TIME, completion_criteria.APPROX_TIME], @@ -428,11 +429,34 @@ def validate(self, data): raise ValidationError( {"parent": "This field should only be changed by a move operation"} ) + + # Prevent kind from being changed after creation + if self.instance is not None and "kind" in data: + raise ValidationError( + {"kind": "Content kind cannot be changed after creation"} + ) + tags = data.get("tags") if tags is not None: for tag in tags: if len(tag) > 30: raise ValidationError("tag is greater than 30 characters") + + # Conditional validation for randomize field on exercise creation + if self.instance is None: # Only validate on creation + kind = data.get("kind") + if kind.kind == content_kinds.EXERCISE: + extra_fields = data.get("extra_fields", {}) + if "randomize" not in extra_fields: + raise ValidationError( + { + "extra_fields": { + "randomize": [ + "This field is required for exercise content." + ] + } + } + ) return data def _check_completion_criteria(self, kind, complete, validated_data): From 860e7444d8ce4dd372ca4ca7e276fd8f5e51d43c Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 1 Oct 2025 16:04:10 -0700 Subject: [PATCH 2/3] Loosen kind update restriction to only error when kind is actually changed, not just when included in the payload. --- contentcuration/contentcuration/viewsets/contentnode.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/viewsets/contentnode.py b/contentcuration/contentcuration/viewsets/contentnode.py index 6aaf179b46..895f199167 100644 --- a/contentcuration/contentcuration/viewsets/contentnode.py +++ b/contentcuration/contentcuration/viewsets/contentnode.py @@ -431,7 +431,11 @@ def validate(self, data): ) # Prevent kind from being changed after creation - if self.instance is not None and "kind" in data: + if ( + self.instance is not None + and "kind" in data + and self.instance.kind != data["kind"] + ): raise ValidationError( {"kind": "Content kind cannot be changed after creation"} ) From ec9799ad892215b9bdc653443786feb6988109d7 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 2 Oct 2025 15:55:12 -0700 Subject: [PATCH 3/3] When creating exercises, default randomize to true as data. Remove UI only default. --- .../frontend/channelEdit/components/edit/DetailsTabView.vue | 2 +- .../frontend/channelEdit/vuex/contentNode/actions.js | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue b/contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue index cceab440aa..3002797c94 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue @@ -694,7 +694,7 @@ /* FORM FIELDS */ title: generateGetterSetter('title'), description: generateGetterSetter('description'), - randomizeOrder: generateExtraFieldsGetterSetter('randomize', true), + randomizeOrder: generateExtraFieldsGetterSetter('randomize'), author: generateGetterSetter('author'), provider: generateGetterSetter('provider'), aggregator: generateGetterSetter('aggregator'), diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js index 66d368a51d..d2b13575fc 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js @@ -220,6 +220,10 @@ export function createContentNode(context, { parent, kind, ...payload }) { ...payload, }; + if (kind === ContentKindsNames.EXERCISE) { + contentNodeData.extra_fields.randomize = true; + } + contentNodeData.complete = isNodeComplete({ nodeDetails: contentNodeData, assessmentItems: [],