Merge pull request #8132 from edx/gprice/discussion-api-comment-list-endorsed

Add endorsement fields to comment list API

lms/djangoapps/discussion_api/api.py

@@ -169,7 +169,7 @@ def get_comment_list(request, thread_id, endorsed, page, page_size):
 
     course_key = CourseLocator.from_string(cc_thread["course_id"])
     course = _get_course_or_404(course_key, request.user)
-    context = get_context(course, request.user)
+    context = get_context(course, request.user, cc_thread)
 
     # Ensure user has access to the thread
     if not context["is_requester_privileged"] and cc_thread["group_id"]:

lms/djangoapps/discussion_api/serializers.py

 """
 Discussion API serializers
 """
+from django.contrib.auth.models import User as DjangoUser
+
 from rest_framework import serializers
 
 from django_comment_common.models import (
@@ -9,14 +11,14 @@ from django_comment_common.models import (
     FORUM_ROLE_MODERATOR,
     Role,
 )
-from lms.lib.comment_client.user import User
+from lms.lib.comment_client.user import User as CommentClientUser
 from openedx.core.djangoapps.course_groups.cohorts import get_cohort_names
 
 
-def get_context(course, requester):
+def get_context(course, requester, thread=None):
     """
     Returns a context appropriate for use with ThreadSerializer or
-    CommentSerializer.
+    (if thread is provided) CommentSerializer.
     """
     # TODO: cache staff_user_ids and ta_user_ids if we need to improve perf
     staff_user_ids = {
@@ -38,7 +40,8 @@ def get_context(course, requester):
         "is_requester_privileged": requester.id in staff_user_ids or requester.id in ta_user_ids,
         "staff_user_ids": staff_user_ids,
         "ta_user_ids": ta_user_ids,
-        "cc_requester": User.from_django_user(requester).retrieve(),
+        "cc_requester": CommentClientUser.from_django_user(requester).retrieve(),
+        "thread": thread,
     }
 
 
@@ -60,6 +63,13 @@ class _ContentSerializer(serializers.Serializer):
         # name above and modify it here
         self.fields["id"] = self.fields.pop("id_")
 
+    def _is_user_privileged(self, user_id):
+        """
+        Returns a boolean indicating whether the given user_id identifies a
+        privileged user.
+        """
+        return user_id in self.context["staff_user_ids"] or user_id in self.context["ta_user_ids"]
+
     def _is_anonymous(self, obj):
         """
         Returns a boolean indicating whether the content should be anonymous to
@@ -156,12 +166,49 @@ class CommentSerializer(_ContentSerializer):
     """
     thread_id = serializers.CharField()
     parent_id = serializers.SerializerMethodField("get_parent_id")
+    endorsed = serializers.BooleanField()
+    endorsed_by = serializers.SerializerMethodField("get_endorsed_by")
+    endorsed_by_label = serializers.SerializerMethodField("get_endorsed_by_label")
+    endorsed_at = serializers.SerializerMethodField("get_endorsed_at")
     children = serializers.SerializerMethodField("get_children")
 
     def get_parent_id(self, _obj):
         """Returns the comment's parent's id (taken from the context)."""
         return self.context.get("parent_id")
 
+    def get_endorsed_by(self, obj):
+        """
+        Returns the username of the endorsing user, if the information is
+        available and would not identify the author of an anonymous thread.
+        """
+        endorsement = obj.get("endorsement")
+        if endorsement:
+            endorser_id = int(endorsement["user_id"])
+            # Avoid revealing the identity of an anonymous non-staff question
+            # author who has endorsed a comment in the thread
+            if not (
+                    self._is_anonymous(self.context["thread"]) and
+                    not self._is_user_privileged(endorser_id)
+            ):
+                return DjangoUser.objects.get(id=endorser_id).username
+        return None
+
+    def get_endorsed_by_label(self, obj):
+        """
+        Returns the role label (i.e. "staff" or "community_ta") for the
+        endorsing user
+        """
+        endorsement = obj.get("endorsement")
+        if endorsement:
+            return self._get_user_label(int(endorsement["user_id"]))
+        else:
+            return None
+
+    def get_endorsed_at(self, obj):
+        """Returns the timestamp for the endorsement, if available."""
+        endorsement = obj.get("endorsement")
+        return endorsement["time"] if endorsement else None
+
     def get_children(self, obj):
         """Returns the list of the comment's children, serialized."""
         child_context = dict(self.context)

lms/djangoapps/discussion_api/tests/test_api.py

@@ -745,6 +745,7 @@ class GetCommentListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
                 "created_at": "2015-05-11T00:00:00Z",
                 "updated_at": "2015-05-11T11:11:11Z",
                 "body": "Test body",
+                "endorsed": False,
                 "abuse_flaggers": [],
                 "votes": {"up_count": 4},
                 "children": [],
@@ -759,6 +760,7 @@ class GetCommentListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
                 "created_at": "2015-05-11T22:22:22Z",
                 "updated_at": "2015-05-11T33:33:33Z",
                 "body": "More content",
+                "endorsed": False,
                 "abuse_flaggers": [str(self.user.id)],
                 "votes": {"up_count": 7},
                 "children": [],
@@ -774,6 +776,10 @@ class GetCommentListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
                 "created_at": "2015-05-11T00:00:00Z",
                 "updated_at": "2015-05-11T11:11:11Z",
                 "raw_body": "Test body",
+                "endorsed": False,
+                "endorsed_by": None,
+                "endorsed_by_label": None,
+                "endorsed_at": None,
                 "abuse_flagged": False,
                 "voted": False,
                 "vote_count": 4,
@@ -788,6 +794,10 @@ class GetCommentListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
                 "created_at": "2015-05-11T22:22:22Z",
                 "updated_at": "2015-05-11T33:33:33Z",
                 "raw_body": "More content",
+                "endorsed": False,
+                "endorsed_by": None,
+                "endorsed_by_label": None,
+                "endorsed_at": None,
                 "abuse_flagged": True,
                 "voted": False,
                 "vote_count": 7,
@@ -813,6 +823,22 @@ class GetCommentListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
         non_endorsed_actual = self.get_comment_list(thread, endorsed=False)
         self.assertEqual(non_endorsed_actual["results"][0]["id"], "non_endorsed_comment")
 
+    def test_endorsed_by_anonymity(self):
+        """
+        Ensure thread anonymity is properly considered in serializing
+        endorsed_by.
+        """
+        thread = self.make_minimal_cs_thread({
+            "anonymous": True,
+            "children": [
+                make_minimal_cs_comment({
+                    "endorsement": {"user_id": str(self.author.id), "time": "2015-05-18T12:34:56Z"}
+                })
+            ]
+        })
+        actual_comments = self.get_comment_list(thread)["results"]
+        self.assertIsNone(actual_comments[0]["endorsed_by"])
+
     @ddt.data(
         ("discussion", None, "children", "resp_total"),
         ("question", False, "non_endorsed_responses", "non_endorsed_resp_total"),

lms/djangoapps/discussion_api/tests/test_serializers.py

 """
 Tests for Discussion API serializers
 """
+import itertools
+
 import ddt
 import httpretty
 
@@ -199,7 +201,12 @@ class ThreadSerializerTest(SerializerTestMixin, ModuleStoreTestCase):
 @ddt.ddt
 class CommentSerializerTest(SerializerTestMixin, ModuleStoreTestCase):
     """Tests for CommentSerializer."""
-    def make_cs_content(self, overrides):
+    def setUp(self):
+        super(CommentSerializerTest, self).setUp()
+        self.endorser = UserFactory.create()
+        self.endorsed_at = "2015-05-18T12:34:56Z"
+
+    def make_cs_content(self, overrides=None, with_endorsement=False):
         """
         Create a comment with the given overrides, plus some useful test data.
         """
@@ -207,15 +214,21 @@ class CommentSerializerTest(SerializerTestMixin, ModuleStoreTestCase):
             "user_id": str(self.author.id),
             "username": self.author.username
         }
-        merged_overrides.update(overrides)
+        if with_endorsement:
+            merged_overrides["endorsement"] = {
+                "user_id": str(self.endorser.id),
+                "time": self.endorsed_at
+            }
+        merged_overrides.update(overrides or {})
         return make_minimal_cs_comment(merged_overrides)
 
-    def serialize(self, comment):
+    def serialize(self, comment, thread_data=None):
         """
         Create a serializer with an appropriate context and use it to serialize
         the given comment, returning the result.
         """
-        return CommentSerializer(comment, context=get_context(self.course, self.user)).data
+        context = get_context(self.course, self.user, make_minimal_cs_thread(thread_data))
+        return CommentSerializer(comment, context=context).data
 
     def test_basic(self):
         comment = {
@@ -228,6 +241,7 @@ class CommentSerializerTest(SerializerTestMixin, ModuleStoreTestCase):
             "created_at": "2015-04-28T00:00:00Z",
             "updated_at": "2015-04-28T11:11:11Z",
             "body": "Test body",
+            "endorsed": False,
             "abuse_flaggers": [],
             "votes": {"up_count": 4},
             "children": [],
@@ -241,6 +255,10 @@ class CommentSerializerTest(SerializerTestMixin, ModuleStoreTestCase):
             "created_at": "2015-04-28T00:00:00Z",
             "updated_at": "2015-04-28T11:11:11Z",
             "raw_body": "Test body",
+            "endorsed": False,
+            "endorsed_by": None,
+            "endorsed_by_label": None,
+            "endorsed_at": None,
             "abuse_flagged": False,
             "voted": False,
             "vote_count": 4,
@@ -248,6 +266,63 @@ class CommentSerializerTest(SerializerTestMixin, ModuleStoreTestCase):
         }
         self.assertEqual(self.serialize(comment), expected)
 
+    @ddt.data(
+        *itertools.product(
+            [
+                FORUM_ROLE_ADMINISTRATOR,
+                FORUM_ROLE_MODERATOR,
+                FORUM_ROLE_COMMUNITY_TA,
+                FORUM_ROLE_STUDENT,
+            ],
+            [True, False]
+        )
+    )
+    @ddt.unpack
+    def test_endorsed_by(self, endorser_role_name, thread_anonymous):
+        """
+        Test correctness of the endorsed_by field.
+
+        The endorser should be anonymous iff the thread is anonymous to the
+        requester, and the endorser is not a privileged user.
+
+        endorser_role_name is the name of the endorser's role.
+        thread_anonymous is the value of the anonymous field in the thread.
+        """
+        self.create_role(endorser_role_name, [self.endorser])
+        serialized = self.serialize(
+            self.make_cs_content(with_endorsement=True),
+            thread_data={"anonymous": thread_anonymous}
+        )
+        actual_endorser_anonymous = serialized["endorsed_by"] is None
+        expected_endorser_anonymous = endorser_role_name == FORUM_ROLE_STUDENT and thread_anonymous
+        self.assertEqual(actual_endorser_anonymous, expected_endorser_anonymous)
+
+    @ddt.data(
+        (FORUM_ROLE_ADMINISTRATOR, "staff"),
+        (FORUM_ROLE_MODERATOR, "staff"),
+        (FORUM_ROLE_COMMUNITY_TA, "community_ta"),
+        (FORUM_ROLE_STUDENT, None),
+    )
+    @ddt.unpack
+    def test_endorsed_by_labels(self, role_name, expected_label):
+        """
+        Test correctness of the endorsed_by_label field.
+
+        The label should be "staff", "staff", or "community_ta" for the
+        Administrator, Moderator, and Community TA roles, respectively.
+
+        role_name is the name of the author's role.
+        expected_label is the expected value of the author_label field in the
+          API output.
+        """
+        self.create_role(role_name, [self.endorser])
+        serialized = self.serialize(self.make_cs_content(with_endorsement=True))
+        self.assertEqual(serialized["endorsed_by_label"], expected_label)
+
+    def test_endorsed_at(self):
+        serialized = self.serialize(self.make_cs_content(with_endorsement=True))
+        self.assertEqual(serialized["endorsed_at"], self.endorsed_at)
+
     def test_children(self):
         comment = self.make_cs_content({
             "id": "test_root",

lms/djangoapps/discussion_api/tests/test_views.py

@@ -240,6 +240,7 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             "created_at": "2015-05-11T00:00:00Z",
             "updated_at": "2015-05-11T11:11:11Z",
             "body": "Test body",
+            "endorsed": False,
             "abuse_flaggers": [],
             "votes": {"up_count": 4},
             "children": [],
@@ -253,6 +254,10 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             "created_at": "2015-05-11T00:00:00Z",
             "updated_at": "2015-05-11T11:11:11Z",
             "raw_body": "Test body",
+            "endorsed": False,
+            "endorsed_by": None,
+            "endorsed_by_label": None,
+            "endorsed_at": None,
             "abuse_flagged": False,
             "voted": True,
             "vote_count": 4,

lms/djangoapps/discussion_api/tests/utils.py

@@ -119,7 +119,6 @@ def make_minimal_cs_comment(overrides=None):
         "abuse_flaggers": [],
         "votes": {"up_count": 0},
         "endorsed": False,
-        "endorsement": None,
         "children": [],
     }
     ret.update(overrides or {})

lms/djangoapps/discussion_api/views.py

@@ -174,6 +174,18 @@ class CommentViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
 
             * raw_body: The comment's raw body text without any rendering applied
 
+            * endorsed: Boolean indicating whether the comment has been endorsed
+                (by a privileged user or, for a question thread, the thread
+                author)
+
+            * endorsed_by: The username of the endorsing user, if available
+
+            * endorsed_by_label: A label indicating whether the endorsing user
+                has a special role in the course (see author_label)
+
+            * endorsed_at: The ISO 8601 timestamp for the endorsement, if
+                available
+
             * abuse_flagged: Boolean indicating whether the requesting user has
               flagged the comment for abuse