Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove support for sort_order (order_direction).

Browse Source 
- Default of "desc" is all that is in use, and remains as-is.
- Discussion API keeps order_direction param, to minimize changes,
but it only accepts "desc".
- Discussion webapp simply no longer sends in the sort order.
This commit is contained in:
Robert Raposa
2016-09-13 17:15:17 -04:00
parent 2abe3f7e01
commit f774ccf3f8
7 changed files with 44 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
lms/djangoapps/discussion/views.py
View File

@@ -69,22 +69,21 @@ def get_threads(request, course, user_info, discussion_id=None, per_page=THREADS
if something goes wrong, or ValueError if the group_id is invalid.
Arguments:
request: The user request.
course: The course object.
user_info: The comment client User object as a dict.
discussion_id: Optional discussion id/commentable id for context.
per_page: Optional number of threads per page.
request (WSGIRequest): The user request.
course (CourseDescriptorWithMixins): The course object.
user_info (dict): The comment client User object as a dict.
discussion_id (unicode): Optional discussion id/commentable id for context.
per_page (int): Optional number of threads per page.
Returns:
A tuple of the threads and the query parameters used for the search.
:return:
(tuple of list, dict): A tuple of the list of threads and a dict of the
query parameters used for the search.
"""
default_query_params = {
'page': 1,
'per_page': per_page,
'sort_key': 'activity',
'sort_order': 'desc',
'text': '',
'course_id': unicode(course.id),
'user_id': request.user.id,
@@ -122,7 +121,6 @@ def get_threads(request, course, user_info, discussion_id=None, per_page=THREADS
[
'page',
'sort_key',
'sort_order',
'text',
'commentable_ids',
'flagged',
@@ -483,7 +481,6 @@ def followed_threads(request, course_key, user_id):
'page': 1,
'per_page': THREADS_PER_PAGE, # more than threads_per_page to show more activities
'sort_key': 'date',
'sort_order': 'desc',
}
query_params = merge_dict(
@@ -494,7 +491,6 @@ def followed_threads(request, course_key, user_id):
[
'page',
'sort_key',
'sort_order',
'flagged',
'unread',
'unanswered',

10
lms/djangoapps/discussion_api/api.py
View File

@@ -498,8 +498,9 @@ def get_thread_list(
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".
order_direction: The direction in which to sort the threads by. The default
and only value is "desc". This will be removed in a future major
version.
requested_fields: Indicates which additional fields to return
for each thread. (i.e. ['profile_image'])
@@ -528,9 +529,9 @@ def get_thread_list(
"order_by":
["Invalid value. '{}' must be 'last_activity_at', 'comment_count', or 'vote_count'".format(order_by)]
})
if order_direction not in ["asc", "desc"]:
if order_direction != "desc":
raise ValidationError({
"order_direction": ["Invalid value. '{}' must be 'asc' or 'desc'".format(order_direction)]
"order_direction": ["Invalid value. '{}' must be 'desc'".format(order_direction)]
})
course = _get_course(course_key, request.user)
@@ -546,7 +547,6 @@ def get_thread_list(
"per_page": page_size,
"text": text_search,
"sort_key": cc_map.get(order_by),
"sort_order": order_direction,
}
if view:

2
lms/djangoapps/discussion_api/forms.py
View File

@@ -48,7 +48,7 @@ class ThreadListGetForm(_PaginationForm):
required=False
)
order_direction = ChoiceField(
choices=[(choice, choice) for choice in ["asc", "desc"]],
choices=[(choice, choice) for choice in ["desc"]],
required=False
)
requested_fields = MultiValueField(required=False)

31
lms/djangoapps/discussion_api/tests/test_api.py
View File

@@ -627,7 +627,6 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto
"user_id": [unicode(self.user.id)],
"course_id": [unicode(self.course.id)],
"sort_key": ["activity"],
"sort_order": ["desc"],
"page": ["1"],
"per_page": ["1"],
"commentable_ids": ["topic_x,topic_meow"]
@@ -639,7 +638,6 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto
"user_id": [unicode(self.user.id)],
"course_id": [unicode(self.course.id)],
"sort_key": ["activity"],
"sort_order": ["desc"],
"page": ["6"],
"per_page": ["14"],
})
@@ -851,7 +849,6 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto
"user_id": [unicode(self.user.id)],
"course_id": [unicode(self.course.id)],
"sort_key": ["activity"],
"sort_order": ["desc"],
"page": ["1"],
"per_page": ["10"],
"text": ["test search string"],
@@ -883,7 +880,6 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto
"user_id": [unicode(self.user.id)],
"course_id": [unicode(self.course.id)],
"sort_key": ["activity"],
"sort_order": ["desc"],
"page": ["1"],
"per_page": ["11"],
})
@@ -915,7 +911,6 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto
"user_id": [unicode(self.user.id)],
"course_id": [unicode(self.course.id)],
"sort_key": ["activity"],
"sort_order": ["desc"],
"page": ["1"],
"per_page": ["11"],
query: ["true"],
@@ -957,20 +952,22 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto
"user_id": [unicode(self.user.id)],
"course_id": [unicode(self.course.id)],
"sort_key": [cc_query],
"sort_order": ["desc"],
"page": ["1"],
"per_page": ["11"],
})
@ddt.data("asc", "desc")
def test_order_direction_query(self, http_query):
def test_order_direction(self):
"""
Only "desc" is supported for order. Also, since it is simply swallowed,
it isn't included in the params.
"""
self.register_get_threads_response([], page=1, num_pages=0)
result = get_thread_list(
self.request,
self.course.id,
page=1,
page_size=11,
order_direction=http_query,
order_direction="desc",
).data
expected_result = make_paginated_api_response(
@@ -986,11 +983,25 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto
"user_id": [unicode(self.user.id)],
"course_id": [unicode(self.course.id)],
"sort_key": ["activity"],
"sort_order": [http_query],
"page": ["1"],
"per_page": ["11"],
})
def test_invalid_order_direction(self):
"""
Test with invalid order_direction (e.g. "asc")
"""
with self.assertRaises(ValidationError) as assertion:
self.register_get_threads_response([], page=1, num_pages=0)
get_thread_list( # pylint: disable=expression-not-assigned
self.request,
self.course.id,
page=1,
page_size=11,
order_direction="asc",
).data
self.assertIn("order_direction", assertion.exception.message_dict)
@attr(shard=2)
@ddt.ddt

1
lms/djangoapps/discussion_api/tests/test_forms.py
View File

@@ -147,7 +147,6 @@ class ThreadListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase):
("order_by", "last_activity_at"),
("order_by", "comment_count"),
("order_by", "vote_count"),
("order_direction", "asc"),
("order_direction", "desc"),
)
@ddt.unpack

15
lms/djangoapps/discussion_api/tests/test_views.py
View File

@@ -428,7 +428,6 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro
"user_id": [unicode(self.user.id)],
"course_id": [unicode(self.course.id)],
"sort_key": ["activity"],
"sort_order": ["desc"],
"page": ["1"],
"per_page": ["10"],
})
@@ -449,7 +448,6 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro
"user_id": [unicode(self.user.id)],
"course_id": [unicode(self.course.id)],
"sort_key": ["activity"],
"sort_order": ["desc"],
"page": ["1"],
"per_page": ["10"],
query: ["true"],
@@ -471,7 +469,6 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro
"user_id": [unicode(self.user.id)],
"course_id": [unicode(self.course.id)],
"sort_key": ["activity"],
"sort_order": ["desc"],
"page": ["18"],
"per_page": ["4"],
})
@@ -497,7 +494,6 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro
"user_id": [unicode(self.user.id)],
"course_id": [unicode(self.course.id)],
"sort_key": ["activity"],
"sort_order": ["desc"],
"page": ["1"],
"per_page": ["10"],
"text": ["test search string"],
@@ -589,14 +585,16 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro
self.assert_last_query_params({
"user_id": [unicode(self.user.id)],
"course_id": [unicode(self.course.id)],
"sort_order": ["desc"],
"page": ["1"],
"per_page": ["10"],
"sort_key": [cc_query],
})
@ddt.data("asc", "desc")
def test_order_direction(self, query):
def test_order_direction(self):
"""
Test order direction, of which "desc" is the only valid option. The
option actually just gets swallowed, so it doesn't affect the params.
"""
threads = [make_minimal_cs_thread()]
self.register_get_user_response(self.user)
self.register_get_threads_response(threads, page=1, num_pages=1)
@@ -604,7 +602,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro
self.url,
{
"course_id": unicode(self.course.id),
"order_direction": query,
"order_direction": "desc",
}
)
self.assert_last_query_params({
@@ -613,7 +611,6 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro
"sort_key": ["activity"],
"page": ["1"],
"per_page": ["10"],
"sort_order": [query],
})
def test_mutually_exclusive(self):

6
lms/djangoapps/discussion_api/views.py
View File

@@ -155,8 +155,9 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet):
"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".
* order_direction: Must be "desc". The direction in which to sort the
threads by. The default and only value is "desc". This will be
removed in a future major version.
* following: If true, retrieve only threads the requesting user is
following
@@ -164,6 +165,7 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet):
* view: "unread" for threads the requesting user has not read, or
"unanswered" for question threads with no marked answer. Only one
can be selected.
* requested_fields: (list) Indicates which additional fields to return
for each thread. (supports 'profile_image')