Use JsonResponse when it makes sense

cms/djangoapps/contentstore/tests/test_checklists.py

@@ -117,4 +117,4 @@ class ChecklistTestCase(CourseTestCase):
                                                            'name': self.course.location.name,
                                                            'checklist_index': 100})
         response = self.client.delete(update_url)
-        self.assertContains(response, 'Unsupported request', status_code=400)
+        self.assertEqual(response.status_code, 405)

cms/djangoapps/contentstore/views/assets.py

@@ -188,7 +188,7 @@ def upload_asset(request, org, course, coursename):
                         'msg': 'Upload completed'
                         }
 
-    response = HttpResponse(json.dumps(response_payload), mimetype="application/json")
+    response = JsonResponse(response_payload)
     response['asset_url'] = StaticContent.get_url_path_from_location(content.location)
     return response
 

cms/djangoapps/contentstore/views/checklist.py

 import json
 
-from django.http import HttpResponse, HttpResponseBadRequest
+from util.json_request import JsonResponse
+from django.http import HttpResponseBadRequest
 from django.contrib.auth.decorators import login_required
 from django_future.csrf import ensure_csrf_cookie
 from mitxmako.shortcuts import render_to_response
@@ -71,7 +72,7 @@ def update_checklist(request, org, course, name, checklist_index=None):
             course_module.checklists = course_module.checklists
             checklists, _ = expand_checklist_action_urls(course_module)
             modulestore.update_metadata(location, own_metadata(course_module))
