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..f1375626 --- /dev/null +++ b/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 ### diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index 24acdee7..835590c9 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): @@ -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) - db.session.commit() + + 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) - db.session.commit() + try: + db.session.commit() + except IntegrityError: + db.session.rollback() + raise AlreadyExistsError("task_order") + return task_order @classmethod diff --git a/atst/forms/task_order.py b/atst/forms/task_order.py index 2a1b8ad2..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"), 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) 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/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 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) 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."