Merge pull request #10806 from edx/shr/bug/TNL-3076-Handout-links-not-updated-at-import

Update handout link with current course_id

common/lib/xmodule/xmodule/tests/test_video.py

@@ -18,10 +18,12 @@ from uuid import uuid4
 
 from lxml import etree
 from mock import ANY, Mock, patch
+import ddt
 
 from django.conf import settings
 
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from opaque_keys.edx.keys import CourseKey
 from xblock.field_data import DictFieldData
 from xblock.fields import ScopeIds
 
@@ -232,6 +234,7 @@ class TestCreateYoutubeString(VideoDescriptorTestBase):
         self.assertEqual(create_youtube_string(self.descriptor), expected)
 
 
+@ddt.ddt
 class VideoDescriptorImportTestCase(unittest.TestCase):
     """
     Make sure that VideoDescriptor can import an old XML-based video correctly.
@@ -313,6 +316,54 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
             'transcripts': {'uk': 'ukrainian_translation.srt', 'de': 'german_translation.srt'},
         })
 
+    @ddt.data(
+        ('course-v1:test_course+test_org+test_run',
+         '/asset-v1:test_course+test_org+test_run+type@asset+block@test.png'),
+        ('test_course/test_org/test_run', '/c4x/test_course/test_org/asset/test.png')
+    )
+    @ddt.unpack
+    def test_from_xml_when_handout_is_course_asset(self, course_id_string, expected_handout_link):
+        """
+        Test that if handout link is course_asset then it will contain targeted course_id in handout link.
+        """
+        module_system = DummySystem(load_error_modules=True)
+        course_id = CourseKey.from_string(course_id_string)
+        xml_data = '''
+            <video display_name="Test Video"
+                   youtube="1.0:p2Q6BrNhdh8,0.75:izygArpw-Qo,1.25:1EeWXzPdhSA,1.5:rABDYkeK0x8"
+                   show_captions="false"
+                   download_track="false"
+                   start_time="00:00:01"
+                   download_video="false"
+                   end_time="00:01:00">
+              <source src="http://www.example.com/source.mp4"/>
+              <track src="http://www.example.com/track"/>
+              <handout src="/asset-v1:test_course_1+test_org_1+test_run_1+type@asset+block@test.png"/>
+              <transcript language="uk" src="ukrainian_translation.srt" />
+              <transcript language="de" src="german_translation.srt" />
+            </video>
+        '''
+        id_generator = Mock()
+        id_generator.target_course_id = course_id
+
+        output = VideoDescriptor.from_xml(xml_data, module_system, id_generator)
+        self.assert_attributes_equal(output, {
+            'youtube_id_0_75': 'izygArpw-Qo',
+            'youtube_id_1_0': 'p2Q6BrNhdh8',
+            'youtube_id_1_25': '1EeWXzPdhSA',
+            'youtube_id_1_5': 'rABDYkeK0x8',
+            'show_captions': False,
+            'start_time': datetime.timedelta(seconds=1),
+            'end_time': datetime.timedelta(seconds=60),
+            'track': 'http://www.example.com/track',
+            'handout': expected_handout_link,
+            'download_track': False,
+            'download_video': False,
+            'html5_sources': ['http://www.example.com/source.mp4'],
+            'data': '',
+            'transcripts': {'uk': 'ukrainian_translation.srt', 'de': 'german_translation.srt'},
+        })
+
     def test_from_xml_missing_attributes(self):
         """
         Ensure that attributes have the right values if they aren't

common/lib/xmodule/xmodule/video_module/video_module.py

@@ -27,6 +27,7 @@ from openedx.core.lib.cache_utils import memoize_in_request_cache
 from xblock.core import XBlock
 from xblock.fields import ScopeIds
 from xblock.runtime import KvsFieldData
+from opaque_keys.edx.locator import AssetLocator
 
 from xmodule.modulestore.inheritance import InheritanceKeyValueStore, own_metadata
 from xmodule.x_module import XModule, module_attr
@@ -34,6 +35,7 @@ from xmodule.editing_module import TabsEditingDescriptor
 from xmodule.raw_module import EmptyDataRawDescriptor
 from xmodule.xml_module import is_pointer_tag, name_to_pathname, deserialize_field
 from xmodule.exceptions import NotFoundError
+from xmodule.contentstore.content import StaticContent
 
 from .transcripts_utils import VideoTranscriptsMixin, Transcript, get_html5_ids
 from .video_utils import create_youtube_string, get_video_from_cdn, get_poster
@@ -714,6 +716,14 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
                 # for about a month we did it for Strings also).
                 field_data[attr] = deserialize_field(cls.fields[attr], value)
 
+        course_id = getattr(id_generator, 'target_course_id', None)
+        # Update the handout location with current course_id
+        if 'handout' in field_data.keys() and course_id:
+            handout_location = StaticContent.get_location_from_path(field_data['handout'])
+            if isinstance(handout_location, AssetLocator):
+                handout_new_location = StaticContent.compute_location(course_id, handout_location.path)
+                field_data['handout'] = StaticContent.serialize_asset_key_with_slash(handout_new_location)
+
         # For backwards compatibility: Add `source` if XML doesn't have `download_video`
         # attribute.
         if 'download_video' not in field_data and sources:
@@ -735,7 +745,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
             edxval_api.import_from_xml(
                 video_asset_elem,
                 field_data['edx_video_id'],
-                course_id=getattr(id_generator, 'target_course_id', None)
+                course_id=course_id
             )
 
         # load license if it exists