From 52051df4d44b72b5ba1ce61f9434b4135c9b52a7 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Thu, 16 Oct 2014 14:03:41 -0400 Subject: [PATCH] Fix some bugs in the student view shim, add tests --- common/djangoapps/user_api/helpers.py | 6 +- .../djangoapps/user_api/tests/test_helpers.py | 123 +++++++++++++++++- 2 files changed, 126 insertions(+), 3 deletions(-) diff --git a/common/djangoapps/user_api/helpers.py b/common/djangoapps/user_api/helpers.py index bd9752eb15..0621278074 100644 --- a/common/djangoapps/user_api/helpers.py +++ b/common/djangoapps/user_api/helpers.py @@ -300,9 +300,11 @@ def shim_student_view(view_func, check_logged_in=False): try: response_dict = json.loads(response.content) msg = response_dict.get("value", u"") - redirect_url = response_dict.get("redirect_url") + redirect_url = response_dict.get("redirect_url") or response_dict.get("redirect") + success = response_dict.get("success") except (ValueError, TypeError): msg = response.content + success = True redirect_url = None # If the user is not authenticated, and we expect them to be @@ -317,7 +319,7 @@ def shim_student_view(view_func, check_logged_in=False): response.content = redirect_url # If an error condition occurs, send a status 400 - elif response.status_code != 200 or not response_dict.get("success", False): + elif response.status_code != 200 or not success: # The student views tend to send status 200 even when an error occurs # If the JSON-serialized content has a value "success" set to False, # then we know an error occurred. diff --git a/common/djangoapps/user_api/tests/test_helpers.py b/common/djangoapps/user_api/tests/test_helpers.py index 7daf5c2a4b..49ddff9dfa 100644 --- a/common/djangoapps/user_api/tests/test_helpers.py +++ b/common/djangoapps/user_api/tests/test_helpers.py @@ -1,10 +1,16 @@ """ Tests for helper functions. """ +import json import mock +import ddt from django.test import TestCase from nose.tools import raises -from user_api.helpers import intercept_errors +from django.http import HttpRequest, HttpResponse +from user_api.helpers import ( + intercept_errors, shim_student_view, + FormDescription, InvalidFieldError +) class FakeInputException(Exception): @@ -64,3 +70,118 @@ class InterceptErrorsTest(TestCase): # This will include the stack trace for the original exception # because it's called with log level "ERROR" mock_logger.exception.assert_called_once_with(expected_log_msg) + + +class FormDescriptionTest(TestCase): + + def test_to_json(self): + desc = FormDescription("post", "/submit") + desc.add_field( + "name", + label="label", + field_type="text", + default="default", + placeholder="placeholder", + instructions="instructions", + required=True, + restrictions={ + "min_length": 2, + "max_length": 10 + } + ) + + self.assertEqual(desc.to_json(), json.dumps({ + "method": "post", + "submit_url": "/submit", + "fields": [ + { + "name": "name", + "label": "label", + "type": "text", + "default": "default", + "placeholder": "placeholder", + "instructions": "instructions", + "required": True, + "restrictions": { + "min_length": 2, + "max_length": 10, + } + } + ] + })) + + def test_invalid_field_type(self): + desc = FormDescription("post", "/submit") + with self.assertRaises(InvalidFieldError): + desc.add_field("invalid", field_type="invalid") + + def test_missing_options(self): + desc = FormDescription("post", "/submit") + with self.assertRaises(InvalidFieldError): + desc.add_field("name", field_type="select") + + def test_invalid_restriction(self): + desc = FormDescription("post", "/submit") + with self.assertRaises(InvalidFieldError): + desc.add_field("name", field_type="text", restrictions={"invalid": 0}) + + +@ddt.ddt +class StudentViewShimTest(TestCase): + + def setUp(self): + self.captured_request = None + + def test_strip_enrollment_action(self): + view = self._shimmed_view(HttpResponse()) + request = HttpRequest() + request.POST["enrollment_action"] = "enroll" + request.POST["course_id"] = "edx/101/demo" + view(request) + + # Expect that the enrollment action and course ID + # were stripped out before reaching the wrapped view. + self.assertNotIn("enrollment_action", self.captured_request.POST) + self.assertNotIn("course_id", self.captured_request.POST) + + def test_non_json_response(self): + view = self._shimmed_view(HttpResponse(content="Not a JSON dict")) + response = view(HttpRequest()) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.content, "Not a JSON dict") + + @ddt.data("redirect", "redirect_url") + def test_redirect_from_json(self, redirect_key): + view = self._shimmed_view( + HttpResponse(content=json.dumps({ + "success": True, + redirect_key: "/redirect" + })) + ) + response = view(HttpRequest()) + self.assertEqual(response.status_code, 302) + self.assertEqual(response.content, "/redirect") + + def test_error_from_json(self): + view = self._shimmed_view( + HttpResponse(content=json.dumps({ + "success": False, + "value": "Error!" + })) + ) + response = view(HttpRequest()) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.content, "Error!") + + def test_preserve_headers(self): + view_response = HttpResponse() + view_response["test-header"] = "test" + view = self._shimmed_view(view_response) + response = view(HttpRequest()) + self.assertEqual(response["test-header"], "test") + + def _shimmed_view(self, response): + def stub_view(request): + self.captured_request = request + return response + return shim_student_view(stub_view)