Make forums endpoints return better status codes
Previously, AJAX calls would return 400, and page views and attempts to load inline discussions would return 404 if communication with the comments service failed. Now such problems cause a 500 status code.
This commit is contained in:
@@ -65,7 +65,7 @@ class ViewsExceptionTestCase(UrlResetMixin, ModuleStoreTestCase):
|
||||
self.assertEqual(self.response.status_code, 404)
|
||||
|
||||
@patch('student.models.cc.User.from_django_user')
|
||||
@patch('student.models.cc.User.active_threads')
|
||||
@patch('student.models.cc.User.subscribed_threads')
|
||||
def test_user_followed_threads_exception(self, mock_threads, mock_from_django_user):
|
||||
|
||||
# Mock the code that makes the HTTP requests to the cs_comment_service app
|
||||
|
||||
@@ -115,11 +115,8 @@ def inline_discussion(request, course_id, discussion_id):
|
||||
cc_user = cc.User.from_django_user(request.user)
|
||||
user_info = cc_user.to_dict()
|
||||
except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError):
|
||||
# TODO (vshnayder): since none of this code seems to be aware of the fact that
|
||||
# sometimes things go wrong, I suspect that the js client is also not
|
||||
# checking for errors on request. Check and fix as needed.
|
||||
log.error("Error loading inline discussion threads.")
|
||||
raise Http404
|
||||
raise
|
||||
|
||||
annotated_content_info = utils.get_metadata_for_threads(course_id, threads, request.user, user_info)
|
||||
|
||||
@@ -180,7 +177,7 @@ def forum_form_discussion(request, course_id):
|
||||
return render_to_response('discussion/maintenance.html', {})
|
||||
except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError) as err:
|
||||
log.error("Error loading forum discussion threads: %s", str(err))
|
||||
raise Http404
|
||||
raise
|
||||
|
||||
user = cc.User.from_django_user(request.user)
|
||||
user_info = user.to_dict()
|
||||
@@ -247,7 +244,7 @@ def single_thread(request, course_id, discussion_id, thread_id):
|
||||
thread = cc.Thread.find(thread_id).retrieve(recursive=True, user_id=request.user.id)
|
||||
except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError):
|
||||
log.error("Error loading single thread.")
|
||||
raise Http404
|
||||
raise
|
||||
|
||||
if request.is_ajax():
|
||||
courseware_context = get_courseware_context(thread, course)
|
||||
@@ -272,7 +269,7 @@ def single_thread(request, course_id, discussion_id, thread_id):
|
||||
threads.append(thread.to_dict())
|
||||
except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError):
|
||||
log.error("Error loading single thread.")
|
||||
raise Http404
|
||||
raise
|
||||
|
||||
course = get_course_with_access(request.user, course_id, 'load_forum')
|
||||
|
||||
@@ -370,7 +367,7 @@ def user_profile(request, course_id, user_id):
|
||||
}
|
||||
|
||||
return render_to_response('discussion/user_profile.html', context)
|
||||
except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError, User.DoesNotExist):
|
||||
except User.DoesNotExist:
|
||||
raise Http404
|
||||
|
||||
|
||||
@@ -413,5 +410,5 @@ def followed_threads(request, course_id, user_id):
|
||||
}
|
||||
|
||||
return render_to_response('discussion/user_profile.html', context)
|
||||
except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError, User.DoesNotExist):
|
||||
except User.DoesNotExist:
|
||||
raise Http404
|
||||
|
||||
@@ -18,7 +18,7 @@ class AjaxExceptionMiddleware(object):
|
||||
"""
|
||||
if isinstance(exception, CommentClientError) and request.is_ajax():
|
||||
try:
|
||||
return JsonError(json.loads(exception.message))
|
||||
return JsonError(json.loads(exception.message), 500)
|
||||
except ValueError:
|
||||
return JsonError(exception.message)
|
||||
return JsonError(exception.message, 500)
|
||||
return None
|
||||
|
||||
@@ -20,8 +20,14 @@ class AjaxExceptionTestCase(TestCase):
|
||||
self.request0.META['HTTP_X_REQUESTED_WITH'] = "SHADOWFAX"
|
||||
|
||||
def test_process_exception(self):
|
||||
self.assertIsInstance(self.a.process_exception(self.request1, self.exception1), middleware.JsonError)
|
||||
self.assertIsInstance(self.a.process_exception(self.request1, self.exception2), middleware.JsonError)
|
||||
response1 = self.a.process_exception(self.request1, self.exception1)
|
||||
self.assertIsInstance(response1, middleware.JsonError)
|
||||
self.assertEqual(500, response1.status_code)
|
||||
|
||||
response2 = self.a.process_exception(self.request1, self.exception2)
|
||||
self.assertIsInstance(response2, middleware.JsonError)
|
||||
self.assertEqual(500, response2.status_code)
|
||||
|
||||
self.assertIsNone(self.a.process_exception(self.request1, self.exception0))
|
||||
self.assertIsNone(self.a.process_exception(self.request0, self.exception1))
|
||||
self.assertIsNone(self.a.process_exception(self.request0, self.exception0))
|
||||
|
||||
Reference in New Issue
Block a user