diff --git a/cms/djangoapps/contentstore/features/advanced_settings.py b/cms/djangoapps/contentstore/features/advanced_settings.py index 315da8f72c..76c4a3f2e0 100644 --- a/cms/djangoapps/contentstore/features/advanced_settings.py +++ b/cms/djangoapps/contentstore/features/advanced_settings.py @@ -82,8 +82,8 @@ def it_is_formatted(step): @step('I get an error on save$') def error_on_save(step): assert_regexp_matches( - world.css_text('#notification-error-description'), - "Incorrect format for field '{}'.".format(DISPLAY_NAME_KEY) + world.css_text('.error-item-message'), + "Value stored in a .* must be .*, found .*" ) diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index 9e312b4998..38e2fbb27f 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -458,6 +458,69 @@ class CourseMetadataEditingTest(CourseTestCase): self.assertIn('showanswer', test_model, 'showanswer field ') self.assertIn('xqa_key', test_model, 'xqa_key field ') + def test_validate_and_update_from_json_correct_inputs(self): + is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json( + self.course, + { + "advertised_start": {"value": "start A"}, + "days_early_for_beta": {"value": 2}, + "advanced_modules": {"value": ['combinedopenended']}, + }, + user=self.user + ) + self.assertTrue(is_valid) + self.assertTrue(len(errors) == 0) + self.update_check(test_model) + + # fresh fetch to ensure persistence + fresh = modulestore().get_course(self.course.id) + test_model = CourseMetadata.fetch(fresh) + self.update_check(test_model) + + # Tab gets tested in test_advanced_settings_munge_tabs + self.assertIn('advanced_modules', test_model, 'Missing advanced_modules') + self.assertEqual(test_model['advanced_modules']['value'], ['combinedopenended'], 'advanced_module is not updated') + + def test_validate_and_update_from_json_wrong_inputs(self): + # input incorrectly formatted data + is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json( + self.course, + { + "advertised_start": {"value": 1, "display_name": "Course Advertised Start Date", }, + "days_early_for_beta": {"value": "supposed to be an integer", + "display_name": "Days Early for Beta Users", }, + "advanced_modules": {"value": 1, "display_name": "Advanced Module List", }, + }, + user=self.user + ) + + # Check valid results from validate_and_update_from_json + self.assertFalse(is_valid) + self.assertEqual(len(errors), 3) + self.assertFalse(test_model) + + error_keys = set([error_obj['model']['display_name'] for error_obj in errors]) + test_keys = set(['Advanced Module List', 'Course Advertised Start Date', 'Days Early for Beta Users']) + self.assertEqual(error_keys, test_keys) + + # try fresh fetch to ensure no update happened + fresh = modulestore().get_course(self.course.id) + test_model = CourseMetadata.fetch(fresh) + + self.assertNotEqual(test_model['advertised_start']['value'], 1, 'advertised_start should not be updated to a wrong value') + self.assertNotEqual(test_model['days_early_for_beta']['value'], "supposed to be an integer", + 'days_early_for beta should not be updated to a wrong value') + + def test_correct_http_status(self): + json_data = json.dumps({ + "advertised_start": {"value": 1, "display_name": "Course Advertised Start Date", }, + "days_early_for_beta": {"value": "supposed to be an integer", + "display_name": "Days Early for Beta Users", }, + "advanced_modules": {"value": 1, "display_name": "Advanced Module List", }, + }) + response = self.client.ajax_post(self.course_setting_url, json_data) + self.assertEqual(400, response.status_code) + def test_update_from_json(self): test_model = CourseMetadata.update_from_json( self.course, @@ -487,6 +550,9 @@ class CourseMetadataEditingTest(CourseTestCase): self.assertEqual(test_model['advertised_start']['value'], 'start B', "advertised_start not expected value") def update_check(self, test_model): + """ + checks that updates were made + """ self.assertIn('display_name', test_model, 'Missing editable metadata field') self.assertEqual(test_model['display_name']['value'], 'Robot Super Course', "not expected value") self.assertIn('advertised_start', test_model, 'Missing new advertised_start metadata field') diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 48b91eb95e..eab477925f 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -13,7 +13,7 @@ from django.views.decorators.http import require_http_methods from django.core.exceptions import PermissionDenied from django.core.urlresolvers import reverse from django.http import HttpResponseBadRequest, HttpResponseNotFound, HttpResponse -from util.json_request import JsonResponse +from util.json_request import JsonResponse, JsonResponseBadRequest from util.date_utils import get_default_time_display from edxmako.shortcuts import render_to_response @@ -834,18 +834,26 @@ def _config_course_advanced_components(request, course_module): component_types = tab_component_map.get(tab_type) found_ac_type = False for ac_type in component_types: - if ac_type in request.json[ADVANCED_COMPONENT_POLICY_KEY]["value"] and ac_type in ADVANCED_COMPONENT_TYPES: - # Add tab to the course if needed - changed, new_tabs = add_extra_panel_tab(tab_type, course_module) - # If a tab has been added to the course, then send the - # metadata along to CourseMetadata.update_from_json - if changed: - course_module.tabs = new_tabs - request.json.update({'tabs': {'value': new_tabs}}) - # Indicate that tabs should not be filtered out of - # the metadata - filter_tabs = False # Set this flag to avoid the tab removal code below. - found_ac_type = True # break + + # Check if the user has incorrectly failed to put the value in an iterable. + new_advanced_component_list = request.json[ADVANCED_COMPONENT_POLICY_KEY]['value'] + if hasattr(new_advanced_component_list, '__iter__'): + if ac_type in new_advanced_component_list and ac_type in ADVANCED_COMPONENT_TYPES: + + # Add tab to the course if needed + changed, new_tabs = add_extra_panel_tab(tab_type, course_module) + # If a tab has been added to the course, then send the + # metadata along to CourseMetadata.update_from_json + if changed: + course_module.tabs = new_tabs + request.json.update({'tabs': {'value': new_tabs}}) + # Indicate that tabs should not be filtered out of + # the metadata + filter_tabs = False # Set this flag to avoid the tab removal code below. + found_ac_type = True # break + else: + # If not iterable, return immediately and let validation handle. + return # If we did not find a module type in the advanced settings, # we may need to remove the tab from the course. @@ -891,12 +899,21 @@ def advanced_settings_handler(request, course_key_string): try: # Whether or not to filter the tabs key out of the settings metadata filter_tabs = _config_course_advanced_components(request, course_module) - return JsonResponse(CourseMetadata.update_from_json( + + # validate data formats and update + is_valid, errors, updated_data = CourseMetadata.validate_and_update_from_json( course_module, request.json, filter_tabs=filter_tabs, user=request.user, - )) + ) + + if is_valid: + return JsonResponse(updated_data) + else: + return JsonResponseBadRequest(errors) + + # Handle all errors that validation doesn't catch except (TypeError, ValueError) as err: return HttpResponseBadRequest( django.utils.html.escape(err.message), diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 09d87a2b21..27d5e9faa9 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -82,10 +82,55 @@ class CourseMetadata(object): raise ValueError(_("Incorrect format for field '{name}'. {detailed_message}".format( name=model['display_name'], detailed_message=err.message))) + return cls.update_from_dict(key_values, descriptor, user) + + @classmethod + def validate_and_update_from_json(cls, descriptor, jsondict, user, filter_tabs=True): + """ + Validate the values in the json dict (validated by xblock fields from_json method) + + If all fields validate, go ahead and update those values in the database. + If not, return the error objects list. + + Returns: + did_validate: whether values pass validation or not + errors: list of error objects + result: the updated course metadata or None if error + """ + + filtered_list = list(cls.FILTERED_LIST) + if not filter_tabs: + filtered_list.remove("tabs") + filtered_dict = dict((k, v) for k, v in jsondict.iteritems() if k not in filtered_list) + did_validate = True + errors = [] + key_values = {} + updated_data = None + + for key, model in filtered_dict.iteritems(): + try: + val = model['value'] + if hasattr(descriptor, key) and getattr(descriptor, key) != val: + key_values[key] = descriptor.fields[key].from_json(val) + except (TypeError, ValueError) as err: + did_validate = False + errors.append({'message': err.message, 'model': model}) + + # If did validate, go ahead and update the metadata + if did_validate: + updated_data = cls.update_from_dict(key_values, descriptor, user) + + return did_validate, errors, updated_data + + @classmethod + def update_from_dict(cls, key_values, descriptor, user): + """ + Update metadata descriptor in modulestore from key_values. + """ for key, value in key_values.iteritems(): setattr(descriptor, key, value) - if len(key_values) > 0: + if len(key_values): modulestore().update_item(descriptor, user.id) return cls.fetch(descriptor) diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index 9bd64c5a2a..5ad1e24b36 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -238,6 +238,7 @@ define([ "js/spec/views/modals/base_modal_spec", "js/spec/views/modals/edit_xblock_spec", + "js/spec/views/modals/validation_error_modal_spec", "js/spec/xblock/cms.runtime.v1_spec", diff --git a/cms/static/js/spec/views/modals/validation_error_modal_spec.js b/cms/static/js/spec/views/modals/validation_error_modal_spec.js new file mode 100644 index 0000000000..bc23303bce --- /dev/null +++ b/cms/static/js/spec/views/modals/validation_error_modal_spec.js @@ -0,0 +1,86 @@ +define(['jquery', 'underscore', 'js/spec_helpers/validation_helpers', 'js/views/modals/validation_error_modal'], + function ($, _, validation_helpers, ValidationErrorModal) { + + describe('ValidationErrorModal', function() { + var modal, showModal; + + showModal = function (jsonContent, callback) { + modal = new ValidationErrorModal(); + modal.setResetCallback(callback); + modal.setContent(jsonContent); + modal.show(); + }; + + /* Before each, install templates required for the base modal + and validation error modal. */ + beforeEach(function () { + validation_helpers.installValidationTemplates(); + }); + + afterEach(function() { + validation_helpers.hideModalIfShowing(modal); + }); + + it('is visible after show is called', function () { + showModal([]); + expect(validation_helpers.isShowingModal(modal)).toBeTruthy(); + }); + + it('displays none if no error given', function () { + var errorObjects = []; + + showModal(errorObjects); + expect(validation_helpers.isShowingModal(modal)).toBeTruthy(); + validation_helpers.checkErrorContents(modal, errorObjects); + }); + + it('correctly displays json error message objects', function () { + var errorObjects = [ + { + model: {display_name: 'test_attribute1'}, + message: 'Encountered an error while saving test_attribute1' + }, + { + model: {display_name: 'test_attribute2'}, + message: 'Encountered an error while saving test_attribute2' + } + ]; + + showModal(errorObjects); + expect(validation_helpers.isShowingModal(modal)).toBeTruthy(); + validation_helpers.checkErrorContents(modal, errorObjects); + }); + + it('run callback when undo changes button is clicked', function () { + var errorObjects = [ + { + model: {display_name: 'test_attribute1'}, + message: 'Encountered an error while saving test_attribute1' + }, + { + model: {display_name: 'test_attribute2'}, + message: 'Encountered an error while saving test_attribute2' + } + ]; + + var callback = function() { + return true; + }; + + // Show Modal and click undo changes + showModal(errorObjects, callback); + expect(validation_helpers.isShowingModal(modal)).toBeTruthy(); + validation_helpers.undoChanges(modal); + + // Wait for the callback to be fired + waitsFor(function () { + return callback(); + }, 'the callback to be called', 5000); + + // After checking callback fire, check modal hide + runs(function () { + expect(validation_helpers.isShowingModal(modal)).toBe(false); + }); + }); + }); + }); diff --git a/cms/static/js/spec_helpers/validation_helpers.js b/cms/static/js/spec_helpers/validation_helpers.js new file mode 100644 index 0000000000..f9d5efd519 --- /dev/null +++ b/cms/static/js/spec_helpers/validation_helpers.js @@ -0,0 +1,34 @@ +/** + * Provides helper methods for invoking Validation modal in Jasmine tests. + */ +define(['jquery', 'js/spec_helpers/modal_helpers', 'js/spec_helpers/view_helpers'], + function($, modal_helpers, view_helpers) { + var installValidationTemplates, checkErrorContents, undoChanges; + + installValidationTemplates = function () { + modal_helpers.installModalTemplates(); + view_helpers.installTemplate('validation-error-modal'); + }; + + checkErrorContents = function(validationModal, errorObjects) { + var errorItems = validationModal.$('.error-item-message'); + var i, item; + var num_items = errorItems.length; + expect(num_items).toBe(errorObjects.length); + + for (i = 0; i < num_items; i++) { + item = errorItems[i]; + expect(item.value).toBe(errorObjects[i].message); + } + }; + + undoChanges = function(validationModal) { + modal_helpers.pressModalButton('.action-undo', validationModal); + }; + + return $.extend(modal_helpers, { + 'installValidationTemplates': installValidationTemplates, + 'checkErrorContents': checkErrorContents, + 'undoChanges': undoChanges, + }); + }); \ No newline at end of file diff --git a/cms/static/js/views/modals/validation_error_modal.js b/cms/static/js/views/modals/validation_error_modal.js new file mode 100644 index 0000000000..545fed566d --- /dev/null +++ b/cms/static/js/views/modals/validation_error_modal.js @@ -0,0 +1,65 @@ +define(['jquery', 'underscore', 'gettext', 'js/views/modals/base_modal'], + function($, _, gettext, BaseModal) { + var ValidationErrorModal = BaseModal.extend({ + events: { + 'click .action-cancel': 'cancel', + 'click .action-undo': 'resetAction' + }, + + initialize: function() { + BaseModal.prototype.initialize.call(this); + this.template = this.loadTemplate('validation-error-modal'); + }, + + options: $.extend({}, BaseModal.prototype.options, { + modalName: 'Validation Error Modal', + title: gettext('Validation Error While Saving'), + modalSize: 'md' + }), + + addActionButtons: function() { + this.addActionButton('undo', gettext('Undo Changes'), true); + this.addActionButton('cancel', gettext('Change Manually')); + }, + + render: function() { + BaseModal.prototype.render.call(this); + }, + + /* Set the JSON object of error_models that will be displayed + * it must be an object, not json string. */ + setContent: function(json_object) { + this.response = json_object; + }, + + /* Create the content HTML for this modal by passing necessary + * parameters to template (validation-error-modal) */ + getContentHtml: function() { + + return this.template({ + response: this.response, + num_errors: this.response.length, + }); + }, + + /* Receive calback function from the view, so that it can be + * invoked when the user clicks the reset button */ + setResetCallback: function(reset_callback) { + this.reset_callback = reset_callback; + }, + + /* Upon receiving a user's clicking event on the reset button, + * resets all setting changes, and hide the modal */ + resetAction: function() { + + // reset page content + this.reset_callback(); + + // hide the modal + BaseModal.prototype.hide.call(this); + }, + }); + + return ValidationErrorModal; + } +); diff --git a/cms/static/js/views/settings/advanced.js b/cms/static/js/views/settings/advanced.js index 0c3ab7aef8..7ed7624245 100644 --- a/cms/static/js/views/settings/advanced.js +++ b/cms/static/js/views/settings/advanced.js @@ -1,5 +1,5 @@ -define(["js/views/validation", "jquery", "underscore", "gettext", "codemirror"], - function(ValidatingView, $, _, gettext, CodeMirror) { +define(["js/views/validation", "jquery", "underscore", "gettext", "codemirror", "js/views/modals/validation_error_modal"], + function(ValidatingView, $, _, gettext, CodeMirror, ValidationErrorModal) { var AdvancedView = ValidatingView.extend({ error_saving : "error_saving", @@ -51,8 +51,8 @@ var AdvancedView = ValidatingView.extend({ var self = this; var oldValue = $(textarea).val(); var cm = CodeMirror.fromTextArea(textarea, { - mode: "application/json", - lineNumbers: false, + mode: "application/json", + lineNumbers: false, lineWrapping: false}); cm.on('change', function(instance, changeobj) { instance.save(); @@ -115,7 +115,24 @@ var AdvancedView = ValidatingView.extend({ 'course': course_location_analytics }); }, - silent: true + silent: true, + error: function(model, response, options) { + var json_response, reset_callback, err_modal; + + /* Check that the server came back with a bad request error*/ + if (response.status === 400) { + json_response = $.parseJSON(response.responseText); + reset_callback = function() { + self.revertView(); + }; + + /* initialize and show validation error modal */ + err_modal = new ValidationErrorModal(); + err_modal.setContent(json_response); + err_modal.setResetCallback(reset_callback); + err_modal.show(); + } + } }); }, revertView: function() { diff --git a/cms/static/sass/views/_settings.scss b/cms/static/sass/views/_settings.scss index aff6c81b47..0d22c1284f 100644 --- a/cms/static/sass/views/_settings.scss +++ b/cms/static/sass/views/_settings.scss @@ -899,4 +899,40 @@ .content-supplementary { width: flex-grid(3, 12); } + + .wrapper-modal-window { + + .validation-error-modal-content { + + .error-header { + p { + strong { + color: $error-red; + } + } + } + + hr { + margin: 25px 0; + } + + .error-list { + .error-item { + .error-item-title { + color: $error-red; + } + + .error-item-message { + width:100%; + border: none; + resize: none; + + &:focus { + outline: 0; + } + } + } + } + } + } } diff --git a/cms/templates/js/validation-error-modal.underscore b/cms/templates/js/validation-error-modal.underscore new file mode 100644 index 0000000000..0c2eb7e402 --- /dev/null +++ b/cms/templates/js/validation-error-modal.underscore @@ -0,0 +1,30 @@ +
diff --git a/cms/templates/settings_advanced.html b/cms/templates/settings_advanced.html index 4cbdf4cfed..ab530e88a4 100644 --- a/cms/templates/settings_advanced.html +++ b/cms/templates/settings_advanced.html @@ -10,7 +10,7 @@ <%block name="bodyclass">is-signedin course advanced view-settings%block> <%block name="jsextra"> -% for template_name in ["advanced_entry"]: +% for template_name in ["advanced_entry", "basic-modal", "modal-button", "validation-error-modal"]: diff --git a/common/djangoapps/util/json_request.py b/common/djangoapps/util/json_request.py index b390c8c11c..0f1e61d5d3 100644 --- a/common/djangoapps/util/json_request.py +++ b/common/djangoapps/util/json_request.py @@ -3,7 +3,7 @@ import json from django.core.serializers import serialize from django.core.serializers.json import DjangoJSONEncoder from django.db.models.query import QuerySet -from django.http import HttpResponse +from django.http import HttpResponse, HttpResponseBadRequest def expect_json(view_function): @@ -43,3 +43,23 @@ class JsonResponse(HttpResponse): if status: kwargs["status"] = status super(JsonResponse, self).__init__(content, *args, **kwargs) + + +class JsonResponseBadRequest(HttpResponseBadRequest): + """ + Subclass of HttpResponseBadRequest that defaults to outputting JSON. + Use this to send BadRequestResponse & some Json object along with it. + + Defaults: + dictionary: empty dictionary + status: 400 + encoder: DjangoJSONEncoder + """ + def __init__(self, obj=None, status=400, encoder=DjangoJSONEncoder, *args, **kwargs): + if obj in (None, ""): + content = "" + else: + content = json.dumps(obj, cls=encoder, indent=2, ensure_ascii=False) + kwargs.setdefault("content_type", "application/json") + kwargs["status"] = status + super(JsonResponseBadRequest, self).__init__(content, *args, **kwargs) diff --git a/common/djangoapps/util/tests/test_json_request.py b/common/djangoapps/util/tests/test_json_request.py index 28d6a44286..f65d7ca6b2 100644 --- a/common/djangoapps/util/tests/test_json_request.py +++ b/common/djangoapps/util/tests/test_json_request.py @@ -1,11 +1,18 @@ -from django.http import HttpResponse -from util.json_request import JsonResponse +""" +Test for JsonResponse and JsonResponseBadRequest util classes. +""" + +from django.http import HttpResponse, HttpResponseBadRequest +from util.json_request import JsonResponse, JsonResponseBadRequest import json import unittest import mock class JsonResponseTestCase(unittest.TestCase): + """ + A set of tests to make sure that JsonResponse Class works correctly. + """ def test_empty(self): resp = JsonResponse() self.assertIsInstance(resp, HttpResponse) @@ -60,3 +67,59 @@ class JsonResponseTestCase(unittest.TestCase): self.assertEqual(obj, compare) kwargs = dumps.call_args[1] self.assertIs(kwargs["cls"], encoder) + + +class JsonResponseBadRequestTestCase(unittest.TestCase): + """ + A set of tests to make sure that the JsonResponseBadRequest wrapper class + works as intended. + """ + + def test_empty(self): + resp = JsonResponseBadRequest() + self.assertIsInstance(resp, HttpResponseBadRequest) + self.assertEqual(resp.content, "") + self.assertEqual(resp.status_code, 400) + self.assertEqual(resp["content-type"], "application/json") + + def test_empty_string(self): + resp = JsonResponseBadRequest("") + self.assertIsInstance(resp, HttpResponse) + self.assertEqual(resp.content, "") + self.assertEqual(resp.status_code, 400) + self.assertEqual(resp["content-type"], "application/json") + + def test_dict(self): + obj = {"foo": "bar"} + resp = JsonResponseBadRequest(obj) + compare = json.loads(resp.content) + self.assertEqual(obj, compare) + self.assertEqual(resp.status_code, 400) + self.assertEqual(resp["content-type"], "application/json") + + def test_set_status_kwarg(self): + obj = {"error": "resource not found"} + resp = JsonResponseBadRequest(obj, status=404) + compare = json.loads(resp.content) + self.assertEqual(obj, compare) + self.assertEqual(resp.status_code, 404) + self.assertEqual(resp["content-type"], "application/json") + + def test_set_status_arg(self): + obj = {"error": "resource not found"} + resp = JsonResponseBadRequest(obj, 404) + compare = json.loads(resp.content) + self.assertEqual(obj, compare) + self.assertEqual(resp.status_code, 404) + self.assertEqual(resp["content-type"], "application/json") + + def test_encoder(self): + obj = [1, 2, 3] + encoder = object() + with mock.patch.object(json, "dumps", return_value="[1,2,3]") as dumps: + resp = JsonResponseBadRequest(obj, encoder=encoder) + self.assertEqual(resp.status_code, 400) + compare = json.loads(resp.content) + self.assertEqual(obj, compare) + kwargs = dumps.call_args[1] + self.assertIs(kwargs["cls"], encoder) diff --git a/common/test/acceptance/pages/studio/settings_advanced.py b/common/test/acceptance/pages/studio/settings_advanced.py index 2c514ed23e..4c7f6e87c1 100644 --- a/common/test/acceptance/pages/studio/settings_advanced.py +++ b/common/test/acceptance/pages/studio/settings_advanced.py @@ -7,7 +7,11 @@ from .utils import press_the_notification_button, type_in_codemirror, get_codemi KEY_CSS = '.key h3.title' - +UNDO_BUTTON_SELECTOR = ".action-item .action-undo" +MANUAL_BUTTON_SELECTOR = ".action-item .action-cancel" +MODAL_SELECTOR = ".validation-error-modal-content" +ERROR_ITEM_NAME_SELECTOR = ".error-item-title strong" +ERROR_ITEM_CONTENT_SELECTOR = ".error-item-message" class AdvancedSettingsPage(CoursePage): """ @@ -19,6 +23,57 @@ class AdvancedSettingsPage(CoursePage): def is_browser_on_page(self): return self.q(css='body.advanced').present + def wait_for_modal_load(self): + """ + Wait for validation response from the server, and make sure that + the validation error modal pops up. + + This method should only be called when it is guaranteed that there're + validation errors in the settings changes. + """ + self.wait_for_ajax() + self.wait_for_element_presence(MODAL_SELECTOR, 'Validation Modal is present') + + def refresh_and_wait_for_load(self): + """ + Refresh the page and wait for all resources to load. + """ + self.browser.refresh() + self.wait_for_page() + + def undo_changes_via_modal(self): + """ + Trigger clicking event of the undo changes button in the modal. + Wait for the undoing process to load via ajax call. + """ + self.q(css=UNDO_BUTTON_SELECTOR).click() + self.wait_for_ajax() + + def trigger_manual_changes(self): + """ + Trigger click event of the manual changes button in the modal. + No need to wait for any ajax. + """ + self.q(css=MANUAL_BUTTON_SELECTOR).click() + + def is_validation_modal_present(self): + """ + Checks if the validation modal is present. + """ + return self.q(css=MODAL_SELECTOR).present + + def get_error_item_names(self): + """ + Returns a list of display names of all invalid settings. + """ + return self.q(css=ERROR_ITEM_NAME_SELECTOR).text + + def get_error_item_messages(self): + """ + Returns a list of error messages of all invalid settings. + """ + return self.q(css=ERROR_ITEM_CONTENT_SELECTOR).text + def _get_index_of(self, expected_key): for i, element in enumerate(self.q(css=KEY_CSS)): # Sometimes get stale reference if I hold on to the array of elements @@ -42,3 +97,26 @@ class AdvancedSettingsPage(CoursePage): def get(self, key): index = self._get_index_of(key) return get_codemirror_value(self, index) + + def set_values(self, key_value_map): + """ + Make multiple settings changes and save them. + """ + for key, value in key_value_map.iteritems(): + index = self._get_index_of(key) + type_in_codemirror(self, index, value) + + self.save() + + def get_values(self, key_list): + """ + Get a key-value dictionary of all keys in the given list. + """ + result_map = {} + + for key in key_list: + index = self._get_index_of(key) + val = get_codemirror_value(self, index) + result_map[key] = val + + return result_map diff --git a/common/test/acceptance/tests/test_studio_settings.py b/common/test/acceptance/tests/test_studio_settings.py new file mode 100644 index 0000000000..86f2c40577 --- /dev/null +++ b/common/test/acceptance/tests/test_studio_settings.py @@ -0,0 +1,171 @@ +""" +Acceptance tests for Studio's Setting pages +""" + +from nose.plugins.attrib import attr + +from acceptance.tests.base_studio_test import StudioCourseTest + +from ..pages.studio.settings_advanced import AdvancedSettingsPage + + +@attr('shard_1') +class AdvancedSettingsValidationTest(StudioCourseTest): + """ + Tests for validation feature in Studio's advanced settings tab + """ + def setUp(self): + super(AdvancedSettingsValidationTest, self).setUp() + self.advanced_settings = AdvancedSettingsPage( + self.browser, + self.course_info['org'], + self.course_info['number'], + self.course_info['run'] + ) + + self.type_fields = ['Course Display Name', 'Advanced Module List', 'Discussion Topic Mapping', + 'Maximum Attempts', 'Course Announcement Date'] + + # Before every test, make sure to visit the page first + self.advanced_settings.visit() + self.assertTrue(self.advanced_settings.is_browser_on_page()) + + def test_modal_shows_one_validation_error(self): + """ + Test that advanced settings don't save if there's a single wrong input, + and that it shows the correct error message in the modal. + """ + + # Feed an integer value for String field. + # .set method saves automatically after setting a value + course_display_name = self.advanced_settings.get('Course Display Name') + self.advanced_settings.set('Course Display Name', 1) + self.advanced_settings.wait_for_modal_load() + + # Test Modal + self.check_modal_shows_correct_contents(['Course Display Name']) + self.advanced_settings.refresh_and_wait_for_load() + + self.assertEquals( + self.advanced_settings.get('Course Display Name'), + course_display_name, + 'Wrong input for Course Display Name must not change its value' + ) + + def test_modal_shows_multiple_validation_errors(self): + """ + Test that advanced settings don't save with multiple wrong inputs + """ + + # Save original values and feed wrong inputs + original_values_map = self.get_settings_fields_of_each_type() + self.set_wrong_inputs_to_fields() + self.advanced_settings.wait_for_modal_load() + + # Test Modal + self.check_modal_shows_correct_contents(self.type_fields) + self.advanced_settings.refresh_and_wait_for_load() + + for key, val in original_values_map.iteritems(): + self.assertEquals( + self.advanced_settings.get(key), + val, + 'Wrong input for Advanced Settings Fields must not change its value' + ) + + def test_undo_changes(self): + """ + Test that undo changes button in the modal resets all settings changes + """ + + # Save original values and feed wrong inputs + original_values_map = self.get_settings_fields_of_each_type() + self.set_wrong_inputs_to_fields() + + # Let modal popup + self.advanced_settings.wait_for_modal_load() + + # Press Undo Changes button + self.advanced_settings.undo_changes_via_modal() + + # Check that changes are undone + for key, val in original_values_map.iteritems(): + self.assertEquals( + self.advanced_settings.get(key), + val, + 'Undoing Should revert back to original value' + ) + + def test_manual_change(self): + """ + Test that manual changes button in the modal keeps settings unchanged + """ + inputs = {"Course Display Name": 1, + "Advanced Module List": 1, + "Discussion Topic Mapping": 1, + "Maximum Attempts": '"string"', + "Course Announcement Date": '"string"', + } + + self.set_wrong_inputs_to_fields() + self.advanced_settings.wait_for_modal_load() + self.advanced_settings.trigger_manual_changes() + + # Check that the validation modal went away. + self.assertFalse(self.advanced_settings.is_validation_modal_present()) + + # Iterate through the wrong values and make sure they're still displayed + for key, val in inputs.iteritems(): + print self.advanced_settings.get(key) + print val + self.assertEquals( + str(self.advanced_settings.get(key)), + str(val), + 'manual change should keep: ' + str(val) + ', but is: ' + str(self.advanced_settings.get(key)) + ) + + def check_modal_shows_correct_contents(self, wrong_settings_list): + """ + Helper function that checks if the validation modal contains correct + error messages. + """ + # Check presence of modal + self.assertTrue(self.advanced_settings.is_validation_modal_present()) + + # List of wrong settings item & what is presented in the modal should be the same + error_item_names = self.advanced_settings.get_error_item_names() + self.assertEqual(set(wrong_settings_list), set(error_item_names)) + + error_item_messages = self.advanced_settings.get_error_item_messages() + self.assertEqual(len(error_item_names), len(error_item_messages)) + + def get_settings_fields_of_each_type(self): + """ + Get one of each field type: + - String: Course Display Name + - List: Advanced Module List + - Dict: Discussion Topic Mapping + - Integer: Maximum Attempts + - Date: Course Announcement Date + """ + return { + "Course Display Name": self.advanced_settings.get('Course Display Name'), + "Advanced Module List": self.advanced_settings.get('Advanced Module List'), + "Discussion Topic Mapping": self.advanced_settings.get('Discussion Topic Mapping'), + "Maximum Attempts": self.advanced_settings.get('Maximum Attempts'), + "Course Announcement Date": self.advanced_settings.get('Course Announcement Date'), + } + + def set_wrong_inputs_to_fields(self): + """ + Set wrong values for the chosen fields + """ + self.advanced_settings.set_values( + { + "Course Display Name": 1, + "Advanced Module List": 1, + "Discussion Topic Mapping": 1, + "Maximum Attempts": '"string"', + "Course Announcement Date": '"string"', + } + ) diff --git a/common/test/data/uploads/asset.html b/common/test/data/uploads/asset.html index 7898439363..4ca5651d88 100644 --- a/common/test/data/uploads/asset.html +++ b/common/test/data/uploads/asset.html @@ -7,4 +7,4 @@