Merge pull request #575 from edx/db/integrate-jsonresponse

Integrate JsonResponse into lms/djangoapps/instructor/views/api.py

lms/djangoapps/instructor/tests/test_api.py

 """
 Unit tests for instructor.api methods.
 """
-
+# pylint: disable=E1111
+import unittest
 import json
 from urllib import quote
 from django.test import TestCase
@@ -9,6 +10,7 @@ from nose.tools import raises
 from mock import Mock
 from django.test.utils import override_settings
 from django.core.urlresolvers import reverse
+from django.http import HttpRequest, HttpResponse
 
 from django.contrib.auth.models import User
 from courseware.tests.modulestore_config import TEST_DATA_MONGO_MODULESTORE
@@ -21,7 +23,66 @@ from student.models import CourseEnrollment
 from courseware.models import StudentModule
 
 from instructor.access import allow_access
-from instructor.views.api import _split_input_list, _msk_from_problem_urlname
+from instructor.views.api import (
+    _split_input_list, _msk_from_problem_urlname, common_exceptions_400)
+from instructor_task.api_helper import AlreadyRunningError
+
+
+@common_exceptions_400
+def view_success(request):  # pylint: disable=W0613
+    "A dummy view for testing that returns a simple HTTP response"
+    return HttpResponse('success')
+
+
+@common_exceptions_400
+def view_user_doesnotexist(request):  # pylint: disable=W0613
+    "A dummy view that raises a User.DoesNotExist exception"
+    raise User.DoesNotExist()
+
+
+@common_exceptions_400
+def view_alreadyrunningerror(request):  # pylint: disable=W0613
+    "A dummy view that raises an AlreadyRunningError exception"
+    raise AlreadyRunningError()
+
+
+class TestCommonExceptions400(unittest.TestCase):
+    """
+    Testing the common_exceptions_400 decorator.
+    """
+    def setUp(self):
+        self.request = Mock(spec=HttpRequest)
+        self.request.META = {}
+
+    def test_happy_path(self):
+        resp = view_success(self.request)
+        self.assertEqual(resp.status_code, 200)
+
+    def test_user_doesnotexist(self):
+        self.request.is_ajax.return_value = False
+        resp = view_user_doesnotexist(self.request)
+        self.assertEqual(resp.status_code, 400)
+        self.assertIn("User does not exist", resp.content)
+
+    def test_user_doesnotexist_ajax(self):
+        self.request.is_ajax.return_value = True
+        resp = view_user_doesnotexist(self.request)
+        self.assertEqual(resp.status_code, 400)
+        result = json.loads(resp.content)
+        self.assertIn("User does not exist", result["error"])
+
+    def test_alreadyrunningerror(self):
+        self.request.is_ajax.return_value = False
+        resp = view_alreadyrunningerror(self.request)
+        self.assertEqual(resp.status_code, 400)
+        self.assertIn("Task is already running", resp.content)
+
+    def test_alreadyrunningerror_ajax(self):
+        self.request.is_ajax.return_value = True
+        resp = view_alreadyrunningerror(self.request)
+        self.assertEqual(resp.status_code, 400)
+        result = json.loads(resp.content)
+        self.assertIn("Task is already running", result["error"])
 
 
 @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
@@ -692,12 +753,6 @@ class TestInstructorAPITaskLists(ModuleStoreTestCase, LoginEnrollmentTestCase):
         self.assertEqual(json.loads(response.content), expected_res)
 
 
-
-    # class TestInstructorAPILevelsForums
-    # # list_forum_members
-    # # update_forum_role_membership
-
-
 class TestInstructorAPIHelpers(TestCase):
     """ Test helpers for instructor.api """
     def test_split_input_list(self):

lms/djangoapps/instructor/views/api.py

@@ -7,12 +7,13 @@ Many of these GETs may become PUTs in the future.
 """
 
 import re
-import json
 import logging
 from django_future.csrf import ensure_csrf_cookie
 from django.views.decorators.cache import cache_control
 from django.core.urlresolvers import reverse
-from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbidden
+from django.utils.translation import ugettext as _
+from django.http import HttpResponseBadRequest, HttpResponseForbidden
+from util.json_request import JsonResponse
 
 from courseware.access import has_access
 from courseware.courses import get_course_with_access, get_course_by_id
@@ -41,13 +42,23 @@ def common_exceptions_400(func):
     Catches common exceptions and renders matching 400 errors.
     (decorator without arguments)
     """
-    def wrapped(*args, **kwargs):  # pylint: disable=C0111
+    def wrapped(request, *args, **kwargs):  # pylint: disable=C0111
+        use_json = (request.is_ajax() or
+                    request.META.get("HTTP_ACCEPT", "").startswith("application/json"))
         try:
-            return func(*args, **kwargs)
+            return func(request, *args, **kwargs)
         except User.DoesNotExist:
-            return HttpResponseBadRequest("User does not exist.")
+            message = _("User does not exist.")
+            if use_json:
+                return JsonResponse({"error": message}, 400)
+            else:
+                return HttpResponseBadRequest(message)
         except AlreadyRunningError:
-            return HttpResponseBadRequest("Task already running.")
+            message = _("Task is already running.")
+            if use_json:
+                return JsonResponse({"error": message}, 400)
+            else:
+                return HttpResponseBadRequest(message)
     return wrapped
 
 
@@ -82,10 +93,7 @@ def require_query_params(*args, **kwargs):
                     error_response_data['info'][param] = extra
 
             if len(error_response_data['parameters']) > 0:
