Merge pull request #8485 from edx/gprice/discussion-api-endorse

Add comment endorsement to discussion API

lms/djangoapps/discussion_api/api.py

@@ -420,6 +420,20 @@ def _get_thread_editable_fields(cc_thread, context):
         return _THREAD_EDITABLE_BY_ANY
 
 
+def _check_editable_fields(editable_fields, update_data):
+    """
+    Raise ValidationError if the given update data contains a field that is not
+    in editable_fields.
+    """
+    non_editable_errors = {
+        field: ["This field is not editable."]
+        for field in update_data.keys()
+        if field not in editable_fields
+    }
+    if non_editable_errors:
+        raise ValidationError(non_editable_errors)
+
+
 def update_thread(request, thread_id, update_data):
     """
     Update a thread.
@@ -440,13 +454,7 @@ def update_thread(request, thread_id, update_data):
     """
     cc_thread, context = _get_thread_and_context(request, thread_id)
     editable_fields = _get_thread_editable_fields(cc_thread, context)
-    non_editable_errors = {
-        field: ["This field is not editable."]
-        for field in update_data.keys()
-        if field not in editable_fields
-    }
-    if non_editable_errors:
-        raise ValidationError(non_editable_errors)
+    _check_editable_fields(editable_fields, update_data)
     serializer = ThreadSerializer(cc_thread, data=update_data, partial=True, context=context)
     actions_form = ThreadActionsForm(update_data)
     if not (serializer.is_valid() and actions_form.is_valid()):
@@ -459,6 +467,22 @@ def update_thread(request, thread_id, update_data):
     return api_thread
 
 
+_COMMENT_EDITABLE_BY_AUTHOR = {"raw_body"}
+_COMMENT_EDITABLE_BY_THREAD_AUTHOR = {"endorsed"}
+
+
+def _get_comment_editable_fields(cc_comment, context):
+    """
+    Get the list of editable fields for the given comment in the given context
+    """
+    ret = set()
+    if _is_user_author_or_privileged(cc_comment, context):
+        ret |= _COMMENT_EDITABLE_BY_AUTHOR
+    if _is_user_author_or_privileged(context["thread"], context):
+        ret |= _COMMENT_EDITABLE_BY_THREAD_AUTHOR
+    return ret
+
+
 def update_comment(request, comment_id, update_data):
     """
     Update a comment.
@@ -489,8 +513,8 @@ def update_comment(request, comment_id, update_data):
           is empty or thread_id is included)
     """
     cc_comment, context = _get_comment_and_context(request, comment_id)
-    if not _is_user_author_or_privileged(cc_comment, context):
-        raise PermissionDenied()
+    editable_fields = _get_comment_editable_fields(cc_comment, context)
+    _check_editable_fields(editable_fields, update_data)
     serializer = CommentSerializer(cc_comment, data=update_data, partial=True, context=context)
     if not serializer.is_valid():
         raise ValidationError(serializer.errors)

lms/djangoapps/discussion_api/serializers.py

