From c0d86360f08ee37a4ed6e326e85e4238acf0c369 Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Tue, 9 Jan 2018 20:04:48 +0500 Subject: [PATCH] Improve trancript sorting - EDUCATOR-2030 --- .../contentstore/views/tests/test_videos.py | 13 ++---- cms/djangoapps/contentstore/views/videos.py | 15 ++++--- .../js/spec/views/video_transcripts_spec.js | 45 +++++++++++-------- cms/static/js/views/video_transcripts.js | 16 +------ cms/templates/js/video-transcripts.underscore | 10 ++--- 5 files changed, 46 insertions(+), 53 deletions(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_videos.py b/cms/djangoapps/contentstore/views/tests/test_videos.py index c158b5fb8f..b9c0abbfc9 100644 --- a/cms/djangoapps/contentstore/views/tests/test_videos.py +++ b/cms/djangoapps/contentstore/views/tests/test_videos.py @@ -239,7 +239,7 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase): False, ['edx_video_id', 'client_video_id', 'created', 'duration', 'status', 'course_video_image_url'], [], - {} + [] ), ( True, @@ -254,9 +254,7 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase): 'provider': 'Cielo24' } ], - { - 'en': 'English' - } + ['en'] ), ( True, @@ -278,10 +276,7 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase): 'provider': 'Cielo24' } ], - { - 'en': 'English', - 'es': 'Spanish' - } + ['en', 'es'] ) ) @ddt.unpack @@ -312,7 +307,7 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase): for response_video in response_videos: self.assertEqual(set(response_video.keys()), set(expected_video_keys)) if response_video['edx_video_id'] == self.previous_uploads[0]['edx_video_id']: - self.assertDictEqual(response_video.get('transcripts', {}), expected_transcripts) + self.assertEqual(response_video.get('transcripts', []), expected_transcripts) def test_get_html(self): response = self.client.get(self.url) diff --git a/cms/djangoapps/contentstore/views/videos.py b/cms/djangoapps/contentstore/views/videos.py index 8458f61ce2..e8ae989c99 100644 --- a/cms/djangoapps/contentstore/views/videos.py +++ b/cms/djangoapps/contentstore/views/videos.py @@ -539,11 +539,7 @@ def _get_videos(course): video["status"] = convert_video_status(video) if is_video_transcript_enabled: - all_languages = get_all_transcript_languages() - video['transcripts'] = { - lang_code: all_languages[lang_code] - for lang_code in get_available_transcript_languages([video['edx_video_id']]) - } + video['transcripts'] = get_available_transcript_languages([video['edx_video_id']]) return videos @@ -598,8 +594,15 @@ def get_all_transcript_languages(): third_party_transcription_languages.update(cielo_fidelity['PREMIUM']['languages']) third_party_transcription_languages.update(cielo_fidelity['PROFESSIONAL']['languages']) + all_languages_dict = dict(settings.ALL_LANGUAGES, **third_party_transcription_languages) # Return combined system settings and 3rd party transcript languages. - return dict(settings.ALL_LANGUAGES, **third_party_transcription_languages) + all_languages = [] + for key, value in sorted(all_languages_dict.iteritems(), key=lambda (k, v): (v, k)): + all_languages.append({ + 'language_code': key, + 'language_text': value + }) + return all_languages def videos_index_html(course): diff --git a/cms/static/js/spec/views/video_transcripts_spec.js b/cms/static/js/spec/views/video_transcripts_spec.js index bd5766ac73..36184e842e 100644 --- a/cms/static/js/spec/views/video_transcripts_spec.js +++ b/cms/static/js/spec/views/video_transcripts_spec.js @@ -10,20 +10,27 @@ define( verifyMessage, verifyDetailedErrorMessage, createFakeTranscriptFile, - transcripts = { - en: 'English', - es: 'Spanish', - ur: 'Urdu' - }, + transcripts = ['en', 'es', 'ur'], edxVideoID = 'test-edx-video-id', clientVideoID = 'Video client title name.mp4', - transcriptAvailableLanguages = { - en: 'English', - es: 'Spanish', - cn: 'Chinese', - ar: 'Arabic', - ur: 'Urdu' - }, + transcriptAvailableLanguages = [ + { + language_code: 'en', + language_text: 'English' + }, + { + language_code: 'es', + language_text: 'Spanish' + }, + { + language_code: 'ar', + language_text: 'Chinese' + }, + { + language_code: 'ur', + language_text: 'Urdu' + } + ], TRANSCRIPT_DOWNLOAD_FILE_FORMAT = 'srt', TRANSCRIPT_DOWNLOAD_URL = 'abc.com/transcript_download/course_id', TRANSCRIPT_UPLOAD_URL = 'abc.com/transcript_upload/course_id', @@ -146,7 +153,7 @@ define( videoTranscriptsView.$el.find('.show-video-transcripts-wrapper').hasClass('hidden') ).toEqual(true); expect(videoTranscriptsView.$el.find('.toggle-show-transcripts-button-text').html().trim()).toEqual( - 'Show transcripts (' + _.size(transcripts) + ')' + 'Show transcripts (' + transcripts.length + ')' ); }); @@ -158,7 +165,7 @@ define( // Verify initial button text expect(videoTranscriptsView.$el.find('.toggle-show-transcripts-button-text').html().trim()).toEqual( - 'Show transcripts (' + _.size(transcripts) + ')' + 'Show transcripts (' + transcripts.length + ')' ); videoTranscriptsView.$el.find('.toggle-show-transcripts-button').click(); @@ -169,7 +176,7 @@ define( // Verify button text is changed. expect(videoTranscriptsView.$el.find('.toggle-show-transcripts-button-text').html().trim()).toEqual( - 'Hide transcripts (' + _.size(transcripts) + ')' + 'Hide transcripts (' + transcripts.length + ')' ); }); @@ -179,7 +186,7 @@ define( // Verify button text. expect(videoTranscriptsView.$el.find('.toggle-show-transcripts-button-text').html().trim()).toEqual( - 'Hide transcripts (' + _.size(transcripts) + ')' + 'Hide transcripts (' + transcripts.length + ')' ); // Verify transcript container is not hidden @@ -191,7 +198,7 @@ define( // Verify button text is changed. expect(videoTranscriptsView.$el.find('.toggle-show-transcripts-button-text').html().trim()).toEqual( - 'Show transcripts (' + _.size(transcripts) + ')' + 'Show transcripts (' + transcripts.length + ')' ); // Verify transcript container is hidden @@ -215,10 +222,10 @@ define( // Show transcripts videoTranscriptsView.$el.find('.toggle-show-transcripts-button').click(); expect(videoTranscriptsView.$el.find('.show-video-transcript-content').length).toEqual( - _.size(transcripts) + transcripts.length ); - _.each(transcripts, function(langaugeText, languageCode) { + _.each(transcripts, function(languageCode) { $transcriptEl = videoTranscriptsView.$el.find('.show-video-transcript-content[data-language-code="' + languageCode + '"]'); // eslint-disable-line max-len // Verify correct transcript title is set. expect($transcriptEl.find('.transcript-title').html()).toEqual( diff --git a/cms/static/js/views/video_transcripts.js b/cms/static/js/views/video_transcripts.js index 9b155f5f21..c034ae224e 100644 --- a/cms/static/js/views/video_transcripts.js +++ b/cms/static/js/views/video_transcripts.js @@ -59,18 +59,6 @@ define( ); }, - /* - Sorts object by value and returns a sorted array. - */ - sortByValue: function(itemObject) { - var sortedArray = []; - _.each(itemObject, function(value, key) { - // Push each JSON Object entry in array by [value, key] - sortedArray.push([value, key]); - }); - return sortedArray.sort(); - }, - /* Returns transcript title. */ @@ -113,7 +101,7 @@ define( gettext('{toggleShowTranscriptText} transcripts ({totalTranscripts})'), { toggleShowTranscriptText: $transcriptsWrapperEl.hasClass('hidden') ? gettext('Show') : gettext('Hide'), // eslint-disable-line max-len - totalTranscripts: _.size(this.transcripts) + totalTranscripts: this.transcripts.length } ) ); @@ -275,7 +263,7 @@ define( this.$el, this.template({ transcripts: this.transcripts, - transcriptAvailableLanguages: this.sortByValue(this.transcriptAvailableLanguages), + transcriptAvailableLanguages: this.transcriptAvailableLanguages, edxVideoID: this.edxVideoID, transcriptClientTitle: this.getTranscriptClientTitle(), transcriptFileFormat: this.videoTranscriptSettings.trancript_download_file_format, diff --git a/cms/templates/js/video-transcripts.underscore b/cms/templates/js/video-transcripts.underscore index 6f27464505..8c389c91cc 100644 --- a/cms/templates/js/video-transcripts.underscore +++ b/cms/templates/js/video-transcripts.underscore @@ -1,10 +1,10 @@
-<% if (_.size(transcripts)) { %> +<% if (transcripts.length) { %> @@ -12,7 +12,7 @@ <%- gettext('No transcript uploaded.') %> <% }%>