Commit b1a9d334 by Greg Price

Add authorship data to thread list endpoint

parent fa6e5338
...@@ -77,12 +77,19 @@ def get_course_topics(course, user): ...@@ -77,12 +77,19 @@ def get_course_topics(course, user):
} }
def _cc_thread_to_api_thread(thread, cc_user): def _cc_thread_to_api_thread(thread, cc_user, staff_user_ids, ta_user_ids):
""" """
Convert a thread data dict from the comment_client format (which is a direct 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 representation of the format returned by the comments service) to the format
used in this API used in this API
""" """
is_anonymous = (
thread["anonymous"] or
(
thread["anonymous_to_peers"] and
int(cc_user["id"]) not in (staff_user_ids | ta_user_ids)
)
)
ret = { ret = {
key: thread[key] key: thread[key]
for key in [ for key in [
...@@ -98,6 +105,13 @@ def _cc_thread_to_api_thread(thread, cc_user): ...@@ -98,6 +105,13 @@ def _cc_thread_to_api_thread(thread, cc_user):
} }
ret.update({ ret.update({
"topic_id": thread["commentable_id"], "topic_id": thread["commentable_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
),
"raw_body": thread["body"], "raw_body": thread["body"],
"following": thread["id"] in cc_user["subscribed_thread_ids"], "following": thread["id"] in cc_user["subscribed_thread_ids"],
"abuse_flagged": cc_user["id"] in thread["abuse_flaggers"], "abuse_flagged": cc_user["id"] in thread["abuse_flaggers"],
...@@ -144,6 +158,23 @@ def get_thread_list(request, course_key, page, page_size): ...@@ -144,6 +158,23 @@ def get_thread_list(request, course_key, page, page_size):
# behavior and return a 404 in that case # behavior and return a 404 in that case
if result_page != page: if result_page != page:
raise Http404 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_key
)
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_key)
for user in role.users.all()
}
results = [_cc_thread_to_api_thread(thread, cc_user) for thread in threads] results = [
_cc_thread_to_api_thread(thread, cc_user, staff_user_ids, ta_user_ids)
for thread in threads
]
return get_paginated_data(request, results, page, num_pages) return get_paginated_data(request, results, page, num_pages)
...@@ -336,6 +336,7 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): ...@@ -336,6 +336,7 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
self.request = RequestFactory().get("/test_path") self.request = RequestFactory().get("/test_path")
self.request.user = self.user self.request.user = self.user
self.course = CourseFactory.create() self.course = CourseFactory.create()
self.author = UserFactory.create()
def get_thread_list(self, threads, page=1, page_size=1, num_pages=1, course=None): def get_thread_list(self, threads, page=1, page_size=1, num_pages=1, course=None):
""" """
...@@ -347,6 +348,40 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): ...@@ -347,6 +348,40 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
ret = get_thread_list(self.request, course.id, page, page_size) ret = get_thread_list(self.request, course.id, page, page_size)
return ret return ret
def create_role(self, role_name, users):
"""Create a Role in self.course with the given name and users"""
role = Role.objects.create(name=role_name, course_id=self.course.id)
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",
"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",
"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_empty(self): def test_empty(self):
self.assertEqual( self.assertEqual(
self.get_thread_list([]), self.get_thread_list([]),
...@@ -379,6 +414,10 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): ...@@ -379,6 +414,10 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
"id": "test_thread_id_0", "id": "test_thread_id_0",
"course_id": unicode(self.course.id), "course_id": unicode(self.course.id),
"commentable_id": "topic_x", "commentable_id": "topic_x",
"user_id": str(self.author.id),
"username": self.author.username,
"anonymous": False,
"anonymous_to_peers": False,
"created_at": "2015-04-28T00:00:00Z", "created_at": "2015-04-28T00:00:00Z",
"updated_at": "2015-04-28T11:11:11Z", "updated_at": "2015-04-28T11:11:11Z",
"type": "discussion", "type": "discussion",
...@@ -395,6 +434,10 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): ...@@ -395,6 +434,10 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
"id": "test_thread_id_1", "id": "test_thread_id_1",
"course_id": unicode(self.course.id), "course_id": unicode(self.course.id),
"commentable_id": "topic_y", "commentable_id": "topic_y",
"user_id": str(self.author.id),
"username": self.author.username,
"anonymous": False,
"anonymous_to_peers": False,
"created_at": "2015-04-28T22:22:22Z", "created_at": "2015-04-28T22:22:22Z",
"updated_at": "2015-04-28T00:33:33Z", "updated_at": "2015-04-28T00:33:33Z",
"type": "question", "type": "question",
...@@ -411,6 +454,10 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): ...@@ -411,6 +454,10 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
"id": "test_thread_id_2", "id": "test_thread_id_2",
"course_id": unicode(self.course.id), "course_id": unicode(self.course.id),
"commentable_id": "topic_x", "commentable_id": "topic_x",
"user_id": str(self.author.id),
"username": self.author.username,
"anonymous": False,
"anonymous_to_peers": False,
"created_at": "2015-04-28T00:44:44Z", "created_at": "2015-04-28T00:44:44Z",
"updated_at": "2015-04-28T00:55:55Z", "updated_at": "2015-04-28T00:55:55Z",
"type": "discussion", "type": "discussion",
...@@ -429,6 +476,8 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): ...@@ -429,6 +476,8 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
"id": "test_thread_id_0", "id": "test_thread_id_0",
"course_id": unicode(self.course.id), "course_id": unicode(self.course.id),
"topic_id": "topic_x", "topic_id": "topic_x",
"author": self.author.username,
"author_label": None,
"created_at": "2015-04-28T00:00:00Z", "created_at": "2015-04-28T00:00:00Z",
"updated_at": "2015-04-28T11:11:11Z", "updated_at": "2015-04-28T11:11:11Z",
"type": "discussion", "type": "discussion",
...@@ -447,6 +496,8 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): ...@@ -447,6 +496,8 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
"id": "test_thread_id_1", "id": "test_thread_id_1",
"course_id": unicode(self.course.id), "course_id": unicode(self.course.id),
"topic_id": "topic_y", "topic_id": "topic_y",
"author": self.author.username,
"author_label": None,
"created_at": "2015-04-28T22:22:22Z", "created_at": "2015-04-28T22:22:22Z",
"updated_at": "2015-04-28T00:33:33Z", "updated_at": "2015-04-28T00:33:33Z",
"type": "question", "type": "question",
...@@ -465,6 +516,8 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): ...@@ -465,6 +516,8 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
"id": "test_thread_id_2", "id": "test_thread_id_2",
"course_id": unicode(self.course.id), "course_id": unicode(self.course.id),
"topic_id": "topic_x", "topic_id": "topic_x",
"author": self.author.username,
"author_label": None,
"created_at": "2015-04-28T00:44:44Z", "created_at": "2015-04-28T00:44:44Z",
"updated_at": "2015-04-28T00:55:55Z", "updated_at": "2015-04-28T00:55:55Z",
"type": "discussion", "type": "discussion",
...@@ -542,3 +595,69 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): ...@@ -542,3 +595,69 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
self.register_get_threads_response([], page=3, num_pages=3) self.register_get_threads_response([], page=3, num_pages=3)
with self.assertRaises(Http404): with self.assertRaises(Http404):
get_thread_list(self.request, self.course.id, page=4, page_size=10) 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)
...@@ -121,6 +121,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): ...@@ -121,6 +121,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
"""Tests for ThreadViewSet list""" """Tests for ThreadViewSet list"""
def setUp(self): def setUp(self):
super(ThreadViewSetListTest, self).setUp() super(ThreadViewSetListTest, self).setUp()
self.author = UserFactory.create()
self.url = reverse("thread-list") self.url = reverse("thread-list")
def test_course_id_missing(self): def test_course_id_missing(self):
...@@ -146,6 +147,10 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): ...@@ -146,6 +147,10 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
"id": "test_thread", "id": "test_thread",
"course_id": unicode(self.course.id), "course_id": unicode(self.course.id),
"commentable_id": "test_topic", "commentable_id": "test_topic",
"user_id": str(self.author.id),
"username": self.author.username,
"anonymous": False,
"anonymous_to_peers": False,
"created_at": "2015-04-28T00:00:00Z", "created_at": "2015-04-28T00:00:00Z",
"updated_at": "2015-04-28T11:11:11Z", "updated_at": "2015-04-28T11:11:11Z",
"type": "discussion", "type": "discussion",
...@@ -162,6 +167,8 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): ...@@ -162,6 +167,8 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
"id": "test_thread", "id": "test_thread",
"course_id": unicode(self.course.id), "course_id": unicode(self.course.id),
"topic_id": "test_topic", "topic_id": "test_topic",
"author": self.author.username,
"author_label": None,
"created_at": "2015-04-28T00:00:00Z", "created_at": "2015-04-28T00:00:00Z",
"updated_at": "2015-04-28T11:11:11Z", "updated_at": "2015-04-28T11:11:11Z",
"type": "discussion", "type": "discussion",
......
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