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

Merge pull request #8199 from edx/clee/get_topic_threads

Browse Source 
MA-642 Add topic_id query to discussion/thread api
This commit is contained in:
Christopher Lee
2015-06-03 12:29:05 -04:00
parent 94e98e9cae f552eca7d6
commit 103ae47759
6 changed files with 125 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -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

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

@@ -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"""

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

@@ -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({

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

@@ -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"""

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

@@ -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",
}],
}
)

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

@@ -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"],
)
)