From 9127075bcf5e96ecea5ef85ca7ca37b06cbeee5c Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Tue, 28 May 2019 11:57:15 -0400 Subject: [PATCH] Fix Pylint for enrollments --- openedx/core/djangoapps/enrollments/api.py | 2 +- openedx/core/djangoapps/enrollments/data.py | 1 + openedx/core/djangoapps/enrollments/forms.py | 4 +- .../tests/test_enroll_user_in_course.py | 2 +- .../djangoapps/enrollments/serializers.py | 4 +- .../djangoapps/enrollments/tests/test_api.py | 4 +- .../djangoapps/enrollments/tests/test_data.py | 4 +- .../enrollments/tests/test_views.py | 38 ++++++++--- openedx/core/djangoapps/enrollments/views.py | 65 +++++++++++-------- 9 files changed, 79 insertions(+), 45 deletions(-) diff --git a/openedx/core/djangoapps/enrollments/api.py b/openedx/core/djangoapps/enrollments/api.py index c8a284f195..c3b68f2701 100644 --- a/openedx/core/djangoapps/enrollments/api.py +++ b/openedx/core/djangoapps/enrollments/api.py @@ -324,7 +324,7 @@ def get_course_enrollment_details(course_id, include_expired=False): cached_enrollment_data = None try: cached_enrollment_data = cache.get(cache_key) - except Exception: + except Exception: # pylint: disable=broad-except # The cache backend could raise an exception (for example, memcache keys that contain spaces) log.exception(u"Error occurred while retrieving course enrollment details from the cache") diff --git a/openedx/core/djangoapps/enrollments/data.py b/openedx/core/djangoapps/enrollments/data.py index 4c186be81f..c8021c6777 100644 --- a/openedx/core/djangoapps/enrollments/data.py +++ b/openedx/core/djangoapps/enrollments/data.py @@ -348,6 +348,7 @@ def get_user_roles(user_id): :param user_id: The id of the selected user. :return: All roles for all courses that this user has. """ + # pylint: disable=protected-access user = _get_user(user_id) if not hasattr(user, '_roles'): user._roles = RoleCache(user) diff --git a/openedx/core/djangoapps/enrollments/forms.py b/openedx/core/djangoapps/enrollments/forms.py index 7a52c9e477..d2530ae868 100644 --- a/openedx/core/djangoapps/enrollments/forms.py +++ b/openedx/core/djangoapps/enrollments/forms.py @@ -28,7 +28,7 @@ class CourseEnrollmentsApiListForm(Form): try: return CourseKey.from_string(course_id) except InvalidKeyError: - raise ValidationError("'{}' is not a valid course id.".format(course_id)) + raise ValidationError(u"'{}' is not a valid course id.".format(course_id)) return course_id def clean_username(self): @@ -40,7 +40,7 @@ class CourseEnrollmentsApiListForm(Form): usernames = usernames_csv_string.split(',') if len(usernames) > self.MAX_USERNAME_COUNT: raise ValidationError( - "Too many usernames in a single request - {}. A maximum of {} is allowed".format( + u"Too many usernames in a single request - {}. A maximum of {} is allowed".format( len(usernames), self.MAX_USERNAME_COUNT, ) diff --git a/openedx/core/djangoapps/enrollments/management/tests/test_enroll_user_in_course.py b/openedx/core/djangoapps/enrollments/management/tests/test_enroll_user_in_course.py index e78c190bc5..18c204eee2 100644 --- a/openedx/core/djangoapps/enrollments/management/tests/test_enroll_user_in_course.py +++ b/openedx/core/djangoapps/enrollments/management/tests/test_enroll_user_in_course.py @@ -1,9 +1,9 @@ """ Test the change_enrollment command line script.""" from __future__ import absolute_import -import ddt import unittest from uuid import uuid4 +import ddt from django.conf import settings from django.core.management import call_command diff --git a/openedx/core/djangoapps/enrollments/serializers.py b/openedx/core/djangoapps/enrollments/serializers.py index 7018bcce63..5e946527f9 100644 --- a/openedx/core/djangoapps/enrollments/serializers.py +++ b/openedx/core/djangoapps/enrollments/serializers.py @@ -20,7 +20,7 @@ class StringListField(serializers.CharField): [1,2,3] """ - def field_to_native(self, obj, field_name): + def field_to_native(self, obj, field_name): # pylint: disable=unused-argument """ Serialize the object's class name. """ @@ -99,7 +99,7 @@ class CourseEnrollmentsApiListSerializer(CourseEnrollmentSerializer): fields = CourseEnrollmentSerializer.Meta.fields + ('course_id', ) -class ModeSerializer(serializers.Serializer): +class ModeSerializer(serializers.Serializer): # pylint: disable=abstract-method """Serializes a course's 'Mode' tuples Returns a serialized representation of the modes available for course enrollment. The course diff --git a/openedx/core/djangoapps/enrollments/tests/test_api.py b/openedx/core/djangoapps/enrollments/tests/test_api.py index ef52a0ad9c..50de1b6459 100644 --- a/openedx/core/djangoapps/enrollments/tests/test_api.py +++ b/openedx/core/djangoapps/enrollments/tests/test_api.py @@ -13,7 +13,9 @@ from mock import Mock, patch from course_modes.models import CourseMode from openedx.core.djangoapps.enrollments import api -from openedx.core.djangoapps.enrollments.errors import CourseModeNotFoundError, EnrollmentApiLoadError, EnrollmentNotFoundError +from openedx.core.djangoapps.enrollments.errors import ( + CourseModeNotFoundError, EnrollmentApiLoadError, EnrollmentNotFoundError, +) from openedx.core.djangoapps.enrollments.tests import fake_data_api from openedx.core.djangolib.testing.utils import CacheIsolationTestCase diff --git a/openedx/core/djangoapps/enrollments/tests/test_data.py b/openedx/core/djangoapps/enrollments/tests/test_data.py index 445d5c4896..bc68f2f95f 100644 --- a/openedx/core/djangoapps/enrollments/tests/test_data.py +++ b/openedx/core/djangoapps/enrollments/tests/test_data.py @@ -385,7 +385,9 @@ class EnrollmentDataTest(ModuleStoreTestCase): def test_get_roles(self): """Create a role for a user, then get it""" - expected_role = CourseAccessRoleFactory.create(course_id=self.course.id, user=self.user, role="SuperCoolTestRole") + expected_role = CourseAccessRoleFactory.create( + course_id=self.course.id, user=self.user, role="SuperCoolTestRole", + ) roles = data.get_user_roles(self.user.username) self.assertEqual(roles, {expected_role}) diff --git a/openedx/core/djangoapps/enrollments/tests/test_views.py b/openedx/core/djangoapps/enrollments/tests/test_views.py index 1792002fe4..85dbfbad1d 100644 --- a/openedx/core/djangoapps/enrollments/tests/test_views.py +++ b/openedx/core/djangoapps/enrollments/tests/test_views.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring,redefined-outer-name """ Tests for user enrollment. """ @@ -237,7 +238,10 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase, Ente # Create an enrollment self.assert_enrollment_status() resp = self.client.get( - reverse('courseenrollment', kwargs={'username': self.user.username, "course_id": six.text_type(self.course.id)}) + reverse( + 'courseenrollment', + kwargs={'username': self.user.username, "course_id": six.text_type(self.course.id)}, + ) ) self.assertEqual(resp.status_code, status.HTTP_200_OK) data = json.loads(resp.content) @@ -473,7 +477,12 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase, Ente (None, None, None, None), (datetime.datetime(2015, 1, 2, 3, 4, 5, tzinfo=pytz.UTC), None, "2015-01-02T03:04:05Z", None), (None, datetime.datetime(2015, 1, 2, 3, 4, 5, tzinfo=pytz.UTC), None, "2015-01-02T03:04:05Z"), - (datetime.datetime(2014, 6, 7, 8, 9, 10, tzinfo=pytz.UTC), datetime.datetime(2015, 1, 2, 3, 4, 5, tzinfo=pytz.UTC), "2014-06-07T08:09:10Z", "2015-01-02T03:04:05Z"), + ( + datetime.datetime(2014, 6, 7, 8, 9, 10, tzinfo=pytz.UTC), + datetime.datetime(2015, 1, 2, 3, 4, 5, tzinfo=pytz.UTC), + "2014-06-07T08:09:10Z", + "2015-01-02T03:04:05Z", + ), ) @ddt.unpack def test_get_course_details_course_dates(self, start_datetime, end_datetime, expected_start, expected_end): @@ -528,7 +537,10 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase, Ente def test_get_enrollment_internal_error(self, mock_get_enrollment): mock_get_enrollment.side_effect = CourseEnrollmentError("Something bad happened.") resp = self.client.get( - reverse('courseenrollment', kwargs={'username': self.user.username, "course_id": six.text_type(self.course.id)}) + reverse( + 'courseenrollment', + kwargs={'username': self.user.username, "course_id": six.text_type(self.course.id)}, + ) ) self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) @@ -576,7 +588,7 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase, Ente try: throttle.parse_rate(throttle.get_rate()) except ImproperlyConfigured: - self.fail("No throttle rate set for {}".format(user_scope)) + self.fail(u"No throttle rate set for {}".format(user_scope)) def test_create_enrollment_with_cohort(self): """Enroll in the course, and also add to a cohort.""" @@ -588,7 +600,7 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase, Ente self.assert_enrollment_status(cohort=cohort_name) self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course.id)) - course_mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id) + _, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id) self.assertTrue(is_active) self.assertEqual(cohorts.get_cohort(self.user, self.course.id, assign=False).name, cohort_name) @@ -636,7 +648,11 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase, Ente # Passes the include_expired parameter to the API call v_response = self.client.get( - reverse('courseenrollmentdetails', kwargs={"course_id": six.text_type(self.course.id)}), {'include_expired': True} + reverse( + 'courseenrollmentdetails', + kwargs={"course_id": six.text_type(self.course.id)} + ), + {'include_expired': True}, ) v_data = json.loads(v_response.content) @@ -644,7 +660,9 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase, Ente self.assertEqual(len(v_data['course_modes']), 2) # Omits the include_expired parameter from the API call - h_response = self.client.get(reverse('courseenrollmentdetails', kwargs={"course_id": six.text_type(self.course.id)})) + h_response = self.client.get( + reverse('courseenrollmentdetails', kwargs={"course_id": six.text_type(self.course.id)}), + ) h_data = json.loads(h_response.content) # Ensure that only one course mode is returned and that it is honor @@ -1236,8 +1254,8 @@ class EnrollmentCrossDomainTest(ModuleStoreTestCase): }) resp = self.client.get(url, HTTP_REFERER=self.REFERER) self.assertEqual(resp.status_code, 200) - self.assertIn('prod-edx-csrftoken', resp.cookies) # pylint: disable=no-member - return resp.cookies['prod-edx-csrftoken'].value # pylint: disable=no-member + self.assertIn('prod-edx-csrftoken', resp.cookies) + return resp.cookies['prod-edx-csrftoken'].value def _cross_domain_post(self, csrf_cookie): """Perform a cross-domain POST request. """ @@ -1306,7 +1324,7 @@ class UnenrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase): """ Helper method to create a RetirementStatus with useful defaults """ - pending_state = RetirementState.objects.create( + RetirementState.objects.create( state_name='PENDING', state_execution_order=1, is_dead_end_state=False, diff --git a/openedx/core/djangoapps/enrollments/views.py b/openedx/core/djangoapps/enrollments/views.py index 985a55dad9..a94ee62e77 100644 --- a/openedx/core/djangoapps/enrollments/views.py +++ b/openedx/core/djangoapps/enrollments/views.py @@ -21,7 +21,9 @@ from openedx.core.djangoapps.cors_csrf.decorators import ensure_csrf_cookie_cros from openedx.core.djangoapps.course_groups.cohorts import CourseUserGroup, add_user_to_cohort, get_cohort_by_name from openedx.core.djangoapps.embargo import api as embargo_api from openedx.core.djangoapps.enrollments import api -from openedx.core.djangoapps.enrollments.errors import CourseEnrollmentError, CourseEnrollmentExistsError, CourseModeNotFoundError +from openedx.core.djangoapps.enrollments.errors import ( + CourseEnrollmentError, CourseEnrollmentExistsError, CourseModeNotFoundError, +) from openedx.core.djangoapps.enrollments.forms import CourseEnrollmentsApiListForm from openedx.core.djangoapps.enrollments.paginators import CourseEnrollmentsApiListPagination from openedx.core.djangoapps.enrollments.serializers import CourseEnrollmentsApiListSerializer @@ -164,10 +166,13 @@ class EnrollmentView(APIView, ApiKeyPermissionMixIn): * user: The ID of the user. """ - authentication_classes = (JwtAuthentication, OAuth2AuthenticationAllowInactiveUser, - SessionAuthenticationAllowInactiveUser,) - permission_classes = ApiKeyHeaderPermissionIsAuthenticated, - throttle_classes = EnrollmentUserThrottle, + authentication_classes = ( + JwtAuthentication, + OAuth2AuthenticationAllowInactiveUser, + SessionAuthenticationAllowInactiveUser, + ) + permission_classes = (ApiKeyHeaderPermissionIsAuthenticated,) + throttle_classes = (EnrollmentUserThrottle,) # Since the course about page on the marketing site uses this API to auto-enroll users, # we need to support cross-domain CSRF. @@ -236,11 +241,13 @@ class EnrollmentUserRolesView(APIView): logged-in user, filtered by course_id if given, along with whether or not the user is global staff """ - authentication_classes = (JwtAuthentication, - OAuth2AuthenticationAllowInactiveUser, - EnrollmentCrossDomainSessionAuth) - permission_classes = ApiKeyHeaderPermissionIsAuthenticated, - throttle_classes = EnrollmentUserThrottle, + authentication_classes = ( + JwtAuthentication, + OAuth2AuthenticationAllowInactiveUser, + EnrollmentCrossDomainSessionAuth, + ) + permission_classes = (ApiKeyHeaderPermissionIsAuthenticated,) + throttle_classes = (EnrollmentUserThrottle,) @method_decorator(ensure_csrf_cookie_cross_domain) def get(self, request): @@ -252,7 +259,7 @@ class EnrollmentUserRolesView(APIView): roles_data = api.get_user_roles(request.user.username) if course_id: roles_data = [role for role in roles_data if text_type(role.course_id) == course_id] - except Exception: + except Exception: # pylint: disable=broad-except return Response( status=status.HTTP_400_BAD_REQUEST, data={ @@ -339,7 +346,7 @@ class EnrollmentCourseDetailView(APIView): authentication_classes = [] permission_classes = [] - throttle_classes = EnrollmentUserThrottle, + throttle_classes = (EnrollmentUserThrottle,) def get(self, request, course_id=None): """Read enrollment information for a particular course. @@ -405,7 +412,7 @@ class UnenrollmentView(APIView): returned along with a list of all courses from which the user was unenrolled. """ authentication_classes = (JwtAuthentication,) - permission_classes = (permissions.IsAuthenticated, CanRetireUser) + permission_classes = (permissions.IsAuthenticated, CanRetireUser,) def post(self, request): """ @@ -603,10 +610,13 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): * user: The username of the user. """ - authentication_classes = (JwtAuthentication, OAuth2AuthenticationAllowInactiveUser, - EnrollmentCrossDomainSessionAuth,) - permission_classes = ApiKeyHeaderPermissionIsAuthenticated, - throttle_classes = EnrollmentUserThrottle, + authentication_classes = ( + JwtAuthentication, + OAuth2AuthenticationAllowInactiveUser, + EnrollmentCrossDomainSessionAuth, + ) + permission_classes = (ApiKeyHeaderPermissionIsAuthenticated,) + throttle_classes = (EnrollmentUserThrottle,) # Since the course about page on the marketing site # uses this API to auto-enroll users, we need to support @@ -652,6 +662,7 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): return Response(filtered_data) def post(self, request): + # pylint: disable=too-many-statements """Enrolls the currently logged-in user in a course. Server-to-server calls may deactivate or modify the mode of existing enrollments. All other requests @@ -734,7 +745,7 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): try: enterprise_api_client.post_enterprise_course_enrollment(username, text_type(course_id), None) except EnterpriseApiException as error: - log.exception("An unexpected error occurred while creating the new EnterpriseCourseEnrollment " + log.exception(u"An unexpected error occurred while creating the new EnterpriseCourseEnrollment " "for user [%s] in course run [%s]", username, course_id) raise CourseEnrollmentError(text_type(error)) kwargs = { @@ -766,7 +777,7 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): log.warning(msg) return Response(status=status.HTTP_400_BAD_REQUEST, data={"message": msg}) - if len(missing_attrs) > 0: + if missing_attrs: msg = u"Missing enrollment attributes: requested mode={} required attributes={}".format( mode, REQUIRED_ATTRIBUTES.get(mode) ) @@ -805,7 +816,7 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): org = course_id.org update_email_opt_in(request.user, org, email_opt_in) - log.info('The user [%s] has already been enrolled in course run [%s].', username, course_id) + log.info(u'The user [%s] has already been enrolled in course run [%s].', username, course_id) return Response(response) except CourseModeNotFoundError as error: return Response( @@ -824,11 +835,11 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): } ) except CourseEnrollmentExistsError as error: - log.warning('An enrollment already exists for user [%s] in course run [%s].', username, course_id) + log.warning(u'An enrollment already exists for user [%s] in course run [%s].', username, course_id) return Response(data=error.enrollment) except CourseEnrollmentError: - log.exception("An error occurred while creating the new course enrollment for user " - "[%s] in course run [%s]", username, course_id) + log.exception(u"An error occurred while creating the new course enrollment for user " + u"[%s] in course run [%s]", username, course_id) return Response( status=status.HTTP_400_BAD_REQUEST, data={ @@ -839,11 +850,11 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): } ) except CourseUserGroup.DoesNotExist: - log.exception('Missing cohort [%s] in course run [%s]', cohort_name, course_id) + log.exception(u'Missing cohort [%s] in course run [%s]', cohort_name, course_id) return Response( status=status.HTTP_400_BAD_REQUEST, data={ - "message": "An error occured while adding to cohort [%s]" % cohort_name + "message": u"An error occured while adding to cohort [%s]" % cohort_name }) finally: # Assumes that the ecommerce service uses an API key to authenticate. @@ -930,8 +941,8 @@ class CourseEnrollmentsApiListView(DeveloperErrorViewMixin, ListAPIView): OAuth2AuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser, ) - permission_classes = (permissions.IsAdminUser, ) - throttle_classes = (EnrollmentUserThrottle, ) + permission_classes = (permissions.IsAdminUser,) + throttle_classes = (EnrollmentUserThrottle,) serializer_class = CourseEnrollmentsApiListSerializer pagination_class = CourseEnrollmentsApiListPagination