Commit 30884d19 by christopher lee

Added delete thread in discussion api

parent 77395bdf
...@@ -9,6 +9,8 @@ from django.core.exceptions import ValidationError ...@@ -9,6 +9,8 @@ from django.core.exceptions import ValidationError
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.http import Http404 from django.http import Http404
from rest_framework.exceptions import PermissionDenied
from opaque_keys import InvalidKeyError from opaque_keys import InvalidKeyError
from opaque_keys.edx.locator import CourseKey from opaque_keys.edx.locator import CourseKey
...@@ -72,6 +74,19 @@ def _get_thread_and_context(request, thread_id, parent_id=None, retrieve_kwargs= ...@@ -72,6 +74,19 @@ def _get_thread_and_context(request, thread_id, parent_id=None, retrieve_kwargs=
raise Http404 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): 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 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): ...@@ -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 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 _is_user_author_or_privileged(cc_thread, context):
if context["is_requester_privileged"] or is_author:
return _THREAD_EDITABLE_BY_AUTHOR return _THREAD_EDITABLE_BY_AUTHOR
else: else:
return _THREAD_EDITABLE_BY_ANY return _THREAD_EDITABLE_BY_ANY
...@@ -427,3 +441,26 @@ def update_thread(request, thread_id, update_data): ...@@ -427,3 +441,26 @@ def update_thread(request, thread_id, update_data):
api_thread = serializer.data api_thread = serializer.data
_do_extra_thread_actions(api_thread, cc_thread, update_data.keys(), actions_form, context) _do_extra_thread_actions(api_thread, cc_thread, update_data.keys(), actions_form, context)
return api_thread 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
...@@ -15,12 +15,15 @@ from django.core.exceptions import ValidationError ...@@ -15,12 +15,15 @@ from django.core.exceptions import ValidationError
from django.http import Http404 from django.http import Http404
from django.test.client import RequestFactory from django.test.client import RequestFactory
from rest_framework.exceptions import PermissionDenied
from opaque_keys.edx.locator import CourseLocator from opaque_keys.edx.locator import CourseLocator
from courseware.tests.factories import BetaTesterFactory, StaffFactory from courseware.tests.factories import BetaTesterFactory, StaffFactory
from discussion_api.api import ( from discussion_api.api import (
create_comment, create_comment,
create_thread, create_thread,
delete_thread,
get_comment_list, get_comment_list,
get_course_topics, get_course_topics,
get_thread_list, get_thread_list,
...@@ -1474,7 +1477,7 @@ class UpdateThreadTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTestC ...@@ -1474,7 +1477,7 @@ class UpdateThreadTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTestC
with self.assertRaises(Http404): with self.assertRaises(Http404):
update_thread(self.request, "test_thread", {}) update_thread(self.request, "test_thread", {})
def test_unenrolled(self): def test_not_enrolled(self):
self.register_thread() self.register_thread()
self.request.user = UserFactory.create() self.request.user = UserFactory.create()
with self.assertRaises(Http404): with self.assertRaises(Http404):
...@@ -1633,3 +1636,130 @@ class UpdateThreadTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTestC ...@@ -1633,3 +1636,130 @@ class UpdateThreadTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTestC
assertion.exception.message_dict, assertion.exception.message_dict,
{"raw_body": ["This field is required."]} {"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)
...@@ -394,6 +394,39 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest ...@@ -394,6 +394,39 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest
@httpretty.activate @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): class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
"""Tests for CommentViewSet list""" """Tests for CommentViewSet list"""
def setUp(self): def setUp(self):
......
...@@ -173,6 +173,17 @@ class CommentsServiceMockMixin(object): ...@@ -173,6 +173,17 @@ class CommentsServiceMockMixin(object):
status=200 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): def assert_query_params_equal(self, httpretty_request, expected_params):
""" """
Assert that the given mock request had the expected query parameters Assert that the given mock request had the expected query parameters
......
...@@ -14,6 +14,7 @@ from opaque_keys.edx.locator import CourseLocator ...@@ -14,6 +14,7 @@ from opaque_keys.edx.locator import CourseLocator
from discussion_api.api import ( from discussion_api.api import (
create_comment, create_comment,
create_thread, create_thread,
delete_thread,
get_comment_list, get_comment_list,
get_course_topics, get_course_topics,
get_thread_list, get_thread_list,
...@@ -69,7 +70,7 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet): ...@@ -69,7 +70,7 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
**Use Cases** **Use Cases**
Retrieve the list of threads for a course, post a new thread, or modify Retrieve the list of threads for a course, post a new thread, or modify
an existing thread. or delete an existing thread.
**Example Requests**: **Example Requests**:
...@@ -87,6 +88,8 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet): ...@@ -87,6 +88,8 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
PATCH /api/discussion/v1/threads/thread_id PATCH /api/discussion/v1/threads/thread_id
{"raw_body": "Edited text"} {"raw_body": "Edited text"}
DELETE /api/discussion/v1/threads/thread_id
**GET Parameters**: **GET Parameters**:
* course_id (required): The course to retrieve threads for * course_id (required): The course to retrieve threads for
...@@ -157,6 +160,10 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet): ...@@ -157,6 +160,10 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
that were created or updated since the last time the user read that were created or updated since the last time the user read
the thread the thread
**DELETE response values:
No content is returned for a DELETE request
""" """
lookup_field = "thread_id" lookup_field = "thread_id"
...@@ -192,6 +199,14 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet): ...@@ -192,6 +199,14 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
""" """
return Response(update_thread(request, thread_id, request.DATA)) 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): class CommentViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
""" """
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment