From 4bdf696ca45f33dae72a1a7e29c0551b0a472339 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 9 Apr 2025 21:03:18 -0500 Subject: [PATCH 1/5] fix: Transcripts in downstream creation --- cms/djangoapps/contentstore/helpers.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index 333cdbeb76ee..26bf1e4260fe 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -394,6 +394,12 @@ def import_static_assets_for_library_sync(downstream_xblock: XBlock, lib_block: store = modulestore() try: with store.bulk_operations(downstream_xblock.context_key): + if downstream_xblock.usage_key.block_type == 'video' and not downstream_xblock.edx_video_id: + # If the `downstream_xblock` is a new created block, we need to create + # a new `edx_video_id` to import the transcripts. + downstream_xblock.edx_video_id = create_external_video(display_name='external video') + store.update_item(downstream_xblock, request.user.id) + # Now handle static files that need to go into Files & Uploads. # If the required files already exist, nothing will happen besides updating the olx. notices = _insert_static_files_into_downstream_xblock(downstream_xblock, staged_content.id, request) From e05d8d436329615f1d299fea9e55df54eb0d31da Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 10 Apr 2025 21:23:20 -0500 Subject: [PATCH 2/5] test: Add CreateDownstreamViewTest --- .../v2/views/tests/test_downstreams.py | 56 ++++++++++++++++++- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py index 0ef7c8c32dc4..23eafe050c1d 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py @@ -3,14 +3,17 @@ """ import json from datetime import datetime, timezone -from unittest.mock import patch +from unittest.mock import patch, MagicMock from django.conf import settings +from django.urls import reverse from freezegun import freeze_time from organizations.models import Organization from cms.djangoapps.contentstore.helpers import StaticFileNotices from cms.lib.xblock.upstream_sync import BadUpstream, UpstreamLink +from cms.djangoapps.contentstore.tests.utils import CourseTestCase +from opaque_keys.edx.keys import UsageKey from common.djangoapps.student.tests.factories import UserFactory from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase @@ -171,7 +174,6 @@ def test_404_downstream_not_accessible(self): assert response.status_code == 404 assert "not found" in response.data["developer_message"] - class GetDownstreamViewTest(SharedErrorTestCases, SharedModuleStoreTestCase): """ Test that `GET /api/v2/contentstore/downstreams/...` inspects a downstream's link to an upstream. @@ -329,6 +331,56 @@ def test_400_no_upstream(self): assert "is not linked" in response.data["developer_message"][0] +class CreateDownstreamViewTest(CourseTestCase, SharedErrorTestCases, SharedModuleStoreTestCase): + """ + Tests create new downstream blocks + """ + def call_api(self, library_content_key, category): + data = { + "parent_locator": str(self.course.location), + "display_name": "Test block", + "library_content_key": library_content_key, + "category": category, + } + return self.client.post( + reverse("xblock_handler"), + data=json.dumps(data), + content_type="application/json", + ) + + def test_200(self): + response = self.call_api(self.html_lib_id, "html") + + assert response.status_code == 200 + data = response.json() + assert data["upstreamRef"] == self.html_lib_id + + usage_key = UsageKey.from_string(data["locator"]) + item = modulestore().get_item(usage_key) + assert item.upstream == self.html_lib_id + + @patch("cms.djangoapps.contentstore.helpers._insert_static_files_into_downstream_xblock") + @patch("cms.djangoapps.contentstore.helpers.content_staging_api.stage_xblock_temporarily") + @patch("cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers.sync_from_upstream") + def test_200_video(self, mock_sync, mock_stage, mock_insert): + mock_lib_block = MagicMock() + mock_lib_block.runtime.get_block_assets.return_value = ['mocked_asset'] + mock_sync.return_value = mock_lib_block + mock_stage.return_value = MagicMock() + mock_insert.return_value = StaticFileNotices() + + response = self.call_api(self.video_lib_id, "video") + + assert response.status_code == 200 + data = response.json() + assert data["upstreamRef"] == self.video_lib_id + + usage_key = UsageKey.from_string(data["locator"]) + item = modulestore().get_item(usage_key) + assert item.upstream == self.video_lib_id + assert item.edx_video_id != None + + class PostDownstreamSyncViewTest(_DownstreamSyncViewTestMixin, SharedModuleStoreTestCase): """ Test that `POST /api/v2/contentstore/downstreams/.../sync` initiates a sync from the linked upstream. From d202e18917526f5a3083fad65fdceadf7cd0aabc Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 11 Apr 2025 11:31:24 -0500 Subject: [PATCH 3/5] style: Fix broken lint --- .../rest_api/v2/views/tests/test_downstreams.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py index 23eafe050c1d..861a12a41102 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py @@ -174,6 +174,7 @@ def test_404_downstream_not_accessible(self): assert response.status_code == 404 assert "not found" in response.data["developer_message"] + class GetDownstreamViewTest(SharedErrorTestCases, SharedModuleStoreTestCase): """ Test that `GET /api/v2/contentstore/downstreams/...` inspects a downstream's link to an upstream. @@ -331,11 +332,11 @@ def test_400_no_upstream(self): assert "is not linked" in response.data["developer_message"][0] -class CreateDownstreamViewTest(CourseTestCase, SharedErrorTestCases, SharedModuleStoreTestCase): +class CreateDownstreamViewTest(CourseTestCase, _BaseDownstreamViewTestMixin, SharedModuleStoreTestCase): """ Tests create new downstream blocks """ - def call_api(self, library_content_key, category): + def call_api_post(self, library_content_key, category): data = { "parent_locator": str(self.course.location), "display_name": "Test block", @@ -349,7 +350,7 @@ def call_api(self, library_content_key, category): ) def test_200(self): - response = self.call_api(self.html_lib_id, "html") + response = self.call_api_post(self.html_lib_id, "html") assert response.status_code == 200 data = response.json() @@ -369,7 +370,7 @@ def test_200_video(self, mock_sync, mock_stage, mock_insert): mock_stage.return_value = MagicMock() mock_insert.return_value = StaticFileNotices() - response = self.call_api(self.video_lib_id, "video") + response = self.call_api_post(self.video_lib_id, "video") assert response.status_code == 200 data = response.json() @@ -378,7 +379,7 @@ def test_200_video(self, mock_sync, mock_stage, mock_insert): usage_key = UsageKey.from_string(data["locator"]) item = modulestore().get_item(usage_key) assert item.upstream == self.video_lib_id - assert item.edx_video_id != None + assert item.edx_video_id is not None class PostDownstreamSyncViewTest(_DownstreamSyncViewTestMixin, SharedModuleStoreTestCase): From d46bbed58ec9cf2d8012770555fa67c7f750242a Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 11 Apr 2025 11:43:31 -0500 Subject: [PATCH 4/5] style: Fix broken lint --- .../contentstore/rest_api/v2/views/tests/test_downstreams.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py index 861a12a41102..ca590273677f 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py @@ -337,6 +337,10 @@ class CreateDownstreamViewTest(CourseTestCase, _BaseDownstreamViewTestMixin, Sha Tests create new downstream blocks """ def call_api_post(self, library_content_key, category): + """ + Call the api to create a downstream block using + `library_content_key` as upstream + """ data = { "parent_locator": str(self.course.location), "display_name": "Test block", From ad89ff9d038a028880cc0fec371a721690db914d Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 16 Apr 2025 11:50:30 -0500 Subject: [PATCH 5/5] style: Add comments in the code --- cms/djangoapps/contentstore/helpers.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index b92ac3b2be25..506ce766ff23 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -298,6 +298,8 @@ def _insert_static_files_into_downstream_xblock( static_files=static_files, usage_key=downstream_xblock.usage_key, ) + # FIXME: This code shouldn't have any special cases for specific block types like video + # in the future. if downstream_xblock.usage_key.block_type == 'video': _import_transcripts( downstream_xblock, @@ -390,6 +392,8 @@ def import_static_assets_for_library_sync(downstream_xblock: XBlock, lib_block: store = modulestore() try: with store.bulk_operations(downstream_xblock.context_key): + # FIXME: This code shouldn't have any special cases for specific block types like video + # in the future. if downstream_xblock.usage_key.block_type == 'video' and not downstream_xblock.edx_video_id: # If the `downstream_xblock` is a new created block, we need to create # a new `edx_video_id` to import the transcripts.