Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions cms/djangoapps/contentstore/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -390,6 +392,14 @@ 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.
downstream_xblock.edx_video_id = create_external_video(display_name='external video')
store.update_item(downstream_xblock, request.user.id)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Do we need to save the downstream_xblock here? _insert_static_files_into_downstream_xblock also performs an update_item.

Suggested change
store.update_item(downstream_xblock, request.user.id)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And is there any way to add a test for this case, so we don't accidentally break it in future?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Do we need to save the downstream_xblock here? _insert_static_files_into_downstream_xblock also performs an update_item.

@pomegranited I think it's necessary here, because the update_item in _insert_static_files_into_downstream_xblock is conditional on substitutions, and I'm not sure if those conditions will always apply. What do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, I didn't read the full context of that call. This is fine as is!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And is there any way to add a test for this case, so we don't accidentally break it in future?

Tests added: e05d8d4


# 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -329,6 +332,60 @@ def test_400_no_upstream(self):
assert "is not linked" in response.data["developer_message"][0]


class CreateDownstreamViewTest(CourseTestCase, _BaseDownstreamViewTestMixin, SharedModuleStoreTestCase):
"""
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",
"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_post(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_post(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 is not None


class PostDownstreamSyncViewTest(_DownstreamSyncViewTestMixin, SharedModuleStoreTestCase):
"""
Test that `POST /api/v2/contentstore/downstreams/.../sync` initiates a sync from the linked upstream.
Expand Down