Add group fields to thread list endpoint

lms/djangoapps/discussion_api/api.py

@@ -15,7 +15,7 @@ from django_comment_common.models import (
 )
 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
+from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id, get_cohort_names
 
 
 def get_course_topics(course, user):
@@ -77,11 +77,24 @@ def get_course_topics(course, user):
     }
 
 
-def _cc_thread_to_api_thread(thread, cc_user, staff_user_ids, ta_user_ids):
+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
@@ -95,6 +108,7 @@ def _cc_thread_to_api_thread(thread, cc_user, staff_user_ids, ta_user_ids):
         for key in [
             "id",
             "course_id",
+            "group_id",
             "created_at",
             "updated_at",
             "type",
@@ -105,6 +119,7 @@ def _cc_thread_to_api_thread(thread, cc_user, staff_user_ids, ta_user_ids):
     }
     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
@@ -123,14 +138,14 @@ def _cc_thread_to_api_thread(thread, cc_user, staff_user_ids, ta_user_ids):
     return ret
 
 
-def get_thread_list(request, course_key, page, page_size):
+def get_thread_list(request, course, page, page_size):
     """
     Return the list of all discussion threads pertaining to the given course
 
     Parameters:
 
     request: The django request objects used for build_absolute_uri
-    course_key: The key of the course to get discussion threads for
+    course: The course to get discussion threads for
     page: The page number (1-indexed) to retrieve
     page_size: The number of threads to retrieve per page
 
@@ -140,14 +155,14 @@ def get_thread_list(request, course_key, page, page_size):
     discussion_api.views.ThreadViewSet for more detail.
     """
     user_is_privileged = Role.objects.filter(
-        course_id=course_key,
+        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()
     threads, result_page, num_pages, _ = Thread.search({
-        "course_id": unicode(course_key),
-        "group_id": None if user_is_privileged else get_cohort_id(request.user, course_key),
+        "course_id": unicode(course.id),
+        "group_id": None if user_is_privileged else get_cohort_id(request.user, course.id),
         "sort_key": "date",
         "sort_order": "desc",
         "page": page,
@@ -163,18 +178,20 @@ def get_thread_list(request, course_key, page, page_size):
         user.id
         for role in Role.objects.filter(
             name__in=[FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR],
-            course_id=course_key
+            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_key)
+        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)
+        _cc_thread_to_api_thread(thread, cc_user, staff_user_ids, ta_user_ids, group_ids_to_names)
         for thread in threads
     ]
     return get_paginated_data(request, results, page, num_pages)

lms/djangoapps/discussion_api/tests/test_api.py

@@ -12,8 +12,6 @@ from pytz import UTC
 from django.http import Http404
 from django.test.client import RequestFactory
 
-from opaque_keys.edx.locator import CourseLocator
-
 from courseware.tests.factories import BetaTesterFactory, StaffFactory
 from discussion_api.api import get_course_topics, get_thread_list
 from discussion_api.tests.utils import CommentsServiceMockMixin
@@ -337,6 +335,7 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
         self.request.user = self.user
         self.course = CourseFactory.create()
         self.author = UserFactory.create()
+        self.cohort = CohortFactory.create(course_id=self.course.id)
 
     def get_thread_list(self, threads, page=1, page_size=1, num_pages=1, course=None):
         """
@@ -345,7 +344,7 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
         """
         course = course or self.course
         self.register_get_threads_response(threads, page, num_pages)
-        ret = get_thread_list(self.request, course.id, page, page_size)
+        ret = get_thread_list(self.request, course, page, page_size)
         return ret
 
     def create_role(self, role_name, users):
@@ -363,6 +362,7 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
             "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,
@@ -414,6 +414,7 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
                 "id": "test_thread_id_0",
                 "course_id": unicode(self.course.id),
                 "commentable_id": "topic_x",
+                "group_id": None,
                 "user_id": str(self.author.id),
                 "username": self.author.username,
                 "anonymous": False,
@@ -434,6 +435,7 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
                 "id": "test_thread_id_1",
                 "course_id": unicode(self.course.id),
                 "commentable_id": "topic_y",
+                "group_id": self.cohort.id,
                 "user_id": str(self.author.id),
                 "username": self.author.username,
                 "anonymous": False,
@@ -454,6 +456,7 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
                 "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,
@@ -476,6 +479,8 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
                 "id": "test_thread_id_0",
                 "course_id": unicode(self.course.id),
                 "topic_id": "topic_x",
+                "group_id": None,
+                "group_name": None,
                 "author": self.author.username,
                 "author_label": None,
                 "created_at": "2015-04-28T00:00:00Z",
@@ -496,6 +501,8 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
                 "id": "test_thread_id_1",
                 "course_id": unicode(self.course.id),
                 "topic_id": "topic_y",
+                "group_id": self.cohort.id,
+                "group_name": self.cohort.name,
                 "author": self.author.username,
                 "author_label": None,
                 "created_at": "2015-04-28T22:22:22Z",
@@ -516,6 +523,8 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
                 "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",
@@ -594,7 +603,7 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
         # Test page past the last one
         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)
+            get_thread_list(self.request, self.course, page=4, page_size=10)
 
     @ddt.data(
         (FORUM_ROLE_ADMINISTRATOR, True, False, True),

lms/djangoapps/discussion_api/tests/test_views.py

@@ -147,6 +147,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             "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,
@@ -167,6 +168,8 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             "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",

lms/djangoapps/discussion_api/views.py

@@ -133,12 +133,11 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
         form = ThreadListGetForm(request.GET)
         if not form.is_valid():
             raise ValidationError(form.errors)
-        course_key = form.cleaned_data["course_id"]
-        self.get_course_or_404(request.user, course_key)
+        course = self.get_course_or_404(request.user, form.cleaned_data["course_id"])
         return Response(
             get_thread_list(
                 request,
-                course_key,
+                course,
                 form.cleaned_data["page"],
                 form.cleaned_data["page_size"]
             )

openedx/core/djangoapps/course_groups/cohorts.py

@@ -293,6 +293,12 @@ def get_course_cohorts(course, assignment_type=None):
     query_set = query_set.filter(cohort__assignment_type=assignment_type) if assignment_type else query_set
     return list(query_set)
 
+
+def get_cohort_names(course):
+    """Return a dict that maps cohort ids to names for the given course"""
+    return {cohort.id: cohort.name for cohort in get_course_cohorts(course)}
+
+
 ### Helpers for cohort management views
 
 

openedx/core/djangoapps/course_groups/tests/test_cohorts.py

@@ -462,6 +462,15 @@ class TestCohorts(ModuleStoreTestCase):
         cohort_set = {c.name for c in cohorts.get_course_cohorts(course)}
         self.assertEqual(cohort_set, {"AutoGroup1", "AutoGroup2", "ManualCohort", "ManualCohort2"})
 
+    def test_get_cohort_names(self):
+        course = modulestore().get_course(self.toy_course_key)
+        cohort1 = CohortFactory(course_id=course.id, name="Cohort1")
+        cohort2 = CohortFactory(course_id=course.id, name="Cohort2")
+        self.assertEqual(
+            cohorts.get_cohort_names(course),
+            {cohort1.id: cohort1.name, cohort2.id: cohort2.name}
+        )
+
     def test_is_commentable_cohorted(self):
         course = modulestore().get_course(self.toy_course_key)
         self.assertFalse(cohorts.is_course_cohorted(course.id))