From 97f3f09009296a645dd0534e9bc80858089120b5 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Mon, 20 Oct 2014 08:59:23 -0400 Subject: [PATCH] Add honor code to registration dialog. --- common/djangoapps/user_api/helpers.py | 12 +++++ .../djangoapps/user_api/tests/test_views.py | 50 ++++++++++++++++++- common/djangoapps/user_api/views.py | 46 +++++++++++++---- 3 files changed, 96 insertions(+), 12 deletions(-) diff --git a/common/djangoapps/user_api/helpers.py b/common/djangoapps/user_api/helpers.py index 9bc296fe13..3be67abea6 100644 --- a/common/djangoapps/user_api/helpers.py +++ b/common/djangoapps/user_api/helpers.py @@ -276,6 +276,8 @@ def shim_student_view(view_func, check_logged_in=False): """ @wraps(view_func) def _inner(request): + # Ensure that the POST querydict is mutable + request.POST = request.POST.copy() # The login and registration handlers in student view try to change # the user's enrollment status if these parameters are present. @@ -287,6 +289,16 @@ def shim_student_view(view_func, check_logged_in=False): if "course_id" in request.POST: del request.POST["course_id"] + # Backwards compatibility: the student view expects both + # terms of service and honor code values. Since we're combining + # these into a single checkbox, the only value we may get + # from the new view is "honor_code". + # Longer term, we will need to make this more flexible to support + # open source installations that may have separate checkboxes + # for TOS, privacy policy, etc. + if request.POST.get("honor_code") is not None and request.POST.get("terms_of_service") is None: + request.POST["terms_of_service"] = request.POST.get("honor_code") + # Call the original view to generate a response. # We can safely modify the status code or content # of the response, but to be safe we won't mess diff --git a/common/djangoapps/user_api/tests/test_views.py b/common/djangoapps/user_api/tests/test_views.py index cd4af1388e..1edc3e8a65 100644 --- a/common/djangoapps/user_api/tests/test_views.py +++ b/common/djangoapps/user_api/tests/test_views.py @@ -13,6 +13,7 @@ from django.test.utils import override_settings from unittest import SkipTest, skipUnless import ddt from pytz import UTC +from mock import patch from user_api.api import account as account_api, profile as profile_api @@ -946,6 +947,42 @@ class RegistrationViewTest(ApiTestCase): } ) + @override_settings( + MKTG_URLS={"ROOT": "https://www.test.com/", "HONOR": "honor"}, + ) + @patch.dict(settings.FEATURES, {"ENABLE_MKTG_SITE": True}) + def test_registration_honor_code_mktg_site_enabled(self): + self._assert_reg_field( + {"honor_code": "required"}, + { + "label": "I agree to the Terms of Service and Honor Code", + "name": "honor_code", + "default": False, + "type": "checkbox", + "required": True, + "placeholder": "", + "instructions": "", + "restrictions": {}, + } + ) + + @override_settings(MKTG_URLS_LINK_MAP={"HONOR": "honor"}) + @patch.dict(settings.FEATURES, {"ENABLE_MKTG_SITE": False}) + def test_registration_honor_code_mktg_site_disabled(self): + self._assert_reg_field( + {"honor_code": "required"}, + { + "label": "I agree to the Terms of Service and Honor Code", + "name": "honor_code", + "default": False, + "type": "checkbox", + "required": True, + "placeholder": "", + "instructions": "", + "restrictions": {}, + } + ) + @override_settings(REGISTRATION_EXTRA_FIELDS={ "level_of_education": "optional", "gender": "optional", @@ -954,6 +991,7 @@ class RegistrationViewTest(ApiTestCase): "goals": "optional", "city": "optional", "country": "required", + "honor_code": "required", }) def test_field_order(self): response = self.client.get(self.url) @@ -974,6 +1012,7 @@ class RegistrationViewTest(ApiTestCase): "year_of_birth", "mailing_address", "goals", + "honor_code", ]) def test_register(self): @@ -983,6 +1022,7 @@ class RegistrationViewTest(ApiTestCase): "name": self.NAME, "username": self.USERNAME, "password": self.PASSWORD, + "honor_code": "true", }) self.assertHttpOK(response) @@ -1026,7 +1066,8 @@ class RegistrationViewTest(ApiTestCase): "year_of_birth": self.YEAR_OF_BIRTH, "goals": self.GOALS, "city": self.CITY, - "country": self.COUNTRY + "country": self.COUNTRY, + "honor_code": "true", }) self.assertHttpOK(response) @@ -1046,6 +1087,7 @@ class RegistrationViewTest(ApiTestCase): "name": self.NAME, "username": self.USERNAME, "password": self.PASSWORD, + "honor_code": "true", }) self.assertHttpOK(response) @@ -1104,6 +1146,7 @@ class RegistrationViewTest(ApiTestCase): "name": self.NAME, "username": self.USERNAME, "password": self.PASSWORD, + "honor_code": "true", }) self.assertHttpOK(response) @@ -1113,6 +1156,7 @@ class RegistrationViewTest(ApiTestCase): "name": "Someone Else", "username": "someone_else", "password": self.PASSWORD, + "honor_code": "true", }) self.assertEqual(response.status_code, 409) self.assertEqual(response.content, json.dumps(["email"])) @@ -1124,6 +1168,7 @@ class RegistrationViewTest(ApiTestCase): "name": self.NAME, "username": self.USERNAME, "password": self.PASSWORD, + "honor_code": "true", }) self.assertHttpOK(response) @@ -1133,6 +1178,7 @@ class RegistrationViewTest(ApiTestCase): "name": "Someone Else", "username": self.USERNAME, "password": self.PASSWORD, + "honor_code": "true", }) self.assertEqual(response.status_code, 409) self.assertEqual(response.content, json.dumps(["username"])) @@ -1144,6 +1190,7 @@ class RegistrationViewTest(ApiTestCase): "name": self.NAME, "username": self.USERNAME, "password": self.PASSWORD, + "honor_code": "true", }) self.assertHttpOK(response) @@ -1153,6 +1200,7 @@ class RegistrationViewTest(ApiTestCase): "name": "Someone Else", "username": self.USERNAME, "password": self.PASSWORD, + "honor_code": "true", }) self.assertEqual(response.status_code, 409) self.assertEqual(response.content, json.dumps(["email", "username"])) diff --git a/common/djangoapps/user_api/views.py b/common/djangoapps/user_api/views.py index e964cdff99..3059c08168 100644 --- a/common/djangoapps/user_api/views.py +++ b/common/djangoapps/user_api/views.py @@ -1,4 +1,5 @@ """HTTP end-points for the User API. """ +import copy import json from django.conf import settings @@ -21,6 +22,7 @@ from user_api.serializers import UserSerializer, UserPreferenceSerializer from user_api.models import UserPreference, UserProfile from django_comment_common.models import Role from opaque_keys.edx.locations import SlashSeparatedCourseKey +from edxmako.shortcuts import marketing_link 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 @@ -134,6 +136,7 @@ class RegistrationView(APIView): EXTRA_FIELDS = [ "city", "country", "level_of_education", "gender", "year_of_birth", "mailing_address", "goals", + "honor_code", ] def __init__(self, *args, **kwargs): @@ -172,10 +175,15 @@ class RegistrationView(APIView): for field_name in self.DEFAULT_FIELDS: self.field_handlers[field_name](form_desc, required=True) + # Backwards compatibility: Honor code is required by default, unless + # explicitly set to "optional" in Django settings. + extra_fields_setting = copy.deepcopy(settings.REGISTRATION_EXTRA_FIELDS) + extra_fields_setting["honor_code"] = extra_fields_setting.get("honor_code", "required") + # Extra fields configured in Django settings # may be required, optional, or hidden for field_name in self.EXTRA_FIELDS: - field_setting = settings.REGISTRATION_EXTRA_FIELDS.get(field_name, "hidden") + field_setting = extra_fields_setting.get(field_name, "hidden") handler = self.field_handlers[field_name] if field_setting in ["required", "optional"]: @@ -200,16 +208,6 @@ class RegistrationView(APIView): HttpResponse: 302 if redirecting to another page. """ - # Backwards compatability - # We used to validate that the users had checked - # "honor code" and "terms of service" - # on the registration form. Now we rely on the client - # to display this to users and validate that they - # agree before making the request to this service. - request.POST = request.POST.copy() - request.POST["honor_code"] = "true" - request.POST["terms_of_service"] = "true" - # Handle duplicate username/email conflicts = account_api.check_account_exists( username=request.POST.get('username'), @@ -343,6 +341,32 @@ class RegistrationView(APIView): required=required ) + def _add_honor_code_field(self, form_desc, required=True): + # Translators: This is a legal document users must agree to in order to register a new account. + terms_text = _(u"Terms of Service and Honor Code") + + # Translators: "Terms of service" is a legal document users must agree to in order to register a new account. + label = _( + u"I agree to the {terms_of_service}" + ).format( + terms_of_service=u"{terms_text}".format( + url=marketing_link("HONOR"), + terms_text=terms_text + ) + ) + + # TODO: Open source installations may need + # separate checkboxes for terms or service or privacy + # policies. Instead of hard-coding this, we should create a more flexible + # mechanism for configuring which fields appear on the registration form. + form_desc.add_field( + "honor_code", + label=label, + field_type="checkbox", + default=False, + required=required, + ) + def _options_with_default(self, options): """Include a default option as the first option. """ return (