Merge pull request #4454 from Stanford-Online/sjang92/advanced_settings_feedback
Sjang92/advanced settings feedback
This commit is contained in:
@@ -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 .*"
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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),
|
||||
|
||||
Reference in New Issue
Block a user