From 0709e3c54dc042ba0c32c0858ca13e57cfa0c0fc Mon Sep 17 00:00:00 2001 From: Qubad786 Date: Wed, 7 Mar 2018 11:36:11 +0500 Subject: [PATCH] Upload transcript to basic tab, add/fix unit and acceptance tests --- .../views/tests/test_transcripts.py | 432 ++++++++---------- .../contentstore/views/transcripts_ajax.py | 140 +++--- cms/envs/bok_choy.py | 10 + .../video/transcripts/file_uploader_spec.js | 17 +- .../video/transcripts/message_manager_spec.js | 3 +- .../js/views/video/transcripts/editor.js | 8 + .../views/video/transcripts/file_uploader.js | 10 +- .../video/transcripts/message_manager.js | 3 +- .../video/transcripts/file-upload.underscore | 1 - .../xmodule/video_module/transcripts_utils.py | 3 +- .../video/test_studio_video_transcript.py | 255 ++--------- 11 files changed, 351 insertions(+), 531 deletions(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_transcripts.py b/cms/djangoapps/contentstore/views/tests/test_transcripts.py index d9fffdb51528..05c1bebe45ad 100644 --- a/cms/djangoapps/contentstore/views/tests/test_transcripts.py +++ b/cms/djangoapps/contentstore/views/tests/test_transcripts.py @@ -1,9 +1,10 @@ """Tests for items views.""" import copy +from codecs import BOM_UTF8 import ddt import json -import os +from mock import patch, Mock import tempfile import textwrap from uuid import uuid4 @@ -11,7 +12,7 @@ from django.conf import settings from django.core.urlresolvers import reverse from django.test.utils import override_settings -from mock import patch, Mock +from edxval.api import create_video from opaque_keys.edx.keys import UsageKey from contentstore.tests.utils import CourseTestCase, mock_requests_get @@ -20,11 +21,25 @@ from xmodule.contentstore.django import contentstore from xmodule.exceptions import NotFoundError from xmodule.modulestore.django import modulestore -from xmodule.video_module import transcripts_utils +from xmodule.video_module.transcripts_utils import ( + get_video_transcript_content, + remove_subs_from_store, + Transcript, +) TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE) TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'] = 'test_xcontent_%s' % uuid4().hex +SRT_TRANSCRIPT_CONTENT = """ +1 +00:00:10,500 --> 00:00:13,000 +Elephant's Dream + +2 +00:00:15,000 --> 00:00:18,000 +At the left we can see... +""" + @override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE) class BaseTranscripts(CourseTestCase): @@ -96,120 +111,175 @@ def get_youtube_ids(self): } +@ddt.ddt class TestUploadTranscripts(BaseTranscripts): """ - Tests for '/transcripts/upload' url. + Tests for '/transcripts/upload' endpoint. """ def setUp(self): - """Create initial data.""" super(TestUploadTranscripts, self).setUp() + self.contents = { + 'good': SRT_TRANSCRIPT_CONTENT, + 'bad': 'Some BAD data', + } + # Create temporary transcript files + self.good_srt_file = self.create_transcript_file(content=self.contents['good'], suffix='.srt') + self.bad_data_srt_file = self.create_transcript_file(content=self.contents['bad'], suffix='.srt') + self.bad_name_srt_file = self.create_transcript_file(content=self.contents['good'], suffix='.bad') + self.bom_srt_file = self.create_transcript_file(content=self.contents['good'], suffix='.srt', include_bom=True) + + # Setup a VEDA produced video and persist `edx_video_id` in VAL. + create_video({ + 'edx_video_id': u'123-456-789', + 'status': 'upload', + 'client_video_id': u'Test Video', + 'duration': 0, + 'encoded_videos': [], + 'courses': [unicode(self.course.id)] + }) - self.good_srt_file = tempfile.NamedTemporaryFile(suffix='.srt') - self.good_srt_file.write(textwrap.dedent(""" - 1 - 00:00:10,500 --> 00:00:13,000 - Elephant's Dream - - 2 - 00:00:15,000 --> 00:00:18,000 - At the left we can see... - """)) - self.good_srt_file.seek(0) - - self.bad_data_srt_file = tempfile.NamedTemporaryFile(suffix='.srt') - self.bad_data_srt_file.write('Some BAD data') - self.bad_data_srt_file.seek(0) - - self.bad_name_srt_file = tempfile.NamedTemporaryFile(suffix='.BAD') - self.bad_name_srt_file.write(textwrap.dedent(""" - 1 - 00:00:10,500 --> 00:00:13,000 - Elephant's Dream - - 2 - 00:00:15,000 --> 00:00:18,000 - At the left we can see... - """)) - self.bad_name_srt_file.seek(0) - - self.ufeff_srt_file = tempfile.NamedTemporaryFile(suffix='.srt') - - def test_success_video_module_source_subs_uploading(self): - self.item.data = textwrap.dedent(""" - - """) - modulestore().update_item(self.item, self.user.id) + # Add clean up handler + self.addCleanup(self.clean_temporary_transcripts) - link = reverse('upload_transcripts') - filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0] - resp = self.client.post(link, { - 'locator': self.video_usage_key, - 'transcript-file': self.good_srt_file, - 'video_list': json.dumps([{ - 'type': 'html5', - 'video': filename, - 'mode': 'mp4', - }]) - }) - self.assertEqual(resp.status_code, 200) - self.assertEqual(json.loads(resp.content).get('status'), 'Success') + def create_transcript_file(self, content, suffix, include_bom=False): + """ + Setup a transcript file with suffix and content. + """ + transcript_file = tempfile.NamedTemporaryFile(suffix=suffix) + wrapped_content = textwrap.dedent(content) + if include_bom: + wrapped_content = wrapped_content.encode('utf-8-sig') + # Verify that ufeff(BOM) character is in content. + self.assertIn(BOM_UTF8, wrapped_content) - item = modulestore().get_item(self.video_usage_key) - self.assertEqual(item.sub, filename) + transcript_file.write(wrapped_content) + transcript_file.seek(0) - content_location = StaticContent.compute_location( - self.course.id, 'subs_{0}.srt.sjson'.format(filename)) - self.assertTrue(contentstore().find(content_location)) + return transcript_file - def test_fail_data_without_id(self): - link = reverse('upload_transcripts') - resp = self.client.post(link, {'transcript-file': self.good_srt_file}) - self.assertEqual(resp.status_code, 400) - self.assertEqual(json.loads(resp.content).get('status'), 'POST data without "locator" form data.') + def clean_temporary_transcripts(self): + """ + Close transcript files gracefully. + """ + self.good_srt_file.close() + self.bad_data_srt_file.close() + self.bad_name_srt_file.close() + self.bom_srt_file.close() - def test_fail_data_without_file(self): - link = reverse('upload_transcripts') - resp = self.client.post(link, {'locator': self.video_usage_key}) - self.assertEqual(resp.status_code, 400) - self.assertEqual(json.loads(resp.content).get('status'), 'POST data without "file" form data.') + def upload_transcript(self, locator, transcript_file): + """ + Uploads a transcript for a video + """ + payload = {} + if locator: + payload.update({'locator': locator}) - def test_fail_data_with_bad_locator(self): - # Test for raising `InvalidLocationError` exception. - link = reverse('upload_transcripts') - filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0] - resp = self.client.post(link, { - 'locator': 'BAD_LOCATOR', - 'transcript-file': self.good_srt_file, - 'video_list': json.dumps([{ - 'type': 'html5', - 'video': filename, - 'mode': 'mp4', - }]) - }) - self.assertEqual(resp.status_code, 400) - self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.") + if transcript_file: + payload.update({'transcript-file': transcript_file}) - # Test for raising `ItemNotFoundError` exception. - link = reverse('upload_transcripts') - filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0] - resp = self.client.post(link, { - 'locator': '{0}_{1}'.format(self.video_usage_key, 'BAD_LOCATOR'), - 'transcript-file': self.good_srt_file, - 'video_list': json.dumps([{ - 'type': 'html5', - 'video': filename, - 'mode': 'mp4', - }]) - }) - self.assertEqual(resp.status_code, 400) - self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.") + upload_url = reverse('upload_transcripts') + response = self.client.post(upload_url, payload) - def test_fail_for_non_video_module(self): - # non_video module: setup + return response + + def assert_transcript_upload_response(self, response, expected_status_code, expected_message): + response_content = json.loads(response.content) + self.assertEqual(response.status_code, expected_status_code) + self.assertEqual(response_content['status'], expected_message) + + @ddt.data( + (u'123-456-789', False), + (u'', False), + (u'123-456-789', True) + ) + @ddt.unpack + def test_transcript_upload_success(self, edx_video_id, include_bom): + """ + Tests transcript file upload to video component works as + expected in case of following: + + 1. External video component + 2. VEDA produced video component + 3. Transcript content containing BOM character + """ + # In case of an external video component, the `edx_video_id` must be empty + # and VEDA produced video component will have `edx_video_id` set to VAL video ID. + self.item.edx_video_id = edx_video_id + modulestore().update_item(self.item, self.user.id) + + # Upload a transcript + transcript_file = self.bom_srt_file if include_bom else self.good_srt_file + response = self.upload_transcript(self.video_usage_key, transcript_file) + + # Verify the response + self.assert_transcript_upload_response(response, expected_status_code=200, expected_message='Success') + + # Verify the `edx_video_id` on the video component + json_response = json.loads(response.content) + expected_edx_video_id = edx_video_id if edx_video_id else json_response['edx_video_id'] + video = modulestore().get_item(self.video_usage_key) + self.assertEqual(video.edx_video_id, expected_edx_video_id) + + # Verify transcript content + actual_transcript = get_video_transcript_content(video.edx_video_id, language_code=u'en') + actual_sjson_content = json.loads(actual_transcript['content']) + expected_sjson_content = json.loads(Transcript.convert( + self.contents['good'], + input_format=Transcript.SRT, + output_format=Transcript.SJSON + )) + self.assertDictEqual(actual_sjson_content, expected_sjson_content) + + def test_transcript_upload_without_locator(self): + """ + Test that transcript upload validation fails if the video locator is missing + """ + response = self.upload_transcript(locator=None, transcript_file=self.good_srt_file) + self.assert_transcript_upload_response( + response, + expected_status_code=400, + expected_message=u'Video locator is required.' + ) + + def test_transcript_upload_without_file(self): + """ + Test that transcript upload validation fails if transcript file is missing + """ + response = self.upload_transcript(locator=self.video_usage_key, transcript_file=None) + self.assert_transcript_upload_response( + response, + expected_status_code=400, + expected_message=u'A transcript file is required.' + ) + + def test_transcript_upload_bad_format(self): + """ + Test that transcript upload validation fails if transcript format is not SRT + """ + response = self.upload_transcript(locator=self.video_usage_key, transcript_file=self.bad_name_srt_file) + self.assert_transcript_upload_response( + response, + expected_status_code=400, + expected_message=u'This transcript file type is not supported.' + ) + + def test_transcript_upload_bad_content(self): + """ + Test that transcript upload validation fails in case of bad transcript content. + """ + # Request to upload transcript for the video + response = self.upload_transcript(locator=self.video_usage_key, transcript_file=self.bad_data_srt_file) + self.assert_transcript_upload_response( + response, + expected_status_code=400, + expected_message=u'There is a problem with this transcript file. Try to upload a different file.' + ) + + def test_transcript_upload_unknown_category(self): + """ + Test that transcript upload validation fails if item's category is other than video. + """ + # non_video module setup - i.e. an item whose category is not 'video'. data = { 'parent_locator': unicode(self.course.location), 'category': 'non_video', @@ -221,145 +291,25 @@ def test_fail_for_non_video_module(self): item.data = '' modulestore().update_item(item, self.user.id) - # non_video module: testing - - link = reverse('upload_transcripts') - filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0] - resp = self.client.post(link, { - 'locator': unicode(usage_key), - 'transcript-file': self.good_srt_file, - 'video_list': json.dumps([{ - 'type': 'html5', - 'video': filename, - 'mode': 'mp4', - }]) - }) - self.assertEqual(resp.status_code, 400) - self.assertEqual(json.loads(resp.content).get('status'), 'Transcripts are supported only for "video" modules.') - - def test_fail_bad_xml(self): - self.item.data = '<<