Merge pull request #8544 from edx/gprice/discussion-api-thread-list-following

Add followed thread retrieval to discussion API

lms/djangoapps/discussion_api/api.py

@@ -92,14 +92,28 @@ def _get_comment_and_context(request, comment_id):
         raise Http404
 
 
-def get_thread_list_url(request, course_key, topic_id_list=None):
+def _is_user_author_or_privileged(cc_content, context):
+    """
+    Check if the user is the author of a content object or a privileged user.
+
+    Returns:
+        Boolean
+    """
+    return (
+        context["is_requester_privileged"] or
+        context["cc_requester"]["id"] == cc_content["user_id"]
+    )
+
+
+def get_thread_list_url(request, course_key, topic_id_list=None, following=False):
     """
     Returns the URL for the thread_list_url field, given a list of topic_ids
     """
     path = reverse("thread-list")
     query_list = (
         [("course_id", unicode(course_key))] +
-        [("topic_id", topic_id) for topic_id in topic_id_list or []]
+        [("topic_id", topic_id) for topic_id in topic_id_list or []] +
+        ([("following", following)] if following else [])
     )
     return request.build_absolute_uri(urlunparse(("", "", path, "", urlencode(query_list), "")))
 
@@ -132,7 +146,8 @@ def get_course(request, course_key):
             {"start": blackout["start"].isoformat(), "end": blackout["end"].isoformat()}
             for blackout in course.get_discussion_blackout_datetimes()
         ],
-        "thread_list_url": get_thread_list_url(request, course_key, topic_id_list=[]),
+        "thread_list_url": get_thread_list_url(request, course_key),
+        "following_thread_list_url": get_thread_list_url(request, course_key, following=True),
         "topics_url": request.build_absolute_uri(
             reverse("course_topics", kwargs={"course_id": course_key})
         )
@@ -211,7 +226,7 @@ def get_course_topics(request, course_key):
     }
 
 
-def get_thread_list(request, course_key, page, page_size, topic_id_list=None, text_search=None):
+def get_thread_list(request, course_key, page, page_size, topic_id_list=None, text_search=None, following=False):
     """
     Return the list of all discussion threads pertaining to the given course
 
@@ -223,8 +238,9 @@ def get_thread_list(request, course_key, page, page_size, topic_id_list=None, te
     page_size: The number of threads to retrieve per page
     topic_id_list: The list of topic_ids to get the discussion threads for
     text_search A text search query string to match
+    following: If true, retrieve only threads the requester is following
 
-    Note that topic_id_list and text_search are mutually exclusive.
+    Note that topic_id_list, text_search, and following are mutually exclusive.
 
     Returns:
 
@@ -238,15 +254,13 @@ def get_thread_list(request, course_key, page, page_size, topic_id_list=None, te
     Http404: if the requesting user does not have access to the requested course
       or a page beyond the last is requested
     """
-    exclusive_param_count = sum(1 for param in [topic_id_list, text_search] if param)
+    exclusive_param_count = sum(1 for param in [topic_id_list, text_search, following] if param)
     if exclusive_param_count > 1:  # pragma: no cover
         raise ValueError("More than one mutually exclusive param passed to get_thread_list")
 
     course = _get_course_or_404(course_key, request.user)
     context = get_context(course, request)
