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 a4b5d31fea..0e27445775 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 4667e6e138..864fe23c83 100644 --- a/cms/djangoapps/contentstore/tests/test_checklists.py +++ b/cms/djangoapps/contentstore/tests/test_checklists.py @@ -105,7 +105,7 @@ class ChecklistTestCase(CourseTestCase): 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 3ba3e30493..4aca7751e7 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -6,7 +6,6 @@ import mock 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 datetime import timedelta 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 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # 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 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): 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 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): 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 @@ class ContentStoreTest(ModuleStoreTestCase): 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 @@ class ContentStoreTest(ModuleStoreTestCase): } 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 @@ class ContentStoreTest(ModuleStoreTestCase): 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 @@ class ContentStoreTest(ModuleStoreTestCase): """ 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 @@ class ContentStoreTest(ModuleStoreTestCase): 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 @@ class ContentStoreTest(ModuleStoreTestCase): 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 @@ class ContentStoreTest(ModuleStoreTestCase): """ 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 @@ class ContentStoreTest(ModuleStoreTestCase): """ 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 @@ class ContentStoreTest(ModuleStoreTestCase): '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 @@ class ContentStoreTest(ModuleStoreTestCase): '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 f7ffbad334..d1cbcde4d0 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -176,7 +176,7 @@ class CourseDetailsViewTest(CourseTestCase): 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 @@ class CourseGraderUpdatesTest(CourseTestCase): "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 c47cdb9ef7..1ea500c6f0 100644 --- a/cms/djangoapps/contentstore/tests/test_course_updates.py +++ b/cms/djangoapps/contentstore/tests/test_course_updates.py @@ -22,7 +22,7 @@ class CourseUpdateTest(CourseTestCase): '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 @@ class CourseUpdateTest(CourseTestCase): 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 @@ class CourseUpdateTest(CourseTestCase): 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 @@ class CourseUpdateTest(CourseTestCase): '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 1ddd0d14b2..af577bbb47 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 ca4aa0da8f..88b47b6733 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 49af6db16d..83b4f02897 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 592096aeff..bc155d7413 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 46b703a09b..c84abdf127 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 fd2008eab5..7ef9c30b52 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 32aa4ac9be..2c12c47056 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 55ff2b1371..0e8ae86b0c 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 e187cf3ff6..f6e20cfee2 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -274,8 +274,6 @@ if settings.COURSEWARE_ENABLED: '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'),