diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index b3a3fb0b2c333ed72cf2a27cafeda55fa1d52b5d..549e8272561121cdb425c4e741a45d36df6cb0e5 100644 --- a/lms/djangoapps/discussion_api/api.py +++ b/lms/djangoapps/discussion_api/api.py @@ -239,7 +239,10 @@ def get_thread_list( topic_id_list=None, text_search=None, following=False, - view=None): + view=None, + order_by="last_activity_at", + order_direction="desc", +): """ Return the list of all discussion threads pertaining to the given course @@ -253,6 +256,11 @@ def get_thread_list( 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 + order_by: The key in which to sort the threads by. The only values are + "last_activity_at", "comment_count", and "vote_count". The default is + "last_activity_at". + order_direction: The direction in which to sort the threads by. The only + values are "asc" or "desc". The default is "desc". Note that topic_id_list, text_search, and following are mutually exclusive. @@ -263,7 +271,7 @@ def get_thread_list( Raises: - ValidationError: if an invalid value is passed for a field + 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 @@ -273,19 +281,31 @@ def get_thread_list( if exclusive_param_count > 1: # pragma: no cover raise ValueError("More than one mutually exclusive param passed to get_thread_list") + cc_map = {"last_activity_at": "date", "comment_count": "comments", "vote_count": "votes"} + if order_by not in cc_map: + raise ValidationError({ + "order_by": + ["Invalid value. '{}' must be 'last_activity_at', 'comment_count', or 'vote_count'".format(order_by)] + }) + if order_direction not in ["asc", "desc"]: + raise ValidationError({ + "order_direction": ["Invalid value. '{}' must be 'asc' or 'desc'".format(order_direction)] + }) + 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) ), - "sort_key": "date", - "sort_order": "desc", "page": page, "per_page": page_size, "text": text_search, + "sort_key": cc_map.get(order_by), + "sort_order": order_direction, } text_search_rewrite = None diff --git a/lms/djangoapps/discussion_api/forms.py b/lms/djangoapps/discussion_api/forms.py index 95072e02575ff5d8ddb3a65dd911080b212a9433..c2a1be5a6ee6bbaf56b1515694a7cd3638648f3c 100644 --- a/lms/djangoapps/discussion_api/forms.py +++ b/lms/djangoapps/discussion_api/forms.py @@ -54,9 +54,25 @@ class ThreadListGetForm(_PaginationForm): following = NullBooleanField(required=False) view = ChoiceField( choices=[(choice, choice) for choice in ["unread", "unanswered"]], + required=False, + ) + order_by = ChoiceField( + choices=[(choice, choice) for choice in ["last_activity_at", "comment_count", "vote_count"]], + required=False + ) + order_direction = ChoiceField( + choices=[(choice, choice) for choice in ["asc", "desc"]], required=False ) + def clean_order_by(self): + """Return a default choice""" + return self.cleaned_data.get("order_by") or "last_activity_at" + + def clean_order_direction(self): + """Return a default choice""" + return self.cleaned_data.get("order_direction") or "desc" + def clean_course_id(self): """Validate course_id""" value = self.cleaned_data["course_id"] diff --git a/lms/djangoapps/discussion_api/tests/test_api.py b/lms/djangoapps/discussion_api/tests/test_api.py index 21f6c59b01d7922af076b1075c1013cf7fc41e90..ec4f7c1875e2674fd71071197ff3114b4f28cb71 100644 --- a/lms/djangoapps/discussion_api/tests/test_api.py +++ b/lms/djangoapps/discussion_api/tests/test_api.py @@ -836,6 +836,74 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto query: ["true"], }) + @ddt.data( + ("last_activity_at", "date"), + ("comment_count", "comments"), + ("vote_count", "votes") + ) + @ddt.unpack + def test_order_by_query(self, http_query, cc_query): + """ + Tests the order_by parameter + + Arguments: + http_query (str): Query string sent in the http request + cc_query (str): Query string used for the comments client service + """ + self.register_get_threads_response([], page=1, num_pages=1) + result = get_thread_list( + self.request, + self.course.id, + page=1, + page_size=11, + order_by=http_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": [cc_query], + "sort_order": ["desc"], + "page": ["1"], + "per_page": ["11"], + "recursive": ["False"], + }) + + @ddt.data("asc", "desc") + def test_order_direction_query(self, http_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, + order_direction=http_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": [http_query], + "page": ["1"], + "per_page": ["11"], + "recursive": ["False"], + }) + @ddt.ddt class GetCommentListTest(CommentsServiceMockMixin, SharedModuleStoreTestCase): diff --git a/lms/djangoapps/discussion_api/tests/test_forms.py b/lms/djangoapps/discussion_api/tests/test_forms.py index 2f64f22972b5219ba7dca8116c0bef9a42e9eeb2..92af348cddc4fb0b2f73f20f72aa1c187e3ebff0 100644 --- a/lms/djangoapps/discussion_api/tests/test_forms.py +++ b/lms/djangoapps/discussion_api/tests/test_forms.py @@ -95,7 +95,9 @@ class ThreadListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase): "topic_id": [], "text_search": "", "following": None, - "view": "" + "view": "", + "order_by": "last_activity_at", + "order_direction": "desc", } ) @@ -147,6 +149,20 @@ class ThreadListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase): 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.") + def test_invalid_sort_by_choice(self): + self.form_data["order_by"] = "not_a_valid_choice" + self.assert_error( + "order_by", + "Select a valid choice. not_a_valid_choice is not one of the available choices." + ) + + def test_invalid_sort_direction_choice(self): + self.form_data["order_direction"] = "not_a_valid_choice" + self.assert_error( + "order_direction", + "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_views.py b/lms/djangoapps/discussion_api/tests/test_views.py index 490c161669a11bb69f9a671d59ad10e7a093a498..9113001ed0ab560982800aa5a51ca5cce2dc503b 100644 --- a/lms/djangoapps/discussion_api/tests/test_views.py +++ b/lms/djangoapps/discussion_api/tests/test_views.py @@ -327,6 +327,62 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): "/api/v1/users/{}/subscribed_threads".format(self.user.id) ) + @ddt.data( + ("last_activity_at", "date"), + ("comment_count", "comments"), + ("vote_count", "votes") + ) + @ddt.unpack + def test_order_by(self, http_query, cc_query): + """ + Tests the order_by parameter + + Arguments: + http_query (str): Query string sent in the http request + cc_query (str): Query string used for the comments client service + """ + 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), + "order_by": http_query, + } + ) + self.assert_last_query_params({ + "user_id": [unicode(self.user.id)], + "course_id": [unicode(self.course.id)], + "sort_order": ["desc"], + "recursive": ["False"], + "page": ["1"], + "per_page": ["10"], + "sort_key": [cc_query], + }) + + @ddt.data("asc", "desc") + def test_order_direction(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), + "order_direction": query, + } + ) + self.assert_last_query_params({ + "user_id": [unicode(self.user.id)], + "course_id": [unicode(self.course.id)], + "sort_key": ["date"], + "recursive": ["False"], + "page": ["1"], + "per_page": ["10"], + "sort_order": [query], + }) + @httpretty.activate class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): diff --git a/lms/djangoapps/discussion_api/views.py b/lms/djangoapps/discussion_api/views.py index 810946247919ec9992347aeaf4748ee5d81b8a83..3663a8cb241c6f242c25774272da78a11bf0bb4a 100644 --- a/lms/djangoapps/discussion_api/views.py +++ b/lms/djangoapps/discussion_api/views.py @@ -141,6 +141,13 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet): (including the bodies of comments in the thread) matches the search string will be returned. + * order_by: Must be "last_activity_at", "comment_count", or + "vote_count". The key to sort the threads by. The default is + "last_activity_at". + + * order_direction: Must be "asc" or "desc". The direction in which to + sort the threads by. The default is "desc". + * following: If true, retrieve only threads the requesting user is following @@ -245,6 +252,8 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet): form.cleaned_data["text_search"], form.cleaned_data["following"], form.cleaned_data["view"], + form.cleaned_data["order_by"], + form.cleaned_data["order_direction"], ) )