Move generic mobile API view decorators.

lms/djangoapps/courseware/access.py

@@ -124,7 +124,6 @@ def _has_access_course_desc(user, action, course):
     'load' -- load the courseware, see inside the course
     'load_forum' -- can load and contribute to the forums (one access level for now)
     'load_mobile' -- can load from a mobile context
-    'load_mobile_no_enrollment_check' -- can load from a mobile context without checking for enrollment
     'enroll' -- enroll.  Checks for enrollment window,
                   ACCESS_REQUIRE_STAFF_FOR_COURSE,
     'see_exists' -- can see that the course exists.
@@ -158,29 +157,19 @@ def _has_access_course_desc(user, action, course):
         Can this user access this course from a mobile device?
         """
         return (
-            # check mobile requirements
-            can_load_mobile_no_enroll_check() and
-            # check enrollment
-            (
-                CourseEnrollment.is_enrolled(user, course.id) or
-                _has_staff_access_to_descriptor(user, course, course.id)
-            )
-        )
-
-    def can_load_mobile_no_enroll_check():
-        """
-        Can this enrolled user access this course from a mobile device?
-        Note: does not check for enrollment since it is assumed the caller has done so.
-        """
-        return (
             # check start date
             can_load() and
             # check mobile_available flag
             is_mobile_available_for_user(user, course) and
-            # check staff access, if not then check for unfulfilled milestones
             (
+                # either is a staff user or
                 _has_staff_access_to_descriptor(user, course, course.id) or
-                not any_unfulfilled_milestones(course.id, user.id)
+                (
+                    # check enrollment
+                    CourseEnrollment.is_enrolled(user, course.id) and
+                    # check for unfulfilled milestones
+                    not any_unfulfilled_milestones(course.id, user.id)
+                )
             )
         )
 
@@ -307,7 +296,6 @@ def _has_access_course_desc(user, action, course):
         'view_courseware_with_prerequisites': can_view_courseware_with_prerequisites,
         'load_forum': can_load_forum,
         'load_mobile': can_load_mobile,
-        'load_mobile_no_enrollment_check': can_load_mobile_no_enroll_check,
         'enroll': can_enroll,
         'see_exists': see_exists,
         'staff': lambda: _has_staff_access_to_descriptor(user, course, course.id),

lms/djangoapps/mobile_api/course_info/tests.py

@@ -11,41 +11,12 @@ from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.xml_importer import import_course_from_xml
 
 from ..testutils import (
-    MobileAPITestCase, MobileCourseAccessTestMixin, MobileEnrolledCourseAccessTestMixin, MobileAuthTestMixin
+    MobileAPITestCase, MobileCourseAccessTestMixin, MobileAuthTestMixin
 )
 
 
-class TestAbout(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin):
-    """
-    Tests for /api/mobile/v0.5/course_info/{course_id}/about
-    """
-    REVERSE_INFO = {'name': 'course-about-detail', 'params': ['course_id']}
-
-    def verify_success(self, response):
-        super(TestAbout, self).verify_success(response)
-        self.assertTrue('overview' in response.data)
-
-    def init_course_access(self, course_id=None):
-        # override this method since enrollment is not required for the About endpoint.
-        self.login()
-
-    def test_about_static_rewrite(self):
-        self.login()
-
-        about_usage_key = self.course.id.make_usage_key('about', 'overview')
-        about_module = modulestore().get_item(about_usage_key)
-        underlying_about_html = about_module.data
-
-        # check that we start with relative static assets
-        self.assertIn('\"/static/', underlying_about_html)
-
-        # but shouldn't finish with any
-        response = self.api_response()
-        self.assertNotIn('\"/static/', response.data['overview'])
-
-
 @ddt.ddt
-class TestUpdates(MobileAPITestCase, MobileAuthTestMixin, MobileEnrolledCourseAccessTestMixin):
+class TestUpdates(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin):
     """
     Tests for /api/mobile/v0.5/course_info/{course_id}/updates
     """
@@ -111,7 +82,7 @@ class TestUpdates(MobileAPITestCase, MobileAuthTestMixin, MobileEnrolledCourseAc
             self.assertIn("Update" + str(num), update_data['content'])
 
 
-class TestHandouts(MobileAPITestCase, MobileAuthTestMixin, MobileEnrolledCourseAccessTestMixin):
+class TestHandouts(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin):
     """
     Tests for /api/mobile/v0.5/course_info/{course_id}/handouts
     """

lms/djangoapps/mobile_api/course_info/urls.py

@@ -4,16 +4,11 @@ URLs for course_info API
 from django.conf.urls import patterns, url
 from django.conf import settings
 
-from .views import CourseAboutDetail, CourseUpdatesList, CourseHandoutsList
+from .views import CourseUpdatesList, CourseHandoutsList
 
 urlpatterns = patterns(
     'mobile_api.course_info.views',
     url(
-        r'^{}/about$'.format(settings.COURSE_ID_PATTERN),
-        CourseAboutDetail.as_view(),
-        name='course-about-detail'
-    ),
-    url(
         r'^{}/handouts$'.format(settings.COURSE_ID_PATTERN),
         CourseHandoutsList.as_view(),
         name='course-handouts-list'

lms/djangoapps/mobile_api/course_info/views.py

@@ -87,33 +87,3 @@ class CourseHandoutsList(generics.ListAPIView):
         else:
             # course_handouts_module could be None if there are no handouts
             raise Http404(u"No handouts for {}".format(unicode(course.id)))
-
-
-@mobile_view()
-class CourseAboutDetail(generics.RetrieveAPIView):
-    """
-    **Use Case**
-
-        Get the HTML for the course about page.
-
-    **Example request**:
-
-        GET /api/mobile/v0.5/course_info/{organization}/{course_number}/{course_run}/about
-
-    **Response Values**
-
-        * overview: The HTML for the course About page.
-    """
-
-    @mobile_course_access(verify_enrolled=False)
-    def get(self, request, course, *args, **kwargs):
-        # There are other fields, but they don't seem to be in use.
-        # see courses.py:get_course_about_section.
-        #
-        # This can also return None, so check for that before calling strip()
-        about_section_html = get_course_about_section(course, "overview")
-        about_section_html = make_static_urls_absolute(self.request, about_section_html)
-
-        return Response(
-            {"overview": about_section_html.strip() if about_section_html else ""}
-        )

lms/djangoapps/mobile_api/testutils.py

@@ -7,8 +7,7 @@ Test utilities for mobile API tests:
   Test Mixins to be included by concrete test classes and provide implementation of common test methods:
      MobileAuthTestMixin - tests for APIs with mobile_view and is_user=False.
      MobileAuthUserTestMixin - tests for APIs with mobile_view and is_user=True.
-     MobileCourseAccessTestMixin - tests for APIs with mobile_course_access and verify_enrolled=False.
-     MobileEnrolledCourseAccessTestMixin - tests for APIs with mobile_course_access and verify_enrolled=True.
+     MobileCourseAccessTestMixin - tests for APIs with mobile_course_access.
 """
 # pylint: disable=no-member
 import ddt
