From 8d02efb02145928bdfe31abb446fe75f7a2658d2 Mon Sep 17 00:00:00 2001 From: Renzo Lucioni <renzo@renzolucioni.com> Date: Mon, 10 Nov 2014 13:25:04 -0500 Subject: [PATCH] Clean up pep8 and pylint violations Also fixes failing Python unit tests --- common/djangoapps/enrollment/api.py | 19 +-- common/djangoapps/enrollment/data.py | 9 +- common/djangoapps/enrollment/serializers.py | 3 +- .../enrollment/tests/fake_data_api.py | 10 +- .../djangoapps/enrollment/tests/test_data.py | 3 +- .../djangoapps/enrollment/tests/test_views.py | 3 +- common/djangoapps/enrollment/views.py | 6 +- .../djangoapps/third_party_auth/pipeline.py | 12 +- common/djangoapps/user_api/api/account.py | 1 - common/djangoapps/user_api/api/course_tag.py | 7 +- common/djangoapps/user_api/helpers.py | 6 +- .../user_api/tests/test_constants.py | 2 +- .../djangoapps/user_api/tests/test_helpers.py | 16 +-- .../djangoapps/user_api/tests/test_views.py | 57 ++++++-- common/djangoapps/user_api/urls.py | 1 + common/djangoapps/user_api/views.py | 130 ++++++++++++++++-- common/djangoapps/util/testing.py | 1 + lms/djangoapps/student_account/helpers.py | 4 +- 18 files changed, 222 insertions(+), 68 deletions(-) diff --git a/common/djangoapps/enrollment/api.py b/common/djangoapps/enrollment/api.py index ea76e032a88..7a66d3b3359 100644 --- a/common/djangoapps/enrollment/api.py +++ b/common/djangoapps/enrollment/api.py @@ -11,33 +11,36 @@ log = logging.getLogger(__name__) class CourseEnrollmentError(Exception): - """ Generic Course Enrollment Error. + """Generic Course Enrollment Error. Describes any error that may occur when reading or updating enrollment information for a student or a course. """ def __init__(self, msg, data=None): - super(Exception, self).__init__(msg) + super(CourseEnrollmentError, self).__init__(msg) # Corresponding information to help resolve the error. self.data = data class CourseModeNotFoundError(CourseEnrollmentError): + """The requested course mode could not be found.""" pass class EnrollmentNotFoundError(CourseEnrollmentError): + """The requested enrollment could not be found.""" pass class EnrollmentApiLoadError(CourseEnrollmentError): + """The data API could not be loaded.""" pass DEFAULT_DATA_API = 'enrollment.data' def get_enrollments(student_id): - """ Retrieves all the courses a student is enrolled in. + """Retrieves all the courses a student is enrolled in. Takes a student and retrieves all relative enrollments. Includes information regarding how the student is enrolled in the the course. @@ -107,7 +110,7 @@ def get_enrollments(student_id): def get_enrollment(student_id, course_id): - """ Retrieves all enrollment information for the student in respect to a specific course. + """Retrieves all enrollment information for the student in respect to a specific course. Gets all the course enrollment information specific to a student in a course. @@ -151,7 +154,7 @@ def get_enrollment(student_id, course_id): def add_enrollment(student_id, course_id, mode='honor', is_active=True): - """ Enrolls a student in a course. + """Enrolls a student in a course. Enrolls a student in a course. If the mode is not specified, this will default to 'honor'. @@ -199,7 +202,7 @@ def add_enrollment(student_id, course_id, mode='honor', is_active=True): def deactivate_enrollment(student_id, course_id): - """ Un-enrolls a student in a course + """Un-enrolls a student in a course Deactivate the enrollment of a student in a course. We will not remove the enrollment data, but simply flag it as inactive. @@ -249,7 +252,7 @@ def deactivate_enrollment(student_id, course_id): def update_enrollment(student_id, course_id, mode): - """ Updates the course mode for the enrolled user. + """Updates the course mode for the enrolled user. Update a course enrollment for the given student and course. @@ -295,7 +298,7 @@ def update_enrollment(student_id, course_id, mode): def get_course_enrollment_details(course_id): - """ Get the course modes for course. Also get enrollment start and end date, invite only, etc. + """Get the course modes for course. Also get enrollment start and end date, invite only, etc. Given a course_id, return a serializable dictionary of properties describing course enrollment information. diff --git a/common/djangoapps/enrollment/data.py b/common/djangoapps/enrollment/data.py index 82db6734fac..fe1e07552f2 100644 --- a/common/djangoapps/enrollment/data.py +++ b/common/djangoapps/enrollment/data.py @@ -7,7 +7,6 @@ import logging from django.contrib.auth.models import User from opaque_keys.edx.keys import CourseKey from xmodule.modulestore.django import modulestore -from xmodule.modulestore.exceptions import ItemNotFoundError from enrollment.serializers import CourseEnrollmentSerializer, CourseField from student.models import CourseEnrollment, NonExistentCourseError @@ -17,7 +16,7 @@ log = logging.getLogger(__name__) def get_course_enrollments(student_id): """Retrieve a list representing all aggregated data for a student's course enrollments. - Construct a representation of all course enrollment data for a specific student.. + Construct a representation of all course enrollment data for a specific student. Args: student_id (str): The name of the student to retrieve course enrollment information for. @@ -29,7 +28,7 @@ def get_course_enrollments(student_id): qset = CourseEnrollment.objects.filter( user__username=student_id, is_active=True ).order_by('created') - return CourseEnrollmentSerializer(qset).data + return CourseEnrollmentSerializer(qset).data # pylint: disable=no-member def get_course_enrollment(student_id, course_id): @@ -50,7 +49,7 @@ def get_course_enrollment(student_id, course_id): enrollment = CourseEnrollment.objects.get( user__username=student_id, course_id=course_key ) - return CourseEnrollmentSerializer(enrollment).data + return CourseEnrollmentSerializer(enrollment).data # pylint: disable=no-member except CourseEnrollment.DoesNotExist: return None @@ -79,7 +78,7 @@ def update_course_enrollment(student_id, course_id, mode=None, is_active=None): enrollment.update_enrollment(is_active=is_active, mode=mode) enrollment.save() - return CourseEnrollmentSerializer(enrollment).data + return CourseEnrollmentSerializer(enrollment).data # pylint: disable=no-member def get_course_enrollment_info(course_id): diff --git a/common/djangoapps/enrollment/serializers.py b/common/djangoapps/enrollment/serializers.py index d01deb0023d..5306f228cd5 100644 --- a/common/djangoapps/enrollment/serializers.py +++ b/common/djangoapps/enrollment/serializers.py @@ -3,7 +3,6 @@ Serializers for all Course Enrollment related return objects. """ from rest_framework import serializers -from rest_framework.fields import Field from student.models import CourseEnrollment from course_modes.models import CourseMode @@ -36,7 +35,7 @@ class CourseField(serializers.RelatedField): def to_native(self, course): course_id = unicode(course.id) - course_modes = ModeSerializer(CourseMode.modes_for_course(course.id)).data + course_modes = ModeSerializer(CourseMode.modes_for_course(course.id)).data # pylint: disable=no-member return { "course_id": course_id, diff --git a/common/djangoapps/enrollment/tests/fake_data_api.py b/common/djangoapps/enrollment/tests/fake_data_api.py index d1374c3fefb..80140aaec53 100644 --- a/common/djangoapps/enrollment/tests/fake_data_api.py +++ b/common/djangoapps/enrollment/tests/fake_data_api.py @@ -20,6 +20,7 @@ _ENROLLMENTS = [] _COURSES = [] +# pylint: disable=unused-argument def get_course_enrollments(student_id): """Stubbed out Enrollment data request.""" return _ENROLLMENTS @@ -48,18 +49,21 @@ def get_course_enrollment_info(course_id): def _get_fake_enrollment(student_id, course_id): + """Get an enrollment from the enrollments array.""" for enrollment in _ENROLLMENTS: if student_id == enrollment['student'] and course_id == enrollment['course']['course_id']: return enrollment def _get_fake_course_info(course_id): + """Get a course from the courses array.""" for course in _COURSES: if course_id == course['course_id']: return course def add_enrollment(student_id, course_id, is_active=True, mode='honor'): + """Append an enrollment to the enrollments array.""" enrollment = { "created": datetime.datetime.now(), "mode": mode, @@ -72,6 +76,7 @@ def add_enrollment(student_id, course_id, is_active=True, mode='honor'): def add_course(course_id, enrollment_start=None, enrollment_end=None, invite_only=False, course_modes=None): + """Append course to the courses array.""" course_info = { "course_id": course_id, "enrollment_end": enrollment_end, @@ -90,7 +95,8 @@ def add_course(course_id, enrollment_start=None, enrollment_end=None, invite_onl def reset(): - global _COURSES + """Set the enrollments and courses arrays to be empty.""" + global _COURSES # pylint: disable=global-statement _COURSES = [] - global _ENROLLMENTS + global _ENROLLMENTS # pylint: disable=global-statement _ENROLLMENTS = [] diff --git a/common/djangoapps/enrollment/tests/test_data.py b/common/djangoapps/enrollment/tests/test_data.py index 9655ecb00da..f069f68bcc5 100644 --- a/common/djangoapps/enrollment/tests/test_data.py +++ b/common/djangoapps/enrollment/tests/test_data.py @@ -34,7 +34,7 @@ class EnrollmentDataTest(ModuleStoreTestCase): PASSWORD = "edx" def setUp(self): - """ Create a course and user, then log in. """ + """Create a course and user, then log in. """ super(EnrollmentDataTest, self).setUp() self.course = CourseFactory.create() self.user = UserFactory.create(username=self.USERNAME, email=self.EMAIL, password=self.PASSWORD) @@ -163,6 +163,7 @@ class EnrollmentDataTest(ModuleStoreTestCase): data.get_course_enrollment_info("this/is/bananas") def _create_course_modes(self, course_modes, course=None): + """Create the course modes required for a test. """ course_id = course.id if course else self.course.id for mode_slug in course_modes: CourseModeFactory.create( diff --git a/common/djangoapps/enrollment/tests/test_views.py b/common/djangoapps/enrollment/tests/test_views.py index 0b4adbc7386..06e55a6d02a 100644 --- a/common/djangoapps/enrollment/tests/test_views.py +++ b/common/djangoapps/enrollment/tests/test_views.py @@ -60,7 +60,7 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): mode_display_name=mode_slug, ) - # Enroll in the course and verify the URL we get sent to + # Create an enrollment self._create_enrollment() self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course.id)) @@ -159,6 +159,7 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) def _create_enrollment(self): + """Enroll in the course and verify the URL we are sent to. """ resp = self.client.post(reverse('courseenrollment', kwargs={'course_id': (unicode(self.course.id))})) self.assertEqual(resp.status_code, status.HTTP_200_OK) data = json.loads(resp.content) diff --git a/common/djangoapps/enrollment/views.py b/common/djangoapps/enrollment/views.py index 35a3fc603df..c3970cfe580 100644 --- a/common/djangoapps/enrollment/views.py +++ b/common/djangoapps/enrollment/views.py @@ -14,7 +14,9 @@ from student.models import NonExistentCourseError, CourseEnrollmentException class EnrollmentUserThrottle(UserRateThrottle): - rate = '50/second' # TODO Limit significantly after performance testing. + """Limit the number of requests users can make to the enrollment API.""" + # TODO Limit significantly after performance testing. # pylint: disable=fixme + rate = '50/second' class SessionAuthenticationAllowInactiveUser(SessionAuthentication): @@ -48,7 +50,7 @@ class SessionAuthenticationAllowInactiveUser(SessionAuthentication): """ # Get the underlying HttpRequest object - request = request._request + request = request._request # pylint: disable=protected-access user = getattr(request, 'user', None) # Unauthenticated, CSRF validation not required diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 4114b5ee367..6fb7951fb56 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -111,7 +111,8 @@ AUTH_ENTRY_LOGIN = 'login' AUTH_ENTRY_PROFILE = 'profile' AUTH_ENTRY_REGISTER = 'register' -# TODO (ECOM-369): Repace `AUTH_ENTRY_LOGIN` and `AUTH_ENTRY_REGISTER` +# pylint: disable=fixme +# TODO (ECOM-369): Replace `AUTH_ENTRY_LOGIN` and `AUTH_ENTRY_REGISTER` # with these values once the A/B test completes, then delete # these constants. AUTH_ENTRY_LOGIN_2 = 'account_login' @@ -153,6 +154,7 @@ _AUTH_ENTRY_CHOICES = frozenset([ # login/registration, we needed to introduce two # additional end-points. Once the test completes, # delete these constants from the choices list. + # pylint: disable=fixme AUTH_ENTRY_LOGIN_2, AUTH_ENTRY_REGISTER_2, @@ -445,6 +447,7 @@ def parse_query_params(strategy, response, *args, **kwargs): # TODO (ECOM-369): Delete these once the A/B test # for the combined login/registration form completes. + # pylint: disable=fixme 'is_login_2': auth_entry == AUTH_ENTRY_LOGIN_2, 'is_register_2': auth_entry == AUTH_ENTRY_REGISTER_2, } @@ -457,6 +460,7 @@ def parse_query_params(strategy, response, *args, **kwargs): # these kwargs in `redirect_to_supplementary_form`, but # these should redirect to the same location as "is_login" and "is_register" # (whichever login/registration end-points win in the test). +# pylint: disable=fixme @partial.partial def ensure_user_information( strategy, @@ -500,7 +504,7 @@ def ensure_user_information( return HttpResponseBadRequest() # TODO (ECOM-369): Consolidate this with `dispatch_to_login` - # once the A/B test completes. + # once the A/B test completes. # pylint: disable=fixme dispatch_to_login_2 = is_login_2 and (user_unset or user_inactive) if is_dashboard or is_profile: @@ -510,7 +514,7 @@ def ensure_user_information( return redirect(AUTH_DISPATCH_URLS[AUTH_ENTRY_LOGIN], name='signin_user') # TODO (ECOM-369): Consolidate this with `dispatch_to_login` - # once the A/B test completes. + # once the A/B test completes. # pylint: disable=fixme if dispatch_to_login_2: return redirect(AUTH_DISPATCH_URLS[AUTH_ENTRY_LOGIN_2]) @@ -518,7 +522,7 @@ def ensure_user_information( return redirect(AUTH_DISPATCH_URLS[AUTH_ENTRY_REGISTER], name='register_user') # TODO (ECOM-369): Consolidate this with `is_register` - # once the A/B test completes. + # once the A/B test completes. # pylint: disable=fixme if is_register_2 and user_unset: return redirect(AUTH_DISPATCH_URLS[AUTH_ENTRY_REGISTER_2]) diff --git a/common/djangoapps/user_api/api/account.py b/common/djangoapps/user_api/api/account.py index 06c9c5b035a..118838c7b46 100644 --- a/common/djangoapps/user_api/api/account.py +++ b/common/djangoapps/user_api/api/account.py @@ -8,7 +8,6 @@ information and preferences). """ from django.conf import settings from django.db import transaction, IntegrityError -from django.db.models import Q from django.core.validators import validate_email, validate_slug, ValidationError from django.contrib.auth.forms import PasswordResetForm diff --git a/common/djangoapps/user_api/api/course_tag.py b/common/djangoapps/user_api/api/course_tag.py index 7c7a01ce346..43907fbe723 100644 --- a/common/djangoapps/user_api/api/course_tag.py +++ b/common/djangoapps/user_api/api/course_tag.py @@ -53,6 +53,10 @@ def set_course_tag(user, course_id, key, value): key: arbitrary (<=255 char string) value: arbitrary string """ + # pylint: disable=W0511 + # TODO: There is a risk of IntegrityErrors being thrown here given + # simultaneous calls from many processes. Handle by retrying after + # a short delay? record, _ = UserCourseTag.objects.get_or_create( user=user, @@ -61,6 +65,3 @@ def set_course_tag(user, course_id, key, value): record.value = value record.save() - - # TODO: There is a risk of IntegrityErrors being thrown here given - # simultaneous calls from many processes. Handle by retrying after a short delay? diff --git a/common/djangoapps/user_api/helpers.py b/common/djangoapps/user_api/helpers.py index 06eb2497d70..233284e7912 100644 --- a/common/djangoapps/user_api/helpers.py +++ b/common/djangoapps/user_api/helpers.py @@ -72,9 +72,9 @@ def require_post_params(required_params): HttpResponse """ - def _decorator(func): + def _decorator(func): # pylint: disable=missing-docstring @wraps(func) - def _wrapped(*args, **kwargs): + def _wrapped(*args, **_kwargs): # pylint: disable=missing-docstring request = args[0] missing_params = set(required_params) - set(request.POST.keys()) if len(missing_params) > 0: @@ -342,7 +342,7 @@ def shim_student_view(view_func, check_logged_in=False): """ @wraps(view_func) - def _inner(request): + def _inner(request): # pylint: disable=missing-docstring # Ensure that the POST querydict is mutable request.POST = request.POST.copy() diff --git a/common/djangoapps/user_api/tests/test_constants.py b/common/djangoapps/user_api/tests/test_constants.py index 7f2b086847d..7508965559b 100644 --- a/common/djangoapps/user_api/tests/test_constants.py +++ b/common/djangoapps/user_api/tests/test_constants.py @@ -250,4 +250,4 @@ SORTED_COUNTRIES = [ (u'ZM', u'Zambia'), (u'ZW', u'Zimbabwe'), (u'AX', u'\xc5land Islands') -] \ No newline at end of file +] diff --git a/common/djangoapps/user_api/tests/test_helpers.py b/common/djangoapps/user_api/tests/test_helpers.py index 215957dd408..94101e04c0c 100644 --- a/common/djangoapps/user_api/tests/test_helpers.py +++ b/common/djangoapps/user_api/tests/test_helpers.py @@ -14,12 +14,12 @@ from user_api.helpers import ( class FakeInputException(Exception): - """Fake exception that should be intercepted. """ + """Fake exception that should be intercepted.""" pass class FakeOutputException(Exception): - """Fake exception that should be raised. """ + """Fake exception that should be raised.""" pass @@ -36,9 +36,7 @@ def intercepted_function(raise_error=None): class InterceptErrorsTest(TestCase): - """ - Tests for the decorator that intercepts errors. - """ + """Tests for the decorator that intercepts errors.""" @raises(FakeOutputException) def test_intercepts_errors(self): @@ -73,7 +71,7 @@ class InterceptErrorsTest(TestCase): class FormDescriptionTest(TestCase): - + """Tests of helper functions which generate form descriptions.""" def test_to_json(self): desc = FormDescription("post", "/submit") desc.add_field( @@ -134,7 +132,7 @@ class FormDescriptionTest(TestCase): @ddt.ddt class StudentViewShimTest(TestCase): - + "Tests of the student view shim." def setUp(self): self.captured_request = None @@ -211,8 +209,8 @@ class StudentViewShimTest(TestCase): response = view(HttpRequest()) self.assertEqual(response.status_code, 403) - def _shimmed_view(self, response, check_logged_in=False): - def stub_view(request): + def _shimmed_view(self, response, check_logged_in=False): # pylint: disable=missing-docstring + def stub_view(request): # pylint: disable=missing-docstring self.captured_request = request return response return shim_student_view(stub_view, check_logged_in=check_logged_in) diff --git a/common/djangoapps/user_api/tests/test_views.py b/common/djangoapps/user_api/tests/test_views.py index 49d37418753..01886c0093e 100644 --- a/common/djangoapps/user_api/tests/test_views.py +++ b/common/djangoapps/user_api/tests/test_views.py @@ -112,6 +112,11 @@ class ApiTestCase(TestCase): self.assertEqual(response.status_code, 405) def assertAuthDisabled(self, method, uri): + """ + Assert that the Django rest framework does not interpret basic auth + headers for views exposed to anonymous users as an attempt to authenticate. + + """ # Django rest framework interprets basic auth headers # as an attempt to authenticate with the API. # We don't want this for views available to anonymous users. @@ -987,7 +992,7 @@ class RegistrationViewTest(ApiTestCase): ) def test_register_form_year_of_birth(self): - this_year = datetime.datetime.now(UTC).year + this_year = datetime.datetime.now(UTC).year # pylint: disable=maybe-no-member year_options = ( [{"value": "", "name": "--", "default": True}] + [ {"value": unicode(year), "name": unicode(year)} @@ -1067,13 +1072,17 @@ class RegistrationViewTest(ApiTestCase): self._assert_reg_field( {"honor_code": "required"}, { - "label": "I agree to the <a href=\"https://www.test.com/honor\">Terms of Service and Honor Code</a>", + "label": "I agree to the {platform_name} <a href=\"https://www.test.com/honor\">Terms of Service and Honor Code</a>.".format( + platform_name=settings.PLATFORM_NAME + ), "name": "honor_code", "defaultValue": False, "type": "checkbox", "required": True, "errorMessages": { - "required": "You must agree to the <a href=\"https://www.test.com/honor\">Terms of Service and Honor Code</a>" + "required": "You must agree to the {platform_name} <a href=\"https://www.test.com/honor\">Terms of Service and Honor Code</a>.".format( + platform_name=settings.PLATFORM_NAME + ) } } ) @@ -1084,13 +1093,17 @@ class RegistrationViewTest(ApiTestCase): self._assert_reg_field( {"honor_code": "required"}, { - "label": "I agree to the <a href=\"/honor\">Terms of Service and Honor Code</a>", + "label": "I agree to the {platform_name} <a href=\"/honor\">Terms of Service and Honor Code</a>.".format( + platform_name=settings.PLATFORM_NAME + ), "name": "honor_code", "defaultValue": False, "type": "checkbox", "required": True, "errorMessages": { - "required": "You must agree to the <a href=\"/honor\">Terms of Service and Honor Code</a>" + "required": "You must agree to the {platform_name} <a href=\"/honor\">Terms of Service and Honor Code</a>.".format( + platform_name=settings.PLATFORM_NAME + ) } } ) @@ -1107,13 +1120,17 @@ class RegistrationViewTest(ApiTestCase): self._assert_reg_field( {"honor_code": "required", "terms_of_service": "required"}, { - "label": "I agree to the <a href=\"https://www.test.com/honor\">Honor Code</a>", + "label": "I agree to the {platform_name} <a href=\"https://www.test.com/honor\">Honor Code</a>.".format( + platform_name=settings.PLATFORM_NAME + ), "name": "honor_code", "defaultValue": False, "type": "checkbox", "required": True, "errorMessages": { - "required": "You must agree to the <a href=\"https://www.test.com/honor\">Honor Code</a>" + "required": "You must agree to the {platform_name} <a href=\"https://www.test.com/honor\">Honor Code</a>.".format( + platform_name=settings.PLATFORM_NAME + ) } } ) @@ -1122,13 +1139,17 @@ class RegistrationViewTest(ApiTestCase): self._assert_reg_field( {"honor_code": "required", "terms_of_service": "required"}, { - "label": "I agree to the <a href=\"https://www.test.com/tos\">Terms of Service</a>", + "label": "I agree to the {platform_name} <a href=\"https://www.test.com/tos\">Terms of Service</a>.".format( + platform_name=settings.PLATFORM_NAME + ), "name": "terms_of_service", "defaultValue": False, "type": "checkbox", "required": True, "errorMessages": { - "required": "You must agree to the <a href=\"https://www.test.com/tos\">Terms of Service</a>" + "required": "You must agree to the {platform_name} <a href=\"https://www.test.com/tos\">Terms of Service</a>.".format( + platform_name=settings.PLATFORM_NAME + ) } } ) @@ -1141,13 +1162,17 @@ class RegistrationViewTest(ApiTestCase): self._assert_reg_field( {"honor_code": "required", "terms_of_service": "required"}, { - "label": "I agree to the <a href=\"/honor\">Honor Code</a>", + "label": "I agree to the {platform_name} <a href=\"/honor\">Honor Code</a>.".format( + platform_name=settings.PLATFORM_NAME + ), "name": "honor_code", "defaultValue": False, "type": "checkbox", "required": True, "errorMessages": { - "required": "You must agree to the <a href=\"/honor\">Honor Code</a>" + "required": "You must agree to the {platform_name} <a href=\"/honor\">Honor Code</a>.".format( + platform_name=settings.PLATFORM_NAME + ) } } ) @@ -1156,13 +1181,17 @@ class RegistrationViewTest(ApiTestCase): self._assert_reg_field( {"honor_code": "required", "terms_of_service": "required"}, { - "label": "I agree to the <a href=\"/tos\">Terms of Service</a>", + "label": "I agree to the {platform_name} <a href=\"/tos\">Terms of Service</a>.".format( + platform_name=settings.PLATFORM_NAME + ), "name": "terms_of_service", "defaultValue": False, "type": "checkbox", "required": True, "errorMessages": { - "required": "You must agree to the <a href=\"/tos\">Terms of Service</a>" + "required": "You must agree to the {platform_name} <a href=\"/tos\">Terms of Service</a>.".format( + platform_name=settings.PLATFORM_NAME + ) } } ) @@ -1372,7 +1401,7 @@ class RegistrationViewTest(ApiTestCase): self.assertEqual(response.status_code, 409) self.assertEqual( response.content, - "It looks like {} belongs to an existing account. Try again with a different email address and username.".format( + "It looks like {} belongs to an existing account. Try again with a different username.".format( self.USERNAME ) ) diff --git a/common/djangoapps/user_api/urls.py b/common/djangoapps/user_api/urls.py index 7ae4f5f3502..55a2e077cd3 100644 --- a/common/djangoapps/user_api/urls.py +++ b/common/djangoapps/user_api/urls.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring from django.conf import settings from django.conf.urls import include, patterns, url from rest_framework import routers diff --git a/common/djangoapps/user_api/views.py b/common/djangoapps/user_api/views.py index 4791f15d2d2..c6739c30c3a 100644 --- a/common/djangoapps/user_api/views.py +++ b/common/djangoapps/user_api/views.py @@ -1,6 +1,5 @@ """HTTP end-points for the User API. """ import copy -import json from django.conf import settings from django.contrib.auth.models import User @@ -25,7 +24,6 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from edxmako.shortcuts import marketing_link import third_party_auth -from microsite_configuration import microsite from user_api.api import account as account_api, profile as profile_api from user_api.helpers import FormDescription, shim_student_view, require_post_params @@ -54,7 +52,7 @@ class LoginSessionView(APIView): # so do not require authentication. authentication_classes = [] - def get(self, request): + def get(self, request): # pylint: disable=unused-argument """Return a description of the login form. This decouples clients from the API definition: @@ -64,9 +62,6 @@ class LoginSessionView(APIView): See `user_api.helpers.FormDescription` for examples of the JSON-encoded form description. - Arguments: - request (HttpRequest) - Returns: HttpResponse @@ -307,6 +302,15 @@ class RegistrationView(APIView): return shim_student_view(create_account)(request) def _add_email_field(self, form_desc, required=True): + """Add an email field to a form description. + + Arguments: + form_desc: A form description + + Keyword Arguments: + required (Boolean): Whether this field is required; defaults to True + + """ # Translators: This label appears above a field on the registration form # meant to hold the user's email address. email_label = _(u"Email") @@ -328,6 +332,15 @@ class RegistrationView(APIView): ) def _add_name_field(self, form_desc, required=True): + """Add a name field to a form description. + + Arguments: + form_desc: A form description + + Keyword Arguments: + required (Boolean): Whether this field is required; defaults to True + + """ # Translators: This label appears above a field on the registration form # meant to hold the user's full name. name_label = _(u"Full Name") @@ -347,6 +360,15 @@ class RegistrationView(APIView): ) def _add_username_field(self, form_desc, required=True): + """Add a username field to a form description. + + Arguments: + form_desc: A form description + + Keyword Arguments: + required (Boolean): Whether this field is required; defaults to True + + """ # Translators: This label appears above a field on the registration form # meant to hold the user's public username. username_label = _(u"Username") @@ -369,6 +391,15 @@ class RegistrationView(APIView): ) def _add_password_field(self, form_desc, required=True): + """Add a password field to a form description. + + Arguments: + form_desc: A form description + + Keyword Arguments: + required (Boolean): Whether this field is required; defaults to True + + """ # Translators: This label appears above a field on the registration form # meant to hold the user's password. password_label = _(u"Password") @@ -385,6 +416,15 @@ class RegistrationView(APIView): ) def _add_level_of_education_field(self, form_desc, required=True): + """Add a level of education field to a form description. + + Arguments: + form_desc: A form description + + Keyword Arguments: + required (Boolean): Whether this field is required; defaults to True + + """ # Translators: This label appears above a dropdown menu on the registration # form used to select the user's highest completed level of education. education_level_label = _(u"Highest Level of Education Completed") @@ -399,6 +439,15 @@ class RegistrationView(APIView): ) def _add_gender_field(self, form_desc, required=True): + """Add a gender field to a form description. + + Arguments: + form_desc: A form description + + Keyword Arguments: + required (Boolean): Whether this field is required; defaults to True + + """ # Translators: This label appears above a dropdown menu on the registration # form used to select the user's gender. gender_label = _(u"Gender") @@ -413,6 +462,15 @@ class RegistrationView(APIView): ) def _add_year_of_birth_field(self, form_desc, required=True): + """Add a year of birth field to a form description. + + Arguments: + form_desc: A form description + + Keyword Arguments: + required (Boolean): Whether this field is required; defaults to True + + """ # Translators: This label appears above a dropdown menu on the registration # form used to select the user's year of birth. yob_label = _(u"Year of Birth") @@ -428,6 +486,15 @@ class RegistrationView(APIView): ) def _add_mailing_address_field(self, form_desc, required=True): + """Add a mailing address field to a form description. + + Arguments: + form_desc: A form description + + Keyword Arguments: + required (Boolean): Whether this field is required; defaults to True + + """ # Translators: This label appears above a field on the registration form # meant to hold the user's mailing address. mailing_address_label = _(u"Mailing Address") @@ -440,6 +507,15 @@ class RegistrationView(APIView): ) def _add_goals_field(self, form_desc, required=True): + """Add a goals field to a form description. + + Arguments: + form_desc: A form description + + Keyword Arguments: + required (Boolean): Whether this field is required; defaults to True + + """ # Translators: This phrase appears above a field on the registration form # meant to hold the user's reasons for registering with edX. goals_label = _( @@ -454,6 +530,15 @@ class RegistrationView(APIView): ) def _add_city_field(self, form_desc, required=True): + """Add a city field to a form description. + + Arguments: + form_desc: A form description + + Keyword Arguments: + required (Boolean): Whether this field is required; defaults to True + + """ # Translators: This label appears above a field on the registration form # which allows the user to input the city in which they live. city_label = _(u"City") @@ -465,6 +550,15 @@ class RegistrationView(APIView): ) def _add_country_field(self, form_desc, required=True): + """Add a country field to a form description. + + Arguments: + form_desc: A form description + + Keyword Arguments: + required (Boolean): Whether this field is required; defaults to True + + """ # Translators: This label appears above a dropdown menu on the registration # form used to select the country in which the user lives. country_label = _(u"Country") @@ -486,6 +580,15 @@ class RegistrationView(APIView): ) def _add_honor_code_field(self, form_desc, required=True): + """Add an honor code field to a form description. + + Arguments: + form_desc: A form description + + Keyword Arguments: + required (Boolean): Whether this field is required; defaults to True + + """ # Separate terms of service and honor code checkboxes if self._is_field_visible("terms_of_service"): terms_text = _(u"Honor Code") @@ -531,6 +634,15 @@ class RegistrationView(APIView): ) def _add_terms_of_service_field(self, form_desc, required=True): + """Add a terms of service field to a form description. + + Arguments: + form_desc: A form description + + Keyword Arguments: + required (Boolean): Whether this field is required; defaults to True + + """ # Translators: This is a legal document users must agree to # in order to register a new account. terms_text = _(u"Terms of Service") @@ -615,6 +727,7 @@ class RegistrationView(APIView): restrictions={} ) + class PasswordResetView(APIView): """HTTP end-point for GETting a description of the password reset form. """ @@ -622,7 +735,7 @@ class PasswordResetView(APIView): # so do not require authentication. authentication_classes = [] - def get(self, request): + def get(self, request): # pylint: disable=unused-argument """Return a description of the password reset form. This decouples clients from the API definition: @@ -632,9 +745,6 @@ class PasswordResetView(APIView): See `user_api.helpers.FormDescription` for examples of the JSON-encoded form description. - Arguments: - request (HttpRequest) - Returns: HttpResponse diff --git a/common/djangoapps/util/testing.py b/common/djangoapps/util/testing.py index 311ed89244a..ccfd29e91d0 100644 --- a/common/djangoapps/util/testing.py +++ b/common/djangoapps/util/testing.py @@ -20,6 +20,7 @@ class UrlResetMixin(object): """ def _reset_urls(self, urlconf_modules): + """Reset `urls.py` for a set of Django apps.""" for urlconf in urlconf_modules: if urlconf in sys.modules: reload(sys.modules[urlconf]) diff --git a/lms/djangoapps/student_account/helpers.py b/lms/djangoapps/student_account/helpers.py index 7683791a3b5..cd2e8d947fa 100644 --- a/lms/djangoapps/student_account/helpers.py +++ b/lms/djangoapps/student_account/helpers.py @@ -1,4 +1,4 @@ """Helper functions for the student account app. """ -# TODO: move this function here instead of importing it from student -from student.helpers import auth_pipeline_urls +# TODO: move this function here instead of importing it from student # pylint: disable=fixme +from student.helpers import auth_pipeline_urls # pylint: disable=unused-import -- GitLab