MA-860; No moderator permissions on closed thread

lms/djangoapps/discussion_api/api.py

@@ -601,6 +601,10 @@ def create_comment(request, comment_data):
     except Http404:
         raise ValidationError({"thread_id": ["Invalid value."]})
 
+    # if a thread is closed; no new comments could be made to it
+    if cc_thread['closed']:
+        raise PermissionDenied
+
     _check_initializable_comment_fields(comment_data, context)
     serializer = CommentSerializer(data=comment_data, context=context)
     actions_form = CommentActionsForm(comment_data)

lms/djangoapps/discussion_api/permissions.py

@@ -56,8 +56,16 @@ def get_editable_fields(cc_content, context):
     """
     Return the set of fields that the requester can edit on the given content
     """
+
+    # no edits, except 'abuse_flagged' are allowed on closed threads
+    ret = {"abuse_flagged"}
+    if (cc_content["type"] == "thread" and cc_content["closed"]) or (
+            cc_content["type"] == "comment" and context["thread"]["closed"]
+    ):
+        return ret
+
     # Shared fields
-    ret = {"abuse_flagged", "voted"}
+    ret |= {"voted"}
     if _is_author_or_privileged(cc_content, context):
         ret |= {"raw_body"}
 

lms/djangoapps/discussion_api/tests/test_api.py

@@ -614,7 +614,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto
                 "title": "Another Test Title",
                 "body": "More content",
                 "pinned": False,
-                "closed": True,
+                "closed": False,
                 "abuse_flaggers": [],
                 "votes": {"up_count": 9},
                 "comments_count": 18,
@@ -668,7 +668,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto
                 "raw_body": "More content",
                 "rendered_body": "<p>More content</p>",
                 "pinned": False,
-                "closed": True,
+                "closed": False,
                 "following": False,
                 "abuse_flagged": False,
                 "voted": False,

lms/djangoapps/discussion_api/tests/test_views.py

@@ -63,6 +63,41 @@ class DiscussionAPIViewTestMixin(CommentsServiceMockMixin, UrlResetMixin):
         parsed_content = json.loads(response.content)
         self.assertEqual(parsed_content, expected_content)
 
+    def register_thread(self, overrides=None):
+        """
+        Create cs_thread with minimal fields and register response
+        """
+        cs_thread = make_minimal_cs_thread({
+            "id": "test_thread",
+            "course_id": unicode(self.course.id),
+            "commentable_id": "original_topic",
+            "username": self.user.username,
+            "user_id": str(self.user.id),
+            "thread_type": "discussion",
+            "title": "Original Title",
+            "body": "Original body",
+        })
+        cs_thread.update(overrides or {})
+        self.register_get_thread_response(cs_thread)
+        self.register_put_thread_response(cs_thread)
+
+    def register_comment(self, overrides=None):
+        """
+        Create cs_comment with minimal fields and register response
+        """
+        cs_comment = make_minimal_cs_comment({
+            "id": "test_comment",
+            "course_id": unicode(self.course.id),
+            "thread_id": "test_thread",
+            "username": self.user.username,
+            "user_id": str(self.user.id),
+            "body": "Original body",
+        })
+        cs_comment.update(overrides or {})
+        self.register_get_comment_response(cs_comment)
+        self.register_put_comment_response(cs_comment)
+        self.register_post_comment_response(cs_comment, thread_id="test_thread")
+
     def test_not_authenticated(self):
         self.client.logout()
         response = self.client.get(self.url)
@@ -534,6 +569,7 @@ class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
         self.assertEqual(response_data, expected_response_data)
 
 
+@ddt.ddt
 @httpretty.activate
 @disable_signal(api, 'thread_edited')
 @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
@@ -544,24 +580,11 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest
         super(ThreadViewSetPartialUpdateTest, self).setUp()
         self.url = reverse("thread-detail", kwargs={"thread_id": "test_thread"})
 
