Merge pull request #14664 from edx/mrehan/prioritize-youtube-subs-over-html5-on-save
TNL-6539 – Prioritize youtube subs over html5 ones
This commit is contained in:
@@ -40,6 +40,20 @@ class BaseTranscripts(CourseTestCase):
|
||||
except NotFoundError:
|
||||
pass
|
||||
|
||||
def save_subs_to_store(self, subs, subs_id):
|
||||
"""
|
||||
Save transcripts into `StaticContent`.
|
||||
"""
|
||||
filedata = json.dumps(subs, indent=2)
|
||||
mime_type = 'application/json'
|
||||
filename = 'subs_{0}.srt.sjson'.format(subs_id)
|
||||
|
||||
content_location = StaticContent.compute_location(self.course.id, filename)
|
||||
content = StaticContent(content_location, filename, mime_type, filedata)
|
||||
contentstore().save(content)
|
||||
del_cached_content(content_location)
|
||||
return content_location
|
||||
|
||||
def setUp(self):
|
||||
"""Create initial data."""
|
||||
super(BaseTranscripts, self).setUp()
|
||||
@@ -82,8 +96,9 @@ class BaseTranscripts(CourseTestCase):
|
||||
|
||||
|
||||
class TestUploadTranscripts(BaseTranscripts):
|
||||
"""Tests for '/transcripts/upload' url."""
|
||||
|
||||
"""
|
||||
Tests for '/transcripts/upload' url.
|
||||
"""
|
||||
def setUp(self):
|
||||
"""Create initial data."""
|
||||
super(TestUploadTranscripts, self).setUp()
|
||||
@@ -347,20 +362,9 @@ class TestUploadTranscripts(BaseTranscripts):
|
||||
|
||||
|
||||
class TestDownloadTranscripts(BaseTranscripts):
|
||||
"""Tests for '/transcripts/download' url."""
|
||||
|
||||
def save_subs_to_store(self, subs, subs_id):
|
||||
"""Save transcripts into `StaticContent`."""
|
||||
filedata = json.dumps(subs, indent=2)
|
||||
mime_type = 'application/json'
|
||||
filename = 'subs_{0}.srt.sjson'.format(subs_id)
|
||||
|
||||
content_location = StaticContent.compute_location(self.course.id, filename)
|
||||
content = StaticContent(content_location, filename, mime_type, filedata)
|
||||
contentstore().save(content)
|
||||
del_cached_content(content_location)
|
||||
return content_location
|
||||
|
||||
"""
|
||||
Tests for '/transcripts/download' url.
|
||||
"""
|
||||
def test_success_download_youtube(self):
|
||||
self.item.data = '<video youtube="1:JMD_ifUUfsU" />'
|
||||
modulestore().update_item(self.item, self.user.id)
|
||||
@@ -523,20 +527,9 @@ class TestDownloadTranscripts(BaseTranscripts):
|
||||
|
||||
|
||||
class TestCheckTranscripts(BaseTranscripts):
|
||||
"""Tests for '/transcripts/check' url."""
|
||||
|
||||
def save_subs_to_store(self, subs, subs_id):
|
||||
"""Save transcripts into `StaticContent`."""
|
||||
filedata = json.dumps(subs, indent=2)
|
||||
mime_type = 'application/json'
|
||||
filename = 'subs_{0}.srt.sjson'.format(subs_id)
|
||||
|
||||
content_location = StaticContent.compute_location(self.course.id, filename)
|
||||
content = StaticContent(content_location, filename, mime_type, filedata)
|
||||
contentstore().save(content)
|
||||
del_cached_content(content_location)
|
||||
return content_location
|
||||
|
||||
"""
|
||||
Tests for '/transcripts/check' url.
|
||||
"""
|
||||
def test_success_download_nonyoutube(self):
|
||||
subs_id = str(uuid4())
|
||||
self.item.data = textwrap.dedent("""
|
||||
@@ -766,3 +759,63 @@ class TestCheckTranscripts(BaseTranscripts):
|
||||
resp = self.client.get(link, {'data': json.dumps(data)})
|
||||
self.assertEqual(resp.status_code, 400)
|
||||
self.assertEqual(json.loads(resp.content).get('status'), 'Transcripts are supported only for "video" modules.')
|
||||
|
||||
|
||||
class TestSaveTranscripts(BaseTranscripts):
|
||||
"""
|
||||
Tests for '/transcripts/save' url.
|
||||
"""
|
||||
def assert_current_subs(self, expected_subs):
|
||||
"""
|
||||
Asserts the current subtitles set on the video module.
|
||||
|
||||
Arguments:
|
||||
expected_subs (String): Expected current subtitles for video.
|
||||
"""
|
||||
item = modulestore().get_item(self.video_usage_key)
|
||||
self.assertEqual(item.sub, expected_subs)
|
||||
|
||||
def test_prioritize_youtube_sub_on_save(self):
|
||||
"""
|
||||
Test that the '/transcripts/save' endpoint prioritises youtube subtitles over html5 ones
|
||||
while deciding the current subs for video module.
|
||||
"""
|
||||
# Update video module to contain 1 youtube and 2 html5 sources.
|
||||
youtube_id = str(uuid4())
|
||||
self.item.data = textwrap.dedent(
|
||||
"""
|
||||
<video youtube="1:{youtube_id}" sub="">
|
||||
<source src="http://www.testvid.org/html5/videos/testvid.mp4"/>
|
||||
<source src="http://www.testvid2.org/html5/videos/testvid2.webm"/>
|
||||
</video>
|
||||
""".format(youtube_id=youtube_id)
|
||||
)
|
||||
modulestore().update_item(self.item, self.user.id)
|
||||
self.assert_current_subs(expected_subs='')
|
||||
|
||||
# Save new subs in the content store.
|
||||
subs = {
|
||||
'start': [100, 200, 240],
|
||||
'end': [200, 240, 380],
|
||||
'text': [
|
||||
'subs #1',
|
||||
'subs #2',
|
||||
'subs #3'
|
||||
]
|
||||
}
|
||||
self.save_subs_to_store(subs, youtube_id)
|
||||
|
||||
# Now, make request to /transcripts/save endpoint with new subs.
|
||||
data = {
|
||||
'locator': unicode(self.video_usage_key),
|
||||
'metadata': {
|
||||
'sub': youtube_id
|
||||
}
|
||||
}
|
||||
resp = self.client.get(reverse('save_transcripts'), {'data': json.dumps(data)})
|
||||
self.assertEqual(resp.status_code, 200)
|
||||
self.assertEqual(json.loads(resp.content), {"status": "Success"})
|
||||
|
||||
# Now check item.sub, it should be same as youtube id because /transcripts/save prioritize
|
||||
# youtube subs over html5 ones.
|
||||
self.assert_current_subs(expected_subs=youtube_id)
|
||||
|
||||
@@ -338,7 +338,10 @@ def manage_video_subtitles_save(item, user, old_metadata=None, generate_translat
|
||||
|
||||
# 1.
|
||||
html5_ids = get_html5_ids(item.html5_sources)
|
||||
possible_video_id_list = [item.youtube_id_1_0] + html5_ids
|
||||
|
||||
# Youtube transcript source should always have a higher priority than html5 sources. Appending
|
||||
# `youtube_id_1_0` at the end helps achieve this when we read transcripts list.
|
||||
possible_video_id_list = html5_ids + [item.youtube_id_1_0]
|
||||
sub_name = item.sub
|
||||
for video_id in possible_video_id_list:
|
||||
if not video_id:
|
||||
|
||||
Reference in New Issue
Block a user