From 4ea70aeb197470cf06f6ab7654a86adf687d91fc Mon Sep 17 00:00:00 2001 From: wajeeha-khalid Date: Tue, 10 Nov 2015 18:28:16 +0500 Subject: [PATCH] MA-862; accept application/merge-patch+json for comment/thread update --- common/djangoapps/util/testing.py | 18 ++++++++++++++++++ .../discussion_api/tests/test_views.py | 17 ++++++++++------- lms/djangoapps/discussion_api/views.py | 17 +++++++++++++++++ 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/common/djangoapps/util/testing.py b/common/djangoapps/util/testing.py index 72a94a93cd..a14a4022ed 100644 --- a/common/djangoapps/util/testing.py +++ b/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) diff --git a/lms/djangoapps/discussion_api/tests/test_views.py b/lms/djangoapps/discussion_api/tests/test_views.py index 8a64d3582f..31737d4ae0 100644 --- a/lms/djangoapps/discussion_api/tests/test_views.py +++ b/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."}} diff --git a/lms/djangoapps/discussion_api/views.py b/lms/djangoapps/discussion_api/views.py index 612010c448..516f57686f 100644 --- a/lms/djangoapps/discussion_api/views.py +++ b/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))