From 4952bbc287afd4077faf9479ad4577a17ce1827b Mon Sep 17 00:00:00 2001 From: ruzniaievdm Date: Tue, 13 Feb 2024 20:32:54 +0200 Subject: [PATCH] feat: [AXIMST-496] error notification when xblock restriction conflicts --- .../rest_api/v1/serializers/vertical_block.py | 11 +++ .../v1/views/tests/test_vertical_block.py | 76 ++++++++++++++++--- .../rest_api/v1/views/vertical_block.py | 29 +++---- cms/djangoapps/contentstore/utils.py | 18 +++++ 4 files changed, 110 insertions(+), 24 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py index ab01a4104d05..43371471b3a4 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py @@ -12,6 +12,15 @@ ) +class MessageValidation(serializers.Serializer): + """ + Serializer for representing XBlock error. + """ + + text = serializers.CharField() + type = serializers.CharField() + + class ChildAncestorSerializer(serializers.Serializer): """ Serializer for representing child blocks in the ancestor XBlock. @@ -105,6 +114,8 @@ class ChildVerticalContainerSerializer(serializers.Serializer): actions = serializers.SerializerMethodField() user_partition_info = serializers.DictField() user_partitions = serializers.ListField() + has_validation_error = serializers.BooleanField() + validation_errors = MessageValidation(many=True) def get_actions(self, obj): # pylint: disable=unused-argument """ diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py index cd71063ab88a..848743b0b7f6 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py @@ -1,13 +1,19 @@ """ Unit tests for the vertical block. """ + from django.urls import reverse from rest_framework import status from edx_toggles.toggles.testutils import override_waffle_flag +from xblock.validation import ValidationMessage from cms.djangoapps.contentstore.tests.utils import CourseTestCase from cms.djangoapps.contentstore.toggles import ENABLE_TAGGING_TAXONOMY_LIST_PAGE -from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID +from xmodule.partitions.partitions import ( + ENROLLMENT_TRACK_PARTITION_ID, + Group, + UserPartition, +) from xmodule.modulestore.django import ( modulestore, ) # lint-amnesty, pylint: disable=wrong-import-order @@ -96,6 +102,13 @@ def publish_item(self, store, item_location): with store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): store.publish(item_location, ModuleStoreEnum.UserID.test) + def set_group_access(self, xblock, value): + """ + Sets group_access to specified value and calls update_item to persist the change. + """ + xblock.group_access = value + self.store.update_item(xblock, self.user.id) + class ContainerHandlerViewTest(BaseXBlockContainer): """ @@ -161,7 +174,7 @@ def test_children_content(self): expected_user_partition_info = { "selectable_partitions": [], "selected_partition_index": -1, - "selected_groups_label": "" + "selected_groups_label": "", } expected_user_partitions = [ @@ -170,13 +183,8 @@ def test_children_content(self): "name": "Enrollment Track Groups", "scheme": "enrollment_track", "groups": [ - { - "id": 1, - "name": "Audit", - "selected": False, - "deleted": False - } - ] + {"id": 1, "name": "Audit", "selected": False, "deleted": False} + ], } ] @@ -194,7 +202,9 @@ def test_children_content(self): "can_manage_tags": True, }, "user_partition_info": expected_user_partition_info, - "user_partitions": expected_user_partitions + "user_partitions": expected_user_partitions, + "has_validation_error": False, + "validation_errors": [], }, { "name": self.html_unit_second.display_name_with_default, @@ -210,6 +220,8 @@ def test_children_content(self): }, "user_partition_info": expected_user_partition_info, "user_partitions": expected_user_partitions, + "has_validation_error": False, + "validation_errors": [], }, ] self.assertEqual(response.data["children"], expected_response) @@ -234,3 +246,47 @@ def test_actions_with_turned_off_taxonomy_flag(self): response = self.client.get(url) for children in response.data["children"]: self.assertFalse(children["actions"]["can_manage_tags"]) + + def test_validation_errors(self): + """ + Check that child has an error. + """ + self.course.user_partitions = [ + UserPartition( + 0, + "first_partition", + "Test Partition", + [Group("0", "alpha"), Group("1", "beta")], + ), + ] + self.store.update_item(self.course, self.user.id) + + user_partition = self.course.user_partitions[0] + vertical = self.store.get_item(self.vertical.location) + html_unit_first = self.store.get_item(self.html_unit_first.location) + + group_first = user_partition.groups[0] + group_second = user_partition.groups[1] + + # Set access settings so html will contradict vertical + self.set_group_access(vertical, {user_partition.id: [group_second.id]}) + self.set_group_access(html_unit_first, {user_partition.id: [group_first.id]}) + + # update vertical/html + vertical = self.store.get_item(self.vertical.location) + html_unit_first = self.store.get_item(self.html_unit_first.location) + + url = self.get_reverse_url(self.vertical.location) + response = self.client.get(url) + children_response = response.data["children"] + + # Check for an error in html_unit_first xblock + self.assertTrue(children_response[0]["has_validation_error"]) + + # Verify that html access settings contradict its parent's access settings. + validation = html_unit_first.validate() + self.assertEqual(len(validation.messages), 1) + self.assertEqual(validation.to_json()['messages'][0]['type'], ValidationMessage.ERROR) + + # Check for an error in html_unit_second xblock + self.assertFalse(children_response[1]["has_validation_error"]) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py index eb042a481fd4..54fa8c8e2f65 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py @@ -10,6 +10,7 @@ get_container_handler_context, get_user_partition_info, get_visibility_partition_info, + get_validation_messages, ) from cms.djangoapps.contentstore.views.component import _get_item_in_course from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import get_xblock @@ -186,19 +187,6 @@ def get(self, request: Request, usage_key_string: str): ```json { "children": [ - { - "name": "Drag and Drop", - "block_id": "block-v1:org+101+101+type@drag-and-drop-v2+block@7599275ace6b46f5a482078a2954ca16", - "block_type": "drag-and-drop-v2", - "actions": { - "can_copy": true, - "can_duplicate": true, - "can_move": true, - "can_manage_access": true, - "can_delete": true, - "can_manage_tags": true, - } - }, { "name": "Video", "block_id": "block-v1:org+101+101+type@video+block@0e3d39b12d7c4345981bda6b3511a9bf", @@ -211,6 +199,8 @@ def get(self, request: Request, usage_key_string: str): "can_delete": true, "can_manage_tags": true, } + "has_validation_error": false, + "validation_errors": [], }, { "name": "Text", @@ -223,7 +213,14 @@ def get(self, request: Request, usage_key_string: str): "can_manage_access": true, "can_delete": true, "can_manage_tags": true, - } + }, + "has_validation_error": true, + "validation_errors": [ + { + "text": "This component's access settings contradict its parent's access settings.", + "type": "error" + } + ] }, ], "is_published": false, @@ -242,12 +239,16 @@ def get(self, request: Request, usage_key_string: str): child_info = modulestore().get_item(child) user_partition_info = get_visibility_partition_info(child_info, course=course) user_partitions = get_user_partition_info(child_info, course=course) + validation_errors, has_validation_error = get_validation_messages(child_info) + children.append({ "name": child_info.display_name_with_default, "block_id": child_info.location, "block_type": child_info.location.block_type, "user_partition_info": user_partition_info, "user_partitions": user_partitions, + "has_validation_error": has_validation_error, + "validation_errors": validation_errors, }) is_published = not modulestore().has_changes(current_xblock) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 2b8120039958..946aa34e2c9c 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -2139,3 +2139,21 @@ def track_course_update_event(course_key, user, event_data=None): context = contexts.course_context_from_course_id(course_key) with tracker.get_tracker().context(event_name, context): tracker.emit(event_name, event_data) + + +def get_validation_messages(xblock): + """ + Retrieves validation messages for a given xblock. + + Args: + xblock: The xblock object to validate. + + Returns: + tuple: + - validation_errors (list): A list of validation error messages. + - has_validation_error (bool): True if there are validation errors, False otherwise. + """ + validation_json = xblock.validate().to_json() + validation_errors = validation_json['messages'] + has_validation_error = bool(validation_errors) + return validation_errors, has_validation_error