Commit 97f3f090 by Will Daly

Add honor code to registration dialog.

parent 458186f7
...@@ -276,6 +276,8 @@ def shim_student_view(view_func, check_logged_in=False): ...@@ -276,6 +276,8 @@ def shim_student_view(view_func, check_logged_in=False):
""" """
@wraps(view_func) @wraps(view_func)
def _inner(request): 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 login and registration handlers in student view try to change
# the user's enrollment status if these parameters are present. # the user's enrollment status if these parameters are present.
...@@ -287,6 +289,16 @@ def shim_student_view(view_func, check_logged_in=False): ...@@ -287,6 +289,16 @@ def shim_student_view(view_func, check_logged_in=False):
if "course_id" in request.POST: if "course_id" in request.POST:
del request.POST["course_id"] 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. # Call the original view to generate a response.
# We can safely modify the status code or content # We can safely modify the status code or content
# of the response, but to be safe we won't mess # of the response, but to be safe we won't mess
......
...@@ -13,6 +13,7 @@ from django.test.utils import override_settings ...@@ -13,6 +13,7 @@ from django.test.utils import override_settings
from unittest import SkipTest, skipUnless from unittest import SkipTest, skipUnless
import ddt import ddt
from pytz import UTC from pytz import UTC
from mock import patch
from user_api.api import account as account_api, profile as profile_api from user_api.api import account as account_api, profile as profile_api
...@@ -946,6 +947,42 @@ class RegistrationViewTest(ApiTestCase): ...@@ -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 <a href=\"https://www.test.com/honor\">Terms of Service and Honor Code</a>",
"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 <a href=\"/honor\">Terms of Service and Honor Code</a>",
"name": "honor_code",
"default": False,
"type": "checkbox",
"required": True,
"placeholder": "",
"instructions": "",
"restrictions": {},
}
)
@override_settings(REGISTRATION_EXTRA_FIELDS={ @override_settings(REGISTRATION_EXTRA_FIELDS={
"level_of_education": "optional", "level_of_education": "optional",
"gender": "optional", "gender": "optional",
...@@ -954,6 +991,7 @@ class RegistrationViewTest(ApiTestCase): ...@@ -954,6 +991,7 @@ class RegistrationViewTest(ApiTestCase):
"goals": "optional", "goals": "optional",
"city": "optional", "city": "optional",
"country": "required", "country": "required",
"honor_code": "required",
}) })
def test_field_order(self): def test_field_order(self):
response = self.client.get(self.url) response = self.client.get(self.url)
...@@ -974,6 +1012,7 @@ class RegistrationViewTest(ApiTestCase): ...@@ -974,6 +1012,7 @@ class RegistrationViewTest(ApiTestCase):
"year_of_birth", "year_of_birth",
"mailing_address", "mailing_address",
"goals", "goals",
"honor_code",
]) ])
def test_register(self): def test_register(self):
...@@ -983,6 +1022,7 @@ class RegistrationViewTest(ApiTestCase): ...@@ -983,6 +1022,7 @@ class RegistrationViewTest(ApiTestCase):
"name": self.NAME, "name": self.NAME,
"username": self.USERNAME, "username": self.USERNAME,
"password": self.PASSWORD, "password": self.PASSWORD,
"honor_code": "true",
}) })
self.assertHttpOK(response) self.assertHttpOK(response)
...@@ -1026,7 +1066,8 @@ class RegistrationViewTest(ApiTestCase): ...@@ -1026,7 +1066,8 @@ class RegistrationViewTest(ApiTestCase):
"year_of_birth": self.YEAR_OF_BIRTH, "year_of_birth": self.YEAR_OF_BIRTH,
"goals": self.GOALS, "goals": self.GOALS,
"city": self.CITY, "city": self.CITY,
"country": self.COUNTRY "country": self.COUNTRY,
"honor_code": "true",
}) })
self.assertHttpOK(response) self.assertHttpOK(response)
...@@ -1046,6 +1087,7 @@ class RegistrationViewTest(ApiTestCase): ...@@ -1046,6 +1087,7 @@ class RegistrationViewTest(ApiTestCase):
"name": self.NAME, "name": self.NAME,
"username": self.USERNAME, "username": self.USERNAME,
"password": self.PASSWORD, "password": self.PASSWORD,
"honor_code": "true",
}) })
self.assertHttpOK(response) self.assertHttpOK(response)
...@@ -1104,6 +1146,7 @@ class RegistrationViewTest(ApiTestCase): ...@@ -1104,6 +1146,7 @@ class RegistrationViewTest(ApiTestCase):
"name": self.NAME, "name": self.NAME,
"username": self.USERNAME, "username": self.USERNAME,
"password": self.PASSWORD, "password": self.PASSWORD,
"honor_code": "true",
}) })
self.assertHttpOK(response) self.assertHttpOK(response)
...@@ -1113,6 +1156,7 @@ class RegistrationViewTest(ApiTestCase): ...@@ -1113,6 +1156,7 @@ class RegistrationViewTest(ApiTestCase):
"name": "Someone Else", "name": "Someone Else",
"username": "someone_else", "username": "someone_else",
"password": self.PASSWORD, "password": self.PASSWORD,
"honor_code": "true",
}) })
self.assertEqual(response.status_code, 409) self.assertEqual(response.status_code, 409)
self.assertEqual(response.content, json.dumps(["email"])) self.assertEqual(response.content, json.dumps(["email"]))
...@@ -1124,6 +1168,7 @@ class RegistrationViewTest(ApiTestCase): ...@@ -1124,6 +1168,7 @@ class RegistrationViewTest(ApiTestCase):
"name": self.NAME, "name": self.NAME,
"username": self.USERNAME, "username": self.USERNAME,
"password": self.PASSWORD, "password": self.PASSWORD,
"honor_code": "true",
}) })
self.assertHttpOK(response) self.assertHttpOK(response)
...@@ -1133,6 +1178,7 @@ class RegistrationViewTest(ApiTestCase): ...@@ -1133,6 +1178,7 @@ class RegistrationViewTest(ApiTestCase):
"name": "Someone Else", "name": "Someone Else",
"username": self.USERNAME, "username": self.USERNAME,
"password": self.PASSWORD, "password": self.PASSWORD,
"honor_code": "true",
}) })
self.assertEqual(response.status_code, 409) self.assertEqual(response.status_code, 409)
self.assertEqual(response.content, json.dumps(["username"])) self.assertEqual(response.content, json.dumps(["username"]))
...@@ -1144,6 +1190,7 @@ class RegistrationViewTest(ApiTestCase): ...@@ -1144,6 +1190,7 @@ class RegistrationViewTest(ApiTestCase):
"name": self.NAME, "name": self.NAME,
"username": self.USERNAME, "username": self.USERNAME,
"password": self.PASSWORD, "password": self.PASSWORD,
"honor_code": "true",
}) })
self.assertHttpOK(response) self.assertHttpOK(response)
...@@ -1153,6 +1200,7 @@ class RegistrationViewTest(ApiTestCase): ...@@ -1153,6 +1200,7 @@ class RegistrationViewTest(ApiTestCase):
"name": "Someone Else", "name": "Someone Else",
"username": self.USERNAME, "username": self.USERNAME,
"password": self.PASSWORD, "password": self.PASSWORD,
"honor_code": "true",
}) })
self.assertEqual(response.status_code, 409) self.assertEqual(response.status_code, 409)
self.assertEqual(response.content, json.dumps(["email", "username"])) self.assertEqual(response.content, json.dumps(["email", "username"]))
......
"""HTTP end-points for the User API. """ """HTTP end-points for the User API. """
import copy
import json import json
from django.conf import settings from django.conf import settings
...@@ -21,6 +22,7 @@ from user_api.serializers import UserSerializer, UserPreferenceSerializer ...@@ -21,6 +22,7 @@ from user_api.serializers import UserSerializer, UserPreferenceSerializer
from user_api.models import UserPreference, UserProfile from user_api.models import UserPreference, UserProfile
from django_comment_common.models import Role from django_comment_common.models import Role
from opaque_keys.edx.locations import SlashSeparatedCourseKey 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.api import account as account_api, profile as profile_api
from user_api.helpers import FormDescription, shim_student_view, require_post_params from user_api.helpers import FormDescription, shim_student_view, require_post_params
...@@ -134,6 +136,7 @@ class RegistrationView(APIView): ...@@ -134,6 +136,7 @@ class RegistrationView(APIView):
EXTRA_FIELDS = [ EXTRA_FIELDS = [
"city", "country", "level_of_education", "gender", "city", "country", "level_of_education", "gender",
"year_of_birth", "mailing_address", "goals", "year_of_birth", "mailing_address", "goals",
"honor_code",
] ]
def __init__(self, *args, **kwargs): def __init__(self, *args, **kwargs):
...@@ -172,10 +175,15 @@ class RegistrationView(APIView): ...@@ -172,10 +175,15 @@ class RegistrationView(APIView):
for field_name in self.DEFAULT_FIELDS: for field_name in self.DEFAULT_FIELDS:
self.field_handlers[field_name](form_desc, required=True) 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 # Extra fields configured in Django settings
# may be required, optional, or hidden # may be required, optional, or hidden
for field_name in self.EXTRA_FIELDS: 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] handler = self.field_handlers[field_name]
if field_setting in ["required", "optional"]: if field_setting in ["required", "optional"]:
...@@ -200,16 +208,6 @@ class RegistrationView(APIView): ...@@ -200,16 +208,6 @@ class RegistrationView(APIView):
HttpResponse: 302 if redirecting to another page. 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 # Handle duplicate username/email
conflicts = account_api.check_account_exists( conflicts = account_api.check_account_exists(
username=request.POST.get('username'), username=request.POST.get('username'),
...@@ -343,6 +341,32 @@ class RegistrationView(APIView): ...@@ -343,6 +341,32 @@ class RegistrationView(APIView):
required=required 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"<a href=\"{url}\">{terms_text}</a>".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): def _options_with_default(self, options):
"""Include a default option as the first option. """ """Include a default option as the first option. """
return ( return (
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment