Merge pull request #1162 from dod-ccpo/env-name-bugfix

Env name bugfix

.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",

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(

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

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(

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)
 

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
       }

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"]

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)