From 58865ee0a8616028b810c85dd5e52b0516296391 Mon Sep 17 00:00:00 2001 From: Clinton Blackburn Date: Thu, 6 Aug 2015 18:38:33 -0400 Subject: [PATCH] Added API endpoints for CreditCourse Endpoints support create, read, and list operations. NOTE: This commit also includes a retrofitted SimpleRouter that supports overriding the lookup regex. This retrofit is simpler to implement than updating edx-ora2 which is pinned to DRF 2.3.x. XCOM-524 --- lms/envs/test.py | 2 + openedx/core/djangoapps/credit/routers.py | 32 ++++ openedx/core/djangoapps/credit/serializers.py | 13 ++ .../djangoapps/credit/tests/test_views.py | 150 ++++++++++++++++-- openedx/core/djangoapps/credit/urls.py | 51 +++--- openedx/core/djangoapps/credit/views.py | 37 ++++- 6 files changed, 243 insertions(+), 42 deletions(-) create mode 100644 openedx/core/djangoapps/credit/routers.py create mode 100644 openedx/core/djangoapps/credit/serializers.py diff --git a/lms/envs/test.py b/lms/envs/test.py index eaddc180f1..bc3986ff26 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -63,6 +63,8 @@ FEATURES['ENABLE_SHOPPING_CART'] = True FEATURES['ENABLE_VERIFIED_CERTIFICATES'] = True +FEATURES['ENABLE_CREDIT_API'] = True + # Enable this feature for course staff grade downloads, to enable acceptance tests FEATURES['ENABLE_S3_GRADE_DOWNLOADS'] = True FEATURES['ALLOW_COURSE_STAFF_GRADE_DOWNLOADS'] = True diff --git a/openedx/core/djangoapps/credit/routers.py b/openedx/core/djangoapps/credit/routers.py new file mode 100644 index 0000000000..33a031a6da --- /dev/null +++ b/openedx/core/djangoapps/credit/routers.py @@ -0,0 +1,32 @@ +""" DRF routers. """ + +from rest_framework import routers + + +class SimpleRouter(routers.SimpleRouter): + """ Simple DRF router. """ + + # Note (CCB): This is a retrofit of a DRF 2.4 feature onto DRF 2.3. This is, sadly, simpler than + # updating edx-ora2 to work with DRF 2.4. See https://github.com/tomchristie/django-rest-framework/pull/1333 + # for details on this specific DRF 2.4 feature. + def get_lookup_regex(self, viewset, lookup_prefix=''): + """ + Given a viewset, return the portion of URL regex that is used + to match against a single instance. + Note that lookup_prefix is not used directly inside REST rest_framework + itself, but is required in order to nicely support nested router + implementations, such as drf-nested-routers. + https://github.com/alanjds/drf-nested-routers + """ + base_regex = '(?P<{lookup_prefix}{lookup_field}>{lookup_value})' + lookup_field = getattr(viewset, 'lookup_field', 'pk') + try: + lookup_value = viewset.lookup_value_regex + except AttributeError: + # Don't consume `.json` style suffixes + lookup_value = '[^/.]+' + return base_regex.format( + lookup_prefix=lookup_prefix, + lookup_field=lookup_field, + lookup_value=lookup_value + ) diff --git a/openedx/core/djangoapps/credit/serializers.py b/openedx/core/djangoapps/credit/serializers.py new file mode 100644 index 0000000000..d808233848 --- /dev/null +++ b/openedx/core/djangoapps/credit/serializers.py @@ -0,0 +1,13 @@ +""" Credit API Serializers """ + +from rest_framework import serializers + +from openedx.core.djangoapps.credit.models import CreditCourse + + +class CreditCourseSerializer(serializers.ModelSerializer): + """ CreditCourse Serializer """ + + class Meta(object): # pylint: disable=missing-docstring + model = CreditCourse + exclude = ('id',) diff --git a/openedx/core/djangoapps/credit/tests/test_views.py b/openedx/core/djangoapps/credit/tests/test_views.py index dcf907ae7d..48118e4e58 100644 --- a/openedx/core/djangoapps/credit/tests/test_views.py +++ b/openedx/core/djangoapps/credit/tests/test_views.py @@ -1,22 +1,23 @@ """ Tests for credit app views. """ -import unittest -import json import datetime -import pytz +import json +import unittest import ddt -from mock import patch +from django.conf import settings +from django.core.urlresolvers import reverse from django.test import TestCase from django.test.utils import override_settings -from django.core.urlresolvers import reverse -from django.conf import settings +from mock import patch +from oauth2_provider.tests.factories import AccessTokenFactory, ClientFactory +from opaque_keys.edx.keys import CourseKey +import pytz from student.tests.factories import UserFactory -from util.testing import UrlResetMixin from util.date_utils import to_timestamp -from opaque_keys.edx.keys import CourseKey +from util.testing import UrlResetMixin from openedx.core.djangoapps.credit import api from openedx.core.djangoapps.credit.signature import signature from openedx.core.djangoapps.credit.models import ( @@ -28,7 +29,7 @@ from openedx.core.djangoapps.credit.models import ( CreditRequest, ) - +JSON = 'application/json' TEST_CREDIT_PROVIDER_SECRET_KEY = "931433d583c84ca7ba41784bad3232e6" @@ -167,7 +168,7 @@ class CreditProviderViewTests(UrlResetMixin, TestCase): ) def test_create_credit_request_invalid_parameters(self, request_data): url = reverse("credit:create_request", args=[self.PROVIDER_ID]) - response = self.client.post(url, data=request_data, content_type="application/json") + response = self.client.post(url, data=request_data, content_type=JSON) self.assertEqual(response.status_code, 400) def test_credit_provider_callback_validates_signature(self): @@ -228,7 +229,7 @@ class CreditProviderViewTests(UrlResetMixin, TestCase): ) def test_credit_provider_callback_invalid_parameters(self, request_data): url = reverse("credit:provider_callback", args=[self.PROVIDER_ID]) - response = self.client.post(url, data=request_data, content_type="application/json") + response = self.client.post(url, data=request_data, content_type=JSON) self.assertEqual(response.status_code, 400) def test_credit_provider_invalid_status(self): @@ -286,7 +287,7 @@ class CreditProviderViewTests(UrlResetMixin, TestCase): "username": username, "course_key": unicode(course_key), }), - content_type="application/json", + content_type=JSON, ) def _create_credit_request_and_get_uuid(self, username, course_key): @@ -326,7 +327,7 @@ class CreditProviderViewTests(UrlResetMixin, TestCase): } parameters["signature"] = kwargs.get("sig", signature(parameters, secret_key)) - return self.client.post(url, data=json.dumps(parameters), content_type="application/json") + return self.client.post(url, data=json.dumps(parameters), content_type=JSON) def _assert_request_status(self, uuid, expected_status): """ @@ -334,3 +335,126 @@ class CreditProviderViewTests(UrlResetMixin, TestCase): """ request = CreditRequest.objects.get(uuid=uuid) self.assertEqual(request.status, expected_status) + + +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') +class CreditCourseViewSetTests(TestCase): + """ Tests for the CreditCourse endpoints. + + GET/POST /api/v1/credit/creditcourse/ + GET/PUT /api/v1/credit/creditcourse/:course_id/ + """ + password = 'password' + + def setUp(self): + super(CreditCourseViewSetTests, self).setUp() + + # This value must be set here, as setting it outside of a method results in issues with CMS/Studio tests. + self.path = reverse('credit:creditcourse-list') + + # Create a user and login, so that we can use session auth for the + # tests that aren't specifically testing authentication or authorization. + user = UserFactory(password=self.password, is_staff=True) + self.client.login(username=user.username, password=self.password) + + def _serialize_credit_course(self, credit_course): + """ Serializes a CreditCourse to a Python dict. """ + + return { + 'course_key': unicode(credit_course.course_key), + 'enabled': credit_course.enabled + } + + def test_session_auth(self): + """ Verify the endpoint supports session authentication, and only allows authorization for staff users. """ + user = UserFactory(password=self.password, is_staff=False) + self.client.login(username=user.username, password=self.password) + + # Non-staff users should not have access to the API + response = self.client.get(self.path) + self.assertEqual(response.status_code, 403) + + # Staff users should have access to the API + user.is_staff = True + user.save() # pylint: disable=no-member + response = self.client.get(self.path) + self.assertEqual(response.status_code, 200) + + def test_oauth(self): + """ Verify the endpoint supports OAuth, and only allows authorization for staff users. """ + user = UserFactory(is_staff=False) + oauth_client = ClientFactory.create() + access_token = AccessTokenFactory.create(user=user, client=oauth_client).token + headers = { + 'HTTP_AUTHORIZATION': 'Bearer ' + access_token + } + + # Non-staff users should not have access to the API + response = self.client.get(self.path, **headers) + self.assertEqual(response.status_code, 403) + + # Staff users should have access to the API + user.is_staff = True + user.save() # pylint: disable=no-member + response = self.client.get(self.path, **headers) + self.assertEqual(response.status_code, 200) + + def test_create(self): + """ Verify the endpoint supports creating new CreditCourse objects. """ + course_key = CourseKey.from_string('a/b/c') + enabled = True + data = { + 'course_key': unicode(course_key), + 'enabled': enabled + } + + response = self.client.post(self.path, data=json.dumps(data), content_type=JSON) + self.assertEqual(response.status_code, 201) + + # Verify the API returns the serialized CreditCourse + self.assertDictEqual(json.loads(response.content), data) + + # Verify the CreditCourse was actually created + self.assertTrue(CreditCourse.objects.filter(course_key=course_key, enabled=enabled).exists()) + + def test_get(self): + """ Verify the endpoint supports retrieving CreditCourse objects. """ + course_id = 'a/b/c' + cc1 = CreditCourse.objects.create(course_key=CourseKey.from_string(course_id)) + path = reverse('credit:creditcourse-detail', args=[course_id]) + + response = self.client.get(path) + self.assertEqual(response.status_code, 200) + + # Verify the API returns the serialized CreditCourse + self.assertDictEqual(json.loads(response.content), self._serialize_credit_course(cc1)) + + def test_list(self): + """ Verify the endpoint supports listing all CreditCourse objects. """ + cc1 = CreditCourse.objects.create(course_key=CourseKey.from_string('a/b/c')) + cc2 = CreditCourse.objects.create(course_key=CourseKey.from_string('d/e/f'), enabled=True) + expected = [self._serialize_credit_course(cc1), self._serialize_credit_course(cc2)] + + response = self.client.get(self.path) + self.assertEqual(response.status_code, 200) + + # Verify the API returns a list of serialized CreditCourse objects + self.assertListEqual(json.loads(response.content), expected) + + def test_update(self): + """ Verify the endpoint supports updating a CreditCourse object. """ + course_id = 'course-v1:edX+BlendedX+1T2015' + credit_course = CreditCourse.objects.create(course_key=CourseKey.from_string(course_id), enabled=False) + self.assertFalse(credit_course.enabled) + + path = reverse('credit:creditcourse-detail', args=[course_id]) + data = {'course_key': course_id, 'enabled': True} + response = self.client.put(path, json.dumps(data), content_type=JSON) + self.assertEqual(response.status_code, 200) + + # Verify the serialized CreditCourse is returned + self.assertDictEqual(json.loads(response.content), data) + + # Verify the data was persisted + credit_course = CreditCourse.objects.get(course_key=credit_course.course_key) + self.assertTrue(credit_course.enabled) diff --git a/openedx/core/djangoapps/credit/urls.py b/openedx/core/djangoapps/credit/urls.py index 01efc405fe..a5e92ffc88 100644 --- a/openedx/core/djangoapps/credit/urls.py +++ b/openedx/core/djangoapps/credit/urls.py @@ -1,43 +1,52 @@ """ URLs for the credit app. """ -from django.conf.urls import patterns, url +from django.conf.urls import patterns, url, include -from .api.provider import get_credit_provider_info -from .views import create_credit_request, credit_provider_callback, get_providers_detail, get_eligibility_for_user +from openedx.core.djangoapps.credit import views, routers +from openedx.core.djangoapps.credit.api.provider import get_credit_provider_info PROVIDER_ID_PATTERN = r'(?P[^/]+)' -urlpatterns = patterns( +V1_URLS = patterns( '', url( - r"^v1/providers/(?P[^/]+)/$", + r'^providers/$', + views.get_providers_detail, + name='providers_detail' + ), + + url( + r'^providers/{provider_id}/$'.format(provider_id=PROVIDER_ID_PATTERN), get_credit_provider_info, - name="get_provider_info" - ), - url( - r"^v1/providers/$", - get_providers_detail, - name="providers_detail" + name='get_provider_info' ), url( - r"^v1/providers/{provider_id}/request/$".format(provider_id=PROVIDER_ID_PATTERN), - create_credit_request, - name="create_request" + r'^providers/{provider_id}/request/$'.format(provider_id=PROVIDER_ID_PATTERN), + views.create_credit_request, + name='create_request' ), url( - r"^v1/providers/{provider_id}/callback/?$".format(provider_id=PROVIDER_ID_PATTERN), - credit_provider_callback, - name="provider_callback" + r'^providers/{provider_id}/callback/?$'.format(provider_id=PROVIDER_ID_PATTERN), + views.credit_provider_callback, + name='provider_callback' ), url( - r"^v1/eligibility/$", - get_eligibility_for_user, - name="eligibility_details" + r'^eligibility/$', + views.get_eligibility_for_user, + name='eligibility_details' ), - +) + +router = routers.SimpleRouter() # pylint: disable=invalid-name +router.register(r'courses', views.CreditCourseViewSet) +V1_URLS += router.urls + +urlpatterns = patterns( + '', + url(r'^v1/', include(V1_URLS)), ) diff --git a/openedx/core/djangoapps/credit/views.py b/openedx/core/djangoapps/credit/views.py index 94ef0df283..22a05b6e12 100644 --- a/openedx/core/djangoapps/credit/views.py +++ b/openedx/core/djangoapps/credit/views.py @@ -4,27 +4,28 @@ Views for the credit Django app. import json import datetime import logging -import pytz +from django.conf import settings from django.http import ( HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, Http404 ) -from django.views.decorators.http import require_POST, require_GET from django.views.decorators.csrf import csrf_exempt -from django.conf import settings - -from opaque_keys.edx.keys import CourseKey +from django.views.decorators.http import require_POST, require_GET from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey +import pytz +from rest_framework import viewsets, mixins, permissions, authentication from util.json_request import JsonResponse from util.date_utils import from_timestamp from openedx.core.djangoapps.credit import api -from openedx.core.djangoapps.credit.signature import signature, get_shared_secret_key from openedx.core.djangoapps.credit.exceptions import CreditApiBadRequest, CreditRequestNotFound - +from openedx.core.djangoapps.credit.models import CreditCourse +from openedx.core.djangoapps.credit.serializers import CreditCourseSerializer +from openedx.core.djangoapps.credit.signature import signature, get_shared_secret_key log = logging.getLogger(__name__) @@ -84,7 +85,7 @@ def create_credit_request(request, provider_id): This end-point will get-or-create a record in the database to track the request. It will then calculate the parameters to send to - the credit provider and digitially sign the parameters, using a secret + the credit provider and digitally sign the parameters, using a secret key shared with the credit provider. The user's browser is responsible for POSTing these parameters @@ -366,3 +367,23 @@ def _validate_timestamp(timestamp_value, provider_id): timestamp_value, elapsed_seconds, provider_id, ) return HttpResponseForbidden(u"Timestamp is too far in the past.") + + +class CreditCourseViewSet(mixins.CreateModelMixin, mixins.UpdateModelMixin, viewsets.ReadOnlyModelViewSet): + """ CreditCourse endpoints. """ + + lookup_field = 'course_key' + lookup_value_regex = settings.COURSE_KEY_REGEX + queryset = CreditCourse.objects.all() + serializer_class = CreditCourseSerializer + authentication_classes = (authentication.OAuth2Authentication, authentication.SessionAuthentication,) + permission_classes = (permissions.IsAuthenticated, permissions.IsAdminUser) + + def dispatch(self, request, *args, **kwargs): + # Convert the course ID/key from a string to an actual CourseKey object. + course_id = kwargs.get(self.lookup_field, None) + + if course_id: + kwargs[self.lookup_field] = CourseKey.from_string(course_id) + + return super(CreditCourseViewSet, self).dispatch(request, *args, **kwargs)