From d18b24b051f0c5361a7f80037fe815c4f0e68bf8 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Wed, 10 Jun 2015 15:45:06 -0400 Subject: [PATCH] Add discussion API course endpoint This endpoint returns course metadata relating to discussions as a starting point for clients. --- common/lib/xmodule/xmodule/course_module.py | 44 ++++++++---- lms/djangoapps/discussion_api/api.py | 42 +++++++++++- .../discussion_api/tests/test_api.py | 67 +++++++++++++++++++ .../discussion_api/tests/test_views.py | 30 +++++++++ lms/djangoapps/discussion_api/urls.py | 7 +- lms/djangoapps/discussion_api/views.py | 39 ++++++++++- 6 files changed, 211 insertions(+), 18 deletions(-) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 5d64e0a540..a0334fdd89 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -1419,21 +1419,41 @@ class CourseDescriptor(CourseFields, LicenseMixin, SequenceDescriptor): """ return date_time + u" UTC" - @property - def forum_posts_allowed(self): + def get_discussion_blackout_datetimes(self): + """ + Get a list of dicts with start and end fields with datetime values from + the discussion_blackouts setting + """ date_proxy = Date() try: - blackout_periods = [(date_proxy.from_json(start), - date_proxy.from_json(end)) - for start, end - in filter(None, self.discussion_blackouts)] - now = datetime.now(UTC()) - for start, end in blackout_periods: - if start <= now <= end: - return False - except: - log.exception("Error parsing discussion_blackouts %s for course %s", self.discussion_blackouts, self.id) + ret = [ + {"start": date_proxy.from_json(start), "end": date_proxy.from_json(end)} + for start, end + in filter(None, self.discussion_blackouts) + ] + for blackout in ret: + if not blackout["start"] or not blackout["end"]: + raise ValueError + return ret + except (TypeError, ValueError): + log.exception( + "Error parsing discussion_blackouts %s for course %s", + self.discussion_blackouts, + self.id + ) + return [] + @property + def forum_posts_allowed(self): + """ + Return whether forum posts are allowed by the discussion_blackouts + setting + """ + blackouts = self.get_discussion_blackout_datetimes() + now = datetime.now(UTC()) + for blackout in blackouts: + if blackout["start"] <= now <= blackout["end"]: + return False return True @property diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index a14bcae278..40ed523250 100644 --- a/lms/djangoapps/discussion_api/api.py +++ b/lms/djangoapps/discussion_api/api.py @@ -108,15 +108,53 @@ def _is_user_author_or_privileged(cc_content, context): ) -def get_thread_list_url(request, course_key, topic_id_list): +def get_thread_list_url(request, course_key, topic_id_list=None): """ 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] + query_list = ( + [("course_id", unicode(course_key))] + + [("topic_id", topic_id) for topic_id in topic_id_list or []] + ) return request.build_absolute_uri(urlunparse(("", "", path, "", urlencode(query_list), ""))) +def get_course(request, course_key): + """ + Return general discussion information for the course. + + Parameters: + + request: The django request object used for build_absolute_uri and + determining the requesting user. + + course_key: The key of the course to get information for + + Returns: + + The course information; see discussion_api.views.CourseView for more + detail. + + Raises: + + Http404: if the course does not exist or is not accessible to the + requesting user + """ + course = _get_course_or_404(course_key, request.user) + return { + "id": unicode(course_key), + "blackouts": [ + {"start": blackout["start"].isoformat(), "end": blackout["end"].isoformat()} + for blackout in course.get_discussion_blackout_datetimes() + ], + "thread_list_url": get_thread_list_url(request, course_key, topic_id_list=[]), + "topics_url": request.build_absolute_uri( + reverse("course_topics", kwargs={"course_id": course_key}) + ) + } + + def get_course_topics(request, course_key): """ Return the course topic listing for the given course and user. diff --git a/lms/djangoapps/discussion_api/tests/test_api.py b/lms/djangoapps/discussion_api/tests/test_api.py index be157a4273..f67ef81507 100644 --- a/lms/djangoapps/discussion_api/tests/test_api.py +++ b/lms/djangoapps/discussion_api/tests/test_api.py @@ -26,6 +26,7 @@ from discussion_api.api import ( delete_comment, delete_thread, get_comment_list, + get_course, get_course_topics, get_thread_list, update_comment, @@ -63,6 +64,72 @@ def _remove_discussion_tab(course, user_id): modulestore().update_item(course, user_id) +@ddt.ddt +class GetCourseTest(UrlResetMixin, ModuleStoreTestCase): + """Test for get_course""" + + @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + def setUp(self): + super(GetCourseTest, self).setUp() + self.course = CourseFactory.create(org="x", course="y", run="z") + 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 test_nonexistent_course(self): + with self.assertRaises(Http404): + get_course(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(self.request, self.course.id) + + def test_discussions_disabled(self): + _remove_discussion_tab(self.course, self.user.id) + with self.assertRaises(Http404): + get_course(self.request, self.course.id) + + def test_basic(self): + self.assertEqual( + get_course(self.request, self.course.id), + { + "id": unicode(self.course.id), + "blackouts": [], + "thread_list_url": "http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz", + "topics_url": "http://testserver/api/discussion/v1/course_topics/x/y/z", + } + ) + + def test_blackout(self): + # A variety of formats is accepted + self.course.discussion_blackouts = [ + ["2015-06-09T00:00:00Z", "6-10-15"], + [1433980800000, datetime(2015, 6, 12)], + ] + modulestore().update_item(self.course, self.user.id) + result = get_course(self.request, self.course.id) + self.assertEqual( + result["blackouts"], + [ + {"start": "2015-06-09T00:00:00+00:00", "end": "2015-06-10T00:00:00+00:00"}, + {"start": "2015-06-11T00:00:00+00:00", "end": "2015-06-12T00:00:00+00:00"}, + ] + ) + + @ddt.data(None, "not a datetime", "2015", []) + def test_blackout_errors(self, bad_value): + self.course.discussion_blackouts = [ + [bad_value, "2015-06-09T00:00:00Z"], + ["2015-06-10T00:00:00Z", "2015-06-11T00:00:00Z"], + ] + modulestore().update_item(self.course, self.user.id) + result = get_course(self.request, self.course.id) + self.assertEqual(result["blackouts"], []) + + @mock.patch.dict("django.conf.settings.FEATURES", {"DISABLE_START_DATES": False}) class GetCourseTopicsTest(UrlResetMixin, ModuleStoreTestCase): """Test for get_course_topics""" diff --git a/lms/djangoapps/discussion_api/tests/test_views.py b/lms/djangoapps/discussion_api/tests/test_views.py index 1b1b038a3d..6eaa9c1ca3 100644 --- a/lms/djangoapps/discussion_api/tests/test_views.py +++ b/lms/djangoapps/discussion_api/tests/test_views.py @@ -67,6 +67,36 @@ class DiscussionAPIViewTestMixin(CommentsServiceMockMixin, UrlResetMixin): ) +class CourseViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): + """Tests for CourseView""" + def setUp(self): + super(CourseViewTest, self).setUp() + self.url = reverse("discussion_course", kwargs={"course_id": unicode(self.course.id)}) + + def test_404(self): + response = self.client.get( + reverse("course_topics", kwargs={"course_id": "non/existent/course"}) + ) + self.assert_response_correct( + response, + 404, + {"developer_message": "Not found."} + ) + + def test_get_success(self): + response = self.client.get(self.url) + self.assert_response_correct( + response, + 200, + { + "id": unicode(self.course.id), + "blackouts": [], + "thread_list_url": "http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz", + "topics_url": "http://testserver/api/discussion/v1/course_topics/x/y/z", + } + ) + + class CourseTopicsViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): """Tests for CourseTopicsView""" def setUp(self): diff --git a/lms/djangoapps/discussion_api/urls.py b/lms/djangoapps/discussion_api/urls.py index b2808c5567..ed150b6a8d 100644 --- a/lms/djangoapps/discussion_api/urls.py +++ b/lms/djangoapps/discussion_api/urls.py @@ -6,7 +6,7 @@ from django.conf.urls import include, patterns, url from rest_framework.routers import SimpleRouter -from discussion_api.views import CommentViewSet, CourseTopicsView, ThreadViewSet +from discussion_api.views import CommentViewSet, CourseTopicsView, CourseView, ThreadViewSet ROUTER = SimpleRouter() @@ -15,6 +15,11 @@ ROUTER.register("comments", CommentViewSet, base_name="comment") urlpatterns = patterns( "discussion_api", + url( + r"^v1/courses/{}".format(settings.COURSE_ID_PATTERN), + CourseView.as_view(), + name="discussion_course" + ), url( r"^v1/course_topics/{}".format(settings.COURSE_ID_PATTERN), CourseTopicsView.as_view(), diff --git a/lms/djangoapps/discussion_api/views.py b/lms/djangoapps/discussion_api/views.py index 500ae5d570..2c95c24a87 100644 --- a/lms/djangoapps/discussion_api/views.py +++ b/lms/djangoapps/discussion_api/views.py @@ -9,7 +9,7 @@ from rest_framework.response import Response from rest_framework.views import APIView from rest_framework.viewsets import ViewSet -from opaque_keys.edx.locator import CourseLocator +from opaque_keys.edx.keys import CourseKey from discussion_api.api import ( create_comment, @@ -17,6 +17,7 @@ from discussion_api.api import ( delete_thread, delete_comment, get_comment_list, + get_course, get_course_topics, get_thread_list, update_comment, @@ -35,6 +36,38 @@ class _ViewMixin(object): permission_classes = (IsAuthenticated,) +class CourseView(_ViewMixin, DeveloperErrorViewMixin, APIView): + """ + **Use Cases** + + Retrieve general discussion metadata for a course. + + **Example Requests**: + + GET /api/discussion/v1/courses/course-v1:ExampleX+Subject101+2015 + + **Response Values**: + + * id: The identifier of the course + + * blackouts: A list of objects representing blackout periods (during + which discussions are read-only except for privileged users). Each + item in the list includes: + + * start: The ISO 8601 timestamp for the start of the blackout period + + * end: The ISO 8601 timestamp for the end of the blackout period + + * thread_list_url: The URL of the list of all threads in the course. + + * topics_url: The URL of the topic listing for the course. + """ + def get(self, request, course_id): + """Implements the GET method as described in the class docstring.""" + course_key = CourseKey.from_string(course_id) # TODO: which class is right? + return Response(get_course(request, course_key)) + + class CourseTopicsView(_ViewMixin, DeveloperErrorViewMixin, APIView): """ **Use Cases** @@ -44,7 +77,7 @@ class CourseTopicsView(_ViewMixin, DeveloperErrorViewMixin, APIView): **Example Requests**: - GET /api/discussion/v1/course_topics/{course_id} + GET /api/discussion/v1/course_topics/course-v1:ExampleX+Subject101+2015 **Response Values**: @@ -63,7 +96,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) + course_key = CourseKey.from_string(course_id) return Response(get_course_topics(request, course_key))