diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py
index f4df4a79745c931fe761ca383e9363472c1038a1..597f08fd80b4c76a8e623a0c8f3ca97f2a2848f5 100644
--- a/lms/djangoapps/discussion_api/api.py
+++ b/lms/djangoapps/discussion_api/api.py
@@ -1,9 +1,14 @@
"""
Discussion API internal interface
"""
+from urllib import urlencode
+from urlparse import urlunparse
+
from django.core.exceptions import ValidationError
+from django.core.urlresolvers import reverse
from django.http import Http404
+
from collections import defaultdict
from opaque_keys import InvalidKeyError
@@ -38,7 +43,16 @@ def _get_course_or_404(course_key, user):
return course
-def get_course_topics(course_key, user):
+def get_thread_list_url(request, course_key, topic_id_list):
+ """
+ Returns the URL for the thread_list_url field, given a list of topic_ids
+ """
+ path = reverse("thread-list")
+ query_list = [("course_id", unicode(course_key))] + [("topic_id", topic_id) for topic_id in topic_id_list]
+ return request.build_absolute_uri(urlunparse(("", "", path, "", urlencode(query_list), "")))
+
+
+def get_course_topics(request, course_key):
"""
Return the course topic listing for the given course and user.
@@ -59,22 +73,33 @@ def get_course_topics(course_key, user):
"""
return module.sort_key or module.discussion_target
- course = _get_course_or_404(course_key, user)
- discussion_modules = get_accessible_discussion_modules(course, user)
+ course = _get_course_or_404(course_key, request.user)
+ discussion_modules = get_accessible_discussion_modules(course, request.user)
modules_by_category = defaultdict(list)
for module in discussion_modules:
modules_by_category[module.discussion_category].append(module)
+
+ def get_sorted_modules(category):
+ """Returns key sorted modules by category"""
+ return sorted(modules_by_category[category], key=get_module_sort_key)
+
courseware_topics = [
{
"id": None,
"name": category,
+ "thread_list_url": get_thread_list_url(
+ request,
+ course_key,
+ [item.discussion_id for item in get_sorted_modules(category)]
+ ),
"children": [
{
"id": module.discussion_id,
"name": module.discussion_target,
+ "thread_list_url": get_thread_list_url(request, course_key, [module.discussion_id]),
"children": [],
}
- for module in sorted(modules_by_category[category], key=get_module_sort_key)
+ for module in get_sorted_modules(category)
],
}
for category in sorted(modules_by_category.keys())
@@ -84,6 +109,7 @@ def get_course_topics(course_key, user):
{
"id": entry["id"],
"name": name,
+ "thread_list_url": get_thread_list_url(request, course_key, [entry["id"]]),
"children": [],
}
for name, entry in sorted(
@@ -98,7 +124,7 @@ def get_course_topics(course_key, user):
}
-def get_thread_list(request, course_key, page, page_size):
+def get_thread_list(request, course_key, page, page_size, topic_id_list=None):
"""
Return the list of all discussion threads pertaining to the given course
@@ -108,6 +134,7 @@ def get_thread_list(request, course_key, page, page_size):
course_key: The key of the course to get discussion threads for
page: The page number (1-indexed) to retrieve
page_size: The number of threads to retrieve per page
+ topic_id_list: The list of topic_ids to get the discussion threads for
Returns:
@@ -116,6 +143,7 @@ def get_thread_list(request, course_key, page, page_size):
"""
course = _get_course_or_404(course_key, request.user)
context = get_context(course, request)
+ topic_ids_csv = ",".join(topic_id_list) if topic_id_list else None
threads, result_page, num_pages, _ = Thread.search({
"course_id": unicode(course.id),
"group_id": (
@@ -126,6 +154,7 @@ def get_thread_list(request, course_key, page, page_size):
"sort_order": "desc",
"page": page,
"per_page": page_size,
+ "commentable_ids": topic_ids_csv,
})
# 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
diff --git a/lms/djangoapps/discussion_api/forms.py b/lms/djangoapps/discussion_api/forms.py
index 08dd42330a1b3a30a36405b2ba1ff2cc09dcc961..2ae045c0c5388ac261d2c2d0925d034f3da9c78b 100644
--- a/lms/djangoapps/discussion_api/forms.py
+++ b/lms/djangoapps/discussion_api/forms.py
@@ -2,12 +2,31 @@
Discussion API forms
"""
from django.core.exceptions import ValidationError
-from django.forms import BooleanField, CharField, Form, IntegerField, NullBooleanField
+from django.forms import (
+ BooleanField,
+ CharField,
+ Field,
+ Form,
+ IntegerField,
+ MultipleHiddenInput,
+ NullBooleanField,
+)
from opaque_keys import InvalidKeyError
from opaque_keys.edx.locator import CourseLocator
+class TopicIdField(Field):
+ """
+ Field for a list of topic_ids
+ """
+ widget = MultipleHiddenInput
+
+ def validate(self, value):
+ if value and "" in value:
+ raise ValidationError("This field cannot be empty.")
+
+
class _PaginationForm(Form):
"""A form that includes pagination fields"""
page = IntegerField(required=False, min_value=1)
@@ -27,6 +46,7 @@ class ThreadListGetForm(_PaginationForm):
A form to validate query parameters in the thread list retrieval endpoint
"""
course_id = CharField()
+ topic_id = TopicIdField(required=False)
def clean_course_id(self):
"""Validate course_id"""
diff --git a/lms/djangoapps/discussion_api/tests/test_api.py b/lms/djangoapps/discussion_api/tests/test_api.py
index 296070cf3ae8ac695470cb7937677b78d55efb5d..96843a70f544b5dd2d979012065d9296eaea8524 100644
--- a/lms/djangoapps/discussion_api/tests/test_api.py
+++ b/lms/djangoapps/discussion_api/tests/test_api.py
@@ -3,7 +3,8 @@ Tests for Discussion API internal interface
"""
from datetime import datetime, timedelta
import itertools
-from urlparse import urlparse
+from urlparse import urlparse, urlunparse
+from urllib import urlencode
import ddt
import httpretty
@@ -57,9 +58,10 @@ def _remove_discussion_tab(course, user_id):
@mock.patch.dict("django.conf.settings.FEATURES", {"DISABLE_START_DATES": False})
-class GetCourseTopicsTest(ModuleStoreTestCase):
+class GetCourseTopicsTest(UrlResetMixin, ModuleStoreTestCase):
"""Test for get_course_topics"""
+ @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
def setUp(self):
super(GetCourseTopicsTest, self).setUp()
self.maxDiff = None # pylint: disable=invalid-name
@@ -81,6 +83,8 @@ class GetCourseTopicsTest(ModuleStoreTestCase):
days_early_for_beta=3
)
self.user = UserFactory.create()
+ self.request = RequestFactory().get("/dummy")
+ self.request.user = self.user
CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id)
def make_discussion_module(self, topic_id, category, subcategory, **kwargs):
@@ -94,34 +98,46 @@ class GetCourseTopicsTest(ModuleStoreTestCase):
**kwargs
)
- def get_course_topics(self, user=None):
+ def get_thread_list_url(self, topic_id_list):
+ """
+ Returns the URL for the thread_list_url field, given a list of topic_ids
+ """
+ path = "http://testserver/api/discussion/v1/threads/"
+ query_list = [("course_id", unicode(self.course.id))] + [("topic_id", topic_id) for topic_id in topic_id_list]
+ return urlunparse(("", "", path, "", urlencode(query_list), ""))
+
+ def get_course_topics(self):
"""
Get course topics for self.course, using the given user or self.user if
not provided, and generating absolute URIs with a test scheme/host.
"""
- return get_course_topics(self.course.id, user or self.user)
+ return get_course_topics(self.request, self.course.id)
def make_expected_tree(self, topic_id, name, children=None):
"""
Build an expected result tree given a topic id, display name, and
children
"""
+ topic_id_list = [topic_id] if topic_id else [child["id"] for child in children]
children = children or []
node = {
"id": topic_id,
"name": name,
"children": children,
+ "thread_list_url": self.get_thread_list_url(topic_id_list)
}
+
return node
def test_nonexistent_course(self):
with self.assertRaises(Http404):
- get_course_topics(CourseLocator.from_string("non/existent/course"), self.user)
+ get_course_topics(self.request, CourseLocator.from_string("non/existent/course"))
def test_not_enrolled(self):
unenrolled_user = UserFactory.create()
+ self.request.user = unenrolled_user
with self.assertRaises(Http404):
- get_course_topics(self.course.id, unenrolled_user)
+ self.get_course_topics()
def test_discussions_disabled(self):
_remove_discussion_tab(self.course, self.user.id)
@@ -310,8 +326,8 @@ class GetCourseTopicsTest(ModuleStoreTestCase):
],
}
self.assertEqual(student_actual, student_expected)
-
- beta_actual = self.get_course_topics(beta_tester)
+ self.request.user = beta_tester
+ beta_actual = self.get_course_topics()
beta_expected = {
"courseware_topics": [
self.make_expected_tree(
@@ -334,7 +350,8 @@ class GetCourseTopicsTest(ModuleStoreTestCase):
}
self.assertEqual(beta_actual, beta_expected)
- staff_actual = self.get_course_topics(staff)
+ self.request.user = staff
+ staff_actual = self.get_course_topics()
staff_expected = {
"courseware_topics": [
self.make_expected_tree(
@@ -381,14 +398,14 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTest
self.author = UserFactory.create()
self.cohort = CohortFactory.create(course_id=self.course.id)
- def get_thread_list(self, threads, page=1, page_size=1, num_pages=1, course=None):
+ def get_thread_list(self, threads, page=1, page_size=1, num_pages=1, course=None, topic_id_list=None):
"""
Register the appropriate comments service response, then call
get_thread_list and return the result.
"""
course = course or self.course
self.register_get_threads_response(threads, page, num_pages)
- ret = get_thread_list(self.request, course.id, page, page_size)
+ ret = get_thread_list(self.request, course.id, page, page_size, topic_id_list)
return ret
def test_nonexistent_course(self):
@@ -415,6 +432,19 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTest
}
)
+ def test_get_threads_by_topic_id(self):
+ 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({
+ "course_id": [unicode(self.course.id)],
+ "sort_key": ["date"],
+ "sort_order": ["desc"],
+ "page": ["1"],
+ "per_page": ["1"],
+ "recursive": ["False"],
+ "commentable_ids": ["topic_x,topic_meow"]
+ })
+
def test_basic_query_params(self):
self.get_thread_list([], page=6, page_size=14)
self.assert_last_query_params({
diff --git a/lms/djangoapps/discussion_api/tests/test_forms.py b/lms/djangoapps/discussion_api/tests/test_forms.py
index 584314d0cc335ce25ab137b2265fa5b7bba98d5d..07c712831135906efe1b334223ffc188dc317c16 100644
--- a/lms/djangoapps/discussion_api/tests/test_forms.py
+++ b/lms/djangoapps/discussion_api/tests/test_forms.py
@@ -2,6 +2,9 @@
Tests for Discussion API forms
"""
from unittest import TestCase
+from urllib import urlencode
+
+from django.http import QueryDict
from opaque_keys.edx.locator import CourseLocator
@@ -66,13 +69,19 @@ class ThreadListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase):
def setUp(self):
super(ThreadListGetFormTest, self).setUp()
- self.form_data = {
- "course_id": "Foo/Bar/Baz",
- "page": "2",
- "page_size": "13",
- }
+ self.form_data = QueryDict(
+ urlencode(
+ {
+ "course_id": "Foo/Bar/Baz",
+ "page": "2",
+ "page_size": "13",
+ }
+ ),
+ mutable=True
+ )
def test_basic(self):
+ self.form_data.setlist("topic_id", ["example topic_id", "example 2nd topic_id"])
form = self.get_form(expected_valid=True)
self.assertEqual(
form.cleaned_data,
@@ -80,6 +89,7 @@ class ThreadListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase):
"course_id": CourseLocator.from_string("Foo/Bar/Baz"),
"page": 2,
"page_size": 13,
+ "topic_id": ["example topic_id", "example 2nd topic_id"],
}
)
@@ -91,6 +101,10 @@ class ThreadListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase):
self.form_data["course_id"] = "invalid course id"
self.assert_error("course_id", "'invalid course id' is not a valid course id")
+ def test_empty_topic_id(self):
+ self.form_data.setlist("topic_id", ["", "not empty"])
+ self.assert_error("topic_id", "This field cannot be empty.")
+
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 545f8165ddc8b5f20095a61236af5b92fa3fa1e0..4bdc87193e8f8a4fdd40d0aed0048151b6973402 100644
--- a/lms/djangoapps/discussion_api/tests/test_views.py
+++ b/lms/djangoapps/discussion_api/tests/test_views.py
@@ -85,7 +85,9 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
"non_courseware_topics": [{
"id": "test_topic",
"name": "Test Topic",
- "children": []
+ "children": [],
+ "thread_list_url":
+ "http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz&topic_id=test_topic",
}],
}
)
diff --git a/lms/djangoapps/discussion_api/views.py b/lms/djangoapps/discussion_api/views.py
index 96ce59ec06351463a8c838dbb5db25b9a5bcb06f..a1978f0aa529c7fe0339eff7e722d897add9fe3c 100644
--- a/lms/djangoapps/discussion_api/views.py
+++ b/lms/djangoapps/discussion_api/views.py
@@ -60,7 +60,7 @@ class CourseTopicsView(_ViewMixin, DeveloperErrorViewMixin, APIView):
def get(self, request, course_id):
"""Implements the GET method as described in the class docstring."""
course_key = CourseLocator.from_string(course_id)
- return Response(get_course_topics(course_key, request.user))
+ return Response(get_course_topics(request, course_key))
class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
@@ -90,6 +90,10 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
* page_size: The number of items per page (default is 10, max is 100)
+ * topic_id: The id of the topic to retrieve the threads. There can be
+ multiple topic_id queries to retrieve threads from multiple topics
+ at once.
+
**POST Parameters**:
* course_id (required): The course to create the thread in
@@ -157,7 +161,8 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet):
request,
form.cleaned_data["course_id"],
form.cleaned_data["page"],
- form.cleaned_data["page_size"]
+ form.cleaned_data["page_size"],
+ form.cleaned_data["topic_id"],
)
)