diff --git a/cms/djangoapps/contentstore/rest_api/v1/mixins.py b/cms/djangoapps/contentstore/rest_api/v1/mixins.py index 849a4834905e..3ac4795680ff 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/mixins.py +++ b/cms/djangoapps/contentstore/rest_api/v1/mixins.py @@ -2,10 +2,16 @@ Common mixins for module. """ import json +import logging from unittest.mock import patch +from django.http import Http404 +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import UsageKey from rest_framework import status +log = logging.getLogger(__name__) + class PermissionAccessMixin: """ @@ -30,7 +36,7 @@ def test_permissions_unauthenticated(self): self.assertEqual(error, "Authentication credentials were not provided.") self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) - @patch.dict('django.conf.settings.FEATURES', {'DISABLE_ADVANCED_SETTINGS': True}) + @patch.dict("django.conf.settings.FEATURES", {"DISABLE_ADVANCED_SETTINGS": True}) def test_permissions_unauthorized(self): """ Test that an error is returned if the user is unauthorised. @@ -40,3 +46,20 @@ def test_permissions_unauthorized(self): error = self.get_and_check_developer_response(response) self.assertEqual(error, "You do not have permission to perform this action.") self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + +class ContainerHandlerMixin: + """ + A mixin providing common functionality for container handler views. + """ + + def get_object(self, usage_key_string): + """ + Get an object by usage-id of the block + """ + try: + usage_key = UsageKey.from_string(usage_key_string) + return usage_key + except InvalidKeyError as err: + log.error(f"Invalid usage key: {usage_key_string}", exc_info=True) + raise Http404(f"Object not found for usage key: {usage_key_string}") from err diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/__init__.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/__init__.py index d5b41dd5b99a..8914306902fb 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/__init__.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/__init__.py @@ -21,4 +21,4 @@ VideoUsageSerializer, VideoDownloadSerializer ) -from .vertical_block import ContainerHandlerSerializer +from .vertical_block import ContainerHandlerSerializer, VerticalContainerSerializer 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 630f558f05ec..4f985572bbb8 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py @@ -91,3 +91,21 @@ def get_assets_url(self, obj): "assets_handler", kwargs={"course_key_string": context_course.id} ) return None + + +class ChildVerticalContainerSerializer(serializers.Serializer): + """ + Serializer for representing a xblock child of vertical container. + """ + + name = serializers.CharField(source="display_name_with_default") + block_id = serializers.CharField(source="location") + + +class VerticalContainerSerializer(serializers.Serializer): + """ + Serializer for representing a vertical container with state and children. + """ + + children = ChildVerticalContainerSerializer(many=True) + is_published = serializers.BooleanField() diff --git a/cms/djangoapps/contentstore/rest_api/v1/urls.py b/cms/djangoapps/contentstore/rest_api/v1/urls.py index 66760ea3c303..e43d0762babe 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v1/urls.py @@ -21,7 +21,8 @@ ProctoringErrorsView, HelpUrlsView, VideoUsageView, - VideoDownloadView + VideoDownloadView, + VerticalContainerView, ) app_name = 'v1' @@ -107,6 +108,11 @@ ContainerHandlerView.as_view(), name="container_handler" ), + re_path( + fr'^container/vertical/{settings.USAGE_KEY_PATTERN}/children$', + VerticalContainerView.as_view(), + name="container_vertical" + ), # Authoring API # Do not use under v1 yet (Nov. 23). The Authoring API is still experimental and the v0 versions should be used diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py b/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py index b7415b78c29a..4b302d4235f1 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py @@ -15,4 +15,4 @@ VideoDownloadView ) from .help_urls import HelpUrlsView -from .vertical_block import ContainerHandlerView +from .vertical_block import ContainerHandlerView, VerticalContainerView 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 ff117c5ecfe6..23a318c5f5ff 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 @@ -5,63 +5,174 @@ from rest_framework import status from cms.djangoapps.contentstore.tests.utils import CourseTestCase -from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.tests.factories import BlockFactory # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.modulestore.django import ( + modulestore, +) # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.modulestore.tests.factories import ( + BlockFactory, +) # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.modulestore import ( + ModuleStoreEnum, +) # lint-amnesty, pylint: disable=wrong-import-order -class ContainerHandlerViewTest(CourseTestCase): +class BaseXBlockContainer(CourseTestCase): """ - Unit tests for the ContainerHandlerView. + Base xBlock container handler. + + Contains common function for processing course xblocks. """ + view_name = None + def setUp(self): super().setUp() - self.chapter = BlockFactory.create( - parent=self.course, category="chapter", display_name="Week 1" - ) - self.sequential = BlockFactory.create( - parent=self.chapter, category="sequential", display_name="Lesson 1" - ) - self.vertical = self._create_block(self.sequential, "vertical", "Unit") - self.store = modulestore() - self.store.publish(self.vertical.location, self.user.id) + self.setup_xblock() - def _get_reverse_url(self, location): + def setup_xblock(self): """ - Creates url to current handler view api + Set up XBlock objects for testing purposes. + + This method creates XBlock objects representing a course structure with chapters, + sequentials, verticals and others. """ - return reverse( - "cms.djangoapps.contentstore:v1:container_handler", - kwargs={"usage_key_string": location}, + self.chapter = self.create_block( + parent=self.course.location, + category="chapter", + display_name="Week 1", + ) + + self.sequential = self.create_block( + parent=self.chapter.location, + category="sequential", + display_name="Lesson 1", + ) + + self.vertical = self.create_block(self.sequential.location, "vertical", "Unit") + + self.html_unit_first = self.create_block( + parent=self.vertical.location, + category="html", + display_name="Html Content 1", + ) + + self.html_unit_second = self.create_block( + parent=self.vertical.location, + category="html", + display_name="Html Content 2", ) - def _create_block(self, parent, category, display_name, **kwargs): + def create_block(self, parent, category, display_name, **kwargs): """ Creates a block without publishing it. """ return BlockFactory.create( - parent=parent, + parent_location=parent, category=category, display_name=display_name, + modulestore=self.store, publish_item=False, user_id=self.user.id, - **kwargs + **kwargs, + ) + + def get_reverse_url(self, location): + """ + Creates url to current view api name + """ + return reverse( + f"cms.djangoapps.contentstore:v1:{self.view_name}", + kwargs={"usage_key_string": location}, ) + def publish_item(self, store, item_location): + """ + Publish the item at the given location + """ + with store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + store.publish(item_location, ModuleStoreEnum.UserID.test) + + +class ContainerHandlerViewTest(BaseXBlockContainer): + """ + Unit tests for the ContainerHandlerView. + """ + + view_name = "container_handler" + def test_success_response(self): """ Check that endpoint is valid and success response. """ - url = self._get_reverse_url(self.vertical.location) + url = self.get_reverse_url(self.vertical.location) + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_not_valid_usage_key_string(self): + """ + Check that invalid 'usage_key_string' raises Http404. + """ + usage_key_string = ( + "i4x://InvalidOrg/InvalidCourse/vertical/static/InvalidContent" + ) + url = self.get_reverse_url(usage_key_string) + response = self.client.get(url) + self.assertEqual(response.status_code, 404) + + +class ContainerVerticalViewTest(BaseXBlockContainer): + """ + Unit tests for the ContainerVerticalViewTest. + """ + + view_name = "container_vertical" + + def test_success_response(self): + """ + Check that endpoint returns valid response data. + """ + url = self.get_reverse_url(self.vertical.location) response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(len(response.data["children"]), 2) + self.assertFalse(response.data["is_published"]) + + def test_xblock_is_published(self): + """ + Check that published xBlock container returns. + """ + self.publish_item(self.store, self.vertical.location) + url = self.get_reverse_url(self.vertical.location) + response = self.client.get(url) + self.assertTrue(response.data["is_published"]) + + def test_children_content(self): + """ + Check that returns valid response with children of vertical container. + """ + url = self.get_reverse_url(self.vertical.location) + response = self.client.get(url) + + expected_response = [ + { + "name": self.html_unit_first.display_name_with_default, + "block_id": str(self.html_unit_first.location), + }, + { + "name": self.html_unit_second.display_name_with_default, + "block_id": str(self.html_unit_second.location), + }, + ] + self.assertEqual(response.data["children"], expected_response) def test_not_valid_usage_key_string(self): """ Check that invalid 'usage_key_string' raises Http404. """ - usage_key_string = "i4x://InvalidOrg/InvalidCourse/vertical/static/InvalidContent" - url = self._get_reverse_url(usage_key_string) + usage_key_string = ( + "i4x://InvalidOrg/InvalidCourse/vertical/static/InvalidContent" + ) + url = self.get_reverse_url(usage_key_string) response = self.client.get(url) self.assertEqual(response.status_code, 404) 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 42d751c64f06..cbd90c81ef41 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py @@ -1,37 +1,31 @@ """ API Views for unit page """ import edx_api_doc_tools as apidocs -from django.http import Http404, HttpResponseBadRequest -from opaque_keys import InvalidKeyError -from opaque_keys.edx.keys import UsageKey +from django.http import HttpResponseBadRequest from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView from cms.djangoapps.contentstore.utils import get_container_handler_context from cms.djangoapps.contentstore.views.component import _get_item_in_course -from cms.djangoapps.contentstore.rest_api.v1.serializers import ContainerHandlerSerializer +from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import get_xblock +from cms.djangoapps.contentstore.rest_api.v1.serializers import ( + ContainerHandlerSerializer, + VerticalContainerSerializer, +) from openedx.core.lib.api.view_utils import view_auth_classes from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order +from cms.djangoapps.contentstore.rest_api.v1.mixins import ContainerHandlerMixin + @view_auth_classes(is_authenticated=True) -class ContainerHandlerView(APIView): +class ContainerHandlerView(APIView, ContainerHandlerMixin): """ View for container xblock requests to get vertical data. """ - def get_object(self, usage_key_string): - """ - Get an object by usage-id of the block - """ - try: - usage_key = UsageKey.from_string(usage_key_string) - except InvalidKeyError: - raise Http404 # lint-amnesty, pylint: disable=raise-missing-from - return usage_key - @apidocs.schema( parameters=[ apidocs.string_parameter( @@ -146,3 +140,73 @@ def get(self, request: Request, usage_key_string: str): }) serializer = ContainerHandlerSerializer(context) return Response(serializer.data) + + +@view_auth_classes(is_authenticated=True) +class VerticalContainerView(APIView, ContainerHandlerMixin): + """ + View for container xblock requests to get vertical state and children data. + """ + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + "usage_key_string", + apidocs.ParameterLocation.PATH, + description="Vertical usage key", + ), + ], + responses={ + 200: VerticalContainerSerializer, + 401: "The requester is not authenticated.", + 404: "The requested locator does not exist.", + }, + ) + def get(self, request: Request, usage_key_string: str): + """ + Get an object containing vertical state with children data. + + **Example Request** + + GET /api/contentstore/v1/container/vertical/{usage_key_string}/children + + **Response Values** + + If the request is successful, an HTTP 200 "OK" response is returned. + + The HTTP 200 response contains a single dict that contains keys that + are the vertical's container children data. + + **Example Response** + + ```json + { + "children": [ + { + "name": "Drag and Drop", + "block_id": "block-v1:org+101+101+type@drag-and-drop-v2+block@7599275ace6b46f5a482078a2954ca16" + }, + { + "name": "Video", + "block_id": "block-v1:org+101+101+type@video+block@0e3d39b12d7c4345981bda6b3511a9bf" + }, + { + "name": "Text", + "block_id": "block-v1:org+101+101+type@html+block@3e3fa1f88adb4a108cd14e9002143690" + } + ], + "is_published": false + } + ``` + """ + usage_key = self.get_object(usage_key_string) + current_xblock = get_xblock(usage_key, request.user) + + with modulestore().bulk_operations(usage_key.course_key): + children = [ + modulestore().get_item(child) for child in current_xblock.children + ] + is_published = not modulestore().has_changes(current_xblock) + container_data = {"children": children, "is_published": is_published} + serializer = VerticalContainerSerializer(container_data) + return Response(serializer.data) diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index 535e930cc09a..bcfe015f03e5 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -48,6 +48,7 @@ from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE from openedx.core.lib.gating import api as gating_api from openedx.core.lib.cache_utils import request_cached +from openedx.core.lib.xblock_utils import get_icon from openedx.core.toggles import ENTRANCE_EXAMS from xmodule.course_block import DEFAULT_START_DATE from xmodule.modulestore import EdxJSONEncoder, ModuleStoreEnum @@ -1095,6 +1096,7 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements "show_correctness": xblock.show_correctness, "hide_from_toc": xblock.hide_from_toc, "enable_hide_from_toc_ui": settings.FEATURES.get("ENABLE_HIDE_FROM_TOC_UI", False), + "xblock_type": get_icon(xblock), } ) diff --git a/xmodule/split_test_block.py b/xmodule/split_test_block.py index 88b51db19b3b..05ca3a5db454 100644 --- a/xmodule/split_test_block.py +++ b/xmodule/split_test_block.py @@ -15,6 +15,7 @@ from web_fragments.fragment import Fragment from webob import Response from xblock.core import XBlock +from xblock.exceptions import NoSuchServiceError from xblock.fields import Integer, ReferenceValueDict, Scope, String from xmodule.mako_block import MakoTemplateBlockBase from xmodule.modulestore.inheritance import UserPartitionList @@ -172,10 +173,20 @@ def child_block(self): def child(self): """ Return the user bound child block for the partition or None. + + Handles the AttributeError exception that may occur when attempting to retrieve + an icon for the split_test xblock within the CMS. """ - if self.child_block is not None: - return self.runtime.get_block_for_descriptor(self.child_block) - else: + try: + if self.child_block is not None: + return self.runtime.get_block_for_descriptor(self.child_block) + else: + return None + except AttributeError: + log.warning( + "Error while getting block instance for descriptor with location: [%s]", + self.location + ) return None def get_child_block_by_location(self, location): @@ -212,13 +223,31 @@ def get_content_titles(self): def get_child_blocks(self): """ For grading--return just the chosen child. + + Handles the NoSuchServiceError and ValueError exception that may occur when attempting to retrieve + an icon for the split_test xblock within the CMS. """ - group_id = self.get_group_id() + try: + group_id = self.get_group_id() + except NoSuchServiceError: + log.warning( + "Error while getting user service in runtime with location: [%s]", + self.location + ) + return [] + except ValueError: + log.warning( + "Error while getting group ID for partition with location: [%s]", + self.location + ) + return [] + if group_id is None: return [] # group_id_to_child comes from json, so it has to have string keys str_group_id = str(group_id) + child_block = None if str_group_id in self.group_id_to_child: child_location = self.group_id_to_child[str_group_id] child_block = self.get_child_block_by_location(child_location)