Commit 54756317 by Greg Price Committed by christopher lee

Add followed thread retrieval to discussion API

This implements the "following" value for the "view" query parameter on
the thread list endpoint.
parent 9a94c601
...@@ -92,14 +92,28 @@ def _get_comment_and_context(request, comment_id): ...@@ -92,14 +92,28 @@ def _get_comment_and_context(request, comment_id):
raise Http404 raise Http404
def get_thread_list_url(request, course_key, topic_id_list=None): def _is_user_author_or_privileged(cc_content, context):
"""
Check if the user is the author of a content object or a privileged user.
Returns:
Boolean
"""
return (
context["is_requester_privileged"] or
context["cc_requester"]["id"] == cc_content["user_id"]
)
def get_thread_list_url(request, course_key, topic_id_list=None, following=False):
""" """
Returns the URL for the thread_list_url field, given a list of topic_ids Returns the URL for the thread_list_url field, given a list of topic_ids
""" """
path = reverse("thread-list") path = reverse("thread-list")
query_list = ( query_list = (
[("course_id", unicode(course_key))] + [("course_id", unicode(course_key))] +
[("topic_id", topic_id) for topic_id in topic_id_list or []] [("topic_id", topic_id) for topic_id in topic_id_list or []] +
([("following", following)] if following else [])
) )
return request.build_absolute_uri(urlunparse(("", "", path, "", urlencode(query_list), ""))) return request.build_absolute_uri(urlunparse(("", "", path, "", urlencode(query_list), "")))
...@@ -132,7 +146,8 @@ def get_course(request, course_key): ...@@ -132,7 +146,8 @@ def get_course(request, course_key):
{"start": blackout["start"].isoformat(), "end": blackout["end"].isoformat()} {"start": blackout["start"].isoformat(), "end": blackout["end"].isoformat()}
for blackout in course.get_discussion_blackout_datetimes() for blackout in course.get_discussion_blackout_datetimes()
], ],
"thread_list_url": get_thread_list_url(request, course_key, topic_id_list=[]), "thread_list_url": get_thread_list_url(request, course_key),
"following_thread_list_url": get_thread_list_url(request, course_key, following=True),
"topics_url": request.build_absolute_uri( "topics_url": request.build_absolute_uri(
reverse("course_topics", kwargs={"course_id": course_key}) reverse("course_topics", kwargs={"course_id": course_key})
) )
...@@ -211,7 +226,7 @@ def get_course_topics(request, course_key): ...@@ -211,7 +226,7 @@ def get_course_topics(request, course_key):
} }
def get_thread_list(request, course_key, page, page_size, topic_id_list=None, text_search=None): def get_thread_list(request, course_key, page, page_size, topic_id_list=None, text_search=None, following=False):
""" """
Return the list of all discussion threads pertaining to the given course Return the list of all discussion threads pertaining to the given course
...@@ -223,8 +238,9 @@ def get_thread_list(request, course_key, page, page_size, topic_id_list=None, te ...@@ -223,8 +238,9 @@ def get_thread_list(request, course_key, page, page_size, topic_id_list=None, te
page_size: The number of threads to retrieve per page page_size: The number of threads to retrieve per page
topic_id_list: The list of topic_ids to get the discussion threads for topic_id_list: The list of topic_ids to get the discussion threads for
text_search A text search query string to match text_search A text search query string to match
following: If true, retrieve only threads the requester is following
Note that topic_id_list and text_search are mutually exclusive. Note that topic_id_list, text_search, and following are mutually exclusive.
Returns: Returns:
...@@ -238,15 +254,13 @@ def get_thread_list(request, course_key, page, page_size, topic_id_list=None, te ...@@ -238,15 +254,13 @@ def get_thread_list(request, course_key, page, page_size, topic_id_list=None, te
Http404: if the requesting user does not have access to the requested course Http404: if the requesting user does not have access to the requested course
or a page beyond the last is requested or a page beyond the last is requested
""" """
exclusive_param_count = sum(1 for param in [topic_id_list, text_search] if param) exclusive_param_count = sum(1 for param in [topic_id_list, text_search, following] if param)
if exclusive_param_count > 1: # pragma: no cover if exclusive_param_count > 1: # pragma: no cover
raise ValueError("More than one mutually exclusive param passed to get_thread_list") raise ValueError("More than one mutually exclusive param passed to get_thread_list")
course = _get_course_or_404(course_key, request.user) course = _get_course_or_404(course_key, request.user)
context = get_context(course, request) context = get_context(course, request)
topic_ids_csv = ",".join(topic_id_list) if topic_id_list else None query_params = {
threads, result_page, num_pages, text_search_rewrite = Thread.search({
"course_id": unicode(course.id),
"group_id": ( "group_id": (
None if context["is_requester_privileged"] else None if context["is_requester_privileged"] else
get_cohort_id(request.user, course.id) get_cohort_id(request.user, course.id)
...@@ -255,9 +269,16 @@ def get_thread_list(request, course_key, page, page_size, topic_id_list=None, te ...@@ -255,9 +269,16 @@ def get_thread_list(request, course_key, page, page_size, topic_id_list=None, te
"sort_order": "desc", "sort_order": "desc",
"page": page, "page": page,
"per_page": page_size, "per_page": page_size,
"commentable_ids": topic_ids_csv,
"text": text_search, "text": text_search,
}) }
text_search_rewrite = None
if following:
threads, result_page, num_pages = context["cc_requester"].subscribed_threads(query_params)
else:
query_params["course_id"] = unicode(course.id)
query_params["commentable_ids"] = ",".join(topic_id_list) if topic_id_list else None
query_params["text"] = text_search
threads, result_page, num_pages, text_search_rewrite = Thread.search(query_params)
# The comments service returns the last page of results if the requested # The comments service returns the last page of results if the requested
# page is beyond the last page, but we want be consistent with DRF's general # page is beyond the last page, but we want be consistent with DRF's general
# behavior and return a 404 in that case # behavior and return a 404 in that case
......
...@@ -5,6 +5,7 @@ from django.core.exceptions import ValidationError ...@@ -5,6 +5,7 @@ from django.core.exceptions import ValidationError
from django.forms import ( from django.forms import (
BooleanField, BooleanField,
CharField, CharField,
ChoiceField,
Field, Field,
Form, Form,
IntegerField, IntegerField,
...@@ -45,11 +46,12 @@ class ThreadListGetForm(_PaginationForm): ...@@ -45,11 +46,12 @@ class ThreadListGetForm(_PaginationForm):
""" """
A form to validate query parameters in the thread list retrieval endpoint A form to validate query parameters in the thread list retrieval endpoint
""" """
EXCLUSIVE_PARAMS = ["topic_id", "text_search"] EXCLUSIVE_PARAMS = ["topic_id", "text_search", "following"]
course_id = CharField() course_id = CharField()
topic_id = TopicIdField(required=False) topic_id = TopicIdField(required=False)
text_search = CharField(required=False) text_search = CharField(required=False)
following = NullBooleanField(required=False)
def clean_course_id(self): def clean_course_id(self):
"""Validate course_id""" """Validate course_id"""
...@@ -59,6 +61,14 @@ class ThreadListGetForm(_PaginationForm): ...@@ -59,6 +61,14 @@ class ThreadListGetForm(_PaginationForm):
except InvalidKeyError: except InvalidKeyError:
raise ValidationError("'{}' is not a valid course id".format(value)) raise ValidationError("'{}' is not a valid course id".format(value))
def clean_following(self):
"""Validate following"""
value = self.cleaned_data["following"]
if value is False:
raise ValidationError("The value of the 'following' parameter must be true.")
else:
return value
def clean(self): def clean(self):
cleaned_data = super(ThreadListGetForm, self).clean() cleaned_data = super(ThreadListGetForm, self).clean()
exclusive_params_count = sum( exclusive_params_count = sum(
......
...@@ -47,6 +47,8 @@ def get_context(course, request, thread=None): ...@@ -47,6 +47,8 @@ def get_context(course, request, thread=None):
for user in role.users.all() for user in role.users.all()
} }
requester = request.user requester = request.user
cc_requester = CommentClientUser.from_django_user(requester).retrieve()
cc_requester["course_id"] = course.id
return { return {
"course": course, "course": course,
"request": request, "request": request,
...@@ -56,7 +58,7 @@ def get_context(course, request, thread=None): ...@@ -56,7 +58,7 @@ def get_context(course, request, thread=None):
"is_requester_privileged": requester.id in staff_user_ids or requester.id in ta_user_ids, "is_requester_privileged": requester.id in staff_user_ids or requester.id in ta_user_ids,
"staff_user_ids": staff_user_ids, "staff_user_ids": staff_user_ids,
"ta_user_ids": ta_user_ids, "ta_user_ids": ta_user_ids,
"cc_requester": CommentClientUser.from_django_user(requester).retrieve(), "cc_requester": cc_requester,
} }
......
...@@ -99,6 +99,9 @@ class GetCourseTest(UrlResetMixin, ModuleStoreTestCase): ...@@ -99,6 +99,9 @@ class GetCourseTest(UrlResetMixin, ModuleStoreTestCase):
"id": unicode(self.course.id), "id": unicode(self.course.id),
"blackouts": [], "blackouts": [],
"thread_list_url": "http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz", "thread_list_url": "http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz",
"following_thread_list_url": (
"http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz&following=True"
),
"topics_url": "http://testserver/api/discussion/v1/course_topics/x/y/z", "topics_url": "http://testserver/api/discussion/v1/course_topics/x/y/z",
} }
) )
...@@ -741,6 +744,31 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTest ...@@ -741,6 +744,31 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTest
"text": ["test search string"], "text": ["test search string"],
}) })
def test_following(self):
self.register_subscribed_threads_response(self.user, [], page=1, num_pages=1)
result = get_thread_list(
self.request,
self.course.id,
page=1,
page_size=11,
following=True,
)
self.assertEqual(
result,
{"results": [], "next": None, "previous": None, "text_search_rewrite": None}
)
self.assertEqual(
urlparse(httpretty.last_request().path).path,
"/api/v1/users/{}/subscribed_threads".format(self.user.id)
)
self.assert_last_query_params({
"course_id": [unicode(self.course.id)],
"sort_key": ["date"],
"sort_order": ["desc"],
"page": ["1"],
"per_page": ["11"],
})
@ddt.ddt @ddt.ddt
class GetCommentListTest(CommentsServiceMockMixin, ModuleStoreTestCase): class GetCommentListTest(CommentsServiceMockMixin, ModuleStoreTestCase):
......
...@@ -94,6 +94,7 @@ class ThreadListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase): ...@@ -94,6 +94,7 @@ class ThreadListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase):
"page_size": 13, "page_size": 13,
"topic_id": [], "topic_id": [],
"text_search": "", "text_search": "",
"following": None,
} }
) )
...@@ -125,12 +126,20 @@ class ThreadListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase): ...@@ -125,12 +126,20 @@ class ThreadListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase):
self.form_data.setlist("topic_id", ["", "not empty"]) self.form_data.setlist("topic_id", ["", "not empty"])
self.assert_error("topic_id", "This field cannot be empty.") self.assert_error("topic_id", "This field cannot be empty.")
@ddt.data(*itertools.combinations(["topic_id", "text_search"], 2)) def test_following_true(self):
self.form_data["following"] = "True"
self.assert_field_value("following", True)
def test_following_false(self):
self.form_data["following"] = "False"
self.assert_error("following", "The value of the 'following' parameter must be true.")
@ddt.data(*itertools.combinations(["topic_id", "text_search", "following"], 2))
def test_mutually_exclusive(self, params): def test_mutually_exclusive(self, params):
self.form_data.update({param: "dummy" for param in params}) self.form_data.update({param: "True" for param in params})
self.assert_error( self.assert_error(
"__all__", "__all__",
"The following query parameters are mutually exclusive: topic_id, text_search" "The following query parameters are mutually exclusive: topic_id, text_search, following"
) )
......
...@@ -92,6 +92,9 @@ class CourseViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): ...@@ -92,6 +92,9 @@ class CourseViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
"id": unicode(self.course.id), "id": unicode(self.course.id),
"blackouts": [], "blackouts": [],
"thread_list_url": "http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz", "thread_list_url": "http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz",
"following_thread_list_url": (
"http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz&following=True"
),
"topics_url": "http://testserver/api/discussion/v1/course_topics/x/y/z", "topics_url": "http://testserver/api/discussion/v1/course_topics/x/y/z",
} }
) )
...@@ -270,6 +273,28 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): ...@@ -270,6 +273,28 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
"text": ["test search string"], "text": ["test search string"],
}) })
def test_following(self):
self.register_get_user_response(self.user)
self.register_subscribed_threads_response(self.user, [], page=1, num_pages=1)
response = self.client.get(
self.url,
{
"course_id": unicode(self.course.id),
"page": "1",
"page_size": "4",
"following": "True",
}
)
self.assert_response_correct(
response,
200,
{"results": [], "next": None, "previous": None, "text_search_rewrite": None}
)
self.assertEqual(
urlparse(httpretty.last_request().path).path,
"/api/v1/users/{}/subscribed_threads".format(self.user.id)
)
@httpretty.activate @httpretty.activate
class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
......
...@@ -191,6 +191,19 @@ class CommentsServiceMockMixin(object): ...@@ -191,6 +191,19 @@ class CommentsServiceMockMixin(object):
status=200 status=200
) )
def register_subscribed_threads_response(self, user, threads, page, num_pages):
"""Register a mock response for GET on the CS user instance endpoint"""
httpretty.register_uri(
httpretty.GET,
"http://localhost:4567/api/v1/users/{}/subscribed_threads".format(user.id),
body=json.dumps({
"collection": threads,
"page": page,
"num_pages": num_pages,
}),
status=200
)
def register_subscription_response(self, user): def register_subscription_response(self, user):
""" """
Register a mock response for POST and DELETE on the CS user subscription Register a mock response for POST and DELETE on the CS user subscription
......
...@@ -141,6 +141,12 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet): ...@@ -141,6 +141,12 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
(including the bodies of comments in the thread) matches the search (including the bodies of comments in the thread) matches the search
string will be returned. string will be returned.
* following: If true, retrieve only threads the requesting user is
following
The topic_id, text_search, and following parameters are mutually
exclusive (i.e. only one may be specified in a request)
**POST Parameters**: **POST Parameters**:
* course_id (required): The course to create the thread in * course_id (required): The course to create the thread in
...@@ -229,6 +235,7 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet): ...@@ -229,6 +235,7 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
form.cleaned_data["page_size"], form.cleaned_data["page_size"],
form.cleaned_data["topic_id"], form.cleaned_data["topic_id"],
form.cleaned_data["text_search"], form.cleaned_data["text_search"],
form.cleaned_data["following"],
) )
) )
......
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