-                return HttpResponseBadRequest(
-                    json.dumps(error_response_data),
-                    mimetype="application/json",
-                )
+                return JsonResponse(error_response_data, status=400)
             else:
                 return func(*args, **kwargs)
         return wrapped
@@ -181,7 +189,7 @@ def students_update_enrollment(request, course_id):
             })
         # catch and log any exceptions
         # so that one error doesn't cause a 500.
-        except Exception as exc:
+        except Exception as exc:  # pylint: disable=W0703
             log.exception("Error while #{}ing student")
             log.exception(exc)
             results.append({
@@ -194,10 +202,7 @@ def students_update_enrollment(request, course_id):
         'results': results,
         'auto_enroll': auto_enroll,
     }
-    response = HttpResponse(
-        json.dumps(response_payload), content_type="application/json"
-    )
-    return response
+    return JsonResponse(response_payload)
 
 
 @ensure_csrf_cookie
@@ -255,10 +260,7 @@ def modify_access(request, course_id):
         'action': action,
         'success': 'yes',
     }
-    response = HttpResponse(
-        json.dumps(response_payload), content_type="application/json"
-    )
-    return response
+    return JsonResponse(response_payload)
 
 
 @ensure_csrf_cookie
@@ -308,10 +310,7 @@ def list_course_role_members(request, course_id):
             course, rolename
         )),
     }
-    response = HttpResponse(
-        json.dumps(response_payload), content_type="application/json"
-    )
-    return response
+    return JsonResponse(response_payload)
 
 
 @ensure_csrf_cookie
@@ -330,10 +329,7 @@ def get_grading_config(request, course_id):
         'course_id': course_id,
         'grading_config_summary': grading_config_summary,
     }
-    response = HttpResponse(
-        json.dumps(response_payload), content_type="application/json"
-    )
-    return response
+    return JsonResponse(response_payload)
 
 
 @ensure_csrf_cookie
@@ -362,10 +358,7 @@ def get_students_features(request, course_id, csv=False):  # pylint: disable=W06
             'queried_features': query_features,
             'available_features': available_features,
         }
-        response = HttpResponse(
-            json.dumps(response_payload), content_type="application/json"
-        )
-        return response
+        return JsonResponse(response_payload)
     else:
         header, datarows = analytics.csvs.format_dictlist(student_data, query_features)
         return analytics.csvs.create_csv_response("enrolled_profiles.csv", header, datarows)
@@ -408,19 +401,16 @@ def get_distribution(request, course_id):
     if not feature is None:
         p_dist = analytics.distributions.profile_distribution(course_id, feature)
         response_payload['feature_results'] = {
-            'feature':  p_dist.feature,
-            'feature_display_name':  p_dist.feature_display_name,
-            'data':  p_dist.data,
-            'type':  p_dist.type,
+            'feature': p_dist.feature,
+            'feature_display_name': p_dist.feature_display_name,
+            'data': p_dist.data,
+            'type': p_dist.type,
         }
 
         if p_dist.type == 'EASY_CHOICE':
             response_payload['feature_results']['choices_display_names'] = p_dist.choices_display_names
 
-    response = HttpResponse(
-        json.dumps(response_payload), content_type="application/json"
-    )
-    return response
+    return JsonResponse(response_payload)
 
 
 @ensure_csrf_cookie
@@ -449,10 +439,7 @@ def get_student_progress_url(request, course_id):
         'course_id': course_id,
         'progress_url': progress_url,
     }
-    response = HttpResponse(
-        json.dumps(response_payload), content_type="application/json"
-    )
-    return response
+    return JsonResponse(response_payload)
 
 
 @ensure_csrf_cookie
@@ -521,10 +508,7 @@ def reset_student_attempts(request, course_id):
     else:
         return HttpResponseBadRequest()
 
-    response = HttpResponse(
-        json.dumps(response_payload), content_type="application/json"
-    )
-    return response
+    return JsonResponse(response_payload)
 
 
 @ensure_csrf_cookie
@@ -572,10 +556,7 @@ def rescore_problem(request, course_id):
     else:
         return HttpResponseBadRequest()
 
-    response = HttpResponse(
-        json.dumps(response_payload), content_type="application/json"
-    )
-    return response
+    return JsonResponse(response_payload)
 
 
 @ensure_csrf_cookie
@@ -618,10 +599,7 @@ def list_instructor_tasks(request, course_id):
     response_payload = {
         'tasks': map(extract_task_features, tasks),
     }
-    response = HttpResponse(
-        json.dumps(response_payload), content_type="application/json"
-    )
-    return response
+    return JsonResponse(response_payload)
 
 
 @ensure_csrf_cookie
@@ -680,10 +658,7 @@ def list_forum_members(request, course_id):
         'course_id': course_id,
         rolename: map(extract_user_info, users),
     }
-    response = HttpResponse(
-        json.dumps(response_payload), content_type="application/json"
-    )
-    return response
+    return JsonResponse(response_payload)
 
 
 @ensure_csrf_cookie
@@ -747,10 +722,7 @@ def update_forum_role_membership(request, course_id):
         'course_id': course_id,
         'action': action,
     }
-    response = HttpResponse(
-        json.dumps(response_payload), content_type="application/json"
-    )
-    return response
+    return JsonResponse(response_payload)
 
 
 def _split_input_list(str_list):
@@ -782,6 +754,6 @@ def _msk_from_problem_urlname(course_id, urlname):
 
     urlname = "problem/" + urlname
 
-    (org, course_name, _) = course_id.split("/")
+    (org, course_name, __) = course_id.split("/")
     module_state_key = "i4x://" + org + "/" + course_name + "/" + urlname
     return module_state_key