From 5fdfc60957e9a9e8d3445e432cf5a30f5eb22176 Mon Sep 17 00:00:00 2001 From: cahrens Date: Tue, 26 Nov 2013 15:02:25 -0500 Subject: [PATCH] Change video transcripts to use locators instead of locations. Part of STUD-870 --- .../contentstore/tests/test_contentstore.py | 9 +- .../contentstore/tests/test_transcripts.py | 80 +++++++-------- .../contentstore/views/component.py | 10 +- .../contentstore/views/transcripts_ajax.py | 97 ++++++++++--------- .../js/spec/transcripts/file_uploader_spec.js | 2 +- .../spec/transcripts/message_manager_spec.js | 10 +- .../js/spec/transcripts/videolist_spec.js | 8 +- cms/static/js/views/transcripts/editor.js | 18 ++-- .../js/views/transcripts/file_uploader.js | 2 +- .../js/views/transcripts/message_manager.js | 10 +- .../views/transcripts/metadata_videolist.js | 6 +- cms/static/js/views/transcripts/utils.js | 6 +- .../js/transcripts/file-upload.underscore | 2 +- .../messages/transcripts-found.underscore | 2 +- .../messages/transcripts-uploaded.underscore | 2 +- cms/templates/unit.html | 4 +- 16 files changed, 130 insertions(+), 138 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 0aaf2dfb29..91dce0f6ee 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -137,8 +137,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): locator = loc_mapper().translate_location(course.location.course_id, descriptor.location, False, True) resp = self.client.get_html(locator.url_reverse('unit')) self.assertEqual(resp.status_code, 200) - # TODO: uncomment when video transcripts no longer require IDs. - # _test_no_locations(self, resp) + _test_no_locations(self, resp) for expected in expected_types: self.assertIn(expected, resp.content) @@ -1354,8 +1353,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): unit_locator = loc_mapper().translate_location(course_id, descriptor.location, False, True) resp = self.client.get_html(unit_locator.url_reverse('unit')) self.assertEqual(resp.status_code, 200) - # TODO: uncomment when video transcripts no longer require IDs. - # _test_no_locations(self, resp) + _test_no_locations(self, resp) @override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE, MODULESTORE=TEST_MODULESTORE) @@ -1682,8 +1680,7 @@ class ContentStoreTest(ModuleStoreTestCase): unit_locator = loc_mapper().translate_location(loc.course_id, unit_location, False, True) resp = self.client.get_html(unit_locator.url_reverse('unit')) self.assertEqual(resp.status_code, 200) - # TODO: uncomment when video transcripts no longer require IDs. - # _test_no_locations(self, resp) + _test_no_locations(self, resp) def delete_item(category, name): """ Helper method for testing the deletion of an xblock item. """ diff --git a/cms/djangoapps/contentstore/tests/test_transcripts.py b/cms/djangoapps/contentstore/tests/test_transcripts.py index 4c481383ab..f4c7f773ad 100644 --- a/cms/djangoapps/contentstore/tests/test_transcripts.py +++ b/cms/djangoapps/contentstore/tests/test_transcripts.py @@ -60,7 +60,7 @@ class Basetranscripts(CourseTestCase): 'type': 'video' } resp = self.client.ajax_post('/xblock', data) - self.item_location = self._get_location(resp) + self.item_locator, self.item_location = self._get_locator(resp) self.assertEqual(resp.status_code, 200) # hI10vDNYz4M - valid Youtube ID with transcripts. @@ -73,10 +73,10 @@ class Basetranscripts(CourseTestCase): # Remove all transcripts for current module. self.clear_subs_content() - def _get_location(self, resp): - """ Returns the location (as a string) from the response returned by a create operation. """ + def _get_locator(self, resp): + """ Returns the locator and old-style location (as a string) from the response returned by a create operation. """ locator = json.loads(resp.content).get('locator') - return loc_mapper().translate_locator_to_location(BlockUsageLocator(locator)).url() + return locator, loc_mapper().translate_locator_to_location(BlockUsageLocator(locator)).url() def get_youtube_ids(self): """Return youtube speeds and ids.""" @@ -142,7 +142,7 @@ class TestUploadtranscripts(Basetranscripts): link = reverse('upload_transcripts') filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0] resp = self.client.post(link, { - 'id': self.item_location, + 'locator': self.item_locator, 'file': self.good_srt_file, 'video_list': json.dumps([{ 'type': 'html5', @@ -164,20 +164,20 @@ class TestUploadtranscripts(Basetranscripts): link = reverse('upload_transcripts') resp = self.client.post(link, {'file': self.good_srt_file}) self.assertEqual(resp.status_code, 400) - self.assertEqual(json.loads(resp.content).get('status'), 'POST data without "id" form data.') + self.assertEqual(json.loads(resp.content).get('status'), 'POST data without "locator" form data.') def test_fail_data_without_file(self): link = reverse('upload_transcripts') - resp = self.client.post(link, {'id': self.item_location}) + resp = self.client.post(link, {'locator': self.item_locator}) self.assertEqual(resp.status_code, 400) self.assertEqual(json.loads(resp.content).get('status'), 'POST data without "file" form data.') - def test_fail_data_with_bad_location(self): + 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, { - 'id': 'BAD_LOCATION', + 'locator': 'BAD_LOCATOR', 'file': self.good_srt_file, 'video_list': json.dumps([{ 'type': 'html5', @@ -186,13 +186,13 @@ class TestUploadtranscripts(Basetranscripts): }]) }) self.assertEqual(resp.status_code, 400) - self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by location.") + self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.") # 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, { - 'id': '{0}_{1}'.format(self.item_location, 'BAD_LOCATION'), + 'locator': '{0}_{1}'.format(self.item_locator, 'BAD_LOCATOR'), 'file': self.good_srt_file, 'video_list': json.dumps([{ 'type': 'html5', @@ -201,7 +201,7 @@ class TestUploadtranscripts(Basetranscripts): }]) }) self.assertEqual(resp.status_code, 400) - self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by location.") + self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.") def test_fail_for_non_video_module(self): # non_video module: setup @@ -211,7 +211,7 @@ class TestUploadtranscripts(Basetranscripts): 'type': 'non_video' } resp = self.client.ajax_post('/xblock', data) - item_location = self._get_location(resp) + item_locator, item_location = self._get_locator(resp) data = '' modulestore().update_item(item_location, data) @@ -220,7 +220,7 @@ class TestUploadtranscripts(Basetranscripts): link = reverse('upload_transcripts') filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0] resp = self.client.post(link, { - 'id': item_location, + 'locator': item_locator, 'file': self.good_srt_file, 'video_list': json.dumps([{ 'type': 'html5', @@ -238,7 +238,7 @@ class TestUploadtranscripts(Basetranscripts): link = reverse('upload_transcripts') filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0] resp = self.client.post(link, { - 'id': self.item_location, + 'locator': self.item_locator, 'file': self.good_srt_file, 'video_list': json.dumps([{ 'type': 'html5', @@ -255,7 +255,7 @@ class TestUploadtranscripts(Basetranscripts): link = reverse('upload_transcripts') filename = os.path.splitext(os.path.basename(self.bad_data_srt_file.name))[0] resp = self.client.post(link, { - 'id': self.item_location, + 'locator': self.item_locator, 'file': self.bad_data_srt_file, 'video_list': json.dumps([{ 'type': 'html5', @@ -270,7 +270,7 @@ class TestUploadtranscripts(Basetranscripts): link = reverse('upload_transcripts') filename = os.path.splitext(os.path.basename(self.bad_name_srt_file.name))[0] resp = self.client.post(link, { - 'id': self.item_location, + 'locator': self.item_locator, 'file': self.bad_name_srt_file, 'video_list': json.dumps([{ 'type': 'html5', @@ -297,7 +297,7 @@ class TestUploadtranscripts(Basetranscripts): link = reverse('upload_transcripts') filename = os.path.splitext(os.path.basename(srt_file.name))[0] resp = self.client.post(link, { - 'id': self.item_location, + 'locator': self.item_locator, 'file': srt_file, 'video_list': json.dumps([{ 'type': 'html5', @@ -359,7 +359,7 @@ class TestDownloadtranscripts(Basetranscripts): self.save_subs_to_store(subs, 'JMD_ifUUfsU') link = reverse('download_transcripts') - resp = self.client.get(link, {'id': self.item_location, 'subs_id': "JMD_ifUUfsU"}) + resp = self.client.get(link, {'locator': self.item_locator, 'subs_id': "JMD_ifUUfsU"}) self.assertEqual(resp.status_code, 200) self.assertEqual(resp.content, """0\n00:00:00,100 --> 00:00:00,200\nsubs #1\n\n1\n00:00:00,200 --> 00:00:00,240\nsubs #2\n\n2\n00:00:00,240 --> 00:00:00,380\nsubs #3\n\n""") @@ -386,7 +386,7 @@ class TestDownloadtranscripts(Basetranscripts): self.save_subs_to_store(subs, subs_id) link = reverse('download_transcripts') - resp = self.client.get(link, {'id': self.item_location, 'subs_id': subs_id}) + resp = self.client.get(link, {'locator': self.item_locator, 'subs_id': subs_id}) self.assertEqual(resp.status_code, 200) self.assertEqual( resp.content, @@ -397,21 +397,21 @@ class TestDownloadtranscripts(Basetranscripts): def test_fail_data_without_file(self): link = reverse('download_transcripts') - resp = self.client.get(link, {'id': ''}) + resp = self.client.get(link, {'locator': ''}) self.assertEqual(resp.status_code, 404) resp = self.client.get(link, {}) self.assertEqual(resp.status_code, 404) - def test_fail_data_with_bad_location(self): + def test_fail_data_with_bad_locator(self): # Test for raising `InvalidLocationError` exception. link = reverse('download_transcripts') - resp = self.client.get(link, {'id': 'BAD_LOCATION'}) + resp = self.client.get(link, {'locator': 'BAD_LOCATOR'}) self.assertEqual(resp.status_code, 404) # Test for raising `ItemNotFoundError` exception. link = reverse('download_transcripts') - resp = self.client.get(link, {'id': '{0}_{1}'.format(self.item_location, 'BAD_LOCATION')}) + resp = self.client.get(link, {'locator': '{0}_{1}'.format(self.item_locator, 'BAD_LOCATOR')}) self.assertEqual(resp.status_code, 404) def test_fail_for_non_video_module(self): @@ -422,7 +422,7 @@ class TestDownloadtranscripts(Basetranscripts): 'type': 'videoalpha' } resp = self.client.ajax_post('/xblock', data) - item_location = self._get_location(resp) + item_locator, item_location = self._get_locator(resp) subs_id = str(uuid4()) data = textwrap.dedent(""" @@ -445,7 +445,7 @@ class TestDownloadtranscripts(Basetranscripts): self.save_subs_to_store(subs, subs_id) link = reverse('download_transcripts') - resp = self.client.get(link, {'id': item_location}) + resp = self.client.get(link, {'locator': item_locator}) self.assertEqual(resp.status_code, 404) def test_fail_nonyoutube_subs_dont_exist(self): @@ -459,7 +459,7 @@ class TestDownloadtranscripts(Basetranscripts): modulestore().update_item(self.item_location, data) link = reverse('download_transcripts') - resp = self.client.get(link, {'id': self.item_location}) + resp = self.client.get(link, {'locator': self.item_locator}) self.assertEqual(resp.status_code, 404) def test_empty_youtube_attr_and_sub_attr(self): @@ -473,7 +473,7 @@ class TestDownloadtranscripts(Basetranscripts): modulestore().update_item(self.item_location, data) link = reverse('download_transcripts') - resp = self.client.get(link, {'id': self.item_location}) + resp = self.client.get(link, {'locator': self.item_locator}) self.assertEqual(resp.status_code, 404) @@ -498,7 +498,7 @@ class TestDownloadtranscripts(Basetranscripts): self.save_subs_to_store(subs, 'JMD_ifUUfsU') link = reverse('download_transcripts') - resp = self.client.get(link, {'id': self.item_location}) + resp = self.client.get(link, {'locator': self.item_locator}) self.assertEqual(resp.status_code, 404) @@ -553,7 +553,7 @@ class TestChecktranscripts(Basetranscripts): self.save_subs_to_store(subs, subs_id) data = { - 'id': self.item_location, + 'locator': self.item_locator, 'videos': [{ 'type': 'html5', 'video': subs_id, @@ -597,7 +597,7 @@ class TestChecktranscripts(Basetranscripts): self.save_subs_to_store(subs, 'JMD_ifUUfsU') link = reverse('check_transcripts') data = { - 'id': self.item_location, + 'locator': self.item_locator, 'videos': [{ 'type': 'youtube', 'video': 'JMD_ifUUfsU', @@ -625,7 +625,7 @@ class TestChecktranscripts(Basetranscripts): def test_fail_data_without_id(self): link = reverse('check_transcripts') data = { - 'id': '', + 'locator': '', 'videos': [{ 'type': '', 'video': '', @@ -634,13 +634,13 @@ 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'), "Can't find item by location.") + self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.") - def test_fail_data_with_bad_location(self): + def test_fail_data_with_bad_locator(self): # Test for raising `InvalidLocationError` exception. link = reverse('check_transcripts') data = { - 'id': '', + 'locator': '', 'videos': [{ 'type': '', 'video': '', @@ -649,11 +649,11 @@ 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'), "Can't find item by location.") + self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.") # Test for raising `ItemNotFoundError` exception. data = { - 'id': '{0}_{1}'.format(self.item_location, 'BAD_LOCATION'), + 'locator': '{0}_{1}'.format(self.item_locator, 'BAD_LOCATOR'), 'videos': [{ 'type': '', 'video': '', @@ -662,7 +662,7 @@ 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'), "Can't find item by location.") + self.assertEqual(json.loads(resp.content).get('status'), "Can't find item by locator.") def test_fail_for_non_video_module(self): # Not video module: setup @@ -672,7 +672,7 @@ class TestChecktranscripts(Basetranscripts): 'type': 'not_video' } resp = self.client.ajax_post('/xblock', data) - item_location = self._get_location(resp) + item_locator, item_location = self._get_locator(resp) subs_id = str(uuid4()) data = textwrap.dedent(""" @@ -695,7 +695,7 @@ class TestChecktranscripts(Basetranscripts): self.save_subs_to_store(subs, subs_id) data = { - 'id': item_location, + 'locator': item_locator, 'videos': [{ 'type': '', 'video': '', diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 3742c7af20..ce20e004ee 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -223,13 +223,9 @@ def unit_handler(request, tag=None, course_id=None, branch=None, version_guid=No ) components = [ - [ - # TODO: old location needed for video transcripts. - component.location.url(), - loc_mapper().translate_location( - course.location.course_id, component.location, False, True - ) - ] + loc_mapper().translate_location( + course.location.course_id, component.location, False, True + ) for component in item.get_children() ] diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index 9db2cb9c13..9d5cdf8e80 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -18,11 +18,12 @@ from django.conf import settings from xmodule.contentstore.content import StaticContent from xmodule.exceptions import NotFoundError -from xmodule.modulestore.django import modulestore +from xmodule.modulestore.django import modulestore, loc_mapper from xmodule.contentstore.django import contentstore -from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError +from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError from util.json_request import JsonResponse +from xmodule.modulestore.locator import BlockUsageLocator from ..transcripts_utils import ( generate_subs_from_source, @@ -77,20 +78,14 @@ def upload_transcripts(request): 'subs': '', } - item_location = request.POST.get('id') - if not item_location: - return error_response(response, 'POST data without "id" form data.') + locator = request.POST.get('locator') + if not locator: + return error_response(response, 'POST data without "locator" form data.') - # This is placed before has_access() to validate item_location, - # because has_access() raises InvalidLocationError if location is invalid. try: - item = modulestore().get_item(item_location) - except (ItemNotFoundError, InvalidLocationError): - return error_response(response, "Can't find item by location.") - - # Check permissions for this user within this course. - if not has_access(request.user, item_location): - raise PermissionDenied() + item = _get_item(request, request.POST) + except (ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError): + return error_response(response, "Can't find item by locator.") if 'file' not in request.FILES: return error_response(response, 'POST data without "file" form data.') @@ -156,23 +151,17 @@ def download_transcripts(request): Raises Http404 if unsuccessful. """ - item_location = request.GET.get('id') - if not item_location: - log.debug('GET data without "id" property.') + locator = request.GET.get('locator') + if not locator: + log.debug('GET data without "locator" property.') raise Http404 - # This is placed before has_access() to validate item_location, - # because has_access() raises InvalidLocationError if location is invalid. try: - item = modulestore().get_item(item_location) - except (ItemNotFoundError, InvalidLocationError): - log.debug("Can't find item by location.") + item = _get_item(request, request.GET) + except (ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError): + log.debug("Can't find item by locator.") raise Http404 - # Check permissions for this user within this course. - if not has_access(request.user, item_location): - raise PermissionDenied() - subs_id = request.GET.get('subs_id') if not subs_id: log.debug('GET data without "subs_id" property.') @@ -240,7 +229,7 @@ def check_transcripts(request): 'status': 'Error', } try: - __, videos, item = validate_transcripts_data(request) + __, videos, item = _validate_transcripts_data(request) except TranscriptsRequestValidationException as e: return error_response(transcripts_presence, e.message) @@ -303,7 +292,7 @@ def check_transcripts(request): if len(html5_subs) == 2: # check html5 transcripts for equality transcripts_presence['html5_equal'] = json.loads(html5_subs[0]) == json.loads(html5_subs[1]) - command, subs_to_use = transcripts_logic(transcripts_presence, videos) + command, subs_to_use = _transcripts_logic(transcripts_presence, videos) transcripts_presence.update({ 'command': command, 'subs': subs_to_use, @@ -311,7 +300,7 @@ def check_transcripts(request): return JsonResponse(transcripts_presence) -def transcripts_logic(transcripts_presence, videos): +def _transcripts_logic(transcripts_presence, videos): """ By `transcripts_presence` content, figure what show to user: @@ -386,7 +375,7 @@ def choose_transcripts(request): } try: - data, videos, item = validate_transcripts_data(request) + data, videos, item = _validate_transcripts_data(request) except TranscriptsRequestValidationException as e: return error_response(response, e.message) @@ -416,7 +405,7 @@ def replace_transcripts(request): response = {'status': 'Error', 'subs': ''} try: - __, videos, item = validate_transcripts_data(request) + __, videos, item = _validate_transcripts_data(request) except TranscriptsRequestValidationException as e: return error_response(response, e.message) @@ -435,7 +424,7 @@ def replace_transcripts(request): return JsonResponse(response) -def validate_transcripts_data(request): +def _validate_transcripts_data(request): """ Validates, that request contains all proper data for transcripts processing. @@ -452,18 +441,10 @@ def validate_transcripts_data(request): if not data: raise TranscriptsRequestValidationException('Incoming video data is empty.') - item_location = data.get('id') - - # This is placed before has_access() to validate item_location, - # because has_access() raises InvalidLocationError if location is invalid. try: - item = modulestore().get_item(item_location) - except (ItemNotFoundError, InvalidLocationError): - raise TranscriptsRequestValidationException("Can't find item by location.") - - # Check permissions for this user within this course. - if not has_access(request.user, item_location): - raise PermissionDenied() + item = _get_item(request, data) + except (ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError): + raise TranscriptsRequestValidationException("Can't find item by locator.") if item.category != 'video': raise TranscriptsRequestValidationException('Transcripts are supported only for "video" modules.') @@ -492,7 +473,7 @@ def rename_transcripts(request): response = {'status': 'Error', 'subs': ''} try: - __, videos, item = validate_transcripts_data(request) + __, videos, item = _validate_transcripts_data(request) except TranscriptsRequestValidationException as e: return error_response(response, e.message) @@ -525,11 +506,10 @@ def save_transcripts(request): if not data: return error_response(response, 'Incoming video data is empty.') - item_location = data.get('id') try: - item = modulestore().get_item(item_location) - except (ItemNotFoundError, InvalidLocationError): - return error_response(response, "Can't find item by location.") + item = _get_item(request, data) + except (ItemNotFoundError, InvalidLocationError, InsufficientSpecificationError): + return error_response(response, "Can't find item by locator.") metadata = data.get('metadata') if metadata is not None: @@ -553,3 +533,24 @@ def save_transcripts(request): response['status'] = 'Success' return JsonResponse(response) + + +def _get_item(request, data): + """ + Obtains from 'data' the locator for an item. + Next, gets that item from the modulestore (allowing any errors to raise up). + Finally, verifies that the user has access to the item. + + Returns the item. + """ + locator = BlockUsageLocator(data.get('locator')) + old_location = loc_mapper().translate_locator_to_location(locator) + + # This is placed before has_access() to validate the location, + # because has_access() raises InvalidLocationError if location is invalid. + item = modulestore().get_item(old_location) + + if not has_access(request.user, locator): + raise PermissionDenied() + + return item diff --git a/cms/static/js/spec/transcripts/file_uploader_spec.js b/cms/static/js/spec/transcripts/file_uploader_spec.js index c896ad811a..20b653527c 100644 --- a/cms/static/js/spec/transcripts/file_uploader_spec.js +++ b/cms/static/js/spec/transcripts/file_uploader_spec.js @@ -48,7 +48,7 @@ function ($, _, Utils, FileUploader) { el: $container, messenger: messenger, videoListObject: videoListObject, - component_id: 'component_id' + component_locator: 'component_locator' }); }); diff --git a/cms/static/js/spec/transcripts/message_manager_spec.js b/cms/static/js/spec/transcripts/message_manager_spec.js index c6d3bc7909..40eef7f02e 100644 --- a/cms/static/js/spec/transcripts/message_manager_spec.js +++ b/cms/static/js/spec/transcripts/message_manager_spec.js @@ -52,7 +52,7 @@ function ($, _, Utils, MessageManager, FileUploader, sinon) { view = new MessageManager({ el: $container, parent: videoList, - component_id: 'component_id' + component_locator: 'component_locator' }); }); @@ -60,7 +60,7 @@ function ($, _, Utils, MessageManager, FileUploader, sinon) { expect(fileUploader.initialize).toHaveBeenCalledWith({ el: view.$el, messenger: view, - component_id: view.component_id, + component_locator: view.component_locator, videoListObject: view.options.parent }); }); @@ -215,7 +215,7 @@ function ($, _, Utils, MessageManager, FileUploader, sinon) { function() { expect(Utils.command).toHaveBeenCalledWith( action, - view.component_id, + view.component_locator, videoList, void(0) ); @@ -245,7 +245,7 @@ function ($, _, Utils, MessageManager, FileUploader, sinon) { function () { expect(Utils.command).toHaveBeenCalledWith( action, - view.component_id, + view.component_locator, videoList, { html5_id: extraParamas @@ -268,7 +268,7 @@ function ($, _, Utils, MessageManager, FileUploader, sinon) { function () { expect(Utils.command).toHaveBeenCalledWith( action, - view.component_id, + view.component_locator, videoList, void(0) ); diff --git a/cms/static/js/spec/transcripts/videolist_spec.js b/cms/static/js/spec/transcripts/videolist_spec.js index cf189c2549..de850eb3ee 100644 --- a/cms/static/js/spec/transcripts/videolist_spec.js +++ b/cms/static/js/spec/transcripts/videolist_spec.js @@ -11,7 +11,7 @@ function ($, _, Utils, VideoList, MetadataView, MetadataModel, AbstractEditor, s 'transcripts/metadata-videolist-entry.underscore' ), abstractEditor = AbstractEditor.prototype, - component_id = 'component_id', + component_locator = 'component_locator', videoList = [ { mode: "youtube", @@ -62,7 +62,7 @@ function ($, _, Utils, VideoList, MetadataView, MetadataModel, AbstractEditor, s var tpl = sandbox({ 'class': 'component', - 'data-id': component_id + 'data-locator': component_locator }), model = new MetadataModel(modelStub), videoList, $el; @@ -157,7 +157,7 @@ function ($, _, Utils, VideoList, MetadataView, MetadataModel, AbstractEditor, s waitsForResponse(function () { expect(abstractEditor.initialize).toHaveBeenCalled(); expect(messenger.initialize).toHaveBeenCalled(); - expect(view.component_id).toBe(component_id); + expect(view.component_locator).toBe(component_locator); expect(view.$el).toHandle('input'); }); }); @@ -167,7 +167,7 @@ function ($, _, Utils, VideoList, MetadataView, MetadataModel, AbstractEditor, s expect(abstractEditor.render).toHaveBeenCalled(); expect(Utils.command).toHaveBeenCalledWith( 'check', - component_id, + component_locator, videoList ); diff --git a/cms/static/js/views/transcripts/editor.js b/cms/static/js/views/transcripts/editor.js index 21e48a03b0..78280a0d66 100644 --- a/cms/static/js/views/transcripts/editor.js +++ b/cms/static/js/views/transcripts/editor.js @@ -72,7 +72,7 @@ function($, Backbone, _, Utils, MetadataView, MetadataCollection) { syncBasicTab: function (metadataCollection, metadataView) { var result = [], getField = Utils.getField, - component_id = this.$el.closest('.component').data('id'), + component_locator = this.$el.closest('.component').data('locator'), subs = getField(metadataCollection, 'sub'), values = {}, videoUrl, metadata, modifiedValues; @@ -99,7 +99,7 @@ function($, Backbone, _, Utils, MetadataView, MetadataCollection) { if (isSubsModified) { metadata = $.extend(true, {}, modifiedValues); // Save module state - Utils.command('save', component_id, null, { + Utils.command('save', component_locator, null, { metadata: metadata, current_subs: _.pluck( Utils.getVideoList(videoUrl.getDisplayValue()), @@ -110,18 +110,16 @@ function($, Backbone, _, Utils, MetadataView, MetadataCollection) { // Get values from `Advanced` tab fields (`html5_sources`, // `youtube_id_1_0`) that should be synchronized. - html5Sources = getField(metadataCollection, 'html5_sources') -                                    .getDisplayValue(); + var html5Sources = getField(metadataCollection, 'html5_sources').getDisplayValue(); -            values.youtube = getField(metadataCollection, 'youtube_id_1_0') -                                    .getDisplayValue(); + values.youtube = getField(metadataCollection, 'youtube_id_1_0').getDisplayValue(); -            values.html5Sources = _.filter(html5Sources, function (value) { -                var link = Utils.parseLink(value), + values.html5Sources = _.filter(html5Sources, function (value) { + var link = Utils.parseLink(value), mode = link && link.mode; -                return mode === 'html5' && mode; -            }); + return mode === 'html5' && mode; + }); // The length of youtube video_id should be 11 characters. diff --git a/cms/static/js/views/transcripts/file_uploader.js b/cms/static/js/views/transcripts/file_uploader.js index c9c33820e4..d307c7c85e 100644 --- a/cms/static/js/views/transcripts/file_uploader.js +++ b/cms/static/js/views/transcripts/file_uploader.js @@ -39,7 +39,7 @@ function($, Backbone, _, Utils) { tplContainer.html(this.template({ ext: this.validFileExtensions, - component_id: this.options.component_id, + component_locator: this.options.component_locator, video_list: videoList })); diff --git a/cms/static/js/views/transcripts/message_manager.js b/cms/static/js/views/transcripts/message_manager.js index 80f70752cf..506f3143ac 100644 --- a/cms/static/js/views/transcripts/message_manager.js +++ b/cms/static/js/views/transcripts/message_manager.js @@ -31,12 +31,12 @@ function($, Backbone, _, Utils, FileUploader, gettext) { initialize: function () { _.bindAll(this); - this.component_id = this.$el.closest('.component').data('id'); + this.component_locator = this.$el.closest('.component').data('locator'); this.fileUploader = new FileUploader({ el: this.$el, messenger: this, - component_id: this.component_id, + component_locator: this.component_locator, videoListObject: this.options.parent }); }, @@ -76,7 +76,7 @@ function($, Backbone, _, Utils, FileUploader, gettext) { this.$el.find('.transcripts-status') .removeClass('is-invisible') .find(this.elClass).html(template({ - component_id: encodeURIComponent(this.component_id), + component_locator: encodeURIComponent(this.component_locator), html5_list: html5List, grouped_list: groupedList, subs_id: (params) ? params.subs: '' @@ -204,7 +204,7 @@ function($, Backbone, _, Utils, FileUploader, gettext) { */ processCommand: function (action, errorMessage, videoId) { var self = this, - component_id = this.component_id, + component_locator = this.component_locator, videoList = this.options.parent.getVideoObjectsList(), extraParam, xhr; @@ -212,7 +212,7 @@ function($, Backbone, _, Utils, FileUploader, gettext) { extraParam = { html5_id: videoId }; } - xhr = Utils.command(action, component_id, videoList, extraParam) + xhr = Utils.command(action, component_locator, videoList, extraParam) .done(function (resp) { var sub = resp.subs; diff --git a/cms/static/js/views/transcripts/metadata_videolist.js b/cms/static/js/views/transcripts/metadata_videolist.js index 6e7e82232b..7e7dac7f19 100644 --- a/cms/static/js/views/transcripts/metadata_videolist.js +++ b/cms/static/js/views/transcripts/metadata_videolist.js @@ -46,7 +46,7 @@ function($, Backbone, _, AbstractEditor, Utils, MessageManager, MetadataView) { _.debounce(_.bind(this.inputHandler, this), this.inputDelay) ); - this.component_id = this.$el.closest('.component').data('id'); + this.component_locator = this.$el.closest('.component').data('locator'); }, render: function () { @@ -55,7 +55,7 @@ function($, Backbone, _, AbstractEditor, Utils, MessageManager, MetadataView) { .apply(this, arguments); var self = this, - component_id = this.$el.closest('.component').data('id'), + component_locator = this.$el.closest('.component').data('locator'), videoList = this.getVideoObjectsList(), showServerError = function (response) { @@ -82,7 +82,7 @@ function($, Backbone, _, AbstractEditor, Utils, MessageManager, MetadataView) { } // Check current state of Timed Transcripts. - Utils.command('check', component_id, videoList) + Utils.command('check', component_locator, videoList) .done(function (resp) { var params = resp, len = videoList.length, diff --git a/cms/static/js/views/transcripts/utils.js b/cms/static/js/views/transcripts/utils.js index 7965d0b9e3..a6865906e5 100644 --- a/cms/static/js/views/transcripts/utils.js +++ b/cms/static/js/views/transcripts/utils.js @@ -295,7 +295,7 @@ define(["jquery", "underscore", "jquery.ajaxQueue"], function($, _) { * * @param {string} action Action that will be invoked on server. Is a part * of url. - * @param {string} component_id Id of component. + * @param {string} component_locator the locator of component. * @param {array} videoList List of object with information about inserted * urls. * @param {object} extraParams Extra parameters that can be send to the @@ -314,7 +314,7 @@ define(["jquery", "underscore", "jquery.ajaxQueue"], function($, _) { // _command() function. var xhr = null; - return function (action, component_id, videoList, extraParams) { + return function (action, component_locator, videoList, extraParams) { var params, data; if (extraParams) { @@ -326,7 +326,7 @@ define(["jquery", "underscore", "jquery.ajaxQueue"], function($, _) { } data = $.extend( - { id: component_id }, + { locator: component_locator }, { videos: videoList }, params ); diff --git a/cms/templates/js/transcripts/file-upload.underscore b/cms/templates/js/transcripts/file-upload.underscore index 90dfb80db6..d48f9af887 100644 --- a/cms/templates/js/transcripts/file-upload.underscore +++ b/cms/templates/js/transcripts/file-upload.underscore @@ -5,6 +5,6 @@ method="post" enctype="multipart/form-data"> - + diff --git a/cms/templates/js/transcripts/messages/transcripts-found.underscore b/cms/templates/js/transcripts/messages/transcripts-found.underscore index 6a12edcd3f..286063ed6a 100644 --- a/cms/templates/js/transcripts/messages/transcripts-found.underscore +++ b/cms/templates/js/transcripts/messages/transcripts-found.underscore @@ -10,7 +10,7 @@ - "> + "> <%= gettext("Download to Edit") %> diff --git a/cms/templates/js/transcripts/messages/transcripts-uploaded.underscore b/cms/templates/js/transcripts/messages/transcripts-uploaded.underscore index fa328208fd..c17d83f710 100644 --- a/cms/templates/js/transcripts/messages/transcripts-uploaded.underscore +++ b/cms/templates/js/transcripts/messages/transcripts-uploaded.underscore @@ -10,7 +10,7 @@ - "> + "> <%= gettext("Download to Edit") %> diff --git a/cms/templates/unit.html b/cms/templates/unit.html index 9f3daf0b20..2318e9cd97 100644 --- a/cms/templates/unit.html +++ b/cms/templates/unit.html @@ -48,8 +48,8 @@ require(["domReady!", "jquery", "js/models/module_info", "coffee/src/views/unit"

    - % for id, locator in components: -
  1. + % for locator in components: +
  2. % endfor