diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index 7350c0364d..1519cf50d7 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -187,17 +187,17 @@ class ToyCourseFactory(SampleCourseFactory): """ store = kwargs.get('modulestore') user_id = kwargs.get('user_id', ModuleStoreEnum.UserID.test) - toy_course = super(ToyCourseFactory, cls)._create( - target_class, - block_info_tree=TOY_BLOCK_INFO_TREE, - textbooks=[["Textbook", "path/to/a/text_book"]], - wiki_slug="toy", - graded=True, - discussion_topics={"General": {"id": "i4x-edX-toy-course-2012_Fall"}}, - graceperiod=datetime.timedelta(days=2, seconds=21599), - start=datetime.datetime(2015, 07, 17, 12, tzinfo=pytz.utc), - xml_attributes={"filename": ["course/2012_Fall.xml", "course/2012_Fall.xml"]}, - pdf_textbooks=[ + + fields = { + 'block_info_tree': TOY_BLOCK_INFO_TREE, + 'textbooks': [["Textbook", "path/to/a/text_book"]], + 'wiki_slug': "toy", + 'graded': True, + 'discussion_topics': {"General": {"id": "i4x-edX-toy-course-2012_Fall"}}, + 'graceperiod': datetime.timedelta(days=2, seconds=21599), + 'start': datetime.datetime(2015, 07, 17, 12, tzinfo=pytz.utc), + 'xml_attributes': {"filename": ["course/2012_Fall.xml", "course/2012_Fall.xml"]}, + 'pdf_textbooks': [ { "tab_title": "Sample Multi Chapter Textbook", "id": "MyTextbook", @@ -207,8 +207,13 @@ class ToyCourseFactory(SampleCourseFactory): ] } ], - course_image="just_a_test.jpg", - **kwargs + 'course_image': "just_a_test.jpg", + } + fields.update(kwargs) + + toy_course = super(ToyCourseFactory, cls)._create( + target_class, + **fields ) with store.bulk_operations(toy_course.id, emit_signals=False): with store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, toy_course.id): diff --git a/lms/djangoapps/course_api/api.py b/lms/djangoapps/course_api/api.py new file mode 100644 index 0000000000..b6089b2f1b --- /dev/null +++ b/lms/djangoapps/course_api/api.py @@ -0,0 +1,76 @@ +""" +Course API +""" + +from django.contrib.auth.models import User +from django.http import Http404 +from rest_framework.exceptions import NotFound, PermissionDenied + +from lms.djangoapps.courseware.courses import get_courses, get_course_with_access + +from .permissions import can_view_courses_for_username +from .serializers import CourseSerializer + + +def get_effective_user(requesting_user, target_username): + """ + Get the user we want to view information on behalf of. + """ + if target_username == requesting_user.username: + return requesting_user + elif can_view_courses_for_username(requesting_user, target_username): + return User.objects.get(username=target_username) + else: + raise PermissionDenied() + + +def course_detail(request, username, course_key): + """ + Return a single course identified by `course_key`. + + The course must be visible to the user identified by `username` and the + logged-in user should have permission to view courses available to that + user. + + Arguments: + request (HTTPRequest): + Used to identify the logged-in user and to instantiate the course + module to retrieve the course about description + username (string): + The name of the user `requesting_user would like to be identified as. + course_key (CourseKey): Identifies the course of interest + + Return value: + CourseSerializer object representing the requested course + """ + user = get_effective_user(request.user, username) + try: + course = get_course_with_access(user, 'see_exists', course_key) + except Http404: + raise NotFound() + return CourseSerializer(course, context={'request': request}).data + + +def list_courses(request, username): + """ + Return a list of available courses. + + The courses returned are all be visible to the user identified by + `username` and the logged in user should have permission to view courses + available to that user. + + Arguments: + request (HTTPRequest): + Used to identify the logged-in user and to instantiate the course + module to retrieve the course about description + username (string): + The name of the user the logged-in user would like to be + identified as + + + Return value: + A CourseSerializer object representing the collection of courses. + """ + user = get_effective_user(request.user, username) + courses = get_courses(user) + return CourseSerializer(courses, context={'request': request}, many=True).data diff --git a/lms/djangoapps/course_api/permissions.py b/lms/djangoapps/course_api/permissions.py new file mode 100644 index 0000000000..24356da4e1 --- /dev/null +++ b/lms/djangoapps/course_api/permissions.py @@ -0,0 +1,35 @@ +""" +Course API Authorization functions +""" + +from student.roles import GlobalStaff + + +def can_view_courses_for_username(requesting_user, target_username): + """ + Determine whether `requesting_user` has permission to view courses available + to the user identified by `target_username`. + + Arguments: + requesting_user (User): The user requesting permission to view another + target_username (string): + The name of the user `requesting_user` would like + to access. + + Return value: + Boolean: + `True` if `requesting_user` is authorized to view courses as + `target_username`. Otherwise, `False` + Raises: + TypeError if target_username is empty or None. + """ + + # AnonymousUser has no username, so we test for requesting_user's own + # username before prohibiting an empty target_username. + if requesting_user.username == target_username: + return True + elif not target_username: + raise TypeError("target_username must be specified") + else: + staff = GlobalStaff() + return staff.has_user(requesting_user) diff --git a/lms/djangoapps/course_api/serializers.py b/lms/djangoapps/course_api/serializers.py new file mode 100644 index 0000000000..1114b0f6bc --- /dev/null +++ b/lms/djangoapps/course_api/serializers.py @@ -0,0 +1,94 @@ +""" +Course API Serializers. Representing course catalog data +""" + +import urllib + +from django.core.urlresolvers import reverse +from django.template import defaultfilters + +from rest_framework import serializers + +from lms.djangoapps.courseware.courses import course_image_url, get_course_about_section +from xmodule.course_module import DEFAULT_START_DATE + + +class _MediaSerializer(serializers.Serializer): # pylint: disable=abstract-method + """ + Nested serializer to represent a media object. + """ + def __init__(self, uri_parser, *args, **kwargs): + super(_MediaSerializer, self).__init__(*args, **kwargs) + self.uri_parser = uri_parser + + uri = serializers.SerializerMethodField(source='*') + + def get_uri(self, course): + """ + Get the representation for the media resource's URI + """ + return self.uri_parser(course) + + +class _CourseApiMediaCollectionSerializer(serializers.Serializer): # pylint: disable=abstract-method + """ + Nested serializer to represent a collection of media objects + """ + course_image = _MediaSerializer(source='*', uri_parser=course_image_url) + + +class CourseSerializer(serializers.Serializer): # pylint: disable=abstract-method + """ + Serializer for Course objects + """ + + course_id = serializers.CharField(source='id', read_only=True) # pylint: disable=invalid-name + name = serializers.CharField(source='display_name_with_default') + number = serializers.CharField(source='display_number_with_default') + org = serializers.CharField(source='display_org_with_default') + description = serializers.SerializerMethodField() + media = _CourseApiMediaCollectionSerializer(source='*') + start = serializers.DateTimeField() + start_type = serializers.SerializerMethodField() + start_display = serializers.SerializerMethodField() + end = serializers.DateTimeField() + enrollment_start = serializers.DateTimeField() + enrollment_end = serializers.DateTimeField() + blocks_url = serializers.SerializerMethodField() + + def get_start_type(self, course): + """ + Get the representation for SerializerMethodField `start_type` + """ + if course.advertised_start is not None: + return u'string' + elif course.start != DEFAULT_START_DATE: + return u'timestamp' + else: + return u'empty' + + def get_start_display(self, course): + """ + Get the representation for SerializerMethodField `start_display` + """ + if course.advertised_start is not None: + return course.advertised_start + elif course.start != DEFAULT_START_DATE: + return defaultfilters.date(course.start, "DATE_FORMAT") + else: + return None + + def get_description(self, course): + """ + Get the representation for SerializerMethodField `description` + """ + return get_course_about_section(self.context['request'], course, 'short_description').strip() + + def get_blocks_url(self, course): + """ + Get the representation for SerializerMethodField `blocks_url` + """ + return '?'.join([ + reverse('blocks_in_course'), + urllib.urlencode({'course_id': course.id}), + ]) diff --git a/lms/djangoapps/course_api/tests/__init__.py b/lms/djangoapps/course_api/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/course_api/tests/mixins.py b/lms/djangoapps/course_api/tests/mixins.py new file mode 100644 index 0000000000..bdac14f05e --- /dev/null +++ b/lms/djangoapps/course_api/tests/mixins.py @@ -0,0 +1,42 @@ +""" +Common mixins for Course API Tests +""" + +from datetime import datetime + +from student.tests.factories import UserFactory +from xmodule.modulestore.tests.factories import ToyCourseFactory + + +TEST_PASSWORD = u'edx' + + +class CourseApiFactoryMixin(object): + """ + Mixin to allow creation of test courses and users. + """ + + @staticmethod + def create_course(**kwargs): + """ + Create a course for use in test cases + """ + + return ToyCourseFactory.create( + end=datetime(2015, 9, 19, 18, 0, 0), + enrollment_start=datetime(2015, 6, 15, 0, 0, 0), + enrollment_end=datetime(2015, 7, 15, 0, 0, 0), + **kwargs + ) + + @staticmethod + def create_user(username, is_staff): + """ + Create a user as identified by username, email, password and is_staff. + """ + return UserFactory( + username=username, + email=u'{}@example.com'.format(username), + password=TEST_PASSWORD, + is_staff=is_staff + ) diff --git a/lms/djangoapps/course_api/tests/test_api.py b/lms/djangoapps/course_api/tests/test_api.py new file mode 100644 index 0000000000..8d4b446e52 --- /dev/null +++ b/lms/djangoapps/course_api/tests/test_api.py @@ -0,0 +1,213 @@ +""" +Test for course API +""" + +from datetime import datetime +from django.contrib.auth.models import AnonymousUser +from django.test import RequestFactory +from rest_framework.exceptions import NotFound, PermissionDenied + +from opaque_keys.edx.keys import CourseKey +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase, ModuleStoreTestCase +from xmodule.course_module import DEFAULT_START_DATE + +from ..api import course_detail, list_courses +from .mixins import CourseApiFactoryMixin + + +class CourseApiTestMixin(CourseApiFactoryMixin): + """ + Establish basic functionality for Course API tests + """ + + maxDiff = 5000 # long enough to show mismatched dicts + + expected_course_data = { + 'course_id': u'edX/toy/2012_Fall', + 'name': u'Toy Course', + 'number': u'toy', + 'org': u'edX', + 'description': u'A course about toys.', + 'media': { + 'course_image': { + 'uri': u'/c4x/edX/toy/asset/just_a_test.jpg', + } + }, + 'start': u'2015-07-17T12:00:00Z', + 'start_type': u'timestamp', + 'start_display': u'July 17, 2015', + 'end': u'2015-09-19T18:00:00Z', + 'enrollment_start': u'2015-06-15T00:00:00Z', + 'enrollment_end': u'2015-07-15T00:00:00Z', + 'blocks_url': '/api/courses/v1/blocks/?course_id=edX%2Ftoy%2F2012_Fall', + } + + @classmethod + def setUpClass(cls): + super(CourseApiTestMixin, cls).setUpClass() + cls.request_factory = RequestFactory() + + +class CourseDetailTestMixin(CourseApiTestMixin): + """ + Common functionality for course_detail tests + """ + def _make_api_call(self, requesting_user, target_user, course_key): + """ + Call the `course_detail` api endpoint to get information on the course + identified by `course_key`. + """ + request = self.request_factory.get('/') + request.user = requesting_user + return course_detail(request, target_user, course_key) + + +class TestGetCourseDetail(CourseDetailTestMixin, SharedModuleStoreTestCase): + """ + Test course_detail api function + """ + @classmethod + def setUpClass(cls): + super(TestGetCourseDetail, cls).setUpClass() + cls.course = cls.create_course() + cls.hidden_course = cls.create_course(course=u'hidden', visible_to_staff_only=True) + cls.honor_user = cls.create_user('honor', is_staff=False) + cls.staff_user = cls.create_user('staff', is_staff=True) + + def test_get_existing_course(self): + result = self._make_api_call(self.honor_user, self.honor_user.username, self.course.id) + self.assertEqual(self.expected_course_data, result) + + def test_get_nonexistent_course(self): + course_key = CourseKey.from_string(u'edX/toy/nope') + with self.assertRaises(NotFound): + self._make_api_call(self.honor_user, self.honor_user.username, course_key) + + def test_hidden_course_for_honor(self): + with self.assertRaises(NotFound): + self._make_api_call(self.honor_user, self.honor_user.username, self.hidden_course.id) + + def test_hidden_course_for_staff(self): + result = self._make_api_call(self.staff_user, self.staff_user.username, self.hidden_course.id) + self.assertIsInstance(result, dict) + self.assertEqual(result['course_id'], u'edX/hidden/2012_Fall') + + def test_hidden_course_for_staff_as_honor(self): + with self.assertRaises(NotFound): + self._make_api_call(self.staff_user, self.honor_user.username, self.hidden_course.id) + + +class TestGetCourseDetailStartDate(CourseDetailTestMixin, ModuleStoreTestCase): + """ + Test variations of start_date field responses + """ + + def setUp(self): + super(TestGetCourseDetailStartDate, self).setUp() + self.staff_user = self.create_user('staff', is_staff=True) + + def test_course_with_advertised_start(self): + course = self.create_course( + course=u'custom', + start=datetime(2015, 3, 15), + advertised_start=u'The Ides of March' + ) + result = self._make_api_call(self.staff_user, self.staff_user.username, course.id) + self.assertEqual(result['course_id'], u'edX/custom/2012_Fall') + self.assertEqual(result['start_type'], u'string') + self.assertEqual(result['start_display'], u'The Ides of March') + + def test_course_with_empty_start_date(self): + course = self.create_course(start=DEFAULT_START_DATE, course=u'custom2') + result = self._make_api_call(self.staff_user, self.staff_user.username, course.id) + self.assertEqual(result['course_id'], u'edX/custom2/2012_Fall') + self.assertEqual(result['start_type'], u'empty') + self.assertIsNone(result['start_display']) + + +class CourseListTestMixin(CourseApiTestMixin): + """ + Common behavior for list_courses tests + """ + def _make_api_call(self, requesting_user, specified_user): + """ + Call the list_courses api endpoint to get information about + `specified_user` on behalf of `requesting_user`. + """ + request = self.request_factory.get('/') + request.user = requesting_user + return list_courses(request, specified_user.username) + + +class TestGetCourseList(CourseListTestMixin, SharedModuleStoreTestCase): + """ + Test the behavior of the `list_courses` api function. + """ + @classmethod + def setUpClass(cls): + super(TestGetCourseList, cls).setUpClass() + cls.create_course() + cls.staff_user = cls.create_user("staff", is_staff=True) + cls.honor_user = cls.create_user("honor", is_staff=False) + + def test_as_staff(self): + courses = self._make_api_call(self.staff_user, self.staff_user) + self.assertEqual(len(courses), 1) + self.assertEqual(courses[0], self.expected_course_data) + + def test_for_honor_user_as_staff(self): + courses = self._make_api_call(self.staff_user, self.honor_user) + self.assertEqual(len(courses), 1) + self.assertEqual(courses[0], self.expected_course_data) + + def test_as_honor(self): + courses = self._make_api_call(self.honor_user, self.honor_user) + self.assertEqual(len(courses), 1) + self.assertEqual(courses[0], self.expected_course_data) + + def test_for_staff_user_as_honor(self): + with self.assertRaises(PermissionDenied): + self._make_api_call(self.honor_user, self.staff_user) + + def test_as_anonymous(self): + anonuser = AnonymousUser() + courses = self._make_api_call(anonuser, anonuser) + self.assertEqual(len(courses), 1) + self.assertEqual(courses[0], self.expected_course_data) + + def test_for_honor_user_as_anonymous(self): + anonuser = AnonymousUser() + with self.assertRaises(PermissionDenied): + self._make_api_call(anonuser, self.staff_user) + + def test_multiple_courses(self): + self.create_course(course='second') + courses = self._make_api_call(self.honor_user, self.honor_user) + self.assertEqual(len(courses), 2) + + +class TestGetCourseListExtras(CourseListTestMixin, ModuleStoreTestCase): + """ + Tests of course_list api function that require alternative configurations + of created courses. + """ + @classmethod + def setUpClass(cls): + super(TestGetCourseListExtras, cls).setUpClass() + cls.staff_user = cls.create_user("staff", is_staff=True) + cls.honor_user = cls.create_user("honor", is_staff=False) + + def test_no_courses(self): + courses = self._make_api_call(self.honor_user, self.honor_user) + self.assertEqual(len(courses), 0) + + def test_hidden_course_for_honor(self): + self.create_course(visible_to_staff_only=True) + courses = self._make_api_call(self.honor_user, self.honor_user) + self.assertEqual(len(courses), 0) + + def test_hidden_course_for_staff(self): + self.create_course(visible_to_staff_only=True) + courses = self._make_api_call(self.staff_user, self.staff_user) + self.assertEqual(len(courses), 1) + self.assertEqual(courses[0], self.expected_course_data) diff --git a/lms/djangoapps/course_api/tests/test_permissions.py b/lms/djangoapps/course_api/tests/test_permissions.py new file mode 100644 index 0000000000..fbfccd0a00 --- /dev/null +++ b/lms/djangoapps/course_api/tests/test_permissions.py @@ -0,0 +1,48 @@ +""" +Test authorization functions +""" + +from django.contrib.auth.models import AnonymousUser +from django.test import TestCase + +from .mixins import CourseApiFactoryMixin + +from ..permissions import can_view_courses_for_username + + +class ViewCoursesForUsernameTestCase(CourseApiFactoryMixin, TestCase): + """ + Verify functionality of view_courses_for_username. + + Any user should be able to view their own courses, and staff users + should be able to view anyone's courses. + """ + + @classmethod + def setUpClass(cls): + super(ViewCoursesForUsernameTestCase, cls).setUpClass() + cls.staff_user = cls.create_user('staff', is_staff=True) + cls.honor_user = cls.create_user('honor', is_staff=False) + cls.anonymous_user = AnonymousUser() + + def test_for_staff(self): + self.assertTrue(can_view_courses_for_username(self.staff_user, self.staff_user.username)) + + def test_for_honor(self): + self.assertTrue(can_view_courses_for_username(self.honor_user, self.honor_user.username)) + + def test_for_staff_as_honor(self): + self.assertTrue(can_view_courses_for_username(self.staff_user, self.honor_user.username)) + + def test_for_honor_as_staff(self): + self.assertFalse(can_view_courses_for_username(self.honor_user, self.staff_user.username)) + + def test_for_none_as_staff(self): + with self.assertRaises(TypeError): + can_view_courses_for_username(self.staff_user, None) + + def test_for_anonymous(self): + self.assertTrue(can_view_courses_for_username(self.anonymous_user, self.anonymous_user.username)) + + def test_for_anonymous_as_honor(self): + self.assertFalse(can_view_courses_for_username(self.anonymous_user, self.honor_user.username)) diff --git a/lms/djangoapps/course_api/tests/test_views.py b/lms/djangoapps/course_api/tests/test_views.py new file mode 100644 index 0000000000..ab85274423 --- /dev/null +++ b/lms/djangoapps/course_api/tests/test_views.py @@ -0,0 +1,146 @@ +""" +Tests for Blocks Views +""" + +from django.core.urlresolvers import reverse +from django.test import RequestFactory +from rest_framework.exceptions import NotFound + +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase + +from .mixins import CourseApiFactoryMixin, TEST_PASSWORD + +from ..views import CourseDetailView + + +class CourseApiTestViewMixin(CourseApiFactoryMixin): + """ + Mixin class for test helpers for Course API views + """ + + def setup_user(self, requesting_user): + """ + log in the specified user, and remember it as `self.user` + """ + self.user = requesting_user # pylint: disable=attribute-defined-outside-init + self.client.login(username=self.user.username, password=TEST_PASSWORD) + + def verify_response(self, expected_status_code=200, params=None, url=None): + """ + Ensure that sending a GET request to self.url returns the expected + status code (200 by default). + + Arguments: + expected_status_code: (default 200) + params: + query parameters to include in the request. Can include + `username`. + + Returns: + response: (HttpResponse) The response returned by the request + """ + query_params = {} + query_params.update(params or {}) + response = self.client.get(url or self.url, data=query_params) + self.assertEqual(response.status_code, expected_status_code) + return response + + +class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase): + """ + Test responses returned from CourseListView. + """ + + @classmethod + def setUpClass(cls): + super(CourseListViewTestCase, cls).setUpClass() + cls.course = cls.create_course() + cls.url = reverse('course-list') + cls.staff_user = cls.create_user(username='staff', is_staff=True) + cls.honor_user = cls.create_user(username='honor', is_staff=False) + + def test_as_staff(self): + self.setup_user(self.staff_user) + self.verify_response() + + def test_as_staff_for_honor(self): + self.setup_user(self.staff_user) + self.verify_response(params={'username': self.honor_user.username}) + + def test_as_honor(self): + self.setup_user(self.honor_user) + self.verify_response() + + def test_as_honor_for_explicit_self(self): + self.setup_user(self.honor_user) + self.verify_response(params={'username': self.honor_user.username}) + + def test_as_honor_for_staff(self): + self.setup_user(self.honor_user) + self.verify_response(expected_status_code=403, params={'username': self.staff_user.username}) + + def test_not_logged_in(self): + self.client.logout() + self.verify_response() + + +class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase): + """ + Test responses returned from CourseDetailView. + """ + + @classmethod + def setUpClass(cls): + super(CourseDetailViewTestCase, cls).setUpClass() + cls.course = cls.create_course() + cls.hidden_course = cls.create_course(course=u'hidden', visible_to_staff_only=True) + cls.url = reverse('course-detail', kwargs={'course_key_string': cls.course.id}) + cls.hidden_url = reverse('course-detail', kwargs={'course_key_string': cls.hidden_course.id}) + cls.nonexistent_url = reverse('course-detail', kwargs={'course_key_string': 'edX/nope/Fall_2014'}) + cls.staff_user = cls.create_user(username='staff', is_staff=True) + cls.honor_user = cls.create_user(username='honor', is_staff=False) + + def test_as_honor(self): + self.setup_user(self.honor_user) + self.verify_response() + + def test_as_honor_for_explicit_self(self): + self.setup_user(self.honor_user) + self.verify_response(params={'username': self.honor_user.username}) + + def test_as_honor_for_staff(self): + self.setup_user(self.honor_user) + self.verify_response(expected_status_code=403, params={'username': self.staff_user.username}) + + def test_as_staff(self): + self.setup_user(self.staff_user) + self.verify_response() + + def test_as_staff_for_honor(self): + self.setup_user(self.staff_user) + self.verify_response(params={'username': self.honor_user.username}) + + def test_as_anonymous_user(self): + self.verify_response(expected_status_code=401) + + def test_hidden_course_as_honor(self): + self.setup_user(self.honor_user) + self.verify_response(expected_status_code=404, url=self.hidden_url) + + def test_hidden_course_as_staff(self): + self.setup_user(self.staff_user) + self.verify_response(url=self.hidden_url) + + def test_nonexistent_course(self): + self.setup_user(self.staff_user) + self.verify_response(expected_status_code=404, url=self.nonexistent_url) + + def test_invalid_course_key(self): + # Our URL patterns try to block invalid course keys. If one got + # through, this is how the view would respond. + request_factory = RequestFactory() + request = request_factory.get('/') + request.query_params = {} + request.user = self.staff_user + with self.assertRaises(NotFound): + CourseDetailView().get(request, 'a:b:c') diff --git a/lms/djangoapps/course_api/urls.py b/lms/djangoapps/course_api/urls.py index 7800ad2e67..bf24e7ead4 100644 --- a/lms/djangoapps/course_api/urls.py +++ b/lms/djangoapps/course_api/urls.py @@ -4,11 +4,12 @@ Course API URLs from django.conf import settings from django.conf.urls import patterns, url, include -from .views import CourseView +from .views import CourseDetailView, CourseListView urlpatterns = patterns( '', - url(r'^v1/courses/{}'.format(settings.COURSE_KEY_PATTERN), CourseView.as_view(), name="course_detail"), + url(r'^v1/courses/$', CourseListView.as_view(), name="course-list"), + url(r'^v1/courses/{}'.format(settings.COURSE_KEY_PATTERN), CourseDetailView.as_view(), name="course-detail"), url(r'', include('course_api.blocks.urls')) ) diff --git a/lms/djangoapps/course_api/views.py b/lms/djangoapps/course_api/views.py index 64d20bd889..b730bd5cc5 100644 --- a/lms/djangoapps/course_api/views.py +++ b/lms/djangoapps/course_api/views.py @@ -2,52 +2,167 @@ Course API Views """ -from rest_framework.views import APIView -from rest_framework.response import Response -from rest_framework.reverse import reverse +from rest_framework.exceptions import NotFound +from rest_framework.views import APIView, Response +from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey + + from openedx.core.lib.api.view_utils import view_auth_classes -from xmodule.modulestore.django import modulestore + +from .api import course_detail, list_courses @view_auth_classes() -class CourseView(APIView): +class CourseDetailView(APIView): """ - Course API view + **Use Cases** + + Request information on a course + + **Example Requests** + + GET /api/courses/v1/courses/{course_key}/ + + **Response Values** + + Body consists of the following fields: + + * blocks_url: used to fetch the course blocks + * media: An object that contains named media items. Included here: + * course_image: An image to show for the course. Represented + as an object with the following fields: + * uri: The location of the image + * name: + * description: + * type: + * end: Date the course ends + * enrollment_end: Date enrollment ends + * enrollment_start: Date enrollment begins + * course_id: Course key + * name: Name of the course + * number: Catalog number of the course + * org: Name of the organization that owns the course + * description: A textual description of the course + * start: Date the course begins + * start_display: Readably formatted start of the course + * start_type: Hint describing how `start_display` is set. One of: + * `"string"`: manually set + * `"timestamp"`: generated form `start` timestamp + * `"empty"`: the start date should not be shown + + **Parameters:** + + username (optional): + The username of the specified user whose visible courses we + want to see. Defaults to the current user. + + **Returns** + + * 200 on success with above fields. + * 403 if a user who does not have permission to masquerade as + another user specifies a username other than their own. + * 404 if the course is not available or cannot be seen. + + Example response: + + { + "blocks_url": "/api/courses/v1/blocks/?course_id=edX%2Fexample%2F2012_Fall", + "media": { + "course_image": { + "uri": "/c4x/edX/example/asset/just_a_test.jpg", + "name": "Course Image" + } + }, + "description": "An example course.", + "end": "2015-09-19T18:00:00Z", + "enrollment_end": "2015-07-15T00:00:00Z", + "enrollment_start": "2015-06-15T00:00:00Z", + "id": "edX/example/2012_Fall", + "name": "Example Course", + "number": "example", + "org": "edX", + "start": "2015-07-17T12:00:00Z", + "start_display": "July 17, 2015", + "start_type": "timestamp" + } """ def get(self, request, course_key_string): """ - Request information on a course specified by `course_key_string`. - Body consists of a `blocks_url` that can be used to fetch the - blocks for the requested course. + GET /api/courses/v1/courses/{course_key}/ + """ - Arguments: - request (HttpRequest) - course_key_string - - Returns: - HttpResponse: 200 on success + username = request.query_params.get('username', request.user.username) + try: + course_key = CourseKey.from_string(course_key_string) + except InvalidKeyError: + raise NotFound() + content = course_detail(request, username, course_key) + return Response(content) - Example Usage: +class CourseListView(APIView): + """ + **Use Cases** - GET /api/courses/v1/[course_key_string] - 200 OK + Request information on all courses visible to the specified user. + + **Example Requests** + + GET /api/courses/v1/courses/ + + **Response Values** + + Body comprises a list of objects as returned by `CourseDetailView`. + + **Parameters** + + username (optional): + The username of the specified user whose visible courses we + want to see. Defaults to the current user. + + **Returns** + + * 200 on success, with a list of course discovery objects as returned + by `CourseDetailView`. + * 403 if a user who does not have permission to masquerade as + another user specifies a username other than their own. + * 404 if the specified user does not exist, or the requesting user does + not have permission to view their courses. Example response: - {"blocks_url": "https://server/api/courses/v1/blocks/[usage_key]"} + [ + { + "blocks_url": "/api/courses/v1/blocks/?course_id=edX%2Fexample%2F2012_Fall", + "media": { + "course_image": { + "uri": "/c4x/edX/example/asset/just_a_test.jpg", + "name": "Course Image" + } + }, + "description": "An example course.", + "end": "2015-09-19T18:00:00Z", + "enrollment_end": "2015-07-15T00:00:00Z", + "enrollment_start": "2015-06-15T00:00:00Z", + "id": "edX/example/2012_Fall", + "name": "Example Course", + "number": "example", + "org": "edX", + "start": "2015-07-17T12:00:00Z", + "start_display": "July 17, 2015", + "start_type": "timestamp" + } + ] + """ + + def get(self, request): """ + GET /api/courses/v1/courses/ + """ + username = request.query_params.get('username', request.user.username) - course_key = CourseKey.from_string(course_key_string) - course_usage_key = modulestore().make_course_usage_key(course_key) - - blocks_url = reverse( - 'blocks_in_block_tree', - kwargs={'usage_key_string': unicode(course_usage_key)}, - request=request, - ) - - return Response({'blocks_url': blocks_url}) + content = list_courses(request, username) + return Response(content) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 6f049d5ccf..ba371bc2e3 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -302,10 +302,6 @@ def _has_access_course_desc(user, action, course): """ Can see if can enroll, but also if can load it: if user enrolled in a course and now it's past the enrollment period, they should still see it. - - TODO (vshnayder): This means that courses with limited enrollment periods will not appear - to non-staff visitors after the enrollment period is over. If this is not what we want, will - need to change this logic. """ # VS[compat] -- this setting should go away once all courses have # properly configured enrollment_start times (if course should be diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index d5c674f6fb..709cc2fd42 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -44,21 +44,6 @@ from opaque_keys.edx.keys import UsageKey log = logging.getLogger(__name__) -def get_request_for_thread(): - """Walk up the stack, return the nearest first argument named "request".""" - frame = None - try: - for f in inspect.stack()[1:]: - frame = f[0] - code = frame.f_code - if code.co_varnames[:1] == ("request",): - return frame.f_locals["request"] - elif code.co_varnames[:2] == ("self", "request",): - return frame.f_locals["request"] - finally: - del frame - - def get_course(course_id, depth=0): """ Given a course id, return the corresponding course descriptor. @@ -178,7 +163,7 @@ def get_course_university_about_section(course): # pylint: disable=invalid-name return course.display_org_with_default -def get_course_about_section(course, section_key): +def get_course_about_section(request, course, section_key): """ This returns the snippet of html to be rendered on the course about page, given the key for the section. @@ -206,17 +191,30 @@ def get_course_about_section(course, section_key): # markup. This can change without effecting this interface when we find a # good format for defining so many snippets of text/html. - # TODO: Remove number, instructors from this list - if section_key in ['short_description', 'description', 'key_dates', 'video', - 'course_staff_short', 'course_staff_extended', - 'requirements', 'syllabus', 'textbook', 'faq', 'more_info', - 'number', 'instructors', 'overview', - 'effort', 'end_date', 'prerequisites', 'ocw_links']: + # TODO: Remove number, instructors from this set + html_sections = { + 'short_description', + 'description', + 'key_dates', + 'video', + 'course_staff_short', + 'course_staff_extended', + 'requirements', + 'syllabus', + 'textbook', + 'faq', + 'more_info', + 'number', + 'instructors', + 'overview', + 'effort', + 'end_date', + 'prerequisites', + 'ocw_links' + } + if section_key in html_sections: try: - - request = get_request_for_thread() - loc = course.location.replace(category='about', name=section_key) # Use an empty cache diff --git a/lms/djangoapps/courseware/tests/test_courses.py b/lms/djangoapps/courseware/tests/test_courses.py index 23806a07a6..935b5d9d2e 100644 --- a/lms/djangoapps/courseware/tests/test_courses.py +++ b/lms/djangoapps/courseware/tests/test_courses.py @@ -198,12 +198,10 @@ class CoursesRenderTest(ModuleStoreTestCase): course_info = get_course_info_section(self.request, self.course, 'handouts') self.assertIn("this module is temporarily unavailable", course_info) - @mock.patch('courseware.courses.get_request_for_thread') - def test_get_course_about_section_render(self, mock_get_request): - mock_get_request.return_value = self.request + def test_get_course_about_section_render(self): # Test render works okay - course_about = get_course_about_section(self.course, 'short_description') + course_about = get_course_about_section(self.request, self.course, 'short_description') self.assertEqual(course_about, "A course about toys.") # Test when render raises an exception @@ -211,7 +209,7 @@ class CoursesRenderTest(ModuleStoreTestCase): mock_module_render.return_value = mock.MagicMock( render=mock.Mock(side_effect=Exception('Render failed!')) ) - course_about = get_course_about_section(self.course, 'short_description') + course_about = get_course_about_section(self.request, self.course, 'short_description') self.assertIn("this module is temporarily unavailable", course_about) diff --git a/lms/templates/course.html b/lms/templates/course.html index f52be12a90..b9251a37b3 100644 --- a/lms/templates/course.html +++ b/lms/templates/course.html @@ -5,25 +5,25 @@ from django.core.urlresolvers import reverse from courseware.courses import course_image_url, get_course_about_section %> <%page args="course" /> -
+
- ${get_course_about_section(course, 'title')} ${course.display_number_with_default} + ${get_course_about_section(request, course, 'title')} ${course.display_number_with_default}
- % if get_course_about_section(course, "video"): + % if get_course_about_section(request, course, "video"):
@@ -217,7 +217,7 @@ from edxmako.shortcuts import marketing_link
- ${get_course_about_section(course, "overview")} + ${get_course_about_section(request, course, "overview")}
@@ -231,10 +231,10 @@ from edxmako.shortcuts import marketing_link ## or something allowing themes to do whatever they ## want here (and on this whole page, really). % if self.stanford_theme_enabled(): -
- % else: @@ -246,7 +246,7 @@ from edxmako.shortcuts import marketing_link ## Twitter account. {url} should appear at the end of the text. tweet_text = _("I just enrolled in {number} {title} through {account}: {url}").format( number=course.number, - title=get_course_about_section(course, 'title'), + title=get_course_about_section(request, course, 'title'), account=microsite.get_value('course_about_twitter_account', settings.PLATFORM_TWITTER_ACCOUNT), url=u"http://{domain}{path}".format( domain=site_domain, @@ -261,7 +261,7 @@ from edxmako.shortcuts import marketing_link subject=_("Take a course with {platform} online").format(platform=platform_name), body=_("I just enrolled in {number} {title} through {platform} {url}").format( number=course.number, - title=get_course_about_section(course, 'title'), + title=get_course_about_section(request, course, 'title'), platform=platform_name, url=u"http://{domain}{path}".format( domain=site_domain, @@ -291,13 +291,13 @@ from edxmako.shortcuts import marketing_link % endif ## We plan to ditch end_date (which is not stored in course metadata), ## but for backwards compatibility, show about/end_date blob if it exists. - % if get_course_about_section(course, "end_date") or course.end: + % if get_course_about_section(request, course, "end_date") or course.end:
  • ${_("Classes End")}

    - % if get_course_about_section(course, "end_date"): - ${get_course_about_section(course, "end_date")} + % if get_course_about_section(request, course, "end_date"): + ${get_course_about_section(request, course, "end_date")} % else: ${course.end_datetime_text()} % endif @@ -305,8 +305,8 @@ from edxmako.shortcuts import marketing_link
  • % endif - % if get_course_about_section(course, "effort"): -
  • ${_("Estimated Effort")}

    ${get_course_about_section(course, "effort")}
  • + % if get_course_about_section(request, course, "effort"): +
  • ${_("Estimated Effort")}

    ${get_course_about_section(request, course, "effort")}
  • % endif ##
  • ${_('Course Length')}

    ${_('{number} weeks').format(number=15)}
  • @@ -335,15 +335,15 @@ from edxmako.shortcuts import marketing_link

    % endif - % if get_course_about_section(course, "prerequisites"): -
  • ${_("Requirements")}

    ${get_course_about_section(course, "prerequisites")}
  • + % if get_course_about_section(request, course, "prerequisites"): +
  • ${_("Requirements")}

    ${get_course_about_section(request, course, "prerequisites")}
  • % endif ## For now, ocw links are the only thing that goes in additional resources - % if get_course_about_section(course, "ocw_links"): + % if get_course_about_section(request, course, "ocw_links"):

    ${_("Additional Resources")}

    @@ -352,7 +352,7 @@ from edxmako.shortcuts import marketing_link
    ## "MITOpenCourseware" should *not* be translated

    MITOpenCourseware

    - ${get_course_about_section(course, "ocw_links")} + ${get_course_about_section(request, course, "ocw_links")}
    %endif diff --git a/lms/templates/shoppingcart/receipt.html b/lms/templates/shoppingcart/receipt.html index 3940af4c63..3524ab6599 100644 --- a/lms/templates/shoppingcart/receipt.html +++ b/lms/templates/shoppingcart/receipt.html @@ -292,7 +292,7 @@ from microsite_configuration import microsite
    ${course.display_number_with_default | h} ${get_course_about_section(course, 'title')} Image + alt="${course.display_number_with_default | h} ${get_course_about_section(request, course, 'title')} Image"/>
    diff --git a/lms/templates/shoppingcart/registration_code_receipt.html b/lms/templates/shoppingcart/registration_code_receipt.html index b2f37b452d..b03096cea9 100644 --- a/lms/templates/shoppingcart/registration_code_receipt.html +++ b/lms/templates/shoppingcart/registration_code_receipt.html @@ -20,7 +20,7 @@ from courseware.courses import course_image_url, get_course_about_section ${_(
    diff --git a/lms/templates/shoppingcart/registration_code_redemption.html b/lms/templates/shoppingcart/registration_code_redemption.html index 3c91e41ce3..b8b62da538 100644 --- a/lms/templates/shoppingcart/registration_code_redemption.html +++ b/lms/templates/shoppingcart/registration_code_redemption.html @@ -20,7 +20,7 @@ from courseware.courses import course_image_url, get_course_about_section ${_(
    diff --git a/lms/templates/shoppingcart/shopping_cart.html b/lms/templates/shoppingcart/shopping_cart.html index 9d2a4b53a7..01b71824fb 100644 --- a/lms/templates/shoppingcart/shopping_cart.html +++ b/lms/templates/shoppingcart/shopping_cart.html @@ -67,7 +67,7 @@ from django.utils.translation import ungettext
    ${course.display_number_with_default | h} ${get_course_about_section(course, 'title')} ${_('Cover Image')} + alt="${course.display_number_with_default | h} ${get_course_about_section(request, course, 'title')} ${_('Cover Image')}" />
    ## Translators: "Registration for:" is followed by a course name diff --git a/lms/templates/video_modal.html b/lms/templates/video_modal.html index a895df78d0..37c465052c 100644 --- a/lms/templates/video_modal.html +++ b/lms/templates/video_modal.html @@ -1,10 +1,10 @@ -<%! +<%! from courseware.courses import get_course_about_section %> <%namespace name='static' file='static_content.html'/>