Commit 26891bc0 by Toby Lawrence Committed by Robert Raposa

Default to not requesting responses when grabbing a single thread.

We're often grabbing the metadata of a specific thread to then be able
to perform other operations, but we never need the actual responses or
comments of a thread unless we're displaying it in the normal forum
view.

This change sets a default of with_responses=False, which instructs the
comment service to not send back the responses/comments for the given
thread.  We only ask for responses in the case of rendering a single
thread or inline discussion.
parent f2fc3a14
...@@ -294,6 +294,7 @@ def single_thread(request, course_key, discussion_id, thread_id): ...@@ -294,6 +294,7 @@ def single_thread(request, course_key, discussion_id, thread_id):
# the comments service. # the comments service.
try: try:
thread = cc.Thread.find(thread_id).retrieve( thread = cc.Thread.find(thread_id).retrieve(
with_responses=True,
recursive=request.is_ajax(), recursive=request.is_ajax(),
user_id=request.user.id, user_id=request.user.id,
response_skip=request.GET.get("resp_skip"), response_skip=request.GET.get("resp_skip"),
......
...@@ -96,6 +96,7 @@ def _get_thread_and_context(request, thread_id, retrieve_kwargs=None): ...@@ -96,6 +96,7 @@ def _get_thread_and_context(request, thread_id, retrieve_kwargs=None):
""" """
retrieve_kwargs = retrieve_kwargs or {} retrieve_kwargs = retrieve_kwargs or {}
try: try:
retrieve_kwargs["with_responses"] = True
if "mark_as_read" not in retrieve_kwargs: if "mark_as_read" not in retrieve_kwargs:
retrieve_kwargs["mark_as_read"] = False retrieve_kwargs["mark_as_read"] = False
cc_thread = Thread(id=thread_id).retrieve(**retrieve_kwargs) cc_thread = Thread(id=thread_id).retrieve(**retrieve_kwargs)
......
...@@ -631,6 +631,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto ...@@ -631,6 +631,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto
"page": ["1"], "page": ["1"],
"per_page": ["1"], "per_page": ["1"],
"recursive": ["False"], "recursive": ["False"],
"with_responses": ["True"],
"commentable_ids": ["topic_x,topic_meow"] "commentable_ids": ["topic_x,topic_meow"]
}) })
...@@ -644,6 +645,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto ...@@ -644,6 +645,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto
"page": ["6"], "page": ["6"],
"per_page": ["14"], "per_page": ["14"],
"recursive": ["False"], "recursive": ["False"],
"with_responses": ["True"],
}) })
def test_thread_content(self): def test_thread_content(self):
...@@ -857,6 +859,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto ...@@ -857,6 +859,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto
"page": ["1"], "page": ["1"],
"per_page": ["10"], "per_page": ["10"],
"recursive": ["False"], "recursive": ["False"],
"with_responses": ["True"],
"text": ["test search string"], "text": ["test search string"],
}) })
...@@ -923,6 +926,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto ...@@ -923,6 +926,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto
"per_page": ["11"], "per_page": ["11"],
"recursive": ["False"], "recursive": ["False"],
query: ["true"], query: ["true"],
"with_responses": ["True"],
}) })
@ddt.data( @ddt.data(
...@@ -965,6 +969,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto ...@@ -965,6 +969,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto
"page": ["1"], "page": ["1"],
"per_page": ["11"], "per_page": ["11"],
"recursive": ["False"], "recursive": ["False"],
"with_responses": ["True"],
}) })
@ddt.data("asc", "desc") @ddt.data("asc", "desc")
...@@ -995,6 +1000,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto ...@@ -995,6 +1000,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto
"page": ["1"], "page": ["1"],
"per_page": ["11"], "per_page": ["11"],
"recursive": ["False"], "recursive": ["False"],
"with_responses": ["True"],
}) })
...@@ -1181,6 +1187,7 @@ class GetCommentListTest(CommentsServiceMockMixin, SharedModuleStoreTestCase): ...@@ -1181,6 +1187,7 @@ class GetCommentListTest(CommentsServiceMockMixin, SharedModuleStoreTestCase):
"mark_as_read": ["False"], "mark_as_read": ["False"],
"resp_skip": ["70"], "resp_skip": ["70"],
"resp_limit": ["14"], "resp_limit": ["14"],
"with_responses": ["True"]
} }
) )
......
...@@ -432,6 +432,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro ...@@ -432,6 +432,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro
"page": ["1"], "page": ["1"],
"per_page": ["10"], "per_page": ["10"],
"recursive": ["False"], "recursive": ["False"],
"with_responses": ["True"],
}) })
@ddt.data("unread", "unanswered") @ddt.data("unread", "unanswered")
...@@ -452,6 +453,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro ...@@ -452,6 +453,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro
"sort_key": ["activity"], "sort_key": ["activity"],
"sort_order": ["desc"], "sort_order": ["desc"],
"recursive": ["False"], "recursive": ["False"],
"with_responses": ["True"],
"page": ["1"], "page": ["1"],
"per_page": ["10"], "per_page": ["10"],
query: ["true"], query: ["true"],
...@@ -477,6 +479,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro ...@@ -477,6 +479,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro
"page": ["18"], "page": ["18"],
"per_page": ["4"], "per_page": ["4"],
"recursive": ["False"], "recursive": ["False"],
"with_responses": ["True"],
}) })
def test_text_search(self): def test_text_search(self):
...@@ -504,6 +507,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro ...@@ -504,6 +507,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro
"page": ["1"], "page": ["1"],
"per_page": ["10"], "per_page": ["10"],
"recursive": ["False"], "recursive": ["False"],
"with_responses": ["True"],
"text": ["test search string"], "text": ["test search string"],
}) })
...@@ -598,6 +602,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro ...@@ -598,6 +602,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro
"page": ["1"], "page": ["1"],
"per_page": ["10"], "per_page": ["10"],
"sort_key": [cc_query], "sort_key": [cc_query],
"with_responses": ["True"],
}) })
@ddt.data("asc", "desc") @ddt.data("asc", "desc")
...@@ -620,6 +625,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro ...@@ -620,6 +625,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro
"page": ["1"], "page": ["1"],
"per_page": ["10"], "per_page": ["10"],
"sort_order": [query], "sort_order": [query],
"with_responses": ["True"],
}) })
def test_mutually_exclusive(self): def test_mutually_exclusive(self):
...@@ -1130,6 +1136,7 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pr ...@@ -1130,6 +1136,7 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pr
"resp_limit": ["10"], "resp_limit": ["10"],
"user_id": [str(self.user.id)], "user_id": [str(self.user.id)],
"mark_as_read": ["False"], "mark_as_read": ["False"],
"with_responses": ["True"],
} }
) )
...@@ -1163,6 +1170,7 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pr ...@@ -1163,6 +1170,7 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pr
"resp_limit": ["4"], "resp_limit": ["4"],
"user_id": [str(self.user.id)], "user_id": [str(self.user.id)],
"mark_as_read": ["False"], "mark_as_read": ["False"],
"with_responses": ["True"],
} }
) )
......
...@@ -734,7 +734,7 @@ class ViewsTestCase( ...@@ -734,7 +734,7 @@ class ViewsTestCase(
('get', '{prefix}/threads/518d4237b023791dca00000d'.format(prefix=CS_PREFIX)), ('get', '{prefix}/threads/518d4237b023791dca00000d'.format(prefix=CS_PREFIX)),
{ {
'data': None, 'data': None,
'params': {'mark_as_read': True, 'request_id': ANY}, 'params': {'mark_as_read': True, 'request_id': ANY, 'with_responses': False},
'headers': ANY, 'headers': ANY,
'timeout': 5 'timeout': 5
} }
...@@ -752,7 +752,7 @@ class ViewsTestCase( ...@@ -752,7 +752,7 @@ class ViewsTestCase(
('get', '{prefix}/threads/518d4237b023791dca00000d'.format(prefix=CS_PREFIX)), ('get', '{prefix}/threads/518d4237b023791dca00000d'.format(prefix=CS_PREFIX)),
{ {
'data': None, 'data': None,
'params': {'mark_as_read': True, 'request_id': ANY}, 'params': {'mark_as_read': True, 'request_id': ANY, 'with_responses': False},
'headers': ANY, 'headers': ANY,
'timeout': 5 'timeout': 5
} }
...@@ -812,7 +812,7 @@ class ViewsTestCase( ...@@ -812,7 +812,7 @@ class ViewsTestCase(
('get', '{prefix}/threads/518d4237b023791dca00000d'.format(prefix=CS_PREFIX)), ('get', '{prefix}/threads/518d4237b023791dca00000d'.format(prefix=CS_PREFIX)),
{ {
'data': None, 'data': None,
'params': {'mark_as_read': True, 'request_id': ANY}, 'params': {'mark_as_read': True, 'request_id': ANY, 'with_responses': False},
'headers': ANY, 'headers': ANY,
'timeout': 5 'timeout': 5
} }
...@@ -830,7 +830,7 @@ class ViewsTestCase( ...@@ -830,7 +830,7 @@ class ViewsTestCase(
('get', '{prefix}/threads/518d4237b023791dca00000d'.format(prefix=CS_PREFIX)), ('get', '{prefix}/threads/518d4237b023791dca00000d'.format(prefix=CS_PREFIX)),
{ {
'data': None, 'data': None,
'params': {'mark_as_read': True, 'request_id': ANY}, 'params': {'mark_as_read': True, 'request_id': ANY, 'with_responses': False},
'headers': ANY, 'headers': ANY,
'timeout': 5 'timeout': 5
} }
......
...@@ -48,7 +48,8 @@ class Thread(models.Model): ...@@ -48,7 +48,8 @@ class Thread(models.Model):
default_params = {'page': 1, default_params = {'page': 1,
'per_page': 20, 'per_page': 20,
'course_id': query_params['course_id'], 'course_id': query_params['course_id'],
'recursive': False} 'recursive': False,
'with_responses': True}
params = merge_dict(default_params, strip_blank(strip_none(query_params))) params = merge_dict(default_params, strip_blank(strip_none(query_params)))
if query_params.get('text'): if query_params.get('text'):
...@@ -131,6 +132,7 @@ class Thread(models.Model): ...@@ -131,6 +132,7 @@ class Thread(models.Model):
url = self.url(action='get', params=self.attributes) url = self.url(action='get', params=self.attributes)
request_params = { request_params = {
'recursive': kwargs.get('recursive'), 'recursive': kwargs.get('recursive'),
'with_responses': kwargs.get('with_responses', False),
'user_id': kwargs.get('user_id'), 'user_id': kwargs.get('user_id'),
'mark_as_read': kwargs.get('mark_as_read', True), 'mark_as_read': kwargs.get('mark_as_read', True),
'resp_skip': kwargs.get('response_skip'), 'resp_skip': kwargs.get('response_skip'),
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment