Commit cb5c03f0 by AlasdairSwan

ECOM-626 Added required check for select dropdowns and validation

parent ad3c11e5
...@@ -125,7 +125,7 @@ class FormDescription(object): ...@@ -125,7 +125,7 @@ class FormDescription(object):
def add_field( def add_field(
self, name, label=u"", field_type=u"text", default=u"", self, name, label=u"", field_type=u"text", default=u"",
placeholder=u"", instructions=u"", required=True, restrictions=None, placeholder=u"", instructions=u"", required=True, restrictions=None,
options=None, error_messages=None options=None, include_default_option=False, error_messages=None
): ):
"""Add a field to the form description. """Add a field to the form description.
...@@ -158,6 +158,9 @@ class FormDescription(object): ...@@ -158,6 +158,9 @@ class FormDescription(object):
and `display_name` is the name to display to the user. and `display_name` is the name to display to the user.
If the field type is "select", you *must* provide this kwarg. If the field type is "select", you *must* provide this kwarg.
include_default_option (boolean): If True, include a "default" empty option
at the beginning of the options list.
error_messages (dict): Custom validation error messages. error_messages (dict): Custom validation error messages.
Currently, the only supported key is "required" indicating Currently, the only supported key is "required" indicating
that the messages should be displayed if the user does that the messages should be displayed if the user does
...@@ -188,10 +191,20 @@ class FormDescription(object): ...@@ -188,10 +191,20 @@ class FormDescription(object):
if field_type == "select": if field_type == "select":
if options is not None: if options is not None:
field_dict["options"] = [ field_dict["options"] = []
# Include an empty "default" option at the beginning of the list
if include_default_option:
field_dict["options"].append({
"value": "",
"name": "--",
"default": True
})
field_dict["options"].extend([
{"value": option_value, "name": option_name} {"value": option_value, "name": option_name}
for option_value, option_name in options for option_value, option_name in options
] ])
else: else:
raise InvalidFieldError("You must provide options for a select field.") raise InvalidFieldError("You must provide options for a select field.")
......
...@@ -955,7 +955,7 @@ class RegistrationViewTest(ApiTestCase): ...@@ -955,7 +955,7 @@ class RegistrationViewTest(ApiTestCase):
"required": False, "required": False,
"label": "Highest Level of Education Completed", "label": "Highest Level of Education Completed",
"options": [ "options": [
{"value": "", "name": "--"}, {"value": "", "name": "--", "default": True},
{"value": "p", "name": "Doctorate"}, {"value": "p", "name": "Doctorate"},
{"value": "m", "name": "Master's or professional degree"}, {"value": "m", "name": "Master's or professional degree"},
{"value": "b", "name": "Bachelor's degree"}, {"value": "b", "name": "Bachelor's degree"},
...@@ -978,7 +978,7 @@ class RegistrationViewTest(ApiTestCase): ...@@ -978,7 +978,7 @@ class RegistrationViewTest(ApiTestCase):
"required": False, "required": False,
"label": "Gender", "label": "Gender",
"options": [ "options": [
{"value": "", "name": "--"}, {"value": "", "name": "--", "default": True},
{"value": "m", "name": "Male"}, {"value": "m", "name": "Male"},
{"value": "f", "name": "Female"}, {"value": "f", "name": "Female"},
{"value": "o", "name": "Other"}, {"value": "o", "name": "Other"},
...@@ -989,7 +989,7 @@ class RegistrationViewTest(ApiTestCase): ...@@ -989,7 +989,7 @@ class RegistrationViewTest(ApiTestCase):
def test_register_form_year_of_birth(self): def test_register_form_year_of_birth(self):
this_year = datetime.datetime.now(UTC).year this_year = datetime.datetime.now(UTC).year
year_options = ( year_options = (
[{"value": "", "name": "--"}] + [ [{"value": "", "name": "--", "default": True}] + [
{"value": unicode(year), "name": unicode(year)} {"value": unicode(year), "name": unicode(year)}
for year in range(this_year, this_year - 120, -1) for year in range(this_year, this_year - 120, -1)
] ]
...@@ -1042,7 +1042,7 @@ class RegistrationViewTest(ApiTestCase): ...@@ -1042,7 +1042,7 @@ class RegistrationViewTest(ApiTestCase):
def test_registration_form_country(self): def test_registration_form_country(self):
country_options = ( country_options = (
[{"name": "--", "value": ""}] + [{"name": "--", "value": "", "default": True}] +
[ [
{"value": country_code, "name": unicode(country_name)} {"value": country_code, "name": unicode(country_name)}
for country_code, country_name in SORTED_COUNTRIES for country_code, country_name in SORTED_COUNTRIES
......
...@@ -379,7 +379,8 @@ class RegistrationView(APIView): ...@@ -379,7 +379,8 @@ class RegistrationView(APIView):
"level_of_education", "level_of_education",
label=education_level_label, label=education_level_label,
field_type="select", field_type="select",
options=self._options_with_default(UserProfile.LEVEL_OF_EDUCATION_CHOICES), options=UserProfile.LEVEL_OF_EDUCATION_CHOICES,
include_default_option=True,
required=required required=required
) )
...@@ -392,7 +393,8 @@ class RegistrationView(APIView): ...@@ -392,7 +393,8 @@ class RegistrationView(APIView):
"gender", "gender",
label=gender_label, label=gender_label,
field_type="select", field_type="select",
options=self._options_with_default(UserProfile.GENDER_CHOICES), options=UserProfile.GENDER_CHOICES,
include_default_option=True,
required=required required=required
) )
...@@ -406,7 +408,8 @@ class RegistrationView(APIView): ...@@ -406,7 +408,8 @@ class RegistrationView(APIView):
"year_of_birth", "year_of_birth",
label=yob_label, label=yob_label,
field_type="select", field_type="select",
options=self._options_with_default(options), options=options,
include_default_option=True,
required=required required=required
) )
...@@ -463,7 +466,8 @@ class RegistrationView(APIView): ...@@ -463,7 +466,8 @@ class RegistrationView(APIView):
"country", "country",
label=country_label, label=country_label,
field_type="select", field_type="select",
options=self._options_with_default(options), options=options,
include_default_option=True,
required=required required=required
) )
...@@ -534,12 +538,6 @@ class RegistrationView(APIView): ...@@ -534,12 +538,6 @@ class RegistrationView(APIView):
} }
) )
def _options_with_default(self, options):
"""Include a default option as the first option. """
return (
[("", "--")] + list(options)
)
def _apply_third_party_auth_overrides(self, request, form_desc): def _apply_third_party_auth_overrides(self, request, form_desc):
"""Modify the registration form if the user has authenticated with a third-party provider. """Modify the registration form if the user has authenticated with a third-party provider.
......
...@@ -52,10 +52,10 @@ describe('edx.utils.validate', function () { ...@@ -52,10 +52,10 @@ describe('edx.utils.validate', function () {
createFixture('text', 'username', true, MIN_LENGTH, MAX_LENGTH, ''); createFixture('text', 'username', true, MIN_LENGTH, MAX_LENGTH, '');
expectInvalid(REQUIRED_ERROR_FRAGMENT); expectInvalid(REQUIRED_ERROR_FRAGMENT);
}); });
it('fails if a field is provided a value below its minimum character limit', function () { it('fails if a field is provided a value below its minimum character limit', function () {
createFixture('text', 'username', false, MIN_LENGTH, MAX_LENGTH, SHORT_STRING); createFixture('text', 'username', false, MIN_LENGTH, MAX_LENGTH, SHORT_STRING);
// Verify optional field behavior // Verify optional field behavior
expectInvalid(MIN_ERROR_FRAGMENT); expectInvalid(MIN_ERROR_FRAGMENT);
...@@ -66,7 +66,7 @@ describe('edx.utils.validate', function () { ...@@ -66,7 +66,7 @@ describe('edx.utils.validate', function () {
it('succeeds if a field with no minimum character limit is provided a value below its maximum character limit', function () { it('succeeds if a field with no minimum character limit is provided a value below its maximum character limit', function () {
createFixture('text', 'username', false, null, MAX_LENGTH, SHORT_STRING); createFixture('text', 'username', false, null, MAX_LENGTH, SHORT_STRING);
// Verify optional field behavior // Verify optional field behavior
expectValid(); expectValid();
...@@ -154,6 +154,31 @@ describe('edx.utils.validate', function () { ...@@ -154,6 +154,31 @@ describe('edx.utils.validate', function () {
expectInvalid(REQUIRED_ERROR_FRAGMENT); expectInvalid(REQUIRED_ERROR_FRAGMENT);
}); });
it('succeeds if a select is optional, or required and default is selected, but fails if a required select has the default option selected', function () {
var select = [
'<select id="dropdown" name="country">',
'<option value="" data-isdefault="true">Please select a country</option>',
'<option value="BE">Belgium</option>',
'<option value="DE">Germany</option>',
'</select>'
].join('');
setFixtures(select);
dropdown = $('#dropdown');
// Optional
expectValid();
// Required, default text selected
dropdown.attr('required', true);
expectInvalid(REQUIRED_ERROR_FRAGMENT);
// Required, country selected
dropdown.val('BE');
expectValid();
});
it('returns a custom error message if an invalid field has one attached', function () { it('returns a custom error message if an invalid field has one attached', function () {
// Create a blank required field // Create a blank required field
createFixture('text', 'username', true, MIN_LENGTH, MAX_LENGTH, ''); createFixture('text', 'username', true, MIN_LENGTH, MAX_LENGTH, '');
......
...@@ -87,7 +87,18 @@ var edx = edx || {}; ...@@ -87,7 +87,18 @@ var edx = edx || {};
}, },
isBlank: function( $el ) { isBlank: function( $el ) {
return ( $el.attr('type') === 'checkbox' ) ? !$el.prop('checked') : !$el.val(); var type = $el.attr('type'),
isBlank;
if ( type === 'checkbox' ) {
isBlank = !$el.prop('checked');
} else if ( type === 'select' ) {
isBlank = ( $el.data('isdefault') === true );
} else {
isBlank = !$el.val();
}
return isBlank;
}, },
email: { email: {
......
...@@ -208,6 +208,10 @@ ...@@ -208,6 +208,10 @@
select { select {
width: 100%; width: 100%;
&.error {
border-color: tint($red,50%);
}
} }
/** FROM _accounts.scss - end **/ /** FROM _accounts.scss - end **/
} }
......
...@@ -10,9 +10,10 @@ ...@@ -10,9 +10,10 @@
<select id="<%= form %>-<%= name %>" <select id="<%= form %>-<%= name %>"
name="<%= name %>" name="<%= name %>"
class="input-inline" class="input-inline"
aria-describedby="<%= form %>-<%= name %>-desc"> aria-describedby="<%= form %>-<%= name %>-desc"
<% if ( required ) { %> required<% } %>>
<% _.each(options, function(el) { %> <% _.each(options, function(el) { %>
<option value="<%= el.value%>"><%= el.name %></option> <option value="<%= el.value%>"<% if ( el.default ) { %> data-isdefault="true"<% } %>><%= el.name %></option>
<% }); %> <% }); %>
</select> </select>
<% } else if ( type === 'textarea' ) { %> <% } else if ( type === 'textarea' ) { %>
......
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