Merge pull request #2285 from antoviaque/registration-extra-fields

Registration extra fields: Selectively require/hide registration fields & add optional country/city fields

common/djangoapps/student/models.py

@@ -29,6 +29,7 @@ from django.dispatch import receiver, Signal
 import django.dispatch
 from django.forms import ModelForm, forms
 from django.core.exceptions import ObjectDoesNotExist
+from django_countries import CountryField
 from track import contexts
 from track.views import server_track
 from eventtracking import tracker
@@ -213,6 +214,8 @@ class UserProfile(models.Model):
         choices=LEVEL_OF_EDUCATION_CHOICES
     )
     mailing_address = models.TextField(blank=True, null=True)
+    city = models.TextField(blank=True, null=True)
+    country = CountryField(blank=True, null=True)
     goals = models.TextField(blank=True, null=True)
     allow_certificate = models.BooleanField(default=1)
 

common/djangoapps/student/views.py

@@ -825,6 +825,8 @@ def _do_create_account(post_vars):
     profile.level_of_education = post_vars.get('level_of_education')
     profile.gender = post_vars.get('gender')
     profile.mailing_address = post_vars.get('mailing_address')
+    profile.city = post_vars.get('city')
+    profile.country = post_vars.get('country')
     profile.goals = post_vars.get('goals')
 
     try:
@@ -849,6 +851,7 @@ def create_account(request, post_override=None):
     js = {'success': False}
 
     post_vars = post_override if post_override else request.POST
+    extra_fields = getattr(settings, 'REGISTRATION_EXTRA_FIELDS', {})
 
     # if doing signup for an external authorization, then get email, password, name from the eamap
     # don't use the ones from the form, since the user could have hacked those
@@ -877,18 +880,23 @@ def create_account(request, post_override=None):
             js['field'] = a
             return HttpResponse(json.dumps(js))
 
-    if post_vars.get('honor_code', 'false') != u'true':
+    if extra_fields.get('honor_code', 'required') == 'required' and \
+            post_vars.get('honor_code', 'false') != u'true':
         js['value'] = _("To enroll, you must follow the honor code.").format(field=a)
         js['field'] = 'honor_code'
         return HttpResponse(json.dumps(js))
 
     # Can't have terms of service for certain SHIB users, like at Stanford
-    tos_not_required = (settings.FEATURES.get("AUTH_USE_SHIB") and
-                        settings.FEATURES.get('SHIB_DISABLE_TOS') and
-                        DoExternalAuth and
-                        eamap.external_domain.startswith(external_auth.views.SHIBBOLETH_DOMAIN_PREFIX))
+    tos_required = (
+        not settings.FEATURES.get("AUTH_USE_SHIB") or
+        not settings.FEATURES.get("SHIB_DISABLE_TOS") or
+        not DoExternalAuth or
+        not eamap.external_domain.startswith(
+            external_auth.views.SHIBBOLETH_DOMAIN_PREFIX
+        )
+    )
 
-    if not tos_not_required:
+    if tos_required:
         if post_vars.get('terms_of_service', 'false') != u'true':
             js['value'] = _("You must accept the terms of service.").format(field=a)
             js['field'] = 'terms_of_service'
@@ -900,20 +908,36 @@ def create_account(request, post_override=None):
     # this is a good idea
     # TODO: Check password is sane
 
-    required_post_vars = ['username', 'email', 'name', 'password', 'terms_of_service', 'honor_code']
-    if tos_not_required:
-        required_post_vars = ['username', 'email', 'name', 'password', 'honor_code']
-
-    for a in required_post_vars:
-        if len(post_vars[a]) < 2:
-            error_str = {'username': 'Username must be minimum of two characters long.',
-                         'email': 'A properly formatted e-mail is required.',
-                         'name': 'Your legal name must be a minimum of two characters long.',
-                         'password': 'A valid password is required.',
-                         'terms_of_service': 'Accepting Terms of Service is required.',
-                         'honor_code': 'Agreeing to the Honor Code is required.'}
-            js['value'] = error_str[a]
-            js['field'] = a
+    required_post_vars = ['username', 'email', 'name', 'password']
+    required_post_vars += [fieldname for fieldname, val in extra_fields.items()
+                           if val == 'required']
+    if tos_required:
+        required_post_vars.append('terms_of_service')
+
+    for field_name in required_post_vars:
+        if field_name in ('gender', 'level_of_education'):
+            min_length = 1
+        else:
+            min_length = 2
+
+        if len(post_vars[field_name]) < min_length:
+            error_str = {
+                'username': _('Username must be minimum of two characters long.'),
+                'email': _('A properly formatted e-mail is required.'),
+                'name': _('Your legal name must be a minimum of two characters long.'),
+                'password': _('A valid password is required.'),
+                'terms_of_service': _('Accepting Terms of Service is required.'),
+                'honor_code': _('Agreeing to the Honor Code is required.'),
+                'level_of_education': _('A level of education is required.'),
+                'gender': _('Your gender is required'),
+                'year_of_birth': _('Your year of birth is required'),
+                'mailing_address': _('Your mailing address is required'),
+                'goals': _('A description of your goals is required'),
+                'city': _('A city is required'),
+                'country': _('A country is required')
+            }
+            js['value'] = error_str[field_name]
+            js['field'] = field_name
             return HttpResponse(json.dumps(js))
 
     try:

lms/djangoapps/courseware/tests/test_registration_extra_vars.py

+# -*- coding: utf-8
+"""
+Tests for extra registration variables
+"""
+import json
+import uuid
+
+from django.conf import settings
+from django.core.urlresolvers import reverse
+from mock import patch
+
+from courseware.tests.helpers import LoginEnrollmentTestCase, check_for_post_code
+
+
+class TestExtraRegistrationVariables(LoginEnrollmentTestCase):
+    """
+    Test that extra registration variables are properly checked according to settings
+    """
+
+    def _do_register_attempt(self, **extra_fields_values):
+        """
+        Helper method to make the call to the do registration
+        """
+        username = 'foo_bar' + uuid.uuid4().hex
+        fields_values = {
+            'username': username,
+            'email': 'foo' + uuid.uuid4().hex + '@bar.com',
+            'password': 'password',
+            'name': username,
+            'terms_of_service': 'true',
+        }
+        fields_values = dict(fields_values.items() + extra_fields_values.items())
+        resp = check_for_post_code(self, 200, reverse('create_account'), fields_values)
+        data = json.loads(resp.content)
+        return data
+
+    def test_default_missing_honor(self):
+        """
+        By default, the honor code must be required
+        """
+        data = self._do_register_attempt(honor_code='')
+        self.assertEqual(data['success'], False)
+        self.assertEqual(data['value'], u'To enroll, you must follow the honor code.')
+
+    @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'honor_code': 'optional'})
+    def test_optional_honor(self):
+        """
+        With the honor code is made optional, should pass without extra vars
+        """
+        data = self._do_register_attempt(honor_code='')
+        self.assertEqual(data['success'], True)
+
+    @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {
+        'level_of_education': 'hidden',
+        'gender': 'hidden',
+        'year_of_birth': 'hidden',
+        'mailing_address': 'hidden',
+        'goals': 'hidden',
+        'honor_code': 'hidden',
+        'city': 'hidden',
+        'country': 'hidden'})
+    def test_all_hidden(self):
+        """
+        When the fields are all hidden, should pass without extra vars
+        """
+        data = self._do_register_attempt()
+        self.assertEqual(data['success'], True)
+
+    @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'city': 'required'})
+    def test_required_city_missing(self):
+        """
+        Should require the city if configured as 'required' but missing
+        """
+        data = self._do_register_attempt(honor_code='true', city='')
+        self.assertEqual(data['success'], False)
+        self.assertEqual(data['value'], u'A city is required')
+
+        data = self._do_register_attempt(honor_code='true', city='New York')
+        self.assertEqual(data['success'], True)
+
+    @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'country': 'required'})
+    def test_required_country_missing(self):
+        """
+        Should require the country if configured as 'required' but missing
+        """
+        data = self._do_register_attempt(honor_code='true', country='')
+        self.assertEqual(data['success'], False)
+        self.assertEqual(data['value'], u'A country is required')
+
+        data = self._do_register_attempt(honor_code='true', country='New York')
+        self.assertEqual(data['success'], True)
+
+    @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'level_of_education': 'required'})
+    def test_required_level_of_education_missing(self):
+        """
+        Should require the level_of_education if configured as 'required' but missing
+        """
+        data = self._do_register_attempt(honor_code='true', level_of_education='')
+        self.assertEqual(data['success'], False)
+        self.assertEqual(data['value'], u'A level of education is required.')
+
+        data = self._do_register_attempt(honor_code='true', level_of_education='p')
+        self.assertEqual(data['success'], True)
+
+    @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'gender': 'required'})
+    def test_required_gender_missing(self):
+        """
+        Should require the gender if configured as 'required' but missing
+        """
+        data = self._do_register_attempt(honor_code='true', gender='')
+        self.assertEqual(data['success'], False)
+        self.assertEqual(data['value'], u'Your gender is required')
+
+        data = self._do_register_attempt(honor_code='true', gender='m')
+        self.assertEqual(data['success'], True)
+
+    @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'year_of_birth': 'required'})
+    def test_required_year_of_birth_missing(self):
+        """
+        Should require the year_of_birth if configured as 'required' but missing
+        """
+        data = self._do_register_attempt(honor_code='true', year_of_birth='')
+        self.assertEqual(data['success'], False)
+        self.assertEqual(data['value'], u'Your year of birth is required')
+
+        data = self._do_register_attempt(honor_code='true', year_of_birth='1982')
+        self.assertEqual(data['success'], True)
+
+    @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'mailing_address': 'required'})
+    def test_required_mailing_address_missing(self):
+        """
+        Should require the mailing_address if configured as 'required' but missing
+        """
+        data = self._do_register_attempt(honor_code='true', mailing_address='')
+        self.assertEqual(data['success'], False)
+        self.assertEqual(data['value'], u'Your mailing address is required')
+
+        data = self._do_register_attempt(honor_code='true', mailing_address='my address')
+        self.assertEqual(data['success'], True)
+
+    @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'goals': 'required'})
+    def test_required_goals_missing(self):
+        """
+        Should require the goals if configured as 'required' but missing
+        """
+        data = self._do_register_attempt(honor_code='true', goals='')
+        self.assertEqual(data['success'], False)
+        self.assertEqual(data['value'], u'A description of your goals is required')
+
+        data = self._do_register_attempt(honor_code='true', goals='my goals')
+        self.assertEqual(data['success'], True)

