Skip to content

E
edx-platform
Commit 7309352e by Greg Price
Options

Refactor discussion API to use DRF serializer 

This will make it easier to add the creation and update interfaces.
parent 565cdb8e
Showing with 353 additions and 241 deletions
lms/djangoapps/discussion_api/api.py
......@@ -7,16 +7,10 @@ from collections import defaultdict
from courseware.courses import get_course_with_access
from discussion_api.pagination import get_paginated_data
from discussion_api.serializers import ThreadSerializer, get_context
from django_comment_client.utils import get_accessible_discussion_modules
from django_comment_common.models import (
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_MODERATOR,
Role,
)
from lms.lib.comment_client.thread import Thread
from lms.lib.comment_client.user import User
from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id, get_cohort_names
from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id
from xmodule.tabs import DiscussionTab
......@@ -92,67 +86,6 @@ def get_course_topics(course_key, user):
}
def _cc_thread_to_api_thread(thread, cc_user, staff_user_ids, ta_user_ids, group_ids_to_names):
"""
Convert a thread data dict from the comment_client format (which is a direct
representation of the format returned by the comments service) to the format
used in this API
Arguments:
thread (comment_client.thread.Thread): The thread to convert
cc_user (comment_client.user.User): The comment_client representation of
the requesting user
staff_user_ids (set): The set of user ids for users with the Moderator or
Administrator role in the course
ta_user_ids (set): The set of user ids for users with the Community TA
role in the course
group_ids_to_names (dict): A mapping of group ids to names
Returns:
dict: The discussion_api format representation of the thread.
"""
is_anonymous = (
thread["anonymous"] or
(
thread["anonymous_to_peers"] and
int(cc_user["id"]) not in (staff_user_ids | ta_user_ids)
)
)
ret = {
key: thread[key]
for key in [
"id",
"course_id",
"group_id",
"created_at",
"updated_at",
"title",
"pinned",
"closed",
]
}
ret.update({
"topic_id": thread["commentable_id"],
"group_name": group_ids_to_names.get(thread["group_id"]),
"author": None if is_anonymous else thread["username"],
"author_label": (
None if is_anonymous else
"staff" if int(thread["user_id"]) in staff_user_ids else
"community_ta" if int(thread["user_id"]) in ta_user_ids else
None
),
"type": thread["thread_type"],
"raw_body": thread["body"],
"following": thread["id"] in cc_user["subscribed_thread_ids"],
"abuse_flagged": cc_user["id"] in thread["abuse_flaggers"],
"voted": thread["id"] in cc_user["upvoted_ids"],
"vote_count": thread["votes"]["up_count"],
"comment_count": thread["comments_count"],
"unread_comment_count": thread["unread_comments_count"],
})
return ret
def get_thread_list(request, course_key, page, page_size):
"""
Return the list of all discussion threads pertaining to the given course
......@@ -170,15 +103,13 @@ def get_thread_list(request, course_key, page, page_size):
discussion_api.views.ThreadViewSet for more detail.
"""
course = _get_course_or_404(course_key, request.user)
user_is_privileged = Role.objects.filter(
course_id=course.id,
name__in=[FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA],
users=request.user
).exists()
cc_user = User.from_django_user(request.user).retrieve()
context = get_context(course, request.user)
threads, result_page, num_pages, _ = Thread.search({
"course_id": unicode(course.id),
"group_id": None if user_is_privileged else get_cohort_id(request.user, course.id),
"group_id": (
None if context["is_requester_privileged"] else
get_cohort_id(request.user, course.id)
),
"sort_key": "date",
"sort_order": "desc",
"page": page,
......@@ -189,25 +120,6 @@ def get_thread_list(request, course_key, page, page_size):
# behavior and return a 404 in that case
if result_page != page:
raise Http404
# TODO: cache staff_user_ids and ta_user_ids if we need to improve perf
staff_user_ids = {
user.id
for role in Role.objects.filter(
name__in=[FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR],
course_id=course.id
)
for user in role.users.all()
}
ta_user_ids = {
user.id
for role in Role.objects.filter(name=FORUM_ROLE_COMMUNITY_TA, course_id=course.id)
for user in role.users.all()
}
# For now, the only groups are cohorts
group_ids_to_names = get_cohort_names(course)
results = [
_cc_thread_to_api_thread(thread, cc_user, staff_user_ids, ta_user_ids, group_ids_to_names)
for thread in threads
]
results = [ThreadSerializer(thread, context=context).data for thread in threads]
return get_paginated_data(request, results, page, num_pages)
lms/djangoapps/discussion_api/serializers.py 0 → 100644
"""
Discussion API serializers
"""
from rest_framework import serializers
from django_comment_common.models import (
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_MODERATOR,
Role,
)
from lms.lib.comment_client.user import User
from openedx.core.djangoapps.course_groups.cohorts import get_cohort_names
def get_context(course, requester):
"""Returns a context appropriate for use with ThreadSerializer."""
# TODO: cache staff_user_ids and ta_user_ids if we need to improve perf
staff_user_ids = {
user.id
for role in Role.objects.filter(
name__in=[FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR],
course_id=course.id
)
for user in role.users.all()
}
ta_user_ids = {
user.id
for role in Role.objects.filter(name=FORUM_ROLE_COMMUNITY_TA, course_id=course.id)
for user in role.users.all()
}
return {
# For now, the only groups are cohorts
"group_ids_to_names": get_cohort_names(course),
"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(),
}
class ThreadSerializer(serializers.Serializer):
"""
A serializer for thread data.
N.B. This should not be used with a comment_client Thread object that has
not had retrieve() called, because of the interaction between DRF's attempts
at introspection and Thread's __getattr__.
"""
id_ = serializers.CharField(read_only=True)
course_id = serializers.CharField()
topic_id = serializers.CharField(source="commentable_id")
group_id = serializers.IntegerField()
group_name = serializers.SerializerMethodField("get_group_name")
author = serializers.SerializerMethodField("get_author")
author_label = serializers.SerializerMethodField("get_author_label")
created_at = serializers.CharField(read_only=True)
updated_at = serializers.CharField(read_only=True)
type_ = serializers.ChoiceField(source="thread_type", choices=("discussion", "question"))
title = serializers.CharField()
raw_body = serializers.CharField(source="body")
pinned = serializers.BooleanField()
closed = serializers.BooleanField()
following = serializers.SerializerMethodField("get_following")
abuse_flagged = serializers.SerializerMethodField("get_abuse_flagged")
voted = serializers.SerializerMethodField("get_voted")
vote_count = serializers.SerializerMethodField("get_vote_count")
comment_count = serializers.IntegerField(source="comments_count")
unread_comment_count = serializers.IntegerField(source="unread_comments_count")
def __init__(self, *args, **kwargs):
super(ThreadSerializer, self).__init__(*args, **kwargs)
# type and id are invalid class attribute names, so we must declare
# different names above and modify them here
self.fields["id"] = self.fields.pop("id_")
self.fields["type"] = self.fields.pop("type_")
def get_group_name(self, obj):
"""Returns the name of the group identified by the thread's group_id."""
return self.context["group_ids_to_names"].get(obj["group_id"])
def _is_anonymous(self, obj):
"""
Returns a boolean indicating whether the thread should be anonymous to
the requester.
"""
return (
obj["anonymous"] or
obj["anonymous_to_peers"] and not self.context["is_requester_privileged"]
)
def get_author(self, obj):
"""Returns the author's username, or None if the thread is anonymous."""
return None if self._is_anonymous(obj) else obj["username"]
def _get_user_label(self, user_id):
"""
Returns the role label (i.e. "staff" or "community_ta") for the user
with the given id.
"""
return (
"staff" if user_id in self.context["staff_user_ids"] else
"community_ta" if user_id in self.context["ta_user_ids"] else
None
)
def get_author_label(self, obj):
"""Returns the role label for the thread author."""
return None if self._is_anonymous(obj) else self._get_user_label(int(obj["user_id"]))
def get_following(self, obj):
"""
Returns a boolean indicating whether the requester is following the
thread.
"""
return obj["id"] in self.context["cc_requester"]["subscribed_thread_ids"]
def get_abuse_flagged(self, obj):
"""
Returns a boolean indicating whether the requester has flagged the
thread as abusive.
"""
return self.context["cc_requester"]["id"] in obj["abuse_flaggers"]
def get_voted(self, obj):
"""
Returns a boolean indicating whether the requester has voted for the
thread.
"""
return obj["id"] in self.context["cc_requester"]["upvoted_ids"]
def get_vote_count(self, obj):
"""Returns the number of votes for the thread."""
return obj["votes"]["up_count"]
lms/djangoapps/discussion_api/tests/test_api.py
......@@ -384,35 +384,6 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
role.users = users
role.save()
def make_cs_thread(self, thread_data):
"""
Create a dictionary containing all needed thread fields as returned by
the comments service with dummy data overridden by thread_data
"""
ret = {
"id": "dummy",
"course_id": unicode(self.course.id),
"commentable_id": "dummy",
"group_id": None,
"user_id": str(self.author.id),
"username": self.author.username,
"anonymous": False,
"anonymous_to_peers": False,
"created_at": "1970-01-01T00:00:00Z",
"updated_at": "1970-01-01T00:00:00Z",
"thread_type": "discussion",
"title": "dummy",
"body": "dummy",
"pinned": False,
"closed": False,
"abuse_flaggers": [],
"votes": {"up_count": 0},
"comments_count": 0,
"unread_comments_count": 0,
}
ret.update(thread_data)
return ret
def test_nonexistent_course(self):
with self.assertRaises(Http404):
get_thread_list(self.request, CourseLocator.from_string("non/existent/course"), 1, 1)
......@@ -449,11 +420,6 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
})
def test_thread_content(self):
self.register_get_user_response(
self.user,
subscribed_thread_ids=["test_thread_id_0"],
upvoted_ids=["test_thread_id_1"]
)
source_threads = [
{
"id": "test_thread_id_0",
......@@ -497,27 +463,6 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
"comments_count": 18,
"unread_comments_count": 0,
},
{
"id": "test_thread_id_2",
"course_id": unicode(self.course.id),
"commentable_id": "topic_x",
"group_id": self.cohort.id + 1, # non-existent group
"user_id": str(self.author.id),
"username": self.author.username,
"anonymous": False,
"anonymous_to_peers": False,
"created_at": "2015-04-28T00:44:44Z",
"updated_at": "2015-04-28T00:55:55Z",
"thread_type": "discussion",
"title": "Yet Another Test Title",
"body": "Still more content",
"pinned": True,
"closed": False,
"abuse_flaggers": [str(self.user.id)],
"votes": {"up_count": 0},
"comments_count": 0,
"unread_comments_count": 0,
},
]
expected_threads = [
{
......@@ -535,7 +480,7 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
"raw_body": "Test body",
"pinned": False,
"closed": False,
"following": True,
"following": False,
"abuse_flagged": False,
"voted": False,
"vote_count": 4,
......@@ -559,33 +504,11 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
"closed": True,
"following": False,
"abuse_flagged": False,
"voted": True,
"voted": False,
"vote_count": 9,
"comment_count": 18,
"unread_comment_count": 0,
},
{
"id": "test_thread_id_2",
"course_id": unicode(self.course.id),
"topic_id": "topic_x",
"group_id": self.cohort.id + 1,
"group_name": None,
"author": self.author.username,
"author_label": None,
"created_at": "2015-04-28T00:44:44Z",
"updated_at": "2015-04-28T00:55:55Z",
"type": "discussion",
"title": "Yet Another Test Title",
"raw_body": "Still more content",
"pinned": True,
"closed": False,
"following": False,
"abuse_flagged": True,
"voted": False,
"vote_count": 0,
"comment_count": 0,
"unread_comment_count": 0,
},
]
self.assertEqual(
self.get_thread_list(source_threads),
......@@ -650,69 +573,3 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
self.register_get_threads_response([], page=3, num_pages=3)
with self.assertRaises(Http404):
get_thread_list(self.request, self.course.id, page=4, page_size=10)
@ddt.data(
(FORUM_ROLE_ADMINISTRATOR, True, False, True),
(FORUM_ROLE_ADMINISTRATOR, False, True, False),
(FORUM_ROLE_MODERATOR, True, False, True),
(FORUM_ROLE_MODERATOR, False, True, False),
(FORUM_ROLE_COMMUNITY_TA, True, False, True),
(FORUM_ROLE_COMMUNITY_TA, False, True, False),
(FORUM_ROLE_STUDENT, True, False, True),
(FORUM_ROLE_STUDENT, False, True, True),
)
@ddt.unpack
def test_anonymity(self, role_name, anonymous, anonymous_to_peers, expected_api_anonymous):
"""
Test that a thread is properly made anonymous.
A thread should be anonymous iff the anonymous field is true or the
anonymous_to_peers field is true and the requester does not have a
privileged role.
role_name is the name of the requester's role.
thread_anon is the value of the anonymous field in the thread data.
thread_anon_to_peers is the value of the anonymous_to_peers field in the
thread data.
expected_api_anonymous is whether the thread should actually be
anonymous in the API output when requested by a user with the given
role.
"""
self.create_role(role_name, [self.user])
result = self.get_thread_list([
self.make_cs_thread({
"anonymous": anonymous,
"anonymous_to_peers": anonymous_to_peers,
})
])
actual_api_anonymous = result["results"][0]["author"] is None
self.assertEqual(actual_api_anonymous, expected_api_anonymous)
@ddt.data(
(FORUM_ROLE_ADMINISTRATOR, False, "staff"),
(FORUM_ROLE_ADMINISTRATOR, True, None),
(FORUM_ROLE_MODERATOR, False, "staff"),
(FORUM_ROLE_MODERATOR, True, None),
(FORUM_ROLE_COMMUNITY_TA, False, "community_ta"),
(FORUM_ROLE_COMMUNITY_TA, True, None),
(FORUM_ROLE_STUDENT, False, None),
(FORUM_ROLE_STUDENT, True, None),
)
@ddt.unpack
def test_author_labels(self, role_name, anonymous, expected_label):
"""
Test correctness of the author_label field.
The label should be "staff", "staff", or "community_ta" for the
Administrator, Moderator, and Community TA roles, respectively, but
the label should not be present if the thread is anonymous.
role_name is the name of the author's role.
anonymous is the value of the anonymous field in the thread data.
expected_label is the expected value of the author_label field in the
API output.
"""
self.create_role(role_name, [self.author])
result = self.get_thread_list([self.make_cs_thread({"anonymous": anonymous})])
actual_label = result["results"][0]["author_label"]
self.assertEqual(actual_label, expected_label)
lms/djangoapps/discussion_api/tests/test_serializers.py 0 → 100644
"""
Tests for Discussion API serializers
"""
import ddt
import httpretty
from discussion_api.serializers import ThreadSerializer, get_context
from discussion_api.tests.utils import CommentsServiceMockMixin
from django_comment_common.models import (
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_MODERATOR,
FORUM_ROLE_STUDENT,
Role,
)
from student.tests.factories import UserFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
@ddt.ddt
class ThreadSerializerTest(CommentsServiceMockMixin, ModuleStoreTestCase):
"""Tests for ThreadSerializer."""
def setUp(self):
super(ThreadSerializerTest, self).setUp()
httpretty.reset()
httpretty.enable()
self.addCleanup(httpretty.disable)
self.maxDiff = None # pylint: disable=invalid-name
self.user = UserFactory.create()
self.register_get_user_response(self.user)
self.course = CourseFactory.create()
self.author = UserFactory.create()
def create_role(self, role_name, users, course=None):
"""Create a Role in self.course with the given name and users"""
course = course or self.course
role = Role.objects.create(name=role_name, course_id=course.id)
role.users = users
def make_cs_thread(self, thread_data=None):
"""
Create a dictionary containing all needed thread fields as returned by
the comments service with dummy data overridden by thread_data
"""
ret = {
"id": "dummy",
"course_id": unicode(self.course.id),
"commentable_id": "dummy",
"group_id": None,
"user_id": str(self.author.id),
"username": self.author.username,
"anonymous": False,
"anonymous_to_peers": False,
"created_at": "1970-01-01T00:00:00Z",
"updated_at": "1970-01-01T00:00:00Z",
"thread_type": "discussion",
"title": "dummy",
"body": "dummy",
"pinned": False,
"closed": False,
"abuse_flaggers": [],
"votes": {"up_count": 0},
"comments_count": 0,
"unread_comments_count": 0,
"children": [],
"resp_total": 0,
}
if thread_data:
ret.update(thread_data)
return ret
def serialize(self, thread):
"""
Create a serializer with an appropriate context and use it to serialize
the given thread, returning the result.
"""
return ThreadSerializer(thread, context=get_context(self.course, self.user)).data
def test_basic(self):
thread = {
"id": "test_thread",
"course_id": unicode(self.course.id),
"commentable_id": "test_topic",
"group_id": None,
"user_id": str(self.author.id),
"username": self.author.username,
"anonymous": False,
"anonymous_to_peers": False,
"created_at": "2015-04-28T00:00:00Z",
"updated_at": "2015-04-28T11:11:11Z",
"thread_type": "discussion",
"title": "Test Title",
"body": "Test body",
"pinned": True,
"closed": False,
"abuse_flaggers": [],
"votes": {"up_count": 4},
"comments_count": 5,
"unread_comments_count": 3,
}
expected = {
"id": "test_thread",
"course_id": unicode(self.course.id),
"topic_id": "test_topic",
"group_id": None,
"group_name": None,
"author": self.author.username,
"author_label": None,
"created_at": "2015-04-28T00:00:00Z",
"updated_at": "2015-04-28T11:11:11Z",
"type": "discussion",
"title": "Test Title",
"raw_body": "Test body",
"pinned": True,
"closed": False,
"following": False,
"abuse_flagged": False,
"voted": False,
"vote_count": 4,
"comment_count": 5,
"unread_comment_count": 3,
}
self.assertEqual(self.serialize(thread), expected)
def test_group(self):
cohort = CohortFactory.create(course_id=self.course.id)
serialized = self.serialize(self.make_cs_thread({"group_id": cohort.id}))
self.assertEqual(serialized["group_id"], cohort.id)
self.assertEqual(serialized["group_name"], cohort.name)
@ddt.data(
(FORUM_ROLE_ADMINISTRATOR, True, False, True),
(FORUM_ROLE_ADMINISTRATOR, False, True, False),
(FORUM_ROLE_MODERATOR, True, False, True),
(FORUM_ROLE_MODERATOR, False, True, False),
(FORUM_ROLE_COMMUNITY_TA, True, False, True),
(FORUM_ROLE_COMMUNITY_TA, False, True, False),
(FORUM_ROLE_STUDENT, True, False, True),
(FORUM_ROLE_STUDENT, False, True, True),
)
@ddt.unpack
def test_anonymity(self, role_name, anonymous, anonymous_to_peers, expected_serialized_anonymous):
"""
Test that content is properly made anonymous.
Content should be anonymous iff the anonymous field is true or the
anonymous_to_peers field is true and the requester does not have a
privileged role.
role_name is the name of the requester's role.
anonymous is the value of the anonymous field in the content.
anonymous_to_peers is the value of the anonymous_to_peers field in the
content.
expected_serialized_anonymous is whether the content should actually be
anonymous in the API output when requested by a user with the given
role.
"""
self.create_role(role_name, [self.user])
serialized = self.serialize(
self.make_cs_thread({"anonymous": anonymous, "anonymous_to_peers": anonymous_to_peers})
)
actual_serialized_anonymous = serialized["author"] is None
self.assertEqual(actual_serialized_anonymous, expected_serialized_anonymous)
@ddt.data(
(FORUM_ROLE_ADMINISTRATOR, False, "staff"),
(FORUM_ROLE_ADMINISTRATOR, True, None),
(FORUM_ROLE_MODERATOR, False, "staff"),
(FORUM_ROLE_MODERATOR, True, None),
(FORUM_ROLE_COMMUNITY_TA, False, "community_ta"),
(FORUM_ROLE_COMMUNITY_TA, True, None),
(FORUM_ROLE_STUDENT, False, None),
(FORUM_ROLE_STUDENT, True, None),
)
@ddt.unpack
def test_author_labels(self, role_name, anonymous, expected_label):
"""
Test correctness of the author_label field.
The label should be "staff", "staff", or "community_ta" for the
Administrator, Moderator, and Community TA roles, respectively, but
the label should not be present if the thread is anonymous.
role_name is the name of the author's role.
anonymous is the value of the anonymous field in the content.
expected_label is the expected value of the author_label field in the
API output.
"""
self.create_role(role_name, [self.author])
serialized = self.serialize(self.make_cs_thread({"anonymous": anonymous}))
self.assertEqual(serialized["author_label"], expected_label)
def test_following(self):
thread_id = "test_thread"
self.register_get_user_response(self.user, subscribed_thread_ids=[thread_id])
serialized = self.serialize(self.make_cs_thread({"id": thread_id}))
self.assertEqual(serialized["following"], True)
def test_abuse_flagged(self):
serialized = self.serialize(self.make_cs_thread({"abuse_flaggers": [str(self.user.id)]}))
self.assertEqual(serialized["abuse_flagged"], True)
def test_voted(self):
thread_id = "test_thread"
self.register_get_user_response(self.user, upvoted_ids=[thread_id])
serialized = self.serialize(self.make_cs_thread({"id": thread_id}))
self.assertEqual(serialized["voted"], True)
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