diff --git a/.secrets.baseline b/.secrets.baseline index 9e94b782..1060e869 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": "^.secrets.baseline$|^.*pgsslrootcert.yml$", "lines": null }, - "generated_at": "2019-11-01T18:50:54Z", + "generated_at": "2019-11-06T21:35:23Z", "plugins_used": [ { "base64_limit": 4.5, @@ -170,15 +170,6 @@ "type": "Private Key" } ], - "tests/forms/test_validators.py": [ - { - "hashed_secret": "260408f687da9094705a841acda9b029563053ee", - "is_secret": false, - "is_verified": false, - "line_number": 30, - "type": "Hex High Entropy String" - } - ], "tests/routes/task_orders/test_new.py": [ { "hashed_secret": "e4f14805dfd1e6af030359090c535e149e6b4207", diff --git a/atst/forms/application.py b/atst/forms/application.py index 16db3827..de6294cd 100644 --- a/atst/forms/application.py +++ b/atst/forms/application.py @@ -1,4 +1,4 @@ -from .forms import BaseForm +from .forms import BaseForm, remove_empty_string from wtforms.fields import StringField, TextAreaField, FieldList from wtforms.validators import Required, Optional from atst.forms.validators import ListItemRequired, ListItemsUnique @@ -7,24 +7,31 @@ from atst.utils.localization import translate class EditEnvironmentForm(BaseForm): name = StringField( - label=translate("forms.environments.name_label"), validators=[Required()] + label=translate("forms.environments.name_label"), + validators=[Required()], + filters=[remove_empty_string], ) class NameAndDescriptionForm(BaseForm): name = StringField( - label=translate("forms.application.name_label"), validators=[Required()] + label=translate("forms.application.name_label"), + validators=[Required()], + filters=[remove_empty_string], ) description = TextAreaField( label=translate("forms.application.description_label"), validators=[Optional()], - filters=[lambda x: x or None], + filters=[remove_empty_string], ) class EnvironmentsForm(BaseForm): environment_names = FieldList( - StringField(label=translate("forms.application.environment_names_label")), + StringField( + label=translate("forms.application.environment_names_label"), + filters=[remove_empty_string], + ), validators=[ ListItemRequired( message=translate( diff --git a/atst/forms/forms.py b/atst/forms/forms.py index 4287c81c..cd63b278 100644 --- a/atst/forms/forms.py +++ b/atst/forms/forms.py @@ -1,11 +1,20 @@ from flask_wtf import FlaskForm from flask import current_app, request as http_request +import re from atst.utils.flash import formatted_flash as flash EMPTY_LIST_FIELD = ["", None] +def remove_empty_string(value): + # only return strings that contain non whitespace characters + if value and re.search(r"\S", value): + return value.strip() + else: + return None + + class BaseForm(FlaskForm): def __init__(self, formdata=None, **kwargs): # initialize the form with data from the cache @@ -35,7 +44,3 @@ class BaseForm(FlaskForm): if not valid and flash_invalid: flash("form_errors") return valid - - @classmethod - def remove_empty_string(cls, value): - return value or None diff --git a/atst/forms/portfolio.py b/atst/forms/portfolio.py index 6e807876..307f463a 100644 --- a/atst/forms/portfolio.py +++ b/atst/forms/portfolio.py @@ -8,7 +8,7 @@ from wtforms.fields import ( from wtforms.validators import Length, Optional from wtforms.widgets import ListWidget, CheckboxInput -from .forms import BaseForm +from .forms import BaseForm, remove_empty_string from atst.utils.localization import translate from .data import ( @@ -49,7 +49,7 @@ class PortfolioCreationForm(BaseForm): translate("forms.task_order.defense_component_label"), choices=SERVICE_BRANCHES, default="", - filters=[BaseForm.remove_empty_string], + filters=[remove_empty_string], ) description = TextAreaField( @@ -78,7 +78,6 @@ class PortfolioCreationForm(BaseForm): description=translate("forms.task_order.complexity.description"), choices=APPLICATION_COMPLEXITY, default=None, - filters=[BaseForm.remove_empty_string], widget=ListWidget(prefix_label=False), option_widget=CheckboxInput(), ) @@ -86,7 +85,7 @@ class PortfolioCreationForm(BaseForm): complexity_other = StringField( translate("forms.task_order.complexity_other_label"), default=None, - filters=[BaseForm.remove_empty_string], + filters=[remove_empty_string], ) dev_team = SelectMultipleField( @@ -94,7 +93,6 @@ class PortfolioCreationForm(BaseForm): description=translate("forms.task_order.dev_team.description"), choices=DEV_TEAM, default=None, - filters=[BaseForm.remove_empty_string], widget=ListWidget(prefix_label=False), option_widget=CheckboxInput(), ) @@ -102,7 +100,7 @@ class PortfolioCreationForm(BaseForm): dev_team_other = StringField( translate("forms.task_order.dev_team_other_label"), default=None, - filters=[BaseForm.remove_empty_string], + filters=[remove_empty_string], ) team_experience = RadioField( diff --git a/atst/forms/validators.py b/atst/forms/validators.py index cac21e3e..a16bf4f5 100644 --- a/atst/forms/validators.py +++ b/atst/forms/validators.py @@ -65,10 +65,12 @@ def Name(message=translate("forms.validators.name_message")): def ListItemRequired( message=translate("forms.validators.list_item_required_message"), - empty_values=("", None), + empty_values=[None], ): def _list_item_required(form, field): - non_empty_values = [v for v in field.data if v not in empty_values] + non_empty_values = [ + v for v in field.data if (v not in empty_values and re.search(r"\S", v)) + ] if len(non_empty_values) == 0: raise ValidationError(message) diff --git a/js/components/forms/new_application/environments.js b/js/components/forms/new_application/environments.js index 655fd817..e2a49c5e 100644 --- a/js/components/forms/new_application/environments.js +++ b/js/components/forms/new_application/environments.js @@ -91,7 +91,9 @@ export default { if (this.environments.length > 1) { // only want to display this error if we have multiple envs and one or // more do not have names - return this.environments.every(e => e.name !== '') + return this.environments.every( + e => !e.name.match(/^\s+$/) && e.name !== '' + ) } else { return true } diff --git a/tests/forms/test_base_form.py b/tests/forms/test_base_form.py index fe6456e7..8f0c000e 100644 --- a/tests/forms/test_base_form.py +++ b/tests/forms/test_base_form.py @@ -1,8 +1,8 @@ import pytest -from wtforms.fields import RadioField +from wtforms.fields import RadioField, FieldList, StringField from werkzeug.datastructures import ImmutableMultiDict -from atst.forms.forms import BaseForm +from atst.forms.forms import BaseForm, remove_empty_string class FormWithChoices(BaseForm): @@ -16,6 +16,12 @@ class FormWithChoices(BaseForm): ) +class FormWithList(BaseForm): + fancy_list = FieldList( + StringField("a very fancy list", filters=[remove_empty_string]) + ) + + class TestBaseForm: class Foo: person = {"force_side": None} @@ -34,3 +40,17 @@ class TestBaseForm: form_data_3 = ImmutableMultiDict({"force_side": "dark"}) form_3 = FormWithChoices(form_data_3, obj=self.obj) assert form_3.data["force_side"] is "dark" + + @pytest.mark.parametrize( + "form_data", + [["testing", "", "QA"], ["testing", " ", "QA"], ["testing", None, "QA"]], + ) + def test_blank_list_items_removed(self, form_data): + form = FormWithList(fancy_list=form_data) + assert form.validate(flash_invalid=False) + assert not form.data == ["testing", "QA"] + + def test_remove_empty_string_clips_whitespace(self): + form = FormWithList(fancy_list=[" QA", " testing "]) + assert form.validate(flash_invalid=False) + assert form.fancy_list.data == ["QA", "testing"] diff --git a/tests/forms/test_validators.py b/tests/forms/test_validators.py index f19d06cb..bac06f47 100644 --- a/tests/forms/test_validators.py +++ b/tests/forms/test_validators.py @@ -85,3 +85,20 @@ class TestFileLength: dummy_field.data = "random string" assert validator(dummy_form, dummy_field) + + +class TestListItemRequired: + @pytest.mark.parametrize("valid", [[" a", ""], ["a ", ""], ["a", ""]]) + def test_ListItemRequired(self, valid, dummy_form, dummy_field): + validator = ListItemRequired() + dummy_field.data = valid + validator(dummy_form, dummy_field) + + @pytest.mark.parametrize("invalid", [[""], [" "], [None], []]) + def test_ListItemRequired_rejects_blank_names( + self, invalid, dummy_form, dummy_field + ): + validator = ListItemRequired() + dummy_field.data = invalid + with pytest.raises(ValidationError): + validator(dummy_form, dummy_field)