lms/envs/aws.py

@@ -139,7 +139,7 @@ EMAIL_USE_TLS = ENV_TOKENS.get('EMAIL_USE_TLS', False)  # django default is Fals
 SITE_NAME = ENV_TOKENS['SITE_NAME']
 SESSION_ENGINE = ENV_TOKENS.get('SESSION_ENGINE', SESSION_ENGINE)
 SESSION_COOKIE_DOMAIN = ENV_TOKENS.get('SESSION_COOKIE_DOMAIN')
-REGISTRATION_OPTIONAL_FIELDS = ENV_TOKENS.get('REGISTRATION_OPTIONAL_FIELDS', REGISTRATION_OPTIONAL_FIELDS)
+REGISTRATION_EXTRA_FIELDS = ENV_TOKENS.get('REGISTRATION_EXTRA_FIELDS', REGISTRATION_EXTRA_FIELDS)
 
 CMS_BASE = ENV_TOKENS.get('CMS_BASE', 'studio.edx.org')
 

lms/envs/common.py

@@ -1153,14 +1153,21 @@ if FEATURES.get('AUTH_USE_CAS'):
 
 ###################### Registration ##################################
 
-# Remove some of the fields from the list to not display them
-REGISTRATION_OPTIONAL_FIELDS = set([
-    'level_of_education',
-    'gender',
-    'year_of_birth',
-    'mailing_address',
-    'goals',
-])
+# For each of the fields, give one of the following values:
+# - 'required': to display the field, and make it mandatory
+# - 'optional': to display the field, and make it non-mandatory
+# - 'hidden': to not display the field
+
+REGISTRATION_EXTRA_FIELDS = {
+    'level_of_education': 'optional',
+    'gender': 'optional',
+    'year_of_birth': 'optional',
+    'mailing_address': 'optional',
+    'goals': 'optional',
+    'honor_code': 'required',
+    'city': 'hidden',
+    'country': 'hidden',
+}
 
 ###################### Grade Downloads ######################
 GRADES_DOWNLOAD_ROUTING_KEY = HIGH_MEM_QUEUE

lms/templates/register.html

@@ -184,14 +184,33 @@
       </div>
 
       <div class="group group-form group-form-secondary group-form-personalinformation">
-        <h2 class="sr">${_("Optional Personal Information")}</h2>
+        <h2 class="sr">${_("Extra Personal Information")}</h2>
 
         <ol class="list-input">
-          % if 'level_of_education' in settings.REGISTRATION_OPTIONAL_FIELDS:
+          % if settings.REGISTRATION_EXTRA_FIELDS['city'] != 'hidden':
+          <li class="field ${settings.REGISTRATION_EXTRA_FIELDS['city']} text" id="field-city">
+            <label for="city">${_('City')}</label>
+            <input id="city" type="text" name="city" value="" placeholder="${_('example: New York')}" aria-describedby="city-tip" ${'required aria-required="true"' if settings.REGISTRATION_EXTRA_FIELDS['city'] == 'required' else ''} />
+          </li>
+          % endif
+          % if settings.REGISTRATION_EXTRA_FIELDS['country'] != 'hidden':
+          <li class="field-group">
+          <div class="field ${settings.REGISTRATION_EXTRA_FIELDS['country']} select" id="field-country">
+              <label for="country">${_("Country")}</label>
+              <select id="country" name="country" ${'required aria-required="true"' if settings.REGISTRATION_EXTRA_FIELDS['country'] == 'required' else ''}>
+                <option value="">--</option>
+                %for code, country_name in COUNTRIES:
+                <option value="${code}">${ unicode(country_name) }</option>
+                %endfor
+              </select>
+            </div>
+          </li>
+          % endif
+          % if settings.REGISTRATION_EXTRA_FIELDS['level_of_education'] != 'hidden':
           <li class="field-group">
-            <div class="field select" id="field-education-level">
+          <div class="field ${settings.REGISTRATION_EXTRA_FIELDS['level_of_education']} select" id="field-education-level">
               <label for="education-level">${_("Highest Level of Education Completed")}</label>
-              <select id="education-level" name="level_of_education">
+              <select id="education-level" name="level_of_education" ${'required aria-required="true"' if settings.REGISTRATION_EXTRA_FIELDS['level_of_education'] == 'required' else ''}>
                 <option value="">--</option>
                 %for code, ed_level in UserProfile.LEVEL_OF_EDUCATION_CHOICES:
                 <option value="${code}">${ed_level}</option>
@@ -200,11 +219,11 @@
             </div>
           </li>
           % endif
-          % if 'gender' in settings.REGISTRATION_OPTIONAL_FIELDS:
+          % if settings.REGISTRATION_EXTRA_FIELDS['gender'] != 'hidden':
           <li class="field-group">
-            <div class="field select" id="field-gender">
+            <div class="field ${settings.REGISTRATION_EXTRA_FIELDS['gender']} select" id="field-gender">
               <label for="gender">${_("Gender")}</label>
-              <select id="gender" name="gender">
+              <select id="gender" name="gender" ${'required aria-required="true"' if settings.REGISTRATION_EXTRA_FIELDS['gender'] == 'required' else ''}>
                 <option value="">--</option>
                 %for code, gender in UserProfile.GENDER_CHOICES:
                 <option value="${code}">${gender}</option>
@@ -213,11 +232,11 @@
             </div>
           </li>
           % endif
-          % if 'year_of_birth' in settings.REGISTRATION_OPTIONAL_FIELDS:
+          % if settings.REGISTRATION_EXTRA_FIELDS['year_of_birth'] != 'hidden':
           <li class="field-group">
-            <div class="field select" id="field-yob">
+            <div class="field ${settings.REGISTRATION_EXTRA_FIELDS['year_of_birth']} select" id="field-yob">
               <label for="yob">${_("Year of Birth")}</label>
-              <select id="yob" name="year_of_birth">
+              <select id="yob" name="year_of_birth" ${'required aria-required="true"' if settings.REGISTRATION_EXTRA_FIELDS['year_of_birth'] == 'required' else ''}>
                 <option value="">--</option>
                 %for year in UserProfile.VALID_YEARS:
                 <option value="${year}">${year}</option>
@@ -230,20 +249,20 @@
       </div>
 
       <div class="group group-form group-form-personalinformation2">
-        <h2 class="sr">${_("Optional Personal Information")}</h2>
+        <h2 class="sr">${_("Extra Personal Information")}</h2>
 
         <ol class="list-input">
-          % if 'mailing_address' in settings.REGISTRATION_OPTIONAL_FIELDS:
-          <li class="field text" id="field-address-mailing">
+          % if settings.REGISTRATION_EXTRA_FIELDS['mailing_address'] != 'hidden':
+          <li class="field ${settings.REGISTRATION_EXTRA_FIELDS['mailing_address']} text" id="field-address-mailing">
             <label for="address-mailing">${_("Mailing Address")}</label>
-            <textarea id="address-mailing" name="mailing_address" value=""></textarea>
+            <textarea id="address-mailing" name="mailing_address" value="" ${'required aria-required="true"' if settings.REGISTRATION_EXTRA_FIELDS['mailing_address'] == 'required' else ''}></textarea>
           </li>
           % endif
 
-          % if 'goals' in settings.REGISTRATION_OPTIONAL_FIELDS:
-          <li class="field text" id="field-goals">
+          % if settings.REGISTRATION_EXTRA_FIELDS['goals'] != 'hidden':
+          <li class="field ${settings.REGISTRATION_EXTRA_FIELDS['goals']} text" id="field-goals">
             <label for="goals">${_("Please share with us your reasons for registering with {platform_name}").format(platform_name=platform_name)}</label>
-            <textarea id="goals" name="goals" value=""></textarea>
+            <textarea id="goals" name="goals" value="" ${'required aria-required="true"' if settings.REGISTRATION_EXTRA_FIELDS['goals'] == 'required' else ''}></textarea>
           </li>
           % endif
         </ol>
@@ -256,17 +275,16 @@
           <li class="field-group">
 
             % if has_extauth_info is UNDEFINED or ask_for_tos :
-
             <div class="field required checkbox" id="field-tos">
               <input id="tos-yes" type="checkbox" name="terms_of_service" value="true" required aria-required="true" />
               <label for="tos-yes">${_('I agree to the {link_start}Terms of Service{link_end}').format(
                 link_start='<a href="{url}" class="new-vp">'.format(url=marketing_link('TOS')),
                 link_end='</a>')}</label>
             </div>
-
             % endif
 
-            <div class="field required checkbox" id="field-honorcode">
+            % if settings.REGISTRATION_EXTRA_FIELDS['honor_code'] != 'hidden':
+            <div class="field ${settings.REGISTRATION_EXTRA_FIELDS['honor_code']} checkbox" id="field-honorcode">
               <input id="honorcode-yes" type="checkbox" name="honor_code" value="true" />
               <%
                 ## TODO: provide a better way to override these links
@@ -279,6 +297,7 @@
                 link_start='<a href="{url}" class="new-vp">'.format(url=honor_code_path),
                 link_end='</a>')}</label>
             </div>
+            % endif
           </li>
         </ol>
       </div>

lms/templates/signup_modal.html

@@ -59,7 +59,7 @@
 
         <div class="input-group">
 
-          % if 'level_of_education' in settings.REGISTRATION_OPTIONAL_FIELDS:
+          % if settings.REGISTRATION_EXTRA_FIELDS['level_of_education'] != 'hidden':
           <section class="citizenship">
             <label data-field="level_of_education" for="signup_ed_level">${_("Ed. Completed")}</label>
             <div class="input-wrapper">
@@ -73,7 +73,7 @@
           </section>
           % endif
 
-          % if 'gender' in settings.REGISTRATION_OPTIONAL_FIELDS:
+          % if settings.REGISTRATION_EXTRA_FIELDS['gender'] != 'hidden':
           <section class="gender">
             <label data-field="gender" for="signup_gender">${_("Gender")}</label>
             <div class="input-wrapper">
@@ -87,7 +87,7 @@
           </section>
           % endif
 
-          % if 'year_of_birth' in settings.REGISTRATION_OPTIONAL_FIELDS:
+          % if settings.REGISTRATION_EXTRA_FIELDS['year_of_birth'] != 'hidden':
           <section class="date-of-birth">
             <label data-field="date-of-birth" for="signup_birth_year">${_("Year of birth")}</label>
             <div class="input-wrapper">
@@ -102,12 +102,12 @@
           </section>
           % endif
 
-          % if 'mailing_address' in settings.REGISTRATION_OPTIONAL_FIELDS:
+          % if settings.REGISTRATION_EXTRA_FIELDS['mailing_address'] != 'hidden':
           <label data-field="mailing_address" for="signup_mailing_address">${_("Mailing address")}</label>
 	    <textarea id="signup_mailing_address" name="mailing_address"></textarea>
           % endif
 
-          % if 'goals' in settings.REGISTRATION_OPTIONAL_FIELDS:
+          % if settings.REGISTRATION_EXTRA_FIELDS['goals'] != 'hidden':
           <label data-field="goals" for="signup_goals">${_("Goals in signing up for {platform_name}").format(platform_name=settings.PLATFORM_NAME)}</label>
 	    <textarea name="goals" id="signup_goals"></textarea>
           % endif
@@ -122,12 +122,14 @@
               link_end='</a>')}
           </label>
 
+          % if settings.REGISTRATION_EXTRA_FIELDS['honor_code'] != 'hidden':
           <label data-field="honor_code" class="honor-code" for="signup_honor">
             <input id="signup_honor" name="honor_code" type="checkbox" value="true">
             ${_('I agree to the {link_start}Honor Code{link_end}*').format(
               link_start='<a href="{url}" target="_blank">'.format(url=reverse('honor')),
               link_end='</a>')}
           </label>
+          % endif
         </div>
 
         <div class="submit">