-    topic_ids_csv = ",".join(topic_id_list) if topic_id_list else None
-    threads, result_page, num_pages, text_search_rewrite = Thread.search({
-        "course_id": unicode(course.id),
+    query_params = {
         "group_id": (
             None if context["is_requester_privileged"] else
             get_cohort_id(request.user, course.id)
@@ -255,9 +269,16 @@ def get_thread_list(request, course_key, page, page_size, topic_id_list=None, te
         "sort_order": "desc",
         "page": page,
         "per_page": page_size,
-        "commentable_ids": topic_ids_csv,
         "text": text_search,
-    })
+    }
+    text_search_rewrite = None
+    if following:
+        threads, result_page, num_pages = context["cc_requester"].subscribed_threads(query_params)
+    else:
+        query_params["course_id"] = unicode(course.id)
+        query_params["commentable_ids"] = ",".join(topic_id_list) if topic_id_list else None
+        query_params["text"] = text_search
+        threads, result_page, num_pages, text_search_rewrite = Thread.search(query_params)
     # The comments service returns the last page of results if the requested
     # page is beyond the last page, but we want be consistent with DRF's general
     # behavior and return a 404 in that case

lms/djangoapps/discussion_api/forms.py

@@ -5,6 +5,7 @@ from django.core.exceptions import ValidationError
 from django.forms import (
     BooleanField,
     CharField,
+    ChoiceField,
     Field,
     Form,
     IntegerField,
@@ -45,11 +46,12 @@ class ThreadListGetForm(_PaginationForm):
     """
     A form to validate query parameters in the thread list retrieval endpoint
     """
-    EXCLUSIVE_PARAMS = ["topic_id", "text_search"]
+    EXCLUSIVE_PARAMS = ["topic_id", "text_search", "following"]
 
     course_id = CharField()
     topic_id = TopicIdField(required=False)
     text_search = CharField(required=False)
+    following = NullBooleanField(required=False)
 
     def clean_course_id(self):
         """Validate course_id"""
@@ -59,6 +61,14 @@ class ThreadListGetForm(_PaginationForm):
         except InvalidKeyError:
             raise ValidationError("'{}' is not a valid course id".format(value))
 
+    def clean_following(self):
+        """Validate following"""
+        value = self.cleaned_data["following"]
+        if value is False:
+            raise ValidationError("The value of the 'following' parameter must be true.")
+        else:
+            return value
+
     def clean(self):
         cleaned_data = super(ThreadListGetForm, self).clean()
         exclusive_params_count = sum(

lms/djangoapps/discussion_api/serializers.py

@@ -47,6 +47,8 @@ def get_context(course, request, thread=None):
         for user in role.users.all()
     }
     requester = request.user
+    cc_requester = CommentClientUser.from_django_user(requester).retrieve()
+    cc_requester["course_id"] = course.id
     return {
         "course": course,
         "request": request,
@@ -56,7 +58,7 @@ def get_context(course, request, thread=None):
         "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": CommentClientUser.from_django_user(requester).retrieve(),
+        "cc_requester": cc_requester,
     }
 
 

lms/djangoapps/discussion_api/tests/test_api.py

@@ -99,6 +99,9 @@ class GetCourseTest(UrlResetMixin, ModuleStoreTestCase):
                 "id": unicode(self.course.id),
                 "blackouts": [],
                 "thread_list_url": "http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz",
+                "following_thread_list_url": (
+                    "http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz&following=True"
+                ),
                 "topics_url": "http://testserver/api/discussion/v1/course_topics/x/y/z",
             }
         )
@@ -741,6 +744,31 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTest
             "text": ["test search string"],
         })
 
+    def test_following(self):
+        self.register_subscribed_threads_response(self.user, [], page=1, num_pages=1)
+        result = get_thread_list(
+            self.request,
+            self.course.id,
+            page=1,
+            page_size=11,
+            following=True,
+        )
+        self.assertEqual(
+            result,
+            {"results": [], "next": None, "previous": None, "text_search_rewrite": None}
+        )
+        self.assertEqual(
+            urlparse(httpretty.last_request().path).path,
+            "/api/v1/users/{}/subscribed_threads".format(self.user.id)
+        )
+        self.assert_last_query_params({
+            "course_id": [unicode(self.course.id)],
+            "sort_key": ["date"],
+            "sort_order": ["desc"],
+            "page": ["1"],
+            "per_page": ["11"],
+        })
+
 
 @ddt.ddt
 class GetCommentListTest(CommentsServiceMockMixin, ModuleStoreTestCase):

