From ed20c6a6a24ef59b077810f6a36471a50373f12c Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 11 Mar 2019 15:05:14 -0400 Subject: [PATCH 1/6] When saving a RadioField to the DB, check that what is being saved is one of the options --- atst/forms/forms.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/atst/forms/forms.py b/atst/forms/forms.py index e3f2081f..31e62476 100644 --- a/atst/forms/forms.py +++ b/atst/forms/forms.py @@ -18,11 +18,16 @@ class BaseForm(FlaskForm): def data(self): # remove 'csrf_token' key/value pair # remove empty strings and None from list fields + # prevent values that are not an option in a RadioField from being saved to the DB _data = super().data _data.pop("csrf_token", None) for field in _data: if _data[field].__class__.__name__ == "list": _data[field] = [el for el in _data[field] if el not in EMPTY_LIST_FIELD] + if self[field].__class__.__name__ == "RadioField": + choices = [el[0] for el in self[field].choices] + if _data[field] not in choices: + _data[field] = None return _data def validate(self, *args, **kwargs): From 1b642e22f600f9abf0e6cab4d9862168bac215f8 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 11 Mar 2019 15:15:35 -0400 Subject: [PATCH 2/6] Add in default values and filters to the TO Forms --- atst/forms/task_order.py | 58 ++++++++++++++++++++++++++++------ atst/routes/task_orders/new.py | 12 +++++-- 2 files changed, 58 insertions(+), 12 deletions(-) diff --git a/atst/forms/task_order.py b/atst/forms/task_order.py index cc24c14e..3740ffcd 100644 --- a/atst/forms/task_order.py +++ b/atst/forms/task_order.py @@ -31,6 +31,13 @@ class AppInfoWithExistingPortfolioForm(BaseForm): scope = TextAreaField( translate("forms.task_order.scope_label"), description=translate("forms.task_order.scope_description"), + filters=[lambda x: x or None], + ) + defense_component = SelectField( + translate("forms.task_order.defense_component_label"), + choices=SERVICE_BRANCHES, + default="", + filters=[lambda x: x or None], ) app_migration = RadioField( translate("forms.task_order.app_migration.label"), @@ -50,20 +57,30 @@ class AppInfoWithExistingPortfolioForm(BaseForm): translate("forms.task_order.complexity.label"), description=translate("forms.task_order.complexity.description"), choices=APPLICATION_COMPLEXITY, - default="", + default=None, + filters=[lambda x: x or None], widget=ListWidget(prefix_label=False), option_widget=CheckboxInput(), ) - complexity_other = StringField(translate("forms.task_order.complexity_other_label")) + complexity_other = StringField( + translate("forms.task_order.complexity_other_label"), + default=None, + filters=[lambda x: x or None], + ) dev_team = SelectMultipleField( translate("forms.task_order.dev_team.label"), description=translate("forms.task_order.dev_team.description"), choices=DEV_TEAM, - default="", + default=None, + filters=[lambda x: x or None], widget=ListWidget(prefix_label=False), option_widget=CheckboxInput(), ) - dev_team_other = StringField(translate("forms.task_order.dev_team_other_label")) + dev_team_other = StringField( + translate("forms.task_order.dev_team_other_label"), + default=None, + filters=[lambda x: x or None], + ) team_experience = RadioField( translate("forms.task_order.team_experience.label"), description=translate("forms.task_order.team_experience.description"), @@ -139,19 +156,26 @@ class UnclassifiedFundingForm(FundingForm): class OversightForm(BaseForm): ko_first_name = StringField( - translate("forms.task_order.oversight_first_name_label") + translate("forms.task_order.oversight_first_name_label"), + filters=[lambda x: x or None], + ) + ko_last_name = StringField( + translate("forms.task_order.oversight_last_name_label"), + filters=[lambda x: x or None], ) - ko_last_name = StringField(translate("forms.task_order.oversight_last_name_label")) ko_email = StringField( translate("forms.task_order.oversight_email_label"), validators=[Optional(), Email()], + filters=[lambda x: x or None], ) ko_phone_number = TelField( translate("forms.task_order.oversight_phone_label"), validators=[Optional(), PhoneNumber()], + filters=[lambda x: x or None], ) ko_dod_id = StringField( translate("forms.task_order.oversight_dod_id_label"), + filters=[lambda x: x or None], validators=[ RequiredIf(lambda form: form._fields.get("ko_invite").data), Length(min=10), @@ -161,15 +185,21 @@ class OversightForm(BaseForm): am_cor = BooleanField(translate("forms.task_order.oversight_am_cor_label")) cor_first_name = StringField( - translate("forms.task_order.oversight_first_name_label") + translate("forms.task_order.oversight_first_name_label"), + filters=[lambda x: x or None], + ) + cor_last_name = StringField( + translate("forms.task_order.oversight_last_name_label"), + filters=[lambda x: x or None], ) - cor_last_name = StringField(translate("forms.task_order.oversight_last_name_label")) cor_email = StringField( translate("forms.task_order.oversight_email_label"), + filters=[lambda x: x or None], validators=[Optional(), Email()], ) cor_phone_number = TelField( translate("forms.task_order.oversight_phone_label"), + filters=[lambda x: x or None], validators=[ RequiredIf(lambda form: not form._fields.get("am_cor").data), Optional(), @@ -178,6 +208,7 @@ class OversightForm(BaseForm): ) cor_dod_id = StringField( translate("forms.task_order.oversight_dod_id_label"), + filters=[lambda x: x or None], validators=[ RequiredIf( lambda form: not form._fields.get("am_cor").data @@ -189,19 +220,26 @@ class OversightForm(BaseForm): ) so_first_name = StringField( - translate("forms.task_order.oversight_first_name_label") + translate("forms.task_order.oversight_first_name_label"), + filters=[lambda x: x or None], + ) + so_last_name = StringField( + translate("forms.task_order.oversight_last_name_label"), + filters=[lambda x: x or None], ) - so_last_name = StringField(translate("forms.task_order.oversight_last_name_label")) so_email = StringField( translate("forms.task_order.oversight_email_label"), + filters=[lambda x: x or None], validators=[Optional(), Email()], ) so_phone_number = TelField( translate("forms.task_order.oversight_phone_label"), + filters=[lambda x: x or None], validators=[Optional(), PhoneNumber()], ) so_dod_id = StringField( translate("forms.task_order.oversight_dod_id_label"), + filters=[lambda x: x or None], validators=[ RequiredIf(lambda form: form._fields.get("so_invite").data), Length(min=10), diff --git a/atst/routes/task_orders/new.py b/atst/routes/task_orders/new.py index 799fd024..f7fe1b37 100644 --- a/atst/routes/task_orders/new.py +++ b/atst/routes/task_orders/new.py @@ -190,9 +190,17 @@ class UpdateTaskOrderWorkflow(ShowTaskOrderWorkflow): to_data.pop("defense_component") # don't save other text in DB unless "other" is checked - if "complexity" in to_data and "other" not in to_data["complexity"]: + if ( + "complexity" in to_data + and bool(to_data["complexity"]) + and "other" not in to_data["complexity"] + ): to_data["complexity_other"] = None - if "dev_team" in to_data and "other" not in to_data["dev_team"]: + if ( + "dev_team" in to_data + and bool(to_data["dev_team"]) + and "other" not in to_data["dev_team"] + ): to_data["dev_team_other"] = None if self.form_data.get("am_cor"): From 21a0168d7ecefb98640569c250794eae720f4c99 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 12 Mar 2019 10:07:17 -0400 Subject: [PATCH 3/6] Move filters and default to the AppInfoForm intead of parent form --- atst/forms/task_order.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/atst/forms/task_order.py b/atst/forms/task_order.py index 3740ffcd..0114161a 100644 --- a/atst/forms/task_order.py +++ b/atst/forms/task_order.py @@ -31,13 +31,9 @@ class AppInfoWithExistingPortfolioForm(BaseForm): scope = TextAreaField( translate("forms.task_order.scope_label"), description=translate("forms.task_order.scope_description"), - filters=[lambda x: x or None], ) defense_component = SelectField( - translate("forms.task_order.defense_component_label"), - choices=SERVICE_BRANCHES, - default="", - filters=[lambda x: x or None], + translate("forms.task_order.defense_component_label"), choices=SERVICE_BRANCHES ) app_migration = RadioField( translate("forms.task_order.app_migration.label"), @@ -94,6 +90,7 @@ class AppInfoForm(AppInfoWithExistingPortfolioForm): portfolio_name = StringField( translate("forms.task_order.portfolio_name_label"), description=translate("forms.task_order.portfolio_name_description"), + filters=[lambda x: x or None], validators=[ Required(), Length( @@ -104,7 +101,10 @@ class AppInfoForm(AppInfoWithExistingPortfolioForm): ], ) defense_component = SelectField( - translate("forms.task_order.defense_component_label"), choices=SERVICE_BRANCHES + translate("forms.task_order.defense_component_label"), + choices=SERVICE_BRANCHES, + default="", + filters=[lambda x: x or None], ) From 9277e5e4d0ddbd89cec88610e0252f64422f1c2d Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 12 Mar 2019 10:40:46 -0400 Subject: [PATCH 4/6] Move filter into a function --- atst/forms/task_order.py | 50 ++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/atst/forms/task_order.py b/atst/forms/task_order.py index 0114161a..21127261 100644 --- a/atst/forms/task_order.py +++ b/atst/forms/task_order.py @@ -27,6 +27,10 @@ from .data import ( from atst.utils.localization import translate +def remove_empty_string(value): + return value or None + + class AppInfoWithExistingPortfolioForm(BaseForm): scope = TextAreaField( translate("forms.task_order.scope_label"), @@ -54,28 +58,28 @@ class AppInfoWithExistingPortfolioForm(BaseForm): description=translate("forms.task_order.complexity.description"), choices=APPLICATION_COMPLEXITY, default=None, - filters=[lambda x: x or None], + filters=[remove_empty_string], widget=ListWidget(prefix_label=False), option_widget=CheckboxInput(), ) complexity_other = StringField( translate("forms.task_order.complexity_other_label"), default=None, - filters=[lambda x: x or None], + filters=[remove_empty_string], ) dev_team = SelectMultipleField( translate("forms.task_order.dev_team.label"), description=translate("forms.task_order.dev_team.description"), choices=DEV_TEAM, default=None, - filters=[lambda x: x or None], + filters=[remove_empty_string], widget=ListWidget(prefix_label=False), option_widget=CheckboxInput(), ) dev_team_other = StringField( translate("forms.task_order.dev_team_other_label"), default=None, - filters=[lambda x: x or None], + filters=[remove_empty_string], ) team_experience = RadioField( translate("forms.task_order.team_experience.label"), @@ -90,7 +94,7 @@ class AppInfoForm(AppInfoWithExistingPortfolioForm): portfolio_name = StringField( translate("forms.task_order.portfolio_name_label"), description=translate("forms.task_order.portfolio_name_description"), - filters=[lambda x: x or None], + filters=[remove_empty_string], validators=[ Required(), Length( @@ -104,7 +108,7 @@ class AppInfoForm(AppInfoWithExistingPortfolioForm): translate("forms.task_order.defense_component_label"), choices=SERVICE_BRANCHES, default="", - filters=[lambda x: x or None], + filters=[remove_empty_string], ) @@ -146,36 +150,36 @@ class FundingForm(BaseForm): class UnclassifiedFundingForm(FundingForm): clin_02 = StringField( translate("forms.task_order.unclassified_clin_02_label"), - filters=[lambda x: x or None], + filters=[remove_empty_string], ) clin_04 = StringField( translate("forms.task_order.unclassified_clin_04_label"), - filters=[lambda x: x or None], + filters=[remove_empty_string], ) class OversightForm(BaseForm): ko_first_name = StringField( translate("forms.task_order.oversight_first_name_label"), - filters=[lambda x: x or None], + filters=[remove_empty_string], ) ko_last_name = StringField( translate("forms.task_order.oversight_last_name_label"), - filters=[lambda x: x or None], + filters=[remove_empty_string], ) ko_email = StringField( translate("forms.task_order.oversight_email_label"), validators=[Optional(), Email()], - filters=[lambda x: x or None], + filters=[remove_empty_string], ) ko_phone_number = TelField( translate("forms.task_order.oversight_phone_label"), validators=[Optional(), PhoneNumber()], - filters=[lambda x: x or None], + filters=[remove_empty_string], ) ko_dod_id = StringField( translate("forms.task_order.oversight_dod_id_label"), - filters=[lambda x: x or None], + filters=[remove_empty_string], validators=[ RequiredIf(lambda form: form._fields.get("ko_invite").data), Length(min=10), @@ -186,20 +190,20 @@ class OversightForm(BaseForm): am_cor = BooleanField(translate("forms.task_order.oversight_am_cor_label")) cor_first_name = StringField( translate("forms.task_order.oversight_first_name_label"), - filters=[lambda x: x or None], + filters=[remove_empty_string], ) cor_last_name = StringField( translate("forms.task_order.oversight_last_name_label"), - filters=[lambda x: x or None], + filters=[remove_empty_string], ) cor_email = StringField( translate("forms.task_order.oversight_email_label"), - filters=[lambda x: x or None], + filters=[remove_empty_string], validators=[Optional(), Email()], ) cor_phone_number = TelField( translate("forms.task_order.oversight_phone_label"), - filters=[lambda x: x or None], + filters=[remove_empty_string], validators=[ RequiredIf(lambda form: not form._fields.get("am_cor").data), Optional(), @@ -208,7 +212,7 @@ class OversightForm(BaseForm): ) cor_dod_id = StringField( translate("forms.task_order.oversight_dod_id_label"), - filters=[lambda x: x or None], + filters=[remove_empty_string], validators=[ RequiredIf( lambda form: not form._fields.get("am_cor").data @@ -221,25 +225,25 @@ class OversightForm(BaseForm): so_first_name = StringField( translate("forms.task_order.oversight_first_name_label"), - filters=[lambda x: x or None], + filters=[remove_empty_string], ) so_last_name = StringField( translate("forms.task_order.oversight_last_name_label"), - filters=[lambda x: x or None], + filters=[remove_empty_string], ) so_email = StringField( translate("forms.task_order.oversight_email_label"), - filters=[lambda x: x or None], + filters=[remove_empty_string], validators=[Optional(), Email()], ) so_phone_number = TelField( translate("forms.task_order.oversight_phone_label"), - filters=[lambda x: x or None], + filters=[remove_empty_string], validators=[Optional(), PhoneNumber()], ) so_dod_id = StringField( translate("forms.task_order.oversight_dod_id_label"), - filters=[lambda x: x or None], + filters=[remove_empty_string], validators=[ RequiredIf(lambda form: form._fields.get("so_invite").data), Length(min=10), From 355ebddf893a7e44ab074b013cd832d779709675 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 12 Mar 2019 11:33:27 -0400 Subject: [PATCH 5/6] Add test for preventing values that are not a choice in a RadioField from being saved --- tests/forms/test_base_form.py | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 tests/forms/test_base_form.py diff --git a/tests/forms/test_base_form.py b/tests/forms/test_base_form.py new file mode 100644 index 00000000..fe6456e7 --- /dev/null +++ b/tests/forms/test_base_form.py @@ -0,0 +1,36 @@ +import pytest +from wtforms.fields import RadioField +from werkzeug.datastructures import ImmutableMultiDict + +from atst.forms.forms import BaseForm + + +class FormWithChoices(BaseForm): + force_side = RadioField( + "Choose your side", + choices=[ + ("light", "Light Side"), + ("dark", "Dark Side"), + ("neutral", "Chaotic Neutral"), + ], + ) + + +class TestBaseForm: + class Foo: + person = {"force_side": None} + + obj = Foo() + + def test_radio_field_saves_only_as_choice(self): + form_data_1 = ImmutableMultiDict({"force_side": "None"}) + form_1 = FormWithChoices(form_data_1, obj=self.obj) + assert form_1.data["force_side"] is None + + form_data_2 = ImmutableMultiDict({"force_side": "a fake choice"}) + form_2 = FormWithChoices(form_data_2, obj=self.obj) + assert form_2.data["force_side"] is None + + form_data_3 = ImmutableMultiDict({"force_side": "dark"}) + form_3 = FormWithChoices(form_data_3, obj=self.obj) + assert form_3.data["force_side"] is "dark" From e09162fd561cba743b582ed8b851942d98fbe773 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 12 Mar 2019 17:50:45 -0400 Subject: [PATCH 6/6] Fix rebase issue --- atst/forms/task_order.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/atst/forms/task_order.py b/atst/forms/task_order.py index 21127261..b5041d67 100644 --- a/atst/forms/task_order.py +++ b/atst/forms/task_order.py @@ -36,9 +36,6 @@ class AppInfoWithExistingPortfolioForm(BaseForm): translate("forms.task_order.scope_label"), description=translate("forms.task_order.scope_description"), ) - defense_component = SelectField( - translate("forms.task_order.defense_component_label"), choices=SERVICE_BRANCHES - ) app_migration = RadioField( translate("forms.task_order.app_migration.label"), description=translate("forms.task_order.app_migration.description"),