MA-862; accept application/merge-patch+json for comment/thread update

common/djangoapps/util/testing.py

@@ -1,3 +1,8 @@
+"""
+Utility Mixins for unit tests
+"""
+
+import json
 import sys
 
 from mock import patch
@@ -90,3 +95,16 @@ class EventTestMixin(object):
         Reset the mock tracker in order to forget about old events.
         """
         self.mock_tracker.reset_mock()
+
+
+class PatchMediaTypeMixin(object):
+    """
+    Generic mixin for verifying unsupported media type in PATCH
+    """
+    def test_patch_unsupported_media_type(self):
+        response = self.client.patch(  # pylint: disable=no-member
+            self.url,
+            json.dumps({}),
+            content_type=self.unsupported_media_type
+        )
+        self.assertEqual(response.status_code, 415)

lms/djangoapps/discussion_api/tests/test_views.py

@@ -11,6 +11,7 @@ import mock
 from pytz import UTC
 
 from django.core.urlresolvers import reverse
+from rest_framework.parsers import JSONParser
 
 from rest_framework.test import APIClient
 from xmodule.modulestore import ModuleStoreEnum
@@ -24,7 +25,7 @@ from discussion_api.tests.utils import (
     make_minimal_cs_thread,
 )
 from student.tests.factories import CourseEnrollmentFactory, UserFactory
-from util.testing import UrlResetMixin
+from util.testing import UrlResetMixin, PatchMediaTypeMixin
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls, ItemFactory
 
@@ -536,9 +537,10 @@ class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
 @httpretty.activate
 @disable_signal(api, 'thread_edited')
 @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
-class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
+class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, PatchMediaTypeMixin):
     """Tests for ThreadViewSet partial_update"""
     def setUp(self):
+        self.unsupported_media_type = JSONParser.media_type
         super(ThreadViewSetPartialUpdateTest, self).setUp()
         self.url = reverse("thread-detail", kwargs={"thread_id": "test_thread"})
 
@@ -592,7 +594,7 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest
         response = self.client.patch(  # pylint: disable=no-member
             self.url,
             json.dumps(request_data),
-            content_type="application/json"
+            content_type="application/merge-patch+json"
         )
         self.assertEqual(response.status_code, 200)
         response_data = json.loads(response.content)
@@ -625,7 +627,7 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest
         response = self.client.patch(  # pylint: disable=no-member
             self.url,
             json.dumps(request_data),
-            content_type="application/json"
+            content_type="application/merge-patch+json"
         )
         expected_response_data = {
             "field_errors": {"title": {"developer_message": "This field may not be blank."}}
@@ -930,9 +932,10 @@ class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
 
 @disable_signal(api, 'comment_edited')
 @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
-class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
+class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, PatchMediaTypeMixin):
     """Tests for CommentViewSet partial_update"""
     def setUp(self):
+        self.unsupported_media_type = JSONParser.media_type
         super(CommentViewSetPartialUpdateTest, self).setUp()
         httpretty.reset()
         httpretty.enable()
@@ -982,7 +985,7 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes
         response = self.client.patch(  # pylint: disable=no-member
             self.url,
             json.dumps(request_data),
-            content_type="application/json"
+            content_type="application/merge-patch+json"
         )
         self.assertEqual(response.status_code, 200)
         response_data = json.loads(response.content)
@@ -1004,7 +1007,7 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes
         response = self.client.patch(  # pylint: disable=no-member
             self.url,
             json.dumps(request_data),
-            content_type="application/json"
+            content_type="application/merge-patch+json"
         )
         expected_response_data = {
             "field_errors": {"raw_body": {"developer_message": "This field may not be blank."}}

lms/djangoapps/discussion_api/views.py

@@ -2,6 +2,8 @@
 Discussion API views
 """
 from django.core.exceptions import ValidationError
+from rest_framework.exceptions import UnsupportedMediaType
+from rest_framework.parsers import JSONParser
 
 from rest_framework.response import Response
 from rest_framework.views import APIView
@@ -25,6 +27,7 @@ from discussion_api.api import (
     update_thread,
 )
 from discussion_api.forms import CommentListGetForm, ThreadListGetForm, _PaginationForm
+from openedx.core.lib.api.parsers import MergePatchParser
 from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes
 
 
@@ -119,6 +122,7 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet):
 
         PATCH /api/discussion/v1/threads/thread_id
         {"raw_body": "Edited text"}
+        Content Type: "application/merge-patch+json"
 
         DELETE /api/discussion/v1/threads/thread_id
 
@@ -175,6 +179,9 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet):
         topic_id, type, title, and raw_body are accepted with the same meaning
         as in a POST request
 
+        If "application/merge-patch+json" is not the specified content type,
+        a 415 error is returned.
+
     **GET Response Values**:
 
         * results: The list of threads; each item in the list has the same
@@ -232,6 +239,7 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet):
 
     """
     lookup_field = "thread_id"
+    parser_classes = (JSONParser, MergePatchParser,)
 
     def list(self, request):
         """
@@ -274,6 +282,8 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet):
         Implements the PATCH method for the instance endpoint as described in
         the class docstring.
         """
+        if request.content_type != MergePatchParser.media_type:
+            raise UnsupportedMediaType(request.content_type)
         return Response(update_thread(request, thread_id, request.data))
 
     def destroy(self, request, thread_id):
@@ -307,6 +317,7 @@ class CommentViewSet(DeveloperErrorViewMixin, ViewSet):
 
         PATCH /api/discussion/v1/comments/comment_id
         {"raw_body": "Edited text"}
+        Content Type: "application/merge-patch+json"
 
         DELETE /api/discussion/v1/comments/comment_id
 
@@ -347,6 +358,9 @@ class CommentViewSet(DeveloperErrorViewMixin, ViewSet):
 
         raw_body is accepted with the same meaning as in a POST request
 
+        If "application/merge-patch+json" is not the specified content type,
+        a 415 error is returned.
+
     **GET Response Values**:
 
         * results: The list of comments; each item in the list has the same
@@ -409,6 +423,7 @@ class CommentViewSet(DeveloperErrorViewMixin, ViewSet):
 
     """
     lookup_field = "comment_id"
+    parser_classes = (JSONParser, MergePatchParser,)
 
     def list(self, request):
         """
@@ -465,4 +480,6 @@ class CommentViewSet(DeveloperErrorViewMixin, ViewSet):
         Implements the PATCH method for the instance endpoint as described in
         the class docstring.
         """
+        if request.content_type != MergePatchParser.media_type:
+            raise UnsupportedMediaType(request.content_type)
         return Response(update_comment(request, comment_id, request.data))