Check for forum data errors in LMS

Some messages generated by the comments service are not readily
translated because they come from third-party libraries. Thus, we plan
to try to avoid showing any comments service generated message to the
user. This check preempts the only end-user-visible CS-generated error
message that we are presently aware of.

lms/djangoapps/django_comment_client/base/tests.py

@@ -107,6 +107,139 @@ class ViewsTestCase(UrlResetMixin, ModuleStoreTestCase):
         )
         assert_equal(response.status_code, 200)
 
+    def _test_request_error(self, view_name, view_kwargs, data, mock_request):
+        """
+        Submit a request against the given view with the given data and ensure
+        that the result is a 400 error and that no data was posted using
+        mock_request
+        """
+        mock_request.return_value.status_code = 200
+        # This represents the minimum fields required for views to function
+        cs_data = {
+            "user_id": str(self.student.id),
+            "closed": False,
+        }
+        if view_name == "create_sub_comment":
+            cs_data["depth"] = 0
+        mock_request.return_value.text = json.dumps(cs_data)
+
+        response = self.client.post(reverse(view_name, kwargs=view_kwargs), data=data)
+        self.assertEqual(response.status_code, 400)
+        for call in mock_request.call_args_list:
+            self.assertEqual(call[0][0].lower(), "get")
+
+    def test_create_thread_no_title(self, mock_request):
+        self._test_request_error(
+            "create_thread",
+            {"commentable_id": "dummy", "course_id": self.course_id},
+            {"body": "foo"},
+            mock_request
+        )
+
+    def test_create_thread_empty_title(self, mock_request):
+        self._test_request_error(
+            "create_thread",
+            {"commentable_id": "dummy", "course_id": self.course_id},
+            {"body": "foo", "title": " "},
+            mock_request
+        )
+
+    def test_create_thread_no_body(self, mock_request):
+        self._test_request_error(
+            "create_thread",
+            {"commentable_id": "dummy", "course_id": self.course_id},
+            {"title": "foo"},
+            mock_request
+        )
+
+    def test_create_thread_empty_body(self, mock_request):
+        self._test_request_error(
+            "create_thread",
+            {"commentable_id": "dummy", "course_id": self.course_id},
+            {"body": " ", "title": "foo"},
+            mock_request
+        )
+
+    def test_update_thread_no_title(self, mock_request):
+        self._test_request_error(
+            "update_thread",
+            {"thread_id": "dummy", "course_id": self.course_id},
+            {"body": "foo"},
+            mock_request
+        )
+
+    def test_update_thread_empty_title(self, mock_request):
+        self._test_request_error(
+            "update_thread",
+            {"thread_id": "dummy", "course_id": self.course_id},
+            {"body": "foo", "title": " "},
+            mock_request
+        )
+
+    def test_update_thread_no_body(self, mock_request):
+        self._test_request_error(
+            "update_thread",
+            {"thread_id": "dummy", "course_id": self.course_id},
+            {"title": "foo"},
+            mock_request
+        )
+
+    def test_update_thread_empty_body(self, mock_request):
+        self._test_request_error(
+            "update_thread",
+            {"thread_id": "dummy", "course_id": self.course_id},
+            {"body": " ", "title": "foo"},
+            mock_request
+        )
+
+    def test_create_comment_no_body(self, mock_request):
+        self._test_request_error(
+            "create_comment",
+            {"thread_id": "dummy", "course_id": self.course_id},
+            {},
+            mock_request
+        )
+
+    def test_create_comment_empty_body(self, mock_request):
+        self._test_request_error(
+            "create_comment",
+            {"thread_id": "dummy", "course_id": self.course_id},
+            {"body": " "},
+            mock_request
+        )
+
+    def test_create_sub_comment_no_body(self, mock_request):
+        self._test_request_error(
+            "create_sub_comment",
+            {"comment_id": "dummy", "course_id": self.course_id},
+            {},
+            mock_request
+        )
+
+    def test_create_sub_comment_empty_body(self, mock_request):
+        self._test_request_error(
+            "create_sub_comment",
+            {"comment_id": "dummy", "course_id": self.course_id},
+            {"body": " "},
+            mock_request
+        )
+
+    def test_update_comment_no_body(self, mock_request):
+        self._test_request_error(
+            "update_comment",
+            {"comment_id": "dummy", "course_id": self.course_id},
+            {},
+            mock_request
+        )
+
+    def test_update_comment_empty_body(self, mock_request):
+        self._test_request_error(
+            "update_comment",
+            {"comment_id": "dummy", "course_id": self.course_id},
+            {"body": " "},
+            mock_request
+        )
+
     def test_flag_thread(self, mock_request):
         mock_request.return_value.status_code = 200
         mock_request.return_value.text = u'{"title":"Hello",\

lms/djangoapps/django_comment_client/base/views.py

@@ -83,6 +83,11 @@ def create_thread(request, course_id, commentable_id):
     else:
         anonymous_to_peers = False
 
+    if 'title' not in post or not post['title'].strip():
+        return JsonError(_("Title can't be empty"))
+    if 'body' not in post or not post['body'].strip():
+        return JsonError(_("Body can't be empty"))
+
     thread = cc.Thread(**extract(post, ['body', 'title']))
     thread.update_attributes(**{
         'anonymous': anonymous,
@@ -139,6 +144,10 @@ def update_thread(request, course_id, thread_id):
     """
     Given a course id and thread id, update a existing thread, used for both static and ajax submissions
     """
+    if 'title' not in request.POST or not request.POST['title'].strip():
+        return JsonError(_("Title can't be empty"))
+    if 'body' not in request.POST or not request.POST['body'].strip():
+        return JsonError(_("Body can't be empty"))
     thread = cc.Thread.find(thread_id)
     thread.update_attributes(**extract(request.POST, ['body', 'title']))
     thread.save()
@@ -154,6 +163,9 @@ def _create_comment(request, course_id, thread_id=None, parent_id=None):
     called from create_comment to do the actual creation
     """
     post = request.POST
+
+    if 'body' not in post or not post['body'].strip():
+        return JsonError(_("Body can't be empty"))
     comment = cc.Comment(**extract(post, ['body']))
 
     course = get_course_with_access(request.user, course_id, 'load')
@@ -221,6 +233,8 @@ def update_comment(request, course_id, comment_id):
     handles static and ajax submissions
     """
     comment = cc.Comment.find(comment_id)
+    if 'body' not in request.POST or not request.POST['body'].strip():
+        return JsonError(_("Body can't be empty"))
     comment.update_attributes(**extract(request.POST, ['body']))
     comment.save()
     if request.is_ajax():

lms/djangoapps/django_comment_client/utils.py

@@ -205,7 +205,7 @@ class JsonResponse(HttpResponse):
 
 class JsonError(HttpResponse):
     def __init__(self, error_messages=[], status=400):
-        if isinstance(error_messages, str):
+        if isinstance(error_messages, basestring):
             error_messages = [error_messages]
         content = simplejson.dumps({'errors': error_messages},
                                    indent=2,