From ffbf6122903db0ee6e3a47db136b8b1be57c129e Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Fri, 13 Dec 2019 16:08:22 -0500 Subject: [PATCH] Update route to catch error when app name uniqueness is violated and display a error message --- atst/routes/applications/new.py | 38 ++++++++++++++++++++------- atst/utils/flash.py | 5 ++++ tests/routes/applications/test_new.py | 18 +++++++++++++ translations.yaml | 2 ++ 4 files changed, 53 insertions(+), 10 deletions(-) diff --git a/atst/routes/applications/new.py b/atst/routes/applications/new.py index a1a1a54c..e9238775 100644 --- a/atst/routes/applications/new.py +++ b/atst/routes/applications/new.py @@ -2,6 +2,7 @@ from flask import redirect, render_template, request as http_request, url_for, g 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 @@ -37,6 +38,31 @@ 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//applications/new") @applications_bp.route("/applications//new/step_1") @user_can(Permissions.CREATE_APPLICATION, message="view create new application form") @@ -64,17 +90,9 @@ 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) - if form.validate(): - application = None - 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) + if application: return redirect( url_for( "applications.update_new_application_step_2", diff --git a/atst/utils/flash.py b/atst/utils/flash.py index fabba4d2..0de143bb 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -64,6 +64,11 @@ MESSAGES = { "message_template": "You have successfully updated the permissions for {{ user_name }}", "category": "success", }, + "application_name_error": { + "title_template": "", + "message_template": """{{ 'flash.application.name_error.message' | translate({ 'name': name }) }}""", + "category": "error", + }, "ccpo_user_added": { "title_template": translate("flash.success"), "message_template": "You have successfully given {{ user_name }} CCPO permissions.", diff --git a/tests/routes/applications/test_new.py b/tests/routes/applications/test_new.py index c4fb83ed..045ac19e 100644 --- a/tests/routes/applications/test_new.py +++ b/tests/routes/applications/test_new.py @@ -70,6 +70,24 @@ def test_post_name_and_description_for_update(client, session, user_session): assert application.description == "This is only a test" +def test_post_name_and_description_enforces_unique_name(client, user_session, session): + portfolio = PortfolioFactory.create() + name = "Test Application" + application = ApplicationFactory.create(portfolio=portfolio, name=name) + user_session(portfolio.owner) + + session.begin_nested() + response = client.post( + url_for( + "applications.create_new_application_step_1", portfolio_id=portfolio.id + ), + data={"name": name, "description": "This is only a test"}, + ) + session.rollback() + + assert response.status_code == 400 + + def test_get_environments(client, user_session): application = ApplicationFactory.create() user_session(application.portfolio.owner) diff --git a/translations.yaml b/translations.yaml index 1d876e97..6a678f48 100644 --- a/translations.yaml +++ b/translations.yaml @@ -114,6 +114,8 @@ flash: message: '{application_name} has been successfully created. You may continue on to provision environments and assign team members now, or come back and complete these tasks at a later time.' updated: 'You have successfully updated the {application_name} application.' deleted: 'You have successfully deleted the {application_name} application. To view the retained activity log, visit the portfolio administration page.' + name_error: + message: 'The application name {name} has already been used in this portfolio. Please enter a unique name.' delete_member_success: 'You have successfully deleted {member_name} from the portfolio.' deleted_member: Portfolio member deleted environment_added: 'The environment "{env_name}" has been added to the application.'