Add migration for enforcing uniqueness of an application name within a portfolio and update create/update Applicaiton domain methods.

.secrets.baseline

@@ -3,7 +3,7 @@
     "files": "^.secrets.baseline$|^.*pgsslrootcert.yml$",
     "lines": null
   },
-  "generated_at": "2019-12-06T21:22:07Z",
+  "generated_at": "2019-12-13T20:38:57Z",
   "plugins_used": [
     {
       "base64_limit": 4.5,
@@ -170,7 +170,7 @@
         "hashed_secret": "e4f14805dfd1e6af030359090c535e149e6b4207",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 656,
+        "line_number": 659,
         "type": "Hex High Entropy String"
       }
     ]

alembic/versions/c487d91f1a26_add_application_name_and_portfolio_id_.py

@@ -0,0 +1,28 @@
+"""add application name and portfolio_id unique constraint
+
+Revision ID: c487d91f1a26
+Revises: 3bd8552f1c57
+Create Date: 2019-12-13 14:33:23.952450
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = 'c487d91f1a26' # pragma: allowlist secret
+down_revision = '3bd8552f1c57' # pragma: allowlist secret
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.create_unique_constraint('name_portfolio_id_unique_constraint', 'applications', ['name', 'portfolio_id'])
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.drop_constraint('name_portfolio_id_unique_constraint', 'applications', type_='unique')
+    # ### end Alembic commands ###

atst/domain/applications.py

@@ -1,4 +1,7 @@
+from sqlalchemy.exc import IntegrityError
+
 from . import BaseDomainClass
+from .exceptions import AlreadyExistsError
 from flask import g
 from atst.database import db
 from atst.domain.application_roles import ApplicationRoles
@@ -28,7 +31,12 @@ class Applications(BaseDomainClass):
         if environment_names:
             Environments.create_many(user, application, environment_names)
 
-        db.session.commit()
+        try:
+            db.session.commit()
+        except IntegrityError:
+            db.session.rollback()
+            raise AlreadyExistsError("application")
+
         return application
 
     @classmethod
@@ -54,7 +62,12 @@ class Applications(BaseDomainClass):
                 g.current_user, application, new_data["environment_names"]
             )
         db.session.add(application)
-        db.session.commit()
+
+        try:
+            db.session.commit()
+        except IntegrityError:
+            db.session.rollback()
+            raise AlreadyExistsError("application")
 
         return application
 

atst/models/application.py

@@ -1,4 +1,4 @@
-from sqlalchemy import and_, Column, ForeignKey, String
+from sqlalchemy import and_, Column, ForeignKey, String, UniqueConstraint
 from sqlalchemy.orm import relationship, synonym
 
 from atst.models.base import Base
@@ -34,6 +34,11 @@ class Application(
         ),
     )
     members = synonym("roles")
+    __table_args__ = (
+        UniqueConstraint(
+            "name", "portfolio_id", name="name_portfolio_id_unique_constraint"
+        ),
+    )
 
     @property
     def users(self):

tests/domain/test_applications.py

@@ -5,7 +5,7 @@ from atst.models import CSPRole, ApplicationRoleStatus
 from atst.domain.application_roles import ApplicationRoles
 from atst.domain.applications import Applications
 from atst.domain.environment_roles import EnvironmentRoles
-from atst.domain.exceptions import NotFoundError
+from atst.domain.exceptions import AlreadyExistsError, NotFoundError
 from atst.domain.permission_sets import PermissionSets
 
 from tests.factories import (
@@ -177,3 +177,22 @@ def test_invite_to_nonexistent_environment():
                 {"environment_id": uuid4(), "role": CSPRole.BASIC_ACCESS.value},
             ],
         )
+
+
+def test_create_does_not_duplicate_names_within_portfolio():
+    portfolio = PortfolioFactory.create()
+    name = "An Awesome Application"
+
+    assert Applications.create(portfolio.owner, portfolio, name, "")
+    with pytest.raises(AlreadyExistsError):
+        Applications.create(portfolio.owner, portfolio, name, "")
+
+
+def test_update_does_not_duplicate_names_within_portfolio():
+    portfolio = PortfolioFactory.create()
+    name = "An Awesome Application"
+    application = ApplicationFactory.create(portfolio=portfolio, name=name)
+    dupe_application = ApplicationFactory.create(portfolio=portfolio)
+
+    with pytest.raises(AlreadyExistsError):
+        Applications.update(dupe_application, {"name": name})

tests/domain/test_portfolios.py

@@ -1,4 +1,5 @@
 import pytest
+import random
 from uuid import uuid4
 
 from atst.domain.exceptions import NotFoundError, UnauthorizedError
@@ -97,7 +98,7 @@ def test_scoped_portfolio_returns_all_applications_for_portfolio_admin(
         Applications.create(
             portfolio.owner,
             portfolio,
-            "My Application",
+            "My Application %s" % (random.randrange(1, 1000)),
             "My application",
             ["dev", "staging", "prod"],
         )
@@ -120,7 +121,7 @@ def test_scoped_portfolio_returns_all_applications_for_portfolio_owner(
         Applications.create(
             portfolio.owner,
             portfolio,
-            "My Application",
+            "My Application %s" % (random.randrange(1, 1000)),
             "My application",
             ["dev", "staging", "prod"],
         )

tests/test_access.py

@@ -1,6 +1,7 @@
 from unittest.mock import Mock
 
 import pytest
+import random
 
 from flask import url_for, Response
 
@@ -264,26 +265,28 @@ def test_applications_post_application_step_1(post_url_assert_status):
     rando = user_with()
     portfolio = PortfolioFactory.create(owner=owner)
     application = ApplicationFactory.create(portfolio=portfolio)
-    step_1_form_data = {
-        "name": "Test Application",
-        "description": "This is only a test",
-    }
+
+    def _form_data():
+        return {
+            "name": "Test Application %s" % (random.randrange(1, 1000)),
+            "description": "This is only a test",
+        }
 
     url = url_for(
         "applications.create_new_application_step_1", portfolio_id=portfolio.id
     )
-    post_url_assert_status(ccpo, url, 302, data=step_1_form_data)
-    post_url_assert_status(owner, url, 302, data=step_1_form_data)
-    post_url_assert_status(rando, url, 404, data=step_1_form_data)
+    post_url_assert_status(ccpo, url, 302, data=_form_data())
+    post_url_assert_status(owner, url, 302, data=_form_data())
+    post_url_assert_status(rando, url, 404, data=_form_data())
 
     url = url_for(
         "applications.update_new_application_step_1",
         portfolio_id=portfolio.id,
         application_id=application.id,
     )
-    post_url_assert_status(ccpo, url, 302, data=step_1_form_data)
-    post_url_assert_status(owner, url, 302, data=step_1_form_data)
-    post_url_assert_status(rando, url, 404, data=step_1_form_data)
+    post_url_assert_status(ccpo, url, 302, data=_form_data())
+    post_url_assert_status(owner, url, 302, data=_form_data())
+    post_url_assert_status(rando, url, 404, data=_form_data())
 
 
 # applications.view_new_application_step_2