diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index 85c3a9fc56..b3a3fb0b2c 100644 --- a/lms/djangoapps/discussion_api/api.py +++ b/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: diff --git a/lms/djangoapps/discussion_api/forms.py b/lms/djangoapps/discussion_api/forms.py index e53639ff69..95072e0257 100644 --- a/lms/djangoapps/discussion_api/forms.py +++ b/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""" diff --git a/lms/djangoapps/discussion_api/serializers.py b/lms/djangoapps/discussion_api/serializers.py index 8dc68fe1de..f0c414274b 100644 --- a/lms/djangoapps/discussion_api/serializers.py +++ b/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 diff --git a/lms/djangoapps/discussion_api/tests/test_api.py b/lms/djangoapps/discussion_api/tests/test_api.py index 619484e86c..8768858fde 100644 --- a/lms/djangoapps/discussion_api/tests/test_api.py +++ b/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( diff --git a/lms/djangoapps/discussion_api/tests/test_forms.py b/lms/djangoapps/discussion_api/tests/test_forms.py index 1d3c8e87e5..2f64f22972 100644 --- a/lms/djangoapps/discussion_api/tests/test_forms.py +++ b/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""" diff --git a/lms/djangoapps/discussion_api/tests/test_serializers.py b/lms/djangoapps/discussion_api/tests/test_serializers.py index cae90175d9..636067b160 100644 --- a/lms/djangoapps/discussion_api/tests/test_serializers.py +++ b/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): diff --git a/lms/djangoapps/discussion_api/tests/test_views.py b/lms/djangoapps/discussion_api/tests/test_views.py index 71807c9558..490c161669 100644 --- a/lms/djangoapps/discussion_api/tests/test_views.py +++ b/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, diff --git a/lms/djangoapps/discussion_api/tests/utils.py b/lms/djangoapps/discussion_api/tests/utils.py index 61ed6974fa..5e72528833 100644 --- a/lms/djangoapps/discussion_api/tests/utils.py +++ b/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 diff --git a/lms/djangoapps/discussion_api/views.py b/lms/djangoapps/discussion_api/views.py index 9002187fe6..8109462479 100644 --- a/lms/djangoapps/discussion_api/views.py +++ b/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"], ) )