Merge pull request #8355 from edx/clee/discussion_api_delete_thread

MA-661: Clee/discussion api delete thread

lms/djangoapps/discussion_api/api.py

@@ -9,6 +9,8 @@ from django.core.exceptions import ValidationError
 from django.core.urlresolvers import reverse
 from django.http import Http404
 
+from rest_framework.exceptions import PermissionDenied
+
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.locator import CourseKey
 
@@ -72,6 +74,19 @@ def _get_thread_and_context(request, thread_id, parent_id=None, retrieve_kwargs=
         raise Http404
 
 
+def _is_user_author_or_privileged(cc_thread, context):
+    """
+    Check if the user is the author of a thread or a privileged user.
+
+    Returns:
+        Boolean
+    """
+    return (
+        context["is_requester_privileged"] or
+        context["cc_requester"]["id"] == cc_thread["user_id"]
+    )
+
+
 def get_thread_list_url(request, course_key, topic_id_list):
     """
     Returns the URL for the thread_list_url field, given a list of topic_ids
@@ -383,8 +398,7 @@ def _get_thread_editable_fields(cc_thread, context):
     """
     Get the list of editable fields for the given thread in the given context
     """
-    is_author = context["cc_requester"]["id"] == cc_thread["user_id"]
-    if context["is_requester_privileged"] or is_author:
+    if _is_user_author_or_privileged(cc_thread, context):
         return _THREAD_EDITABLE_BY_AUTHOR
     else:
         return _THREAD_EDITABLE_BY_ANY
@@ -427,3 +441,26 @@ def update_thread(request, thread_id, update_data):
     api_thread = serializer.data
     _do_extra_thread_actions(api_thread, cc_thread, update_data.keys(), actions_form, context)
     return api_thread
+
+
+def delete_thread(request, thread_id):
+    """
+    Delete a thread.
+
+    Parameters:
+
+        request: The django request object used for build_absolute_uri and
+          determining the requesting user.
+
+        thread_id: The id for the thread to delete
+
+    Raises:
+
+        PermissionDenied: if user does not have permission to delete thread
+
+    """
+    cc_thread, context = _get_thread_and_context(request, thread_id)
+    if _is_user_author_or_privileged(cc_thread, context):
+        cc_thread.delete()
+    else:
+        raise PermissionDenied

lms/djangoapps/discussion_api/tests/test_api.py

@@ -15,12 +15,15 @@ from django.core.exceptions import ValidationError
 from django.http import Http404
 from django.test.client import RequestFactory
 
+from rest_framework.exceptions import PermissionDenied
+
 from opaque_keys.edx.locator import CourseLocator
 
 from courseware.tests.factories import BetaTesterFactory, StaffFactory
 from discussion_api.api import (
     create_comment,
     create_thread,
+    delete_thread,
     get_comment_list,
     get_course_topics,
     get_thread_list,
@@ -1474,7 +1477,7 @@ class UpdateThreadTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTestC
         with self.assertRaises(Http404):
             update_thread(self.request, "test_thread", {})
 
-    def test_unenrolled(self):
+    def test_not_enrolled(self):
         self.register_thread()
         self.request.user = UserFactory.create()
         with self.assertRaises(Http404):
@@ -1633,3 +1636,130 @@ class UpdateThreadTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTestC
             assertion.exception.message_dict,
             {"raw_body": ["This field is required."]}
         )
+
+
+@ddt.ddt
+class DeleteThreadTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTestCase):
+    """Tests for delete_thread"""
+    @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
+    def setUp(self):
+        super(DeleteThreadTest, self).setUp()
+        httpretty.reset()
+        httpretty.enable()
+        self.addCleanup(httpretty.disable)
+        self.user = UserFactory.create()
+        self.register_get_user_response(self.user)
+        self.request = RequestFactory().get("/test_path")
+        self.request.user = self.user
+        self.course = CourseFactory.create()
+        self.thread_id = "test_thread"
+        CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id)
+
+    def register_thread(self, overrides=None):
+        """
+        Make a thread with appropriate data overridden by the overrides
+        parameter and register mock responses for both GET and DELETE on its
+        endpoint.
+        """
+        cs_data = make_minimal_cs_thread({
+            "id": self.thread_id,
+            "course_id": unicode(self.course.id),
+            "user_id": str(self.user.id),
+        })
+        cs_data.update(overrides or {})
+        self.register_get_thread_response(cs_data)
+        self.register_delete_thread_response(cs_data["id"])
+
+    def test_basic(self):
+        self.register_thread()
+        self.assertIsNone(delete_thread(self.request, self.thread_id))
+        self.assertEqual(
+            urlparse(httpretty.last_request().path).path,
+            "/api/v1/threads/{}".format(self.thread_id)
+        )
+        self.assertEqual(httpretty.last_request().method, "DELETE")
+
+    def test_thread_id_not_found(self):
+        self.register_get_thread_error_response("missing_thread", 404)
+        with self.assertRaises(Http404):
+            delete_thread(self.request, "missing_thread")
+
+    def test_nonexistent_course(self):
+        self.register_thread({"course_id": "non/existent/course"})
+        with self.assertRaises(Http404):
+            delete_thread(self.request, self.thread_id)
+
+    def test_not_enrolled(self):
+        self.register_thread()
+        self.request.user = UserFactory.create()
+        with self.assertRaises(Http404):
+            delete_thread(self.request, self.thread_id)
+
+    def test_discussions_disabled(self):
+        self.register_thread()
+        _remove_discussion_tab(self.course, self.user.id)
+        with self.assertRaises(Http404):
+            delete_thread(self.request, self.thread_id)
+
+    @ddt.data(
+        FORUM_ROLE_ADMINISTRATOR,
+        FORUM_ROLE_MODERATOR,
+        FORUM_ROLE_COMMUNITY_TA,
+        FORUM_ROLE_STUDENT,
+    )
+    def test_non_author_delete_allowed(self, role_name):
+        role = Role.objects.create(name=role_name, course_id=self.course.id)
+        role.users = [self.user]
+        self.register_thread({"user_id": str(self.user.id + 1)})
+        expected_error = role_name == FORUM_ROLE_STUDENT
+        try:
+            delete_thread(self.request, "test_thread")
+            self.assertFalse(expected_error)
+        except PermissionDenied:
+            self.assertTrue(expected_error)
+
+    @ddt.data(
+        *itertools.product(
+            [
+                FORUM_ROLE_ADMINISTRATOR,
+                FORUM_ROLE_MODERATOR,
+                FORUM_ROLE_COMMUNITY_TA,
+                FORUM_ROLE_STUDENT,
+            ],
+            [True, False],
+            ["no_group", "match_group", "different_group"],
+        )
+    )
+    @ddt.unpack
+    def test_group_access(self, role_name, course_is_cohorted, thread_group_state):
+        """
+        Tests group access for deleting a thread
+
+        All privileged roles are able to delete a thread. A student role can
+        only delete a thread if,
+        the student role is the author and the thread is not in a cohort,
+        the student role is the author and the thread is in the author's cohort.
+        """
+        cohort_course = CourseFactory.create(cohort_config={"cohorted": course_is_cohorted})
+        CourseEnrollmentFactory.create(user=self.user, course_id=cohort_course.id)
+        cohort = CohortFactory.create(course_id=cohort_course.id, users=[self.user])
+        role = Role.objects.create(name=role_name, course_id=cohort_course.id)
+        role.users = [self.user]
+        self.register_thread({
+            "course_id": unicode(cohort_course.id),
+            "group_id": (
+                None if thread_group_state == "no_group" else
+                cohort.id if thread_group_state == "match_group" else
+                cohort.id + 1
+            ),
+        })
+        expected_error = (
+            role_name == FORUM_ROLE_STUDENT and
+            course_is_cohorted and
+            thread_group_state == "different_group"
+        )
+        try:
+            delete_thread(self.request, "test_thread")
+            self.assertFalse(expected_error)
+        except Http404:
+            self.assertTrue(expected_error)

lms/djangoapps/discussion_api/tests/test_views.py

@@ -394,6 +394,39 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest
 
 
 @httpretty.activate
+class ThreadViewSetDeleteTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
+    """Tests for ThreadViewSet delete"""
+    def setUp(self):
+        super(ThreadViewSetDeleteTest, self).setUp()
+        self.url = reverse("thread-detail", kwargs={"thread_id": "test_thread"})
+        self.thread_id = "test_thread"
+
+    def test_basic(self):
+        self.register_get_user_response(self.user)
+        cs_thread = make_minimal_cs_thread({
+            "id": self.thread_id,
+            "course_id": unicode(self.course.id),
+            "username": self.user.username,
+            "user_id": str(self.user.id),
+        })
+        self.register_get_thread_response(cs_thread)
+        self.register_delete_thread_response(self.thread_id)
+        response = self.client.delete(self.url)
+        self.assertEqual(response.status_code, 204)
+        self.assertEqual(response.content, "")
+        self.assertEqual(
+            urlparse(httpretty.last_request().path).path,
+            "/api/v1/threads/{}".format(self.thread_id)
+        )
+        self.assertEqual(httpretty.last_request().method, "DELETE")
+
+    def test_delete_nonexistent_thread(self):
+        self.register_get_thread_error_response(self.thread_id, 404)
+        response = self.client.delete(self.url)
+        self.assertEqual(response.status_code, 404)
+
+
+@httpretty.activate
 class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
     """Tests for CommentViewSet list"""
     def setUp(self):

lms/djangoapps/discussion_api/tests/utils.py

@@ -173,6 +173,17 @@ class CommentsServiceMockMixin(object):
                 status=200
             )
 
+    def register_delete_thread_response(self, thread_id):
+        """
+        Register a mock response for DELETE on the CS thread instance endpoint
+        """
+        httpretty.register_uri(
+            httpretty.DELETE,
+            "http://localhost:4567/api/v1/threads/{id}".format(id=thread_id),
+            body=json.dumps({}),  # body is unused
+            status=200
+        )
+
     def assert_query_params_equal(self, httpretty_request, expected_params):
         """
         Assert that the given mock request had the expected query parameters

lms/djangoapps/discussion_api/views.py

@@ -14,6 +14,7 @@ from opaque_keys.edx.locator import CourseLocator
 from discussion_api.api import (
     create_comment,
     create_thread,
+    delete_thread,
     get_comment_list,
     get_course_topics,
     get_thread_list,
@@ -69,7 +70,7 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
     **Use Cases**
 
         Retrieve the list of threads for a course, post a new thread, or modify
-        an existing thread.
+        or delete an existing thread.
 
     **Example Requests**:
 
@@ -87,6 +88,8 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
         PATCH /api/discussion/v1/threads/thread_id
         {"raw_body": "Edited text"}
 
+        DELETE /api/discussion/v1/threads/thread_id
+
     **GET Parameters**:
 
         * course_id (required): The course to retrieve threads for
@@ -157,6 +160,10 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
             that were created or updated since the last time the user read
             the thread
 
+    **DELETE response values:
+
+        No content is returned for a DELETE request
+
     """
     lookup_field = "thread_id"
 
@@ -192,6 +199,14 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
         """
         return Response(update_thread(request, thread_id, request.DATA))
 
+    def destroy(self, request, thread_id):
+        """
+        Implements the DELETE method for the instance endpoint as described in
+        the class docstring
+        """
+        delete_thread(request, thread_id)
+        return Response(status=204)
+
 
 class CommentViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
     """