Add endorsement fields to comment list API
This commit is contained in:
@@ -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"]:
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
"""
|
||||
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)
|
||||
|
||||
@@ -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"),
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
"""
|
||||
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",
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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 {})
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user