From a097a0ce6150a5cb111c4dddedff4602f7a21b10 Mon Sep 17 00:00:00 2001 From: graham-dds Date: Wed, 11 Dec 2019 11:30:27 -0500 Subject: [PATCH 01/11] Refactor New Portfolio page according to designs. New designs call for a streamlined New Portfolio page, with far fewer input options. This commit refactors that page according to those designs. Some of the route functions in this commit refer to a "step 1" of creating a new Portfolio. Though there is no "step 2" right now, the designs call for a multistep flow for Portfolio creation process, so this commit sets the stage for that. --- ...cd013_remove_unneeded_portfolio_columns.py | 40 +++++++ atst/forms/data.py | 110 +----------------- atst/forms/portfolio.py | 89 ++------------ atst/models/portfolio.py | 11 +- atst/routes/portfolios/index.py | 7 +- load-test/locustfile.py | 11 +- templates/home.html | 2 +- templates/portfolios/new.html | 54 --------- templates/portfolios/new/step_1.html | 45 +++++++ tests/factories.py | 6 - tests/routes/portfolios/test_index.py | 4 +- tests/test_access.py | 2 +- translations.yaml | 82 +++++++------ 13 files changed, 157 insertions(+), 306 deletions(-) create mode 100644 alembic/versions/802071bcd013_remove_unneeded_portfolio_columns.py delete mode 100644 templates/portfolios/new.html create mode 100644 templates/portfolios/new/step_1.html diff --git a/alembic/versions/802071bcd013_remove_unneeded_portfolio_columns.py b/alembic/versions/802071bcd013_remove_unneeded_portfolio_columns.py new file mode 100644 index 00000000..adc6b824 --- /dev/null +++ b/alembic/versions/802071bcd013_remove_unneeded_portfolio_columns.py @@ -0,0 +1,40 @@ +"""Remove unneeded portfolio columns + +Revision ID: 802071bcd013 +Revises: 67a2151d6269 +Create Date: 2019-12-11 13:26:34.770480 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '802071bcd013' # pragma: allowlist secret +down_revision = '67a2151d6269' # pragma: allowlist secret +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('portfolios', 'dev_team') + op.drop_column('portfolios', 'complexity') + op.drop_column('portfolios', 'team_experience') + op.drop_column('portfolios', 'dev_team_other') + op.drop_column('portfolios', 'app_migration') + op.drop_column('portfolios', 'native_apps') + op.drop_column('portfolios', 'complexity_other') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('portfolios', sa.Column('complexity_other', sa.VARCHAR(), autoincrement=False, nullable=True)) + op.add_column('portfolios', sa.Column('native_apps', sa.VARCHAR(), autoincrement=False, nullable=True)) + op.add_column('portfolios', sa.Column('app_migration', sa.VARCHAR(), autoincrement=False, nullable=True)) + op.add_column('portfolios', sa.Column('dev_team_other', sa.VARCHAR(), autoincrement=False, nullable=True)) + op.add_column('portfolios', sa.Column('team_experience', sa.VARCHAR(), autoincrement=False, nullable=True)) + op.add_column('portfolios', sa.Column('complexity', postgresql.ARRAY(sa.VARCHAR()), autoincrement=False, nullable=True)) + op.add_column('portfolios', sa.Column('dev_team', postgresql.ARRAY(sa.VARCHAR()), autoincrement=False, nullable=True)) + # ### end Alembic commands ### diff --git a/atst/forms/data.py b/atst/forms/data.py index 7b1b4a40..5f1f9532 100644 --- a/atst/forms/data.py +++ b/atst/forms/data.py @@ -3,112 +3,14 @@ from atst.utils.localization import translate SERVICE_BRANCHES = [ - ("", "- Select -"), - ("Air Force, Department of the", "Air Force, Department of the"), - ("Army and Air Force Exchange Service", "Army and Air Force Exchange Service"), - ("Army, Department of the", "Army, Department of the"), + ("air_force", translate("forms.portfolio.defense_component.choices.air_force")), + ("army", translate("forms.portfolio.defense_component.choices.army")), ( - "Defense Advanced Research Applications Agency", - "Defense Advanced Research Applications Agency", + "marine_corps", + translate("forms.portfolio.defense_component.choices.marine_corps"), ), - ("Defense Commissary Agency", "Defense Commissary Agency"), - ("Defense Contract Audit Agency", "Defense Contract Audit Agency"), - ("Defense Contract Management Agency", "Defense Contract Management Agency"), - ("Defense Finance & Accounting Service", "Defense Finance & Accounting Service"), - ("Defense Health Agency", "Defense Health Agency"), - ("Defense Information System Agency", "Defense Information System Agency"), - ("Defense Intelligence Agency", "Defense Intelligence Agency"), - ("Defense Legal Services Agency", "Defense Legal Services Agency"), - ("Defense Logistics Agency", "Defense Logistics Agency"), - ("Defense Media Activity", "Defense Media Activity"), - ("Defense Micro Electronics Activity", "Defense Micro Electronics Activity"), - ("Defense POW-MIA Accounting Agency", "Defense POW-MIA Accounting Agency"), - ("Defense Security Cooperation Agency", "Defense Security Cooperation Agency"), - ("Defense Security Service", "Defense Security Service"), - ("Defense Technical Information Center", "Defense Technical Information Center"), - ( - "Defense Technology Security Administration", - "Defense Technology Security Administration", - ), - ("Defense Threat Reduction Agency", "Defense Threat Reduction Agency"), - ("DoD Education Activity", "DoD Education Activity"), - ("DoD Human Recourses Activity", "DoD Human Recourses Activity"), - ("DoD Inspector General", "DoD Inspector General"), - ("DoD Test Resource Management Center", "DoD Test Resource Management Center"), - ( - "Headquarters Defense Human Resource Activity ", - "Headquarters Defense Human Resource Activity ", - ), - ("Joint Staff", "Joint Staff"), - ("Missile Defense Agency", "Missile Defense Agency"), - ("National Defense University", "National Defense University"), - ( - "National Geospatial Intelligence Agency (NGA)", - "National Geospatial Intelligence Agency (NGA)", - ), - ( - "National Oceanic and Atmospheric Administration (NOAA)", - "National Oceanic and Atmospheric Administration (NOAA)", - ), - ("National Reconnaissance Office", "National Reconnaissance Office"), - ("National Reconnaissance Office (NRO)", "National Reconnaissance Office (NRO)"), - ("National Security Agency (NSA)", "National Security Agency (NSA)"), - ( - "National Security Agency-Central Security Service", - "National Security Agency-Central Security Service", - ), - ("Navy, Department of the", "Navy, Department of the"), - ("Office of Economic Adjustment", "Office of Economic Adjustment"), - ("Office of the Secretary of Defense", "Office of the Secretary of Defense"), - ("Pentagon Force Protection Agency", "Pentagon Force Protection Agency"), - ( - "Uniform Services University of the Health Sciences", - "Uniform Services University of the Health Sciences", - ), - ("US Cyber Command (USCYBERCOM)", "US Cyber Command (USCYBERCOM)"), - ( - "US Special Operations Command (USSOCOM)", - "US Special Operations Command (USSOCOM)", - ), - ("US Strategic Command (USSTRATCOM)", "US Strategic Command (USSTRATCOM)"), - ( - "US Transportation Command (USTRANSCOM)", - "US Transportation Command (USTRANSCOM)", - ), - ("Washington Headquarters Services", "Washington Headquarters Services"), -] - -APP_MIGRATION = [ - ("on_premise", translate("forms.task_order.app_migration.on_premise")), - ("cloud", translate("forms.task_order.app_migration.cloud")), - ("both", translate("forms.task_order.app_migration.both")), - ("none", translate("forms.task_order.app_migration.none")), - ("not_sure", translate("forms.task_order.app_migration.not_sure")), -] - -APPLICATION_COMPLEXITY = [ - ("storage", translate("forms.task_order.complexity.storage")), - ("data_analytics", translate("forms.task_order.complexity.data_analytics")), - ("conus", translate("forms.task_order.complexity.conus")), - ("oconus", translate("forms.task_order.complexity.oconus")), - ("tactical_edge", translate("forms.task_order.complexity.tactical_edge")), - ("not_sure", translate("forms.task_order.complexity.not_sure")), - ("other", translate("forms.task_order.complexity.other")), -] - -DEV_TEAM = [ - ("civilians", translate("forms.task_order.dev_team.civilians")), - ("military", translate("forms.task_order.dev_team.military")), - ("contractor", translate("forms.task_order.dev_team.contractor")), - ("other", translate("forms.task_order.dev_team.other")), -] - -TEAM_EXPERIENCE = [ - ("none", translate("forms.task_order.team_experience.none")), - ("planned", translate("forms.task_order.team_experience.planned")), - ("built_1", translate("forms.task_order.team_experience.built_1")), - ("built_3", translate("forms.task_order.team_experience.built_3")), - ("built_many", translate("forms.task_order.team_experience.built_many")), + ("navy", translate("forms.portfolio.defense_component.choices.navy")), + ("other", translate("forms.portfolio.defense_component.choices.other")), ] ENV_ROLE_NO_ACCESS = "No Access" diff --git a/atst/forms/portfolio.py b/atst/forms/portfolio.py index 307f463a..270020ab 100644 --- a/atst/forms/portfolio.py +++ b/atst/forms/portfolio.py @@ -1,33 +1,25 @@ from wtforms.fields import ( - RadioField, - SelectField, SelectMultipleField, StringField, TextAreaField, ) -from wtforms.validators import Length, Optional +from wtforms.validators import Length, InputRequired from wtforms.widgets import ListWidget, CheckboxInput -from .forms import BaseForm, remove_empty_string +from .forms import BaseForm from atst.utils.localization import translate -from .data import ( - APPLICATION_COMPLEXITY, - APP_MIGRATION, - DEV_TEAM, - SERVICE_BRANCHES, - TEAM_EXPERIENCE, -) +from .data import SERVICE_BRANCHES class PortfolioForm(BaseForm): name = StringField( - translate("forms.portfolio.name_label"), + translate("forms.portfolio.name.label"), validators=[ Length( min=4, max=100, - message=translate("forms.portfolio.name_length_validation_message"), + message=translate("forms.portfolio.name.length_validation_message"), ) ], ) @@ -35,78 +27,21 @@ class PortfolioForm(BaseForm): class PortfolioCreationForm(BaseForm): name = StringField( - translate("forms.portfolio.name_label"), + translate("forms.portfolio.name.label"), validators=[ Length( min=4, max=100, - message=translate("forms.portfolio.name_length_validation_message"), + message=translate("forms.portfolio.name.length_validation_message"), ) ], ) - - defense_component = SelectField( - translate("forms.task_order.defense_component_label"), + description = TextAreaField(translate("forms.portfolio.description.label"),) + defense_component = SelectMultipleField( choices=SERVICE_BRANCHES, - default="", - filters=[remove_empty_string], - ) - - description = TextAreaField( - translate("forms.task_order.scope_label"), - description=translate("forms.task_order.scope_description"), - ) - - app_migration = RadioField( - translate("forms.task_order.app_migration.label"), - description=translate("forms.task_order.app_migration.description"), - choices=APP_MIGRATION, - default="", - validators=[Optional()], - ) - - native_apps = RadioField( - translate("forms.task_order.native_apps.label"), - description=translate("forms.task_order.native_apps.description"), - choices=[("yes", "Yes"), ("no", "No"), ("not_sure", "Not Sure")], - default="", - validators=[Optional()], - ) - - complexity = SelectMultipleField( - translate("forms.task_order.complexity.label"), - description=translate("forms.task_order.complexity.description"), - choices=APPLICATION_COMPLEXITY, - default=None, widget=ListWidget(prefix_label=False), option_widget=CheckboxInput(), - ) - - complexity_other = StringField( - translate("forms.task_order.complexity_other_label"), - default=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, - widget=ListWidget(prefix_label=False), - option_widget=CheckboxInput(), - ) - - dev_team_other = StringField( - translate("forms.task_order.dev_team_other_label"), - default=None, - filters=[remove_empty_string], - ) - - team_experience = RadioField( - translate("forms.task_order.team_experience.label"), - description=translate("forms.task_order.team_experience.description"), - choices=TEAM_EXPERIENCE, - default="", - validators=[Optional()], + validators=[ + InputRequired(message="You must select at least one defense component.") + ], ) diff --git a/atst/models/portfolio.py b/atst/models/portfolio.py index ed26b4d4..8cfc35d6 100644 --- a/atst/models/portfolio.py +++ b/atst/models/portfolio.py @@ -1,6 +1,5 @@ from sqlalchemy import Column, String from sqlalchemy.orm import relationship -from sqlalchemy.types import ARRAY from itertools import chain from atst.models.base import Base @@ -19,19 +18,11 @@ class Portfolio( id = types.Id() name = Column(String, nullable=False) + description = Column(String) defense_component = Column( String, nullable=False ) # Department of Defense Component - app_migration = Column(String) # App Migration - complexity = Column(ARRAY(String)) # Application Complexity - complexity_other = Column(String) - description = Column(String) - dev_team = Column(ARRAY(String)) # Development Team - dev_team_other = Column(String) - native_apps = Column(String) # Native Apps - team_experience = Column(String) # Team Experience - applications = relationship( "Application", back_populates="portfolio", diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index 67831c9c..0447be57 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -12,10 +12,9 @@ from atst.utils.flash import formatted_flash as flash @portfolios_bp.route("/portfolios/new") -def new_portfolio(): +def new_portfolio_step_1(): form = PortfolioCreationForm() - - return render_template("portfolios/new.html", form=form) + return render_template("portfolios/new/step_1.html", form=form) @portfolios_bp.route("/portfolios", methods=["POST"]) @@ -28,7 +27,7 @@ def create_portfolio(): url_for("applications.portfolio_applications", portfolio_id=portfolio.id) ) else: - return render_template("portfolios/new.html", form=form), 400 + return render_template("portfolios/new/step_1.html", form=form), 400 @portfolios_bp.route("/portfolios//reports") diff --git a/load-test/locustfile.py b/load-test/locustfile.py index 0ed2bd63..15b89db6 100644 --- a/load-test/locustfile.py +++ b/load-test/locustfile.py @@ -15,7 +15,7 @@ PASSWORD = os.getenv("ATAT_BA_PASSWORD", "") DISABLE_VERIFY = os.getenv("DISABLE_VERIFY", "true").lower() == "true" # Alpha numerics for random entity names -LETTERS = "qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJKLZXCVBNM1234567890" +LETTERS = "qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJKLZXCVBNM1234567890" #pragma: allowlist secret NEW_PORTFOLIO_CHANCE = 10 NEW_APPLICATION_CHANCE = 10 @@ -141,15 +141,8 @@ def create_portfolio(l): new_portfolio_form = l.client.get("/portfolios/new") new_portfolio_body = { "name": f"Load Test Created - {''.join(choices(LETTERS, k=5))}", - "defense_component": "Army, Department of the", + "defense_component": "army", "description": "Test", - "app_migration": "none", - "native_apps": "yes", - "complexity": "storage", - "complexity_other": "", - "dev_team": "civilians", - "dev_team_other": "", - "team_experience": "none", "csrf_token": get_csrf_token(new_portfolio_form), } diff --git a/templates/home.html b/templates/home.html index 91774138..4e459639 100644 --- a/templates/home.html +++ b/templates/home.html @@ -14,7 +14,7 @@ {% endif %} {% call StickyCTA(sticky_header) %} - + {{ "home.add_portfolio_button_text" | translate }} {% endcall %} diff --git a/templates/portfolios/new.html b/templates/portfolios/new.html deleted file mode 100644 index dbd788f2..00000000 --- a/templates/portfolios/new.html +++ /dev/null @@ -1,54 +0,0 @@ -{% from "components/multi_checkbox_input.html" import MultiCheckboxInput %} -{% from "components/options_input.html" import OptionsInput %} -{% from "components/save_button.html" import SaveButton %} -{% from "components/text_input.html" import TextInput %} - -{% extends "base.html" %} - -{% block content %} - -
- {% include "fragments/flash.html" %} -

New Portfolio Form

- -
- {{ form.csrf_token }} - - {{ TextInput(form.name, optional=False) }} - {{ OptionsInput(form.defense_component, optional=False) }} - {{ TextInput(form.description, paragraph=True) }} - -

{{ "task_orders.new.app_info.project_title" | translate }}

- -
- - {{ OptionsInput(form.app_migration) }} - - {{ OptionsInput(form.native_apps) }} -

{{ "forms.task_order.native_apps.not_sure_help" | translate }}

- {{ MultiCheckboxInput(form.complexity, form.complexity_other) }} - -
- -

{{ "task_orders.new.app_info.team_title" | translate }}

-

{{ "task_orders.new.app_info.subtitle" | translate }}

- {{ MultiCheckboxInput(form.dev_team, form.dev_team_other) }} - {{ OptionsInput(form.team_experience) }} - -
- -
- {{ - SaveButton( - text=('common.save' | translate), - form="portfolio-create", - element="input", - ) - }} -
-
- -
- -{% endblock %} - diff --git a/templates/portfolios/new/step_1.html b/templates/portfolios/new/step_1.html new file mode 100644 index 00000000..fe968b9c --- /dev/null +++ b/templates/portfolios/new/step_1.html @@ -0,0 +1,45 @@ +{% from "components/multi_checkbox_input.html" import MultiCheckboxInput %} +{% from "components/options_input.html" import OptionsInput %} +{% from "components/save_button.html" import SaveButton %} +{% from "components/text_input.html" import TextInput %} +{% from "components/sticky_cta.html" import StickyCTA %} + +{% extends "base.html" %} + +{% block content %} + +
+ {% include "fragments/flash.html" %} +
+

{{ "portfolios.header" | translate }}

+

{{ "New Portfolio" }}

+
+ {{ StickyCTA(text="Name and Describe Portfolio", context="Step 1 of 2") }} + +
+ {{ form.csrf_token }} + {{ TextInput(form.name, optional=False) }} + {{"forms.portfolio.name.help_text" | translate | safe }} +
+ {{ TextInput(form.description, paragraph=True) }} + {{"forms.portfolio.description.help_text" | translate | safe }} +
+
+ {{ "forms.portfolio.defense_component.label" | translate }} + {{ "forms.portfolio.defense_component.help_text" | translate | safe }} + {{ MultiCheckboxInput(form.defense_component, optional=False) }} +
+
+ {{ + SaveButton( + text=('common.save' | translate), + form="portfolio-create", + element="input", + ) + }} +
+
+ +
+{% endblock %} + diff --git a/tests/factories.py b/tests/factories.py index 9c8f8c02..ee4159f6 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -105,13 +105,7 @@ class PortfolioFactory(Base): name = factory.Faker("domain_word") defense_component = factory.LazyFunction(random_service_branch) - - app_migration = random_choice(data.APP_MIGRATION) - complexity = [random_choice(data.APPLICATION_COMPLEXITY)] description = factory.Faker("sentence") - dev_team = [random_choice(data.DEV_TEAM)] - native_apps = random.choice(["yes", "no", "not_sure"]) - team_experience = random_choice(data.TEAM_EXPERIENCE) @classmethod def _create(cls, model_class, *args, **kwargs): diff --git a/tests/routes/portfolios/test_index.py b/tests/routes/portfolios/test_index.py index dbee544a..745430f0 100644 --- a/tests/routes/portfolios/test_index.py +++ b/tests/routes/portfolios/test_index.py @@ -18,7 +18,7 @@ def test_new_portfolio(client, user_session): user = UserFactory.create() user_session(user) - response = client.get(url_for("portfolios.new_portfolio")) + response = client.get(url_for("portfolios.new_portfolio_step_1")) assert response.status_code == 200 @@ -34,7 +34,7 @@ def test_create_portfolio_success(client, user_session): data={ "name": "My project name", "description": "My project description", - "defense_component": "Air Force, Department of the", + "defense_component": "army", }, ) diff --git a/tests/test_access.py b/tests/test_access.py index 668d66d9..19aaf749 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -25,7 +25,7 @@ _NO_ACCESS_CHECK_REQUIRED = _NO_LOGIN_REQUIRED + [ "dev.test_email", # dev tool "portfolios.accept_invitation", # available to all users; access control is built into invitation logic "portfolios.create_portfolio", # create a portfolio - "portfolios.new_portfolio", # all users can create a portfolio + "portfolios.new_portfolio_step_1", # all users can create a portfolio "task_orders.get_started", # all users can start a new TO "users.update_user", # available to all users "users.user", # available to all users diff --git a/translations.yaml b/translations.yaml index cb6b5376..11c88a3f 100644 --- a/translations.yaml +++ b/translations.yaml @@ -166,8 +166,49 @@ forms: portfolio_mgmt: Portfolio management reporting: Reporting portfolio: - name_label: Portfolio name - name_length_validation_message: Portfolio names can be between 4-100 characters + name: + label: Portfolio Name + length_validation_message: Portfolio names can be between 4-100 characters + help_text: | +
+

+ Naming can be difficult. Choose a name that is descriptive enough for users to identify the Portfolio. You may consider naming based on your organization. +

+

+ Writer's Block? A naming example +

    +
  • Design Support for Army Developers
    • +
+

+
+ description: + label: Portfolio Description + help_text: | +
+

+ Add a brief one to two sentence description of your Portfolio. Consider this your statement of work. +

+

+ Writer's Block? A description example includes: +

    +
  • Build security applications for FOB Clark
    • +
+

+
+ defense_component: + label: "Select DoD component(s) funding your Portfolio:" + choices: + air_force: Air Force + army: Army + marine_corps: Marine Corps + navy: Navy + other: Other + help_text: | +

+ Select the DOD component(s) that will fund all Applications within this Portfolio. + In JEDI, multiple DoD organizations can fund the same Portfolio.
+ Select all that apply.
+

attachment: object_name: length_error: Object name may be no longer than 40 characters. @@ -176,41 +217,8 @@ forms: task_order: upload_error: There was an error uploading your file. Please try again. If you encounter repeated problems uploading this file, please contact CCPO. size_error: The file you have selected is too large. Please choose a file no larger than 64MB. - app_migration: - both: 'Yes, migrating from both an on-premise data center and another cloud provider' - cloud: 'Yes, migrating from another cloud provider' - description: Do you plan to migrate one or more existing application(s) to the cloud? - label: App migration - none: Not planning to migrate any applications - not_sure: Not sure - on_premise: 'Yes, migrating from an on-premise data center' - complexity: - conus: CONUS access - data_analytics: Data analytics - description: Which of these describes how complex your team's use of the cloud will be? Select all that apply. - label: Project complexity - not_sure: Not sure - oconus: OCONUS access - other: Other - storage: Storage - tactical_edge: Tactical edge access - complexity_other_label: Project Complexity Other - defense_component_label: Department of Defense component - dev_team: - civilians: Government civilians - contractor: Contractor - description: Who is building your cloud application(s)? Select all that apply. - label: Development team - military: Military - other: Other (E.g. University or other partner) - dev_team_other_label: Development Team Other + defense_component_label: Select DoD component(s) funding your Portfolio file_format_not_allowed: Only PDF or PNG files can be uploaded. - native_apps: - description: Do you plan to develop any applications in the cloud? - label: Native apps - 'no': 'No, not planning to develop natively in the cloud' - not_sure: 'Not sure, unsure if planning to develop natively in the cloud' - not_sure_help: Not sure? Talk to your technical lead about where and how they plan on developing your application. number_description: Task order number (13 digits) pop_errors: date_order: PoP start date must be before end date. @@ -219,8 +227,6 @@ forms: end_pre_contract: PoP end date must be after or on {date}. start_past_contract: PoP start date must be before or on {date}. start_pre_contract: PoP start date must be on or after {date}. - scope_description: 'What do you plan to do on the cloud? Some examples might include migrating an existing application or creating a prototype. You don’t need to include a detailed plan of execution, but should list key requirements. This section will be reviewed by your contracting officer, but won’t be sent to the CCPO.

Not sure how to describe your scope? Read some examples to get some inspiration.

' - scope_label: Cloud project scope clin_funding_errors: obligated_amount_error: Obligated amount must be less than or equal to total amount funding_range_error: Dollar amount must be from $0.00 to $1,000,000,000.00 From f4f3e3dee3e25496eb9a8b57a1b8e74ad16276bf Mon Sep 17 00:00:00 2001 From: graham-dds Date: Wed, 11 Dec 2019 15:49:56 -0500 Subject: [PATCH 02/11] Add horizontal breaks to form --- styles/components/_forms.scss | 3 +++ templates/portfolios/new/step_1.html | 27 +++++++++++++++++---------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/styles/components/_forms.scss b/styles/components/_forms.scss index 19ca2fb5..a6e0709a 100644 --- a/styles/components/_forms.scss +++ b/styles/components/_forms.scss @@ -1,6 +1,9 @@ // Form Grid .form-row { margin: ($gap * 4) 0; + &--separated { + border-bottom: $color-gray-lighter 1px solid; + } .form-col { flex-grow: 1; diff --git a/templates/portfolios/new/step_1.html b/templates/portfolios/new/step_1.html index fe968b9c..4613b063 100644 --- a/templates/portfolios/new/step_1.html +++ b/templates/portfolios/new/step_1.html @@ -14,21 +14,28 @@

{{ "portfolios.header" | translate }}

{{ "New Portfolio" }}

- {{ StickyCTA(text="Name and Describe Portfolio", context="Step 1 of 2") }} + {{ StickyCTA(text="Create New Portfolio") }}
{{ form.csrf_token }} - {{ TextInput(form.name, optional=False) }} - {{"forms.portfolio.name.help_text" | translate | safe }} -
- {{ TextInput(form.description, paragraph=True) }} - {{"forms.portfolio.description.help_text" | translate | safe }} +
+
+ {{ TextInput(form.name, optional=False) }} + {{"forms.portfolio.name.help_text" | translate | safe }}
-
- {{ "forms.portfolio.defense_component.label" | translate }} - {{ "forms.portfolio.defense_component.help_text" | translate | safe }} - {{ MultiCheckboxInput(form.defense_component, optional=False) }} +
+
+
+ {{ TextInput(form.description, paragraph=True) }} + {{"forms.portfolio.description.help_text" | translate | safe }}
+
+
+
+ {{ MultiCheckboxInput(form.defense_component, optional=False) }} + {{ "forms.portfolio.defense_component.help_text" | translate | safe }} +
+
{{ SaveButton( From 80f028540cf47d87d72a82335ed3adefdcdf09b1 Mon Sep 17 00:00:00 2001 From: graham-dds Date: Wed, 11 Dec 2019 15:51:55 -0500 Subject: [PATCH 03/11] Refactor multi_checkbox_input This component was made when having an "other" value as a check option also meant typing in a custom value into an input field. Since this is no longer needed, we were able to remove the markup / vue code for that feature. --- js/components/multi_checkbox_input.js | 22 +++++-------------- .../components/multi_checkbox_input.html | 11 ---------- 2 files changed, 6 insertions(+), 27 deletions(-) diff --git a/js/components/multi_checkbox_input.js b/js/components/multi_checkbox_input.js index fdba839f..f2d878da 100644 --- a/js/components/multi_checkbox_input.js +++ b/js/components/multi_checkbox_input.js @@ -13,22 +13,14 @@ export default { type: Array, default: () => [], }, - initialOtherValue: String, optional: Boolean, }, data: function() { - const showError = (this.initialErrors && this.initialErrors.length) || false return { - showError: showError, - showValid: !showError && this.initialValue.length > 0, + showError: this.initialErrors.length > 0, + showValid: false, validationError: this.initialErrors.join(' '), - otherChecked: this.initialValue.includes('other') - ? true - : this.otherChecked, - otherText: this.initialValue.includes('other') - ? this.initialOtherValue - : '', selections: this.initialValue, } }, @@ -36,17 +28,15 @@ export default { methods: { onInput: function(e) { emitFieldChange(this) - this.showError = false - this.showValid = true - }, - otherToggle: function() { - this.otherChecked = !this.otherChecked + this.showError = !this.valid + this.showValid = !this.showError + this.validationError = 'This field is required.' }, }, computed: { valid: function() { - return this.optional || this.showValid + return this.optional || this.selections.length > 0 }, }, } diff --git a/templates/components/multi_checkbox_input.html b/templates/components/multi_checkbox_input.html index 2f2c65ec..6de6331b 100644 --- a/templates/components/multi_checkbox_input.html +++ b/templates/components/multi_checkbox_input.html @@ -45,19 +45,8 @@
    {% for choice in field.choices %}
  • - {% if choice[0] != 'other' %} - {% else %} - - - - {% if other_input_field %} -
    - -
    - {% endif %} - {% endif %}
    • {% endfor %}
From cbea71259c55bd11435c807d94d3f052f20b50ec Mon Sep 17 00:00:00 2001 From: graham-dds Date: Thu, 12 Dec 2019 10:41:04 -0500 Subject: [PATCH 04/11] Move checkbox validation message to translations --- atst/forms/portfolio.py | 6 +++++- translations.yaml | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/atst/forms/portfolio.py b/atst/forms/portfolio.py index 270020ab..7f281787 100644 --- a/atst/forms/portfolio.py +++ b/atst/forms/portfolio.py @@ -42,6 +42,10 @@ class PortfolioCreationForm(BaseForm): widget=ListWidget(prefix_label=False), option_widget=CheckboxInput(), validators=[ - InputRequired(message="You must select at least one defense component.") + InputRequired( + message=translate( + "forms.portfolio.defense_component.validation_message" + ) + ) ], ) diff --git a/translations.yaml b/translations.yaml index 11c88a3f..1fa09b83 100644 --- a/translations.yaml +++ b/translations.yaml @@ -203,6 +203,7 @@ forms: marine_corps: Marine Corps navy: Navy other: Other + validation_message: You must select at least one defense component. help_text: |

Select the DOD component(s) that will fund all Applications within this Portfolio. From 07b4238c2b24d4a1654469eb51e81f2597a0bc9f Mon Sep 17 00:00:00 2001 From: graham-dds Date: Fri, 13 Dec 2019 12:53:07 -0500 Subject: [PATCH 05/11] Write tests for multi checkbox input vue component --- .../__tests__/multi_checkbox_input.test.js | 102 ++++++++++++++++++ tests/render_vue_component.py | 23 +++- 2 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 js/components/__tests__/multi_checkbox_input.test.js diff --git a/js/components/__tests__/multi_checkbox_input.test.js b/js/components/__tests__/multi_checkbox_input.test.js new file mode 100644 index 00000000..8e012db4 --- /dev/null +++ b/js/components/__tests__/multi_checkbox_input.test.js @@ -0,0 +1,102 @@ +import { mount } from '@vue/test-utils' +import multicheckboxinput from '../multi_checkbox_input' +import { makeTestWrapper } from '../../test_utils/component_test_helpers' + +const WrapperComponent = makeTestWrapper({ + components: { + multicheckboxinput, + }, + templatePath: 'multi_checkbox_input_template.html', + data: function() { + const { initialvalue, optional } = this.initialData + return { initialvalue, optional } + }, +}) + +describe('MultiCheckboxInput Renders Correctly', () => { + it('Should initialize unchecked and with no validation showing', () => { + const wrapper = mount(WrapperComponent, { + propsData: { + name: 'testCheck', + initialData: { + initialvalue: [], + }, + }, + }) + expect(wrapper.contains('.usa-input--success')).toBe(false) + expect(wrapper.contains('.usa-input--error')).toBe(false) + expect(wrapper.find('.usa-input input[value="a"]').element.checked).toBe( + false + ) + expect(wrapper.find('.usa-input input[value="b"]').element.checked).toBe( + false + ) + }) + + it('Should initialize with "a" checked', () => { + const wrapper = mount(WrapperComponent, { + propsData: { + name: 'testCheck', + initialData: { + initialvalue: ['a'], + }, + }, + }) + expect(wrapper.find('.usa-input input[value="a"]').element.checked).toBe( + true + ) + expect(wrapper.find('.usa-input input[value="b"]').element.checked).toBe( + false + ) + }) +}) + +describe('Multicheckbox shows validation states correctly', () => { + it('Should be valid when any checkbox is clicked', () => { + const wrapper = mount(WrapperComponent, { + propsData: { + name: 'testCheck', + initialData: { initialvalue: [] }, + }, + }) + wrapper.find('.usa-input input[value="a"]').setChecked() + expect(wrapper.contains('.usa-input--success')).toBe(true) + expect(wrapper.contains('.usa-input--error')).toBe(false) + }) + + it('Should be invalid when no checkboxes are checked', () => { + const wrapper = mount(WrapperComponent, { + propsData: { + name: 'testCheck', + initialData: { + initialvalue: [], + }, + }, + }) + + // Check and then uncheck a checkbox + const checkboxA = wrapper.find('.usa-input input[value="a"]') + checkboxA.setChecked() + checkboxA.setChecked(false) + + expect(wrapper.contains('.usa-input--error')).toBe(true) + expect(wrapper.contains('.usa-input--success')).toBe(false) + }) + + it('Should be valid when no checkboxes are checked but it is optional', () => { + const wrapper = mount(WrapperComponent, { + propsData: { + name: 'testCheck', + initialData: { initialvalue: [], optional: true }, + }, + }) + + // Check and then uncheck a checkbox + const checkboxA = wrapper.find('.usa-input input[value="a"]') + checkboxA.setChecked() + checkboxA.setChecked(false) + + expect(wrapper.contains('.usa-input--error')).toBe(false) + expect(wrapper.contains('.usa-input--success')).toBe(true) + }) +}) diff --git a/tests/render_vue_component.py b/tests/render_vue_component.py index c4a7c135..62106a67 100644 --- a/tests/render_vue_component.py +++ b/tests/render_vue_component.py @@ -1,10 +1,11 @@ import pytest from bs4 import BeautifulSoup +from flask import Markup from wtforms import Form, FormField from wtforms.fields import StringField from wtforms.validators import InputRequired -from wtforms.widgets import CheckboxInput +from wtforms.widgets import ListWidget, CheckboxInput from atst.forms.task_order import CLINForm from atst.forms.task_order import TaskOrderForm @@ -56,6 +57,12 @@ def checkbox_input_macro(env): return getattr(checkbox_template.module, "CheckboxInput") +@pytest.fixture +def multi_checkbox_input_macro(env): + multi_checkbox_template = env.get_template("components/multi_checkbox_input.html") + return getattr(multi_checkbox_template.module, "MultiCheckboxInput") + + @pytest.fixture def initial_value_form(scope="function"): return InitialValueForm() @@ -82,6 +89,20 @@ def test_make_checkbox_input_template(checkbox_input_macro, initial_value_form): write_template(rendered_checkbox_macro, "checkbox_input_template.html") +def test_make_multi_checkbox_input_template( + multi_checkbox_input_macro, initial_value_form +): + initial_value_form.datafield.widget = ListWidget() + initial_value_form.datafield.option_widget = CheckboxInput() + initial_value_form.datafield.choices = [("a", "A"), ("b", "B")] + rendered_multi_checkbox_input_macro = multi_checkbox_input_macro( + initial_value_form.datafield, optional=Markup("'optional'") + ) + write_template( + rendered_multi_checkbox_input_macro, "multi_checkbox_input_template.html" + ) + + def test_make_upload_input_template(upload_input_macro, task_order_form): rendered_upload_macro = upload_input_macro(task_order_form.pdf) write_template(rendered_upload_macro, "upload_input_template.html") From 85252812e08cd5db4da2e3711eaa476c63f52211 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 10 Dec 2019 16:26:00 -0500 Subject: [PATCH 06/11] Update number column on task orders table to have a unique constraint --- ...57_add_unique_constraint_to_task_order_.py | 28 +++++++++++++++++++ atst/models/task_order.py | 2 +- 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py diff --git a/alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py b/alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py new file mode 100644 index 00000000..01794a93 --- /dev/null +++ b/alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py @@ -0,0 +1,28 @@ +"""add unique constraint to task order number + +Revision ID: 3bd8552f1c57 +Revises: 67a2151d6269 +Create Date: 2019-12-10 12:45:17.535973 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '3bd8552f1c57' # pragma: allowlist secret +down_revision = '67a2151d6269' # pragma: allowlist secret +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_unique_constraint(None, 'task_orders', ['number']) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint(None, 'task_orders', type_='unique') + # ### end Alembic commands ### diff --git a/atst/models/task_order.py b/atst/models/task_order.py index 1a46e505..eba34147 100644 --- a/atst/models/task_order.py +++ b/atst/models/task_order.py @@ -39,7 +39,7 @@ class TaskOrder(Base, mixins.TimestampsMixin): pdf_attachment_id = Column(ForeignKey("attachments.id")) _pdf = relationship("Attachment", foreign_keys=[pdf_attachment_id]) - number = Column(String) # Task Order Number + number = Column(String, unique=True,) # Task Order Number signer_dod_id = Column(String) signed_at = Column(DateTime) From 6446b4fbd0781cdc0387fd0af14c726935ba530c Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 11 Dec 2019 11:41:47 -0500 Subject: [PATCH 07/11] Raise AlreadyExistsError if a task order is created or updated with a number of an existing task order --- atst/domain/task_orders.py | 17 +++++++++++++---- tests/domain/test_task_orders.py | 16 ++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index 24acdee7..ce4ba77b 100644 --- a/atst/domain/task_orders.py +++ b/atst/domain/task_orders.py @@ -1,9 +1,11 @@ import datetime +from sqlalchemy.exc import IntegrityError from atst.database import db from atst.models.clin import CLIN from atst.models.task_order import TaskOrder, SORT_ORDERING from . import BaseDomainClass +from .exceptions import AlreadyExistsError class TaskOrders(BaseDomainClass): @@ -12,9 +14,12 @@ class TaskOrders(BaseDomainClass): @classmethod def create(cls, portfolio_id, number, clins, pdf): - task_order = TaskOrder(portfolio_id=portfolio_id, number=number, pdf=pdf) - db.session.add(task_order) - db.session.commit() + try: + task_order = TaskOrder(portfolio_id=portfolio_id, number=number, pdf=pdf) + db.session.add(task_order) + db.session.commit() + except IntegrityError: + raise AlreadyExistsError("task_order") TaskOrders.create_clins(task_order.id, clins) @@ -35,7 +40,11 @@ class TaskOrders(BaseDomainClass): task_order.number = number db.session.add(task_order) - db.session.commit() + try: + db.session.commit() + except IntegrityError: + raise AlreadyExistsError("task_order") + return task_order @classmethod diff --git a/tests/domain/test_task_orders.py b/tests/domain/test_task_orders.py index 7bc4cf41..49b9bae6 100644 --- a/tests/domain/test_task_orders.py +++ b/tests/domain/test_task_orders.py @@ -2,6 +2,7 @@ import pytest from datetime import date, timedelta from decimal import Decimal +from atst.domain.exceptions import AlreadyExistsError from atst.domain.task_orders import TaskOrders from atst.models import Attachment from atst.models.task_order import TaskOrder, SORT_ORDERING, Status @@ -154,3 +155,18 @@ def test_task_order_sort_by_status(): assert len(sorted_by_status["Expired"]) == 2 assert len(sorted_by_status["Unsigned"]) == 1 assert list(sorted_by_status.keys()) == [status.value for status in SORT_ORDERING] + + +def test_create_enforces_unique_number(): + portfolio = PortfolioFactory.create() + number = "1234567890123" + assert TaskOrders.create(portfolio.id, number, [], None) + with pytest.raises(AlreadyExistsError): + TaskOrders.create(portfolio.id, number, [], None) + + +def test_update_enforces_unique_number(): + task_order = TaskOrderFactory.create() + dupe_task_order = TaskOrderFactory.create() + with pytest.raises(AlreadyExistsError): + TaskOrders.update(dupe_task_order.id, task_order.number, [], None) From 9caef2883a32a085a7b43a6340e63a6dabf6962a Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 12 Dec 2019 11:58:59 -0500 Subject: [PATCH 08/11] migration fix --- .../3bd8552f1c57_add_unique_constraint_to_task_order_.py | 4 ++-- atst/domain/task_orders.py | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py b/alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py index 01794a93..24a0fcfa 100644 --- a/alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py +++ b/alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py @@ -18,11 +18,11 @@ depends_on = None def upgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.create_unique_constraint(None, 'task_orders', ['number']) + op.create_unique_constraint('task_orders_number_key', 'task_orders', ['number']) # ### end Alembic commands ### def downgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.drop_constraint(None, 'task_orders', type_='unique') + op.drop_constraint('task_orders_number_key', 'task_orders', type_='unique') # ### end Alembic commands ### diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index ce4ba77b..835590c9 100644 --- a/atst/domain/task_orders.py +++ b/atst/domain/task_orders.py @@ -14,11 +14,13 @@ class TaskOrders(BaseDomainClass): @classmethod def create(cls, portfolio_id, number, clins, pdf): + task_order = TaskOrder(portfolio_id=portfolio_id, number=number, pdf=pdf) + db.session.add(task_order) + try: - task_order = TaskOrder(portfolio_id=portfolio_id, number=number, pdf=pdf) - db.session.add(task_order) db.session.commit() except IntegrityError: + db.session.rollback() raise AlreadyExistsError("task_order") TaskOrders.create_clins(task_order.id, clins) @@ -43,6 +45,7 @@ class TaskOrders(BaseDomainClass): try: db.session.commit() except IntegrityError: + db.session.rollback() raise AlreadyExistsError("task_order") return task_order From 78ef47f649751607769ae89c1b80223433b73ebd Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 12 Dec 2019 14:21:30 -0500 Subject: [PATCH 09/11] Update TO route helper function to catch error and display flash message when a user tries to save a TO with an existing number. Update TaskOrderForm so that it converts empty string for number into None, this was causing an issue where new TOs were being saved with an empty string for the number, which violated the unique constraint. --- atst/forms/task_order.py | 8 +++++ atst/routes/task_orders/new.py | 47 +++++++++++++++++----------- atst/utils/flash.py | 5 +++ tests/forms/test_task_order.py | 8 ++++- tests/routes/task_orders/test_new.py | 20 ++++++++++++ translations.yaml | 2 ++ 6 files changed, 70 insertions(+), 20 deletions(-) diff --git a/atst/forms/task_order.py b/atst/forms/task_order.py index 2a1b8ad2..41ebeb81 100644 --- a/atst/forms/task_order.py +++ b/atst/forms/task_order.py @@ -142,6 +142,14 @@ class TaskOrderForm(BaseForm): ) clins = FieldList(FormField(CLINForm)) + @property + def data(self): + _data = super().data + if _data["number"] == "": + _data["number"] = None + + return _data + class SignatureForm(BaseForm): signature = BooleanField( diff --git a/atst/routes/task_orders/new.py b/atst/routes/task_orders/new.py index dca751e3..f6e53b75 100644 --- a/atst/routes/task_orders/new.py +++ b/atst/routes/task_orders/new.py @@ -10,7 +10,7 @@ from flask import ( from .blueprint import task_orders_bp from atst.domain.authz.decorator import user_can_access_decorator as user_can -from atst.domain.exceptions import NoAccessError +from atst.domain.exceptions import NoAccessError, AlreadyExistsError from atst.domain.task_orders import TaskOrders from atst.forms.task_order import TaskOrderForm, SignatureForm from atst.models.permissions import Permissions @@ -50,7 +50,26 @@ def render_task_orders_edit( return render_template(template, **render_args) -def update_task_order( +def update_task_order(form, portfolio_id=None, task_order_id=None, flash_invalid=True): + if form.validate(flash_invalid=flash_invalid): + task_order = None + try: + if task_order_id: + task_order = TaskOrders.update(task_order_id, **form.data) + portfolio_id = task_order.portfolio_id + else: + task_order = TaskOrders.create(portfolio_id, **form.data) + + return task_order + + except AlreadyExistsError: + flash("task_order_number_error", to_number=form.data["number"]) + return False + else: + return False + + +def update_and_render_next( form_data, next_page, current_template, portfolio_id=None, task_order_id=None ): form = None @@ -60,14 +79,8 @@ def update_task_order( else: form = TaskOrderForm(form_data) - if form.validate(): - task_order = None - if task_order_id: - task_order = TaskOrders.update(task_order_id, **form.data) - portfolio_id = task_order.portfolio_id - else: - task_order = TaskOrders.create(portfolio_id, **form.data) - + task_order = update_task_order(form, portfolio_id, task_order_id) + if task_order: return redirect(url_for(next_page, task_order_id=task_order.id)) else: return ( @@ -149,7 +162,7 @@ def submit_form_step_one_add_pdf(portfolio_id=None, task_order_id=None): next_page = "task_orders.form_step_two_add_number" current_template = "task_orders/step_1.html" - return update_task_order( + return update_and_render_next( form_data, next_page, current_template, @@ -176,12 +189,8 @@ def cancel_edit(task_order_id=None, portfolio_id=None): else: form = TaskOrderForm(form_data) - if form.validate(flash_invalid=False): - task_order = None - if task_order_id: - task_order = TaskOrders.update(task_order_id, **form.data) - else: - task_order = TaskOrders.create(portfolio_id, **form.data) + update_task_order(form, portfolio_id, task_order_id, flash_invalid=False) + elif not save and task_order_id: TaskOrders.delete(task_order_id) @@ -205,7 +214,7 @@ def submit_form_step_two_add_number(task_order_id): next_page = "task_orders.form_step_three_add_clins" current_template = "task_orders/step_2.html" - return update_task_order( + return update_and_render_next( form_data, next_page, current_template, task_order_id=task_order_id ) @@ -225,7 +234,7 @@ def submit_form_step_three_add_clins(task_order_id): next_page = "task_orders.form_step_four_review" current_template = "task_orders/step_3.html" - return update_task_order( + return update_and_render_next( form_data, next_page, current_template, task_order_id=task_order_id ) diff --git a/atst/utils/flash.py b/atst/utils/flash.py index 87b543b8..fabba4d2 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -165,6 +165,11 @@ MESSAGES = { "message_template": translate("task_orders.form.draft_alert_message"), "category": "warning", }, + "task_order_number_error": { + "title_template": "", + "message_template": """{{ 'flash.task_order_number_error.message' | translate({ 'to_number': to_number }) }}""", + "category": "error", + }, "task_order_submitted": { "title_template": "Your Task Order has been uploaded successfully.", "message_template": """ diff --git a/tests/forms/test_task_order.py b/tests/forms/test_task_order.py index de95e555..97759c81 100644 --- a/tests/forms/test_task_order.py +++ b/tests/forms/test_task_order.py @@ -2,7 +2,7 @@ import datetime from dateutil.relativedelta import relativedelta from flask import current_app as app -from atst.forms.task_order import CLINForm +from atst.forms.task_order import CLINForm, TaskOrderForm from atst.models import JEDICLINType from atst.utils.localization import translate @@ -106,3 +106,9 @@ def test_clin_form_dollar_amounts_out_of_range(): assert ( translate("forms.task_order.clin_funding_errors.funding_range_error") ) in invalid_clin_form.obligated_amount.errors + + +def test_no_number(): + http_request_form_data = {} + form = TaskOrderForm(http_request_form_data) + assert form.data["number"] is None diff --git a/tests/routes/task_orders/test_new.py b/tests/routes/task_orders/test_new.py index 1e8e16eb..a7f5a991 100644 --- a/tests/routes/task_orders/test_new.py +++ b/tests/routes/task_orders/test_new.py @@ -170,6 +170,26 @@ def test_task_orders_submit_form_step_two_add_number(client, user_session, task_ assert task_order.number == "1234567890" +def test_task_orders_submit_form_step_two_enforces_unique_number( + client, user_session, task_order, session +): + number = "1234567890123" + dupe_task_order = TaskOrderFactory.create(number=number) + portfolio = task_order.portfolio + user_session(task_order.portfolio.owner) + form_data = {"number": number} + session.begin_nested() + response = client.post( + url_for( + "task_orders.submit_form_step_two_add_number", task_order_id=task_order.id + ), + data=form_data, + ) + session.rollback() + + assert response.status_code == 400 + + def test_task_orders_submit_form_step_two_add_number_existing_to( client, user_session, task_order ): diff --git a/translations.yaml b/translations.yaml index 1fa09b83..1d876e97 100644 --- a/translations.yaml +++ b/translations.yaml @@ -123,6 +123,8 @@ flash: new_ppoc_message: 'You have successfully added {ppoc_name} as the primary point of contact. You are no longer the PPoC.' new_ppoc_title: Primary point of contact updated success: Success! + task_order_number_error: + message: 'The TO number has already been entered for a JEDI task order #{to_number}. Please double-check the TO number you are entering. If you believe this is in error, please contact support@cloud.mil.' new_application_member: title: "{user_name}'s invitation has been sent" message: "{user_name}'s access to this Application is pending until they sign in for the first time." From f0505ee1228eb5fb80f358276eb42e331a157811 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Fri, 13 Dec 2019 11:57:26 -0500 Subject: [PATCH 10/11] Remove unused imports Use remove_empty_string filter for TaskOrderForm.number instead of declaring the data property Update display of TOs without a number --- ...2f1c57_add_unique_constraint_to_task_order_.py | 2 -- atst/forms/task_order.py | 15 +++++---------- templates/task_orders/index.html | 2 +- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py b/alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py index 24a0fcfa..8128a40a 100644 --- a/alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py +++ b/alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py @@ -6,8 +6,6 @@ Create Date: 2019-12-10 12:45:17.535973 """ from alembic import op -import sqlalchemy as sa -from sqlalchemy.dialects import postgresql # revision identifiers, used by Alembic. revision = '3bd8552f1c57' # pragma: allowlist secret diff --git a/atst/forms/task_order.py b/atst/forms/task_order.py index 41ebeb81..a5e02e8b 100644 --- a/atst/forms/task_order.py +++ b/atst/forms/task_order.py @@ -13,7 +13,7 @@ from numbers import Number from .data import JEDI_CLIN_TYPES from .fields import SelectField -from .forms import BaseForm +from .forms import BaseForm, remove_empty_string from atst.utils.localization import translate from flask import current_app as app @@ -134,7 +134,10 @@ class AttachmentForm(BaseForm): class TaskOrderForm(BaseForm): - number = StringField(label=translate("forms.task_order.number_description")) + number = StringField( + label=translate("forms.task_order.number_description"), + filters=[remove_empty_string], + ) pdf = FormField( AttachmentForm, label=translate("task_orders.form.supporting_docs_size_limit"), @@ -142,14 +145,6 @@ class TaskOrderForm(BaseForm): ) clins = FieldList(FormField(CLINForm)) - @property - def data(self): - _data = super().data - if _data["number"] == "": - _data["number"] = None - - return _data - class SignatureForm(BaseForm): signature = BooleanField( diff --git a/templates/task_orders/index.html b/templates/task_orders/index.html index b9277123..160fb483 100644 --- a/templates/task_orders/index.html +++ b/templates/task_orders/index.html @@ -19,7 +19,7 @@ {% if task_orders|length > 0 %} {% for task_order in task_orders %} {% set to_number %} - {% if task_order.number != "" %} + {% if task_order.number != None %} Task Order #{{ task_order.number }} {% else %} New Task Order From e38358992d8fa7f35820baf170d4b01856a4e1f3 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Fri, 13 Dec 2019 14:56:31 -0500 Subject: [PATCH 11/11] Fix migration chain --- .../3bd8552f1c57_add_unique_constraint_to_task_order_.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py b/alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py index 8128a40a..f1375626 100644 --- a/alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py +++ b/alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py @@ -1,7 +1,7 @@ """add unique constraint to task order number Revision ID: 3bd8552f1c57 -Revises: 67a2151d6269 +Revises: 802071bcd013 Create Date: 2019-12-10 12:45:17.535973 """ @@ -9,7 +9,7 @@ from alembic import op # revision identifiers, used by Alembic. revision = '3bd8552f1c57' # pragma: allowlist secret -down_revision = '67a2151d6269' # pragma: allowlist secret +down_revision = '802071bcd013' # pragma: allowlist secret branch_labels = None depends_on = None