Merge pull request #5429 from edx/dcs/mobile-delint

Cleaning up pylint and pep8 errors in mobile API.

lms/djangoapps/mobile_api/__init__.py

@@ -0,0 +1,3 @@
+"""
+Mobile API
+"""

lms/djangoapps/mobile_api/course_info/__init__.py

@@ -0,0 +1,3 @@
+"""
+Course info API
+"""

lms/djangoapps/mobile_api/course_info/models.py

@@ -1 +1,3 @@
-# A models.py is required to make this an app (until we move to Django 1.7)
+"""
+A models.py is required to make this an app (until we move to Django 1.7)
+"""

lms/djangoapps/mobile_api/course_info/tests.py

@@ -4,7 +4,7 @@ Tests for course_info
 from django.test.utils import override_settings
 from django.core.urlresolvers import reverse
 from rest_framework.test import APITestCase
-from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from courseware.tests.factories import UserFactory
 from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
@@ -12,6 +12,9 @@ from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
 
 @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
 class TestVideoOutline(ModuleStoreTestCase, APITestCase):
+    """
+    Tests for /api/mobile/v0.5/course_info/...
+    """
     def setUp(self):
         super(TestVideoOutline, self).setUp()
         self.user = UserFactory.create()
@@ -22,7 +25,7 @@ class TestVideoOutline(ModuleStoreTestCase, APITestCase):
         url = reverse('course-about-detail', kwargs={'course_id': unicode(self.course.id)})
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
-        self.assertTrue('overview' in response.data)
+        self.assertTrue('overview' in response.data)  # pylint: disable=E1103
 
     def test_handouts(self):
         url = reverse('course-handouts-list', kwargs={'course_id': unicode(self.course.id)})
@@ -33,5 +36,5 @@ class TestVideoOutline(ModuleStoreTestCase, APITestCase):
         url = reverse('course-updates-list', kwargs={'course_id': unicode(self.course.id)})
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.data, [])
+        self.assertEqual(response.data, [])  # pylint: disable=E1103
         # TODO: add handouts and updates, somehow

lms/djangoapps/mobile_api/course_info/urls.py

@@ -1,7 +1,8 @@
-from django.conf.urls import patterns, url, include
+"""
+URLs for course_info API
+"""
+from django.conf.urls import patterns, url
 from django.conf import settings
-from rest_framework import routers
-from rest_framework.urlpatterns import format_suffix_patterns
 
 from .views import CourseAboutDetail, CourseUpdatesList, CourseHandoutsList
 
@@ -23,4 +24,3 @@ urlpatterns = patterns(
         name='course-updates-list'
     ),
 )
-

lms/djangoapps/mobile_api/course_info/views.py

@@ -5,15 +5,11 @@ from django.http import Http404
 from rest_framework import generics, permissions
 from rest_framework.authentication import OAuth2Authentication, SessionAuthentication
 from rest_framework.response import Response
-from rest_framework.views import APIView
 
-from courseware.model_data import FieldDataCache
-from courseware.module_render import get_module
 from courseware.courses import get_course_about_section, get_course_info_section_module
 from opaque_keys.edx.keys import CourseKey
 
 from xmodule.modulestore.django import modulestore
-from student.models import CourseEnrollment, User
 
 
 class CourseUpdatesList(generics.ListAPIView):
@@ -55,6 +51,9 @@ class CourseHandoutsList(generics.ListAPIView):
 
 
 class CourseAboutDetail(generics.RetrieveAPIView):
+    """
+    Renders course 'about' page
+    """
     authentication_classes = (OAuth2Authentication, SessionAuthentication)
     permission_classes = (permissions.IsAuthenticated,)
 

lms/djangoapps/mobile_api/models.py

@@ -1 +1,3 @@
-# A models.py is required to make this an app (until we move to Django 1.7)
+"""
+A models.py is required to make this an app (until we move to Django 1.7)
+"""

lms/djangoapps/mobile_api/urls.py

@@ -1,10 +1,13 @@
+"""
+URLs for mobile API
+"""
 from django.conf.urls import patterns, url, include
-from rest_framework import routers
 
 from .users.views import my_user_info
 
 # Additionally, we include login URLs for the browseable API.
