Merge pull request #8571 from edx/clee/discussion-api-view-thread-endorsed

Added get unanswered/unread threads in discussion api

lms/djangoapps/discussion_api/api.py

@@ -231,7 +231,15 @@ def get_course_topics(request, course_key):
     }
 
 
-def get_thread_list(request, course_key, page, page_size, topic_id_list=None, text_search=None, following=False):
+def get_thread_list(
+        request,
+        course_key,
+        page,
+        page_size,
+        topic_id_list=None,
+        text_search=None,
+        following=False,
+        view=None):
     """
     Return the list of all discussion threads pertaining to the given course
 
@@ -244,6 +252,7 @@ def get_thread_list(request, course_key, page, page_size, topic_id_list=None, te
     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
+    view: filters for either "unread" or "unanswered" threads
 
     Note that topic_id_list, text_search, and following are mutually exclusive.
 
@@ -254,6 +263,7 @@ def get_thread_list(request, course_key, page, page_size, topic_id_list=None, te
 
     Raises:
 
+    ValidationError: if an invalid value is passed for a field
     ValueError: if more than one of the mutually exclusive parameters is
       provided
     Http404: if the requesting user does not have access to the requested course
@@ -266,6 +276,7 @@ def get_thread_list(request, course_key, page, page_size, topic_id_list=None, te
     course = _get_course_or_404(course_key, request.user)
     context = get_context(course, request)
     query_params = {
+        "user_id": unicode(request.user.id),
         "group_id": (
             None if context["is_requester_privileged"] else
             get_cohort_id(request.user, course.id)
@@ -276,7 +287,17 @@ def get_thread_list(request, course_key, page, page_size, topic_id_list=None, te
         "per_page": page_size,
         "text": text_search,
     }
+
     text_search_rewrite = None
+
+    if view:
+        if view in ["unread", "unanswered"]:
+            query_params[view] = "true"
+        else:
+            ValidationError({
+                "view": ["Invalid value. '{}' must be 'unread' or 'unanswered'".format(view)]
+            })
+
     if following:
         threads, result_page, num_pages = context["cc_requester"].subscribed_threads(query_params)
     else:

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,
@@ -51,6 +52,10 @@ class ThreadListGetForm(_PaginationForm):
     topic_id = TopicIdField(required=False)
     text_search = CharField(required=False)
     following = NullBooleanField(required=False)
+    view = ChoiceField(
+        choices=[(choice, choice) for choice in ["unread", "unanswered"]],
+        required=False
+    )
 
     def clean_course_id(self):
         """Validate course_id"""

lms/djangoapps/discussion_api/serializers.py

@@ -185,6 +185,8 @@ class ThreadSerializer(_ContentSerializer):
     comment_list_url = serializers.SerializerMethodField("get_comment_list_url")
     endorsed_comment_list_url = serializers.SerializerMethodField("get_endorsed_comment_list_url")
     non_endorsed_comment_list_url = serializers.SerializerMethodField("get_non_endorsed_comment_list_url")
+    read = serializers.BooleanField(read_only=True)
+    has_endorsed = serializers.BooleanField(read_only=True, source="endorsed")
 
     non_updatable_fields = NON_UPDATABLE_THREAD_FIELDS
 

lms/djangoapps/discussion_api/tests/test_api.py

@@ -521,6 +521,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTest
         self.get_thread_list([], topic_id_list=["topic_x", "topic_meow"])
         self.assertEqual(urlparse(httpretty.last_request().path).path, "/api/v1/threads")
         self.assert_last_query_params({
+            "user_id": [unicode(self.user.id)],
             "course_id": [unicode(self.course.id)],
             "sort_key": ["date"],
             "sort_order": ["desc"],
@@ -533,6 +534,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTest
     def test_basic_query_params(self):
         self.get_thread_list([], page=6, page_size=14)
         self.assert_last_query_params({
+            "user_id": [unicode(self.user.id)],
             "course_id": [unicode(self.course.id)],
             "sort_key": ["date"],
             "sort_order": ["desc"],
@@ -564,6 +566,8 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTest
                 "votes": {"up_count": 4},
                 "comments_count": 5,
                 "unread_comments_count": 3,
+                "endorsed": True,
+                "read": True,
             },
             {
                 "type": "thread",
@@ -586,6 +590,8 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTest
                 "votes": {"up_count": 9},
                 "comments_count": 18,
                 "unread_comments_count": 0,
+                "endorsed": False,
+                "read": False,
             },
         ]
         expected_threads = [
@@ -615,6 +621,8 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTest
                 "endorsed_comment_list_url": None,
                 "non_endorsed_comment_list_url": None,
                 "editable_fields": ["abuse_flagged", "following", "voted"],
+                "has_endorsed": True,
+                "read": True,
             },
             {
                 "id": "test_thread_id_1",
@@ -646,6 +654,8 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTest
                     "http://testserver/api/discussion/v1/comments/?thread_id=test_thread_id_1&endorsed=False"
                 ),
                 "editable_fields": ["abuse_flagged", "following", "voted"],
+                "has_endorsed": False,
+                "read": False,
             },
         ]
         self.assertEqual(
@@ -735,6 +745,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTest
             }
         )
         self.assert_last_query_params({
+            "user_id": [unicode(self.user.id)],
             "course_id": [unicode(self.course.id)],
             "sort_key": ["date"],
             "sort_order": ["desc"],
@@ -762,6 +773,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTest
             "/api/v1/users/{}/subscribed_threads".format(self.user.id)
         )
         self.assert_last_query_params({
+            "user_id": [unicode(self.user.id)],
             "course_id": [unicode(self.course.id)],
             "sort_key": ["date"],
             "sort_order": ["desc"],
@@ -769,6 +781,35 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTest
             "per_page": ["11"],
         })
 
+    @ddt.data("unanswered", "unread")
+    def test_view_query(self, query):
+        self.register_get_threads_response([], page=1, num_pages=1)
+        result = get_thread_list(
+            self.request,
+            self.course.id,
+            page=1,
+            page_size=11,
+            view=query,
+        )
+        self.assertEqual(
+            result,
+            {"results": [], "next": None, "previous": None, "text_search_rewrite": None}
+        )
+        self.assertEqual(
+            urlparse(httpretty.last_request().path).path,
+            "/api/v1/threads"
+        )
+        self.assert_last_query_params({
+            "user_id": [unicode(self.user.id)],
+            "course_id": [unicode(self.course.id)],
+            "sort_key": ["date"],
+            "sort_order": ["desc"],
+            "page": ["1"],
+            "per_page": ["11"],
+            "recursive": ["False"],
+            query: ["true"],
+        })
+
 
 @ddt.ddt
 class GetCommentListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
@@ -1240,6 +1281,8 @@ class CreateThreadTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTestC
             "endorsed_comment_list_url": None,
             "non_endorsed_comment_list_url": None,
             "editable_fields": ["abuse_flagged", "following", "raw_body", "title", "topic_id", "type", "voted"],
+            'read': False,
+            'has_endorsed': False
         }
         self.assertEqual(actual, expected)
         self.assertEqual(
@@ -1745,6 +1788,8 @@ class UpdateThreadTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTestC
             "endorsed_comment_list_url": None,
             "non_endorsed_comment_list_url": None,
             "editable_fields": ["abuse_flagged", "following", "raw_body", "title", "topic_id", "type", "voted"],
+            'read': False,
+            'has_endorsed': False,
         }
         self.assertEqual(actual, expected)
         self.assertEqual(

lms/djangoapps/discussion_api/tests/test_forms.py

@@ -95,6 +95,7 @@ class ThreadListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase):
                 "topic_id": [],
                 "text_search": "",
                 "following": None,
+                "view": ""
             }
         )
 
@@ -142,6 +143,10 @@ class ThreadListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase):
             "The following query parameters are mutually exclusive: topic_id, text_search, following"
         )
 
+    def test_invalid_view_choice(self):
+        self.form_data["view"] = "not_a_valid_choice"
+        self.assert_error("view", "Select a valid choice. not_a_valid_choice is not one of the available choices.")
+
 
 class CommentListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase):
     """Tests for CommentListGetForm"""

lms/djangoapps/discussion_api/tests/test_serializers.py

@@ -138,6 +138,8 @@ class ThreadSerializerSerializationTest(SerializerTestMixin, ModuleStoreTestCase
             "course_id": unicode(self.course.id),
             "user_id": str(self.author.id),
             "username": self.author.username,
+            "read": True,
+            "endorsed": True
         }
         merged_overrides.update(overrides)
         return make_minimal_cs_thread(merged_overrides)
@@ -171,6 +173,8 @@ class ThreadSerializerSerializationTest(SerializerTestMixin, ModuleStoreTestCase
             "votes": {"up_count": 4},
             "comments_count": 5,
             "unread_comments_count": 3,
+            "read": False,
+            "endorsed": False
         }
         expected = {
             "id": "test_thread",
@@ -198,6 +202,8 @@ class ThreadSerializerSerializationTest(SerializerTestMixin, ModuleStoreTestCase
             "endorsed_comment_list_url": None,
             "non_endorsed_comment_list_url": None,
             "editable_fields": ["abuse_flagged", "following", "voted"],
+            "read": False,
+            "has_endorsed": False
         }
         self.assertEqual(self.serialize(thread), expected)
 
@@ -424,6 +430,8 @@ class ThreadSerializerDeserializationTest(CommentsServiceMockMixin, UrlResetMixi
             "title": "Original Title",
             "body": "Original body",
             "user_id": str(self.user.id),
+            "read": "False",
+            "endorsed": "False"
         }))
 
     def save_and_reserialize(self, data, instance=None):

lms/djangoapps/discussion_api/tests/test_views.py

@@ -5,6 +5,7 @@ from datetime import datetime
 import json
 from urlparse import urlparse
 
+import ddt
 import httpretty
 import mock
 from pytz import UTC
@@ -134,6 +135,7 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
         )
 
 
+@ddt.ddt
 @httpretty.activate
 class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
     """Tests for ThreadViewSet list"""
@@ -181,6 +183,8 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             "votes": {"up_count": 4},
             "comments_count": 5,
             "unread_comments_count": 3,
+            "read": False,
+            "endorsed": False
         }]
         expected_threads = [{
             "id": "test_thread",
@@ -208,6 +212,8 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             "endorsed_comment_list_url": None,
             "non_endorsed_comment_list_url": None,
             "editable_fields": ["abuse_flagged", "following", "voted"],
+            "read": False,
+            "has_endorsed": False
         }]
         self.register_get_threads_response(source_threads, page=1, num_pages=2)
         response = self.client.get(self.url, {"course_id": unicode(self.course.id)})
@@ -222,6 +228,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             }
         )
         self.assert_last_query_params({
+            "user_id": [unicode(self.user.id)],
             "course_id": [unicode(self.course.id)],
             "sort_key": ["date"],
             "sort_order": ["desc"],
@@ -230,6 +237,29 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             "recursive": ["False"],
         })
 
+    @ddt.data("unread", "unanswered")
+    def test_view_query(self, query):
+        threads = [make_minimal_cs_thread()]
+        self.register_get_user_response(self.user)
+        self.register_get_threads_response(threads, page=1, num_pages=1)
+        self.client.get(
+            self.url,
+            {
+                "course_id": unicode(self.course.id),
+                "view": query,
+            }
+        )
+        self.assert_last_query_params({
+            "user_id": [unicode(self.user.id)],
+            "course_id": [unicode(self.course.id)],
+            "sort_key": ["date"],
+            "sort_order": ["desc"],
+            "recursive": ["False"],
+            "page": ["1"],
+            "per_page": ["10"],
+            query: ["true"],
+        })
+
     def test_pagination(self):
         self.register_get_user_response(self.user)
         self.register_get_threads_response([], page=1, num_pages=1)
@@ -243,6 +273,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             {"developer_message": "Not found."}
         )
         self.assert_last_query_params({
+            "user_id": [unicode(self.user.id)],
             "course_id": [unicode(self.course.id)],
             "sort_key": ["date"],
             "sort_order": ["desc"],
@@ -264,6 +295,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             {"results": [], "next": None, "previous": None, "text_search_rewrite": None}
         )
         self.assert_last_query_params({
+            "user_id": [unicode(self.user.id)],
             "course_id": [unicode(self.course.id)],
             "sort_key": ["date"],
             "sort_order": ["desc"],
@@ -344,6 +376,8 @@ class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             "endorsed_comment_list_url": None,
             "non_endorsed_comment_list_url": None,
             "editable_fields": ["abuse_flagged", "following", "raw_body", "title", "topic_id", "type", "voted"],
+            "read": False,
+            "has_endorsed": False
         }
         response = self.client.post(
             self.url,
@@ -435,6 +469,8 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest
             "endorsed_comment_list_url": None,
             "non_endorsed_comment_list_url": None,
             "editable_fields": ["abuse_flagged", "following", "raw_body", "title", "topic_id", "type", "voted"],
+            "read": False,
+            "has_endorsed": False
         }
         response = self.client.patch(  # pylint: disable=no-member
             self.url,

lms/djangoapps/discussion_api/tests/utils.py

@@ -330,6 +330,8 @@ def make_minimal_cs_thread(overrides=None):
         "unread_comments_count": 0,
         "children": [],
         "resp_total": 0,
+        "read": False,
+        "endorsed": False,
     }
     ret.update(overrides or {})
     return ret

lms/djangoapps/discussion_api/views.py

@@ -117,7 +117,7 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
           "topic_id": "quux",
           "type": "discussion",
           "title": "Title text",
-          "body": "Body text"
+          "raw_body": "Body text"
         }
 
         PATCH /api/discussion/v1/threads/thread_id
@@ -144,6 +144,10 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
         * following: If true, retrieve only threads the requesting user is
             following
 
+        * view: "unread" for threads the requesting user has not read, or
+            "unanswered" for question threads with no marked answer. Only one
+            can be selected.
+
         The topic_id, text_search, and following parameters are mutually
         exclusive (i.e. only one may be specified in a request)
 
@@ -212,6 +216,10 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
         * editable_fields: The fields that the requesting user is allowed to
             modify with a PATCH request
 
+        * read: Boolean indicating whether the user has read this thread
+
+        * has_endorsed: Boolean indicating whether this thread has been answered
+
     **DELETE response values:
 
         No content is returned for a DELETE request
@@ -236,6 +244,7 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
                 form.cleaned_data["topic_id"],
                 form.cleaned_data["text_search"],
                 form.cleaned_data["following"],
+                form.cleaned_data["view"],
             )
         )