@@ -130,7 +129,6 @@ class MobileAuthUserTestMixin(MobileAuthTestMixin):
 class MobileCourseAccessTestMixin(MobileAPIMilestonesMixin):
     """
     Test Mixin for testing APIs marked with mobile_course_access.
-    (Use MobileEnrolledCourseAccessTestMixin when verify_enrolled is set to True.)
     Subclasses are expected to inherit from MobileAPITestCase.
     Subclasses can override verify_success, verify_failure, and init_course_access methods.
     """
@@ -197,11 +195,6 @@ class MobileCourseAccessTestMixin(MobileAPIMilestonesMixin):
         else:
             self.verify_failure(response)
 
-
-class MobileEnrolledCourseAccessTestMixin(MobileCourseAccessTestMixin):
-    """
-    Test Mixin for testing APIs marked with mobile_course_access with verify_enrolled=True.
-    """
     def test_unenrolled_user(self):
         self.login()
         self.unenroll()

lms/djangoapps/mobile_api/users/serializers.py

@@ -31,16 +31,10 @@ class CourseField(serializers.RelatedField):
                 kwargs={'course_id': course_id},
                 request=request
             )
-            course_about_url = reverse(
-                'course-about-detail',
-                kwargs={'course_id': course_id},
-                request=request
-            )
         else:
             video_outline_url = None
             course_updates_url = None
             course_handouts_url = None
-            course_about_url = None
 
         return {
             "id": course_id,
@@ -59,7 +53,6 @@ class CourseField(serializers.RelatedField):
             "video_outline": video_outline_url,
             "course_updates": course_updates_url,
             "course_handouts": course_handouts_url,
-            "course_about": course_about_url,
             "subscription_id": course.clean_id(padding_char='_'),
         }
 

lms/djangoapps/mobile_api/users/tests.py

@@ -10,7 +10,7 @@ from certificates.models import CertificateStatuses
 from certificates.tests.factories import GeneratedCertificateFactory
 
 from .. import errors
-from ..testutils import MobileAPITestCase, MobileAuthTestMixin, MobileAuthUserTestMixin, MobileEnrolledCourseAccessTestMixin
+from ..testutils import MobileAPITestCase, MobileAuthTestMixin, MobileAuthUserTestMixin, MobileCourseAccessTestMixin
 from .serializers import CourseEnrollmentSerializer
 
 
@@ -43,7 +43,7 @@ class TestUserInfoApi(MobileAPITestCase, MobileAuthTestMixin):
         self.assertTrue(self.username in response['location'])
 
 
-class TestUserEnrollmentApi(MobileAPITestCase, MobileAuthUserTestMixin, MobileEnrolledCourseAccessTestMixin):
+class TestUserEnrollmentApi(MobileAPITestCase, MobileAuthUserTestMixin, MobileCourseAccessTestMixin):
     """
     Tests for /api/mobile/v0.5/users/<user_name>/course_enrollments/
     """
@@ -160,7 +160,7 @@ class CourseStatusAPITestCase(MobileAPITestCase):
         )
 
 
-class TestCourseStatusGET(CourseStatusAPITestCase, MobileAuthUserTestMixin, MobileEnrolledCourseAccessTestMixin):
+class TestCourseStatusGET(CourseStatusAPITestCase, MobileAuthUserTestMixin, MobileCourseAccessTestMixin):
     """
     Tests for GET of /api/mobile/v0.5/users/<user_name>/course_status_info/{course_id}
     """
@@ -178,7 +178,7 @@ class TestCourseStatusGET(CourseStatusAPITestCase, MobileAuthUserTestMixin, Mobi
         )
 
 
-class TestCourseStatusPATCH(CourseStatusAPITestCase, MobileAuthUserTestMixin, MobileEnrolledCourseAccessTestMixin):
+class TestCourseStatusPATCH(CourseStatusAPITestCase, MobileAuthUserTestMixin, MobileCourseAccessTestMixin):
     """
     Tests for PATCH of /api/mobile/v0.5/users/<user_name>/course_status_info/{course_id}
     """

lms/djangoapps/mobile_api/users/views.py

@@ -212,7 +212,6 @@ class UserCourseEnrollmentsList(generics.ListAPIView):
           * url: URL to the downloadable version of the certificate, if exists.
         * course: A collection of data about the course:
 
-          * course_about: The URI to get the data for the course About page.
           * course_updates: The URI to get data for course updates.
           * number: The course number.
           * org: The organization that created the course.

lms/djangoapps/mobile_api/utils.py

 """
 Common utility methods and decorators for Mobile APIs.
 """
+from openedx.core.lib.api.view_utils import view_course_access, view_auth_classes
 
-import functools
 
-from django.http import Http404
-
-from rest_framework import permissions, status, response
-
-from opaque_keys.edx.keys import CourseKey
-
-from courseware.courses import get_course_with_access
-from openedx.core.lib.api.permissions import IsUserInUrl
-from openedx.core.lib.api.authentication import (
-    SessionAuthenticationAllowInactiveUser,
-    OAuth2AuthenticationAllowInactiveUser,
-)
-from util.milestones_helpers import any_unfulfilled_milestones
-from xmodule.modulestore.django import modulestore
-
-
-def mobile_course_access(depth=0, verify_enrolled=True):
+def mobile_course_access(depth=0):
     """
     Method decorator for a mobile API endpoint that verifies the user has access to the course in a mobile context.
     """
-    def _decorator(func):
-        """Outer method decorator."""
-        @functools.wraps(func)
-        def _wrapper(self, request, *args, **kwargs):
-            """
-            Expects kwargs to contain 'course_id'.
-            Passes the course descriptor to the given decorated function.
-            Raises 404 if access to course is disallowed.
-            """
-            course_id = CourseKey.from_string(kwargs.pop('course_id'))
-            with modulestore().bulk_operations(course_id):
-                try:
-                    course = get_course_with_access(
-                        request.user,
-                        'load_mobile' if verify_enrolled else 'load_mobile_no_enrollment_check',
-                        course_id,
-                        depth=depth
-                    )
-                except Http404:
-                    # any_unfulfilled_milestones called a second time since get_course_with_access returns a bool
-                    if any_unfulfilled_milestones(course_id, request.user.id):
-                        message = {
-                            "developer_message": "Cannot access content with unfulfilled pre-requisites or unpassed entrance exam."  # pylint: disable=line-too-long
-                        }
-                        return response.Response(
-                            data=message,
-                            status=status.HTTP_204_NO_CONTENT
-                        )
-                    else:
-                        raise
-                return func(self, request, course=course, *args, **kwargs)
-        return _wrapper
-    return _decorator
+    return view_course_access(depth=depth, access_action='load_mobile', check_for_milestones=True)
 
 
 def mobile_view(is_user=False):
     """
     Function and class decorator that abstracts the authentication and permission checks for mobile api views.
     """
-    def _decorator(func_or_class):
-        """
-        Requires either OAuth2 or Session-based authentication.
-        If is_user is True, also requires username in URL matches the request user.
-        """
-        func_or_class.authentication_classes = (
-            OAuth2AuthenticationAllowInactiveUser,
-            SessionAuthenticationAllowInactiveUser
-        )
-        func_or_class.permission_classes = (permissions.IsAuthenticated,)
-        if is_user:
-            func_or_class.permission_classes += (IsUserInUrl,)
-        return func_or_class
-    return _decorator
+    return view_auth_classes(is_user)

lms/djangoapps/mobile_api/video_outlines/tests.py

@@ -18,7 +18,7 @@ from xmodule.partitions.partitions import Group, UserPartition
 from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
 from openedx.core.djangoapps.course_groups.models import CourseUserGroupPartitionGroup
 
-from ..testutils import MobileAPITestCase, MobileAuthTestMixin, MobileEnrolledCourseAccessTestMixin
+from ..testutils import MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin
 
 
 class TestVideoAPITestCase(MobileAPITestCase):
@@ -407,7 +407,7 @@ class TestNonStandardCourseStructure(MobileAPITestCase, TestVideoAPIMixin):
 
 @ddt.ddt
 class TestVideoSummaryList(
-    TestVideoAPITestCase, MobileAuthTestMixin, MobileEnrolledCourseAccessTestMixin, TestVideoAPIMixin  # pylint: disable=bad-continuation
+    TestVideoAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin, TestVideoAPIMixin  # pylint: disable=bad-continuation
 ):
     """
     Tests for /api/mobile/v0.5/video_outlines/courses/{course_id}..
@@ -863,7 +863,7 @@ class TestVideoSummaryList(
 
 
 class TestTranscriptsDetail(
-    TestVideoAPITestCase, MobileAuthTestMixin, MobileEnrolledCourseAccessTestMixin, TestVideoAPIMixin  # pylint: disable=bad-continuation
+    TestVideoAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin, TestVideoAPIMixin  # pylint: disable=bad-continuation
 ):
     """
     Tests for /api/mobile/v0.5/video_outlines/transcripts/{course_id}..

openedx/core/lib/api/permissions.py

 from django.conf import settings
 from rest_framework import permissions
-from rest_framework.exceptions import PermissionDenied
 from django.http import Http404
 
 

openedx/core/lib/api/view_utils.py

 """
 Utilities related to API views
 """
+import functools
 from django.core.exceptions import NON_FIELD_ERRORS, ValidationError
 from django.http import Http404
 
+from rest_framework import status, response
 from rest_framework.exceptions import APIException
 from rest_framework.response import Response
 
+from lms.djangoapps.courseware.courses import get_course_with_access
+from opaque_keys.edx.keys import CourseKey
+from xmodule.modulestore.django import modulestore
+
+from openedx.core.lib.api.authentication import (
+    SessionAuthenticationAllowInactiveUser,
+    OAuth2AuthenticationAllowInactiveUser,
+)
+from openedx.core.lib.api.permissions import IsUserInUrl, IsAuthenticatedOrDebug
+from util.milestones_helpers import any_unfulfilled_milestones
+
 
 class DeveloperErrorViewMixin(object):
     """
@@ -48,3 +61,60 @@ class DeveloperErrorViewMixin(object):
             return self.make_validation_error_response(exc)
         else:
             raise
+
+
+def view_course_access(depth=0, access_action='load', check_for_milestones=False):
+    """
+    Method decorator for an API endpoint that verifies the user has access to the course.
+    """
+    def _decorator(func):
+        """Outer method decorator."""
+        @functools.wraps(func)
+        def _wrapper(self, request, *args, **kwargs):
+            """
+            Expects kwargs to contain 'course_id'.
+            Passes the course descriptor to the given decorated function.
+            Raises 404 if access to course is disallowed.
+            """
+            course_id = CourseKey.from_string(kwargs.pop('course_id'))
+            with modulestore().bulk_operations(course_id):
+                try:
+                    course = get_course_with_access(
+                        request.user,
+                        access_action,
+                        course_id,
+                        depth=depth
+                    )
+                except Http404:
+                    # any_unfulfilled_milestones called a second time since has_access returns a bool
+                    if check_for_milestones and any_unfulfilled_milestones(course_id, request.user.id):
+                        message = {
+                            "developer_message": "Cannot access content with unfulfilled "
+                                                 "pre-requisites or unpassed entrance exam."
+                        }
+                        return response.Response(data=message, status=status.HTTP_204_NO_CONTENT)
+                    else:
+                        raise
+                return func(self, request, course=course, *args, **kwargs)
+        return _wrapper
+    return _decorator
+
+
+def view_auth_classes(is_user=False):
+    """
+    Function and class decorator that abstracts the authentication and permission checks for api views.
+    """
+    def _decorator(func_or_class):
+        """
+        Requires either OAuth2 or Session-based authentication.
+        If is_user is True, also requires username in URL matches the request user.
+        """
+        func_or_class.authentication_classes = (
+            OAuth2AuthenticationAllowInactiveUser,
+            SessionAuthenticationAllowInactiveUser
+        )
+        func_or_class.permission_classes = (IsAuthenticatedOrDebug,)
+        if is_user:
+            func_or_class.permission_classes += (IsUserInUrl,)
+        return func_or_class
+    return _decorator