-urlpatterns = patterns('',
+urlpatterns = patterns(
+    '',
     url(r'^users/', include('mobile_api.users.urls')),
     url(r'^my_user_info', my_user_info),
     url(r'^video_outlines/', include('mobile_api.video_outlines.urls')),

lms/djangoapps/mobile_api/users/__init__.py

@@ -0,0 +1,3 @@
+"""
+User API
+"""

lms/djangoapps/mobile_api/users/models.py

@@ -1 +1,3 @@
-# A models.py is required to make this an app (until we move to Django 1.7)
+"""
+A models.py is required to make this an app (until we move to Django 1.7)
+"""

lms/djangoapps/mobile_api/users/serializers.py

@@ -1,8 +1,9 @@
+"""
+Serializer for user API
+"""
 from rest_framework import serializers
 from rest_framework.reverse import reverse
 
-from xmodule.modulestore.search import path_to_location
-
 from courseware.courses import course_image_url
 from student.models import CourseEnrollment, User
 
@@ -59,22 +60,28 @@ class CourseField(serializers.RelatedField):
 
 
 class CourseEnrollmentSerializer(serializers.ModelSerializer):
+    """
+    Serializes CourseEnrollment models
+    """
     course = CourseField()
 
-    class Meta:
+    class Meta:  # pylint: disable=C0111
         model = CourseEnrollment
         fields = ('created', 'mode', 'is_active', 'course')
         lookup_field = 'username'
 
 
 class UserSerializer(serializers.HyperlinkedModelSerializer):
+    """
+    Serializes User models
+    """
     name = serializers.Field(source='profile.name')
     course_enrollments = serializers.HyperlinkedIdentityField(
         view_name='courseenrollment-detail',
         lookup_field='username'
     )
 
-    class Meta:
+    class Meta:  # pylint: disable=C0111
         model = User
         fields = ('id', 'username', 'email', 'name', 'course_enrollments')
         lookup_field = 'username'

lms/djangoapps/mobile_api/users/tests.py

@@ -4,7 +4,7 @@ Tests for users API
 from rest_framework.test import APITestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
-from courseware.tests.factories import StaffFactory, UserFactory
+from courseware.tests.factories import UserFactory
 from django.core.urlresolvers import reverse
 from mobile_api.users.serializers import CourseEnrollmentSerializer
 from student.models import CourseEnrollment
@@ -25,7 +25,10 @@ class TestUserApi(ModuleStoreTestCase, APITestCase):
         super(TestUserApi, self).tearDown()
         self.client.logout()
 
-    def enroll(self):
+    def _enroll(self):
+        """
+        enroll test user in test course
+        """
         resp = self.client.post(reverse('change_enrollment'), {
             'enrollment_action': 'enroll',
             'course_id': self.course.id.to_deprecated_string(),
@@ -39,13 +42,13 @@ class TestUserApi(ModuleStoreTestCase, APITestCase):
         self.client.login(username=self.username, password=self.password)
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.data, [])
+        self.assertEqual(response.data, [])  # pylint: disable=E1103
 
-        self.enroll()
+        self._enroll()
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
 
-        courses = response.data
+        courses = response.data  # pylint: disable=E1103
 
         self.assertTrue(len(courses), 1)
         course = courses[0]['course']
@@ -59,7 +62,7 @@ class TestUserApi(ModuleStoreTestCase, APITestCase):
         url = reverse('user-detail', kwargs={'username': self.user.username})
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
-        data = response.data
+        data = response.data  # pylint: disable=E1103
         self.assertEqual(data['username'], self.user.username)
         self.assertEqual(data['email'], self.user.email)
 
@@ -86,7 +89,7 @@ class TestUserApi(ModuleStoreTestCase, APITestCase):
 
     def test_course_serializer(self):
         self.client.login(username=self.username, password=self.password)
-        self.enroll()
+        self._enroll()
-        serialized = CourseEnrollmentSerializer(CourseEnrollment.enrollments_for_user(self.user)[0]).data
+        serialized = CourseEnrollmentSerializer(CourseEnrollment.enrollments_for_user(self.user)[0]).data  # pylint: disable=E1101
         self.assertEqual(serialized['course']['video_outline'], None)
         self.assertEqual(serialized['course']['name'], self.course.display_name)

lms/djangoapps/mobile_api/users/urls.py

@@ -1,11 +1,12 @@
-from django.conf.urls import patterns, url, include
+"""
-
+URLs for user API
-from rest_framework import routers
+"""
-from rest_framework.urlpatterns import format_suffix_patterns
+from django.conf.urls import patterns, url
 
 from .views import UserDetail, UserCourseEnrollmentsList
 
-urlpatterns = patterns('mobile_api.users.views',
+urlpatterns = patterns(
+    'mobile_api.users.views',
     url(r'^(?P<username>[\w.+-]+)$', UserDetail.as_view(), name='user-detail'),
     url(
         r'^(?P<username>[\w.+-]+)/course_enrollments/$',
@@ -13,4 +14,3 @@ urlpatterns = patterns('mobile_api.users.views',
         name='courseenrollment-detail'
     ),
 )
-

lms/djangoapps/mobile_api/users/views.py

@@ -1,22 +1,23 @@
-from django.core.exceptions import PermissionDenied
+"""
+Views for user API
+"""
 from django.shortcuts import redirect
 
 from rest_framework import generics, permissions
 from rest_framework.authentication import OAuth2Authentication, SessionAuthentication
 from rest_framework.decorators import api_view, authentication_classes, permission_classes
-from rest_framework.exceptions import PermissionDenied
 from rest_framework.permissions import IsAuthenticated
-from rest_framework.response import Response
 
 from courseware.access import has_access
-from student.forms import PasswordResetFormNoActive
 from student.models import CourseEnrollment, User
-from xmodule.modulestore.django import modulestore
 
 from .serializers import CourseEnrollmentSerializer, UserSerializer
 
 
 class IsUser(permissions.BasePermission):
+    """
+    Permission that checks to see if the request user matches the User models
+    """
     def has_object_permission(self, request, view, obj):
         return request.user == obj
 
@@ -56,8 +57,12 @@ class UserCourseEnrollmentsList(generics.ListAPIView):
 @authentication_classes((OAuth2Authentication, SessionAuthentication))
 @permission_classes((IsAuthenticated,))
 def my_user_info(request):
+    """
+    Redirect to the currently-logged-in user's info page
+    """
     return redirect("user-detail", username=request.user.username)
 
+
 def mobile_course_enrollments(enrollments, user):
     """
     Return enrollments only if courses are mobile_available (or if the user has staff access)

lms/djangoapps/mobile_api/video_outlines/__init__.py

@@ -0,0 +1,3 @@
+"""
+Video outline API
+"""

lms/djangoapps/mobile_api/video_outlines/models.py

@@ -1 +1,3 @@
-# A models.py is required to make this an app (until we move to Django 1.7)
+"""
+A models.py is required to make this an app (until we move to Django 1.7)
+"""

lms/djangoapps/mobile_api/video_outlines/serializers.py

@@ -1,3 +1,6 @@
+"""
+Serializer for video outline
+"""
 from rest_framework.reverse import reverse
 
 from courseware.access import has_access
@@ -8,19 +11,21 @@ from edxval.api import (
 
 
 class BlockOutline(object):
-
+    """
+    Serializes course videos, pulling data from VAL and the video modules.
+    """
     def __init__(self, course_id, start_block, categories_to_outliner, request):
         """Create a BlockOutline using `start_block` as a starting point."""
         self.start_block = start_block
         self.categories_to_outliner = categories_to_outliner
         self.course_id = course_id
-        self.request = request # needed for making full URLS
+        self.request = request  # needed for making full URLS
         self.local_cache = {}
         try:
             self.local_cache['course_videos'] = get_video_info_for_course_and_profile(
                 unicode(course_id), "mobile_low"
             )
-        except ValInternalError: # pragma: nocover
+        except ValInternalError:  # pragma: nocover
             self.local_cache['course_videos'] = {}
 
     def __iter__(self):
@@ -29,6 +34,7 @@ class BlockOutline(object):
 
         # path should be optional
         def path(block):
+            """path for block"""
             block_path = []
             while block in child_to_parent:
                 block = child_to_parent[block]
@@ -40,6 +46,7 @@ class BlockOutline(object):
             return reversed(block_path)
 
         def find_urls(block):
+            """section and unit urls for block"""
             block_path = []
             while block in child_to_parent:
                 block = child_to_parent[block]
@@ -81,8 +88,8 @@ class BlockOutline(object):
                     continue
 
                 summary_fn = self.categories_to_outliner[curr_block.category]
-                block_path = list(path(block))
+                block_path = list(path(curr_block))
-                unit_url, section_url = find_urls(block)
+                unit_url, section_url = find_urls(curr_block)
                 yield {
                     "path": block_path,
                     "named_path": [b["name"] for b in block_path[:-1]],
@@ -98,6 +105,9 @@ class BlockOutline(object):
 
 
 def video_summary(course, course_id, video_descriptor, request, local_cache):
+    """
+    returns summary dict for the given video module
+    """
     # First try to check VAL for the URLs we want.
     val_video_info = local_cache['course_videos'].get(video_descriptor.edx_video_id, {})
     if val_video_info:

lms/djangoapps/mobile_api/video_outlines/tests.py

@@ -17,8 +17,12 @@ import copy
 TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE)
 TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'] = 'test_xcontent_%s' % uuid4().hex
 
+
 @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE, CONTENTSTORE=TEST_DATA_CONTENTSTORE)
 class TestVideoOutline(ModuleStoreTestCase, APITestCase):
+    """
+    Tests for /api/mobile/v0.5/video_outlines/
+    """
     def setUp(self):
         super(TestVideoOutline, self).setUp()
         self.user = UserFactory.create()
@@ -57,15 +61,16 @@ class TestVideoOutline(ModuleStoreTestCase, APITestCase):
             'extension': 'mp4',
             'width': 1280,
             'height': 720
-            })
+        })
         api.create_profile({
             'profile_name': 'mobile_low',
             'extension': 'mp4',
             'width': 640,
             'height': 480
-            })
+        })
 
-        val_video = api.create_video({
+        # create the video in VAL
+        api.create_video({
             'edx_video_id': self.edx_video_id,
             'client_video_id': u"test video omega \u03a9",
             'duration': 12,
@@ -94,7 +99,7 @@ class TestVideoOutline(ModuleStoreTestCase, APITestCase):
             sub=subid
         )
 
-        result_location = transcripts_utils.save_subs_to_store({
+        transcripts_utils.save_subs_to_store({
             'start': [100, 200, 240, 390, 1000],
             'end': [200, 240, 380, 1000, 1500],
             'text': [
@@ -116,20 +121,20 @@ class TestVideoOutline(ModuleStoreTestCase, APITestCase):
         self.assertEqual(response.status_code, 403)
 
     def test_course_list(self):
-        second_video = ItemFactory.create(
+        ItemFactory.create(
             parent_location=self.other_unit.location,
             category="video",
             display_name=u"test video omega 2 \u03a9",
             html5_sources=[self.html5_video_url]
         )
-        third_video = ItemFactory.create(
+        ItemFactory.create(
             parent_location=self.other_unit.location,
             category="video",
             display_name=u"test video omega 3 \u03a9",
             source=self.html5_video_url
         )
 
-        draft_video = ItemFactory.create(
+        ItemFactory.create(
             parent_location=self.unit.location,
             category="video",
             edx_video_id=self.edx_video_id,
@@ -141,7 +146,7 @@ class TestVideoOutline(ModuleStoreTestCase, APITestCase):
         response = self.client.get(url)
 
         self.assertEqual(response.status_code, 200)
-        course_outline = response.data
+        course_outline = response.data  # pylint: disable=E1103
         self.assertEqual(len(course_outline), 3)
         vid = course_outline[0]
         self.assertTrue('test_subsection_omega_%CE%A9' in vid['section_url'])
@@ -161,7 +166,7 @@ class TestVideoOutline(ModuleStoreTestCase, APITestCase):
             'course_id': unicode(self.course.id),
             'block_id': unicode(self.video.scope_ids.usage_id.block_id),
             'lang': 'pl'
-            }
+        }
         url = reverse('video-transcripts-detail', kwargs=kwargs)
         response = self.client.get(url)
         self.assertEqual(response.status_code, 404)

lms/djangoapps/mobile_api/video_outlines/urls.py

@@ -1,11 +1,13 @@
-from django.conf.urls import patterns, url, include
+"""
+URLs for video outline API
+"""
+from django.conf.urls import patterns, url
 from django.conf import settings
-from rest_framework import routers
-from rest_framework.urlpatterns import format_suffix_patterns
 
 from .views import VideoSummaryList, VideoTranscripts
 
-urlpatterns = patterns('mobile_api.video_outlines.views',
+urlpatterns = patterns(
+    'mobile_api.video_outlines.views',
     url(
         r'^courses/{}$'.format(settings.COURSE_ID_PATTERN),
         VideoSummaryList.as_view(),
@@ -17,4 +19,3 @@ urlpatterns = patterns('mobile_api.video_outlines.views',
         name='video-transcripts-detail'
     ),
 )
-

lms/djangoapps/mobile_api/video_outlines/views.py

@@ -8,19 +8,16 @@ general XBlock representation in this rather specialized formatting.
 """
 from functools import partial
 
-from django.core.cache import cache
 from django.http import Http404, HttpResponse
 
 from rest_framework import generics, permissions
 from rest_framework.authentication import OAuth2Authentication, SessionAuthentication
 from rest_framework.response import Response
-from rest_framework.views import APIView
 from rest_framework.exceptions import PermissionDenied
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locator import BlockUsageLocator
 
 from courseware.access import has_access
-from student.models import CourseEnrollment, User
 from xmodule.exceptions import NotFoundError
 from xmodule.modulestore.django import modulestore
 
@@ -38,8 +35,8 @@ class VideoSummaryList(generics.ListAPIView):
 
         video_outline = list(
             BlockOutline(
-                course_id, 
+                course_id,
-                course, 
+                course,
                 {"video": partial(video_summary, course)},
                 request,
             )