Merge pull request #5637 from edx/will/honor-code-and-tos

Add honor code to registration dialog.

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

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 <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={
         "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"]))

common/djangoapps/user_api/views.py

 """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"<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):
         """Include a default option as the first option. """
         return (