From 230795fb07dd32dd8f146a392285bf7bd23db978 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Boros?= Date: Fri, 12 Nov 2021 15:24:59 +0100 Subject: [PATCH] feat: add `optional-exposed` extra field type to registration form This defines optional extra fields that are not hidden under the toggle on the registration page. --- lms/envs/common.py | 3 +- .../js/spec/student_account/register_spec.js | 36 ++++++++++++++++++- .../js/student_account/views/RegisterView.js | 19 ++++++++-- openedx/core/djangoapps/user_api/helpers.py | 11 +++++- .../djangoapps/user_api/tests/test_helpers.py | 5 +-- .../user_authn/views/registration_form.py | 7 +++- .../user_authn/views/tests/test_login.py | 5 +-- .../views/tests/test_reset_password.py | 4 +-- 8 files changed, 78 insertions(+), 12 deletions(-) diff --git a/lms/envs/common.py b/lms/envs/common.py index 765b50775b..5014f095ff 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3437,7 +3437,8 @@ LOGIN_REDIRECT_WHITELIST = [] # 'terms_of_service': 'hidden', 'city': 'hidden', 'country': 'hidden'} # .. setting_description: The signup form may contain extra fields that are presented to every user. For every field, we # can specifiy whether it should be "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. +# the optional field as part of a toggled input field list; "optional-exposed": to display the optional fields among +# the required fields, and make it non-mandatory; "hidden": to not display the field. # When the terms of service are not visible and agreement to the honor code is required (the default), the signup page # includes a paragraph that links to the honor code page (defined my MKTG_URLS["HONOR"]). This page might not be # available for all Open edX platforms. In such cases, the "honor_code" registration field should be "hidden". diff --git a/lms/static/js/spec/student_account/register_spec.js b/lms/static/js/spec/student_account/register_spec.js index 9827c637bb..a41213a54f 100644 --- a/lms/static/js/spec/student_account/register_spec.js +++ b/lms/static/js/spec/student_account/register_spec.js @@ -88,6 +88,7 @@ defaultValue: '', type: 'email', required: true, + exposed: true, instructions: 'Enter your email.', restrictions: {} }, @@ -98,6 +99,7 @@ defaultValue: '', type: 'text', required: true, + exposed: true, instructions: 'Enter your email.', restrictions: {} }, @@ -108,6 +110,7 @@ defaultValue: '', type: 'text', required: true, + exposed: true, instructions: 'Enter your username.', restrictions: {} }, @@ -118,6 +121,7 @@ defaultValue: '', type: 'text', required: true, + exposed: true, instructions: 'Enter your username.', restrictions: {} }, @@ -128,6 +132,7 @@ defaultValue: '', type: 'password', required: true, + exposed: true, instructions: 'Enter your password.', restrictions: {} }, @@ -144,6 +149,7 @@ {value: 'b', name: "Bachelor's degree"} ], required: false, + exposed: false, instructions: 'Select your education level.', restrictions: {} }, @@ -160,6 +166,7 @@ {value: 'o', name: 'Other'} ], required: false, + exposed: false, instructions: 'Select your gender.', restrictions: {} }, @@ -176,6 +183,7 @@ {value: 2014, name: '2014'} ], required: false, + exposed: false, instructions: 'Select your year of birth.', restrictions: {} }, @@ -186,6 +194,7 @@ defaultValue: '', type: 'textarea', required: false, + exposed: false, instructions: 'Enter your mailing address.', restrictions: {} }, @@ -196,6 +205,7 @@ defaultValue: '', type: 'textarea', required: false, + exposed: false, instructions: "If you'd like, tell us why you're interested in edX.", restrictions: {} }, @@ -206,11 +216,12 @@ defaultValue: '', type: 'checkbox', required: true, + exposed: true, instructions: '', restrictions: {}, supplementalLink: '/honor', supplementalText: 'Review the Terms of Service and Honor Code' - } + }, ] }; var createRegisterView = function(that, formFields) { @@ -502,6 +513,29 @@ expect(view.$submitButton).toHaveAttr('disabled'); }); + it('shows optional exposed fields', function() { + var formFields = FORM_DESCRIPTION.fields + formFields.push({ + placeholder: '', + name: 'exposed_custom_optional_field', + label: 'Exposed custom optional field.', + defaultValue: '', + type: 'checkbox', + required: false, + exposed: true, + instructions: 'Check this field if you would like to.', + restrictions: {} + }) + + createRegisterView(this, formFields); + var elementClasses = view.$('.exposed-optional-fields').attr('class'); + var elementChildren = view.$('.exposed-optional-fields .form-field') + // Expect the exposed optional fields container does not have other + // classes assigned, like .hidden + expect(elementClasses).toEqual('exposed-optional-fields'); + expect(elementChildren.length).toEqual(1) + }); + it('hides optional fields by default', function() { createRegisterView(this); expect(view.$('.optional-fields')).toHaveClass('hidden'); diff --git a/lms/static/js/student_account/views/RegisterView.js b/lms/static/js/student_account/views/RegisterView.js index 48d5409414..5de123c2b0 100644 --- a/lms/static/js/student_account/views/RegisterView.js +++ b/lms/static/js/student_account/views/RegisterView.js @@ -101,7 +101,8 @@ field, len = data.length, requiredFields = [], - optionalFields = []; + optionalFields = [], + exposedOptionalFields = []; this.fields = data; @@ -123,12 +124,18 @@ // input elements that are visible on the page. this.hasOptionalFields = true; } - optionalFields.push(field); + + if (field.exposed) { + exposedOptionalFields.push(field); + } else { + optionalFields.push(field); + } } } html = this.renderFields(requiredFields, 'required-fields'); + html.push.apply(html, this.renderFields(exposedOptionalFields, 'exposed-optional-fields')); html.push.apply(html, this.renderFields( optionalFields, `optional-fields ${!this.enableCoppaCompliance ? '' : 'full-length-fields'}` )); @@ -251,6 +258,14 @@ $('.optional-fields').toggleClass('hidden'); }); + // Since the honor TOS text has a composed css selector, it is more future proof + // to insert the not toggled optional fields before .honor_tos_combined's parent + // that is the container for the honor TOS text and checkbox. + // xss-lint: disable=javascript-jquery-insert-into-target + $('.exposed-optional-fields').insertBefore( + $('.honor_tos_combined').parent() + ); + // We are swapping the order of these elements here because the honor code agreement // is a required checkbox field and the optional fields toggle is a cosmetic // improvement so that we don't have to show all the optional fields. diff --git a/openedx/core/djangoapps/user_api/helpers.py b/openedx/core/djangoapps/user_api/helpers.py index 72f654e3e8..7de43410a8 100644 --- a/openedx/core/djangoapps/user_api/helpers.py +++ b/openedx/core/djangoapps/user_api/helpers.py @@ -135,7 +135,7 @@ class FormDescription: def add_field( self, name, label="", field_type="text", default="", - placeholder="", instructions="", required=True, restrictions=None, + placeholder="", instructions="", exposed=None, required=True, restrictions=None, options=None, include_default_option=False, error_messages=None, supplementalLink="", supplementalText="" ): @@ -159,6 +159,9 @@ class FormDescription: instructions (unicode): Short instructions for using the field (e.g. "This is the email address you used when you registered.") + exposed (boolean): Whether the field is shown if not required. + If the field is not set, the field will be visible if it's required. + required (boolean): Whether the field is required or optional. restrictions (dict): Validation restrictions for the field. @@ -195,6 +198,9 @@ class FormDescription: ) raise InvalidFieldError(msg) + if exposed is None: + exposed = required + field_dict = { "name": name, "label": label, @@ -202,6 +208,7 @@ class FormDescription: "defaultValue": default, "placeholder": placeholder, "instructions": instructions, + "exposed": exposed, "required": required, "restrictions": {}, "errorMessages": {}, @@ -268,6 +275,7 @@ class FormDescription: "label": "Cheese or Wine?", "defaultValue": "cheese", "type": "select", + "exposed": True, "required": True, "placeholder": "", "instructions": "", @@ -283,6 +291,7 @@ class FormDescription: "label": "comments", "defaultValue": "", "type": "text", + "exposed": False, "required": False, "placeholder": "Any comments?", "instructions": "Please enter additional comments here." diff --git a/openedx/core/djangoapps/user_api/tests/test_helpers.py b/openedx/core/djangoapps/user_api/tests/test_helpers.py index 0d7ef25b27..274e1a265b 100644 --- a/openedx/core/djangoapps/user_api/tests/test_helpers.py +++ b/openedx/core/djangoapps/user_api/tests/test_helpers.py @@ -95,6 +95,7 @@ class FormDescriptionTest(TestCase): placeholder="placeholder", instructions="instructions", required=True, + exposed=True, restrictions={ "min_length": 2, "max_length": 10 @@ -110,8 +111,8 @@ class FormDescriptionTest(TestCase): json.dumps({'method': 'post', 'submit_url': '/submit', 'fields': [{'name': 'name', 'label': 'label', 'type': 'text', 'defaultValue': 'default', - 'placeholder': 'placeholder', 'instructions': 'instructions', 'required': True, - 'restrictions': {'min_length': 2, 'max_length': 10}, + 'placeholder': 'placeholder', 'instructions': 'instructions', 'exposed': True, + 'required': True, 'restrictions': {'min_length': 2, 'max_length': 10}, 'errorMessages': {'required': 'You must provide a value!'}, 'supplementalLink': '', 'supplementalText': '', 'loginIssueSupportLink': 'https://support.example.com/login-issue-help.html'}]}) diff --git a/openedx/core/djangoapps/user_authn/views/registration_form.py b/openedx/core/djangoapps/user_authn/views/registration_form.py index 9ab83e1547..1c96a6bbda 100644 --- a/openedx/core/djangoapps/user_authn/views/registration_form.py +++ b/openedx/core/djangoapps/user_authn/views/registration_form.py @@ -328,12 +328,16 @@ class RegistrationFormFactory: def _is_field_visible(self, field_name): """Check whether a field is visible based on Django settings. """ - return self._extra_fields_setting.get(field_name) in ["required", "optional"] + return self._extra_fields_setting.get(field_name) in ["required", "optional", "optional-exposed"] def _is_field_required(self, field_name): """Check whether a field is required based on Django settings. """ return self._extra_fields_setting.get(field_name) == "required" + def _is_field_exposed(self, field_name): + """Check whether a field is optional and should be toggled. """ + return self._extra_fields_setting.get(field_name) in ["required", "optional-exposed"] + def __init__(self): if settings.ENABLE_COPPA_COMPLIANCE and 'year_of_birth' in self.EXTRA_FIELDS: @@ -441,6 +445,7 @@ class RegistrationFormFactory: FormDescription.FIELD_TYPE_MAP.get(field.__class__)), placeholder=field.initial, instructions=field.help_text, + exposed=self._is_field_exposed(field_name), required=(self._is_field_required(field_name) or field.required), restrictions=restrictions, options=getattr(field, 'choices', None), error_messages=field.error_messages, diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_login.py b/openedx/core/djangoapps/user_authn/views/tests/test_login.py index 8c5a085215..daf6b7f660 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -1020,8 +1020,8 @@ class LoginSessionViewTest(ApiTestCase, OpenEdxEventsTestMixin): form_desc = json.loads(response.content.decode('utf-8')) assert form_desc['method'] == 'post' assert form_desc['submit_url'] == reverse('user_api_login_session', kwargs={'api_version': 'v1'}) - assert form_desc['fields'] == [{'name': 'email', 'defaultValue': '', 'type': 'email', 'required': True, - 'label': 'Email', 'placeholder': '', + assert form_desc['fields'] == [{'name': 'email', 'defaultValue': '', 'type': 'email', 'exposed': True, + 'required': True, 'label': 'Email', 'placeholder': '', 'instructions': 'The email address you used to register with {platform_name}' .format(platform_name=settings.PLATFORM_NAME), 'restrictions': {'min_length': EMAIL_MIN_LENGTH, @@ -1033,6 +1033,7 @@ class LoginSessionViewTest(ApiTestCase, OpenEdxEventsTestMixin): {'name': 'password', 'defaultValue': '', 'type': 'password', + 'exposed': True, 'required': True, 'label': 'Password', 'placeholder': '', diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py b/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py index ba8f73b35a..e79e5fa24b 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py @@ -710,8 +710,8 @@ class PasswordResetViewTest(UserAPITestCase): assert form_desc['method'] == 'post' assert form_desc['submit_url'] == reverse('password_change_request') assert form_desc['fields'] ==\ - [{'name': 'email', 'defaultValue': '', 'type': 'email', 'required': True, - 'label': 'Email', 'placeholder': 'username@domain.com', + [{'name': 'email', 'defaultValue': '', 'type': 'email', 'exposed': True, + 'required': True, 'label': 'Email', 'placeholder': 'username@domain.com', 'instructions': 'The email address you used to register with {platform_name}' .format(platform_name=settings.PLATFORM_NAME), 'restrictions': {'min_length': EMAIL_MIN_LENGTH,