diff --git a/cms/djangoapps/contentstore/rest_api/v0/tests/test_assets.py b/cms/djangoapps/contentstore/rest_api/v0/tests/test_assets.py index 46b636916df8..3772dbb64e77 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/tests/test_assets.py +++ b/cms/djangoapps/contentstore/rest_api/v0/tests/test_assets.py @@ -60,13 +60,8 @@ def send_request(self, _url, _data): } ), ) - @patch( - f"cms.djangoapps.contentstore.rest_api.{VERSION}.views.xblock.toggles.use_studio_content_api", - return_value=True, - ) def make_request( self, - mock_use_studio_content_api, mock_handle_assets, run_assertions=None, course_id=None, @@ -125,13 +120,6 @@ def assert_assets_handler_called(self, *, mock_handle_assets, response): def send_request(self, url, data): return self.client.get(url) - def test_api_behind_feature_flag(self): - # should return 404 if the feature flag is not enabled - url = self.get_url() - - response = self.client.get(url) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_assets_handler_called_with_correct_arguments(self): self.client.login( username=self.course_instructor.username, password=self.password @@ -182,13 +170,6 @@ def assert_assets_handler_called(self, *, mock_handle_assets, response): def send_request(self, url, data): return self.client.post(url, data=data, format="multipart") - def test_api_behind_feature_flag(self): - # should return 404 if the feature flag is not enabled - url = self.get_url() - - response = self.client.post(url) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_assets_handler_called_with_correct_arguments(self): self.client.login( username=self.course_instructor.username, password=self.password @@ -232,13 +213,6 @@ def assert_assets_handler_called(self, *, mock_handle_assets, response): def send_request(self, url, data): return self.client.put(url, data=data, format="json") - def test_api_behind_feature_flag(self): - # should return 404 if the feature flag is not enabled - url = self.get_url() - - response = self.client.put(url) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_assets_handler_called_with_correct_arguments(self): self.client.login( username=self.course_instructor.username, password=self.password @@ -277,13 +251,6 @@ def assert_assets_handler_called(self, *, mock_handle_assets, response): def send_request(self, url, data): return self.client.delete(url) - def test_api_behind_feature_flag(self): - # should return 404 if the feature flag is not enabled - url = self.get_url() - - response = self.client.delete(url) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_assets_handler_called_with_correct_arguments(self): self.client.login( username=self.course_instructor.username, password=self.password diff --git a/cms/djangoapps/contentstore/rest_api/v0/tests/test_xblock.py b/cms/djangoapps/contentstore/rest_api/v0/tests/test_xblock.py index e4c21eb353b1..512c92f6ffe9 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/tests/test_xblock.py +++ b/cms/djangoapps/contentstore/rest_api/v0/tests/test_xblock.py @@ -55,13 +55,8 @@ def send_request(self, _url, _data): } ), ) - @patch( - f"cms.djangoapps.contentstore.rest_api.{VERSION}.views.xblock.toggles.use_studio_content_api", - return_value=True, - ) def make_request( self, - mock_use_studio_content_api, mock_handle_xblock, run_assertions=None, course_id=None, @@ -111,13 +106,6 @@ def assert_xblock_handler_called(self, *, mock_handle_xblock, response): def send_request(self, url, data): return self.client.get(url) - def test_api_behind_feature_flag(self): - # should return 404 if the feature flag is not enabled - url = self.get_url() - - response = self.client.get(url) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_xblock_handler_called_with_correct_arguments(self): self.client.login( username=self.course_instructor.username, password=self.password @@ -167,13 +155,6 @@ def assert_xblock_handler_called(self, *, mock_handle_xblock, response): def send_request(self, url, data): return self.client.post(url, data=data, format="json") - def test_api_behind_feature_flag(self): - # should return 404 if the feature flag is not enabled - url = self.get_url() - - response = self.client.post(url) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_xblock_handler_called_with_correct_arguments(self): self.client.login( username=self.course_instructor.username, password=self.password @@ -218,13 +199,6 @@ def assert_xblock_handler_called(self, *, mock_handle_xblock, response): def send_request(self, url, data): return self.client.put(url, data=data, format="json") - def test_api_behind_feature_flag(self): - # should return 404 if the feature flag is not enabled - url = self.get_url() - - response = self.client.put(url) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_xblock_handler_called_with_correct_arguments(self): self.client.login( username=self.course_instructor.username, password=self.password @@ -269,13 +243,6 @@ def assert_xblock_handler_called(self, *, mock_handle_xblock, response): def send_request(self, url, data): return self.client.patch(url, data=data, format="json") - def test_api_behind_feature_flag(self): - # should return 404 if the feature flag is not enabled - url = self.get_url() - - response = self.client.patch(url) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_xblock_handler_called_with_correct_arguments(self): self.client.login( username=self.course_instructor.username, password=self.password @@ -310,13 +277,6 @@ def assert_xblock_handler_called(self, *, mock_handle_xblock, response): def send_request(self, url, data): return self.client.delete(url) - def test_api_behind_feature_flag(self): - # should return 404 if the feature flag is not enabled - url = self.get_url() - - response = self.client.delete(url) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_xblock_handler_called_with_correct_arguments(self): self.client.login( username=self.course_instructor.username, password=self.password diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/api_heartbeat.py b/cms/djangoapps/contentstore/rest_api/v0/views/api_heartbeat.py index f8b539557e5e..f322f2360939 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/api_heartbeat.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/api_heartbeat.py @@ -5,7 +5,6 @@ from rest_framework.response import Response from rest_framework import status from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes -import cms.djangoapps.contentstore.toggles as toggles class APIHeartBeatView(DeveloperErrorViewMixin, APIView): @@ -43,6 +42,4 @@ def get(self, request: Request): } ``` """ - if toggles.use_studio_content_api(): - return Response({'status': 'heartbeat successful'}, status=status.HTTP_200_OK) - return Response(status=status.HTTP_403_FORBIDDEN) + return Response({'status': 'heartbeat successful'}, status=status.HTTP_200_OK) diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/assets.py b/cms/djangoapps/contentstore/rest_api/v0/views/assets.py index 0c0c24aeab69..a7a495256514 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/assets.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/assets.py @@ -4,7 +4,6 @@ import logging from rest_framework.generics import CreateAPIView, RetrieveAPIView, UpdateAPIView, DestroyAPIView from django.views.decorators.csrf import csrf_exempt -from django.http import Http404 from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes from common.djangoapps.util.json_request import expect_json_in_class_view @@ -12,7 +11,6 @@ from cms.djangoapps.contentstore.api import course_author_access_required from cms.djangoapps.contentstore.asset_storage_handlers import handle_assets -import cms.djangoapps.contentstore.toggles as contentstore_toggles from ..serializers.assets import AssetSerializer from .utils import validate_request_with_serializer @@ -20,7 +18,6 @@ from openedx.core.lib.api.parsers import TypedFileUploadParser log = logging.getLogger(__name__) -toggles = contentstore_toggles @view_auth_classes() @@ -33,17 +30,6 @@ class AssetsCreateRetrieveView(DeveloperErrorViewMixin, CreateAPIView, RetrieveA serializer_class = AssetSerializer parser_classes = (JSONParser, MultiPartParser, FormParser, TypedFileUploadParser) - def dispatch(self, request, *args, **kwargs): - # TODO: probably want to refactor this to a decorator. - """ - The dispatch method of a View class handles HTTP requests in general - and calls other methods to handle specific HTTP methods. - We use this to raise a 404 if the content api is disabled. - """ - if not toggles.use_studio_content_api(): - raise Http404 - return super().dispatch(request, *args, **kwargs) - @csrf_exempt @course_author_access_required @validate_request_with_serializer @@ -66,17 +52,6 @@ class AssetsUpdateDestroyView(DeveloperErrorViewMixin, UpdateAPIView, DestroyAPI serializer_class = AssetSerializer parser_classes = (JSONParser, MultiPartParser, FormParser, TypedFileUploadParser) - def dispatch(self, request, *args, **kwargs): - # TODO: probably want to refactor this to a decorator. - """ - The dispatch method of a View class handles HTTP requests in general - and calls other methods to handle specific HTTP methods. - We use this to raise a 404 if the content api is disabled. - """ - if not toggles.use_studio_content_api(): - raise Http404 - return super().dispatch(request, *args, **kwargs) - @course_author_access_required @expect_json_in_class_view @validate_request_with_serializer diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/authoring_videos.py b/cms/djangoapps/contentstore/rest_api/v0/views/authoring_videos.py index 8fb66070f3bf..f97bf7a98d16 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/authoring_videos.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/authoring_videos.py @@ -9,7 +9,6 @@ ) from rest_framework.parsers import (MultiPartParser, FormParser) from django.views.decorators.csrf import csrf_exempt -from django.http import Http404 from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes from openedx.core.lib.api.parsers import TypedFileUploadParser @@ -27,12 +26,10 @@ VideoUploadSerializer, VideoImageSerializer, ) -import cms.djangoapps.contentstore.toggles as contentstore_toggles from .utils import validate_request_with_serializer log = logging.getLogger(__name__) -toggles = contentstore_toggles @view_auth_classes() @@ -44,17 +41,6 @@ class VideosUploadsView(DeveloperErrorViewMixin, RetrieveAPIView, DestroyAPIView """ serializer_class = VideoUploadSerializer - def dispatch(self, request, *args, **kwargs): - # TODO: probably want to refactor this to a decorator. - """ - The dispatch method of a View class handles HTTP requests in general - and calls other methods to handle specific HTTP methods. - We use this to raise a 404 if the content api is disabled. - """ - if not toggles.use_studio_content_api(): - raise Http404 - return super().dispatch(request, *args, **kwargs) - @course_author_access_required def retrieve(self, request, course_key, edx_video_id=None): # pylint: disable=arguments-differ return handle_videos(request, course_key.html_id(), edx_video_id) @@ -73,17 +59,6 @@ class VideosCreateUploadView(DeveloperErrorViewMixin, CreateAPIView): """ serializer_class = VideoUploadSerializer - def dispatch(self, request, *args, **kwargs): - # TODO: probably want to refactor this to a decorator. - """ - The dispatch method of a View class handles HTTP requests in general - and calls other methods to handle specific HTTP methods. - We use this to raise a 404 if the content api is disabled. - """ - if not toggles.use_studio_content_api(): - raise Http404 - return super().dispatch(request, *args, **kwargs) - @csrf_exempt @course_author_access_required @expect_json_in_class_view @@ -102,17 +77,6 @@ class VideoImagesView(DeveloperErrorViewMixin, CreateAPIView): serializer_class = VideoImageSerializer parser_classes = (MultiPartParser, FormParser, TypedFileUploadParser) - def dispatch(self, request, *args, **kwargs): - # TODO: probably want to refactor this to a decorator. - """ - The dispatch method of a View class handles HTTP requests in general - and calls other methods to handle specific HTTP methods. - We use this to raise a 404 if the content api is disabled. - """ - if not toggles.use_studio_content_api(): - raise Http404 - return super().dispatch(request, *args, **kwargs) - @csrf_exempt @course_author_access_required @expect_json_in_class_view @@ -133,17 +97,6 @@ class VideoEncodingsDownloadView(DeveloperErrorViewMixin, RetrieveAPIView): # does not specify a serializer class. swagger_schema = None - def dispatch(self, request, *args, **kwargs): - # TODO: probably want to refactor this to a decorator. - """ - The dispatch method of a View class handles HTTP requests in general - and calls other methods to handle specific HTTP methods. - We use this to raise a 404 if the content api is disabled. - """ - if not toggles.use_studio_content_api(): - raise Http404 - return super().dispatch(request, *args, **kwargs) - @csrf_exempt @course_author_access_required def retrieve(self, request, course_key): # pylint: disable=arguments-differ @@ -161,17 +114,6 @@ class VideoFeaturesView(DeveloperErrorViewMixin, RetrieveAPIView): # does not specify a serializer class. swagger_schema = None - def dispatch(self, request, *args, **kwargs): - # TODO: probably want to refactor this to a decorator. - """ - The dispatch method of a View class handles HTTP requests in general - and calls other methods to handle specific HTTP methods. - We use this to raise a 404 if the content api is disabled. - """ - if not toggles.use_studio_content_api(): - raise Http404 - return super().dispatch(request, *args, **kwargs) - @csrf_exempt def retrieve(self, request): # pylint: disable=arguments-differ return enabled_video_features(request) diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/transcripts.py b/cms/djangoapps/contentstore/rest_api/v0/views/transcripts.py index 9a63693e12b9..3344fdbd1f43 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/transcripts.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/transcripts.py @@ -8,7 +8,6 @@ DestroyAPIView ) from django.views.decorators.csrf import csrf_exempt -from django.http import Http404 from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes from common.djangoapps.util.json_request import expect_json_in_class_view @@ -20,7 +19,6 @@ delete_video_transcript_or_404, handle_transcript_download, ) -import cms.djangoapps.contentstore.toggles as contentstore_toggles from ..serializers import TranscriptSerializer, YoutubeTranscriptCheckSerializer, YoutubeTranscriptUploadSerializer from rest_framework.parsers import (MultiPartParser, FormParser) from openedx.core.lib.api.parsers import TypedFileUploadParser @@ -28,7 +26,6 @@ from cms.djangoapps.contentstore.rest_api.v0.views.utils import validate_request_with_serializer log = logging.getLogger(__name__) -toggles = contentstore_toggles @view_auth_classes() @@ -42,11 +39,6 @@ class TranscriptView(DeveloperErrorViewMixin, CreateAPIView, RetrieveAPIView, De serializer_class = TranscriptSerializer parser_classes = (MultiPartParser, FormParser, TypedFileUploadParser) - def dispatch(self, request, *args, **kwargs): - if not toggles.use_studio_content_api(): - raise Http404 - return super().dispatch(request, *args, **kwargs) - @csrf_exempt @course_author_access_required @expect_json_in_class_view @@ -81,11 +73,6 @@ class YoutubeTranscriptCheckView(DeveloperErrorViewMixin, RetrieveAPIView): serializer_class = YoutubeTranscriptCheckSerializer parser_classes = (MultiPartParser, FormParser, TypedFileUploadParser) - def dispatch(self, request, *args, **kwargs): - if not toggles.use_studio_content_api(): - raise Http404 - return super().dispatch(request, *args, **kwargs) - @course_author_access_required def retrieve(self, request, course_key_string): # pylint: disable=arguments-differ """ @@ -104,11 +91,6 @@ class YoutubeTranscriptUploadView(DeveloperErrorViewMixin, RetrieveAPIView): serializer_class = YoutubeTranscriptUploadSerializer parser_classes = (MultiPartParser, FormParser, TypedFileUploadParser) - def dispatch(self, request, *args, **kwargs): - if not toggles.use_studio_content_api(): - raise Http404 - return super().dispatch(request, *args, **kwargs) - @course_author_access_required def retrieve(self, request, course_key_string): # pylint: disable=arguments-differ """ diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/xblock.py b/cms/djangoapps/contentstore/rest_api/v0/views/xblock.py index cc26619fb83a..8e678ae845e0 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/xblock.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/xblock.py @@ -4,21 +4,18 @@ import logging from rest_framework.generics import RetrieveUpdateDestroyAPIView, CreateAPIView from django.views.decorators.csrf import csrf_exempt -from django.http import Http404 from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes from common.djangoapps.util.json_request import expect_json_in_class_view from cms.djangoapps.contentstore.api import course_author_access_required from cms.djangoapps.contentstore.xblock_storage_handlers import view_handlers -import cms.djangoapps.contentstore.toggles as contentstore_toggles from ..serializers import XblockSerializer from .utils import validate_request_with_serializer log = logging.getLogger(__name__) -toggles = contentstore_toggles handle_xblock = view_handlers.handle_xblock @@ -32,17 +29,6 @@ class XblockView(DeveloperErrorViewMixin, RetrieveUpdateDestroyAPIView): """ serializer_class = XblockSerializer - def dispatch(self, request, *args, **kwargs): - # TODO: probably want to refactor this to a decorator. - """ - The dispatch method of a View class handles HTTP requests in general - and calls other methods to handle specific HTTP methods. - We use this to raise a 404 if the content api is disabled. - """ - if not toggles.use_studio_content_api(): - raise Http404 - return super().dispatch(request, *args, **kwargs) - # pylint: disable=arguments-differ @course_author_access_required @expect_json_in_class_view @@ -77,17 +63,6 @@ class XblockCreateView(DeveloperErrorViewMixin, CreateAPIView): """ serializer_class = XblockSerializer - def dispatch(self, request, *args, **kwargs): - # TODO: probably want to refactor this to a decorator. - """ - The dispatch method of a View class handles HTTP requests in general - and calls other methods to handle specific HTTP methods. - We use this to raise a 404 if the content api is disabled. - """ - if not toggles.use_studio_content_api(): - raise Http404 - return super().dispatch(request, *args, **kwargs) - # pylint: disable=arguments-differ @csrf_exempt @course_author_access_required diff --git a/cms/djangoapps/contentstore/toggles.py b/cms/djangoapps/contentstore/toggles.py index af125e0d9b7a..4fcc8372c594 100644 --- a/cms/djangoapps/contentstore/toggles.py +++ b/cms/djangoapps/contentstore/toggles.py @@ -5,6 +5,7 @@ from openedx.core.djangoapps.content.search import api as search_api from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag + # .. toggle_name: FEATURES['ENABLE_EXPORT_GIT'] # .. toggle_implementation: SettingDictToggle # .. toggle_default: False @@ -160,37 +161,6 @@ def use_new_problem_editor(): return ENABLE_NEW_PROBLEM_EDITOR_FLAG.is_enabled() -# .. toggle_name: new_core_editors.use_advanced_problem_editor -# .. toggle_implementation: WaffleFlag -# .. toggle_default: False -# .. toggle_description: This flag enables the use of the new core problem xblock advanced editor as the default -# .. toggle_use_cases: temporary -# .. toggle_creation_date: 2024-07-25 -# .. toggle_target_removal_date: 2024-08-31 -# .. toggle_tickets: TNL-11694 -# .. toggle_warning: -ENABLE_DEFAULT_ADVANCED_PROBLEM_EDITOR_FLAG = WaffleFlag('new_core_editors.use_advanced_problem_editor', __name__) - - -# .. toggle_name: new_editors.add_game_block_button -# .. toggle_implementation: WaffleFlag -# .. toggle_default: False -# .. toggle_description: This flag enables the creation of the new games block -# .. toggle_use_cases: temporary -# .. toggle_creation_date: 2023-07-26 -# .. toggle_target_removal_date: 2023-09-31 -# .. toggle_tickets: TNL-10924 -# .. toggle_warning: -ENABLE_ADD_GAME_BLOCK_FLAG = WaffleFlag('new_editors.add_game_block_button', __name__) - - -def use_add_game_block(): - """ - Returns a boolean if add game block button is enabled - """ - return ENABLE_ADD_GAME_BLOCK_FLAG.is_enabled() - - # .. toggle_name: contentstore.individualize_anonymous_user_id # .. toggle_implementation: CourseWaffleFlag # .. toggle_default: False @@ -211,30 +181,6 @@ def individualize_anonymous_user_id(course_id): return INDIVIDUALIZE_ANONYMOUS_USER_ID.is_enabled(course_id) -# .. toggle_name: contentstore.enable_studio_content_api -# .. toggle_implementation: WaffleFlag -# .. toggle_default: False -# .. toggle_description: Enables the new (experimental and unsafe!) Studio Content REST API for course authors, -# .. which provides CRUD capabilities for course content and xblock editing. -# .. Use at your own peril - you can easily delete learner data when editing running courses. -# .. This can be triggered by deleting blocks, editing subsections, problems, assignments, discussions, -# .. creating new problems or graded sections, and by other things you do. -# .. toggle_use_cases: open_edx -# .. toggle_creation_date: 2023-05-26 -# .. toggle_tickets: TNL-10208 -ENABLE_STUDIO_CONTENT_API = WaffleFlag( - f'{CONTENTSTORE_NAMESPACE}.enable_studio_content_api', - __name__, -) - - -def use_studio_content_api(): - """ - Returns a boolean if studio editing API is enabled - """ - return ENABLE_STUDIO_CONTENT_API.is_enabled() - - # .. toggle_name: new_studio_mfe.use_new_home_page # .. toggle_implementation: WaffleFlag # .. toggle_default: False diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index 93e382fb7fba..ef85c446c81e 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -560,9 +560,6 @@ def assert_xblock_info(xblock, xblock_info): else: self.assertNotIn("ancestors", response) xblock_info = get_block_info(xblock) - # TODO: remove after beta testing for the new problem editor parser - if xblock_info["category"] == "problem": - xblock_info["metadata"]["default_to_advanced"] = False self.assertEqual(xblock_info, response) diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index 19e20804c2fb..3506825ae357 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -33,7 +33,6 @@ from xblock.fields import Scope from cms.djangoapps.contentstore.config.waffle import SHOW_REVIEW_RULES_FLAG -from cms.djangoapps.contentstore.toggles import ENABLE_DEFAULT_ADVANCED_PROBLEM_EDITOR_FLAG from cms.djangoapps.models.settings.course_grading import CourseGradingModel from cms.lib.ai_aside_summary_config import AiAsideSummaryConfig from cms.lib.xblock.upstream_sync import BadUpstream, sync_from_upstream @@ -187,11 +186,6 @@ def handle_xblock(request, usage_key_string=None): # TODO: pass fields to get_block_info and only return those with modulestore().bulk_operations(usage_key.course_key): response = get_block_info(get_xblock(usage_key, request.user)) - # TODO: remove after beta testing for the new problem editor parser - if response["category"] == "problem": - response["metadata"]["default_to_advanced"] = ( - ENABLE_DEFAULT_ADVANCED_PROBLEM_EDITOR_FLAG.is_enabled() - ) if "customReadToken" in fields: parent_children = _get_block_parent_children(get_xblock(usage_key, request.user)) response.update(parent_children) diff --git a/cms/envs/common.py b/cms/envs/common.py index 6cc193827170..9be3eb9e0956 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1317,16 +1317,6 @@ ##### custom vendor plugin variables ##### -# .. setting_name: JS_ENV_EXTRA_CONFIG -# .. setting_default: {} -# .. setting_description: JavaScript code can access this dictionary using `process.env.JS_ENV_EXTRA_CONFIG` -# One of the current use cases for this is enabling custom TinyMCE plugins -# (TINYMCE_ADDITIONAL_PLUGINS) and overriding the TinyMCE configuration (TINYMCE_CONFIG_OVERRIDES). -# .. setting_warning: This Django setting is DEPRECATED! Starting in Sumac, Webpack will no longer -# use Django settings. Please set the JS_ENV_EXTRA_CONFIG environment variable to an equivalent JSON -# string instead. For details, see: https://github.com/openedx/edx-platform/issues/31895 -JS_ENV_EXTRA_CONFIG = json.loads(os.environ.get('JS_ENV_EXTRA_CONFIG', '{}')) - ############################### PIPELINE ####################################### PIPELINE = { @@ -1508,14 +1498,6 @@ } } -# .. setting_name: WEBPACK_CONFIG_PATH -# .. setting_default: "webpack.prod.config.js" -# .. setting_description: Path to the Webpack configuration file. Used by Paver scripts. -# .. setting_warning: This Django setting is DEPRECATED! Starting in Sumac, Webpack will no longer -# use Django settings. Please set the WEBPACK_CONFIG_PATH environment variable instead. For details, -# see: https://github.com/openedx/edx-platform/issues/31895 -WEBPACK_CONFIG_PATH = os.environ.get('WEBPACK_CONFIG_PATH', 'webpack.prod.config.js') - ############################ SERVICE_VARIANT ################################## diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index ac21c2ee01a3..63dff1fbb9c2 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -66,9 +66,6 @@ 'django.contrib.staticfiles.finders.AppDirectoriesFinder', ] -# Load development webpack donfiguration -WEBPACK_CONFIG_PATH = 'webpack.dev.config.js' - ############################ PYFS XBLOCKS SERVICE ############################# # Set configuration for Django pyfilesystem diff --git a/cms/envs/mock.yml b/cms/envs/mock.yml index 6e37b1818066..fb2233501ef4 100644 --- a/cms/envs/mock.yml +++ b/cms/envs/mock.yml @@ -501,7 +501,6 @@ IDA_LOGOUT_URI_LIST: - https://discovery.localhost/logout/ - https://commerce-coordinator.localhost/logout/ ID_VERIFICATION_SUPPORT_LINK: https://support.localhost/hc/en-us/articles/206503858-How-do-I-complete-photo-verification- -JS_ENV_EXTRA_CONFIG: {} JWT_AUTH: JWT_AUDIENCE: test_jwt_audience JWT_AUTH_COOKIE_HEADER_PAYLOAD: jwt-cookie-header-payload diff --git a/common/templates/xblock_v2/xblock_iframe.html b/common/templates/xblock_v2/xblock_iframe.html index add3c46bce55..69af639da2a8 100644 --- a/common/templates/xblock_v2/xblock_iframe.html +++ b/common/templates/xblock_v2/xblock_iframe.html @@ -180,8 +180,8 @@ - -
+ +
{{ fragment.body_html | safe }} @@ -360,7 +360,7 @@ error_message_div.html('Error: '+response.message); error_message_div.css('display', 'block'); } - }); + }); }); }); } @@ -437,21 +437,55 @@ const rootNode = document.querySelector('.xblock, .xblock-v1'); // will always return the first matching element initializeXBlockAndChildren(rootNode, () => { }); + (function() { + // If this view is rendered in an iframe within the authoring microfrontend app + // it will report the height of its contents to the parent window when the + // document loads, window resizes, or DOM mutates. + if (window !== window.parent) { + var lastHeight = window.parent[0].offsetHeight; + var lastWidth = window.parent[0].offsetWidth; - let lastHeight = -1; - function checkFrameHeight() { - const newHeight = document.documentElement.scrollHeight; - if (newHeight !== lastHeight) { - lastHeight = newHeight; + function dispatchResizeMessage(event) { + // Note: event is actually an Array of MutationRecord objects when fired from the MutationObserver + var newHeight = rootNode.scrollHeight; + var newWidth = rootNode.offsetWidth; + + window.parent.postMessage( + { + type: 'plugin.resize', + payload: { + width: newWidth, + height: newHeight, + } + }, document.referrer + ); + + lastHeight = newHeight; + lastWidth = newWidth; + + // Within the authoring microfrontend the iframe resizes to match the + // height of this document and it should never scroll. It does scroll + // ocassionally when javascript is used to focus elements on the page + // before the parent iframe has been resized to match the content + // height. This window.scrollTo is an attempt to keep the content at the + // top of the page. + window.scrollTo(0, 0); + } + + // Create an observer instance linked to the callback function + const observer = new MutationObserver(dispatchResizeMessage); + + // Start observing the target node for configured mutations + observer.observe(rootNode, { attributes: true, childList: true, subtree: true }); + + const resizeObserver = new ResizeObserver(dispatchResizeMessage); + resizeObserver.observe(rootNode); } - } - // Check the size whenever the DOM changes: - new MutationObserver(checkFrameHeight).observe(document.body, { attributes: true, childList: true, subtree: true }); - // And whenever the IFrame is resized - window.addEventListener('resize', checkFrameHeight); + }()); } window.addEventListener('load', blockFrameJS); + diff --git a/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py b/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py index b6840ca2c0b0..7b3f3f598bfe 100644 --- a/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py +++ b/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py @@ -49,6 +49,9 @@ def send_ace_message(goal, session_id): Returns true if sent, false if it absorbed an exception and did not send """ user = goal.user + if not user.has_usable_password(): + log.info(f'Goal Reminder User is disabled {user.username} course {goal.course_key}') + return False try: course = CourseOverview.get_from_id(goal.course_key) except CourseOverview.DoesNotExist: diff --git a/lms/djangoapps/course_goals/management/commands/tests/test_goal_reminder_email.py b/lms/djangoapps/course_goals/management/commands/tests/test_goal_reminder_email.py index 214fb58219ea..46d46832a6d9 100644 --- a/lms/djangoapps/course_goals/management/commands/tests/test_goal_reminder_email.py +++ b/lms/djangoapps/course_goals/management/commands/tests/test_goal_reminder_email.py @@ -1,5 +1,5 @@ """Tests for the goal_reminder_email command""" - +import uuid from datetime import datetime from botocore.exceptions import NoCredentialsError @@ -18,7 +18,7 @@ from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory -from lms.djangoapps.course_goals.management.commands.goal_reminder_email import send_email_using_ses +from lms.djangoapps.course_goals.management.commands.goal_reminder_email import send_email_using_ses, send_ace_message from lms.djangoapps.course_goals.models import CourseGoalReminderStatus from lms.djangoapps.course_goals.tests.factories import ( CourseGoalFactory, CourseGoalReminderStatusFactory, UserActivityFactory, @@ -220,6 +220,21 @@ def test_params_without_ses(self, mock_ace): assert 'override_default_channel' not in msg.options assert 'from_address' not in msg.options + @ddt.data(True, False) + @mock.patch('lms.djangoapps.course_goals.management.commands.goal_reminder_email.ace.send') + def test_goal_reminder_email_sent_to_disable_user(self, value, mock_ace): + """ + Test that the goal reminder email is not sent to disabled users. + """ + goal = self.make_valid_goal() + if value: + goal.user.set_password("12345678") + else: + goal.user.set_unusable_password() + goal.user.save() + send_ace_message(goal, str(uuid.uuid4())) + assert mock_ace.called is value + class TestGoalReminderEmailSES(TestCase): """ diff --git a/lms/djangoapps/course_home_api/outline/tests/test_view.py b/lms/djangoapps/course_home_api/outline/tests/test_view.py index 76928846f080..fd1680ba533d 100644 --- a/lms/djangoapps/course_home_api/outline/tests/test_view.py +++ b/lms/djangoapps/course_home_api/outline/tests/test_view.py @@ -31,7 +31,6 @@ from openedx.features.course_duration_limits.models import CourseDurationLimitConfig from openedx.features.course_experience import ( COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, - DISPLAY_COURSE_SOCK_FLAG, ENABLE_COURSE_GOALS ) from openedx.features.discounts.applicability import ( @@ -362,12 +361,6 @@ def test_visibility(self, is_enrolled, is_staff, course_visibility): assert (data['access_expiration'] is not None) == show_enrolled assert (data['resume_course']['url'] is not None) == show_enrolled - @ddt.data(True, False) - def test_can_show_upgrade_sock(self, sock_enabled): - with override_waffle_flag(DISPLAY_COURSE_SOCK_FLAG, active=sock_enabled): - response = self.client.get(self.url) - assert response.data['can_show_upgrade_sock'] == sock_enabled - def test_verified_mode(self): enrollment = CourseEnrollment.enroll(self.user, self.course.id) CourseDurationLimitConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1)) diff --git a/lms/djangoapps/course_home_api/serializers.py b/lms/djangoapps/course_home_api/serializers.py index d3aa5617331e..44023c9d50db 100644 --- a/lms/djangoapps/course_home_api/serializers.py +++ b/lms/djangoapps/course_home_api/serializers.py @@ -9,7 +9,6 @@ from lms.djangoapps.courseware.utils import verified_upgrade_deadline_link from openedx.core.djangoapps.courseware_api.utils import serialize_upgrade_info from openedx.features.content_type_gating.models import ContentTypeGatingConfig -from openedx.features.course_experience import DISPLAY_COURSE_SOCK_FLAG from openedx.features.course_experience.utils import dates_banner_should_display @@ -59,13 +58,8 @@ class VerifiedModeSerializer(ReadOnlySerializer): Requires 'course_overview', 'enrollment', and 'request' from self.context. """ - can_show_upgrade_sock = serializers.SerializerMethodField() verified_mode = serializers.SerializerMethodField() - def get_can_show_upgrade_sock(self, _): - course_overview = self.context['course_overview'] - return DISPLAY_COURSE_SOCK_FLAG.is_enabled(course_overview.id) - def get_verified_mode(self, _): """Return verified mode information, or None.""" course_overview = self.context['course_overview'] diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 62bd6bf5c982..5dc4a7ce9107 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -795,10 +795,11 @@ def get_assignments_grades(user, course_id, cache_timeout): collected_block_structure = get_block_structure_manager(course_id).get_collected() total_bytes_in_one_mb = 1024 * 1024 - data_size_in_mbs = round(len(pickle.dumps(collected_block_structure)) / total_bytes_in_one_mb, 2) - if data_size_in_mbs < total_bytes_in_one_mb * 2: + data_size_in_bytes = len(pickle.dumps(collected_block_structure)) + if data_size_in_bytes < total_bytes_in_one_mb * 2: cache.set(cache_key, collected_block_structure, cache_timeout) else: + data_size_in_mbs = round(data_size_in_bytes / total_bytes_in_one_mb, 2) # .. custom_attribute_name: collected_block_structure_size_in_mbs # .. custom_attribute_description: contains the data chunk size in MBs. The size on which # the memcached client failed to store value in cache. diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index da31ebccce45..6303f07cf351 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -341,7 +341,7 @@ def test_index_query_counts(self): self.client.login(username=self.user.username, password=self.user_password) CourseEnrollment.enroll(self.user, course.id) - with self.assertNumQueries(154, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(152, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): with check_mongo_calls(3): url = reverse( 'courseware_section', diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index 7a9242f595ef..e3bd2b34634f 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -48,7 +48,6 @@ default_course_url ) from openedx.features.course_experience.url_helpers import make_learning_mfe_courseware_url -from openedx.features.course_experience.views.course_sock import CourseSockFragmentView from openedx.features.enterprise_support.api import data_sharing_consent_required from ..access import has_access @@ -454,9 +453,6 @@ def _create_courseware_context(self, request): table_of_contents['chapters'], ) - courseware_context['course_sock_fragment'] = CourseSockFragmentView().render_to_fragment( - request, course=self.course) - # entrance exam data self._add_entrance_exam_to_context(courseware_context) diff --git a/lms/envs/common.py b/lms/envs/common.py index 316dad98da3f..719e12fb8b0d 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2806,14 +2806,6 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring } } -# .. setting_name: WEBPACK_CONFIG_PATH -# .. setting_default: "webpack.prod.config.js" -# .. setting_description: Path to the Webpack configuration file. Used by Paver scripts. -# .. setting_warning: This Django setting is DEPRECATED! Starting in Sumac, Webpack will no longer -# use Django settings. Please set the WEBPACK_CONFIG_PATH environment variable instead. For details, -# see: https://github.com/openedx/edx-platform/issues/31895 -WEBPACK_CONFIG_PATH = os.environ.get('WEBPACK_CONFIG_PATH', 'webpack.prod.config.js') - ########################## DJANGO DEBUG TOOLBAR ############################### # We don't enable Django Debug Toolbar universally, but whenever we do, we want diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index d607073b9a29..e024e81edfef 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -136,9 +136,6 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing PIPELINE['SASS_ARGUMENTS'] = '--debug-info' -# Load development webpack configuration -WEBPACK_CONFIG_PATH = 'webpack.dev.config.js' - ########################### VERIFIED CERTIFICATES ################################# FEATURES['AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING'] = True diff --git a/lms/static/sass/_build-course.scss b/lms/static/sass/_build-course.scss index 732eb2caf888..47fde243c7f6 100644 --- a/lms/static/sass/_build-course.scss +++ b/lms/static/sass/_build-course.scss @@ -66,6 +66,3 @@ // responsive @import 'base/layouts'; // temporary spot for responsive course @import 'header'; - -// features -@import 'features/course-sock'; diff --git a/lms/static/sass/bootstrap/lms-main.scss b/lms/static/sass/bootstrap/lms-main.scss index 70bd33da28eb..1299b84fa289 100644 --- a/lms/static/sass/bootstrap/lms-main.scss +++ b/lms/static/sass/bootstrap/lms-main.scss @@ -28,7 +28,6 @@ $static-path: '../..'; @import 'features/bookmarks'; @import 'features/course-experience'; @import 'features/course-search'; -@import 'features/course-sock'; @import 'features/course-upgrade-message'; @import 'features/course-duration-limits'; diff --git a/lms/static/sass/features/_course-sock.scss b/lms/static/sass/features/_course-sock.scss deleted file mode 100644 index a3a1f1484d75..000000000000 --- a/lms/static/sass/features/_course-sock.scss +++ /dev/null @@ -1,170 +0,0 @@ -.verification-sock { - display: inline-block; - position: relative; - width: 100%; - max-width: map-get($container-max-widths, xl); - margin: $baseline auto 0; - -webkit-transition: all 0.4s ease-out; - -moz-transition: all 0.4s ease-out; - -o-transition: all 0.4s ease-out; - -ms-transition: all 0.4s ease-out; - transition: all 0.4s ease-out; - - .action-toggle-verification-sock { - @include left(50%); - @include margin-left(-1 * $baseline * 15/2); - - position: absolute; - top: (-1 * $baseline); - width: ($baseline * 15); - color: theme-color("inverse"); - background-color: theme-color("success"); - border-color: theme-color("success"); - background-image: none; - box-shadow: none; - -webkit-transition: background-color 0.5s; - transition: background-color 0.5s; - cursor: pointer; - - &.active, - &:focus, - &:hover { - color: theme-color("success"); - background-color: theme-color("inverse"); - border-color: theme-color("success"); - background-image: none; - box-shadow: none; - } - } - - .verification-main-panel { - display: none; - overflow: hidden; - border-top: 1px solid $border-color; - padding: ($baseline * 5/2) ($baseline * 2); - -webkit-transition: height ease-out; - transition: height ease-out; - - .verification-desc-panel { - color: $black-t3; - position: relative; - - h2 { - font-size: 1.5rem; - font-weight: $font-weight-bold; - } - - h3 { - font-size: 1.25rem; - } - - @media (max-width: 960px) { - .mini-cert { - display: none; - border: 1px solid $black-t0; - } - } - - .mini-cert { - @include right($baseline); - - position: absolute; - top: $baseline; - width: ($baseline * 13); - } - - .learner-story-container { - display: flex; - max-width: 630px; - - .student-image { - margin: ($baseline / 4) $baseline 0 0; - height: ($baseline * 5/2); - width: ($baseline * 5/2); - } - - .story-quote > .author { - display: block; - margin-top: ($baseline / 4); - font-weight: 600; - } - - &:not(:first-child) { - margin-top: ($baseline * 2); - } - } - - .action-upgrade-certificate { - position: absolute; - right: $baseline; - background-color: theme-color("success"); - border-color: theme-color("success"); - color: theme-color("inverse"); - background-image: none; - box-shadow: none; - cursor: pointer; - - &:hover { - background-color: theme-color("inverse"); - color: theme-color("success"); - } - - @media (max-width: 960px) { - & { - position: relative; - margin-top: ($baseline * 2); - } - } - - @media (min-width: 960px) { - &.stuck-top { - bottom: auto; - top: $baseline * (52 / 5); - } - - &.stuck-bottom { - top: auto; - bottom: $baseline * (-1 * 3/2); - } - - &.attached { - @include right($baseline); - - position: fixed; - bottom: $baseline; - top: auto; - } - } - } - } - } -} - -// Overrides for the courseware page. -.view-courseware { - .verification-sock { - margin-top: 0; - border-top: none; - border-bottom: none; - - .action-toggle-verification-sock { - top: (-1 * $baseline * 5/4); - - &:not(.active) { - color: theme-color("inverse"); - background-color: theme-color("success"); - box-shadow: none; - border: 1px solid theme-color("success"); - - &:hover { - background-color: $success-color-hover; - } - } - } - - .verification-main-panel { - border-top: 0; - border-bottom: 1px solid $border-color; - } - } -} diff --git a/lms/templates/courseware/courseware.html b/lms/templates/courseware/courseware.html index eb94fab17c91..ce9dd19b2934 100644 --- a/lms/templates/courseware/courseware.html +++ b/lms/templates/courseware/courseware.html @@ -290,7 +290,6 @@
- ${HTML(course_sock_fragment.body_html())}
% if settings.FEATURES.get("LICENSING", False): @@ -317,36 +316,3 @@ % endif % endif - -<%static:require_module_async module_name="js/commerce/track_ecommerce_events" class_name="TrackECommerceEvents"> - - var fbeLink = $("#FBE_banner"); - var welcomeLink = $("#welcome"); - var accessDeniedUpsellLink = $("#accessDeniedUpsell"); - var sockLink = $("#sock"); - - TrackECommerceEvents.trackUpsellClick(fbeLink, 'in_course_audit_access_expires', { - pageName: "in_course", - linkType: "link", - linkCategory: "FBE_banner" - }); - - TrackECommerceEvents.trackUpsellClick(welcomeLink, 'in_course_welcome', { - pageName: "in_course", - linkType: "link", - linkCategory: "welcome" - }); - - TrackECommerceEvents.trackUpsellClick(accessDeniedUpsellLink, 'in_course_upgrade', { - pageName: "in_course", - linkType: "link", - linkCategory: "(none)" - }); - - TrackECommerceEvents.trackUpsellClick(sockLink, 'in_course_sock', { - pageName: "in_course", - linkType: "button", - linkCategory: "green_upgrade" - }); - - diff --git a/openedx/core/djangoapps/content/block_structure/store.py b/openedx/core/djangoapps/content/block_structure/store.py index 409dae988e92..bb6359b9d3a3 100644 --- a/openedx/core/djangoapps/content/block_structure/store.py +++ b/openedx/core/djangoapps/content/block_structure/store.py @@ -137,8 +137,9 @@ def _add_to_cache(self, serialized_data, bs_model): """ cache_key = self._encode_root_cache_key(bs_model) total_bytes_in_one_mb = 1024 * 1024 - data_size_in_mbs = round(len(serialized_data) / total_bytes_in_one_mb, 2) - if data_size_in_mbs < total_bytes_in_one_mb * 2: + data_size_in_bytes = len(serialized_data) + data_size_in_mbs = round(data_size_in_bytes / total_bytes_in_one_mb, 2) + if data_size_in_bytes < total_bytes_in_one_mb * 2: self._cache.set(cache_key, serialized_data, timeout=config.cache_timeout_in_seconds()) logger.info("BlockStructure: Added to cache; %s, size: %.2fMB", bs_model, data_size_in_mbs) else: diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index a597ae51e865..49bf4be7687d 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -17,8 +17,12 @@ from meilisearch import Client as MeilisearchClient from meilisearch.errors import MeilisearchApiError, MeilisearchError from meilisearch.models.task import TaskInfo -from opaque_keys.edx.keys import UsageKey -from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryCollectionLocator +from opaque_keys.edx.keys import UsageKey, OpaqueKey +from opaque_keys.edx.locator import ( + LibraryCollectionLocator, + LibraryContainerLocator, + LibraryLocatorV2, +) from openedx_learning.api import authoring as authoring_api from common.djangoapps.student.roles import GlobalStaff from rest_framework.request import Request @@ -42,7 +46,7 @@ searchable_doc_for_collection, searchable_doc_for_container, searchable_doc_for_library_block, - searchable_doc_for_usage_key, + searchable_doc_for_key, searchable_doc_collections, searchable_doc_tags, searchable_doc_tags_for_collection, @@ -461,8 +465,9 @@ def index_collection_batch(batch, num_done, library_key) -> int: docs = [] for collection in batch: try: - doc = searchable_doc_for_collection(library_key, collection.key, collection=collection) - doc.update(searchable_doc_tags_for_collection(library_key, collection.key)) + collection_key = lib_api.library_collection_locator(library_key, collection.key) + doc = searchable_doc_for_collection(collection_key, collection=collection) + doc.update(searchable_doc_tags_for_collection(collection_key)) docs.append(doc) except Exception as err: # pylint: disable=broad-except status_cb(f"Error indexing collection {collection}: {err}") @@ -486,8 +491,7 @@ def index_container_batch(batch, num_done, library_key) -> int: container, ) doc = searchable_doc_for_container(container_key) - # TODO: when we add container tags - # doc.update(searchable_doc_tags_for_container(container_key)) + doc.update(searchable_doc_tags(container_key)) docs.append(doc) except Exception as err: # pylint: disable=broad-except status_cb(f"Error indexing container {container.key}: {err}") @@ -511,7 +515,7 @@ def index_container_batch(batch, num_done, library_key) -> int: collections = authoring_api.get_collections(library.learning_package_id, enabled=True) num_collections = collections.count() num_collections_done = 0 - status_cb(f"{num_collections_done + 1}/{num_collections}. Now indexing collections in library {lib_key}") + status_cb(f"{num_collections_done}/{num_collections}. Now indexing collections in library {lib_key}") paginator = Paginator(collections, 100) for p in paginator.page_range: num_collections_done = index_collection_batch( @@ -613,14 +617,14 @@ def add_with_children(block): _update_index_docs(docs) -def delete_index_doc(usage_key: UsageKey) -> None: +def delete_index_doc(key: OpaqueKey) -> None: """ Deletes the document for the given XBlock from the search index Args: - usage_key (UsageKey): The usage key of the XBlock to be removed from the index + key (OpaqueKey): The opaque key of the XBlock/Container to be removed from the index """ - doc = searchable_doc_for_usage_key(usage_key) + doc = searchable_doc_for_key(key) _delete_index_doc(doc[Fields.id]) @@ -703,13 +707,13 @@ def _get_document_from_index(document_id: str) -> dict: return document -def upsert_library_collection_index_doc(library_key: LibraryLocatorV2, collection_key: str) -> None: +def upsert_library_collection_index_doc(collection_key: LibraryCollectionLocator) -> None: """ Creates, updates, or deletes the document for the given Library Collection in the search index. If the Collection is not found or disabled (i.e. soft-deleted), then delete it from the search index. """ - doc = searchable_doc_for_collection(library_key, collection_key) + doc = searchable_doc_for_collection(collection_key) update_components = False # Soft-deleted/disabled collections are removed from the index @@ -739,12 +743,11 @@ def upsert_library_collection_index_doc(library_key: LibraryLocatorV2, collectio if update_components: from .tasks import update_library_components_collections as update_task - update_task.delay(str(library_key), collection_key) + update_task.delay(str(collection_key)) def update_library_components_collections( - library_key: LibraryLocatorV2, - collection_key: str, + collection_key: LibraryCollectionLocator, batch_size: int = 1000, ) -> None: """ @@ -752,8 +755,12 @@ def update_library_components_collections( Because there may be a lot of components, we send these updates to Meilisearch in batches. """ + library_key = collection_key.library_key library = lib_api.get_library(library_key) - components = authoring_api.get_collection_components(library.learning_package_id, collection_key) + components = authoring_api.get_collection_components( + library.learning_package_id, + collection_key.collection_id, + ) paginator = Paginator(components, batch_size) for page in paginator.page_range: @@ -811,12 +818,12 @@ def upsert_content_library_index_docs(library_key: LibraryLocatorV2) -> None: _update_index_docs(docs) -def upsert_block_tags_index_docs(usage_key: UsageKey): +def upsert_content_object_tags_index_doc(key: OpaqueKey): """ - Updates the tags data in documents for the given Course/Library block + Updates the tags data in document for the given Course/Library item """ - doc = {Fields.id: meili_id_from_opaque_key(usage_key)} - doc.update(searchable_doc_tags(usage_key)) + doc = {Fields.id: meili_id_from_opaque_key(key)} + doc.update(searchable_doc_tags(key)) _update_index_docs([doc]) @@ -829,12 +836,12 @@ def upsert_block_collections_index_docs(usage_key: UsageKey): _update_index_docs([doc]) -def upsert_collection_tags_index_docs(collection_usage_key: LibraryCollectionLocator): +def upsert_collection_tags_index_docs(collection_key: LibraryCollectionLocator): """ Updates the tags data in documents for the given library collection """ - doc = searchable_doc_tags_for_collection(collection_usage_key.library_key, collection_usage_key.collection_id) + doc = searchable_doc_tags_for_collection(collection_key) _update_index_docs([doc]) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 061015751955..1c093034271d 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -8,8 +8,8 @@ from django.core.exceptions import ObjectDoesNotExist from django.utils.text import slugify -from opaque_keys.edx.keys import LearningContextKey, UsageKey -from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2 +from opaque_keys.edx.keys import LearningContextKey, UsageKey, OpaqueKey +from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator from openedx_learning.api import authoring as authoring_api from openedx_learning.api.authoring_models import Collection from rest_framework.exceptions import NotFound @@ -113,7 +113,7 @@ class PublishStatus: modified = "modified" -def meili_id_from_opaque_key(usage_key: UsageKey) -> str: +def meili_id_from_opaque_key(key: OpaqueKey) -> str: """ Meilisearch requires each document to have a primary key that's either an integer or a string composed of alphanumeric characters (a-z A-Z 0-9), @@ -124,7 +124,7 @@ def meili_id_from_opaque_key(usage_key: UsageKey) -> str: we could use PublishableEntity's primary key / UUID instead. """ # The slugified key _may_ not be unique so we append a hashed string to make it unique: - key_str = str(usage_key) + key_str = str(key) key_bin = key_str.encode() suffix = blake2b(key_bin, digest_size=4, usedforsecurity=False).hexdigest() @@ -140,12 +140,12 @@ def _meili_access_id_from_context_key(context_key: LearningContextKey) -> int: return access.id -def searchable_doc_for_usage_key(usage_key: UsageKey) -> dict: +def searchable_doc_for_key(key: OpaqueKey) -> dict: """ - Generates a base document identified by its usage key. + Generates a base document identified by its opaque key. """ return { - Fields.id: meili_id_from_opaque_key(usage_key), + Fields.id: meili_id_from_opaque_key(key), } @@ -244,7 +244,7 @@ class implementation returns only: return block_data -def _tags_for_content_object(object_id: UsageKey | LearningContextKey) -> dict: +def _tags_for_content_object(object_id: OpaqueKey) -> dict: """ Given an XBlock, course, library, etc., get the tag data for its index doc. @@ -406,7 +406,7 @@ def searchable_doc_for_library_block(xblock_metadata: lib_api.LibraryXBlockMetad block_published = None publish_status = PublishStatus.never - doc = searchable_doc_for_usage_key(xblock_metadata.usage_key) + doc = searchable_doc_for_key(xblock_metadata.usage_key) doc.update({ Fields.type: DocType.library_block, Fields.breadcrumbs: [], @@ -427,13 +427,13 @@ def searchable_doc_for_library_block(xblock_metadata: lib_api.LibraryXBlockMetad return doc -def searchable_doc_tags(usage_key: UsageKey) -> dict: +def searchable_doc_tags(key: OpaqueKey) -> dict: """ Generate a dictionary document suitable for ingestion into a search engine like Meilisearch or Elasticsearch, with the tags data for the given content object. """ - doc = searchable_doc_for_usage_key(usage_key) - doc.update(_tags_for_content_object(usage_key)) + doc = searchable_doc_for_key(key) + doc.update(_tags_for_content_object(key)) return doc @@ -443,26 +443,21 @@ def searchable_doc_collections(usage_key: UsageKey) -> dict: Generate a dictionary document suitable for ingestion into a search engine like Meilisearch or Elasticsearch, with the collections data for the given content object. """ - doc = searchable_doc_for_usage_key(usage_key) + doc = searchable_doc_for_key(usage_key) doc.update(_collections_for_content_object(usage_key)) return doc def searchable_doc_tags_for_collection( - library_key: LibraryLocatorV2, - collection_key: str, + collection_key: LibraryCollectionLocator ) -> dict: """ Generate a dictionary document suitable for ingestion into a search engine like Meilisearch or Elasticsearch, with the tags data for the given library collection. """ - collection_usage_key = lib_api.get_library_collection_usage_key( - library_key, - collection_key, - ) - doc = searchable_doc_for_usage_key(collection_usage_key) - doc.update(_tags_for_content_object(collection_usage_key)) + doc = searchable_doc_for_key(collection_key) + doc.update(_tags_for_content_object(collection_key)) return doc @@ -473,7 +468,7 @@ def searchable_doc_for_course_block(block) -> dict: like Meilisearch or Elasticsearch, so that the given course block can be found using faceted search. """ - doc = searchable_doc_for_usage_key(block.usage_key) + doc = searchable_doc_for_key(block.usage_key) doc.update({ Fields.type: DocType.course_block, }) @@ -484,8 +479,7 @@ def searchable_doc_for_course_block(block) -> dict: def searchable_doc_for_collection( - library_key: LibraryLocatorV2, - collection_key: str, + collection_key: LibraryCollectionLocator, *, # Optionally provide the collection if we've already fetched one collection: Collection | None = None, @@ -498,21 +492,16 @@ def searchable_doc_for_collection( If no collection is found for the given library_key + collection_key, the returned document will contain only basic information derived from the collection usage key, and no Fields.type value will be included in the returned dict. """ - collection_usage_key = lib_api.get_library_collection_usage_key( - library_key, - collection_key, - ) - - doc = searchable_doc_for_usage_key(collection_usage_key) + doc = searchable_doc_for_key(collection_key) try: - collection = collection or lib_api.get_library_collection_from_usage_key(collection_usage_key) + collection = collection or lib_api.get_library_collection_from_locator(collection_key) except lib_api.ContentLibraryCollectionNotFound: # Collection not found, so we can only return the base doc pass if collection: - assert collection.key == collection_key + assert collection.key == collection_key.collection_id draft_num_children = authoring_api.filter_publishable_entities( collection.entities, @@ -524,9 +513,9 @@ def searchable_doc_for_collection( ).count() doc.update({ - Fields.context_key: str(library_key), - Fields.org: str(library_key.org), - Fields.usage_key: str(collection_usage_key), + Fields.context_key: str(collection_key.library_key), + Fields.org: str(collection_key.org), + Fields.usage_key: str(collection_key), Fields.block_id: collection.key, Fields.type: DocType.collection, Fields.display_name: collection.title, @@ -537,7 +526,7 @@ def searchable_doc_for_collection( Fields.published: { Fields.published_num_children: published_num_children, }, - Fields.access_id: _meili_access_id_from_context_key(library_key), + Fields.access_id: _meili_access_id_from_context_key(collection_key.library_key), Fields.breadcrumbs: [{"display_name": collection.learning_package.title}], }) diff --git a/openedx/core/djangoapps/content/search/handlers.py b/openedx/core/djangoapps/content/search/handlers.py index 4565165e87ea..d817de1519bd 100644 --- a/openedx/core/djangoapps/content/search/handlers.py +++ b/openedx/core/djangoapps/content/search/handlers.py @@ -8,7 +8,7 @@ from django.dispatch import receiver from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import UsageKey -from opaque_keys.edx.locator import LibraryCollectionLocator +from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator from openedx_events.content_authoring.data import ( ContentLibraryData, ContentObjectChangedData, @@ -41,11 +41,12 @@ from .api import ( only_if_meilisearch_enabled, upsert_block_collections_index_docs, - upsert_block_tags_index_docs, + upsert_content_object_tags_index_doc, upsert_collection_tags_index_docs, ) from .tasks import ( delete_library_block_index_doc, + delete_library_container_index_doc, delete_xblock_index_doc, update_content_library_index_docs, update_library_collection_index_doc, @@ -186,16 +187,14 @@ def library_collection_updated_handler(**kwargs) -> None: if library_collection.background: update_library_collection_index_doc.delay( - str(library_collection.library_key), - library_collection.collection_key, + str(library_collection.collection_key), ) else: # Update collection index synchronously to make sure that search index is updated before # the frontend invalidates/refetches index. # See content_library_updated_handler for more details. update_library_collection_index_doc.apply(args=[ - str(library_collection.library_key), - library_collection.collection_key, + str(library_collection.collection_key), ]) @@ -211,15 +210,20 @@ def content_object_associations_changed_handler(**kwargs) -> None: return try: - # Check if valid if course or library block + # Check if valid course or library block usage_key = UsageKey.from_string(str(content_object.object_id)) except InvalidKeyError: try: - # Check if valid if library collection + # Check if valid library collection usage_key = LibraryCollectionLocator.from_string(str(content_object.object_id)) except InvalidKeyError: - log.error("Received invalid content object id") - return + try: + # Check if valid library container + usage_key = LibraryContainerLocator.from_string(str(content_object.object_id)) + except InvalidKeyError: + # Invalid content object id + log.error("Received invalid content object id") + return # This event's changes may contain both "tags" and "collections", but this will happen rarely, if ever. # So we allow a potential double "upsert" here. @@ -227,13 +231,12 @@ def content_object_associations_changed_handler(**kwargs) -> None: if isinstance(usage_key, LibraryCollectionLocator): upsert_collection_tags_index_docs(usage_key) else: - upsert_block_tags_index_docs(usage_key) + upsert_content_object_tags_index_doc(usage_key) if not content_object.changes or "collections" in content_object.changes: upsert_block_collections_index_docs(usage_key) @receiver(LIBRARY_CONTAINER_CREATED) -@receiver(LIBRARY_CONTAINER_DELETED) @receiver(LIBRARY_CONTAINER_UPDATED) @only_if_meilisearch_enabled def library_container_updated_handler(**kwargs) -> None: @@ -247,14 +250,28 @@ def library_container_updated_handler(**kwargs) -> None: if library_container.background: update_library_container_index_doc.delay( - str(library_container.library_key), - library_container.container_key, + str(library_container.container_key), ) else: # Update container index synchronously to make sure that search index is updated before # the frontend invalidates/refetches index. # See content_library_updated_handler for more details. update_library_container_index_doc.apply(args=[ - str(library_container.library_key), - library_container.container_key, + str(library_container.container_key), ]) + + +@receiver(LIBRARY_CONTAINER_DELETED) +@only_if_meilisearch_enabled +def library_container_deleted(**kwargs) -> None: + """ + Delete the index for the content library container + """ + library_container = kwargs.get("library_container", None) + if not library_container or not isinstance(library_container, LibraryContainerData): # pragma: no cover + log.error("Received null or incorrect data for event") + return + + # Update content library index synchronously to make sure that search index is updated before + # the frontend invalidates/refetches results. This is only a single document update so is very fast. + delete_library_container_index_doc.apply(args=[str(library_container.container_key)]) diff --git a/openedx/core/djangoapps/content/search/tasks.py b/openedx/core/djangoapps/content/search/tasks.py index f23ca9aa304e..48fc6b327c0a 100644 --- a/openedx/core/djangoapps/content/search/tasks.py +++ b/openedx/core/djangoapps/content/search/tasks.py @@ -11,7 +11,12 @@ from edx_django_utils.monitoring import set_code_owner_attribute from meilisearch.errors import MeilisearchError from opaque_keys.edx.keys import UsageKey -from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2 +from opaque_keys.edx.locator import ( + LibraryCollectionLocator, + LibraryContainerLocator, + LibraryLocatorV2, + LibraryUsageLocatorV2, +) from . import api @@ -88,39 +93,54 @@ def update_content_library_index_docs(library_key_str: str) -> None: @shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError)) @set_code_owner_attribute -def update_library_collection_index_doc(library_key_str: str, collection_key: str) -> None: +def update_library_collection_index_doc(collection_key_str: str) -> None: """ Celery task to update the content index document for a library collection """ - library_key = LibraryLocatorV2.from_string(library_key_str) + collection_key = LibraryCollectionLocator.from_string(collection_key_str) + library_key = collection_key.library_key log.info("Updating content index documents for collection %s in library%s", collection_key, library_key) - api.upsert_library_collection_index_doc(library_key, collection_key) + api.upsert_library_collection_index_doc(collection_key) @shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError)) @set_code_owner_attribute -def update_library_components_collections(library_key_str: str, collection_key: str) -> None: +def update_library_components_collections(collection_key_str: str) -> None: """ Celery task to update the "collections" field for components in the given content library collection. """ - library_key = LibraryLocatorV2.from_string(library_key_str) + collection_key = LibraryCollectionLocator.from_string(collection_key_str) + library_key = collection_key.library_key log.info("Updating document.collections for library %s collection %s components", library_key, collection_key) - api.update_library_components_collections(library_key, collection_key) + api.update_library_components_collections(collection_key) @shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError)) @set_code_owner_attribute -def update_library_container_index_doc(library_key_str: str, container_key_str: str) -> None: +def update_library_container_index_doc(container_key_str: str) -> None: """ Celery task to update the content index document for a library container """ - library_key = LibraryLocatorV2.from_string(library_key_str) container_key = LibraryContainerLocator.from_string(container_key_str) + library_key = container_key.library_key log.info("Updating content index documents for container %s in library%s", container_key, library_key) api.upsert_library_container_index_doc(container_key) + + +@shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError)) +@set_code_owner_attribute +def delete_library_container_index_doc(container_key_str: str) -> None: + """ + Celery task to delete the content index document for a library block + """ + container_key = LibraryContainerLocator.from_string(container_key_str) + + log.info("Deleting content index document for library block with id: %s", container_key) + + api.delete_index_doc(container_key) diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 813db0241db1..bd9fb803c1aa 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -8,6 +8,7 @@ from datetime import datetime, timezone from unittest.mock import MagicMock, Mock, call, patch from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.locator import LibraryCollectionLocator import ddt import pytest @@ -188,11 +189,13 @@ def setUp(self): created_by=None, description="my collection description" ) - self.collection_usage_key = "lib-collection:org1:lib:MYCOL" + self.collection_key = LibraryCollectionLocator.from_string( + "lib-collection:org1:lib:MYCOL", + ) self.collection_dict = { "id": "lib-collectionorg1libmycol-5b647617", "block_id": self.collection.key, - "usage_key": self.collection_usage_key, + "usage_key": str(self.collection_key), "type": "collection", "display_name": "my_collection", "description": "my collection description", @@ -234,7 +237,6 @@ def setUp(self): "modified": created_date.timestamp(), "access_id": lib_access.id, "breadcrumbs": [{"display_name": "Library"}], - # "tags" should be here but we haven't implemented them yet # "published" is not set since we haven't published it yet } @@ -262,6 +264,7 @@ def test_reindex_meilisearch(self, mock_meilisearch): doc_collection = copy.deepcopy(self.collection_dict) doc_collection["tags"] = {} doc_unit = copy.deepcopy(self.unit_dict) + doc_unit["tags"] = {} api.rebuild_index() assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 4 @@ -292,6 +295,7 @@ def test_reindex_meilisearch_incremental(self, mock_meilisearch): doc_collection = copy.deepcopy(self.collection_dict) doc_collection["tags"] = {} doc_unit = copy.deepcopy(self.unit_dict) + doc_unit["tags"] = {} api.rebuild_index(incremental=True) assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 4 @@ -472,8 +476,7 @@ def test_index_xblock_tags(self, mock_meilisearch): """ Test indexing an XBlock with tags. """ - - # Tag XBlock (these internally call `upsert_block_tags_index_docs`) + # Tag XBlock (these internally call `upsert_content_object_tags_index_doc`) tagging_api.tag_object(str(self.sequential.usage_key), self.taxonomyA, ["one", "two"]) tagging_api.tag_object(str(self.sequential.usage_key), self.taxonomyB, ["three", "four"]) @@ -737,8 +740,8 @@ def test_delete_all_drafts(self, mock_meilisearch): @override_settings(MEILISEARCH_ENABLED=True) def test_index_tags_in_collections(self, mock_meilisearch): # Tag collection - tagging_api.tag_object(self.collection_usage_key, self.taxonomyA, ["one", "two"]) - tagging_api.tag_object(self.collection_usage_key, self.taxonomyB, ["three", "four"]) + tagging_api.tag_object(str(self.collection_key), self.taxonomyA, ["one", "two"]) + tagging_api.tag_object(str(self.collection_key), self.taxonomyB, ["three", "four"]) # Build expected docs with tags at each stage doc_collection_with_tags1 = { @@ -866,3 +869,54 @@ def test_delete_collection(self, mock_meilisearch): mock_meilisearch.return_value.index.return_value.update_documents.assert_called_once_with([ doc_problem_without_collection, ]) + + @override_settings(MEILISEARCH_ENABLED=True) + def test_delete_index_container(self, mock_meilisearch): + """ + Test delete a container index. + """ + library_api.delete_container(self.unit.container_key) + + mock_meilisearch.return_value.index.return_value.delete_document.assert_called_once_with( + self.unit_dict["id"], + ) + + @override_settings(MEILISEARCH_ENABLED=True) + def test_index_library_container_metadata(self, mock_meilisearch): + """ + Test indexing a Library Container. + """ + api.upsert_library_container_index_doc(self.unit.container_key) + + mock_meilisearch.return_value.index.return_value.update_documents.assert_called_once_with([self.unit_dict]) + + @override_settings(MEILISEARCH_ENABLED=True) + def test_index_tags_in_containers(self, mock_meilisearch): + # Tag collection + tagging_api.tag_object(self.unit_key, self.taxonomyA, ["one", "two"]) + tagging_api.tag_object(self.unit_key, self.taxonomyB, ["three", "four"]) + + # Build expected docs with tags at each stage + doc_unit_with_tags1 = { + "id": "lctorg1libunitunit-1-e4527f7c", + "tags": { + 'taxonomy': ['A'], + 'level0': ['A > one', 'A > two'] + } + } + doc_unit_with_tags2 = { + "id": "lctorg1libunitunit-1-e4527f7c", + "tags": { + 'taxonomy': ['A', 'B'], + 'level0': ['A > one', 'A > two', 'B > four', 'B > three'] + } + } + + assert mock_meilisearch.return_value.index.return_value.update_documents.call_count == 2 + mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls( + [ + call([doc_unit_with_tags1]), + call([doc_unit_with_tags2]), + ], + any_order=True, + ) diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index a2964436d039..1f6c56f7d691 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -5,6 +5,7 @@ from datetime import datetime, timezone from freezegun import freeze_time +from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator from openedx_learning.api import authoring as authoring_api from organizations.models import Organization @@ -81,12 +82,24 @@ def setUpClass(cls): created_by=None, description="my toy collection description" ) - cls.collection_usage_key = "lib-collection:edX:2012_Fall:TOY_COLLECTION" + cls.collection_key = LibraryCollectionLocator.from_string( + "lib-collection:edX:2012_Fall:TOY_COLLECTION", + ) cls.library_block = library_api.create_library_block( cls.library.key, "html", "text2", ) + cls.container = library_api.create_container( + cls.library.key, + container_type=library_api.ContainerType.Unit, + slug="unit1", + title="A Unit in the Search Index", + user_id=None, + ) + cls.container_key = LibraryContainerLocator.from_string( + "lct:edX:2012_Fall:unit:unit1", + ) # Add the problem block to the collection library_api.update_library_collection_components( @@ -115,7 +128,8 @@ def setUpClass(cls): tagging_api.tag_object(str(cls.html_block_key), cls.subject_tags, tags=["Chinese", "Jump Links"]) tagging_api.tag_object(str(cls.html_block_key), cls.difficulty_tags, tags=["Normal"]) tagging_api.tag_object(str(cls.library_block.usage_key), cls.difficulty_tags, tags=["Normal"]) - tagging_api.tag_object(cls.collection_usage_key, cls.difficulty_tags, tags=["Normal"]) + tagging_api.tag_object(str(cls.collection_key), cls.difficulty_tags, tags=["Normal"]) + tagging_api.tag_object(str(cls.container_key), cls.difficulty_tags, tags=["Normal"]) @property def toy_course_access_id(self): @@ -442,13 +456,13 @@ def test_html_published_library_block(self): assert doc["publish_status"] == "modified" def test_collection_with_library(self): - doc = searchable_doc_for_collection(self.library.key, self.collection.key) - doc.update(searchable_doc_tags_for_collection(self.library.key, self.collection.key)) + doc = searchable_doc_for_collection(self.collection_key) + doc.update(searchable_doc_tags_for_collection(self.collection_key)) assert doc == { "id": "lib-collectionedx2012_falltoy_collection-d1d907a4", "block_id": self.collection.key, - "usage_key": self.collection_usage_key, + "usage_key": str(self.collection_key), "type": "collection", "org": "edX", "display_name": "Toy Collection", @@ -471,13 +485,13 @@ def test_collection_with_library(self): def test_collection_with_published_library(self): library_api.publish_changes(self.library.key) - doc = searchable_doc_for_collection(self.library.key, self.collection.key) - doc.update(searchable_doc_tags_for_collection(self.library.key, self.collection.key)) + doc = searchable_doc_for_collection(self.collection_key) + doc.update(searchable_doc_tags_for_collection(self.collection_key)) assert doc == { "id": "lib-collectionedx2012_falltoy_collection-d1d907a4", "block_id": self.collection.key, - "usage_key": self.collection_usage_key, + "usage_key": str(self.collection_key), "type": "collection", "org": "edX", "display_name": "Toy Collection", @@ -501,17 +515,8 @@ def test_draft_container(self): """ Test creating a search document for a draft-only container """ - created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) - with freeze_time(created_date): - container_meta = library_api.create_container( - self.library.key, - container_type=library_api.ContainerType.Unit, - slug="unit1", - title="A Unit in the Search Index", - user_id=None, - ) - - doc = searchable_doc_for_container(container_meta.container_key) + doc = searchable_doc_for_container(self.container.container_key) + doc.update(searchable_doc_tags(self.container.container_key)) assert doc == { "id": "lctedx2012_fallunitunit1-edd13a0c", @@ -529,7 +534,10 @@ def test_draft_container(self): "breadcrumbs": [{"display_name": "some content_library"}], "created": 1680674828.0, "modified": 1680674828.0, - # "tags" should be here but we haven't implemented them yet + "tags": { + "taxonomy": ["Difficulty"], + "level0": ["Difficulty > Normal"] + }, # "published" is not set since we haven't published it yet } @@ -537,23 +545,17 @@ def test_published_container(self): """ Test creating a search document for a published container """ - created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) - with freeze_time(created_date): - container_meta = library_api.create_container( - self.library.key, - container_type=library_api.ContainerType.Unit, - slug="unit1", - title="A Unit in the Search Index", - user_id=None, - ) + with freeze_time(self.container.created): + # Create a container with a block in it library_api.update_container_children( - container_meta.container_key, + self.container.container_key, [self.library_block.usage_key], user_id=None, ) library_api.publish_changes(self.library.key) - doc = searchable_doc_for_container(container_meta.container_key) + doc = searchable_doc_for_container(self.container.container_key) + doc.update(searchable_doc_tags(self.container.container_key)) assert doc == { "id": "lctedx2012_fallunitunit1-edd13a0c", @@ -571,29 +573,22 @@ def test_published_container(self): "breadcrumbs": [{"display_name": "some content_library"}], "created": 1680674828.0, "modified": 1680674828.0, + "tags": { + "taxonomy": ["Difficulty"], + "level0": ["Difficulty > Normal"] + }, "published": {"num_children": 1}, - # "tags" should be here but we haven't implemented them yet - # "published" is not set since we haven't published it yet } def test_published_container_with_changes(self): """ Test creating a search document for a published container """ - created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) - with freeze_time(created_date): - container_meta = library_api.create_container( - self.library.key, - container_type=library_api.ContainerType.Unit, - slug="unit1", - title="A Unit in the Search Index", - user_id=None, - ) - library_api.update_container_children( - container_meta.container_key, - [self.library_block.usage_key], - user_id=None, - ) + library_api.update_container_children( + self.container.container_key, + [self.library_block.usage_key], + user_id=None, + ) library_api.publish_changes(self.library.key) block_2 = library_api.create_library_block( self.library.key, @@ -602,15 +597,16 @@ def test_published_container_with_changes(self): ) # Add another component after publish - with freeze_time(created_date): + with freeze_time(self.container.created): library_api.update_container_children( - container_meta.container_key, + self.container.container_key, [block_2.usage_key], user_id=None, entities_action=authoring_api.ChildrenEntitiesAction.APPEND, ) - doc = searchable_doc_for_container(container_meta.container_key) + doc = searchable_doc_for_container(self.container.container_key) + doc.update(searchable_doc_tags(self.container.container_key)) assert doc == { "id": "lctedx2012_fallunitunit1-edd13a0c", @@ -628,9 +624,11 @@ def test_published_container_with_changes(self): "breadcrumbs": [{"display_name": "some content_library"}], "created": 1680674828.0, "modified": 1680674828.0, + "tags": { + "taxonomy": ["Difficulty"], + "level0": ["Difficulty > Normal"] + }, "published": {"num_children": 1}, - # "tags" should be here but we haven't implemented them yet - # "published" is not set since we haven't published it yet } def test_mathjax_plain_text_conversion_for_search(self): diff --git a/openedx/core/djangoapps/content_libraries/api/blocks.py b/openedx/core/djangoapps/content_libraries/api/blocks.py index 24510762267c..04649a83503f 100644 --- a/openedx/core/djangoapps/content_libraries/api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/api/blocks.py @@ -59,6 +59,7 @@ LibraryBlockAlreadyExists, ) from .libraries import ( + library_collection_locator, library_component_usage_key, require_permission_for_library_key, PublishableItem, @@ -284,8 +285,7 @@ def set_library_block_olx(usage_key: LibraryUsageLocatorV2, new_olx_str: str) -> for container in affected_containers: LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( - library_key=usage_key.lib_key, - container_key=str(container.container_key), + container_key=container.container_key, background=True, ) ) @@ -527,8 +527,10 @@ def delete_library_block(usage_key: LibraryUsageLocatorV2, remove_from_parent=Tr for collection in affected_collections: LIBRARY_COLLECTION_UPDATED.send_event( library_collection=LibraryCollectionData( - library_key=library_key, - collection_key=collection.key, + collection_key=library_collection_locator( + library_key=library_key, + collection_key=collection.key, + ), background=True, ) ) @@ -540,8 +542,7 @@ def delete_library_block(usage_key: LibraryUsageLocatorV2, remove_from_parent=Tr for container in affected_containers: LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( - library_key=library_key, - container_key=str(container.container_key), + container_key=container.container_key, background=True, ) ) @@ -580,8 +581,10 @@ def restore_library_block(usage_key: LibraryUsageLocatorV2) -> None: for collection in affected_collections: LIBRARY_COLLECTION_UPDATED.send_event( library_collection=LibraryCollectionData( - library_key=library_key, - collection_key=collection.key, + collection_key=library_collection_locator( + library_key=library_key, + collection_key=collection.key, + ), background=True, ) ) @@ -594,8 +597,7 @@ def restore_library_block(usage_key: LibraryUsageLocatorV2) -> None: for container in affected_containers: LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( - library_key=library_key, - container_key=str(container.container_key), + container_key=container.container_key, background=True, ) ) diff --git a/openedx/core/djangoapps/content_libraries/api/collections.py b/openedx/core/djangoapps/content_libraries/api/collections.py index 435804c4140f..f94c9e5401a1 100644 --- a/openedx/core/djangoapps/content_libraries/api/collections.py +++ b/openedx/core/djangoapps/content_libraries/api/collections.py @@ -32,8 +32,8 @@ "update_library_collection", "update_library_collection_components", "set_library_component_collections", - "get_library_collection_usage_key", - "get_library_collection_from_usage_key", + "library_collection_locator", + "get_library_collection_from_locator", ] @@ -218,8 +218,10 @@ def set_library_component_collections( for collection in affected_collections: LIBRARY_COLLECTION_UPDATED.send_event( library_collection=LibraryCollectionData( - library_key=library_key, - collection_key=collection.key, + collection_key=library_collection_locator( + library_key=library_key, + collection_key=collection.key, + ), background=True, ) ) @@ -227,7 +229,7 @@ def set_library_component_collections( return component -def get_library_collection_usage_key( +def library_collection_locator( library_key: LibraryLocatorV2, collection_key: str, ) -> LibraryCollectionLocator: @@ -238,15 +240,14 @@ def get_library_collection_usage_key( return LibraryCollectionLocator(library_key, collection_key) -def get_library_collection_from_usage_key( - collection_usage_key: LibraryCollectionLocator, +def get_library_collection_from_locator( + collection_locator: LibraryCollectionLocator, ) -> Collection: """ Return a Collection using the LibraryCollectionLocator """ - - library_key = collection_usage_key.library_key - collection_key = collection_usage_key.collection_id + library_key = collection_locator.library_key + collection_key = collection_locator.collection_id content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] assert content_library.learning_package_id is not None # shouldn't happen but it's technically possible. try: diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index bf519901ae57..31eae20e7fa6 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -44,6 +44,7 @@ "library_container_locator", "update_container", "delete_container", + "restore_container", "update_container_children", "get_containers_contains_component", ] @@ -121,7 +122,7 @@ def library_container_locator( ) -def _get_container(container_key: LibraryContainerLocator) -> Container: +def _get_container(container_key: LibraryContainerLocator, isDeleted=False) -> Container: """ Internal method to fetch the Container object from its LibraryContainerLocator @@ -135,7 +136,7 @@ def _get_container(container_key: LibraryContainerLocator) -> Container: learning_package.id, key=container_key.container_id, ) - if container and container.versioning.draft: + if container and (isDeleted or container.versioning.draft): return container raise ContentLibraryContainerNotFound @@ -189,8 +190,7 @@ def create_container( LIBRARY_CONTAINER_CREATED.send_event( library_container=LibraryContainerData( - library_key=library_key, - container_key=str(container_key), + container_key=container_key, ) ) @@ -218,8 +218,7 @@ def update_container( LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( - library_key=library_key, - container_key=str(container_key), + container_key=container_key, ) ) @@ -234,23 +233,34 @@ def delete_container( No-op if container doesn't exist or has already been soft-deleted. """ - try: - container = _get_container(container_key) - except ContentLibraryContainerNotFound: - return + container = _get_container(container_key) authoring_api.soft_delete_draft(container.pk) LIBRARY_CONTAINER_DELETED.send_event( library_container=LibraryContainerData( - library_key=container_key.library_key, - container_key=str(container_key), + container_key=container_key, ) ) # TODO: trigger a LIBRARY_COLLECTION_UPDATED for each collection the container was in +def restore_container(container_key: LibraryContainerLocator) -> None: + """ + Restore the specified library container. + """ + container = _get_container(container_key, isDeleted=True) + + authoring_api.set_draft_version(container.pk, container.versioning.latest.pk) + + LIBRARY_CONTAINER_CREATED.send_event( + library_container=LibraryContainerData( + container_key=str(container_key), + ) + ) + + def get_container_children( container_key: LibraryContainerLocator, published=False, @@ -311,8 +321,7 @@ def update_container_children( LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( - library_key=library_key, - container_key=str(container_key), + container_key=container_key, ) ) diff --git a/openedx/core/djangoapps/content_libraries/api/libraries.py b/openedx/core/djangoapps/content_libraries/api/libraries.py index bf9ca6f60d29..e6ba323b9aae 100644 --- a/openedx/core/djangoapps/content_libraries/api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/api/libraries.py @@ -75,6 +75,7 @@ from .. import permissions from ..constants import ALL_RIGHTS_RESERVED from ..models import ContentLibrary, ContentLibraryPermission +from .collections import library_collection_locator from .exceptions import ( LibraryAlreadyExists, LibraryPermissionIntegrityError, @@ -210,12 +211,17 @@ class LibraryXBlockMetadata(PublishableItem): Class that represents the metadata about an XBlock in a content library. """ usage_key: LibraryUsageLocatorV2 + # TODO: move tags_count to LibraryItem as all objects under a library can be tagged. + tags_count: int = 0 @classmethod def from_component(cls, library_key, component, associated_collections=None): """ Construct a LibraryXBlockMetadata from a Component object. """ + # Import content_tagging.api here to avoid circular imports + from openedx.core.djangoapps.content_tagging.api import get_object_tag_counts + last_publish_log = component.versioning.last_publish_log published_by = None @@ -226,12 +232,11 @@ def from_component(cls, library_key, component, associated_collections=None): published = component.versioning.published last_draft_created = draft.created if draft else None last_draft_created_by = draft.publishable_entity_version.created_by if draft else None + usage_key = library_component_usage_key(library_key, component) + tags = get_object_tag_counts(str(usage_key), count_implicit=True) return cls( - usage_key=library_component_usage_key( - library_key, - component, - ), + usage_key=usage_key, display_name=draft.title, created=component.created, modified=draft.created, @@ -244,6 +249,7 @@ def from_component(cls, library_key, component, associated_collections=None): has_unpublished_changes=component.versioning.has_unpublished_changes, collections=associated_collections or [], can_stand_alone=component.publishable_entity.can_stand_alone, + tags_count=tags.get(str(usage_key), 0), ) @@ -739,8 +745,10 @@ def revert_changes(library_key: LibraryLocatorV2) -> None: for collection in authoring_api.get_collections(learning_package.id): LIBRARY_COLLECTION_UPDATED.send_event( library_collection=LibraryCollectionData( - library_key=library_key, - collection_key=collection.key, + collection_key=library_collection_locator( + library_key=library_key, + collection_key=collection.key, + ), background=True, ) ) diff --git a/openedx/core/djangoapps/content_libraries/library_context.py b/openedx/core/djangoapps/content_libraries/library_context.py index d5f121d8396e..34a5a90269fe 100644 --- a/openedx/core/djangoapps/content_libraries/library_context.py +++ b/openedx/core/djangoapps/content_libraries/library_context.py @@ -125,8 +125,7 @@ def send_container_updated_events(self, usage_key: UsageKeyV2): for container in affected_containers: LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( - library_key=usage_key.lib_key, - container_key=str(container.container_key), + container_key=container.container_key, background=True, ) ) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py index 95e468b4a43a..3c65fc006bf6 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/containers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -273,3 +273,23 @@ def patch(self, request, container_key: LibraryContainerLocator): container_key, action=authoring_api.ChildrenEntitiesAction.REPLACE, ) + + +@method_decorator(non_atomic_requests, name="dispatch") +@view_auth_classes() +class LibraryContainerRestore(GenericAPIView): + """ + View to restore soft-deleted library containers. + """ + @convert_exceptions + def post(self, request, container_key: LibraryContainerLocator) -> Response: + """ + Restores a soft-deleted library container + """ + api.require_permission_for_library_key( + container_key.library_key, + request.user, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + ) + api.restore_container(container_key) + return Response(None, status=HTTP_204_NO_CONTENT) diff --git a/openedx/core/djangoapps/content_libraries/signal_handlers.py b/openedx/core/djangoapps/content_libraries/signal_handlers.py index 68cc7512e540..08b30b10bba6 100644 --- a/openedx/core/djangoapps/content_libraries/signal_handlers.py +++ b/openedx/core/djangoapps/content_libraries/signal_handlers.py @@ -25,7 +25,7 @@ from lms.djangoapps.grades.api import signals as grades_signals -from .api import library_component_usage_key +from .api import library_collection_locator, library_component_usage_key from .models import ContentLibrary, LtiGradedResource log = logging.getLogger(__name__) @@ -86,15 +86,19 @@ def library_collection_saved(sender, instance, created, **kwargs): if created: LIBRARY_COLLECTION_CREATED.send_event( library_collection=LibraryCollectionData( - library_key=library.library_key, - collection_key=instance.key, + collection_key=library_collection_locator( + library_key=library.library_key, + collection_key=instance.key, + ), ) ) else: LIBRARY_COLLECTION_UPDATED.send_event( library_collection=LibraryCollectionData( - library_key=library.library_key, - collection_key=instance.key, + collection_key=library_collection_locator( + library_key=library.library_key, + collection_key=instance.key, + ), ) ) @@ -112,8 +116,10 @@ def library_collection_deleted(sender, instance, **kwargs): LIBRARY_COLLECTION_DELETED.send_event( library_collection=LibraryCollectionData( - library_key=library.library_key, - collection_key=instance.key, + collection_key=library_collection_locator( + library_key=library.library_key, + collection_key=instance.key, + ), ) ) diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index 6adb8184ea00..3c07aabe06c4 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -34,6 +34,7 @@ URL_LIB_BLOCK_ASSET_FILE = URL_LIB_BLOCK + 'assets/{file_name}' # Get, delete, or upload a specific static asset file URL_LIB_CONTAINER = URL_PREFIX + 'containers/{container_key}/' # Get a container in this library URL_LIB_CONTAINER_COMPONENTS = URL_LIB_CONTAINER + 'children/' # Get, add or delete a component in this container +URL_LIB_CONTAINER_RESTORE = URL_LIB_CONTAINER + 'restore/' # Restore a deleted container URL_LIB_LTI_PREFIX = URL_PREFIX + 'lti/1.3/' URL_LIB_LTI_JWKS = URL_LIB_LTI_PREFIX + 'pub/jwks/' @@ -386,6 +387,10 @@ def _delete_container(self, container_key: str, expect_response=204): """ Delete a container (unit etc.) """ return self._api('delete', URL_LIB_CONTAINER.format(container_key=container_key), None, expect_response) + def _restore_container(self, container_key: str, expect_response=204): + """ Restore a deleted a container (unit etc.) """ + return self._api('post', URL_LIB_CONTAINER_RESTORE.format(container_key=container_key), None, expect_response) + def _get_container_components(self, container_key: str, expect_response=200): """ Get container components""" return self._api( diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index 71360cde3e31..b0934768b794 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -352,8 +352,10 @@ def test_create_library_collection(self): "signal": LIBRARY_COLLECTION_CREATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib2.library_key, - collection_key="COL4", + collection_key=api.library_collection_locator( + self.lib2.library_key, + collection_key="COL4", + ), ), }, event_receiver.call_args_list[0].kwargs, @@ -388,8 +390,10 @@ def test_update_library_collection(self): "signal": LIBRARY_COLLECTION_UPDATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib1.library_key, - collection_key="COL1", + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key="COL1", + ), ), }, event_receiver.call_args_list[0].kwargs, @@ -418,8 +422,10 @@ def test_delete_library_collection(self): "signal": LIBRARY_COLLECTION_DELETED, "sender": None, "library_collection": LibraryCollectionData( - self.lib1.library_key, - collection_key="COL1", + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key="COL1", + ), ), }, event_receiver.call_args_list[0].kwargs, @@ -493,8 +499,10 @@ def test_update_library_collection_components_event(self): "signal": LIBRARY_COLLECTION_UPDATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib1.library_key, - collection_key="COL1", + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key="COL1", + ), ), }, event_receiver.call_args_list[2].kwargs, @@ -544,8 +552,10 @@ def test_set_library_component_collections(self): "signal": LIBRARY_COLLECTION_UPDATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib2.library_key, - collection_key=self.col2.key, + collection_key=api.library_collection_locator( + self.lib2.library_key, + collection_key=self.col2.key, + ), background=True, ), }, @@ -556,8 +566,10 @@ def test_set_library_component_collections(self): "signal": LIBRARY_COLLECTION_UPDATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib2.library_key, - collection_key=self.col3.key, + collection_key=api.library_collection_locator( + self.lib2.library_key, + collection_key=self.col3.key, + ), background=True, ), }, @@ -585,8 +597,10 @@ def test_delete_library_block(self): "signal": LIBRARY_COLLECTION_UPDATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib1.library_key, - collection_key=self.col1.key, + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key=self.col1.key, + ), background=True, ), }, @@ -614,8 +628,10 @@ def test_restore_library_block(self): "signal": LIBRARY_COLLECTION_UPDATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib1.library_key, - collection_key=self.col1.key, + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key=self.col1.key, + ), background=True, ), }, @@ -656,8 +672,10 @@ def test_add_component_and_revert(self): "signal": LIBRARY_COLLECTION_UPDATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib1.library_key, - collection_key=self.col1.key, + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key=self.col1.key, + ), background=True, ), }, @@ -715,8 +733,10 @@ def test_delete_component_and_revert(self): "signal": LIBRARY_COLLECTION_UPDATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib1.library_key, - collection_key=self.col1.key, + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key=self.col1.key, + ), background=True, ), }, @@ -824,8 +844,7 @@ def _validate_calls_of_html_block(self, event_mock): "signal": LIBRARY_CONTAINER_UPDATED, "sender": None, "library_container": LibraryContainerData( - library_key=self.lib1.library_key, - container_key=str(self.unit1.container_key), + container_key=self.unit1.container_key, background=True, ) }, @@ -836,8 +855,7 @@ def _validate_calls_of_html_block(self, event_mock): "signal": LIBRARY_CONTAINER_UPDATED, "sender": None, "library_container": LibraryContainerData( - library_key=self.lib1.library_key, - container_key=str(self.unit2.container_key), + container_key=self.unit2.container_key, background=True, ) }, diff --git a/openedx/core/djangoapps/content_libraries/tests/test_containers.py b/openedx/core/djangoapps/content_libraries/tests/test_containers.py index 52546396f2bb..85e04260cf52 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_containers.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_containers.py @@ -7,7 +7,7 @@ import ddt from freezegun import freeze_time -from opaque_keys.edx.locator import LibraryLocatorV2 +from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2 from openedx_events.content_authoring.data import LibraryContainerData from openedx_events.content_authoring.signals import ( LIBRARY_CONTAINER_CREATED, @@ -83,13 +83,15 @@ def test_unit_crud(self): self.assertDictContainsEntries(container_data, expected_data) assert create_receiver.call_count == 1 + container_key = LibraryContainerLocator.from_string( + "lct:CL-TEST:containers:unit:u1", + ) self.assertDictContainsSubset( { "signal": LIBRARY_CONTAINER_CREATED, "sender": None, "library_container": LibraryContainerData( - lib_key, - container_key="lct:CL-TEST:containers:unit:u1", + container_key, ), }, create_receiver.call_args_list[0].kwargs, @@ -114,8 +116,7 @@ def test_unit_crud(self): "signal": LIBRARY_CONTAINER_UPDATED, "sender": None, "library_container": LibraryContainerData( - lib_key, - container_key="lct:CL-TEST:containers:unit:u1", + container_key, ), }, update_receiver.call_args_list[0].kwargs, @@ -135,8 +136,7 @@ def test_unit_crud(self): "signal": LIBRARY_CONTAINER_DELETED, "sender": None, "library_container": LibraryContainerData( - lib_key, - container_key="lct:CL-TEST:containers:unit:u1", + container_key, ), }, delete_receiver.call_args_list[0].kwargs, @@ -214,8 +214,9 @@ def test_unit_add_children(self): "signal": LIBRARY_CONTAINER_UPDATED, "sender": None, "library_container": LibraryContainerData( - lib_key, - container_key=container_data["container_key"], + container_key=LibraryContainerLocator.from_string( + container_data["container_key"], + ), ), }, update_receiver.call_args_list[0].kwargs, @@ -263,8 +264,9 @@ def test_unit_remove_children(self): "signal": LIBRARY_CONTAINER_UPDATED, "sender": None, "library_container": LibraryContainerData( - lib_key, - container_key=container_data["container_key"], + container_key=LibraryContainerLocator.from_string( + container_data["container_key"], + ), ), }, update_receiver.call_args_list[0].kwargs, @@ -324,9 +326,59 @@ def test_unit_replace_children(self): "signal": LIBRARY_CONTAINER_UPDATED, "sender": None, "library_container": LibraryContainerData( - lib_key, - container_key=container_data["container_key"], + container_key=LibraryContainerLocator.from_string( + container_data["container_key"], + ), ), }, update_receiver.call_args_list[0].kwargs, ) + + def test_restore_unit(self): + """ + Test restore a deleted unit. + """ + lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + lib_key = LibraryLocatorV2.from_string(lib["id"]) + + # Create a unit: + create_date = datetime(2024, 9, 8, 7, 6, 5, tzinfo=timezone.utc) + with freeze_time(create_date): + container_data = self._create_container(lib["id"], "unit", slug="u1", display_name="Test Unit") + + # Delete the unit + self._delete_container(container_data["container_key"]) + + create_receiver = mock.Mock() + LIBRARY_CONTAINER_CREATED.connect(create_receiver) + + # Restore container + self._restore_container(container_data["container_key"]) + new_container_data = self._get_container(container_data["container_key"]) + expected_data = { + "container_key": "lct:CL-TEST:containers:unit:u1", + "container_type": "unit", + "display_name": "Test Unit", + "last_published": None, + "published_by": "", + "last_draft_created": "2024-09-08T07:06:05Z", + "last_draft_created_by": 'Bob', + 'has_unpublished_changes': True, + 'created': '2024-09-08T07:06:05Z', + 'modified': '2024-09-08T07:06:05Z', + 'collections': [], + } + + self.assertDictContainsEntries(new_container_data, expected_data) + + assert create_receiver.call_count == 1 + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_CREATED, + "sender": None, + "library_container": LibraryContainerData( + container_key="lct:CL-TEST:containers:unit:u1", + ), + }, + create_receiver.call_args_list[0].kwargs, + ) diff --git a/openedx/core/djangoapps/content_libraries/urls.py b/openedx/core/djangoapps/content_libraries/urls.py index 99e98a2cc424..c46cc8641cd8 100644 --- a/openedx/core/djangoapps/content_libraries/urls.py +++ b/openedx/core/djangoapps/content_libraries/urls.py @@ -82,6 +82,8 @@ path('', containers.LibraryContainerView.as_view()), # update components under container path('children/', containers.LibraryContainerChildrenView.as_view()), + # Restore a soft-deleted container + path('restore/', containers.LibraryContainerRestore.as_view()), # Update collections for a given container # path('collections/', views.LibraryContainerCollectionsView.as_view(), name='update-collections-ct'), # path('publish/', views.LibraryContainerPublishView.as_view()), diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 5a99868ece66..6ba1049082d1 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -14,7 +14,7 @@ from django.contrib.auth import get_user_model from django.core.files.uploadedfile import SimpleUploadedFile from edx_django_utils.cache import RequestCache -from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator, LibraryCollectionLocator +from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator, LibraryCollectionLocator, LibraryContainerLocator from openedx_tagging.core.tagging.models import Tag, Taxonomy from openedx_tagging.core.tagging.models.system_defined import SystemDefinedTaxonomy from openedx_tagging.core.tagging.rest_api.v1.serializers import TaxonomySerializer @@ -114,6 +114,9 @@ def _setUp_library(self): def _setUp_collection(self): self.collection_key = str(LibraryCollectionLocator(self.content_libraryA.key, 'test-collection')) + def _setUp_container(self): + self.container_key = str(LibraryContainerLocator(self.content_libraryA.key, 'unit', 'unit1')) + def _setUp_users(self): """ Create users for testing @@ -288,6 +291,7 @@ def setUp(self): self._setUp_users() self._setUp_taxonomies() self._setUp_collection() + self._setUp_container() # Clear all request caches in between test runs to keep query counts consistent. RequestCache.clear_all_namespaces() @@ -1701,6 +1705,50 @@ def test_tag_collection(self, user_attr, taxonomy_attr, tag_values, expected_sta assert status.is_success(new_response.status_code) assert new_response.data == response.data + @ddt.data( + # staffA and staff are staff in collection and can tag using enabled taxonomies + ("user", "tA1", ["Tag 1"], status.HTTP_403_FORBIDDEN), + ("staffA", "tA1", ["Tag 1"], status.HTTP_200_OK), + ("staff", "tA1", ["Tag 1"], status.HTTP_200_OK), + ("user", "tA1", [], status.HTTP_403_FORBIDDEN), + ("staffA", "tA1", [], status.HTTP_200_OK), + ("staff", "tA1", [], status.HTTP_200_OK), + ("user", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_403_FORBIDDEN), + ("staffA", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_200_OK), + ("staff", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_200_OK), + ("user", "open_taxonomy", ["tag1"], status.HTTP_403_FORBIDDEN), + ("staffA", "open_taxonomy", ["tag1"], status.HTTP_200_OK), + ("staff", "open_taxonomy", ["tag1"], status.HTTP_200_OK), + ) + @ddt.unpack + def test_tag_container(self, user_attr, taxonomy_attr, tag_values, expected_status): + """ + Tests that only staff and org level users can tag containers + """ + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + taxonomy = getattr(self, taxonomy_attr) + + response = self._call_put_request(self.container_key, taxonomy.pk, tag_values) + + assert response.status_code == expected_status + if status.is_success(expected_status): + tags_by_taxonomy = response.data[str(self.container_key)]["taxonomies"] + if tag_values: + response_taxonomy = tags_by_taxonomy[0] + assert response_taxonomy["name"] == taxonomy.name + response_tags = response_taxonomy["tags"] + assert [t["value"] for t in response_tags] == tag_values + else: + assert tags_by_taxonomy == [] # No tags are set from any taxonomy + + # Check that re-fetching the tags returns what we set + url = OBJECT_TAG_UPDATE_URL.format(object_id=self.container_key) + new_response = self.client.get(url, format="json") + assert status.is_success(new_response.status_code) + assert new_response.data == response.data + @ddt.data( "staffA", "staff", diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index d85c87d62b17..2661c02fd15c 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_api.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_api.py @@ -6,7 +6,7 @@ from django.test.testcases import TestCase from fs.osfs import OSFS from opaque_keys.edx.keys import CourseKey, UsageKey -from opaque_keys.edx.locator import LibraryLocatorV2, LibraryCollectionLocator +from opaque_keys.edx.locator import LibraryLocatorV2, LibraryCollectionLocator, LibraryContainerLocator from openedx_tagging.core.tagging.models import ObjectTag from organizations.models import Organization from .test_objecttag_export_helpers import TestGetAllObjectTagsMixin, TaggedCourseMixin @@ -397,6 +397,23 @@ def test_tag_collection(self): self.taxonomy_3.id: self.taxonomy_3, } + def test_tag_container(self): + unit_key = LibraryContainerLocator.from_string('lct:orgA:libX:unit:unit1') + + api.tag_object( + object_id=str(unit_key), + taxonomy=self.taxonomy_3, + tags=["Tag 3.1"], + ) + + with self.assertNumQueries(1): + object_tags, taxonomies = api.get_all_object_tags(unit_key) + + assert object_tags == {'lct:orgA:libX:unit:unit1': {3: ['Tag 3.1']}} + assert taxonomies == { + self.taxonomy_3.id: self.taxonomy_3, + } + class TestExportImportTags(TaggedCourseMixin): """ diff --git a/openedx/core/djangoapps/courseware_api/serializers.py b/openedx/core/djangoapps/courseware_api/serializers.py index 47b977532ff6..191d832c5f51 100644 --- a/openedx/core/djangoapps/courseware_api/serializers.py +++ b/openedx/core/djangoapps/courseware_api/serializers.py @@ -80,7 +80,6 @@ class CourseInfoSerializer(serializers.Serializer): # pylint: disable=abstract- """ access_expiration = serializers.DictField() - can_show_upgrade_sock = serializers.BooleanField() content_type_gating_enabled = serializers.BooleanField() course_goals = CourseGoalsSerializer() effort = serializers.CharField() diff --git a/openedx/core/djangoapps/courseware_api/views.py b/openedx/core/djangoapps/courseware_api/views.py index 07e219f83d81..576f0472b770 100644 --- a/openedx/core/djangoapps/courseware_api/views.py +++ b/openedx/core/djangoapps/courseware_api/views.py @@ -55,7 +55,6 @@ from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin from openedx.core.lib.courses import get_course_by_id -from openedx.features.course_experience import DISPLAY_COURSE_SOCK_FLAG from openedx.features.course_experience import ENABLE_COURSE_GOALS from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.course_duration_limits.access import get_access_expiration_data @@ -127,10 +126,6 @@ def content_type_gating_enabled(self): course_key=self.course_key, ) - @property - def can_show_upgrade_sock(self): - return DISPLAY_COURSE_SOCK_FLAG.is_enabled(self.course_key) - @property def license(self): return self.course.license diff --git a/openedx/core/djangoapps/notifications/base_notification.py b/openedx/core/djangoapps/notifications/base_notification.py index ff2cc38c84a3..899046e440ed 100644 --- a/openedx/core/djangoapps/notifications/base_notification.py +++ b/openedx/core/djangoapps/notifications/base_notification.py @@ -191,18 +191,20 @@ 'email_template': '', 'filters': [FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE] }, - 'ora_staff_notification': { + 'ora_staff_notifications': { 'notification_app': 'grading', - 'name': 'ora_staff_notification', + 'name': 'ora_staff_notifications', 'is_core': False, 'info': '', - 'web': False, + 'web': True, 'email': False, 'push': False, 'email_cadence': EmailCadence.DAILY, 'non_editable': [], - 'content_template': _('<{p}>You have a new open response submission awaiting for review for ' + 'content_template': _('<{p}>You have a new open response submission awaiting review for ' '<{strong}>{ora_name}'), + 'grouped_content_template': _('<{p}>You have multiple submissions awaiting review for ' + '<{strong}>{ora_name}'), 'content_context': { 'ora_name': 'Name of ORA in course', }, diff --git a/openedx/core/djangoapps/notifications/exceptions.py b/openedx/core/djangoapps/notifications/exceptions.py new file mode 100644 index 000000000000..2eb8bb794253 --- /dev/null +++ b/openedx/core/djangoapps/notifications/exceptions.py @@ -0,0 +1,6 @@ +""" Notification-related exceptions. """ + + +class InvalidNotificationTypeError(Exception): + """ Exception raised when an invalid notification type is passed. """ + pass # lint-amnesty, pylint: disable=unnecessary-pass diff --git a/openedx/core/djangoapps/notifications/grouping_notifications.py b/openedx/core/djangoapps/notifications/grouping_notifications.py index 0e84ea3f109a..eeb4400462b4 100644 --- a/openedx/core/djangoapps/notifications/grouping_notifications.py +++ b/openedx/core/djangoapps/notifications/grouping_notifications.py @@ -2,14 +2,16 @@ Notification grouping utilities for notifications """ import datetime +from abc import ABC, abstractmethod from typing import Dict, Type, Union from pytz import utc -from abc import ABC, abstractmethod - +from openedx.core.djangoapps.notifications.base_notification import COURSE_NOTIFICATION_TYPES from openedx.core.djangoapps.notifications.models import Notification +from .exceptions import InvalidNotificationTypeError + class BaseNotificationGrouper(ABC): """ @@ -42,6 +44,10 @@ def decorator(grouper_class: Type[BaseNotificationGrouper]) -> Type[BaseNotifica """ Registers the grouper class for the given notification type. """ + if notification_type not in COURSE_NOTIFICATION_TYPES: + raise InvalidNotificationTypeError( + f"'{notification_type}' is not a valid notification type." + ) cls._groupers[notification_type] = grouper_class return grouper_class @@ -106,6 +112,21 @@ def group(self, new_notification, old_notification): } +@NotificationRegistry.register('ora_staff_notifications') +class OraStaffGrouper(BaseNotificationGrouper): + """ + Grouper for new ora staff notifications. + """ + + def group(self, new_notification, old_notification): + """ + Groups new ora staff notifications based on the xblock ID. + """ + content_context = old_notification.content_context + content_context.setdefault("grouped", True) + return content_context + + def group_user_notifications(new_notification: Notification, old_notification: Notification): """ Groups user notification based on notification type and group_id diff --git a/openedx/core/djangoapps/notifications/models.py b/openedx/core/djangoapps/notifications/models.py index ed9c58e50698..e3bad0016ee8 100644 --- a/openedx/core/djangoapps/notifications/models.py +++ b/openedx/core/djangoapps/notifications/models.py @@ -25,7 +25,7 @@ ADDITIONAL_NOTIFICATION_CHANNEL_SETTINGS = ['email_cadence'] # Update this version when there is a change to any course specific notification type or app. -COURSE_NOTIFICATION_CONFIG_VERSION = 12 +COURSE_NOTIFICATION_CONFIG_VERSION = 13 def get_course_notification_preference_config(): diff --git a/openedx/core/djangoapps/notifications/tests/test_notification_grouping.py b/openedx/core/djangoapps/notifications/tests/test_notification_grouping.py index a3aace632c8b..23419e46cb64 100644 --- a/openedx/core/djangoapps/notifications/tests/test_notification_grouping.py +++ b/openedx/core/djangoapps/notifications/tests/test_notification_grouping.py @@ -26,6 +26,10 @@ class TestNotificationRegistry(unittest.TestCase): Tests for the NotificationRegistry class """ + @patch.dict( + 'openedx.core.djangoapps.notifications.base_notification.COURSE_NOTIFICATION_TYPES', + {'test_notification': 'Test Notification'} + ) def test_register_and_get_grouper(self): """ Test that the register and get_grouper methods work as expected diff --git a/openedx/core/djangoapps/notifications/tests/test_views.py b/openedx/core/djangoapps/notifications/tests/test_views.py index 952cda06d528..e624995b050a 100644 --- a/openedx/core/djangoapps/notifications/tests/test_views.py +++ b/openedx/core/djangoapps/notifications/tests/test_views.py @@ -369,8 +369,8 @@ def _expected_api_response(self, course=None): 'enabled': True, 'core_notification_types': [], 'notification_types': { - 'ora_staff_notification': { - 'web': False, + 'ora_staff_notifications': { + 'web': True, 'email': False, 'push': False, 'email_cadence': 'Daily', @@ -1088,7 +1088,7 @@ def setUp(self): "email": True, "email_cadence": "Daily" }, - "ora_staff_notification": { + "ora_staff_notifications": { "web": False, "push": False, "email": False, diff --git a/openedx/core/djangoapps/schedules/tasks.py b/openedx/core/djangoapps/schedules/tasks.py index 55288d63f1a8..6a999d3dd8ed 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -276,6 +276,9 @@ def _schedule_send(msg_str, site_id, delivery_config_var, log_prefix): # lint-a msg = Message.from_string(msg_str) user = User.objects.get(id=msg.recipient.lms_user_id) + if not user.has_usable_password(): + LOG.info(f'{delivery_config_var} Scheduled email User is disabled {user.username}') + return with emulate_http_request(site=site, user=user): _annonate_send_task_for_monitoring(msg) LOG.debug('%s: Sending message = %s', log_prefix, msg_str) diff --git a/openedx/core/djangoapps/schedules/tests/test_tasks.py b/openedx/core/djangoapps/schedules/tests/test_tasks.py index 412a62521733..99f790e35b01 100644 --- a/openedx/core/djangoapps/schedules/tests/test_tasks.py +++ b/openedx/core/djangoapps/schedules/tests/test_tasks.py @@ -9,9 +9,13 @@ import ddt from django.conf import settings +from django.test import TestCase +from edx_ace.recipient import Recipient +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.schedules.message_types import InstructorLedCourseUpdate from openedx.core.djangoapps.schedules.resolvers import DEFAULT_NUM_BINS -from openedx.core.djangoapps.schedules.tasks import BinnedScheduleMessageBaseTask +from openedx.core.djangoapps.schedules.tasks import BinnedScheduleMessageBaseTask, _schedule_send from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms @@ -73,3 +77,44 @@ def test_is_enqueue_enabled(self, enabled): self.schedule_config.enqueue_recurring_nudge = enabled self.schedule_config.save() assert self.basetask.is_enqueue_enabled(self.site) == enabled + + +@ddt.ddt +@skip_unless_lms +class TestScheduleSendForDisabledUser(TestCase): + """ + Tests email send for disabled users + """ + + def setUp(self): + super().setUp() + self.user = UserFactory() + self.site = SiteFactory.create() + ScheduleConfigFactory.create( + site=self.site, + enqueue_recurring_nudge=True, deliver_recurring_nudge=True, + enqueue_upgrade_reminder=True, deliver_upgrade_reminder=True, + enqueue_course_update=True, deliver_course_update=True, + ) + + @ddt.data(True, False) + @patch('openedx.core.djangoapps.schedules.tasks.ace.send') + def test_email_not_sent_to_disable_users(self, user_enabled, mock_send): + """ + Tests email not send for disabled users + """ + if user_enabled: + self.user.set_password("12345678") + else: + self.user.set_unusable_password() + self.user.save() + msg = InstructorLedCourseUpdate().personalize( + Recipient( + self.user.id, + self.user.email, + ), + "en", + {}, + ) + _schedule_send(str(msg), self.site.id, "deliver_course_update", "Course Update") + assert mock_send.called is user_enabled diff --git a/openedx/features/course_experience/__init__.py b/openedx/features/course_experience/__init__.py index 3f4c4d3fefd1..c5f11a301340 100644 --- a/openedx/features/course_experience/__init__.py +++ b/openedx/features/course_experience/__init__.py @@ -18,9 +18,6 @@ f'{WAFFLE_FLAG_NAMESPACE}.disable_course_outline_page', __name__ ) -# Waffle flag to enable the sock on the footer of the home and courseware pages. -DISPLAY_COURSE_SOCK_FLAG = CourseWaffleFlag(f'{WAFFLE_FLAG_NAMESPACE}.display_course_sock', __name__) # lint-amnesty, pylint: disable=toggle-missing-annotation - # Waffle flag to let learners access a course before its start date. COURSE_PRE_START_ACCESS_FLAG = WaffleFlag(f'{WAFFLE_FLAG_NAMESPACE}.pre_start_access', __name__) # lint-amnesty, pylint: disable=toggle-missing-annotation diff --git a/openedx/features/course_experience/static/course_experience/js/CourseSock.js b/openedx/features/course_experience/static/course_experience/js/CourseSock.js deleted file mode 100644 index 676be0a07498..000000000000 --- a/openedx/features/course_experience/static/course_experience/js/CourseSock.js +++ /dev/null @@ -1,90 +0,0 @@ -/* globals Logger */ - -export class CourseSock { // eslint-disable-line import/prefer-default-export - constructor() { - // eslint-disable-next-line no-undef - const $toggleActionButton = $('.action-toggle-verification-sock'); - // eslint-disable-next-line no-undef - const $verificationSock = $('.verification-sock .verification-main-panel'); - // eslint-disable-next-line no-undef - const $upgradeToVerifiedButton = $('.verification-sock .action-upgrade-certificate'); - // eslint-disable-next-line no-undef - const $miniCert = $('.mini-cert'); - const pageLocation = window.location.href.indexOf('courseware') > -1 - ? 'Course Content Page' : 'Home Page'; - - // Behavior to fix button to bottom of screen on scroll - const fixUpgradeButton = () => { - if (!$upgradeToVerifiedButton.is(':visible')) { return; } - - // Grab the current scroll location - // eslint-disable-next-line no-undef - const documentBottom = $(window).scrollTop() + $(window).height(); - - // Establish a sliding window in which the button is fixed - const startFixed = $verificationSock.offset().top + 320; - const endFixed = (startFixed + $verificationSock.height()) - 220; - - // Ensure update button stays in sock even when max-width is exceeded - const distRight = window.outerWidth - ($miniCert.offset().left + $miniCert.width()); - - // Update positioning when scrolling is in fixed window and screen width is sufficient - if ((documentBottom > startFixed && documentBottom < endFixed - // eslint-disable-next-line no-undef - && $(window).width() > 960)) { - $upgradeToVerifiedButton.addClass('attached'); - $upgradeToVerifiedButton.css('right', `${distRight}px`); - } else { - // If outside sliding window, reset to un-attached state - $upgradeToVerifiedButton.removeClass('attached'); - $upgradeToVerifiedButton.css('right', '20px'); - - // Add class to define absolute location - if (documentBottom < startFixed) { - $upgradeToVerifiedButton.addClass('stuck-top'); - $upgradeToVerifiedButton.removeClass('stuck-bottom'); - } else if (documentBottom > endFixed) { - $upgradeToVerifiedButton.addClass('stuck-bottom'); - $upgradeToVerifiedButton.removeClass('stuck-top'); - } - } - }; - - // Fix the sock to the screen on scroll and resize events - if ($upgradeToVerifiedButton.length) { - // eslint-disable-next-line no-undef - $(window).scroll(fixUpgradeButton).resize(fixUpgradeButton); - } - - // Open the sock when user clicks to Learn More - $toggleActionButton.on('click', () => { - const toggleSpeed = 400; - $toggleActionButton.toggleClass('active'); - $verificationSock.slideToggle(toggleSpeed, fixUpgradeButton); - - // Toggle aria-expanded attribute - const newAriaExpandedState = $toggleActionButton.attr('aria-expanded') === 'false'; - $toggleActionButton.attr('aria-expanded', newAriaExpandedState); - - // Log open and close events - const isOpening = $toggleActionButton.hasClass('active'); - const logMessage = isOpening ? 'edx.bi.course.sock.toggle_opened' - : 'edx.bi.course.sock.toggle_closed'; - window.analytics.track( - logMessage, - { - from_page: pageLocation, - }, - ); - }); - - $upgradeToVerifiedButton.on('click', () => { - Logger.log( - 'edx.course.enrollment.upgrade.clicked', - { - location: 'sock', - }, - ); - }); - } -} diff --git a/openedx/features/course_experience/templates/course_experience/course-sock-fragment.html b/openedx/features/course_experience/templates/course_experience/course-sock-fragment.html deleted file mode 100644 index ede2a7f2b297..000000000000 --- a/openedx/features/course_experience/templates/course_experience/course-sock-fragment.html +++ /dev/null @@ -1,76 +0,0 @@ -## mako - -<%page expression_filter="h"/> -<%namespace name='static' file='../static_content.html'/> - -<%! -from django.utils.translation import gettext as _ -from openedx.core.djangolib.markup import HTML, Text -from openedx.features.course_experience import DISPLAY_COURSE_SOCK_FLAG -%> - -<%block name="content"> - % if show_course_sock: - - %endif - - -<%static:webpack entry="CourseSock"> - new CourseSock({ - el:'.verification-sock' - }); - diff --git a/openedx/features/course_experience/tests/views/test_course_sock.py b/openedx/features/course_experience/tests/views/test_course_sock.py deleted file mode 100644 index 5c612e3ca85f..000000000000 --- a/openedx/features/course_experience/tests/views/test_course_sock.py +++ /dev/null @@ -1,117 +0,0 @@ -""" -Tests for course verification sock -""" - - -from unittest import mock - -import ddt -from django.urls import reverse -from edx_toggles.toggles.testutils import override_waffle_flag - -from common.djangoapps.course_modes.models import CourseMode -from lms.djangoapps.commerce.models import CommerceConfiguration -from lms.djangoapps.courseware.tests.helpers import set_preview_mode -from openedx.core.djangolib.markup import HTML -from openedx.features.course_experience import DISPLAY_COURSE_SOCK_FLAG -from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order - -from .helpers import add_course_mode - -TEST_PASSWORD = 'Password1234' -TEST_VERIFICATION_SOCK_LOCATOR = '
DISCOUNT_PRICE"), True)) - ) - def test_upgrade_message_discount(self): - response = self.get_courseware(self.verified_course) - self.assertContains(response, "DISCOUNT_PRICE") - - def assert_verified_sock_is_visible(self, course, response): # lint-amnesty, pylint: disable=unused-argument - return self.assertContains(response, TEST_VERIFICATION_SOCK_LOCATOR, html=False) - - def assert_verified_sock_is_not_visible(self, course, response): # lint-amnesty, pylint: disable=unused-argument - return self.assertNotContains(response, TEST_VERIFICATION_SOCK_LOCATOR, html=False) diff --git a/openedx/features/course_experience/tests/views/test_masquerade.py b/openedx/features/course_experience/tests/views/test_masquerade.py index 0f555e27f7e2..f0c110d8e9aa 100644 --- a/openedx/features/course_experience/tests/views/test_masquerade.py +++ b/openedx/features/course_experience/tests/views/test_masquerade.py @@ -2,11 +2,7 @@ Tests for masquerading functionality on course_experience """ -from django.urls import reverse - -from edx_toggles.toggles.testutils import override_waffle_flag -from lms.djangoapps.courseware.tests.helpers import MasqueradeMixin, set_preview_mode -from openedx.features.course_experience import DISPLAY_COURSE_SOCK_FLAG +from lms.djangoapps.courseware.tests.helpers import MasqueradeMixin from common.djangoapps.student.roles import CourseStaffRole from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order @@ -15,7 +11,6 @@ from xmodule.partitions.partitions_service import PartitionService # lint-amnesty, pylint: disable=wrong-import-order from .helpers import add_course_mode -from .test_course_sock import TEST_VERIFICATION_SOCK_LOCATOR TEST_PASSWORD = 'Password1234' @@ -59,40 +54,3 @@ def get_group_id_by_course_mode_name(self, course_id, mode_name): if group.name == mode_name: return group.id return None - - -@set_preview_mode(True) -class TestVerifiedUpgradesWithMasquerade(MasqueradeTestBase): - """ - Tests for the course verification upgrade messages while the user is being masqueraded. - """ - - @override_waffle_flag(DISPLAY_COURSE_SOCK_FLAG, active=True) - def test_masquerade_as_student(self): - # Elevate the staff user to be student - self.update_masquerade(course=self.verified_course, user_partition_id=ENROLLMENT_TRACK_PARTITION_ID) - response = self.client.get(reverse('courseware', kwargs={'course_id': str(self.verified_course.id)})) - self.assertContains(response, TEST_VERIFICATION_SOCK_LOCATOR, html=False) - - @override_waffle_flag(DISPLAY_COURSE_SOCK_FLAG, active=True) - def test_masquerade_as_verified_student(self): - user_group_id = self.get_group_id_by_course_mode_name( - self.verified_course.id, - 'Verified Certificate' - ) - self.update_masquerade(course=self.verified_course, group_id=user_group_id, - user_partition_id=ENROLLMENT_TRACK_PARTITION_ID) - response = self.client.get(reverse('courseware', kwargs={'course_id': str(self.verified_course.id)})) - self.assertNotContains(response, TEST_VERIFICATION_SOCK_LOCATOR, html=False) - - @override_waffle_flag(DISPLAY_COURSE_SOCK_FLAG, active=True) - def test_masquerade_as_masters_student(self): - user_group_id = self.get_group_id_by_course_mode_name( - self.masters_course.id, - 'Masters' - ) - self.update_masquerade(course=self.masters_course, group_id=user_group_id, - user_partition_id=ENROLLMENT_TRACK_PARTITION_ID) - response = self.client.get(reverse('courseware', kwargs={'course_id': str(self.masters_course.id)})) - - self.assertNotContains(response, TEST_VERIFICATION_SOCK_LOCATOR, html=False) diff --git a/openedx/features/course_experience/views/course_sock.py b/openedx/features/course_experience/views/course_sock.py deleted file mode 100644 index af3221315d24..000000000000 --- a/openedx/features/course_experience/views/course_sock.py +++ /dev/null @@ -1,47 +0,0 @@ -""" -Fragment for rendering the course's sock and associated toggle button. -""" - -from django.template.loader import render_to_string -from web_fragments.fragment import Fragment - -from lms.djangoapps.courseware.utils import ( - can_show_verified_upgrade, - verified_upgrade_deadline_link -) -from openedx.core.djangoapps.plugin_api.views import EdxFragmentView -from openedx.features.discounts.utils import format_strikeout_price -from common.djangoapps.student.models import CourseEnrollment - - -class CourseSockFragmentView(EdxFragmentView): - """ - A fragment to provide extra functionality in a dropdown sock. - """ - def render_to_fragment(self, request, course, **kwargs): # lint-amnesty, pylint: disable=arguments-differ - """ - Render the course's sock fragment. - """ - context = self.get_verification_context(request, course) - html = render_to_string('course_experience/course-sock-fragment.html', context) - return Fragment(html) - - @staticmethod - def get_verification_context(request, course): # lint-amnesty, pylint: disable=missing-function-docstring - enrollment = CourseEnrollment.get_enrollment(request.user, course.id) - show_course_sock = can_show_verified_upgrade(request.user, enrollment, course) - if show_course_sock: - upgrade_url = verified_upgrade_deadline_link(request.user, course=course) - course_price, _ = format_strikeout_price(request.user, course) - else: - upgrade_url = '' - course_price = '' - - context = { - 'show_course_sock': show_course_sock, - 'course_price': course_price, - 'course_id': course.id, - 'upgrade_url': upgrade_url, - } - - return context diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 3663296b289f..f1ef8845df80 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -804,7 +804,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/kernel.in openedx-django-wiki==2.1.0 # via -r requirements/edx/kernel.in -openedx-events==9.20.0 +openedx-events==10.0.0 # via # -r requirements/edx/kernel.in # edx-enterprise @@ -828,7 +828,7 @@ openedx-mongodbproxy==0.2.2 # via -r requirements/edx/kernel.in optimizely-sdk==5.2.0 # via -r requirements/edx/bundled.in -ora2==6.14.7 +ora2==6.15.1 # via -r requirements/edx/bundled.in packaging==24.2 # via diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 2ceb572fbbf7..5bfbca431f47 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1363,7 +1363,7 @@ openedx-django-wiki==2.1.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -openedx-events==9.20.0 +openedx-events==10.0.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1396,7 +1396,7 @@ optimizely-sdk==5.2.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -ora2==6.14.7 +ora2==6.15.1 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 43d2cb5aba2a..7b4fb5682a82 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -976,7 +976,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/base.txt openedx-django-wiki==2.1.0 # via -r requirements/edx/base.txt -openedx-events==9.20.0 +openedx-events==10.0.0 # via # -r requirements/edx/base.txt # edx-enterprise @@ -1000,7 +1000,7 @@ openedx-mongodbproxy==0.2.2 # via -r requirements/edx/base.txt optimizely-sdk==5.2.0 # via -r requirements/edx/base.txt -ora2==6.14.7 +ora2==6.15.1 # via -r requirements/edx/base.txt packaging==24.2 # via diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index d61ffb530edb..404104e9750d 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1034,7 +1034,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/base.txt openedx-django-wiki==2.1.0 # via -r requirements/edx/base.txt -openedx-events==9.20.0 +openedx-events==10.0.0 # via # -r requirements/edx/base.txt # edx-enterprise @@ -1058,7 +1058,7 @@ openedx-mongodbproxy==0.2.2 # via -r requirements/edx/base.txt optimizely-sdk==5.2.0 # via -r requirements/edx/base.txt -ora2==6.14.7 +ora2==6.15.1 # via -r requirements/edx/base.txt packaging==24.2 # via diff --git a/scripts/user_retirement/docs/special_cases.rst b/scripts/user_retirement/docs/special_cases.rst index ae544c3208c6..bded2d76bcd5 100644 --- a/scripts/user_retirement/docs/special_cases.rst +++ b/scripts/user_retirement/docs/special_cases.rst @@ -19,9 +19,10 @@ re-tried. You can do this using the Django admin. In this example, a user retirement errored during forums retirement, so we manually reset their state from ``ERRORED`` to ``ENROLLMENTS_COMPLETE``. -.. digraph:: retirement_states_example +.. graphviz:: :align: center + digraph G { //rankdir=LR; // Rank Direction Left to Right ranksep = "0.3"; @@ -49,6 +50,7 @@ from ``ERRORED`` to ``ENROLLMENTS_COMPLETE``. } ERRORED -> ENROLLMENTS_COMPLETE[style="bold,dashed",color=black,label=" via django\nadmin"] + } Now, the user retirement driver scripts will automatically resume this user's retirement the next time they are executed. diff --git a/webpack.common.config.js b/webpack.common.config.js index 4eea3b5da9b6..89082f218524 100644 --- a/webpack.common.config.js +++ b/webpack.common.config.js @@ -130,7 +130,6 @@ module.exports = Merge.smart({ CompletionOnViewService: './lms/static/completion/js/CompletionOnViewService.js', // Features - CourseSock: './openedx/features/course_experience/static/course_experience/js/CourseSock.js', Currency: './openedx/features/course_experience/static/course_experience/js/currency.js', AnnouncementsView: './openedx/features/announcements/static/announcements/jsx/Announcements.jsx', @@ -140,6 +139,7 @@ module.exports = Merge.smart({ ReactRenderer: './common/static/js/src/ReactRenderer.jsx', XModuleShim: './xmodule/js/src/xmodule.js', VerticalStudentView: './xmodule/assets/vertical/public/js/vertical_student_view.js', + VideoBlockMain: './xmodule/assets/video/public/js/video_block_main.js', commons: 'babel-polyfill' }, @@ -194,19 +194,19 @@ module.exports = Merge.smart({ multiple: [ { search: defineHeader, replace: '' }, { search: defineFooter, replace: '' }, - { + { search: /(\/\* RequireJS) \*\//g, replace(match, p1, offset, string) { return p1; } }, - { + { search: /\/\* Webpack/g, replace(match, p1, offset, string) { return match + ' */'; } }, - { + { search: /text!(.*?\.underscore)/g, replace(match, p1, offset, string) { return p1; @@ -657,13 +657,13 @@ module.exports = Merge.smart({ // We used to have node: { fs: 'empty' } in this file, // that is no longer supported. Adding this based on the recommendation in // https://stackoverflow.com/questions/64361940/webpack-error-configuration-node-has-an-unknown-property-fs - // + // // With this uncommented tests fail // Tests failed in the following suites: // * lms javascript // * xmodule-webpack javascript // Error: define cannot be used indirect - // + // // fallback: { // fs: false // } diff --git a/xmodule/annotatable_block.py b/xmodule/annotatable_block.py index dbcf0123c59a..9dcc27198d5d 100644 --- a/xmodule/annotatable_block.py +++ b/xmodule/annotatable_block.py @@ -140,9 +140,8 @@ def _render_content(self): """ Renders annotatable content with annotation spans and returns HTML. """ xmltree = etree.fromstring(self.data) - content = etree.tostring(xmltree, encoding='unicode') + self._extract_instructions(xmltree) - xmltree = etree.fromstring(content) xmltree.tag = 'div' if 'display_name' in xmltree.attrib: del xmltree.attrib['display_name'] diff --git a/xmodule/assets/video/public/js/play_pause_control.js b/xmodule/assets/video/public/js/play_pause_control.js new file mode 100644 index 000000000000..c2b2bdb2aa0e --- /dev/null +++ b/xmodule/assets/video/public/js/play_pause_control.js @@ -0,0 +1,95 @@ +import $ from 'jquery'; // jQuery import +import _ from 'underscore'; + +'use strict'; + +/** + * PlayPauseControl function + * + * @constructor + * @param {Object} state - The object containing the state of the video + * @param {Object} i18n - The object containing strings with translations + * @return {jQuery.Promise} - A resolved jQuery promise + */ +function VideoPlayPauseControl(state, i18n) { + if (!(this instanceof VideoPlayPauseControl)) { + return new VideoPlayPauseControl(state, i18n); + } + + _.bindAll(this, 'play', 'pause', 'onClick', 'destroy'); + this.state = state; + this.state.videoPlayPauseControl = this; + this.i18n = i18n; + + this.initialize(); + + return $.Deferred().resolve().promise(); +} + +VideoPlayPauseControl.prototype = { + template: [ + '', + ].join(''), + + /** Initializes the module. */ + initialize: function () { + this.el = $(this.template); + this.render(); + this.bindHandlers(); + }, + + /** + * Creates any necessary DOM elements, attach them, and set their, + * initial configuration. + */ + render: function () { + this.state.el.find('.vcr').prepend(this.el); + }, + + /** Bind any necessary function callbacks to DOM events. */ + bindHandlers: function () { + this.el.on('click', this.onClick); + this.state.el.on({ + play: this.play, + 'pause ended': this.pause, + destroy: this.destroy, + }); + }, + + onClick: function (event) { + event.preventDefault(); + this.state.videoCommands.execute('togglePlayback'); + }, + + play: function () { + this.el + .addClass('pause') + .removeClass('play') + .attr({title: gettext('Pause'), 'aria-label': gettext('Pause')}) + .find('.icon') + .removeClass('fa-play') + .addClass('fa-pause'); + }, + + pause: function () { + this.el + .removeClass('pause') + .addClass('play') + .attr({title: gettext('Play'), 'aria-label': gettext('Play')}) + .find('.icon') + .removeClass('fa-pause') + .addClass('fa-play'); + }, + + destroy: function () { + this.el.remove(); + this.state.el.off('destroy', this.destroy); + delete this.state.videoPlayPauseControl; + }, +}; + +export {VideoPlayPauseControl}; diff --git a/xmodule/assets/video/public/js/play_placeholder.js b/xmodule/assets/video/public/js/play_placeholder.js new file mode 100644 index 000000000000..d3d7384573bb --- /dev/null +++ b/xmodule/assets/video/public/js/play_placeholder.js @@ -0,0 +1,87 @@ +import $ from 'jquery'; // jQuery import +import _ from 'underscore'; + +'use strict'; + +/** + * Video Play placeholder control function. + * + * @constructor + * @param {Object} state - The object containing the state of the video + * @param {Object} i18n - The object containing strings with translations + * @return {jQuery.Promise} - A resolved jQuery promise + */ +function VideoPlayPlaceholder(state, i18n) { + if (!(this instanceof VideoPlayPlaceholder)) { + return new VideoPlayPlaceholder(state, i18n); + } + + _.bindAll(this, 'onClick', 'hide', 'show', 'destroy'); + this.state = state; + this.state.videoPlayPlaceholder = this; + this.i18n = i18n; + this.initialize(); + + return $.Deferred().resolve().promise(); +} + +VideoPlayPlaceholder.prototype = { + destroy: function () { + this.el.off('click', this.onClick); + this.state.el.off({ + destroy: this.destroy, + play: this.hide, + 'ended pause': this.show, + }); + this.hide(); + delete this.state.videoPlayPlaceholder; + }, + + /** + * Indicates whether the placeholder should be shown. + * We display it for HTML5 videos on iPad and Android devices. + * @return {Boolean} + */ + shouldBeShown: function () { + return /iPad|Android/i.test(this.state.isTouch[0]) && !this.state.isYoutubeType(); + }, + + /** Initializes the module. */ + initialize: function () { + if (!this.shouldBeShown()) { + return false; + } + + this.el = this.state.el.find('.btn-play'); + this.bindHandlers(); + this.show(); + }, + + /** Bind any necessary function callbacks to DOM events. */ + bindHandlers: function () { + this.el.on('click', this.onClick); + this.state.el.on({ + destroy: this.destroy, + play: this.hide, + 'ended pause': this.show, + }); + }, + + onClick: function () { + this.state.videoCommands.execute('play'); + }, + + hide: function () { + this.el + .addClass('is-hidden') + .attr({'aria-hidden': 'true', tabindex: -1}); + }, + + show: function () { + this.el + .removeClass('is-hidden') + .attr({'aria-hidden': 'false', tabindex: 0}); + } +}; + +export {VideoPlayPlaceholder}; \ No newline at end of file diff --git a/xmodule/assets/video/public/js/play_skip_control.js b/xmodule/assets/video/public/js/play_skip_control.js new file mode 100644 index 000000000000..3be7d8b05398 --- /dev/null +++ b/xmodule/assets/video/public/js/play_skip_control.js @@ -0,0 +1,90 @@ +import $ from 'jquery'; // jQuery import +import _ from 'underscore'; + +'use strict'; + +/** + * Play/skip control module + * + * @constructor + * @param {Object} state - The object containing the state of the video + * @param {Object} i18n - The object containing strings with translations + * @return {jQuery.Promise} - A resolved jQuery promise + */ +function VideoPlaySkipControl(state, i18n) { + if (!(this instanceof VideoPlaySkipControl)) { + return new VideoPlaySkipControl(state, i18n); + } + + _.bindAll(this, 'play', 'onClick', 'destroy'); + this.state = state; + this.state.videoPlaySkipControl = this; + this.i18n = i18n; + + this.initialize(); + + return $.Deferred().resolve().promise(); +} + +VideoPlaySkipControl.prototype = { + template: [ + '', + ].join(''), + + /** Initializes the module. */ + initialize: function () { + this.el = $(this.template); + this.render(); + this.bindHandlers(); + }, + + /** + * Creates any necessary DOM elements, attach them, and set their, + * initial configuration. + */ + render: function () { + this.state.el.find('.vcr').prepend(this.el); + }, + + /** Bind any necessary function callbacks to DOM events. */ + bindHandlers: function () { + this.el.on('click', this.onClick); + this.state.el.on({ + play: this.play, + destroy: this.destroy, + }); + }, + + onClick: function (event) { + event.preventDefault(); + if (this.state.videoPlayer.isPlaying()) { + this.state.videoCommands.execute('skip'); + } else { + this.state.videoCommands.execute('play'); + } + }, + + play: function () { + this.el + .removeClass('play') + .addClass('skip') + .attr('title', gettext('Skip')) + .find('.icon') + .removeClass('fa-play') + .addClass('fa-step-forward'); + // Disable possibility to pause the video. + this.state.el.find('video').off('click'); + }, + + destroy: function () { + this.el.remove(); + this.state.el.off('destroy', this.destroy); + delete this.state.videoPlaySkipControl; + }, +}; + +export {VideoPlaySkipControl}; \ No newline at end of file diff --git a/xmodule/assets/video/public/js/poster.js b/xmodule/assets/video/public/js/poster.js new file mode 100644 index 000000000000..ef059b81fa08 --- /dev/null +++ b/xmodule/assets/video/public/js/poster.js @@ -0,0 +1,69 @@ +import $ from 'jquery'; // jQuery import +import _ from 'underscore'; + +'use strict'; + +/** + * VideoPoster function + * + * @constructor + * @param {jQuery} element + * @param {Object} options + */ +function VideoPoster(element, options) { + if (!(this instanceof VideoPoster)) { + return new VideoPoster(element, options); + } + + _.bindAll(this, 'onClick', 'destroy'); + this.element = element; + this.container = element.find('.video-player'); + this.options = options || {}; + this.initialize(); +} + +VideoPoster.moduleName = 'Poster'; +VideoPoster.prototype = { + template: _.template([ + '
', + '', + '
' + ].join('')), + + initialize: function () { + this.el = $(this.template({ + url: this.options.poster.url, + type: this.options.poster.type + })); + this.element.addClass('is-pre-roll'); + this.render(); + this.bindHandlers(); + }, + + bindHandlers: function () { + this.el.on('click', this.onClick); + this.element.on('destroy', this.destroy); + }, + + render: function () { + this.container.append(this.el); + }, + + onClick: function () { + if (_.isFunction(this.options.onClick)) { + this.options.onClick(); + } + this.destroy(); + }, + + destroy: function () { + this.element.off('destroy', this.destroy).removeClass('is-pre-roll'); + this.el.remove(); + }, +}; + +export {VideoPoster}; \ No newline at end of file diff --git a/xmodule/assets/video/public/js/skip_control.js b/xmodule/assets/video/public/js/skip_control.js new file mode 100644 index 000000000000..326c8e8fbb64 --- /dev/null +++ b/xmodule/assets/video/public/js/skip_control.js @@ -0,0 +1,70 @@ +import $ from 'jquery'; // jQuery import +import _ from 'underscore'; + +'use strict'; + +/** + * VideoSkipControl function + * + * @constructor + * @param {Object} state The object containing the state of the video + * @param {Object} i18n The object containing strings with translations + * @return {jQuery.Promise} Returns a resolved jQuery promise + */ +function VideoSkipControl(state, i18n) { + if (!(this instanceof VideoSkipControl)) { + return new VideoSkipControl(state, i18n); + } + + _.bindAll(this, 'onClick', 'render', 'destroy'); + this.state = state; + this.state.videoSkipControl = this; + this.i18n = i18n; + + this.initialize(); + + return $.Deferred().resolve().promise(); +} + +VideoSkipControl.prototype = { + template: [ + '', + ].join(''), + + initialize: function () { + this.el = $(this.template); + this.bindHandlers(); + }, + + /** Creates any necessary DOM elements, attach them, and set their, initial configuration. */ + render: function () { + this.state.el.find('.vcr .control').after(this.el); + }, + + /** Bind any necessary function callbacks to DOM events. */ + bindHandlers: function () { + this.el.on('click', this.onClick); + + this.state.el.on({ + 'play.skip': _.once(this.render), + 'destroy.skip': this.destroy, + }); + }, + + onClick: function (event) { + event.preventDefault(); + this.state.videoCommands.execute('skip', true); + }, + + destroy: function () { + this.el.remove(); + this.state.el.off('.skip'); + delete this.state.videoSkipControl; + }, +}; + +export {VideoSkipControl}; \ No newline at end of file diff --git a/xmodule/assets/video/public/js/video_accessible_menu.js b/xmodule/assets/video/public/js/video_accessible_menu.js new file mode 100644 index 000000000000..efcbff1cf63f --- /dev/null +++ b/xmodule/assets/video/public/js/video_accessible_menu.js @@ -0,0 +1,56 @@ +import _ from 'underscore'; + +/** + * Video Download Transcript control module. + * + * @constructor + * @param {jQuery} element + * @param {Object} options + */ +function VideoTranscriptDownloadHandler(element, options = {}) { + if (!(this instanceof VideoTranscriptDownloadHandler)) { + return new VideoTranscriptDownloadHandler(element, options); + } + + _.bindAll(this, 'clickHandler'); + + this.container = element; + this.options = options; + + if (this.container.find('.wrapper-downloads .wrapper-download-transcripts')) { + this.initialize(); + } +} + +VideoTranscriptDownloadHandler.prototype = { + // Initializes the module. + initialize() { + this.value = this.options.storage.getItem('transcript_download_format'); + this.el = this.container.find('.list-download-transcripts'); + this.el.on('click', '.btn-link', this.clickHandler); + }, + + // Event handler. We delay link clicks until the file type is set + clickHandler(event) { + event.preventDefault(); + + const fileType = $(event.target).data('value'); + const data = {transcript_download_format: fileType}; + const downloadUrl = $(event.target).attr('href'); + + $.ajax({ + url: this.options.saveStateUrl, + type: 'POST', + dataType: 'json', + data: data, + success: () => { + this.options.storage.setItem('transcript_download_format', fileType); + }, + complete: () => { + document.location.href = downloadUrl; + }, + }); + }, +}; + +export {VideoTranscriptDownloadHandler}; diff --git a/xmodule/assets/video/public/js/video_block_main.js b/xmodule/assets/video/public/js/video_block_main.js new file mode 100644 index 000000000000..90fbd47a1acd --- /dev/null +++ b/xmodule/assets/video/public/js/video_block_main.js @@ -0,0 +1,186 @@ +// Import required modules and dependencies +import $ from 'jquery'; +import _ from 'underscore'; +import {VideoStorage} from './video_storage'; +import {VideoPoster} from './poster'; +import {VideoTranscriptDownloadHandler} from './video_accessible_menu'; +import {VideoSkipControl} from './skip_control'; +import {VideoPlayPlaceholder} from './play_placeholder'; +import {VideoPlaySkipControl} from './play_skip_control'; +import {VideoPlayPauseControl} from './play_pause_control'; + + +// TODO: Uncomment the imports +// import { initialize } from './initialize'; // Assuming this function is imported +// import { +// FocusGrabber, +// VideoControl, +// VideoProgressSlider, +// VideoSpeedControl, +// VideoVolumeControl, +// VideoQualityControl, +// VideoFullScreen, +// VideoCaption, +// VideoCommands, +// VideoContextMenu, +// VideoSaveStatePlugin, +// VideoEventsPlugin, +// VideoCompletionHandler, +// VideoTranscriptFeedback, +// VideoAutoAdvanceControl, +// VideoEventsBumperPlugin, +// VideoSocialSharing, +// VideoBumper, +// } from './video_modules'; // Assuming all necessary modules are grouped here + +// Stub gettext if the runtime doesn't provide it +if (typeof window.gettext === 'undefined') { + window.gettext = function (text) { + return text; + }; +} + + +'use strict'; + +console.log('In video_block_main.js file'); + +(function () { + var youtubeXhr = null; + var oldVideo = window.Video; + + window.Video = function (runtime, element) { + console.log('In Video initialize method'); + + const el = $(element).find('.video'); + const id = el.attr('id').replace(/video_/, ''); + const storage = new VideoStorage('VideoState', id); + const bumperMetadata = el.data('bumper-metadata'); + const autoAdvanceEnabled = el.data('autoadvance-enabled') === 'True'; + + const mainVideoModules = [ + // FocusGrabber, + // VideoControl, + VideoPlayPlaceholder, + VideoPlayPauseControl, + // VideoProgressSlider, + // VideoSpeedControl, + // VideoVolumeControl, + // VideoQualityControl, + // VideoFullScreen, + // VideoCaption, + // VideoCommands, + // VideoContextMenu, + // VideoSaveStatePlugin, + // VideoEventsPlugin, + // VideoCompletionHandler, + // VideoTranscriptFeedback, + // ].concat(autoAdvanceEnabled ? [VideoAutoAdvanceControl] : []); + ] + + const bumperVideoModules = [ + // VideoControl, + VideoPlaySkipControl, + VideoSkipControl, + // VideoVolumeControl, + // VideoCaption, + // VideoCommands, + // VideoSaveStatePlugin, + // VideoTranscriptFeedback, + // VideoEventsBumperPlugin, + // VideoCompletionHandler, + ]; + + const state = { + el: el, + id: id, + metadata: el.data('metadata'), + storage: storage, + options: {}, + youtubeXhr: youtubeXhr, + modules: mainVideoModules, + }; + + const getBumperState = (metadata) => { + return $.extend(true, { + el: el, + id: id, + storage: storage, + options: {SaveStatePlugin: {events: ['language_menu:change']}}, + youtubeXhr: youtubeXhr, + modules: bumperVideoModules, + }, {metadata: metadata}); + }; + + const player = (innerState) => { + return () => { + _.extend(innerState.metadata, {autoplay: true, focusFirstControl: true}); + // TODO: Uncomment following initialize method calling + // initialize(innerState, element); + }; + }; + + VideoTranscriptDownloadHandler(el, { + storage: storage, + saveStateUrl: state.metadata.saveStateUrl, + }); + + // VideoSocialSharing(el); + + if (bumperMetadata) { + VideoPoster(el, { + poster: el.data('poster'), + onClick: _.once(function () { + const mainVideoPlayer = player(state); + + if (storage.getItem('isBumperShown')) { + mainVideoPlayer(); + } else { + const bumperState = getBumperState(bumperMetadata); + const bumper = new VideoBumper(player(bumperState), bumperState); + + state.bumperState = bumperState; + + bumper.getPromise().then(() => { + delete state.bumperState; + mainVideoPlayer(); + }); + } + }), + }); + } else { + // TODO: Uncomment following initialize method calling + // initialize(state, element); + } + + if (!youtubeXhr) { + youtubeXhr = state.youtubeXhr; + } + + el.data('video-player-state', state); + const onSequenceChange = () => { + if (state && state.videoPlayer) { + state.videoPlayer.destroy(); + } + $('.sequence').off('sequence:change', onSequenceChange); + }; + $('.sequence').on('sequence:change', onSequenceChange); + + // Because the 'state' object is only available inside this closure, we will also make it available to + // the caller by returning it. This is necessary so that we can test Video with Jasmine. + return state; + }; + + window.Video.clearYoutubeXhr = function () { + youtubeXhr = null; + }; + + // TODO: Uncomment following initialize related code + // window.Video.loadYouTubeIFrameAPI = initialize.prototype.loadYouTubeIFrameAPI; + + // Invoke the mock Video constructor so that the elements stored within it can be processed by the real + // `window.Video` constructor. + // TODO: Un comment following + // oldVideo(null, true); + +}()); diff --git a/xmodule/assets/video/public/js/video_storage.js b/xmodule/assets/video/public/js/video_storage.js new file mode 100644 index 000000000000..5262041b8778 --- /dev/null +++ b/xmodule/assets/video/public/js/video_storage.js @@ -0,0 +1,79 @@ +'use strict'; + +console.log('In video_storage.js file'); + +/** + * Provides a convenient way to store key-value pairs. + * + * @param {string} namespace Namespace that is used to store data. + * @param {string} id Identifier for instance-specific storage. + * @return {object} VideoStorage API. + */ +export function VideoStorage(namespace = 'VideoStorage', id = Math.random().toString(36).slice(2)) { + /** + * Adds new value to the storage or rewrites existent. + * + * @param {string} name Identifier of the data. + * @param {any} value Data to store. + * @param {boolean} instanceSpecific Data with this flag will be added + * to instance specific storage. + */ + const setItem = (name, value, instanceSpecific) => { + if (name) { + if (instanceSpecific) { + window[namespace][id][name] = value; + } else { + window[namespace][name] = value; + } + } + }; + + /** + * Returns the current value associated with the given name. + * + * @param {string} name Identifier of the data. + * @param {boolean} instanceSpecific Data with this flag will be added + * to instance specific storage. + * @return {any} The current value associated with the given name. + * If the given key does not exist in the list + * associated with the object then this method must return null. + */ + const getItem = (name, instanceSpecific) => { + if (instanceSpecific) { + return window[namespace][id][name]; + } else { + return window[namespace][name]; + } + }; + + /** + * Removes the current value associated with the given name. + * + * @param {string} name Identifier of the data. + * @param {boolean} instanceSpecific Data with this flag will be added + * to instance specific storage. + */ + const removeItem = (name, instanceSpecific) => { + if (instanceSpecific) { + delete window[namespace][id][name]; + } else { + delete window[namespace][name]; + } + }; + + /** Clear the storage. */ + const clear = () => { + window[namespace] = {}; + window[namespace][id] = {}; + }; + + window[namespace] = window[namespace] || {}; + window[namespace][id] = window[namespace][id] || {}; + + return { + clear: clear, + getItem: getItem, + removeItem: removeItem, + setItem: setItem + }; +} diff --git a/xmodule/tests/test_annotatable_block.py b/xmodule/tests/test_annotatable_block.py index b9f419ee3a0f..3de253047a8d 100644 --- a/xmodule/tests/test_annotatable_block.py +++ b/xmodule/tests/test_annotatable_block.py @@ -134,3 +134,11 @@ def test_extract_instructions(self): xmltree = etree.fromstring('foo') actual = self.annotatable._extract_instructions(xmltree) # lint-amnesty, pylint: disable=protected-access assert actual is None + + def test_instruction_removal(self): + xmltree = etree.fromstring(self.sample_xml) + instructions = self.annotatable._extract_instructions(xmltree) # pylint: disable=protected-access + + assert instructions is not None + assert "Read the text." in instructions + assert xmltree.find("instructions") is None diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index b0e01068fbf3..73ed4579ab83 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -246,8 +246,8 @@ def student_view(self, context): """ fragment = Fragment(self.get_html(context=context)) add_css_to_fragment(fragment, 'VideoBlockDisplay.css') - add_webpack_js_to_fragment(fragment, 'VideoBlockDisplay') - shim_xmodule_js(fragment, 'Video') + add_webpack_js_to_fragment(fragment, 'VideoBlockMain') + fragment.initialize_js('Video') return fragment def author_view(self, context):