Merge pull request #1245 from dod-ccpo/to-uniqueness

Enforce TO number uniqueness

alembic/versions/3bd8552f1c57_add_unique_constraint_to_task_order_.py

@@ -0,0 +1,26 @@
+"""add unique constraint to task order number
+
+Revision ID: 3bd8552f1c57
+Revises: 802071bcd013
+Create Date: 2019-12-10 12:45:17.535973
+
+"""
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = '3bd8552f1c57' # pragma: allowlist secret
+down_revision = '802071bcd013' # pragma: allowlist secret
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    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('task_orders_number_key', 'task_orders', type_='unique')
+    # ### end Alembic commands ###

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):
@@ -14,7 +16,12 @@ class TaskOrders(BaseDomainClass):
     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:
             db.session.commit()
+        except IntegrityError:
+            db.session.rollback()
+            raise AlreadyExistsError("task_order")
 
         TaskOrders.create_clins(task_order.id, clins)
 
@@ -35,7 +42,12 @@ class TaskOrders(BaseDomainClass):
             task_order.number = number
             db.session.add(task_order)
 
+        try:
             db.session.commit()
+        except IntegrityError:
+            db.session.rollback()
+            raise AlreadyExistsError("task_order")
+
         return task_order
 
     @classmethod

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

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)
 

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 = update_task_order(form, portfolio_id, task_order_id)
-        task_order = None
+    if task_order:
-        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 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):
+        update_task_order(form, portfolio_id, task_order_id, 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)
     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
     )
 

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

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

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)

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

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
 ):

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