From 45de93a250a271df9ecdcadfbd941675c0e35efd Mon Sep 17 00:00:00 2001 From: Clinton Blackburn Date: Tue, 23 Jun 2015 17:30:10 -0400 Subject: [PATCH] Added course endpoints for Commerce API XCOM-314 --- lms/djangoapps/commerce/api/__init__.py | 0 lms/djangoapps/commerce/api/urls.py | 7 + lms/djangoapps/commerce/api/v1/__init__.py | 0 lms/djangoapps/commerce/api/v1/models.py | 82 ++++++++ lms/djangoapps/commerce/api/v1/permissions.py | 12 ++ lms/djangoapps/commerce/api/v1/serializers.py | 34 ++++ .../commerce/api/v1/tests/__init__.py | 1 + .../commerce/api/v1/tests/test_views.py | 177 ++++++++++++++++++ lms/djangoapps/commerce/api/v1/urls.py | 17 ++ lms/djangoapps/commerce/api/v1/views.py | 43 +++++ lms/djangoapps/commerce/tests/test_views.py | 13 -- lms/djangoapps/commerce/urls.py | 5 +- 12 files changed, 375 insertions(+), 16 deletions(-) create mode 100644 lms/djangoapps/commerce/api/__init__.py create mode 100644 lms/djangoapps/commerce/api/urls.py create mode 100644 lms/djangoapps/commerce/api/v1/__init__.py create mode 100644 lms/djangoapps/commerce/api/v1/models.py create mode 100644 lms/djangoapps/commerce/api/v1/permissions.py create mode 100644 lms/djangoapps/commerce/api/v1/serializers.py create mode 100644 lms/djangoapps/commerce/api/v1/tests/__init__.py create mode 100644 lms/djangoapps/commerce/api/v1/tests/test_views.py create mode 100644 lms/djangoapps/commerce/api/v1/urls.py create mode 100644 lms/djangoapps/commerce/api/v1/views.py diff --git a/lms/djangoapps/commerce/api/__init__.py b/lms/djangoapps/commerce/api/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/commerce/api/urls.py b/lms/djangoapps/commerce/api/urls.py new file mode 100644 index 0000000000..6e216cb041 --- /dev/null +++ b/lms/djangoapps/commerce/api/urls.py @@ -0,0 +1,7 @@ +""" API URLs. """ +from django.conf.urls import patterns, url, include + +urlpatterns = patterns( + '', + url(r'^v1/', include('commerce.api.v1.urls', namespace='v1')), +) diff --git a/lms/djangoapps/commerce/api/v1/__init__.py b/lms/djangoapps/commerce/api/v1/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/commerce/api/v1/models.py b/lms/djangoapps/commerce/api/v1/models.py new file mode 100644 index 0000000000..235b1385d7 --- /dev/null +++ b/lms/djangoapps/commerce/api/v1/models.py @@ -0,0 +1,82 @@ +""" API v1 models. """ +from itertools import groupby +import logging + +from django.db import transaction +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey + +from course_modes.models import CourseMode + +log = logging.getLogger(__name__) + + +class Course(object): + """ Pseudo-course model used to group CourseMode objects. """ + id = None # pylint: disable=invalid-name + modes = None + _deleted_modes = None + + def __init__(self, id, modes): # pylint: disable=invalid-name,redefined-builtin + self.id = CourseKey.from_string(unicode(id)) # pylint: disable=invalid-name + self.modes = list(modes) + self._deleted_modes = [] + + @transaction.commit_on_success + def save(self, *args, **kwargs): # pylint: disable=unused-argument + """ Save the CourseMode objects to the database. """ + for mode in self.modes: + mode.course_id = self.id + mode.mode_display_name = mode.mode_slug + mode.save() + + deleted_mode_ids = [mode.id for mode in self._deleted_modes] + CourseMode.objects.filter(id__in=deleted_mode_ids).delete() + self._deleted_modes = [] + + def update(self, attrs): + """ Update the model with external data (usually passed via API call). """ + existing_modes = {mode.mode_slug: mode for mode in self.modes} + merged_modes = set() + merged_mode_keys = set() + + for posted_mode in attrs.get('modes', []): + merged_mode = existing_modes.get(posted_mode.mode_slug, CourseMode()) + + merged_mode.course_id = self.id + merged_mode.mode_slug = posted_mode.mode_slug + merged_mode.mode_display_name = posted_mode.mode_slug + merged_mode.min_price = posted_mode.min_price + merged_mode.currency = posted_mode.currency + merged_mode.sku = posted_mode.sku + + merged_modes.add(merged_mode) + merged_mode_keys.add(merged_mode.mode_slug) + + deleted_modes = set(existing_modes.keys()) - merged_mode_keys + self._deleted_modes = [existing_modes[mode] for mode in deleted_modes] + self.modes = list(merged_modes) + + @classmethod + def get(cls, course_id): + """ Retrieve a single course. """ + try: + course_id = CourseKey.from_string(unicode(course_id)) + except InvalidKeyError: + log.debug('[%s] is not a valid course key.', course_id) + raise ValueError + + course_modes = CourseMode.objects.filter(course_id=course_id) + + if course_modes: + return cls(unicode(course_id), list(course_modes)) + + return None + + @classmethod + def iterator(cls): + """ Generator that yields all courses. """ + course_modes = CourseMode.objects.order_by('course_id') + + for course_id, modes in groupby(course_modes, lambda o: o.course_id): + yield cls(course_id, list(modes)) diff --git a/lms/djangoapps/commerce/api/v1/permissions.py b/lms/djangoapps/commerce/api/v1/permissions.py new file mode 100644 index 0000000000..4c7679285a --- /dev/null +++ b/lms/djangoapps/commerce/api/v1/permissions.py @@ -0,0 +1,12 @@ +""" Custom API permissions. """ +from rest_framework.permissions import BasePermission, DjangoModelPermissions + +from openedx.core.lib.api.permissions import ApiKeyHeaderPermission + + +class ApiKeyOrModelPermission(BasePermission): + """ Access granted for requests with API key in header, + or made by user with appropriate Django model permissions. """ + def has_permission(self, request, view): + return ApiKeyHeaderPermission().has_permission(request, view) or DjangoModelPermissions().has_permission( + request, view) diff --git a/lms/djangoapps/commerce/api/v1/serializers.py b/lms/djangoapps/commerce/api/v1/serializers.py new file mode 100644 index 0000000000..227bd5c727 --- /dev/null +++ b/lms/djangoapps/commerce/api/v1/serializers.py @@ -0,0 +1,34 @@ +""" API v1 serializers. """ +from rest_framework import serializers + +from commerce.api.v1.models import Course +from course_modes.models import CourseMode + + +class CourseModeSerializer(serializers.ModelSerializer): + """ CourseMode serializer. """ + name = serializers.CharField(source='mode_slug') + price = serializers.IntegerField(source='min_price') + + def get_identity(self, data): + try: + return data.get('name', None) + except AttributeError: + return None + + class Meta(object): # pylint: disable=missing-docstring + model = CourseMode + fields = ('name', 'currency', 'price', 'sku') + + +class CourseSerializer(serializers.Serializer): + """ Course serializer. """ + id = serializers.CharField() # pylint: disable=invalid-name + modes = CourseModeSerializer(many=True, allow_add_remove=True) + + def restore_object(self, attrs, instance=None): + if instance is None: + return Course(attrs['id'], attrs['modes']) + + instance.update(attrs) + return instance diff --git a/lms/djangoapps/commerce/api/v1/tests/__init__.py b/lms/djangoapps/commerce/api/v1/tests/__init__.py new file mode 100644 index 0000000000..0d7de6089c --- /dev/null +++ b/lms/djangoapps/commerce/api/v1/tests/__init__.py @@ -0,0 +1 @@ +""" Commerce API v1 tests. """ diff --git a/lms/djangoapps/commerce/api/v1/tests/test_views.py b/lms/djangoapps/commerce/api/v1/tests/test_views.py new file mode 100644 index 0000000000..97103ca76a --- /dev/null +++ b/lms/djangoapps/commerce/api/v1/tests/test_views.py @@ -0,0 +1,177 @@ +""" Commerce API v1 view tests. """ +import json + +import ddt +from django.conf import settings +from django.contrib.auth.models import Permission +from django.core.urlresolvers import reverse +from django.test.utils import override_settings +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +from course_modes.models import CourseMode +from student.tests.factories import UserFactory + +PASSWORD = 'test' +JSON_CONTENT_TYPE = 'application/json' + + +class CourseApiViewTestMixin(object): + """ Mixin for CourseApi views. + + Automatically creates a course and CourseMode. + """ + + def setUp(self): + super(CourseApiViewTestMixin, self).setUp() + self.course = CourseFactory.create() + self.course_mode = CourseMode.objects.create(course_id=self.course.id, mode_slug=u'verified', min_price=100, + currency=u'USD', sku=u'ABC123') + + @staticmethod + def _serialize_course_mode(course_mode): + """ Serialize a CourseMode to a dict. """ + return { + u'name': course_mode.mode_slug, + u'currency': course_mode.currency, + u'price': course_mode.min_price, + u'sku': course_mode.sku + } + + +class CourseListViewTests(CourseApiViewTestMixin, ModuleStoreTestCase): + """ Tests for CourseListView. """ + path = reverse('commerce:api:v1:courses:list') + + def test_authentication_required(self): + """ Verify only authenticated users can access the view. """ + self.client.logout() + response = self.client.get(self.path, content_type=JSON_CONTENT_TYPE) + self.assertEqual(response.status_code, 401) + + def test_list(self): + """ Verify the view lists the available courses and modes. """ + user = UserFactory.create() + self.client.login(username=user.username, password=PASSWORD) + response = self.client.get(self.path, content_type=JSON_CONTENT_TYPE) + + self.assertEqual(response.status_code, 200) + actual = json.loads(response.content) + expected = [ + { + u'id': unicode(self.course.id), + u'modes': [self._serialize_course_mode(self.course_mode)] + } + ] + self.assertListEqual(actual, expected) + + +@ddt.ddt +class CourseRetrieveUpdateViewTests(CourseApiViewTestMixin, ModuleStoreTestCase): + """ Tests for CourseRetrieveUpdateView. """ + + def setUp(self): + super(CourseRetrieveUpdateViewTests, self).setUp() + self.path = reverse('commerce:api:v1:courses:retrieve_update', args=[unicode(self.course.id)]) + self.user = UserFactory.create() + self.client.login(username=self.user.username, password=PASSWORD) + + @ddt.data('get', 'post', 'put') + def test_authentication_required(self, method): + """ Verify only authenticated users can access the view. """ + self.client.logout() + response = getattr(self.client, method)(self.path, content_type=JSON_CONTENT_TYPE) + self.assertEqual(response.status_code, 401) + + @ddt.data('post', 'put') + def test_authorization_required(self, method): + """ Verify create/edit operations require appropriate permissions. """ + response = getattr(self.client, method)(self.path, content_type=JSON_CONTENT_TYPE) + self.assertEqual(response.status_code, 403) + + def test_retrieve(self): + """ Verify the view displays info for a given course. """ + response = self.client.get(self.path, content_type=JSON_CONTENT_TYPE) + self.assertEqual(response.status_code, 200) + + actual = json.loads(response.content) + expected = { + u'id': unicode(self.course.id), + u'modes': [self._serialize_course_mode(self.course_mode)] + } + self.assertEqual(actual, expected) + + def test_retrieve_invalid_course(self): + """ The view should return HTTP 404 when retrieving data for a course that does not exist. """ + path = reverse('commerce:api:v1:courses:retrieve_update', args=['a/b/c']) + response = self.client.get(path, content_type=JSON_CONTENT_TYPE) + self.assertEqual(response.status_code, 404) + + def test_update(self): + """ Verify the view supports updating a course. """ + permission = Permission.objects.get(name='Can change course mode') + self.user.user_permissions.add(permission) + expected_course_mode = CourseMode(mode_slug=u'verified', min_price=200, currency=u'USD', sku=u'ABC123') + expected = { + u'id': unicode(self.course.id), + u'modes': [self._serialize_course_mode(expected_course_mode)] + } + response = self.client.put(self.path, json.dumps(expected), content_type=JSON_CONTENT_TYPE) + self.assertEqual(response.status_code, 200) + + actual = json.loads(response.content) + self.assertEqual(actual, expected) + + def test_update_overwrite(self): + """ Verify that data submitted via PUT overwrites/deletes modes that are + not included in the body of the request. """ + permission = Permission.objects.get(name='Can change course mode') + self.user.user_permissions.add(permission) + + course_id = unicode(self.course.id) + expected = { + u'id': course_id, + u'modes': [self._serialize_course_mode( + CourseMode(mode_slug=u'credit', min_price=500, currency=u'USD', sku=u'ABC123')), ] + } + path = reverse('commerce:api:v1:courses:retrieve_update', args=[course_id]) + response = self.client.put(path, json.dumps(expected), content_type=JSON_CONTENT_TYPE) + self.assertEqual(response.status_code, 200) + actual = json.loads(response.content) + self.assertEqual(actual, expected) + + # The existing CourseMode should have been removed. + self.assertFalse(CourseMode.objects.filter(id=self.course_mode.id).exists()) + + def assert_can_create_course(self, **request_kwargs): + """ Verify a course can be created by the view. """ + course = CourseFactory.create() + course_id = unicode(course.id) + expected = { + u'id': course_id, + u'modes': [ + self._serialize_course_mode( + CourseMode(mode_slug=u'verified', min_price=150, currency=u'USD', sku=u'ABC123')), + self._serialize_course_mode( + CourseMode(mode_slug=u'honor', min_price=0, currency=u'USD', sku=u'DEADBEEF')), + ] + } + path = reverse('commerce:api:v1:courses:retrieve_update', args=[course_id]) + response = self.client.put(path, json.dumps(expected), content_type=JSON_CONTENT_TYPE, **request_kwargs) + self.assertEqual(response.status_code, 201) + actual = json.loads(response.content) + self.assertEqual(actual, expected) + + def test_create_with_permissions(self): + """ Verify the view supports creating a course as a user with the appropriate permissions. """ + permissions = Permission.objects.filter(name__in=('Can add course mode', 'Can change course mode')) + for permission in permissions: + self.user.user_permissions.add(permission) + + self.assert_can_create_course() + + @override_settings(EDX_API_KEY='edx') + def test_create_with_api_key(self): + """ Verify the view supports creating a course when authenticated with the API header key. """ + self.client.logout() + self.assert_can_create_course(HTTP_X_EDX_API_KEY=settings.EDX_API_KEY) diff --git a/lms/djangoapps/commerce/api/v1/urls.py b/lms/djangoapps/commerce/api/v1/urls.py new file mode 100644 index 0000000000..3fa0ea40d8 --- /dev/null +++ b/lms/djangoapps/commerce/api/v1/urls.py @@ -0,0 +1,17 @@ +""" API v1 URLs. """ +from django.conf import settings +from django.conf.urls import patterns, url, include + +from commerce.api.v1 import views + +COURSE_URLS = patterns( + '', + url(r'^$', views.CourseListView.as_view(), name='list'), + url(r'^{}/$'.format(settings.COURSE_ID_PATTERN), views.CourseRetrieveUpdateView.as_view(), name='retrieve_update'), +) + +urlpatterns = patterns( + '', + url(r'^courses/', include(COURSE_URLS, namespace='courses')), + +) diff --git a/lms/djangoapps/commerce/api/v1/views.py b/lms/djangoapps/commerce/api/v1/views.py new file mode 100644 index 0000000000..ad72561bed --- /dev/null +++ b/lms/djangoapps/commerce/api/v1/views.py @@ -0,0 +1,43 @@ +""" API v1 views. """ +import logging + +from django.http import Http404 +from rest_framework.authentication import OAuth2Authentication, SessionAuthentication +from rest_framework.generics import RetrieveUpdateAPIView, ListAPIView +from rest_framework.permissions import IsAuthenticated + +from commerce.api.v1.models import Course +from commerce.api.v1.permissions import ApiKeyOrModelPermission +from commerce.api.v1.serializers import CourseSerializer +from course_modes.models import CourseMode + +log = logging.getLogger(__name__) + + +class CourseListView(ListAPIView): + """ List courses and modes. """ + authentication_classes = (OAuth2Authentication, SessionAuthentication,) + permission_classes = (IsAuthenticated,) + serializer_class = CourseSerializer + + def get_queryset(self): + return Course.iterator() + + +class CourseRetrieveUpdateView(RetrieveUpdateAPIView): + """ Retrieve, update, or create courses/modes. """ + lookup_field = 'id' + lookup_url_kwarg = 'course_id' + model = CourseMode + authentication_classes = (OAuth2Authentication, SessionAuthentication,) + permission_classes = (ApiKeyOrModelPermission,) + serializer_class = CourseSerializer + + def get_object(self, queryset=None): + course_id = self.kwargs.get(self.lookup_url_kwarg) + course = Course.get(course_id) + + if course: + return course + + raise Http404 diff --git a/lms/djangoapps/commerce/tests/test_views.py b/lms/djangoapps/commerce/tests/test_views.py index 4a5cbc5a47..a73f51f144 100644 --- a/lms/djangoapps/commerce/tests/test_views.py +++ b/lms/djangoapps/commerce/tests/test_views.py @@ -306,19 +306,6 @@ class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase) self._test_successful_ecommerce_api_call(False) -class OrdersViewTests(BasketsViewTests): - """ - Ensures that /orders/ points to and behaves like /baskets/, for backward - compatibility with stale js clients during updates. - - (XCOM-214) remove after release. - """ - - def setUp(self): - super(OrdersViewTests, self).setUp() - self.url = reverse('commerce:orders') - - @attr('shard_1') @override_settings(ECOMMERCE_API_URL=TEST_API_URL, ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY) class BasketOrderViewTests(UserMixin, TestCase): diff --git a/lms/djangoapps/commerce/urls.py b/lms/djangoapps/commerce/urls.py index 4046bf7530..6f0efe4776 100644 --- a/lms/djangoapps/commerce/urls.py +++ b/lms/djangoapps/commerce/urls.py @@ -2,17 +2,16 @@ Defines the URL routes for this app. """ -from django.conf.urls import patterns, url +from django.conf.urls import patterns, url, include from commerce import views BASKET_ID_PATTERN = r'(?P[\w]+)' urlpatterns = patterns( '', - # (XCOM-214) For backwards compatibility with js clients during intial release - url(r'^orders/$', views.BasketsView.as_view(), name="orders"), url(r'^baskets/$', views.BasketsView.as_view(), name="baskets"), url(r'^baskets/{}/order/$'.format(BASKET_ID_PATTERN), views.BasketOrderView.as_view(), name="basket_order"), url(r'^checkout/cancel/$', views.checkout_cancel, name="checkout_cancel"), url(r'^checkout/receipt/$', views.checkout_receipt, name="checkout_receipt"), + url(r'^api/', include('commerce.api.urls', namespace='api')) )