Catch error on update applications route when app name is duplicated within a portfolio

.secrets.baseline

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

atst/routes/applications/new.py

@@ -1,9 +1,7 @@
-from flask import redirect, render_template, request as http_request, url_for, g
+from flask import redirect, render_template, request as http_request, url_for
 
 from .blueprint import applications_bp
 from atst.domain.applications import Applications
-from atst.domain.exceptions import AlreadyExistsError
-from atst.domain.portfolios import Portfolios
 from atst.forms.application import NameAndDescriptionForm, EnvironmentsForm
 from atst.domain.authz.decorator import user_can_access_decorator as user_can
 from atst.models.permissions import Permissions
@@ -13,6 +11,7 @@ from atst.routes.applications.settings import (
     get_new_member_form,
     handle_create_member,
     handle_update_member,
+    handle_update_application,
 )
 
 
@@ -38,31 +37,6 @@ def render_new_application_form(
     return render_template(template, **render_args)
 
 
-def update_application(form, application_id=None, portfolio_id=None):
-    if form.validate():
-        application = None
-        try:
-            if application_id:
-                application = Applications.get(application_id)
-                application = Applications.update(application, form.data)
-                flash("application_updated", application_name=application.name)
-            else:
-                portfolio = Portfolios.get_for_update(portfolio_id)
-                application = Applications.create(
-                    g.current_user, portfolio, **form.data
-                )
-                flash("application_created", application_name=application.name)
-
-            return application
-
-        except AlreadyExistsError:
-            flash("application_name_error", name=form.data["name"])
-            return False
-
-    else:
-        return False
-
-
 @applications_bp.route("/portfolios/<portfolio_id>/applications/new")
 @applications_bp.route("/applications/<application_id>/new/step_1")
 @user_can(Permissions.CREATE_APPLICATION, message="view create new application form")
@@ -90,7 +64,7 @@ def create_or_update_new_application_step_1(portfolio_id=None, application_id=No
     form = get_new_application_form(
         {**http_request.form}, NameAndDescriptionForm, application_id
     )
-    application = update_application(form, application_id, portfolio_id)
+    application = handle_update_application(form, application_id, portfolio_id)
 
     if application:
         return redirect(

atst/routes/applications/settings.py

@@ -16,6 +16,7 @@ from atst.domain.csp.cloud import GeneralCSPException
 from atst.domain.common import Paginator
 from atst.domain.environment_roles import EnvironmentRoles
 from atst.domain.invitations import ApplicationInvitations
+from atst.domain.portfolios import Portfolios
 from atst.forms.application_member import NewForm as NewMemberForm, UpdateMemberForm
 from atst.forms.application import NameAndDescriptionForm, EditEnvironmentForm
 from atst.forms.data import ENV_ROLE_NO_ACCESS as NO_ACCESS
@@ -275,6 +276,29 @@ def handle_update_environment(form, application=None, environment=None):
         return False
 
 
+def handle_update_application(form, application_id=None, portfolio_id=None):
+    if form.validate():
+        application = None
+
+        try:
+            if application_id:
+                application = Applications.get(application_id)
+                application = Applications.update(application, form.data)
+                flash("application_updated", application_name=application.name)
+            else:
+                portfolio = Portfolios.get_for_update(portfolio_id)
+                application = Applications.create(
+                    g.current_user, portfolio, **form.data
+                )
+                flash("application_created", application_name=application.name)
+
+            return application
+
+        except AlreadyExistsError:
+            flash("application_name_error", name=form.data["name"])
+            return False
+
+
 @applications_bp.route("/applications/<application_id>/settings")
 @user_can(Permissions.VIEW_APPLICATION, message="view application edit form")
 def settings(application_id):
@@ -334,10 +358,9 @@ def new_environment(application_id):
 def update(application_id):
     application = Applications.get(application_id)
     form = NameAndDescriptionForm(http_request.form)
-    if form.validate():
+    updated_application = handle_update_application(form, application_id)
-        application_data = form.data
-        Applications.update(application, application_data)
 
+    if updated_application:
         return redirect(
             url_for(
                 "applications.portfolio_applications",
@@ -345,7 +368,10 @@ def update(application_id):
             )
         )
     else:
-        return render_settings_page(application=application, application_form=form)
+        return (
+            render_settings_page(application=application, show_flash=True),
+            400,
+        )
 
 
 @applications_bp.route("/applications/<application_id>/delete", methods=["POST"])

tests/routes/applications/test_settings.py

@@ -274,6 +274,23 @@ def test_user_without_permission_cannot_update_application(client, user_session)
     assert application.description == "Cool stuff happening here!"
 
 
+def test_update_application_enforces_unique_name(client, user_session, session):
+    portfolio = PortfolioFactory.create()
+    name = "Test Application"
+    application = ApplicationFactory.create(portfolio=portfolio, name=name)
+    dupe_application = ApplicationFactory.create(portfolio=portfolio)
+    user_session(portfolio.owner)
+
+    session.begin_nested()
+    response = client.post(
+        url_for("applications.update", application_id=dupe_application.id),
+        data={"name": name, "description": dupe_application.description},
+    )
+    session.rollback()
+
+    assert response.status_code == 400
+
+
 def test_user_can_only_access_apps_in_their_portfolio(client, user_session):
     portfolio = PortfolioFactory.create()
     other_portfolio = PortfolioFactory.create(

tests/test_access.py

@@ -538,10 +538,16 @@ def test_applications_update_access(post_url_assert_status):
     )
     app = portfolio.applications[0]
 
+    def _form_data():
+        return {
+            "name": "Test Application %s" % (random.randrange(1, 1000)),
+            "description": "This is only a test",
+        }
+
     url = url_for("applications.update", application_id=app.id)
-    post_url_assert_status(dev, url, 200)
+    post_url_assert_status(dev, url, 302, data=_form_data())
-    post_url_assert_status(ccpo, url, 200)
+    post_url_assert_status(ccpo, url, 302, data=_form_data())
-    post_url_assert_status(rando, url, 404)
+    post_url_assert_status(rando, url, 404, data=_form_data())
 
 
 # applications.update_environments