-            return HttpResponse(json.dumps(checklists[index]), mimetype="application/json")
+            return JsonResponse(checklists[index])
         else:
             return HttpResponseBadRequest(
                 "Could not save checklist state because the checklist index was out of range or unspecified.",
@@ -81,7 +82,7 @@ def update_checklist(request, org, course, name, checklist_index=None):
         checklists, modified = expand_checklist_action_urls(course_module)
         if modified:
             modulestore.update_metadata(location, own_metadata(course_module))
-        return HttpResponse(json.dumps(checklists), mimetype="application/json")
+        return JsonResponse(checklists)
     else:
         return HttpResponseBadRequest("Unsupported request.", content_type="text/plain")
 

cms/djangoapps/contentstore/views/component.py

@@ -15,7 +15,7 @@ from xmodule.modulestore.django import modulestore
 from xmodule.util.date_utils import get_default_time_display
 
 from xblock.core import Scope
-from util.json_request import expect_json
+from util.json_request import expect_json, JsonResponse
 
 from contentstore.module_info_model import get_module_info, set_module_info
 from contentstore.utils import get_modulestore, get_lms_link_for_item, \
@@ -236,11 +236,9 @@ def assignment_type_update(request, org, course, category, name):
         raise HttpResponseForbidden()
 
     if request.method == 'GET':
-        return HttpResponse(json.dumps(CourseGradingModel.get_section_grader_type(location)),
-                            mimetype="application/json")
+        return JsonResponse(CourseGradingModel.get_section_grader_type(location))
     elif request.method == 'POST':  # post or put, doesn't matter.
-        return HttpResponse(json.dumps(CourseGradingModel.update_section_grader_type(location, request.POST)),
-                            mimetype="application/json")
+        return JsonResponse(CourseGradingModel.update_section_grader_type(location, request.POST))
 
 
 @login_required
@@ -309,8 +307,8 @@ def module_info(request, module_location):
         raise PermissionDenied()
 
     if real_method == 'GET':
-        return HttpResponse(json.dumps(get_module_info(get_modulestore(location), location, rewrite_static_links=rewrite_static_links)), mimetype="application/json")
+        return JsonResponse(get_module_info(get_modulestore(location), location, rewrite_static_links=rewrite_static_links))
     elif real_method == 'POST' or real_method == 'PUT':
-        return HttpResponse(json.dumps(set_module_info(get_modulestore(location), location, request.POST)), mimetype="application/json")
+        return JsonResponse(set_module_info(get_modulestore(location), location, request.POST))
     else:
         return HttpResponseBadRequest()

cms/djangoapps/contentstore/views/course.py

@@ -11,7 +11,7 @@ from django.conf import settings
 from django.views.decorators.http import require_http_methods, require_POST
 from django.core.exceptions import PermissionDenied
 from django.core.urlresolvers import reverse
-from django.http import HttpResponse, HttpResponseBadRequest
+from django.http import HttpResponseBadRequest
 from util.json_request import JsonResponse
 from mitxmako.shortcuts import render_to_response
 
@@ -109,8 +109,9 @@ def create_new_course(request):
     try:
         dest_location = Location('i4x', org, number, 'course', Location.clean(display_name))
     except InvalidLocationError as error:
-        return HttpResponse(json.dumps({'ErrMsg': "Unable to create course '" +
-                                        display_name + "'.\n\n" + error.message}))
+        return JsonResponse({
+            "ErrMsg": "Unable to create course '{name}'.\n\n{err}".format(
+                name=display_name, err=error.message)})
 
     # see if the course already exists
     existing_course = None
@@ -120,13 +121,13 @@ def create_new_course(request):
         pass
 
     if existing_course is not None:
-        return HttpResponse(json.dumps({'ErrMsg': 'There is already a course defined with this name.'}))
+        return JsonResponse({'ErrMsg': 'There is already a course defined with this name.'})
 
     course_search_location = ['i4x', dest_location.org, dest_location.course, 'course', None]
     courses = modulestore().get_items(course_search_location)
 
     if len(courses) > 0:
-        return HttpResponse(json.dumps({'ErrMsg': 'There is already a course defined with the same organization and course number.'}))
+        return JsonResponse({'ErrMsg': 'There is already a course defined with the same organization and course number.'})
 
     new_course = modulestore('direct').clone_item(template, dest_location)
 
@@ -149,7 +150,7 @@ def create_new_course(request):
     # seed the forums
     seed_permissions_roles(new_course.location.course_id)
 
-    return HttpResponse(json.dumps({'id': new_course.location.url()}))
+    return JsonResponse({'id': new_course.location.url()})
 
 
 @login_required
@@ -201,19 +202,16 @@ def course_info_updates(request, org, course, provided_id=None):
     real_method = get_request_method(request)
 
     if request.method == 'GET':
-        return HttpResponse(json.dumps(get_course_updates(location)),
-                            mimetype="application/json")
+        return JsonResponse(get_course_updates(location))
     elif real_method == 'DELETE':
         try:
-            return HttpResponse(json.dumps(delete_course_update(location,
-                                request.POST, provided_id)), mimetype="application/json")
+            return JsonResponse(delete_course_update(location, request.POST, provided_id))
         except:
             return HttpResponseBadRequest("Failed to delete",
                                           content_type="text/plain")
     elif request.method == 'POST':
         try:
-            return HttpResponse(json.dumps(update_course_updates(location,
-                                request.POST, provided_id)), mimetype="application/json")
+            return JsonResponse(update_course_updates(location, request.POST, provided_id))
         except:
             return HttpResponseBadRequest("Failed to save",
                                           content_type="text/plain")
@@ -304,11 +302,9 @@ def course_settings_updates(request, org, course, name, section):
 
     if request.method == 'GET':
         # Cannot just do a get w/o knowing the course name :-(
-        return HttpResponse(json.dumps(manager.fetch(Location(['i4x', org, course, 'course', name])), cls=CourseSettingsEncoder),
-                            mimetype="application/json")
+        return JsonResponse(manager.fetch(Location(['i4x', org, course, 'course', name])), encoder=CourseSettingsEncoder)
     elif request.method == 'POST':  # post or put, doesn't matter.
-        return HttpResponse(json.dumps(manager.update_from_json(request.POST), cls=CourseSettingsEncoder),
-                            mimetype="application/json")
+        return JsonResponse(manager.update_from_json(request.POST), encoder=CourseSettingsEncoder)
 
 
 @expect_json
@@ -328,15 +324,13 @@ def course_grader_updates(request, org, course, name, grader_index=None):
 
     if real_method == 'GET':
         # Cannot just do a get w/o knowing the course name :-(
-        return HttpResponse(json.dumps(CourseGradingModel.fetch_grader(Location(location), grader_index)),
-                            mimetype="application/json")
+        return JsonResponse(CourseGradingModel.fetch_grader(Location(location), grader_index))
     elif real_method == "DELETE":
         # ??? Should this return anything? Perhaps success fail?
         CourseGradingModel.delete_grader(Location(location), grader_index)
-        return HttpResponse()
+        return JsonResponse()
     elif request.method == 'POST':  # post or put, doesn't matter.
-        return HttpResponse(json.dumps(CourseGradingModel.update_grader_from_json(Location(location), request.POST)),
-                            mimetype="application/json")
+        return JsonResponse(CourseGradingModel.update_grader_from_json(Location(location), request.POST))
 
 
 # # NB: expect_json failed on ["key", "key2"] and json payload
@@ -354,12 +348,9 @@ def course_advanced_updates(request, org, course, name):
     real_method = get_request_method(request)
 
     if real_method == 'GET':
-        return HttpResponse(json.dumps(CourseMetadata.fetch(location)),
-                            mimetype="application/json")
+        return JsonResponse(CourseMetadata.fetch(location))
     elif real_method == 'DELETE':
-        return HttpResponse(json.dumps(CourseMetadata.delete_key(location,
-                                                                 json.loads(request.body))),
-                            mimetype="application/json")
+        return JsonResponse(CourseMetadata.delete_key(location, json.loads(request.body)))
     elif real_method == 'POST' or real_method == 'PUT':
         # NOTE: request.POST is messed up because expect_json
         # cloned_request.POST.copy() is creating a defective entry w/ the whole payload as the key
@@ -412,14 +403,12 @@ def course_advanced_updates(request, org, course, name):
                         # Indicate that tabs should *not* be filtered out of the metadata
                         filter_tabs = False
         try:
-            response_json = json.dumps(CourseMetadata.update_from_json(location,
-                                                                   request_body,
-                                                                   filter_tabs=filter_tabs))
-        except (TypeError, ValueError), e:
+            return JsonResponse(CourseMetadata.update_from_json(location,
+                                                                request_body,
+                                                                filter_tabs=filter_tabs))
+        except (TypeError, ValueError) as e:
             return HttpResponseBadRequest("Incorrect setting format. " + str(e), content_type="text/plain")
 
-        return HttpResponse(response_json, mimetype="application/json")
-
 
 class TextbookValidationError(Exception):
     pass

common/djangoapps/util/json_request.py

@@ -31,14 +31,16 @@ class JsonResponse(HttpResponse):
     """
     Django HttpResponse subclass that has sensible defaults for outputting JSON.
     """
-    def __init__(self, object=None, *args, **kwargs):
+    def __init__(self, object=None, status=None, encoder=DjangoJSONEncoder,
+                 *args, **kwargs):
         if object in (None, ""):
             content = ""
-            kwargs.setdefault("status", 204)
+            status = status or 204
         elif isinstance(object, QuerySet):
             content = serialize('json', object)
         else:
-            content = json.dumps(object, indent=2, cls=DjangoJSONEncoder,
-                                 ensure_ascii=False)
+            content = json.dumps(object, cls=encoder, indent=2, ensure_ascii=False)
         kwargs.setdefault("content_type", "application/json")
+        if status:
+            kwargs["status"] = status
         super(JsonResponse, self).__init__(content, *args, **kwargs)

common/djangoapps/util/tests/test_json_request.py

@@ -2,6 +2,7 @@ from django.http import HttpResponse
 from util.json_request import JsonResponse
 import json
 import unittest
+import mock
 
 
 class JsonResponseTestCase(unittest.TestCase):
@@ -33,10 +34,29 @@ class JsonResponseTestCase(unittest.TestCase):
         self.assertEqual(resp.status_code, 200)
         self.assertEqual(resp["content-type"], "application/json")
 
-    def test_set_status(self):
+    def test_set_status_kwarg(self):
         obj = {"error": "resource not found"}
         resp = JsonResponse(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 = JsonResponse(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 = JsonResponse(obj, encoder=encoder)
+        self.assertEqual(resp.status_code, 200)
+        compare = json.loads(resp.content)
+        self.assertEqual(obj, compare)
+        kwargs = dumps.call_args[1]
+        self.assertIs(kwargs["cls"], encoder)