-    def test_basic(self):
-        self.register_get_user_response(self.user)
-        cs_thread = make_minimal_cs_thread({
-            "id": "test_thread",
-            "course_id": unicode(self.course.id),
-            "commentable_id": "original_topic",
-            "username": self.user.username,
-            "user_id": str(self.user.id),
-            "created_at": "2015-05-29T00:00:00Z",
-            "updated_at": "2015-05-29T00:00:00Z",
-            "thread_type": "discussion",
-            "title": "Original Title",
-            "body": "Original body",
-        })
-        self.register_get_thread_response(cs_thread)
-        self.register_put_thread_response(cs_thread)
-        request_data = {"raw_body": "Edited body"}
-        expected_response_data = {
+    def expected_response_data(self, overrides=None):
+        """
+        create expected response data from comment update endpoint
+        """
+        response_data = {
             "id": "test_thread",
             "course_id": unicode(self.course.id),
             "topic_id": "original_topic",
@@ -569,12 +592,12 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest
             "group_name": None,
             "author": self.user.username,
             "author_label": None,
-            "created_at": "2015-05-29T00:00:00Z",
-            "updated_at": "2015-05-29T00:00:00Z",
+            "created_at": "1970-01-01T00:00:00Z",
+            "updated_at": "1970-01-01T00:00:00Z",
             "type": "discussion",
             "title": "Original Title",
-            "raw_body": "Edited body",
-            "rendered_body": "<p>Edited body</p>",
+            "raw_body": "Original body",
+            "rendered_body": "<p>Original body</p>",
             "pinned": False,
             "closed": False,
             "following": False,
@@ -586,19 +609,31 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest
             "comment_list_url": "http://testserver/api/discussion/v1/comments/?thread_id=test_thread",
             "endorsed_comment_list_url": None,
             "non_endorsed_comment_list_url": None,
-            "editable_fields": ["abuse_flagged", "following", "raw_body", "title", "topic_id", "type", "voted"],
+            "editable_fields": [],
             "read": False,
             "has_endorsed": False,
             "response_count": 0,
         }
-        response = self.client.patch(  # pylint: disable=no-member
-            self.url,
-            json.dumps(request_data),
-            content_type="application/merge-patch+json"
-        )
+        response_data.update(overrides or {})
+        return response_data
+
+    def test_basic(self):
+        self.register_get_user_response(self.user)
+        self.register_thread({"created_at": "Test Date", "updated_at": "Test Date"})
+        request_data = {"raw_body": "Edited body"}
+        response = self.request_patch(request_data)
         self.assertEqual(response.status_code, 200)
         response_data = json.loads(response.content)
-        self.assertEqual(response_data, expected_response_data)
+        self.assertEqual(
+            response_data,
+            self.expected_response_data({
+                "raw_body": "Edited body",
+                "rendered_body": "<p>Edited body</p>",
+                "editable_fields": ["abuse_flagged", "following", "raw_body", "title", "topic_id", "type", "voted"],
+                "created_at": "Test Date",
+                "updated_at": "Test Date",
+            })
+        )
         self.assertEqual(
             httpretty.last_request().parsed_body,
             {
@@ -617,18 +652,9 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest
 
     def test_error(self):
         self.register_get_user_response(self.user)
-        cs_thread = make_minimal_cs_thread({
-            "id": "test_thread",
-            "course_id": unicode(self.course.id),
-            "user_id": str(self.user.id),
-        })
-        self.register_get_thread_response(cs_thread)
+        self.register_thread()
         request_data = {"title": ""}
-        response = self.client.patch(  # pylint: disable=no-member
-            self.url,
-            json.dumps(request_data),
-            content_type="application/merge-patch+json"
-        )
+        response = self.request_patch(request_data)
         expected_response_data = {
             "field_errors": {"title": {"developer_message": "This field may not be blank."}}
         }
@@ -636,6 +662,42 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest
         response_data = json.loads(response.content)
         self.assertEqual(response_data, expected_response_data)
 
+    @ddt.data(
+        ("abuse_flagged", True),
+        ("abuse_flagged", False),
+    )
+    @ddt.unpack
+    def test_closed_thread(self, field, value):
+        self.register_get_user_response(self.user)
+        self.register_thread({"closed": True})
+        self.register_flag_response("thread", "test_thread")
+        request_data = {field: value}
+        response = self.request_patch(request_data)
+        self.assertEqual(response.status_code, 200)
+        response_data = json.loads(response.content)
+        self.assertEqual(
+            response_data,
+            self.expected_response_data({
+                "closed": True,
+                "abuse_flagged": value,
+                "editable_fields": ["abuse_flagged"],
+            })
+        )
+
+    @ddt.data(
+        ("raw_body", "Edited body"),
+        ("voted", True),
+        ("following", True),
+    )
+    @ddt.unpack
+    def test_closed_thread_error(self, field, value):
+        self.register_get_user_response(self.user)
+        self.register_thread({"closed": True})
+        self.register_flag_response("thread", "test_thread")
+        request_data = {field: value}
+        response = self.request_patch(request_data)
+        self.assertEqual(response.status_code, 400)
+
 
 @httpretty.activate
 @disable_signal(api, 'thread_deleted')
@@ -855,22 +917,8 @@ class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
 
     def test_basic(self):
         self.register_get_user_response(self.user)
-        self.register_get_thread_response(
-            make_minimal_cs_thread({
-                "id": "test_thread",
-                "course_id": unicode(self.course.id),
-                "commentable_id": "test_topic",
-            })
-        )
-        self.register_post_comment_response(
-            {
-                "id": "test_comment",
-                "username": self.user.username,
-                "created_at": "2015-05-27T00:00:00Z",
-                "updated_at": "2015-05-27T00:00:00Z",
-            },
-            thread_id="test_thread"
-        )
+        self.register_thread()
+        self.register_comment()
         request_data = {
             "thread_id": "test_thread",
             "raw_body": "Test body",
@@ -881,8 +929,8 @@ class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             "parent_id": None,
             "author": self.user.username,
             "author_label": None,
-            "created_at": "2015-05-27T00:00:00Z",
-            "updated_at": "2015-05-27T00:00:00Z",
+            "created_at": "1970-01-01T00:00:00Z",
+            "updated_at": "1970-01-01T00:00:00Z",
             "raw_body": "Test body",
             "rendered_body": "<p>Test body</p>",
             "endorsed": False,
@@ -929,7 +977,23 @@ class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
         response_data = json.loads(response.content)
         self.assertEqual(response_data, expected_response_data)
 
+    def test_closed_thread(self):
+        self.register_get_user_response(self.user)
+        self.register_thread({"closed": True})
+        self.register_comment()
+        request_data = {
+            "thread_id": "test_thread",
+            "raw_body": "Test body"
+        }
+        response = self.client.post(
+            self.url,
+            json.dumps(request_data),
+            content_type="application/json"
+        )
+        self.assertEqual(response.status_code, 403)
+
 
+@ddt.ddt
 @disable_signal(api, 'comment_edited')
 @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
 class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, PatchMediaTypeMixin):
@@ -942,36 +1006,21 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes
         self.addCleanup(httpretty.disable)
         self.register_get_user_response(self.user)
         self.url = reverse("comment-detail", kwargs={"comment_id": "test_comment"})
-        cs_thread = make_minimal_cs_thread({
-            "id": "test_thread",
-            "course_id": unicode(self.course.id),
-        })
-        self.register_get_thread_response(cs_thread)
-        cs_comment = make_minimal_cs_comment({
-            "id": "test_comment",
-            "course_id": cs_thread["course_id"],
-            "thread_id": cs_thread["id"],
-            "username": self.user.username,
-            "user_id": str(self.user.id),
-            "created_at": "2015-06-03T00:00:00Z",
-            "updated_at": "2015-06-03T00:00:00Z",
-            "body": "Original body",
-        })
-        self.register_get_comment_response(cs_comment)
-        self.register_put_comment_response(cs_comment)
 
-    def test_basic(self):
-        request_data = {"raw_body": "Edited body"}
-        expected_response_data = {
+    def expected_response_data(self, overrides=None):
+        """
+        create expected response data from comment update endpoint
+        """
+        response_data = {
             "id": "test_comment",
             "thread_id": "test_thread",
             "parent_id": None,
             "author": self.user.username,
             "author_label": None,
-            "created_at": "2015-06-03T00:00:00Z",
-            "updated_at": "2015-06-03T00:00:00Z",
-            "raw_body": "Edited body",
-            "rendered_body": "<p>Edited body</p>",
+            "created_at": "1970-01-01T00:00:00Z",
+            "updated_at": "1970-01-01T00:00:00Z",
+            "raw_body": "Original body",
+            "rendered_body": "<p>Original body</p>",
             "endorsed": False,
             "endorsed_by": None,
             "endorsed_by_label": None,
@@ -980,16 +1029,28 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes
             "voted": False,
             "vote_count": 0,
             "children": [],
-            "editable_fields": ["abuse_flagged", "raw_body", "voted"],
+            "editable_fields": [],
         }
-        response = self.client.patch(  # pylint: disable=no-member
-            self.url,
-            json.dumps(request_data),
-            content_type="application/merge-patch+json"
-        )
+        response_data.update(overrides or {})
+        return response_data
+
+    def test_basic(self):
+        self.register_thread()
+        self.register_comment({"created_at": "Test Date", "updated_at": "Test Date"})
+        request_data = {"raw_body": "Edited body"}
+        response = self.request_patch(request_data)
         self.assertEqual(response.status_code, 200)
         response_data = json.loads(response.content)
-        self.assertEqual(response_data, expected_response_data)
+        self.assertEqual(
+            response_data,
+            self.expected_response_data({
+                "raw_body": "Edited body",
+                "rendered_body": "<p>Edited body</p>",
+                "editable_fields": ["abuse_flagged", "raw_body", "voted"],
+                "created_at": "Test Date",
+                "updated_at": "Test Date",
+            })
+        )
         self.assertEqual(
             httpretty.last_request().parsed_body,
             {
@@ -1003,12 +1064,10 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes
         )
 
     def test_error(self):
+        self.register_thread()
+        self.register_comment()
         request_data = {"raw_body": ""}
-        response = self.client.patch(  # pylint: disable=no-member
-            self.url,
-            json.dumps(request_data),
-            content_type="application/merge-patch+json"
-        )
+        response = self.request_patch(request_data)
         expected_response_data = {
             "field_errors": {"raw_body": {"developer_message": "This field may not be blank."}}
         }
@@ -1016,6 +1075,40 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes
         response_data = json.loads(response.content)
         self.assertEqual(response_data, expected_response_data)
 
+    @ddt.data(
+        ("abuse_flagged", True),
+        ("abuse_flagged", False),
+    )
+    @ddt.unpack
+    def test_closed_thread(self, field, value):
+        self.register_thread({"closed": True})
+        self.register_comment()
+        self.register_flag_response("comment", "test_comment")
+        request_data = {field: value}
+        response = self.request_patch(request_data)
+        self.assertEqual(response.status_code, 200)
+        response_data = json.loads(response.content)
+        self.assertEqual(
+            response_data,
+            self.expected_response_data({
+                "abuse_flagged": value,
+                "editable_fields": ["abuse_flagged"],
+            })
+        )
+
+    @ddt.data(
+        ("raw_body", "Edited body"),
+        ("voted", True),
+        ("following", True),
+    )
+    @ddt.unpack
+    def test_closed_thread_error(self, field, value):
+        self.register_thread({"closed": True})
+        self.register_comment()
+        request_data = {field: value}
+        response = self.request_patch(request_data)
+        self.assertEqual(response.status_code, 400)
+
 
 @httpretty.activate
 @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})

lms/djangoapps/discussion_api/tests/utils.py

@@ -301,6 +301,16 @@ class CommentsServiceMockMixin(object):
         """
         self.assert_query_params_equal(httpretty.last_request(), expected_params)
 
+    def request_patch(self, request_data):
+        """
+        make a request to PATCH endpoint and return response
+        """
+        return self.client.patch(  # pylint: disable=no-member
+            self.url,
+            json.dumps(request_data),
+            content_type="application/merge-patch+json"
+        )
+
 
 def make_minimal_cs_thread(overrides=None):
     """