@@ -231,7 +231,7 @@ class CommentSerializer(_ContentSerializer):
     """
     thread_id = serializers.CharField()
     parent_id = serializers.CharField(required=False)
-    endorsed = serializers.BooleanField(read_only=True)
+    endorsed = serializers.BooleanField(required=False)
     endorsed_by = serializers.SerializerMethodField("get_endorsed_by")
     endorsed_by_label = serializers.SerializerMethodField("get_endorsed_by_label")
     endorsed_at = serializers.SerializerMethodField("get_endorsed_at")
@@ -300,6 +300,11 @@ class CommentSerializer(_ContentSerializer):
         if instance:
             for key, val in attrs.items():
                 instance[key] = val
+                # TODO: The comments service doesn't populate the endorsement
+                # field on comment creation, so we only provide
+                # endorsement_user_id on update
+                if key == "endorsed":
+                    instance["endorsement_user_id"] = self.context["cc_requester"]["id"]
             return instance
         return Comment(
             course_id=self.context["thread"]["course_id"],

lms/djangoapps/discussion_api/tests/test_api.py

@@ -1789,22 +1789,67 @@ class UpdateCommentTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTest
         except Http404:
             self.assertTrue(expected_error)
 
-    @ddt.data(
-        FORUM_ROLE_ADMINISTRATOR,
-        FORUM_ROLE_MODERATOR,
-        FORUM_ROLE_COMMUNITY_TA,
-        FORUM_ROLE_STUDENT,
-    )
-    def test_role_access(self, role_name):
+    @ddt.data(*itertools.product(
+        [
+            FORUM_ROLE_ADMINISTRATOR,
+            FORUM_ROLE_MODERATOR,
+            FORUM_ROLE_COMMUNITY_TA,
+            FORUM_ROLE_STUDENT,
+        ],
+        [True, False],
+        [True, False],
+    ))
+    @ddt.unpack
+    def test_raw_body_access(self, role_name, is_thread_author, is_comment_author):
         role = Role.objects.create(name=role_name, course_id=self.course.id)
         role.users = [self.user]
-        self.register_comment({"user_id": str(self.user.id + 1)})
-        expected_error = role_name == FORUM_ROLE_STUDENT
+        self.register_comment(
+            {"user_id": str(self.user.id if is_comment_author else (self.user.id + 1))},
+            thread_overrides={
+                "user_id": str(self.user.id if is_thread_author else (self.user.id + 1))
+            }
+        )
+        expected_error = role_name == FORUM_ROLE_STUDENT and not is_comment_author
         try:
             update_comment(self.request, "test_comment", {"raw_body": "edited"})
             self.assertFalse(expected_error)
-        except PermissionDenied:
+        except ValidationError as err:
             self.assertTrue(expected_error)
+            self.assertEqual(
+                err.message_dict,
+                {"raw_body": ["This field is not editable."]}
+            )
+
+    @ddt.data(*itertools.product(
+        [
+            FORUM_ROLE_ADMINISTRATOR,
+            FORUM_ROLE_MODERATOR,
+            FORUM_ROLE_COMMUNITY_TA,
+            FORUM_ROLE_STUDENT,
+        ],
+        [True, False],
+        [True, False],
+    ))
+    @ddt.unpack
+    def test_endorsed_access(self, role_name, is_thread_author, is_comment_author):
+        role = Role.objects.create(name=role_name, course_id=self.course.id)
+        role.users = [self.user]
+        self.register_comment(
+            {"user_id": str(self.user.id if is_comment_author else (self.user.id + 1))},
+            thread_overrides={
+                "user_id": str(self.user.id if is_thread_author else (self.user.id + 1))
+            }
+        )
+        expected_error = role_name == FORUM_ROLE_STUDENT and not is_thread_author
+        try:
+            update_comment(self.request, "test_comment", {"endorsed": True})
+            self.assertFalse(expected_error)
+        except ValidationError as err:
+            self.assertTrue(expected_error)
+            self.assertEqual(
+                err.message_dict,
+                {"endorsed": ["This field is not editable."]}
+            )
 
 
 @ddt.ddt

lms/djangoapps/discussion_api/tests/test_serializers.py

@@ -656,6 +656,27 @@ class CommentSerializerDeserializationTest(CommentsServiceMockMixin, ModuleStore
                 {field: ["This field is required."]}
             )
 
+    def test_create_endorsed(self):
+        # TODO: The comments service doesn't populate the endorsement field on
+        # comment creation, so this is sadly realistic
+        self.register_post_comment_response({}, thread_id="test_thread")
+        data = self.minimal_data.copy()
+        data["endorsed"] = True
+        saved = self.save_and_reserialize(data)
+        self.assertEqual(
+            httpretty.last_request().parsed_body,
+            {
+                "course_id": [unicode(self.course.id)],
+                "body": ["Test body"],
+                "user_id": [str(self.user.id)],
+                "endorsed": ["True"],
+            }
+        )
+        self.assertTrue(saved["endorsed"])
+        self.assertIsNone(saved["endorsed_by"])
+        self.assertIsNone(saved["endorsed_by_label"])
+        self.assertIsNone(saved["endorsed_at"])
+
     def test_update_empty(self):
         self.register_put_comment_response(self.existing_comment.attributes)
         self.save_and_reserialize({}, instance=self.existing_comment)
@@ -672,8 +693,13 @@ class CommentSerializerDeserializationTest(CommentsServiceMockMixin, ModuleStore
         )
 
     def test_update_all(self):
-        self.register_put_comment_response(self.existing_comment.attributes)
-        data = {"raw_body": "Edited body"}
+        cs_response_data = self.existing_comment.attributes.copy()
+        cs_response_data["endorsement"] = {
+            "user_id": str(self.user.id),
+            "time": "2015-06-05T00:00:00Z",
+        }
+        self.register_put_comment_response(cs_response_data)
+        data = {"raw_body": "Edited body", "endorsed": True}
         saved = self.save_and_reserialize(data, instance=self.existing_comment)
         self.assertEqual(
             httpretty.last_request().parsed_body,
@@ -683,10 +709,14 @@ class CommentSerializerDeserializationTest(CommentsServiceMockMixin, ModuleStore
                 "user_id": [str(self.user.id)],
                 "anonymous": ["False"],
                 "anonymous_to_peers": ["False"],
-                "endorsed": ["False"],
+                "endorsed": ["True"],
+                "endorsement_user_id": [str(self.user.id)],
             }
         )
-        self.assertEqual(saved["raw_body"], data["raw_body"])
+        for key in data:
+            self.assertEqual(saved[key], data[key])
+        self.assertEqual(saved["endorsed_by"], self.user.username)
+        self.assertEqual(saved["endorsed_at"], "2015-06-05T00:00:00Z")
 
     def test_update_empty_raw_body(self):
         serializer = CommentSerializer(