diff --git a/cms/djangoapps/contentstore/views/tests/test_videos.py b/cms/djangoapps/contentstore/views/tests/test_videos.py index 0e0cb51042..c3e3d3b2ca 100644 --- a/cms/djangoapps/contentstore/views/tests/test_videos.py +++ b/cms/djangoapps/contentstore/views/tests/test_videos.py @@ -33,6 +33,7 @@ from contentstore.views.videos import KEY_EXPIRATION_IN_SECONDS, StatusDisplaySt from xmodule.modulestore.tests.factories import CourseFactory from openedx.core.djangoapps.profile_images.tests.helpers import make_image_file +from edxval.api import create_or_update_transcript_preferences, get_transcript_preferences def override_switch(switch, active): @@ -871,6 +872,18 @@ class TranscriptPreferencesTestCase(VideoUploadTestBase, CourseTestCase): {}, 'Invalid provider.' ), + ( + { + 'provider': '' + }, + 'Invalid provider.' + ), + ( + { + 'provider': 'dummy-provider' + }, + 'Invalid provider.' + ), ( { 'provider': TranscriptProvider.CIELO24 @@ -970,6 +983,46 @@ class TranscriptPreferencesTestCase(VideoUploadTestBase, CourseTestCase): self.assertEqual(status_code, 200) self.assertTrue(response['transcript_preferences'], preferences_data) + def test_remove_transcript_preferences(self): + """ + Test that transcript handler removes transcript preferences correctly. + """ + # First add course wide transcript preferences. + preferences = create_or_update_transcript_preferences(unicode(self.course.id)) + + # Verify transcript preferences exist + self.assertIsNotNone(preferences) + + response = self.client.delete( + self.get_url_for_course_key(self.course.id), + content_type='application/json' + ) + + self.assertEqual(response.status_code, 204) + + # Verify transcript preferences no loger exist + preferences = get_transcript_preferences(unicode(self.course.id)) + self.assertIsNone(preferences) + + def test_remove_transcript_preferences_not_found(self): + """ + Test that transcript handler works correctly even when no preferences are found. + """ + course_id = 'dummy+course+id' + # Verify transcript preferences do not exist + preferences = get_transcript_preferences(course_id) + self.assertIsNone(preferences) + + response = self.client.delete( + self.get_url_for_course_key(course_id), + content_type='application/json' + ) + self.assertEqual(response.status_code, 204) + + # Verify transcript preferences do not exist + preferences = get_transcript_preferences(course_id) + self.assertIsNone(preferences) + @ddt.data( None, { diff --git a/cms/djangoapps/contentstore/views/videos.py b/cms/djangoapps/contentstore/views/videos.py index aa652ee17e..609af3ebe8 100644 --- a/cms/djangoapps/contentstore/views/videos.py +++ b/cms/djangoapps/contentstore/views/videos.py @@ -30,6 +30,7 @@ from edxval.api import ( get_3rd_party_transcription_plans, get_transcript_preferences, create_or_update_transcript_preferences, + remove_transcript_preferences, ) from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace @@ -327,7 +328,7 @@ def validate_transcript_preferences( @expect_json @login_required -@require_POST +@require_http_methods(('POST', 'DELETE')) def transcript_preferences_handler(request, course_key_string): """ JSON view handler to post the transcript preferences. @@ -338,26 +339,25 @@ def transcript_preferences_handler(request, course_key_string): Returns: valid json response or 400 with error message """ - data = request.json - provider = data.get('provider', '') - - # TODO: if provider == '': delete course preferences - # i.e call delete api end point like delete_transcript_preferences(course_key_string) - - error, preferences = validate_transcript_preferences( - provider=provider, - cielo24_fidelity=data.get('cielo24_fidelity', ''), - cielo24_turnaround=data.get('cielo24_turnaround', ''), - three_play_turnaround=data.get('three_play_turnaround', ''), - preferred_languages=data.get('preferred_languages', []) - ) - - if error: - response = JsonResponse({'error': error}, status=400) - else: - preferences.update({'provider': provider}) - transcript_preferences = create_or_update_transcript_preferences(course_key_string, **preferences) - response = JsonResponse({'transcript_preferences': transcript_preferences}, status=200) + if request.method == 'POST': + data = request.json + provider = data.get('provider') + error, preferences = validate_transcript_preferences( + provider=provider, + cielo24_fidelity=data.get('cielo24_fidelity', ''), + cielo24_turnaround=data.get('cielo24_turnaround', ''), + three_play_turnaround=data.get('three_play_turnaround', ''), + preferred_languages=data.get('preferred_languages', []) + ) + if error: + response = JsonResponse({'error': error}, status=400) + else: + preferences.update({'provider': provider}) + transcript_preferences = create_or_update_transcript_preferences(course_key_string, **preferences) + response = JsonResponse({'transcript_preferences': transcript_preferences}, status=200) + elif request.method == 'DELETE': + remove_transcript_preferences(course_key_string) + response = JsonResponse() return response diff --git a/cms/static/js/spec/views/course_video_settings_spec.js b/cms/static/js/spec/views/course_video_settings_spec.js index 50948e3db3..ac99fc2885 100644 --- a/cms/static/js/spec/views/course_video_settings_spec.js +++ b/cms/static/js/spec/views/course_video_settings_spec.js @@ -186,7 +186,7 @@ define( }) ); - // Send successful upload response. + // Send successful response. AjaxHelpers.respondWithJson(requests, { transcript_preferences: activeTranscriptPreferences }); @@ -200,6 +200,31 @@ define( ); }); + it('removes transcript settings on update settings button click when no provider is selected', function() { + var requests = AjaxHelpers.requests(this); + + // Set no provider selected + courseVideoSettingsView.selectedProvider = null; + $courseVideoSettingsEl.find('.action-update-course-video-settings').click(); + + AjaxHelpers.expectRequest( + requests, + 'DELETE', + transcriptPreferencesUrl + ); + + // Send successful empty content response. + AjaxHelpers.respondWithJson(requests, {}); + + // Verify that success message is shown. + expect($courseVideoSettingsEl.find('.course-video-settings-message-wrapper.success').html()).toEqual( + '
' + + '' + + 'Settings updated' + + '
' + ); + }); + it('shows error message if server sends error', function() { var requests = AjaxHelpers.requests(this); $courseVideoSettingsEl.find('.action-update-course-video-settings').click(); diff --git a/cms/static/js/views/course_video_settings.js b/cms/static/js/views/course_video_settings.js index 0c61eabd3e..7b5e6914ac 100644 --- a/cms/static/js/views/course_video_settings.js +++ b/cms/static/js/views/course_video_settings.js @@ -336,6 +336,43 @@ function($, Backbone, _, gettext, moment, HtmlUtils, StringUtils, TranscriptSett ); }, + updateSuccessResponseStatus: function(data) { + this.renderResponseStatus(gettext('Settings updated'), 'success'); + // Sync ActiveUploadListView with latest active plan. + this.activeTranscriptionPlan = data; + Backbone.trigger('coursevideosettings:syncActiveTranscriptPreferences', this.activeTranscriptionPlan); + }, + + updateFailResponseStatus: function(data) { + var errorMessage; + // Enclose inside try-catch so that if we get erroneous data, we could still + // show some error to user + try { + errorMessage = $.parseJSON(data).error; + } catch (e) {} // eslint-disable-line no-empty + errorMessage = errorMessage || gettext('Error saving data'); + this.renderResponseStatus(errorMessage, 'error'); + }, + + renderResponseStatus: function(responseText, type) { + var addClass = type === 'error' ? 'error' : 'success', + removeClass = type === 'error' ? 'success' : 'error', + iconClass = type === 'error' ? 'fa-info-circle' : 'fa-check-circle', + $messageWrapperEl = this.$el.find('.course-video-settings-message-wrapper'); + $messageWrapperEl.removeClass(removeClass); + $messageWrapperEl.addClass(addClass); + HtmlUtils.setHtml( + $messageWrapperEl, + HtmlUtils.interpolateHtml( + HtmlUtils.HTML('
{text}
'), // eslint-disable-line max-len + { + text: responseText, + iconClass: iconClass + } + ) + ); + }, + clearResponseStatus: function() { // Remove parent level state. var $messageWrapperEl = this.$el.find('.course-video-settings-message-wrapper'); @@ -407,59 +444,38 @@ function($, Backbone, _, gettext, moment, HtmlUtils, StringUtils, TranscriptSett saveTranscriptPreferences: function() { var self = this, - $messageWrapperEl = self.$el.find('.course-video-settings-message-wrapper'); - + responseTranscriptPreferences; // First clear response status if present already this.clearResponseStatus(); - $.postJSON(this.transcriptHandlerUrl, { - provider: self.selectedProvider, - cielo24_fidelity: self.selectedFidelityPlan, - cielo24_turnaround: self.selectedProvider === CIELO24 ? self.selectedTurnaroundPlan : '', - three_play_turnaround: self.selectedProvider === THREE_PLAY_MEDIA ? self.selectedTurnaroundPlan : '', - preferred_languages: self.selectedLanguages, - global: false // Do not trigger global AJAX error handler - }, function(data) { - if (data.transcript_preferences) { - $messageWrapperEl.removeClass('error'); - $messageWrapperEl.addClass('success'); - HtmlUtils.setHtml( - $messageWrapperEl, - HtmlUtils.interpolateHtml( - HtmlUtils.HTML('
{text}
'), // eslint-disable-line max-len - { - text: gettext('Settings updated') - } - ) - ); - self.activeTranscriptionPlan = data.transcript_preferences; - - // Sync ActiveUploadListView with latest active plan. - Backbone.trigger( - 'coursevideosettings:syncActiveTranscriptPreferences', - self.activeTranscriptionPlan - ); - } - }).fail(function(jqXHR) { - var errorMessage; - if (jqXHR.responseText) { - // Enclose inside try-catch so that if we get erroneous data, we could still show some error to user - try { - errorMessage = $.parseJSON(jqXHR.responseText).error; - } catch (e) {} // eslint-disable-line no-empty - $messageWrapperEl.removeClass('success'); - $messageWrapperEl.addClass('error'); - HtmlUtils.setHtml( - $messageWrapperEl, - HtmlUtils.interpolateHtml( - HtmlUtils.HTML('
{text}
'), // eslint-disable-line max-len - { - text: errorMessage || gettext('Error saving data') - } - ) - ); - } - }); + if (self.selectedProvider) { + $.postJSON(self.transcriptHandlerUrl, { + provider: self.selectedProvider, + cielo24_fidelity: self.selectedFidelityPlan, + cielo24_turnaround: self.selectedProvider === CIELO24 ? self.selectedTurnaroundPlan : '', + three_play_turnaround: self.selectedProvider === THREE_PLAY_MEDIA ? self.selectedTurnaroundPlan : '', // eslint-disable-line max-len + preferred_languages: self.selectedLanguages, + global: false // Do not trigger global AJAX error handler + }, function(data) { + responseTranscriptPreferences = data ? data.transcript_preferences : null; + self.updateSuccessResponseStatus(responseTranscriptPreferences); + }).fail(function(jqXHR) { + if (jqXHR.responseText) { + self.updateFailResponseStatus(jqXHR.responseText); + } + }); + } else { + $.ajax({ + type: 'DELETE', + url: self.transcriptHandlerUrl + }).done(function() { + self.updateSuccessResponseStatus(null); + }).fail(function(jqXHR) { + if (jqXHR.responseText) { + self.updateFailResponseStatus(jqXHR.responseText); + } + }); + } }, updateCourseVideoSettings: function() { @@ -489,35 +505,29 @@ function($, Backbone, _, gettext, moment, HtmlUtils, StringUtils, TranscriptSett }, setFixedCourseVideoSettingsPane: function() { - var windowWidth = $(window).width(), - windowHeight = $(window).height(), - $courseVideoSettingsButton = $('.course-video-settings-button'), + var $courseVideoSettingsButton = $('.course-video-settings-button'), $courseVideoSettingsContainer = this.$el.find('.course-video-settings-container'), initialPositionTop = $courseVideoSettingsContainer.offset().top, - courseVideoSettingsButtonLeft = $courseVideoSettingsButton.offset().left, - fixedOffsetRight = windowWidth - - courseVideoSettingsButtonLeft - $courseVideoSettingsButton.width() - 25; + // Button right position = width of window - button left position - button width - paddings - border. + $courseVideoSettingsButtonRight = $(window).width() - + $courseVideoSettingsButton.offset().left - + $courseVideoSettingsButton.width() - + $courseVideoSettingsButton.css('padding-left').replace('px', '') - + $courseVideoSettingsButton.css('padding-right').replace('px', '') - + $courseVideoSettingsButton.css('border-width').replace('px', '') - 5; // Extra pixles for slack; - // set windows total height - $courseVideoSettingsContainer.css('height', windowHeight); - $courseVideoSettingsContainer.css('right', 20); + // Set to windows total height + $courseVideoSettingsContainer.css('height', $(window).height()); + // Start settings pane adjascent to 'course video settings' button. + $courseVideoSettingsContainer.css('right', $courseVideoSettingsButtonRight); // Make sticky when scroll reaches top. $(window).scroll(function() { - // Remove transition when we start scrolling. - // Why we do this? The settings pane come back and forth when it is switched between - // position:fixed and position:absolute, it's right and top position are then being changed wrt to their - // position layout. - $courseVideoSettingsContainer.css('transition', 'none'); - if ($(window).scrollTop() >= initialPositionTop) { $courseVideoSettingsContainer.addClass('fixed-container'); - // TODO: Removes these js calculations and try to do through CSS way. - $courseVideoSettingsContainer.css('right', fixedOffsetRight); } else { $courseVideoSettingsContainer.removeClass('fixed-container'); - $courseVideoSettingsContainer.css('right', 20); } }); },