Commit e57f3e3b by Nimisha Asthagiri

Merge pull request #6572 from edx/mobile/fix-decorator-api

Fix Mobile API decorators
parents eaa4f7c0 9566a46a
...@@ -9,10 +9,10 @@ from courseware.courses import get_course_about_section, get_course_info_section ...@@ -9,10 +9,10 @@ from courseware.courses import get_course_about_section, get_course_info_section
from static_replace import make_static_urls_absolute, replace_static_urls from static_replace import make_static_urls_absolute, replace_static_urls
from xmodule_modifiers import get_course_update_items from xmodule_modifiers import get_course_update_items
from ..utils import MobileView, mobile_course_access from ..utils import mobile_view, mobile_course_access
@MobileView() @mobile_view()
class CourseUpdatesList(generics.ListAPIView): class CourseUpdatesList(generics.ListAPIView):
""" """
**Use Case** **Use Case**
...@@ -57,7 +57,7 @@ class CourseUpdatesList(generics.ListAPIView): ...@@ -57,7 +57,7 @@ class CourseUpdatesList(generics.ListAPIView):
return Response(updates_to_show) return Response(updates_to_show)
@MobileView() @mobile_view()
class CourseHandoutsList(generics.ListAPIView): class CourseHandoutsList(generics.ListAPIView):
""" """
**Use Case** **Use Case**
...@@ -89,7 +89,7 @@ class CourseHandoutsList(generics.ListAPIView): ...@@ -89,7 +89,7 @@ class CourseHandoutsList(generics.ListAPIView):
raise Http404(u"No handouts for {}".format(unicode(course.id))) raise Http404(u"No handouts for {}".format(unicode(course.id)))
@MobileView() @mobile_view()
class CourseAboutDetail(generics.RetrieveAPIView): class CourseAboutDetail(generics.RetrieveAPIView):
""" """
**Use Case** **Use Case**
......
...@@ -4,17 +4,18 @@ Tests for mobile API utilities. ...@@ -4,17 +4,18 @@ Tests for mobile API utilities.
import ddt import ddt
from mock import patch from mock import patch
from django.test import TestCase
from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.factories import CourseFactory
from .utils import mobile_access_when_enrolled from .utils import mobile_access_when_enrolled, mobile_course_access, mobile_view
from .testutils import MobileAPITestCase, ROLE_CASES from .testutils import MobileAPITestCase, ROLE_CASES
@ddt.ddt @ddt.ddt
class TestMobileApiUtils(MobileAPITestCase): class TestMobileAccessWhenEnrolled(MobileAPITestCase):
""" """
Tests for mobile API utilities Tests for mobile_access_when_enrolled utility function.
""" """
@ddt.data(*ROLE_CASES) @ddt.data(*ROLE_CASES)
@ddt.unpack @ddt.unpack
...@@ -45,3 +46,22 @@ class TestMobileApiUtils(MobileAPITestCase): ...@@ -45,3 +46,22 @@ class TestMobileApiUtils(MobileAPITestCase):
Verifies that we handle the case where a course hasn't started Verifies that we handle the case where a course hasn't started
""" """
self.assertFalse(mobile_access_when_enrolled(self.course, self.user)) self.assertFalse(mobile_access_when_enrolled(self.course, self.user))
@ddt.ddt
class TestMobileAPIDecorators(TestCase):
"""
Basic tests for mobile api decorators to ensure they retain the docstrings.
"""
@ddt.data(mobile_view, mobile_course_access)
def test_function_decorator(self, decorator):
@decorator()
def decorated_func():
"""
Test docstring of decorated function.
"""
pass
self.assertIn("Test docstring of decorated function.", decorated_func.__doc__)
self.assertEquals(decorated_func.__name__, "decorated_func")
self.assertTrue(decorated_func.__module__.endswith("tests"))
...@@ -5,8 +5,8 @@ Test utilities for mobile API tests: ...@@ -5,8 +5,8 @@ Test utilities for mobile API tests:
No tests are implemented in this base class. No tests are implemented in this base class.
Test Mixins to be included by concrete test classes and provide implementation of common test methods: Test Mixins to be included by concrete test classes and provide implementation of common test methods:
MobileAuthTestMixin - tests for APIs with MobileView/mobile_view and is_user=False. MobileAuthTestMixin - tests for APIs with mobile_view and is_user=False.
MobileAuthUserTestMixin - tests for APIs with MobileView/mobile_view and is_user=True. MobileAuthUserTestMixin - tests for APIs with mobile_view and is_user=True.
MobileCourseAccessTestMixin - tests for APIs with mobile_course_access and verify_enrolled=False. MobileCourseAccessTestMixin - tests for APIs with mobile_course_access and verify_enrolled=False.
MobileEnrolledCourseAccessTestMixin - tests for APIs with mobile_course_access and verify_enrolled=True. MobileEnrolledCourseAccessTestMixin - tests for APIs with mobile_course_access and verify_enrolled=True.
""" """
...@@ -101,7 +101,7 @@ class MobileAPITestCase(ModuleStoreTestCase, APITestCase): ...@@ -101,7 +101,7 @@ class MobileAPITestCase(ModuleStoreTestCase, APITestCase):
class MobileAuthTestMixin(object): class MobileAuthTestMixin(object):
""" """
Test Mixin for testing APIs decorated with MobileView or mobile_view. Test Mixin for testing APIs decorated with mobile_view.
""" """
def test_no_auth(self): def test_no_auth(self):
self.logout() self.logout()
...@@ -110,7 +110,7 @@ class MobileAuthTestMixin(object): ...@@ -110,7 +110,7 @@ class MobileAuthTestMixin(object):
class MobileAuthUserTestMixin(MobileAuthTestMixin): class MobileAuthUserTestMixin(MobileAuthTestMixin):
""" """
Test Mixin for testing APIs related to users: mobile_view or MobileView with is_user=True. Test Mixin for testing APIs related to users: mobile_view with is_user=True.
""" """
def test_invalid_user(self): def test_invalid_user(self):
self.login_and_enroll() self.login_and_enroll()
......
...@@ -26,10 +26,10 @@ from xmodule.modulestore.exceptions import ItemNotFoundError ...@@ -26,10 +26,10 @@ from xmodule.modulestore.exceptions import ItemNotFoundError
from .serializers import CourseEnrollmentSerializer, UserSerializer from .serializers import CourseEnrollmentSerializer, UserSerializer
from mobile_api import errors from mobile_api import errors
from mobile_api.utils import mobile_access_when_enrolled, mobile_view, MobileView, mobile_course_access from mobile_api.utils import mobile_access_when_enrolled, mobile_view, mobile_course_access
@MobileView(is_user=True) @mobile_view(is_user=True)
class UserDetail(generics.RetrieveAPIView): class UserDetail(generics.RetrieveAPIView):
""" """
**Use Case** **Use Case**
...@@ -67,7 +67,7 @@ class UserDetail(generics.RetrieveAPIView): ...@@ -67,7 +67,7 @@ class UserDetail(generics.RetrieveAPIView):
lookup_field = 'username' lookup_field = 'username'
@MobileView(is_user=True) @mobile_view(is_user=True)
class UserCourseStatus(views.APIView): class UserCourseStatus(views.APIView):
""" """
Endpoints for getting and setting meta data Endpoints for getting and setting meta data
...@@ -202,7 +202,7 @@ class UserCourseStatus(views.APIView): ...@@ -202,7 +202,7 @@ class UserCourseStatus(views.APIView):
return self._get_course_info(request, course) return self._get_course_info(request, course)
@MobileView(is_user=True) @mobile_view(is_user=True)
class UserCourseEnrollmentsList(generics.ListAPIView): class UserCourseEnrollmentsList(generics.ListAPIView):
""" """
**Use Case** **Use Case**
......
...@@ -55,7 +55,7 @@ def mobile_access_when_enrolled(course, user): ...@@ -55,7 +55,7 @@ def mobile_access_when_enrolled(course, user):
def mobile_view(is_user=False): def mobile_view(is_user=False):
""" """
Function decorator that abstracts the authentication and permission checks for mobile api views. Function and class decorator that abstracts the authentication and permission checks for mobile api views.
""" """
class IsUser(permissions.BasePermission): class IsUser(permissions.BasePermission):
""" """
...@@ -64,28 +64,14 @@ def mobile_view(is_user=False): ...@@ -64,28 +64,14 @@ def mobile_view(is_user=False):
def has_permission(self, request, view): def has_permission(self, request, view):
return request.user.username == request.parser_context.get('kwargs', {}).get('username', None) return request.user.username == request.parser_context.get('kwargs', {}).get('username', None)
def _decorator(func): def _decorator(func_or_class):
""" """
Requires either OAuth2 or Session-based authentication. Requires either OAuth2 or Session-based authentication.
If is_user is True, also requires username in URL matches the request user. If is_user is True, also requires username in URL matches the request user.
""" """
func.authentication_classes = (OAuth2Authentication, SessionAuthentication) func_or_class.authentication_classes = (OAuth2Authentication, SessionAuthentication)
func.permission_classes = (permissions.IsAuthenticated,) func_or_class.permission_classes = (permissions.IsAuthenticated,)
if is_user: if is_user:
func.permission_classes += (IsUser,) func_or_class.permission_classes += (IsUser,)
return func return func_or_class
return _decorator return _decorator
class MobileView(object):
"""
Class decorator that abstracts the authentication and permission checks for mobile api views.
"""
def __init__(self, is_user=False):
self.is_user = is_user
def __call__(self, cls):
class _Decorator(cls):
"""Inner decorator class to wrap the given class."""
mobile_view(self.is_user)(cls)
return _Decorator
...@@ -17,11 +17,11 @@ from opaque_keys.edx.locator import BlockUsageLocator ...@@ -17,11 +17,11 @@ from opaque_keys.edx.locator import BlockUsageLocator
from xmodule.exceptions import NotFoundError from xmodule.exceptions import NotFoundError
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from ..utils import MobileView, mobile_course_access from ..utils import mobile_view, mobile_course_access
from .serializers import BlockOutline, video_summary from .serializers import BlockOutline, video_summary
@MobileView() @mobile_view()
class VideoSummaryList(generics.ListAPIView): class VideoSummaryList(generics.ListAPIView):
""" """
**Use Case** **Use Case**
...@@ -89,7 +89,7 @@ class VideoSummaryList(generics.ListAPIView): ...@@ -89,7 +89,7 @@ class VideoSummaryList(generics.ListAPIView):
return Response(video_outline) return Response(video_outline)
@MobileView() @mobile_view()
class VideoTranscripts(generics.RetrieveAPIView): class VideoTranscripts(generics.RetrieveAPIView):
""" """
**Use Case** **Use Case**
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment