Merge pull request #6608 from edx/mobile/MA-216

MA-216 Mobile API: display unreleased courses in enrollment list.

lms/djangoapps/courseware/access.py

@@ -166,11 +166,7 @@ def _has_access_course_desc(user, action, course):
             # check start date
             can_load() and
             # check mobile_available flag
-            (
-                course.mobile_available or
-                auth.has_access(user, CourseBetaTesterRole(course.id)) or
-                _has_staff_access_to_descriptor(user, course, course.id)
-            )
+            is_mobile_available_for_user(user, course)
         )
 
     def can_enroll():
@@ -649,6 +645,20 @@ def _has_staff_access_to_descriptor(user, descriptor, course_key):
     return _has_staff_access_to_location(user, descriptor.location, course_key)
 
 
+def is_mobile_available_for_user(user, course):
+    """
+    Returns whether the given course is mobile_available for the given user.
+    Checks:
+        mobile_available flag on the course
+        Beta User and staff access overrides the mobile_available flag
+    """
+    return (
+        course.mobile_available or
+        auth.has_access(user, CourseBetaTesterRole(course.id)) or
+        _has_staff_access_to_descriptor(user, course, course.id)
+    )
+
+
 def get_user_role(user, course_key):
     """
     Return corresponding string if user has staff, instructor or student

lms/djangoapps/mobile_api/tests.py

@@ -3,49 +3,9 @@ Tests for mobile API utilities.
 """
 
 import ddt
-from mock import patch
 from django.test import TestCase
 
-from xmodule.modulestore.tests.factories import CourseFactory
-
-from .utils import mobile_access_when_enrolled, mobile_course_access, mobile_view
-from .testutils import MobileAPITestCase, ROLE_CASES
-
-
-@ddt.ddt
-class TestMobileAccessWhenEnrolled(MobileAPITestCase):
-    """
-    Tests for mobile_access_when_enrolled utility function.
-    """
-    @ddt.data(*ROLE_CASES)
-    @ddt.unpack
-    def test_mobile_role_access(self, role, should_have_access):
-        """
-        Verifies that our mobile access function properly handles using roles to grant access
-        """
-        non_mobile_course = CourseFactory.create(mobile_available=False)
-        if role:
-            role(non_mobile_course.id).add_users(self.user)
-        self.assertEqual(should_have_access, mobile_access_when_enrolled(non_mobile_course, self.user))
-
-    def test_mobile_explicit_access(self):
-        """
-        Verifies that our mobile access function listens to the mobile_available flag as it should
-        """
-        self.assertTrue(mobile_access_when_enrolled(self.course, self.user))
-
-    def test_missing_course(self):
-        """
-        Verifies that we handle the case where a course doesn't exist
-        """
-        self.assertFalse(mobile_access_when_enrolled(None, self.user))
-
-    @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})
-    def test_unreleased_course(self):
-        """
-        Verifies that we handle the case where a course hasn't started
-        """
-        self.assertFalse(mobile_access_when_enrolled(self.course, self.user))
+from .utils import mobile_course_access, mobile_view
 
 
 @ddt.ddt

lms/djangoapps/mobile_api/testutils.py

@@ -26,15 +26,6 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 
 
-# A tuple of Role Types and Boolean values that indicate whether access should be given to that role.
-ROLE_CASES = (
-    (auth.CourseBetaTesterRole, True),
-    (auth.CourseStaffRole, True),
-    (auth.CourseInstructorRole, True),
-    (None, False)
-)
-
-
 class MobileAPITestCase(ModuleStoreTestCase, APITestCase):
     """
     Base class for testing Mobile APIs.
@@ -140,6 +131,8 @@ class MobileCourseAccessTestMixin(object):
     Subclasses are expected to inherit from MobileAPITestCase.
     Subclasses can override verify_success, verify_failure, and init_course_access methods.
     """
+    ALLOW_ACCESS_TO_UNRELEASED_COURSE = False  # pylint: disable=invalid-name
+
     def verify_success(self, response):
         """Base implementation of verifying a successful response."""
         self.assertEqual(response.status_code, 200)
@@ -170,9 +163,18 @@ class MobileCourseAccessTestMixin(object):
         self.init_course_access()
 
         response = self.api_response(expected_response_code=None)
-        self.verify_failure(response)  # allow subclasses to override verification
+        if self.ALLOW_ACCESS_TO_UNRELEASED_COURSE:
+            self.verify_success(response)
+        else:
+            self.verify_failure(response)
 
-    @ddt.data(*ROLE_CASES)
+    # A tuple of Role Types and Boolean values that indicate whether access should be given to that role.
+    @ddt.data(
+        (auth.CourseBetaTesterRole, True),
+        (auth.CourseStaffRole, True),
+        (auth.CourseInstructorRole, True),
+        (None, False)
+    )
     @ddt.unpack
     def test_non_mobile_available(self, role, should_succeed):
         self.init_course_access()

lms/djangoapps/mobile_api/users/tests.py

@@ -5,7 +5,7 @@ Tests for users API
 import datetime
 from django.utils import timezone
 
-from xmodule.modulestore.tests.factories import ItemFactory
+from xmodule.modulestore.tests.factories import ItemFactory, CourseFactory
 from xmodule.modulestore.django import modulestore
 from student.models import CourseEnrollment
 
@@ -48,6 +48,7 @@ class TestUserEnrollmentApi(MobileAPITestCase, MobileAuthUserTestMixin, MobileEn
     Tests for /api/mobile/v0.5/users/<user_name>/course_enrollments/
     """
     REVERSE_INFO = {'name': 'courseenrollment-detail', 'params': ['username']}
+    ALLOW_ACCESS_TO_UNRELEASED_COURSE = True
 
     def verify_success(self, response):
         super(TestUserEnrollmentApi, self).verify_success(response)
@@ -65,6 +66,23 @@ class TestUserEnrollmentApi(MobileAPITestCase, MobileAuthUserTestMixin, MobileEn
         courses = response.data
         self.assertEqual(len(courses), 0)
 
+    def test_sort_order(self):
+        self.login()
+
+        num_courses = 3
+        courses = []
+        for course_num in range(num_courses):
+            courses.append(CourseFactory.create(mobile_available=True))
+            self.enroll(courses[course_num].id)
+
+        # verify courses are returned in the order of enrollment, with most recently enrolled first.
+        response = self.api_response()
+        for course_num in range(num_courses):
+            self.assertEqual(
+                response.data[course_num]['course']['id'],  # pylint: disable=no-member
+                unicode(courses[num_courses - course_num - 1].id)
+            )
+
 
 class CourseStatusAPITestCase(MobileAPITestCase):
     """

lms/djangoapps/mobile_api/users/views.py

@@ -2,9 +2,6 @@
 Views for user API
 """
 
-from courseware.model_data import FieldDataCache
-from courseware.module_render import get_module_for_descriptor
-
 from django.shortcuts import redirect
 from django.utils import dateparse
 
@@ -12,11 +9,13 @@ from rest_framework import generics, views
 from rest_framework.decorators import api_view
 from rest_framework.response import Response
 
-from courseware.views import get_current_child, save_positions_recursively_up
-
 from opaque_keys.edx.keys import UsageKey
 from opaque_keys import InvalidKeyError
 
+from courseware.access import is_mobile_available_for_user
+from courseware.model_data import FieldDataCache
+from courseware.module_render import get_module_for_descriptor
+from courseware.views import get_current_child, save_positions_recursively_up
 from student.models import CourseEnrollment, User
 
 from xblock.fields import Scope
@@ -25,8 +24,8 @@ from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError
 
 from .serializers import CourseEnrollmentSerializer, UserSerializer
-from mobile_api import errors
-from mobile_api.utils import mobile_access_when_enrolled, mobile_view, mobile_course_access
+from .. import errors
+from ..utils import mobile_view, mobile_course_access
 
 
 @mobile_view(is_user=True)
@@ -46,6 +45,7 @@ class UserDetail(generics.RetrieveAPIView):
 
         GET /api/mobile/v0.5/users/{username}
 
+
     **Response Values**
 
         * id: The ID of the user.
@@ -241,10 +241,13 @@ class UserCourseEnrollmentsList(generics.ListAPIView):
     lookup_field = 'username'
 
     def get_queryset(self):
-        enrollments = self.queryset.filter(user__username=self.kwargs['username'], is_active=True).order_by('created')
+        enrollments = self.queryset.filter(
+            user__username=self.kwargs['username'],
+            is_active=True
+        ).order_by('created').reverse()
         return [
             enrollment for enrollment in enrollments
-            if mobile_access_when_enrolled(enrollment.course, self.request.user)
+            if enrollment.course and is_mobile_available_for_user(self.request.user, enrollment.course)
         ]
 
 

lms/djangoapps/mobile_api/utils.py

@@ -4,7 +4,6 @@ Common utility methods and decorators for Mobile APIs.
 
 
 import functools
-from django.http import Http404
 
 from opaque_keys.edx.keys import CourseKey
 from courseware.courses import get_course_with_access
@@ -37,22 +36,6 @@ def mobile_course_access(depth=0, verify_enrolled=True):
     return _decorator
 
 
-def mobile_access_when_enrolled(course, user):
-    """
-    Determines whether a user has access to a course in a mobile context.
-    Checks the mobile_available flag and the start_date.
-    Note: Does not check if the user is actually enrolled in the course.
-    """
-    # The course doesn't always really exist -- we can have bad data in the enrollments
-    # pointing to non-existent (or removed) courses, in which case `course` is None.
-    if not course:
-        return False
-    try:
-        return get_course_with_access(user, 'load_mobile_no_enrollment_check', course.id) is not None
-    except Http404:
-        return False
-
-
 def mobile_view(is_user=False):
     """
     Function and class decorator that abstracts the authentication and permission checks for mobile api views.