lms/djangoapps/discussion_api/tests/test_forms.py

@@ -94,6 +94,7 @@ class ThreadListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase):
                 "page_size": 13,
                 "topic_id": [],
                 "text_search": "",
+                "following": None,
             }
         )
 
@@ -125,12 +126,20 @@ class ThreadListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase):
         self.form_data.setlist("topic_id", ["", "not empty"])
         self.assert_error("topic_id", "This field cannot be empty.")
 
-    @ddt.data(*itertools.combinations(["topic_id", "text_search"], 2))
+    def test_following_true(self):
+        self.form_data["following"] = "True"
+        self.assert_field_value("following", True)
+
+    def test_following_false(self):
+        self.form_data["following"] = "False"
+        self.assert_error("following", "The value of the 'following' parameter must be true.")
+
+    @ddt.data(*itertools.combinations(["topic_id", "text_search", "following"], 2))
     def test_mutually_exclusive(self, params):
-        self.form_data.update({param: "dummy" for param in params})
+        self.form_data.update({param: "True" for param in params})
         self.assert_error(
             "__all__",
-            "The following query parameters are mutually exclusive: topic_id, text_search"
+            "The following query parameters are mutually exclusive: topic_id, text_search, following"
         )
 
 

lms/djangoapps/discussion_api/tests/test_views.py

@@ -92,6 +92,9 @@ class CourseViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
                 "id": unicode(self.course.id),
                 "blackouts": [],
                 "thread_list_url": "http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz",
+                "following_thread_list_url": (
+                    "http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz&following=True"
+                ),
                 "topics_url": "http://testserver/api/discussion/v1/course_topics/x/y/z",
             }
         )
@@ -270,6 +273,28 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             "text": ["test search string"],
         })
 
+    def test_following(self):
+        self.register_get_user_response(self.user)
+        self.register_subscribed_threads_response(self.user, [], page=1, num_pages=1)
+        response = self.client.get(
+            self.url,
+            {
+                "course_id": unicode(self.course.id),
+                "page": "1",
+                "page_size": "4",
+                "following": "True",
+            }
+        )
+        self.assert_response_correct(
+            response,
+            200,
+            {"results": [], "next": None, "previous": None, "text_search_rewrite": None}
+        )
+        self.assertEqual(
+            urlparse(httpretty.last_request().path).path,
+            "/api/v1/users/{}/subscribed_threads".format(self.user.id)
+        )
+
 
 @httpretty.activate
 class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):

lms/djangoapps/discussion_api/tests/utils.py

@@ -191,6 +191,19 @@ class CommentsServiceMockMixin(object):
             status=200
         )
 
+    def register_subscribed_threads_response(self, user, threads, page, num_pages):
+        """Register a mock response for GET on the CS user instance endpoint"""
+        httpretty.register_uri(
+            httpretty.GET,
+            "http://localhost:4567/api/v1/users/{}/subscribed_threads".format(user.id),
+            body=json.dumps({
+                "collection": threads,
+                "page": page,
+                "num_pages": num_pages,
+            }),
+            status=200
+        )
+
     def register_subscription_response(self, user):
         """
         Register a mock response for POST and DELETE on the CS user subscription

lms/djangoapps/discussion_api/views.py

@@ -141,6 +141,12 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
             (including the bodies of comments in the thread) matches the search
             string will be returned.
 
+        * following: If true, retrieve only threads the requesting user is
+            following
+
+        The topic_id, text_search, and following parameters are mutually
+        exclusive (i.e. only one may be specified in a request)
+
     **POST Parameters**:
 
         * course_id (required): The course to create the thread in
@@ -229,6 +235,7 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
                 form.cleaned_data["page_size"],
                 form.cleaned_data["topic_id"],
                 form.cleaned_data["text_search"],
+                form.cleaned_data["following"],
             )
         )