From 63a5d9274b86a24ffd3e44b15ab7296385ae6207 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 16 Dec 2019 14:38:46 -0500 Subject: [PATCH] Update route to handle error when env names are duplicated --- atst/routes/applications/settings.py | 69 +++++++++++++--------- atst/utils/flash.py | 5 ++ templates/applications/settings.html | 3 + tests/routes/applications/test_settings.py | 20 ++++++- translations.yaml | 6 +- 5 files changed, 70 insertions(+), 33 deletions(-) diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index f2d252a9..66131c15 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -1,4 +1,10 @@ -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, + g, +) from .blueprint import applications_bp from atst.domain.exceptions import AlreadyExistsError @@ -245,16 +251,36 @@ def handle_update_member(application_id, application_role_id, form_data): # TODO: flash error message +def handle_update_environment(form, application=None, environment=None): + if form.validate(): + try: + if environment: + environment = Environments.update( + environment=environment, name=form.name.data + ) + flash("application_environments_updated") + else: + environment = Environments.create( + g.current_user, application=application, name=form.name.data + ) + flash("environment_added", environment_name=form.name.data) + + return environment + + except AlreadyExistsError: + flash("application_environments_name_error", name=form.name.data) + return False + + else: + return False + + @applications_bp.route("/applications//settings") @user_can(Permissions.VIEW_APPLICATION, message="view application edit form") def settings(application_id): application = Applications.get(application_id) - return render_settings_page( - application=application, - active_toggler=http_request.args.get("active_toggler"), - active_toggler_section=http_request.args.get("active_toggler_section"), - ) + return render_settings_page(application=application,) @applications_bp.route("/environments//edit", methods=["POST"]) @@ -264,31 +290,21 @@ def update_environment(environment_id): application = environment.application env_form = EditEnvironmentForm(obj=environment, formdata=http_request.form) + updated_environment = handle_update_environment( + form=env_form, application=application, environment=environment + ) - if env_form.validate(): - Environments.update(environment=environment, name=env_form.name.data) - - flash("application_environments_updated") - + if updated_environment: return redirect( url_for( "applications.settings", application_id=application.id, fragment="application-environments", _anchor="application-environments", - active_toggler=environment.id, - active_toggler_section="edit", ) ) else: - return ( - render_settings_page( - application=application, - active_toggler=environment.id, - active_toggler_section="edit", - ), - 400, - ) + return (render_settings_page(application=application, show_flash=True), 400) @applications_bp.route( @@ -298,14 +314,9 @@ def update_environment(environment_id): def new_environment(application_id): application = Applications.get(application_id) env_form = EditEnvironmentForm(formdata=http_request.form) + environment = handle_update_environment(form=env_form, application=application) - if env_form.validate(): - Environments.create( - g.current_user, application=application, name=env_form.name.data - ) - - flash("environment_added", environment_name=env_form.data["name"]) - + if environment: return redirect( url_for( "applications.settings", @@ -315,7 +326,7 @@ def new_environment(application_id): ) ) else: - return (render_settings_page(application=application), 400) + return (render_settings_page(application=application, show_flash=True), 400) @applications_bp.route("/applications//edit", methods=["POST"]) diff --git a/atst/utils/flash.py b/atst/utils/flash.py index 0de143bb..f876330f 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -29,6 +29,11 @@ MESSAGES = { """, "category": "success", }, + "application_environments_name_error": { + "title_template": "", + "message_template": """{{ 'flash.application.env_name_error.message' | translate({ 'name': name }) }}""", + "category": "error", + }, "application_environments_updated": { "title_template": "Application environments updated", "message_template": "Application environments have been updated", diff --git a/templates/applications/settings.html b/templates/applications/settings.html index c8e41fcd..875f0232 100644 --- a/templates/applications/settings.html +++ b/templates/applications/settings.html @@ -13,6 +13,9 @@ {% block application_content %} + {% if show_flash -%} + {% include "fragments/flash.html" %} + {%- endif %}

{{ 'portfolios.applications.settings.name_description' | translate }}

{% if user_can(permissions.EDIT_APPLICATION) %} diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py index 08c979ad..597664d1 100644 --- a/tests/routes/applications/test_settings.py +++ b/tests/routes/applications/test_settings.py @@ -52,8 +52,6 @@ def test_updating_application_environments_success(client, user_session): _external=True, fragment="application-environments", _anchor="application-environments", - active_toggler=environment.id, - active_toggler_section="edit", ) assert environment.name == "new name a" @@ -78,6 +76,24 @@ def test_update_environment_failure(client, user_session): assert environment.name == "original name" +def test_enforces_unique_env_name(client, user_session, session): + application = ApplicationFactory.create() + user = application.portfolio.owner + name = "New Environment" + environment = EnvironmentFactory.create(application=application, name=name) + form_data = {"name": name} + user_session(user) + + session.begin_nested() + response = client.post( + url_for("applications.new_environment", application_id=application.id), + data=form_data, + ) + session.rollback() + + assert response.status_code == 400 + + def test_application_settings(client, user_session): portfolio = PortfolioFactory.create() application = Applications.create( diff --git a/translations.yaml b/translations.yaml index 6a678f48..e0277fff 100644 --- a/translations.yaml +++ b/translations.yaml @@ -116,6 +116,8 @@ flash: 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.' + env_name_error: + message: 'The environment name {name} has already been used in this application. 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.' @@ -199,7 +201,7 @@ forms:

- defense_component: + defense_component: label: "Select DoD component(s) funding your Portfolio:" choices: air_force: Air Force @@ -211,7 +213,7 @@ forms: help_text: |

Select the DOD component(s) that will fund all Applications within this Portfolio. - In JEDI, multiple DoD organizations can fund the same Portfolio.
+ In JEDI, multiple DoD organizations can fund the same Portfolio.
Select all that apply.

attachment: