Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
Fix use a registration field order when using a registration extensio…
Browse files Browse the repository at this point in the history 
…n form (#26633)
  • Loading branch information
@igobranco
igobranco committed Sep 23, 2021
1 parent 8f59d5e commit 6f0255b
Show file tree
Hide file tree
Showing 2 changed files with 154 additions and 67 deletions.
102 changes: 54 additions & 48 deletions openedx/core/djangoapps/user_authn/views/registration_form.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,14 +356,20 @@ def __init__(self):
handler = getattr(self, f"_add_{field_name}_field")
self.field_handlers[field_name] = handler

custom_form = get_registration_extension_form()
if custom_form:
custom_form_field_names = [field_name for field_name, field in custom_form.fields.items()]
valid_fields.extend(custom_form_field_names)

field_order = configuration_helpers.get_value('REGISTRATION_FIELD_ORDER')
if not field_order:
field_order = settings.REGISTRATION_FIELD_ORDER or valid_fields
# Check that all of the valid_fields are in the field order and vice versa,
# if not append missing fields at end of field order
if set(valid_fields) != set(field_order):
difference = set(valid_fields).difference(set(field_order))
field_order.extend(difference)
# sort the additional fields so we have could have a deterministic result when presenting them
field_order.extend(sorted(difference))

self.field_order = field_order

Expand All @@ -387,57 +393,57 @@ def get_registration_form(self, request):

# Custom form fields can be added via the form set in settings.REGISTRATION_EXTENSION_FORM
custom_form = get_registration_extension_form()

if custom_form:
# Default fields are always required
for field_name in self.DEFAULT_FIELDS:
self.field_handlers[field_name](form_desc, required=True)
custom_form_field_names = [field_name for field_name, field in custom_form.fields.items()]
else:
custom_form_field_names = []

for field_name, field in custom_form.fields.items():
restrictions = {}
if getattr(field, 'max_length', None):
restrictions['max_length'] = field.max_length
if getattr(field, 'min_length', None):
restrictions['min_length'] = field.min_length
field_options = getattr(
getattr(custom_form, 'Meta', None), 'serialization_options', {}
).get(field_name, {})
field_type = field_options.get('field_type', FormDescription.FIELD_TYPE_MAP.get(field.__class__))
if not field_type:
raise ImproperlyConfigured(
"Field type '{}' not recognized for registration extension field '{}'.".format(
field_type,
field_name
)
)
form_desc.add_field(
field_name, label=field.label,
default=field_options.get('default'),
field_type=field_options.get('field_type', FormDescription.FIELD_TYPE_MAP.get(field.__class__)),
placeholder=field.initial, instructions=field.help_text, required=field.required,
restrictions=restrictions,
options=getattr(field, 'choices', None), error_messages=field.error_messages,
include_default_option=field_options.get('include_default_option'),
# Go through the fields in the fields order and add them if they are required or visible
for field_name in self.field_order:
if field_name in self.DEFAULT_FIELDS:
self.field_handlers[field_name](form_desc, required=True)
elif self._is_field_visible(field_name) and self.field_handlers.get(field_name):
self.field_handlers[field_name](
form_desc,
required=self._is_field_required(field_name)
)
elif field_name in custom_form_field_names:
for custom_field_name, field in custom_form.fields.items():
if field_name == custom_field_name:
restrictions = {}
if getattr(field, 'max_length', None):
restrictions['max_length'] = field.max_length
if getattr(field, 'min_length', None):
restrictions['min_length'] = field.min_length
field_options = getattr(
getattr(custom_form, 'Meta', None), 'serialization_options', {}
).get(field_name, {})
field_type = field_options.get(
'field_type',
FormDescription.FIELD_TYPE_MAP.get(field.__class__))
if not field_type:
raise ImproperlyConfigured(
u"Field type '{}' not recognized for registration extension field '{}'.".format(
field_type,
field_name
)
)
if self._is_field_visible(field_name) or field.required:
form_desc.add_field(
field_name,
label=field.label,
default=field_options.get('default'),
field_type=field_options.get(
'field_type',
FormDescription.FIELD_TYPE_MAP.get(field.__class__)),
placeholder=field.initial,
instructions=field.help_text,
required=(self._is_field_required(field_name) or field.required),
restrictions=restrictions,
options=getattr(field, 'choices', None), error_messages=field.error_messages,
include_default_option=field_options.get('include_default_option'),
)

# Extra fields configured in Django settings
# may be required, optional, or hidden
for field_name in self.EXTRA_FIELDS:
if self._is_field_visible(field_name):
self.field_handlers[field_name](
form_desc,
required=self._is_field_required(field_name)
)
else:
# Go through the fields in the fields order and add them if they are required or visible
for field_name in self.field_order:
if field_name in self.DEFAULT_FIELDS:
self.field_handlers[field_name](form_desc, required=True)
elif self._is_field_visible(field_name):
self.field_handlers[field_name](
form_desc,
required=self._is_field_required(field_name)
)
# remove confirm_email form v1 registration form
if is_api_v1(request):
for index, field in enumerate(form_desc.fields):
Expand Down
119 changes: 100 additions & 19 deletions openedx/core/djangoapps/user_authn/views/tests/test_register.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -657,15 +657,15 @@ def test_extension_form_fields(self):
}
)

self._assert_reg_field(
self._assert_reg_absent_field(
no_extra_fields_setting,
{
"name": "favorite_editor",
"type": "select",
"name": u"favorite_editor",
"type": u"select",
"required": False,
"label": "Favorite Editor",
"placeholder": "cat",
"defaultValue": "vim",
"label": u"Favorite Editor",
"placeholder": u"cat",
"defaultValue": u"vim",
"errorMessages": {
'required': 'This field is required.',
'invalid_choice': 'Select a valid choice. %(value)s is not one of the available choices.',
Expand Down Expand Up @@ -1259,6 +1259,7 @@ def test_registration_separate_terms_of_service_mktg_site_disabled(self):
"honor_code": "required",
"confirm_email": "required",
},
REGISTRATION_FIELD_ORDER=None,
REGISTRATION_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm',
)
def test_field_order(self):
Expand All @@ -1268,9 +1269,22 @@ def test_field_order(self):
# Verify that all fields render in the correct order
form_desc = json.loads(response.content.decode('utf-8'))
field_names = [field["name"] for field in form_desc["fields"]]
assert field_names == ['email', 'name', 'username', 'password', 'favorite_movie', 'favorite_editor',
'city', 'state', 'country', 'gender', 'year_of_birth', 'level_of_education',
'mailing_address', 'goals', 'honor_code']
assert field_names == [
"email",
"name",
"username",
"password",
"city",
"state",
"country",
"gender",
"year_of_birth",
"level_of_education",
"mailing_address",
"goals",
"honor_code",
"favorite_movie",
]

@override_settings(
REGISTRATION_EXTRA_FIELDS={
Expand Down Expand Up @@ -1358,9 +1372,23 @@ def test_field_order_invalid_override(self):
# Verify that all fields render in the correct order
form_desc = json.loads(response.content.decode('utf-8'))
field_names = [field["name"] for field in form_desc["fields"]]
assert field_names == ['email', 'name', 'username', 'password', 'favorite_movie', 'favorite_editor', 'city',
'state', 'country', 'gender', 'year_of_birth', 'level_of_education',
'mailing_address', 'goals', 'honor_code']

assert field_names == [
"name",
"password",
"gender",
"year_of_birth",
"level_of_education",
"mailing_address",
"goals",
"honor_code",
"city",
"country",
"email",
"favorite_movie",
"state",
"username",
]

def test_register(self):
# Create a new registration
Expand Down Expand Up @@ -1821,6 +1849,31 @@ def _assert_reg_field(self, extra_fields_setting, expected_field):

self._assert_fields_match(actual_field, expected_field)

def _assert_reg_absent_field(self, extra_fields_setting, expected_absent_field: str):
"""
Retrieve the registration form description from the server and
verify that it not contains the expected absent field.
Args:
extra_fields_setting (dict): Override the Django setting controlling
which extra fields are displayed in the form.
expected_absent_field (str): The field name we expect to be absent in the form.
Raises:
AssertionError
"""
# Retrieve the registration form description
with override_settings(REGISTRATION_EXTRA_FIELDS=extra_fields_setting):
response = self.client.get(self.url)
self.assertHttpOK(response)

# Verify that the form description matches what we'd expect
form_desc = json.loads(response.content.decode('utf-8'))

current_present_field_names = [field["name"] for field in form_desc["fields"]]
assert expected_absent_field not in current_present_field_names, \
"Expected absent field {expected}".format(expected=expected_absent_field)

def _assert_password_field_hidden(self, field_settings):
self._assert_reg_field(field_settings, {
"name": "password",
Expand Down Expand Up @@ -1900,9 +1953,23 @@ def test_field_order_invalid_override(self):
form_desc = json.loads(response.content.decode('utf-8'))
field_names = [field["name"] for field in form_desc["fields"]]

assert field_names == ['email', 'name', 'username', 'password', 'favorite_movie', 'favorite_editor',
'confirm_email', 'city', 'state', 'country', 'gender', 'year_of_birth',
'level_of_education', 'mailing_address', 'goals', 'honor_code']
assert field_names == [
"name",
"confirm_email",
"password",
"gender",
"year_of_birth",
"level_of_education",
"mailing_address",
"goals",
"honor_code",
"city",
"country",
"email",
"favorite_movie",
"state",
"username",
]

@override_settings(
REGISTRATION_EXTRA_FIELDS={
Expand Down Expand Up @@ -1967,6 +2034,7 @@ def test_field_order_override(self):
"honor_code": "required",
"confirm_email": "required",
},
REGISTRATION_FIELD_ORDER=None,
REGISTRATION_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm',
)
def test_field_order(self):
Expand All @@ -1976,10 +2044,23 @@ def test_field_order(self):
# Verify that all fields render in the correct order
form_desc = json.loads(response.content.decode('utf-8'))
field_names = [field["name"] for field in form_desc["fields"]]
assert field_names ==\
['email', 'name', 'username', 'password', 'favorite_movie', 'favorite_editor', 'confirm_email',
'city', 'state', 'country', 'gender', 'year_of_birth', 'level_of_education', 'mailing_address',
'goals', 'honor_code']
assert field_names == [
"email",
"name",
"username",
"password",
"confirm_email",
"city",
"state",
"country",
"gender",
"year_of_birth",
"level_of_education",
"mailing_address",
"goals",
"honor_code",
"favorite_movie",
]

def test_registration_form_confirm_email(self):
self._assert_reg_field(
Expand Down

0 comments on commit 6f0255b

Please sign in to comment.