From 45453fae619f914e4f4a0e223cedc8952e576269 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Wed, 23 Oct 2013 10:31:35 -0400 Subject: [PATCH] Change expect_json to put parsed json in new attr --- CHANGELOG.rst | 37 ++++++++++-------- .../contentstore/tests/test_checklists.py | 2 +- .../contentstore/tests/test_contentstore.py | 38 +++++++++---------- .../tests/test_course_settings.py | 4 +- .../contentstore/tests/test_course_updates.py | 9 ++--- .../contentstore/tests/test_transcripts.py | 8 ++-- cms/djangoapps/contentstore/tests/utils.py | 10 ++++- .../contentstore/views/component.py | 10 ++--- cms/djangoapps/contentstore/views/course.py | 32 +++++++--------- cms/djangoapps/contentstore/views/item.py | 33 ++++++++-------- cms/djangoapps/contentstore/views/tabs.py | 2 +- cms/static/coffee/src/main.coffee | 13 +++++++ .../coffee/src/views/module_edit.coffee | 2 +- cms/static/coffee/src/views/tabs.coffee | 2 +- cms/static/coffee/src/views/unit.coffee | 10 ++--- cms/static/js/base.js | 4 +- cms/static/js/index.js | 2 +- cms/static/js/views/overview.js | 4 +- common/djangoapps/util/json_request.py | 14 +++---- .../staff_grading_service.py | 1 - lms/urls.py | 2 - 21 files changed, 126 insertions(+), 113 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a4b5d31fea9c..0e2744577591 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -33,6 +33,27 @@ in the set contentstore.views.item.DETACHED_CATEGORIES nor 'course'. Studio: Bug fix for text loss in Course Updates when the text exists before the first tag. +Common: expect_json decorator now puts the parsed json payload into a json attr +on the request instead of overwriting the POST attr + +---------- split mongo backend refactoring changelog section ------------ + +Studio: course catalog, assets, checklists, course outline pages now use course +id syntax w/ restful api style + +Common: + separate the non-sql db connection configuration from the modulestore (xblock modeling) configuration. + in split, separate the the db connection and atomic crud ops into a distinct module & class from modulestore + +Common: location mapper: % encode periods and dollar signs when used as key in the mapping dict + +Common: location mapper: added a bunch of new helper functions for generating +old location style info from a CourseLocator + +Common: locators: allow - ~ and . in course, branch, and block ids. + +---------- end split mongo backend section --------- + Blades: Hovering over CC button in video player, when transcripts are hidden, will cause them to show up. Moving the mouse from the CC button will auto hide them. You can hover over the CC button and then move the mouse to the @@ -388,22 +409,6 @@ Studio: Add feedback to end user if there is a problem exporting a course Studio: Improve link re-writing on imports into a different course-id ----------- split mongo backend refactoring changelog section ------------ - -Studio: course catalog and course outline pages new use course id syntax w/ restful api style - -Common: - separate the non-sql db connection configuration from the modulestore (xblock modeling) configuration. - in split, separate the the db connection and atomic crud ops into a distinct module & class from modulestore - -Common: location mapper: % encode periods and dollar signs when used as key in the mapping dict - -Common: location mapper: added a bunch of new helper functions for generating old location style info from a CourseLocator - -Common: locators: allow - ~ and . in course, branch, and block ids. - ----------- end split mongo backend section --------- - XQueue: Fixed (hopefully) worker crash when the connection to RabbitMQ is dropped suddenly. diff --git a/cms/djangoapps/contentstore/tests/test_checklists.py b/cms/djangoapps/contentstore/tests/test_checklists.py index 4667e6e1383f..864fe23c8333 100644 --- a/cms/djangoapps/contentstore/tests/test_checklists.py +++ b/cms/djangoapps/contentstore/tests/test_checklists.py @@ -105,7 +105,7 @@ def test_update_checklists_index(self): self.assertEqual('CourseOutline', get_first_item(payload).get('action_url')) get_first_item(payload)['is_checked'] = True - returned_checklist = json.loads(self.client.post(update_url, json.dumps(payload), "application/json").content) + returned_checklist = json.loads(self.client.ajax_post(update_url, payload).content) self.assertTrue(get_first_item(returned_checklist).get('is_checked')) persisted_checklist = self.get_persisted_checklists()[1] # Verify that persisted checklist does not have expanded action URLs. diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 3ba3e304935b..4aca7751e731 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -6,7 +6,6 @@ from textwrap import dedent -from django.test.client import Client from django.test.utils import override_settings from django.conf import settings from django.core.urlresolvers import reverse @@ -20,7 +19,7 @@ from django.contrib.auth.models import User from django.dispatch import Signal from contentstore.utils import get_modulestore -from contentstore.tests.utils import parse_json +from contentstore.tests.utils import parse_json, AjaxEnabledTestClient from auth.authz import add_user_to_creator_group @@ -98,7 +97,7 @@ def setUp(self): # Save the data that we've just changed to the db. self.user.save() - self.client = Client() + self.client = AjaxEnabledTestClient() self.client.login(username=uname, password=password) def tearDown(self): @@ -420,7 +419,7 @@ def test_static_tab_reordering(self): if tab['type'] == 'static_tab': reverse_tabs.insert(0, 'i4x://edX/999/static_tab/{0}'.format(tab['url_slug'])) - self.client.post(reverse('reorder_static_tabs'), json.dumps({'tabs': reverse_tabs}), "application/json") + self.client.ajax_post(reverse('reorder_static_tabs'), {'tabs': reverse_tabs}) course = module_store.get_item(Location(['i4x', 'edX', '999', 'course', 'Robot_Super_Course', None])) @@ -755,7 +754,7 @@ def test_clone_course(self): expected_children = [] for child_loc_url in source_item.children: child_loc = Location(child_loc_url) - child_loc = child_loc._replace( + child_loc = child_loc.replace( tag=dest_location.tag, org=dest_location.org, course=dest_location.course @@ -1333,7 +1332,7 @@ def setUp(self): self.user.is_staff = True self.user.save() - self.client = Client() + self.client = AjaxEnabledTestClient() self.client.login(username=uname, password=password) self.course_data = { @@ -1344,8 +1343,7 @@ def setUp(self): } def tearDown(self): - mongo = MongoClient() - mongo.drop_database(TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db']) + MongoClient().drop_database(TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db']) _CONTENTSTORE.clear() def test_create_course(self): @@ -1394,7 +1392,7 @@ def test_forum_unseeding_with_multiple_courses(self): def test_create_course_duplicate_course(self): """Test new course creation - error path""" - self.client.post(reverse('create_new_course'), self.course_data) + self.client.ajax_post(reverse('create_new_course'), self.course_data) self.assert_course_creation_failed('There is already a course defined with the same organization, course number, and course run. Please change either organization or course number to be unique.') def assert_course_creation_failed(self, error_message): @@ -1403,7 +1401,7 @@ def assert_course_creation_failed(self, error_message): """ course_id = _get_course_id(self.course_data) initially_enrolled = CourseEnrollment.is_enrolled(self.user, course_id) - resp = self.client.post(reverse('create_new_course'), self.course_data) + resp = self.client.ajax_post(reverse('create_new_course'), self.course_data) self.assertEqual(resp.status_code, 200) data = parse_json(resp) self.assertEqual(data['ErrMsg'], error_message) @@ -1413,7 +1411,7 @@ def assert_course_creation_failed(self, error_message): def test_create_course_duplicate_number(self): """Test new course creation - error path""" - self.client.post(reverse('create_new_course'), self.course_data) + self.client.ajax_post(reverse('create_new_course'), self.course_data) self.course_data['display_name'] = 'Robot Super Course Two' self.course_data['run'] = '2013_Summer' @@ -1422,13 +1420,13 @@ def test_create_course_duplicate_number(self): def test_create_course_case_change(self): """Test new course creation - error path due to case insensitive name equality""" self.course_data['number'] = 'capital' - self.client.post(reverse('create_new_course'), self.course_data) + self.client.ajax_post(reverse('create_new_course'), self.course_data) cache_current = self.course_data['org'] self.course_data['org'] = self.course_data['org'].lower() self.assert_course_creation_failed('There is already a course defined with the same organization and course number. Please change at least one field to be unique.') self.course_data['org'] = cache_current - self.client.post(reverse('create_new_course'), self.course_data) + self.client.ajax_post(reverse('create_new_course'), self.course_data) cache_current = self.course_data['number'] self.course_data['number'] = self.course_data['number'].upper() self.assert_course_creation_failed('There is already a course defined with the same organization and course number. Please change at least one field to be unique.') @@ -1437,14 +1435,14 @@ def test_course_substring(self): """ Test that a new course can be created whose name is a substring of an existing course """ - self.client.post(reverse('create_new_course'), self.course_data) + self.client.ajax_post(reverse('create_new_course'), self.course_data) cache_current = self.course_data['number'] self.course_data['number'] = '{}a'.format(self.course_data['number']) - resp = self.client.post(reverse('create_new_course'), self.course_data) + resp = self.client.ajax_post(reverse('create_new_course'), self.course_data) self.assertEqual(resp.status_code, 200) self.course_data['number'] = cache_current self.course_data['org'] = 'a{}'.format(self.course_data['org']) - resp = self.client.post(reverse('create_new_course'), self.course_data) + resp = self.client.ajax_post(reverse('create_new_course'), self.course_data) self.assertEqual(resp.status_code, 200) def test_create_course_with_bad_organization(self): @@ -1487,7 +1485,7 @@ def assert_course_permission_denied(self): """ Checks that the course did not get created due to a PermissionError. """ - resp = self.client.post(reverse('create_new_course'), self.course_data) + resp = self.client.ajax_post(reverse('create_new_course'), self.course_data) self.assertEqual(resp.status_code, 403) def test_course_index_view_with_no_courses(self): @@ -1546,7 +1544,7 @@ def test_create_item(self): 'display_name': 'Section One', } - resp = self.client.post(reverse('create_item'), section_data) + resp = self.client.ajax_post(reverse('create_item'), section_data) self.assertEqual(resp.status_code, 200) data = parse_json(resp) @@ -1564,7 +1562,7 @@ def test_capa_module(self): 'category': 'problem' } - resp = self.client.post(reverse('create_item'), problem_data) + resp = self.client.ajax_post(reverse('create_item'), problem_data) self.assertEqual(resp.status_code, 200) payload = parse_json(resp) @@ -1934,7 +1932,7 @@ def _create_course(test, course_data): course_id = _get_course_id(course_data) new_location = loc_mapper().translate_location(course_id, CourseDescriptor.id_to_location(course_id), False, True) - response = test.client.post(reverse('create_new_course'), course_data) + response = test.client.ajax_post(reverse('create_new_course'), course_data) test.assertEqual(response.status_code, 200) data = parse_json(response) test.assertNotIn('ErrMsg', data) diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index f7ffbad334a5..d1cbcde4d0d9 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -176,7 +176,7 @@ def alter_field(self, url, details, field, val): payload['end_date'] = CourseDetailsViewTest.convert_datetime_to_iso(details.end_date) payload['enrollment_start'] = CourseDetailsViewTest.convert_datetime_to_iso(details.enrollment_start) payload['enrollment_end'] = CourseDetailsViewTest.convert_datetime_to_iso(details.enrollment_end) - resp = self.client.post(url, json.dumps(payload), "application/json") + resp = self.client.ajax_post(url, payload) self.compare_details_with_encoding(json.loads(resp.content), details.__dict__, field + str(val)) @staticmethod @@ -462,6 +462,6 @@ def test_post(self): "short_label": "yo momma", "weight": 17.3, } - resp = self.client.post(self.url, grader) + resp = self.client.ajax_post(self.url, grader) self.assertEqual(resp.status_code, 200) obj = json.loads(resp.content) diff --git a/cms/djangoapps/contentstore/tests/test_course_updates.py b/cms/djangoapps/contentstore/tests/test_course_updates.py index c47cdb9ef74d..1ea500c6f0d9 100644 --- a/cms/djangoapps/contentstore/tests/test_course_updates.py +++ b/cms/djangoapps/contentstore/tests/test_course_updates.py @@ -22,7 +22,7 @@ def get_response(content, date): 'course': self.course.location.course, 'provided_id': ''}) - resp = self.client.post(url, json.dumps(payload), "application/json") + resp = self.client.ajax_post(url, payload) return json.loads(resp.content) @@ -66,7 +66,6 @@ def get_response(content, date): payload = json.loads(resp.content) self.assertTrue(len(payload) == 2) - # can't test non-json paylod b/c expect_json throws error # try json w/o required fields self.assertContains(self.client.post(url, json.dumps({'garbage': 1}), "application/json"), @@ -86,7 +85,7 @@ def get_response(content, date): payload = {'content': content, 'date': 'January 21, 2013'} self.assertContains( - self.client.post(url, json.dumps(payload), "application/json"), + self.client.ajax_post(url, payload), 'Failed to save', status_code=400) # update w/ malformed html @@ -98,7 +97,7 @@ def get_response(content, date): 'provided_id': ''}) self.assertContains( - self.client.post(url, json.dumps(payload), "application/json"), + self.client.ajax_post(url, payload), ' + # shift arguments if data argument was omitted + if $.isFunction(data) + callback = data + data = `undefined` + $.ajax + url: url + type: "POST" + contentType: "application/json; charset=utf-8" + dataType: "json" + data: JSON.stringify(data) + success: callback + if onTouchBasedDevice() $('body').addClass 'touch-based-device' diff --git a/cms/static/coffee/src/views/module_edit.coffee b/cms/static/coffee/src/views/module_edit.coffee index 1ddd0d14b24f..af577bbb4716 100644 --- a/cms/static/coffee/src/views/module_edit.coffee +++ b/cms/static/coffee/src/views/module_edit.coffee @@ -64,7 +64,7 @@ define ["backbone", "jquery", "underscore", "gettext", "xblock/runtime.v1", createItem: (parent, payload) -> payload.parent_location = parent - $.post( + $.postJSON( "/create_item" payload (data) => diff --git a/cms/static/coffee/src/views/tabs.coffee b/cms/static/coffee/src/views/tabs.coffee index ca4aa0da8f72..88b47b67334f 100644 --- a/cms/static/coffee/src/views/tabs.coffee +++ b/cms/static/coffee/src/views/tabs.coffee @@ -82,7 +82,7 @@ define ["jquery", "jquery.ui", "backbone", "js/views/feedback_prompt", "js/views deleting = new NotificationView.Mini title: gettext('Deleting…') deleting.show() - $.post('/delete_item', { + $.postJSON('/delete_item', { id: $component.data('id') }, => $component.remove() diff --git a/cms/static/coffee/src/views/unit.coffee b/cms/static/coffee/src/views/unit.coffee index 49af6db16db1..83b4f02897f4 100644 --- a/cms/static/coffee/src/views/unit.coffee +++ b/cms/static/coffee/src/views/unit.coffee @@ -134,7 +134,7 @@ define ["jquery", "jquery.ui", "gettext", "backbone", title: gettext('Deleting…'), deleting.show() $component = $(event.currentTarget).parents('.component') - $.post('/delete_item', { + $.postJSON('/delete_item', { id: $component.data('id') }, => deleting.hide() @@ -163,7 +163,7 @@ define ["jquery", "jquery.ui", "gettext", "backbone", deleteDraft: (event) -> @wait(true) - $.post('/delete_item', { + $.postJSON('/delete_item', { id: @$el.data('id') delete_children: true }, => @@ -177,7 +177,7 @@ define ["jquery", "jquery.ui", "gettext", "backbone", createDraft: (event) -> @wait(true) - $.post('/create_draft', { + $.postJSON('/create_draft', { id: @$el.data('id') }, => analytics.track "Created Draft", @@ -191,7 +191,7 @@ define ["jquery", "jquery.ui", "gettext", "backbone", @wait(true) @saveDraft() - $.post('/publish_draft', { + $.postJSON('/publish_draft', { id: @$el.data('id') }, => analytics.track "Published Draft", @@ -211,7 +211,7 @@ define ["jquery", "jquery.ui", "gettext", "backbone", @wait(true) - $.post(target_url, { + $.postJSON(target_url, { id: @$el.data('id') }, => analytics.track "Set Unit Visibility", diff --git a/cms/static/js/base.js b/cms/static/js/base.js index 592096aeff92..bc155d7413c2 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -230,7 +230,7 @@ function createNewUnit(e) { }); - $.post('/create_item', { + $.postJSON('/create_item', { 'parent_location': parent, 'category': category, 'display_name': 'New Unit' @@ -279,7 +279,7 @@ function _deleteItem($el, type) { }); deleting.show(); - $.post('/delete_item', + $.postJSON('/delete_item', {'id': id, 'delete_children': true, 'delete_all_versions': true}, diff --git a/cms/static/js/index.js b/cms/static/js/index.js index 46b703a09b13..c84abdf127af 100644 --- a/cms/static/js/index.js +++ b/cms/static/js/index.js @@ -32,7 +32,7 @@ require(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"], 'run': run }); - $.post('/create_new_course', { + $.postJSON('/create_new_course', { 'org': org, 'number': number, 'display_name': display_name, diff --git a/cms/static/js/views/overview.js b/cms/static/js/views/overview.js index fd2008eab5ca..7ef9c30b52d4 100644 --- a/cms/static/js/views/overview.js +++ b/cms/static/js/views/overview.js @@ -132,7 +132,7 @@ define(["domReady", "jquery", "jquery.ui", "underscore", "gettext", "js/views/fe 'display_name': display_name }); - $.post('/create_item', { + $.postJSON('/create_item', { 'parent_location': parent, 'category': category, 'display_name': display_name @@ -182,7 +182,7 @@ define(["domReady", "jquery", "jquery.ui", "underscore", "gettext", "js/views/fe }); - $.post('/create_item', { + $.postJSON('/create_item', { 'parent_location': parent, 'category': category, 'display_name': display_name diff --git a/common/djangoapps/util/json_request.py b/common/djangoapps/util/json_request.py index 32aa4ac9bef4..2c12c47056a8 100644 --- a/common/djangoapps/util/json_request.py +++ b/common/djangoapps/util/json_request.py @@ -14,17 +14,17 @@ def expect_json(view_function): request.POST with the contents. """ @wraps(view_function) - def expect_json_with_cloned_request(request, *args, **kwargs): + def parse_json_into_request(request, *args, **kwargs): # cdodge: fix postback errors in CMS. The POST 'content-type' header can include additional information # e.g. 'charset', so we can't do a direct string compare - if request.META.get('CONTENT_TYPE', '').lower().startswith("application/json"): - cloned_request = copy.copy(request) - cloned_request.POST = json.loads(request.body) - return view_function(cloned_request, *args, **kwargs) + if "application/json" in request.META.get('CONTENT_TYPE', ''): + request.json = json.loads(request.body) else: - return view_function(request, *args, **kwargs) + request.json = {} - return expect_json_with_cloned_request + return view_function(request, *args, **kwargs) + + return parse_json_into_request class JsonResponse(HttpResponse): diff --git a/lms/djangoapps/open_ended_grading/staff_grading_service.py b/lms/djangoapps/open_ended_grading/staff_grading_service.py index 55ff2b1371ec..0e8ae86b0c14 100644 --- a/lms/djangoapps/open_ended_grading/staff_grading_service.py +++ b/lms/djangoapps/open_ended_grading/staff_grading_service.py @@ -321,7 +321,6 @@ def _get_next(course_id, grader_id, location): 'error': STAFF_ERROR_MESSAGE}) -@expect_json def save_grade(request, course_id): """ Save the grade and feedback for a submission, and, if all goes well, return diff --git a/lms/urls.py b/lms/urls.py index e187cf3ff668..f6e20cfee273 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -274,8 +274,6 @@ 'open_ended_grading.staff_grading_service.get_next', name='staff_grading_get_next'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/staff_grading/save_grade$', 'open_ended_grading.staff_grading_service.save_grade', name='staff_grading_save_grade'), - url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/staff_grading/save_grade$', - 'open_ended_grading.staff_grading_service.save_grade', name='staff_grading_save_grade'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/staff_grading/get_problem_list$', 'open_ended_grading.staff_grading_service.get_problem_list', name='staff_grading_get_problem_list'),