diff --git a/lms/djangoapps/mobile_api/course_info/tests.py b/lms/djangoapps/mobile_api/course_info/tests.py index f0777c94f0..ff76715559 100644 --- a/lms/djangoapps/mobile_api/course_info/tests.py +++ b/lms/djangoapps/mobile_api/course_info/tests.py @@ -11,23 +11,30 @@ from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.xml_importer import import_course_from_xml -from ..testutils import MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin +from mobile_api.testutils import MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin +from mobile_api.utils import API_V05, API_V1 @ddt.ddt class TestUpdates(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin, MilestonesTestCaseMixin): """ - Tests for /api/mobile/v0.5/course_info/{course_id}/updates + Tests for /api/mobile/{api_version}/course_info/{course_id}/updates """ - REVERSE_INFO = {'name': 'course-updates-list', 'params': ['course_id']} + REVERSE_INFO = {'name': 'course-updates-list', 'params': ['course_id', 'api_version']} shard = 3 def verify_success(self, response): super(TestUpdates, self).verify_success(response) self.assertEqual(response.data, []) - @ddt.data(True, False) - def test_updates(self, new_format): + @ddt.data( + (True, API_V05), + (True, API_V1), + (False, API_V05), + (False, API_V1), + ) + @ddt.unpack + def test_updates(self, new_format, api_version): """ Tests updates endpoint with /static in the content. Tests both new updates format (using "items") and old format (using "data"). @@ -64,7 +71,7 @@ class TestUpdates(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTest modulestore().update_item(course_updates, self.user.id) # call API - response = self.api_response() + response = self.api_response(api_version=api_version) # verify static URLs are replaced in the content returned by the API self.assertNotIn("\"/static/", response.content) @@ -85,20 +92,32 @@ class TestUpdates(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTest @ddt.ddt class TestHandouts(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin, MilestonesTestCaseMixin): """ - Tests for /api/mobile/v0.5/course_info/{course_id}/handouts + Tests for /api/mobile/{api_version}/course_info/{course_id}/handouts """ - REVERSE_INFO = {'name': 'course-handouts-list', 'params': ['course_id']} + REVERSE_INFO = {'name': 'course-handouts-list', 'params': ['course_id', 'api_version']} shard = 3 - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_handouts(self, default_ms): + @ddt.data( + (ModuleStoreEnum.Type.mongo, API_V05), + (ModuleStoreEnum.Type.mongo, API_V1), + (ModuleStoreEnum.Type.split, API_V05), + (ModuleStoreEnum.Type.split, API_V1), + ) + @ddt.unpack + def test_handouts(self, default_ms, api_version): with self.store.default_store(default_ms): self.add_mobile_available_toy_course() - response = self.api_response(expected_response_code=200) + response = self.api_response(expected_response_code=200, api_version=api_version) self.assertIn("Sample", response.data['handouts_html']) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_no_handouts(self, default_ms): + @ddt.data( + (ModuleStoreEnum.Type.mongo, API_V05), + (ModuleStoreEnum.Type.mongo, API_V1), + (ModuleStoreEnum.Type.split, API_V05), + (ModuleStoreEnum.Type.split, API_V1), + ) + @ddt.unpack + def test_no_handouts(self, default_ms, api_version): with self.store.default_store(default_ms): self.add_mobile_available_toy_course() @@ -107,11 +126,17 @@ class TestHandouts(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTes with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id): self.store.delete_item(handouts_usage_key, self.user.id) - response = self.api_response(expected_response_code=200) + response = self.api_response(expected_response_code=200, api_version=api_version) self.assertIsNone(response.data['handouts_html']) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_empty_handouts(self, default_ms): + @ddt.data( + (ModuleStoreEnum.Type.mongo, API_V05), + (ModuleStoreEnum.Type.mongo, API_V1), + (ModuleStoreEnum.Type.split, API_V05), + (ModuleStoreEnum.Type.split, API_V1), + ) + @ddt.unpack + def test_empty_handouts(self, default_ms, api_version): with self.store.default_store(default_ms): self.add_mobile_available_toy_course() @@ -120,11 +145,17 @@ class TestHandouts(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTes underlying_handouts = self.store.get_item(handouts_usage_key) underlying_handouts.data = "
    " self.store.update_item(underlying_handouts, self.user.id) - response = self.api_response(expected_response_code=200) + response = self.api_response(expected_response_code=200, api_version=api_version) self.assertIsNone(response.data['handouts_html']) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_handouts_static_rewrites(self, default_ms): + @ddt.data( + (ModuleStoreEnum.Type.mongo, API_V05), + (ModuleStoreEnum.Type.mongo, API_V1), + (ModuleStoreEnum.Type.split, API_V05), + (ModuleStoreEnum.Type.split, API_V1), + ) + @ddt.unpack + def test_handouts_static_rewrites(self, default_ms, api_version): with self.store.default_store(default_ms): self.add_mobile_available_toy_course() @@ -134,11 +165,17 @@ class TestHandouts(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTes self.assertIn('\'/static/', underlying_handouts.data) # but shouldn't finish with any - response = self.api_response() + response = self.api_response(api_version=api_version) self.assertNotIn('\'/static/', response.data['handouts_html']) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_jump_to_id_handout_href(self, default_ms): + @ddt.data( + (ModuleStoreEnum.Type.mongo, API_V05), + (ModuleStoreEnum.Type.mongo, API_V1), + (ModuleStoreEnum.Type.split, API_V05), + (ModuleStoreEnum.Type.split, API_V1), + ) + @ddt.unpack + def test_jump_to_id_handout_href(self, default_ms, api_version): with self.store.default_store(default_ms): self.add_mobile_available_toy_course() @@ -149,11 +186,17 @@ class TestHandouts(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTes self.store.update_item(underlying_handouts, self.user.id) # but shouldn't finish with any - response = self.api_response() + response = self.api_response(api_version=api_version) self.assertIn("/courses/{}/jump_to_id/".format(self.course.id), response.data['handouts_html']) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_course_url_handout_href(self, default_ms): + @ddt.data( + (ModuleStoreEnum.Type.mongo, API_V05), + (ModuleStoreEnum.Type.mongo, API_V1), + (ModuleStoreEnum.Type.split, API_V05), + (ModuleStoreEnum.Type.split, API_V1), + ) + @ddt.unpack + def test_course_url_handout_href(self, default_ms, api_version): with self.store.default_store(default_ms): self.add_mobile_available_toy_course() @@ -164,7 +207,7 @@ class TestHandouts(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTes self.store.update_item(underlying_handouts, self.user.id) # but shouldn't finish with any - response = self.api_response() + response = self.api_response(api_version=api_version) self.assertIn("/courses/{}/".format(self.course.id), response.data['handouts_html']) def add_mobile_available_toy_course(self): diff --git a/lms/djangoapps/mobile_api/testutils.py b/lms/djangoapps/mobile_api/testutils.py index 1dd9cf1673..3459bdbf78 100644 --- a/lms/djangoapps/mobile_api/testutils.py +++ b/lms/djangoapps/mobile_api/testutils.py @@ -25,6 +25,7 @@ from courseware.access_response import MobileAvailabilityError, StartDateError, from courseware.tests.factories import UserFactory from mobile_api.models import IgnoreMobileAvailableFlagConfig from mobile_api.tests.test_milestones import MobileAPIMilestonesMixin +from mobile_api.utils import API_V1 from student import auth from student.models import CourseEnrollment from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -49,6 +50,7 @@ class MobileAPITestCase(ModuleStoreTestCase, APITestCase): self.user = UserFactory.create() self.password = 'test' self.username = self.user.username + self.api_version = API_V1 IgnoreMobileAvailableFlagConfig(enabled=False).save() def tearDown(self): @@ -94,6 +96,8 @@ class MobileAPITestCase(ModuleStoreTestCase, APITestCase): reverse_args.update({'course_id': unicode(kwargs.get('course_id', self.course.id))}) if 'username' in self.REVERSE_INFO['params']: reverse_args.update({'username': kwargs.get('username', self.user.username)}) + if 'api_version' in self.REVERSE_INFO['params']: + reverse_args.update({'api_version': kwargs.get('api_version', self.api_version)}) return reverse(self.REVERSE_INFO['name'], kwargs=reverse_args) def url_method(self, url, data=None, **kwargs): # pylint: disable=unused-argument diff --git a/lms/djangoapps/mobile_api/urls.py b/lms/djangoapps/mobile_api/urls.py index e2ba70e18c..407d4f321d 100644 --- a/lms/djangoapps/mobile_api/urls.py +++ b/lms/djangoapps/mobile_api/urls.py @@ -8,7 +8,7 @@ from .users.views import my_user_info urlpatterns = [ url(r'^users/', include('mobile_api.users.urls')), - url(r'^my_user_info', my_user_info), + url(r'^my_user_info', my_user_info, name='user-info'), url(r'^video_outlines/', include('mobile_api.video_outlines.urls')), url(r'^course_info/', include('mobile_api.course_info.urls')), ] diff --git a/lms/djangoapps/mobile_api/users/models.py b/lms/djangoapps/mobile_api/users/models.py deleted file mode 100644 index d2e8572729..0000000000 --- a/lms/djangoapps/mobile_api/users/models.py +++ /dev/null @@ -1,3 +0,0 @@ -""" -A models.py is required to make this an app (until we move to Django 1.7) -""" diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index c5f64dce88..8bdca6d332 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -7,6 +7,7 @@ from rest_framework.reverse import reverse from lms.djangoapps.certificates.api import certificate_downloadable_status from courseware.access import has_access +from openedx.features.course_duration_limits.access import get_user_course_expiration_date from student.models import CourseEnrollment, User from util.course import get_encoded_course_sharing_utm_params, get_link_for_about_page @@ -15,10 +16,11 @@ class CourseOverviewField(serializers.RelatedField): """ Custom field to wrap a CourseOverview object. Read-only. """ - def to_representation(self, course_overview): course_id = unicode(course_overview.id) request = self.context.get('request') + api_version = self.context.get('api_version') + return { # identifiers 'id': course_id, @@ -56,12 +58,12 @@ class CourseOverviewField(serializers.RelatedField): 'course_sharing_utm_parameters': get_encoded_course_sharing_utm_params(), 'course_updates': reverse( 'course-updates-list', - kwargs={'course_id': course_id}, + kwargs={'api_version': api_version, 'course_id': course_id}, request=request, ), 'course_handouts': reverse( 'course-handouts-list', - kwargs={'course_id': course_id}, + kwargs={'api_version': api_version, 'course_id': course_id}, request=request, ), 'discussion_url': reverse( @@ -72,7 +74,7 @@ class CourseOverviewField(serializers.RelatedField): 'video_outline': reverse( 'video-summary-list', - kwargs={'course_id': course_id}, + kwargs={'api_version': api_version, 'course_id': course_id}, request=request, ), } @@ -84,6 +86,13 @@ class CourseEnrollmentSerializer(serializers.ModelSerializer): """ course = CourseOverviewField(source="course_overview", read_only=True) certificate = serializers.SerializerMethodField() + audit_access_expires = serializers.SerializerMethodField() + + def get_audit_access_expires(self, model): + """ + Returns expiration date for a course audit expiration, if any or null + """ + return get_user_course_expiration_date(model.user, model.course) def get_certificate(self, model): """Returns the information about the user's certificate in the course.""" @@ -97,21 +106,39 @@ class CourseEnrollmentSerializer(serializers.ModelSerializer): else: return {} + class Meta(object): + model = CourseEnrollment + fields = ('audit_access_expires', 'created', 'mode', 'is_active', 'course', 'certificate') + lookup_field = 'username' + + +class CourseEnrollmentSerializerv05(CourseEnrollmentSerializer): + """ + Serializes CourseEnrollment models for v0.5 api + Does not include 'audit_access_expires' field that is present in v1 api + """ class Meta(object): model = CourseEnrollment fields = ('created', 'mode', 'is_active', 'course', 'certificate') lookup_field = 'username' -class UserSerializer(serializers.HyperlinkedModelSerializer): +class UserSerializer(serializers.ModelSerializer): """ Serializes User models """ name = serializers.ReadOnlyField(source='profile.name') - course_enrollments = serializers.HyperlinkedIdentityField( - view_name='courseenrollment-detail', - lookup_field='username' - ) + course_enrollments = serializers.SerializerMethodField() + + def get_course_enrollments(self, model): + request = self.context.get('request') + api_version = self.context.get('api_version') + + return reverse( + 'courseenrollment-detail', + kwargs={'api_version': api_version, 'username': model.username}, + request=request + ) class Meta(object): model = User diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 17881ef80b..b017ad4ac7 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -9,6 +9,7 @@ from django.conf import settings from django.template import defaultfilters from django.test import RequestFactory, override_settings from django.utils import timezone +from django.utils.timezone import now from milestones.tests.utils import MilestonesTestCaseMixin from mock import patch @@ -24,46 +25,54 @@ from mobile_api.testutils import ( MobileAuthUserTestMixin, MobileCourseAccessTestMixin ) +from mobile_api.utils import API_V05, API_V1 from openedx.core.lib.courses import course_image_url from openedx.core.lib.tests import attr +from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag +from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG from student.models import CourseEnrollment +from student.tests.factories import CourseEnrollmentFactory from util.milestones_helpers import set_prerequisite_courses from util.testing import UrlResetMixin from xmodule.course_module import DEFAULT_START_DATE from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from .. import errors -from .serializers import CourseEnrollmentSerializer +from .serializers import CourseEnrollmentSerializer, CourseEnrollmentSerializerv05 @attr(shard=9) +@ddt.ddt class TestUserDetailApi(MobileAPITestCase, MobileAuthUserTestMixin): """ - Tests for /api/mobile/v0.5/users/... + Tests for /api/mobile/{api_version}/users/... """ - REVERSE_INFO = {'name': 'user-detail', 'params': ['username']} + REVERSE_INFO = {'name': 'user-detail', 'params': ['username', 'api_version']} - def test_success(self): + @ddt.data(API_V05, API_V1) + def test_success(self, api_version): self.login() - response = self.api_response() + response = self.api_response(api_version=api_version) self.assertEqual(response.data['username'], self.user.username) self.assertEqual(response.data['email'], self.user.email) @attr(shard=9) +@ddt.ddt class TestUserInfoApi(MobileAPITestCase, MobileAuthTestMixin): """ - Tests for /api/mobile/v0.5/my_user_info + Tests for /api/mobile/{api_version}/my_user_info """ - def reverse_url(self, reverse_args=None, **kwargs): - return '/api/mobile/v0.5/my_user_info' + REVERSE_INFO = {'name': 'user-info', 'params': ['api_version']} - def test_success(self): + @ddt.data(API_V05, API_V1) + def test_success(self, api_version): """Verify the endpoint redirects to the user detail endpoint""" self.login() - response = self.api_response(expected_response_code=302) + response = self.api_response(expected_response_code=302, api_version=api_version) self.assertIn(self.username, response['location']) @@ -73,14 +82,15 @@ class TestUserInfoApi(MobileAPITestCase, MobileAuthTestMixin): class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTestMixin, MobileCourseAccessTestMixin, MilestonesTestCaseMixin): """ - Tests for /api/mobile/v0.5/users//course_enrollments/ + Tests for /api/mobile/{api_version}/users//course_enrollments/ """ - REVERSE_INFO = {'name': 'courseenrollment-detail', 'params': ['username']} + REVERSE_INFO = {'name': 'courseenrollment-detail', 'params': ['username', 'api_version']} ALLOW_ACCESS_TO_UNRELEASED_COURSE = True ALLOW_ACCESS_TO_MILESTONE_COURSE = True ALLOW_ACCESS_TO_NON_VISIBLE_COURSE = True NEXT_WEEK = datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=7) LAST_WEEK = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=7) + THREE_YEARS_AGO = now() - datetime.timedelta(days=(365 * 3)) ADVERTISED_START = "Spring 2016" ENABLED_SIGNALS = ['course_published'] DATES = { @@ -121,7 +131,8 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest self.assertEqual(len(courses), 0) @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) - def test_sort_order(self): + @ddt.data(API_V05, API_V1) + def test_sort_order(self, api_version): self.login() num_courses = 3 @@ -131,19 +142,20 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest self.enroll(courses[course_index].id) # verify courses are returned in the order of enrollment, with most recently enrolled first. - response = self.api_response() + response = self.api_response(api_version=api_version) for course_index in range(num_courses): self.assertEqual( response.data[course_index]['course']['id'], unicode(courses[num_courses - course_index - 1].id) ) + @ddt.data(API_V05, API_V1) @patch.dict(settings.FEATURES, { 'ENABLE_PREREQUISITE_COURSES': True, 'DISABLE_START_DATES': False, 'ENABLE_MKTG_SITE': True, }) - def test_courseware_access(self): + def test_courseware_access(self, api_version): self.login() course_with_prereq = CourseFactory.create(start=self.LAST_WEEK, mobile_available=True) @@ -170,7 +182,7 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest self.enroll(course.id) # Verify courses have the correct response through error code. Last enrolled course is first course in response - response = self.api_response() + response = self.api_response(api_version=api_version) for course_index in range(len(courses)): result = response.data[course_index]['course']['courseware_access'] self.assertEqual(result['error_code'], expected_error_codes[::-1][course_index]) @@ -179,16 +191,22 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest self.assertFalse(result['has_access']) @ddt.data( - ('next_week', ADVERTISED_START, ADVERTISED_START, "string"), - ('next_week', None, defaultfilters.date(NEXT_WEEK, "DATE_FORMAT"), "timestamp"), - ('next_week', '', defaultfilters.date(NEXT_WEEK, "DATE_FORMAT"), "timestamp"), - ('default_start_date', ADVERTISED_START, ADVERTISED_START, "string"), - ('default_start_date', '', None, "empty"), - ('default_start_date', None, None, "empty"), + ('next_week', ADVERTISED_START, ADVERTISED_START, "string", API_V05), + ('next_week', ADVERTISED_START, ADVERTISED_START, "string", API_V1), + ('next_week', None, defaultfilters.date(NEXT_WEEK, "DATE_FORMAT"), "timestamp", API_V05), + ('next_week', None, defaultfilters.date(NEXT_WEEK, "DATE_FORMAT"), "timestamp", API_V1), + ('next_week', '', defaultfilters.date(NEXT_WEEK, "DATE_FORMAT"), "timestamp", API_V05), + ('next_week', '', defaultfilters.date(NEXT_WEEK, "DATE_FORMAT"), "timestamp", API_V1), + ('default_start_date', ADVERTISED_START, ADVERTISED_START, "string", API_V05), + ('default_start_date', ADVERTISED_START, ADVERTISED_START, "string", API_V1), + ('default_start_date', '', None, "empty", API_V05), + ('default_start_date', '', None, "empty", API_V1), + ('default_start_date', None, None, "empty", API_V05), + ('default_start_date', None, None, "empty", API_V1), ) @ddt.unpack @patch.dict(settings.FEATURES, {'DISABLE_START_DATES': False, 'ENABLE_MKTG_SITE': True}) - def test_start_type_and_display(self, start, advertised_start, expected_display, expected_type): + def test_start_type_and_display(self, start, advertised_start, expected_display, expected_type, api_version): """ Tests that the correct start_type and start_display are returned in the case the course has not started @@ -197,19 +215,21 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest course = CourseFactory.create(start=self.DATES[start], advertised_start=advertised_start, mobile_available=True) self.enroll(course.id) - response = self.api_response() + response = self.api_response(api_version=api_version) self.assertEqual(response.data[0]['course']['start_type'], expected_type) self.assertEqual(response.data[0]['course']['start_display'], expected_display) + @ddt.data(API_V05, API_V1) @patch.dict(settings.FEATURES, {"ENABLE_DISCUSSION_SERVICE": True, 'ENABLE_MKTG_SITE': True}) - def test_discussion_url(self): + def test_discussion_url(self, api_version): self.login_and_enroll() - response = self.api_response() + response = self.api_response(api_version=api_version) response_discussion_url = response.data[0]['course']['discussion_url'] self.assertIn('/api/discussion/v1/courses/{}'.format(self.course.id), response_discussion_url) - def test_org_query(self): + @ddt.data(API_V05, API_V1) + def test_org_query(self, api_version): self.login() # Create list of courses with various organizations @@ -226,7 +246,7 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest for course in courses: self.enroll(course.id) - response = self.api_response(data={'org': 'edX'}) + response = self.api_response(data={'org': 'edX'}, api_version=api_version) # Test for 3 expected courses self.assertEqual(len(response.data), 3) @@ -235,14 +255,73 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest for entry in response.data: self.assertEqual(entry['course']['org'], 'edX') + def create_enrollment(self, expired): + if expired: + course = CourseFactory.create(start=self.THREE_YEARS_AGO, mobile_available=True) + enrollment = CourseEnrollmentFactory.create( + user=self.user, + course_id=course.id + ) + ScheduleFactory(start=self.THREE_YEARS_AGO, enrollment=enrollment) + else: + course = CourseFactory.create(start=self.LAST_WEEK, mobile_available=True) + self.enroll(course.id) + + def _get_enrollment_data(self, api_version, expired): + self.login() + self.create_enrollment(expired) + return self.api_response(api_version=api_version).data + + def _assert_enrollment_results(self, api_version, courses, num_courses_returned): + self.assertEqual(len(courses), num_courses_returned) + + if api_version == API_V05: + if num_courses_returned: + self.assertFalse('audit_access_expires' in courses[0]) + else: + self.assertTrue('audit_access_expires' in courses[0]) + self.assertIsNotNone(courses[0].get('audit_access_expires')) + + @ddt.data( + (API_V05, True, 0), + (API_V05, False, 1), + (API_V1, True, 1), + (API_V1, False, 1), + ) + @ddt.unpack + @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) + def test_enrollment_with_gating(self, api_version, expired, num_courses_returned): + ''' + Test that expired courses are only returned in v1 of API + when waffle flag enabled, and un-expired courses always returned + ''' + courses = self._get_enrollment_data(api_version, expired) + self._assert_enrollment_results(api_version, courses, num_courses_returned) + + @ddt.data( + (API_V05, True, 1), + (API_V05, False, 1), + (API_V1, True, 1), + (API_V1, False, 1), + ) + @ddt.unpack + @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, False) + def test_enrollment_no_gating(self, api_version, expired, num_courses_returned): + ''' + Test that expired and non-expired courses returned if waffle flag is disabled + regarless of version of API + ''' + courses = self._get_enrollment_data(api_version, expired) + self._assert_enrollment_results(api_version, courses, num_courses_returned) + @attr(shard=9) @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) class TestUserEnrollmentCertificates(UrlResetMixin, MobileAPITestCase, MilestonesTestCaseMixin): """ - Tests for /api/mobile/v0.5/users//course_enrollments/ + Tests for /api/mobile/{api_version}/users//course_enrollments/ """ - REVERSE_INFO = {'name': 'courseenrollment-detail', 'params': ['username']} + REVERSE_INFO = {'name': 'courseenrollment-detail', 'params': ['username', 'api_version']} ENABLED_SIGNALS = ['course_published'] def verify_pdf_certificate(self): @@ -315,9 +394,9 @@ class TestUserEnrollmentCertificates(UrlResetMixin, MobileAPITestCase, Milestone @attr(shard=9) class CourseStatusAPITestCase(MobileAPITestCase): """ - Base test class for /api/mobile/v0.5/users//course_status_info/{course_id} + Base test class for /api/mobile/{api_version}/users//course_status_info/{course_id} """ - REVERSE_INFO = {'name': 'user-course-status', 'params': ['username', 'course_id']} + REVERSE_INFO = {'name': 'user-course-status', 'params': ['username', 'course_id', 'api_version']} def setUp(self): """ @@ -472,6 +551,7 @@ class TestCourseStatusPATCH(CourseStatusAPITestCase, MobileAuthUserTestMixin, @attr(shard=9) +@ddt.ddt @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) class TestCourseEnrollmentSerializer(MobileAPITestCase, MilestonesTestCaseMixin): @@ -486,14 +566,38 @@ class TestCourseEnrollmentSerializer(MobileAPITestCase, MilestonesTestCaseMixin) self.request = RequestFactory().get('/') self.request.user = self.user - def test_success(self): - serialized = CourseEnrollmentSerializer( + def get_serialized_data(self, api_version): + ''' + Return data from CourseEnrollmentSerializer + ''' + if api_version == API_V05: + serializer = CourseEnrollmentSerializerv05 + else: + serializer = CourseEnrollmentSerializer + + return serializer( CourseEnrollment.enrollments_for_user(self.user)[0], - context={'request': self.request}, + context={'request': self.request, 'api_version': api_version}, ).data + + def _expiration_in_response(self, response, api_version): + ''' + Assert that audit_access_expires field in present in response + based on version of api being used + ''' + if api_version != API_V05: + self.assertTrue('audit_access_expires' in response) + self.assertIsNotNone(response.get('audit_access_expires')) + else: + self.assertFalse('audit_access_expires' in response) + + @ddt.data(API_V05, API_V1) + def test_success(self, api_version): + serialized = self.get_serialized_data(api_version) self.assertEqual(serialized['course']['name'], self.course.display_name) self.assertEqual(serialized['course']['number'], self.course.id.course) self.assertEqual(serialized['course']['org'], self.course.id.org) + self._expiration_in_response(serialized, api_version) # Assert utm parameters expected_utm_parameters = { @@ -502,14 +606,13 @@ class TestCourseEnrollmentSerializer(MobileAPITestCase, MilestonesTestCaseMixin) } self.assertEqual(serialized['course']['course_sharing_utm_parameters'], expected_utm_parameters) - def test_with_display_overrides(self): + @ddt.data(API_V05, API_V1) + def test_with_display_overrides(self, api_version): self.course.display_coursenumber = "overridden_number" self.course.display_organization = "overridden_org" self.store.update_item(self.course, self.user.id) - serialized = CourseEnrollmentSerializer( - CourseEnrollment.enrollments_for_user(self.user)[0], - context={'request': self.request}, - ).data + serialized = self.get_serialized_data(api_version) self.assertEqual(serialized['course']['number'], self.course.display_coursenumber) self.assertEqual(serialized['course']['org'], self.course.display_organization) + self._expiration_in_response(serialized, api_version) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 796ef55bfd..1898d68dc2 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -19,13 +19,17 @@ from courseware.model_data import FieldDataCache from courseware.module_render import get_module_for_descriptor from courseware.views.index import save_positions_recursively_up from experiments.models import ExperimentData, ExperimentKeyValue +from lms.djangoapps.courseware.access_utils import ACCESS_GRANTED +from mobile_api.utils import API_V05 +from openedx.features.course_duration_limits.access import check_course_expired +from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG from student.models import CourseEnrollment, User from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError from .. import errors from ..decorators import mobile_course_access, mobile_view -from .serializers import CourseEnrollmentSerializer, UserSerializer +from .serializers import CourseEnrollmentSerializer, CourseEnrollmentSerializerv05, UserSerializer @mobile_view(is_user=True) @@ -43,7 +47,7 @@ class UserDetail(generics.RetrieveAPIView): **Example Request** - GET /api/mobile/v0.5/users/{username} + GET /api/mobile/{version}/users/{username} **Response Values** @@ -64,6 +68,11 @@ class UserDetail(generics.RetrieveAPIView): serializer_class = UserSerializer lookup_field = 'username' + def get_serializer_context(self): + context = super(UserDetail, self).get_serializer_context() + context['api_version'] = self.kwargs.get('api_version') + return context + @mobile_view(is_user=True) class UserCourseStatus(views.APIView): @@ -75,9 +84,9 @@ class UserCourseStatus(views.APIView): **Example Requests** - GET /api/mobile/v0.5/users/{username}/course_status_info/{course_id} + GET /api/mobile/{version}/users/{username}/course_status_info/{course_id} - PATCH /api/mobile/v0.5/users/{username}/course_status_info/{course_id} + PATCH /api/mobile/{version}/users/{username}/course_status_info/{course_id} **PATCH Parameters** @@ -209,9 +218,14 @@ class UserCourseEnrollmentsList(generics.ListAPIView): Get information about the courses that the currently signed in user is enrolled in. + v1 differs from v0.5 version by returning ALL enrollments for + a user rather than only the enrollments the user has access to (that haven't expired). + An additional attribute "expiration" has been added to the response, which lists the date + when access to the course will expire or null if it doesn't expire. + **Example Request** - GET /api/mobile/v0.5/users/{username}/course_enrollments/ + GET /api/mobile/v1/users/{username}/course_enrollments/ **Response Values** @@ -220,6 +234,8 @@ class UserCourseEnrollmentsList(generics.ListAPIView): The HTTP 200 response has the following values. + * expiration: The course expiration date for given user course pair + or null if the course does not expire. * certificate: Information about the user's earned certificate in the course. * course: A collection of the following data about the course. @@ -258,7 +274,6 @@ class UserCourseEnrollmentsList(generics.ListAPIView): * url: URL to the downloadable version of the certificate, if exists. """ queryset = CourseEnrollment.objects.all() - serializer_class = CourseEnrollmentSerializer lookup_field = 'username' # In Django Rest Framework v3, there is a default pagination @@ -313,24 +328,48 @@ class UserCourseEnrollmentsList(generics.ListAPIView): return False + def get_serializer_context(self): + context = super(UserCourseEnrollmentsList, self).get_serializer_context() + context['api_version'] = self.kwargs.get('api_version') + return context + + def get_serializer_class(self): + api_version = self.kwargs.get('api_version') + if api_version == API_V05: + return CourseEnrollmentSerializerv05 + return CourseEnrollmentSerializer + def get_queryset(self): + api_version = self.kwargs.get('api_version') enrollments = self.queryset.filter( user__username=self.kwargs['username'], is_active=True ).order_by('created').reverse() org = self.request.query_params.get('org', None) - return [ - enrollment for enrollment in enrollments - if enrollment.course_overview and self.is_org(org, enrollment.course_overview.org) and - is_mobile_available_for_user(self.request.user, enrollment.course_overview) and - not self.hide_course_for_enrollment_fee_experiment(self.request.user, enrollment) - ] + + if api_version == API_V05: + # for v0.5 don't return expired courses + return [ + enrollment for enrollment in enrollments + if enrollment.course_overview and self.is_org(org, enrollment.course_overview.org) and + is_mobile_available_for_user(self.request.user, enrollment.course_overview) and + not self.hide_course_for_enrollment_fee_experiment(self.request.user, enrollment) and + (not CONTENT_TYPE_GATING_FLAG.is_enabled() or + check_course_expired(self.request.user, enrollment.course) == ACCESS_GRANTED) + ] + else: + # return all courses, with associated expiration + return [ + enrollment for enrollment in enrollments + if enrollment.course_overview and self.is_org(org, enrollment.course_overview.org) and + is_mobile_available_for_user(self.request.user, enrollment.course_overview) + ] @api_view(["GET"]) @mobile_view() -def my_user_info(request): +def my_user_info(request, api_version): """ Redirect to the currently-logged-in user's info page """ - return redirect("user-detail", username=request.user.username) + return redirect("user-detail", api_version=api_version, username=request.user.username) diff --git a/lms/djangoapps/mobile_api/utils.py b/lms/djangoapps/mobile_api/utils.py index 77d3415696..b923d0e5ef 100644 --- a/lms/djangoapps/mobile_api/utils.py +++ b/lms/djangoapps/mobile_api/utils.py @@ -1,6 +1,8 @@ """ Common utility methods for Mobile APIs. """ +API_V05 = 'v0.5' +API_V1 = 'v1' def parsed_version(version): diff --git a/lms/djangoapps/mobile_api/video_outlines/serializers.py b/lms/djangoapps/mobile_api/video_outlines/serializers.py index 638f0bab9c..5737df6b60 100644 --- a/lms/djangoapps/mobile_api/video_outlines/serializers.py +++ b/lms/djangoapps/mobile_api/video_outlines/serializers.py @@ -17,11 +17,12 @@ class BlockOutline(object): """ Serializes course videos, pulling data from VAL and the video modules. """ - def __init__(self, course_id, start_block, block_types, request, video_profiles): + def __init__(self, course_id, start_block, block_types, request, video_profiles, api_version): """Create a BlockOutline using `start_block` as a starting point.""" self.start_block = start_block self.block_types = block_types self.course_id = course_id + self.api_version = api_version self.request = request # needed for making full URLS self.local_cache = {} try: @@ -81,7 +82,13 @@ class BlockOutline(object): "named_path": [b["name"] for b in block_path], "unit_url": unit_url, "section_url": section_url, - "summary": summary_fn(self.course_id, curr_block, self.request, self.local_cache) + "summary": summary_fn( + self.course_id, + curr_block, + self.request, + self.local_cache, + self.api_version + ) } if curr_block.has_children: @@ -104,7 +111,7 @@ def path(block, child_to_parent, start_block): if block is not start_block: block_path.append({ # to be consistent with other edx-platform clients, return the defaulted display name - 'name': block.display_name_with_default_escaped, + 'name': block.display_name_with_default_escaped, # xss-lint: disable=python-deprecated-display-name 'category': block.category, 'id': unicode(block.location) }) @@ -160,7 +167,7 @@ def find_urls(course_id, block, child_to_parent, request): return unit_url, section_url -def video_summary(video_profiles, course_id, video_descriptor, request, local_cache): +def video_summary(video_profiles, course_id, video_descriptor, request, local_cache, api_version): """ returns summary dict for the given video module """ @@ -224,7 +231,8 @@ def video_summary(video_profiles, course_id, video_descriptor, request, local_ca kwargs={ 'course_id': unicode(course_id), 'block_id': video_descriptor.scope_ids.usage_id.block_id, - 'lang': lang + 'lang': lang, + 'api_version': api_version }, request=request, ) diff --git a/lms/djangoapps/mobile_api/video_outlines/tests.py b/lms/djangoapps/mobile_api/video_outlines/tests.py index ce7eee398f..78783fa78a 100644 --- a/lms/djangoapps/mobile_api/video_outlines/tests.py +++ b/lms/djangoapps/mobile_api/video_outlines/tests.py @@ -16,7 +16,12 @@ from milestones.tests.utils import MilestonesTestCaseMixin from mock import patch from mobile_api.models import MobileApiConfig -from mobile_api.testutils import MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin +from mobile_api.testutils import ( + MobileAPITestCase, + MobileAuthTestMixin, + MobileCourseAccessTestMixin +) +from mobile_api.utils import API_V05, API_V1 from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort, remove_user_from_cohort from openedx.core.djangoapps.course_groups.models import CourseUserGroupPartitionGroup from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory @@ -204,11 +209,12 @@ class TestVideoAPIMixin(object): @attr(shard=9) +@ddt.ddt class TestNonStandardCourseStructure(MobileAPITestCase, TestVideoAPIMixin, MilestonesTestCaseMixin): """ - Tests /api/mobile/v0.5/video_outlines/courses/{course_id} with no course set + Tests /api/mobile/{api_version}/video_outlines/courses/{course_id} with no course set """ - REVERSE_INFO = {'name': 'video-summary-list', 'params': ['course_id']} + REVERSE_INFO = {'name': 'video-summary-list', 'params': ['course_id', 'api_version']} def setUp(self): super(TestNonStandardCourseStructure, self).setUp() @@ -238,7 +244,8 @@ class TestNonStandardCourseStructure(MobileAPITestCase, TestVideoAPIMixin, Miles display_name=u"test factory vertical under section omega \u03a9", ) - def test_structure_course_video(self): + @ddt.data(API_V05, API_V1) + def test_structure_course_video(self, api_version): """ Tests when there is a video without a vertical directly under course """ @@ -248,7 +255,7 @@ class TestNonStandardCourseStructure(MobileAPITestCase, TestVideoAPIMixin, Miles category="video", display_name=u"test factory video omega \u03a9", ) - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data self.assertEqual(len(course_outline), 1) section_url = course_outline[0]["section_url"] unit_url = course_outline[0]["unit_url"] @@ -257,7 +264,8 @@ class TestNonStandardCourseStructure(MobileAPITestCase, TestVideoAPIMixin, Miles self._verify_paths(course_outline, []) - def test_structure_course_vert_video(self): + @ddt.data(API_V05, API_V1) + def test_structure_course_vert_video(self, api_version): """ Tests when there is a video under vertical directly under course """ @@ -267,7 +275,7 @@ class TestNonStandardCourseStructure(MobileAPITestCase, TestVideoAPIMixin, Miles category="video", display_name=u"test factory video omega \u03a9", ) - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data self.assertEqual(len(course_outline), 1) section_url = course_outline[0]["section_url"] unit_url = course_outline[0]["unit_url"] @@ -284,7 +292,8 @@ class TestNonStandardCourseStructure(MobileAPITestCase, TestVideoAPIMixin, Miles ] ) - def test_structure_course_chap_video(self): + @ddt.data(API_V05, API_V1) + def test_structure_course_chap_video(self, api_version): """ Tests when there is a video directly under chapter """ @@ -295,7 +304,7 @@ class TestNonStandardCourseStructure(MobileAPITestCase, TestVideoAPIMixin, Miles category="video", display_name=u"test factory video omega \u03a9", ) - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data self.assertEqual(len(course_outline), 1) section_url = course_outline[0]["section_url"] unit_url = course_outline[0]["unit_url"] @@ -313,7 +322,8 @@ class TestNonStandardCourseStructure(MobileAPITestCase, TestVideoAPIMixin, Miles ] ) - def test_structure_course_section_video(self): + @ddt.data(API_V05, API_V1) + def test_structure_course_section_video(self, api_version): """ Tests when chapter is none, and video under section under course """ @@ -323,7 +333,7 @@ class TestNonStandardCourseStructure(MobileAPITestCase, TestVideoAPIMixin, Miles category="video", display_name=u"test factory video omega \u03a9", ) - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data self.assertEqual(len(course_outline), 1) section_url = course_outline[0]["section_url"] unit_url = course_outline[0]["unit_url"] @@ -341,7 +351,8 @@ class TestNonStandardCourseStructure(MobileAPITestCase, TestVideoAPIMixin, Miles ] ) - def test_structure_course_chap_section_video(self): + @ddt.data(API_V05, API_V1) + def test_structure_course_chap_section_video(self, api_version): """ Tests when chapter and sequential exists, with a video with no vertical. """ @@ -352,7 +363,7 @@ class TestNonStandardCourseStructure(MobileAPITestCase, TestVideoAPIMixin, Miles category="video", display_name=u"meow factory video omega \u03a9", ) - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data self.assertEqual(len(course_outline), 1) section_url = course_outline[0]["section_url"] unit_url = course_outline[0]["unit_url"] @@ -374,7 +385,8 @@ class TestNonStandardCourseStructure(MobileAPITestCase, TestVideoAPIMixin, Miles ] ) - def test_structure_course_section_vert_video(self): + @ddt.data(API_V05, API_V1) + def test_structure_course_section_vert_video(self, api_version): """ Tests chapter->section->vertical->unit """ @@ -384,7 +396,7 @@ class TestNonStandardCourseStructure(MobileAPITestCase, TestVideoAPIMixin, Miles category="video", display_name=u"test factory video omega \u03a9", ) - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data self.assertEqual(len(course_outline), 1) section_url = course_outline[0]["section_url"] unit_url = course_outline[0]["unit_url"] @@ -418,14 +430,15 @@ class TestNonStandardCourseStructure(MobileAPITestCase, TestVideoAPIMixin, Miles class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin, TestVideoAPIMixin, MilestonesTestCaseMixin): """ - Tests for /api/mobile/v0.5/video_outlines/courses/{course_id}.. + Tests for /api/mobile/{api_version}/video_outlines/courses/{course_id}.. """ - REVERSE_INFO = {'name': 'video-summary-list', 'params': ['course_id']} + REVERSE_INFO = {'name': 'video-summary-list', 'params': ['course_id', 'api_version']} - def test_only_on_web(self): + @ddt.data(API_V05, API_V1) + def test_only_on_web(self, api_version): self.login_and_enroll() - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data self.assertEqual(len(course_outline), 0) subid = uuid4().hex @@ -448,7 +461,7 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour subid=subid ) - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data self.assertEqual(len(course_outline), 1) @@ -462,7 +475,8 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour self.assertEqual(course_outline[0]["summary"]["category"], "video") self.assertTrue(course_outline[0]["summary"]["only_on_web"]) - def test_mobile_api_video_profiles(self): + @ddt.data(API_V05, API_V1) + def test_mobile_api_video_profiles(self, api_version): """ Tests VideoSummaryList with different MobileApiConfig video_profiles """ @@ -505,7 +519,7 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour 'video_url': self.video_url_high, 'duration': 12.0, 'transcripts': { - 'en': 'http://testserver/api/mobile/v0.5/video_outlines/transcripts/{}/testing_mobile_high_video/en'.format(self.course.id) # pylint: disable=line-too-long + 'en': 'http://testserver/api/mobile/{api_version}/video_outlines/transcripts/{course_id}/testing_mobile_high_video/en'.format(api_version=api_version, course_id=self.course.id) # pylint: disable=line-too-long }, 'only_on_web': False, 'encoded_videos': { @@ -529,29 +543,28 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour with patch.dict(settings.FEATURES, FALLBACK_TO_ENGLISH_TRANSCRIPTS=False): # Other platform installations may override this setting # This ensures that the server don't return empty English transcripts when there's none! - self.assertFalse(self.api_response().data[0]['summary'].get('transcripts')) + self.assertFalse(self.api_response(api_version=api_version).data[0]['summary'].get('transcripts')) # Testing when video_profiles='mobile_low,mobile_high,youtube' - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data course_outline[0]['summary'].pop("id") self.assertEqual(course_outline[0]['summary'], expected_output) # Testing when there is no mobile_low, and that mobile_high doesn't show MobileApiConfig(video_profiles="mobile_low,youtube").save() - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data expected_output['encoded_videos'].pop('mobile_high') expected_output['video_url'] = self.youtube_url expected_output['size'] = 2222 - course_outline[0]['summary'].pop("id") self.assertEqual(course_outline[0]['summary'], expected_output) # Testing where youtube is the default video over mobile_high MobileApiConfig(video_profiles="youtube,mobile_high").save() - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data expected_output['encoded_videos']['mobile_high'] = { 'url': self.video_url_high, @@ -561,7 +574,8 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour course_outline[0]['summary'].pop("id") self.assertEqual(course_outline[0]['summary'], expected_output) - def test_mobile_api_html5_sources(self): + @ddt.data(API_V05, API_V1) + def test_mobile_api_html5_sources(self, api_version): """ Tests VideoSummaryList without the video pipeline, using fallback HTML5 video URLs """ @@ -584,17 +598,18 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour 'video_url': self.video_url_low, 'duration': None, 'transcripts': { - 'en': 'http://testserver/api/mobile/v0.5/video_outlines/transcripts/{}/testing_html5_sources/en'.format(self.course.id) # pylint: disable=line-too-long + 'en': 'http://testserver/api/mobile/{api_version}/video_outlines/transcripts/{course_id}/testing_html5_sources/en'.format(api_version=api_version, course_id=self.course.id) # pylint: disable=line-too-long }, 'only_on_web': False, 'encoded_videos': None, 'size': 0, } - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data self.assertEqual(course_outline[0]['summary'], expected_output) - def test_video_not_in_val(self): + @ddt.data(API_V05, API_V1) + def test_video_not_in_val(self, api_version): self.login_and_enroll() self._create_video_with_subs() ItemFactory.create( @@ -605,14 +620,15 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour html5_sources=[self.html5_video_url] ) - summary = self.api_response().data[1]['summary'] + summary = self.api_response(api_version=api_version).data[1]['summary'] self.assertEqual(summary['name'], "some non existent video in val") self.assertIsNone(summary['encoded_videos']) self.assertIsNone(summary['duration']) self.assertEqual(summary['size'], 0) self.assertEqual(summary['video_url'], self.html5_video_url) - def test_course_list(self): + @ddt.data(API_V05, API_V1) + def test_course_list(self, api_version): self.login_and_enroll() self._create_video_with_subs() ItemFactory.create( @@ -635,7 +651,7 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour visible_to_staff_only=True, ) - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data self.assertEqual(len(course_outline), 3) vid = course_outline[0] self.assertIn('test_subsection_omega_%CE%A9', vid['section_url']) @@ -654,7 +670,8 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour self.assertEqual(course_outline[2]['summary']['size'], 0) self.assertFalse(course_outline[2]['summary']['only_on_web']) - def test_with_nameless_unit(self): + @ddt.data(API_V05, API_V1) + def test_with_nameless_unit(self, api_version): self.login_and_enroll() ItemFactory.create( parent=self.nameless_unit, @@ -662,11 +679,12 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour edx_video_id=self.edx_video_id, display_name=u"test draft video omega 2 \u03a9" ) - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data self.assertEqual(len(course_outline), 1) self.assertEqual(course_outline[0]['path'][2]['name'], self.nameless_unit.location.block_id) - def test_with_video_in_sub_section(self): + @ddt.data(API_V05, API_V1) + def test_with_video_in_sub_section(self, api_version): """ Tests a non standard xml format where a video is underneath a sequential @@ -680,7 +698,7 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour edx_video_id=self.edx_video_id, display_name=u"video in the sub section" ) - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data self.assertEqual(len(course_outline), 1) self.assertEqual(len(course_outline[0]['path']), 2) section_url = course_outline[0]["section_url"] @@ -695,17 +713,17 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour self.assertEqual(section_url, unit_url) @ddt.data( - *itertools.product([True, False], ["video", "problem"]) + *itertools.product([True, False], ["video", "problem"], [API_V05, API_V1]) ) @ddt.unpack - def test_with_split_block(self, is_user_staff, sub_block_category): + def test_with_split_block(self, is_user_staff, sub_block_category, api_version): """Test with split_module->sub_block_category and for both staff and non-staff users.""" self.login_and_enroll() self.user.is_staff = is_user_staff self.user.save() self._setup_split_module(sub_block_category) - video_outline = self.api_response().data + video_outline = self.api_response(api_version=api_version).data num_video_blocks = 1 if sub_block_category == "video" else 0 self.assertEqual(len(video_outline), num_video_blocks) for block_index in range(num_video_blocks): @@ -721,7 +739,8 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour ) self.assertIn(u"split test block", video_outline[block_index]["summary"]["name"]) - def test_with_split_vertical(self): + @ddt.data(API_V05, API_V1) + def test_with_split_vertical(self, api_version): """Test with split_module->vertical->video structure.""" self.login_and_enroll() split_vertical_a, split_vertical_b = self._setup_split_module("vertical") @@ -737,7 +756,7 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour display_name=u"video in vertical b", ) - video_outline = self.api_response().data + video_outline = self.api_response(api_version=api_version).data # user should see only one of the videos (a or b). self.assertEqual(len(video_outline), 1) @@ -777,8 +796,14 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour display_name=u"video for group " + unicode(group_id), ) - @ddt.data("_create_cohorted_video", "_create_cohorted_vertical_with_video") - def test_with_cohorted_content(self, content_creator_method_name): + @ddt.data( + ("_create_cohorted_video", API_V05), + ("_create_cohorted_video", API_V1), + ("_create_cohorted_vertical_with_video", API_V05), + ("_create_cohorted_vertical_with_video", API_V1), + ) + @ddt.unpack + def test_with_cohorted_content(self, content_creator_method_name, api_version): self.login_and_enroll() self._setup_course_partitions(scheme_id='cohort', is_cohorted=True) @@ -799,7 +824,7 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour add_user_to_cohort(cohorts[cohort_index], self.user.username) # should only see video for this cohort - video_outline = self.api_response().data + video_outline = self.api_response(api_version=api_version).data self.assertEqual(len(video_outline), 1) self.assertEquals( u"video for group " + unicode(cohort_index), @@ -810,16 +835,17 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour remove_user_from_cohort(cohorts[cohort_index], self.user.username) # un-cohorted user should see no videos - video_outline = self.api_response().data + video_outline = self.api_response(api_version=api_version).data self.assertEqual(len(video_outline), 0) # staff user sees all videos self.user.is_staff = True self.user.save() - video_outline = self.api_response().data + video_outline = self.api_response(api_version=api_version).data self.assertEqual(len(video_outline), 2) - def test_with_hidden_blocks(self): + @ddt.data(API_V05, API_V1) + def test_with_hidden_blocks(self, api_version): self.login_and_enroll() hidden_subsection = ItemFactory.create( parent=self.section, @@ -845,10 +871,11 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour category="video", edx_video_id=self.edx_video_id, ) - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data self.assertEqual(len(course_outline), 0) - def test_language(self): + @ddt.data(API_V05, API_V1) + def test_language(self, api_version): self.login_and_enroll() video = ItemFactory.create( parent=self.nameless_unit, @@ -873,11 +900,12 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour for case in language_cases: video.transcripts = case.transcripts modulestore().update_item(video, self.user.id) - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data self.assertEqual(len(course_outline), 1) self.assertEqual(course_outline[0]['summary']['language'], case.expected_language) - def test_transcripts(self): + @ddt.data(API_V05, API_V1) + def test_transcripts(self, api_version): self.login_and_enroll() video = ItemFactory.create( parent=self.nameless_unit, @@ -906,7 +934,7 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour video.transcripts = case.transcripts video.sub = case.english_subtitle modulestore().update_item(video, self.user.id) - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data self.assertEqual(len(course_outline), 1) self.assertSetEqual( set(course_outline[0]['summary']['transcripts'].keys()), @@ -914,17 +942,24 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour ) @ddt.data( - ({}, '', [], ['en']), - ({}, '', ['de'], ['de']), - ({}, '', ['en', 'de'], ['en', 'de']), - ({}, 'en-subs', ['de'], ['en', 'de']), - ({'uk': 1}, 'en-subs', ['de'], ['en', 'uk', 'de']), - ({'uk': 1, 'de': 1}, 'en-subs', ['de', 'en'], ['en', 'uk', 'de']), + ({}, '', [], ['en'], API_V05), + ({}, '', [], ['en'], API_V1), + ({}, '', ['de'], ['de'], API_V05), + ({}, '', ['de'], ['de'], API_V1), + ({}, '', ['en', 'de'], ['en', 'de'], API_V05), + ({}, '', ['en', 'de'], ['en', 'de'], API_V1), + ({}, 'en-subs', ['de'], ['en', 'de'], API_V05), + ({}, 'en-subs', ['de'], ['en', 'de'], API_V1), + ({'uk': 1}, 'en-subs', ['de'], ['en', 'uk', 'de'], API_V05), + ({'uk': 1}, 'en-subs', ['de'], ['en', 'uk', 'de'], API_V1), + ({'uk': 1, 'de': 1}, 'en-subs', ['de', 'en'], ['en', 'uk', 'de'], API_V05), + ({'uk': 1, 'de': 1}, 'en-subs', ['de', 'en'], ['en', 'uk', 'de'], API_V1), ) @ddt.unpack @patch('xmodule.video_module.transcripts_utils.edxval_api.get_available_transcript_languages') def test_val_transcripts_with_feature_enabled(self, transcripts, english_sub, val_transcripts, - expected_transcripts, mock_get_transcript_languages): + expected_transcripts, api_version, + mock_get_transcript_languages): self.login_and_enroll() video = ItemFactory.create( parent=self.nameless_unit, @@ -938,18 +973,19 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour video.sub = english_sub modulestore().update_item(video, self.user.id) - course_outline = self.api_response().data + course_outline = self.api_response(api_version=api_version).data self.assertEqual(len(course_outline), 1) self.assertItemsEqual(course_outline[0]['summary']['transcripts'].keys(), expected_transcripts) @attr(shard=9) +@ddt.ddt class TestTranscriptsDetail(TestVideoAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin, TestVideoAPIMixin, MilestonesTestCaseMixin): """ - Tests for /api/mobile/v0.5/video_outlines/transcripts/{course_id}.. + Tests for /api/mobile/{api_version}/video_outlines/transcripts/{course_id}.. """ - REVERSE_INFO = {'name': 'video-transcripts-detail', 'params': ['course_id']} + REVERSE_INFO = {'name': 'video-transcripts-detail', 'params': ['course_id', 'api_version']} def setUp(self): super(TestTranscriptsDetail, self).setUp() @@ -963,21 +999,24 @@ class TestTranscriptsDetail(TestVideoAPITestCase, MobileAuthTestMixin, MobileCou }) return super(TestTranscriptsDetail, self).reverse_url(reverse_args, **kwargs) - def test_incorrect_language(self): + @ddt.data(API_V05, API_V1) + def test_incorrect_language(self, api_version): self.login_and_enroll() - self.api_response(expected_response_code=404, lang='pl') + self.api_response(expected_response_code=404, lang='pl', api_version=api_version) - def test_transcript_with_unicode_file_name(self): + @ddt.data(API_V05, API_V1) + def test_transcript_with_unicode_file_name(self, api_version): self.video = self._create_video_with_subs(custom_subid=u'你好') self.login_and_enroll() - self.api_response(expected_response_code=200, lang='en') + self.api_response(expected_response_code=200, lang='en', api_version=api_version) + @ddt.data(API_V05, API_V1) @patch( 'xmodule.video_module.transcripts_utils.edxval_api.get_available_transcript_languages', Mock(return_value=['uk']), ) @patch('xmodule.video_module.transcripts_utils.edxval_api.get_video_transcript_data') - def test_val_transcript(self, mock_get_video_transcript_content): + def test_val_transcript(self, api_version, mock_get_video_transcript_content): """ Tests transcript retrieval view with val transcripts. """ @@ -992,7 +1031,7 @@ class TestTranscriptsDetail(TestVideoAPITestCase, MobileAuthTestMixin, MobileCou self.login_and_enroll() # Now, make request to retrieval endpoint - response = self.api_response(expected_response_code=200, lang='uk') + response = self.api_response(expected_response_code=200, lang='uk', api_version=api_version) # Expected headers expected_content = u'0\n00:00:00,010 --> 00:00:00,100\nHi, welcome to Edx.\n\n' diff --git a/lms/djangoapps/mobile_api/video_outlines/views.py b/lms/djangoapps/mobile_api/video_outlines/views.py index 1d015925f1..3244f9d12b 100644 --- a/lms/djangoapps/mobile_api/video_outlines/views.py +++ b/lms/djangoapps/mobile_api/video_outlines/views.py @@ -84,6 +84,7 @@ class VideoSummaryList(generics.ListAPIView): {"video": partial(video_summary, video_profiles)}, request, video_profiles, + kwargs.get('api_version') ) ) return Response(video_outline) diff --git a/lms/urls.py b/lms/urls.py index a05fc75eb6..762cf863c5 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -147,7 +147,7 @@ urlpatterns = [ if settings.FEATURES.get('ENABLE_MOBILE_REST_API'): urlpatterns += [ - url(r'^api/mobile/v0.5/', include('mobile_api.urls')), + url(r'^api/mobile/(?Pv(1|0.5))/', include('mobile_api.urls')), ] if settings.FEATURES.get('ENABLE_OPENBADGES'):