From e89c3dd3ee138ba39496a799907645c21150f5cd Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Thu, 26 Oct 2017 18:27:06 +0500 Subject: [PATCH] Add cancel button, handle errors, change label texts - EDUCATOR-1584 --- .../spec/views/course_video_settings_spec.js | 89 ++++++++++++++----- cms/static/js/views/course_video_settings.js | 46 ++++++---- cms/static/sass/views/_video-upload.scss | 7 +- ...s-update-org-credentials-footer.underscore | 10 ++- ...settings-update-settings-footer.underscore | 4 + ...video-transcript-provider-empty.underscore | 2 +- ...script-organization-credentials.underscore | 4 +- 7 files changed, 114 insertions(+), 48 deletions(-) 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 d1139ab1bd..be7575ee33 100644 --- a/cms/static/js/spec/views/course_video_settings_spec.js +++ b/cms/static/js/spec/views/course_video_settings_spec.js @@ -24,6 +24,7 @@ define( submitOrganizationCredentials, transcriptPreferencesUrl = '/transcript_preferences/course-v1:edX+DemoX+Demo_Course', transcriptCredentialsHandlerUrl = '/transcript_credentials/course-v1:edX+DemoX+Demo_Course', + INTERNAL_SERVER_ERROR = 'An error has occurred. Wait a few minutes, and then try again.', activeTranscriptPreferences = { provider: 'Cielo24', cielo24_fidelity: 'PROFESSIONAL', @@ -93,7 +94,7 @@ define( none: { key: 'none', value: '', - displayName: 'N/A' + displayName: 'None' }, Cielo24: { key: 'Cielo24', @@ -158,7 +159,7 @@ define( verifyProviderList = function(selectedProvider) { var $transcriptProvidersListEl = $courseVideoSettingsEl.find('.transcript-provider-wrapper .transcript-provider-group'); // eslint-disable-line max-len - // Check N/A provider is selected. + // Check None provider is selected. expect($transcriptProvidersListEl.find('input[type=radio]:checked').val()).toEqual(selectedProvider.value); // eslint-disable-line max-len _.each(providers, function(provider, key) { $transcriptProvidersListEl.find('label[for=transcript-provider-' + key + ']').val(provider.displayName); // eslint-disable-line max-len @@ -270,7 +271,7 @@ define( AjaxHelpers.respondWithJson(requests, {}); }; - submitOrganizationCredentials = function(fieldValues, errorMessage) { + submitOrganizationCredentials = function(fieldValues, statusCode, errorMessage) { var requests = AjaxHelpers.requests(this); // Click change button to render organization credentials view. $courseVideoSettingsEl.find('.action-change-provider').click(); @@ -295,11 +296,12 @@ define( ) ); - if (errorMessage) { - // Send error response. - AjaxHelpers.respondWithError(requests, 400, { - error: errorMessage - }); + if (statusCode === 400) { + // Send bad request error response. + AjaxHelpers.respondWithError(requests, statusCode, {error: errorMessage}); + } else if (statusCode === 500) { + // Send internal server error response. + AjaxHelpers.respondWithError(requests, statusCode); } else { // Send empty response. AjaxHelpers.respondWithJson(requests, {}); @@ -359,6 +361,40 @@ define( verifyTranscriptPreferences(); }); + it('resets to active preferences when clicked on cancel', function() { + var selectedProvider = '3PlayMedia'; + + renderCourseVideoSettingsView( + activeTranscriptPreferences, + transcriptionPlans, + transcriptOrganizationCredentials + ); + + // First check preferance are selected correctly in HTML. + verifyTranscriptPreferences(); + expect(courseVideoSettingsView.selectedProvider, providers.Cielo24); + + // Now change preferences. + // Select provider. + changeProvider(selectedProvider); + expect(courseVideoSettingsView.selectedProvider, selectedProvider); + + // Select turnaround. + selectPreference( + '.transcript-turnaround', + transcriptionPlans[selectedProvider].turnaround.default + ); + expect( + courseVideoSettingsView.selectedTurnaroundPlan, + transcriptionPlans[selectedProvider].turnaround.default + ); + + // Now click cancel button and verify active preferences are shown. + $courseVideoSettingsEl.find('.action-cancel-course-video-settings').click(); + verifyTranscriptPreferences(); + expect(courseVideoSettingsView.selectedProvider, providers.Cielo24); + }); + it('shows video source language directly in case of 3Play provider', function() { var sourceLanguages, selectedProvider = '3PlayMedia'; @@ -505,12 +541,12 @@ define( }); it('removes transcript settings on update settings button click when no provider is selected', function() { - // Reset to N/A provider + // Reset to None provider resetProvider(); verifyProviderList(providers.none); // Verify that success message is shown. - verifyMessage('success', 'Settings updated'); + verifyMessage('success', 'Automatic transcripts are disabled.'); }); it('shows error message if server sends error', function() { @@ -574,10 +610,10 @@ define( expect($courseVideoSettingsEl.find('.transcript-provider-wrapper .transcript-provider-group')).not.toExist(); // eslint-disable-line max-len }); - it('does not show transcript preferences or organization credentials if N/A provider is saved', function() { + it('does not show transcript preferences or organization credentials if None provider is saved', function() { // eslint-disable-line max-len renderCourseVideoSettingsView(null, transcriptionPlans); - // Check N/A provider + // Check None provider resetProvider(); verifyProviderList(providers.none); @@ -585,10 +621,10 @@ define( expect($courseVideoSettingsEl.find('.transcript-provider-wrapper .selected-transcript-provider')).not.toExist(); // eslint-disable-line max-len }); - it('does not show transcript preferences or organization credentials if N/A provider is checked', function() { // eslint-disable-line max-len + it('does not show transcript preferences or organization credentials if None provider is checked', function() { // eslint-disable-line max-len renderCourseVideoSettingsView(null, transcriptionPlans); - // Check N/A provider + // Check None provider resetProvider(); verifyProviderList(providers.none); @@ -677,8 +713,8 @@ define( expect($courseVideoSettingsEl.find('.transcription-account-details.warning')).toExist(); // Verify message expect($courseVideoSettingsEl.find('.transcription-account-details').html()).toEqual( - 'By entering the set of credntials below, ' + - 'you will be overwriting your organization\'s credentials.' + 'This action updates the ' + courseVideoSettingsView.selectedProvider + + ' information for your entire organization.' ); }); @@ -691,8 +727,7 @@ define( expect($courseVideoSettingsEl.find('.transcription-account-details.warning')).not.toExist(); // Initial detail message is shown instead. expect($courseVideoSettingsEl.find('.transcription-account-details').html()).toEqual( - 'Please enter your organization\'s account information. ' + - 'Remember that all courses in your organization will use this account.' + 'Enter the account information for your organization.' ); }); @@ -738,16 +773,28 @@ define( verifyCredentialsSaved(); }); + it('shows error message on saving organization credentials if server sends bad request error', function() { + verifyProviderSelectedView(); + + submitOrganizationCredentials({ + api_key: 'api-key', + username: 'username' + }, 400, 'Error saving credentials.'); + + // Verify that bad request error message is shown. + verifyMessage('error', 'Error saving credentials.'); + }); + it('shows error message on saving organization credentials if server sends error', function() { verifyProviderSelectedView(); submitOrganizationCredentials({ api_key: 'api-key', username: 'username' - }, 'Error saving credentials'); + }, 500); - // Verify that error message is shown. - verifyMessage('error', 'Error saving credentials'); + // Verify that server error message is shown. + verifyMessage('error', INTERNAL_SERVER_ERROR); }); // TODO: Add more tests like clicking on add language, remove and their scenarios and some other tests diff --git a/cms/static/js/views/course_video_settings.js b/cms/static/js/views/course_video_settings.js index 6fe054b4bb..2126c92717 100644 --- a/cms/static/js/views/course_video_settings.js +++ b/cms/static/js/views/course_video_settings.js @@ -21,7 +21,8 @@ function($, Backbone, _, gettext, moment, ViewUtils, HtmlUtils, StringUtils, Tra var CourseVideoSettingsView, CIELO24 = 'Cielo24', - THREE_PLAY_MEDIA = '3PlayMedia'; + THREE_PLAY_MEDIA = '3PlayMedia', + INTERNAL_SERVER_ERROR_MESSAGE = gettext('An error has occurred. Wait a few minutes, and then try again.'); CourseVideoSettingsView = Backbone.View.extend({ el: 'div.video-transcript-settings-wrapper', @@ -36,6 +37,7 @@ function($, Backbone, _, gettext, moment, ViewUtils, HtmlUtils, StringUtils, Tra 'click .action-change-provider': 'renderOrganizationCredentials', 'click .action-update-org-credentials': 'updateOrganizationCredentials', 'click .action-update-course-video-settings': 'updateCourseVideoSettings', + 'click .action-cancel-course-video-settings': 'discardChanges', 'click .action-close-course-video-settings': 'closeCourseVideoSettings' }, @@ -177,13 +179,17 @@ function($, Backbone, _, gettext, moment, ViewUtils, HtmlUtils, StringUtils, Tra }, providerSelected: function(event) { + this.resetPlanData(); + this.selectedProvider = event.target.value; + // Re-render view + this.reRenderView(); + }, + + reRenderView: function() { var $courseVideoSettingsContentEl = this.$el.find('.course-video-settings-content'), dateModified = this.activeTranscriptionPlan ? moment.utc(this.activeTranscriptionPlan.modified).format('ll') : ''; - this.resetPlanData(); - this.selectedProvider = event.target.value; - if (!this.selectedProvider) { // Hide organization credentials and transcript preferences views $courseVideoSettingsContentEl.hide(); @@ -254,7 +260,7 @@ function($, Backbone, _, gettext, moment, ViewUtils, HtmlUtils, StringUtils, Tra { key: 'none', value: '', - name: gettext('N/A'), + name: gettext('None'), checked: this.selectedProvider === '' ? 'checked' : '' }, { @@ -504,9 +510,8 @@ function($, Backbone, _, gettext, moment, ViewUtils, HtmlUtils, StringUtils, Tra // 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'); + } catch (e) {} // eslint-disable-line no-empty + this.renderResponseStatus(errorMessage || INTERNAL_SERVER_ERROR_MESSAGE, 'error'); }, renderResponseStatus: function(responseText, type) { @@ -663,20 +668,20 @@ function($, Backbone, _, gettext, moment, ViewUtils, HtmlUtils, StringUtils, Tra responseTranscriptPreferences = data ? data.transcript_preferences : null; self.updateSuccessResponseStatus(responseTranscriptPreferences); }).fail(function(jqXHR) { - if (jqXHR.responseText) { - self.updateFailResponseStatus(jqXHR.responseText); - } + self.updateFailResponseStatus(jqXHR.responseText); }); } else { $.ajax({ type: 'DELETE', url: self.transcriptHandlerUrl }).done(function() { - self.updateSuccessResponseStatus(null); + responseTranscriptPreferences = null; + self.updateSuccessResponseStatus( + responseTranscriptPreferences, + gettext('Automatic transcripts are disabled.') + ); }).fail(function(jqXHR) { - if (jqXHR.responseText) { - self.updateFailResponseStatus(jqXHR.responseText); - } + self.updateFailResponseStatus(jqXHR.responseText); }); } }, @@ -716,9 +721,7 @@ function($, Backbone, _, gettext, moment, ViewUtils, HtmlUtils, StringUtils, Tra ) ); }).fail(function(jqXHR) { - if (jqXHR.responseText) { - self.updateFailResponseStatus(jqXHR.responseText); - } + self.updateFailResponseStatus(jqXHR.responseText); }); }, @@ -737,6 +740,13 @@ function($, Backbone, _, gettext, moment, ViewUtils, HtmlUtils, StringUtils, Tra } }, + discardChanges: function() { + this.setActiveTranscriptPlanData(); + // Re-render views + this.renderProviders(); + this.reRenderView(); + }, + renderOrganizationCredentials: function() { var $courseVideoSettingsContentEl = this.$el.find('.course-video-settings-content'); diff --git a/cms/static/sass/views/_video-upload.scss b/cms/static/sass/views/_video-upload.scss index 2fbe813633..3dae1a8d55 100644 --- a/cms/static/sass/views/_video-upload.scss +++ b/cms/static/sass/views/_video-upload.scss @@ -189,15 +189,16 @@ padding: ($baseline/2); } - .action-update-org-credentials { - margin-top: ($baseline*1.6); + .action-cancel-course-video-settings { + @include margin-right($baseline/2); } .course-video-settings-footer { margin-top: ($baseline*1.6); .last-updated-text { @include font-size(12); - @include margin-left($baseline/4); + display: block; + margin-top: ($baseline/2); } } diff --git a/cms/templates/js/course-video-settings-update-org-credentials-footer.underscore b/cms/templates/js/course-video-settings-update-org-credentials-footer.underscore index 4df4aa605c..0d93a55ca2 100644 --- a/cms/templates/js/course-video-settings-update-org-credentials-footer.underscore +++ b/cms/templates/js/course-video-settings-update-org-credentials-footer.underscore @@ -1,4 +1,8 @@ - + diff --git a/cms/templates/js/course-video-settings-update-settings-footer.underscore b/cms/templates/js/course-video-settings-update-settings-footer.underscore index 7445b8b8c1..8dfb25de0d 100644 --- a/cms/templates/js/course-video-settings-update-settings-footer.underscore +++ b/cms/templates/js/course-video-settings-update-settings-footer.underscore @@ -1,3 +1,7 @@ +