From b270d0e4b0bcc25545d37fd50fa2678fc39911dd Mon Sep 17 00:00:00 2001 From: graham-dds Date: Tue, 17 Dec 2019 16:10:21 -0500 Subject: [PATCH 1/7] Update new portfolio layout - Move CTA buttons to floating footer - separated -> bordered --- styles/components/_forms.scss | 3 +- templates/portfolios/new/step_1.html | 54 ++++++++++++++-------------- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/styles/components/_forms.scss b/styles/components/_forms.scss index a6e0709a..43c31c43 100644 --- a/styles/components/_forms.scss +++ b/styles/components/_forms.scss @@ -1,7 +1,8 @@ // Form Grid .form-row { margin: ($gap * 4) 0; - &--separated { + + &--bordered { border-bottom: $color-gray-lighter 1px solid; } diff --git a/templates/portfolios/new/step_1.html b/templates/portfolios/new/step_1.html index 4613b063..3383128b 100644 --- a/templates/portfolios/new/step_1.html +++ b/templates/portfolios/new/step_1.html @@ -16,36 +16,36 @@ {{ StickyCTA(text="Create New Portfolio") }} -
- {{ form.csrf_token }} -
-
- {{ TextInput(form.name, optional=False) }} - {{"forms.portfolio.name.help_text" | translate | safe }} +
+ + {{ form.csrf_token }} +
+
+ {{ TextInput(form.name, optional=False, classes="form-col") }} + {{"forms.portfolio.name.help_text" | translate | safe }} +
-
-
-
- {{ TextInput(form.description, paragraph=True) }} - {{"forms.portfolio.description.help_text" | translate | safe }} +
+
+ {{ TextInput(form.description, paragraph=True) }} + {{"forms.portfolio.description.help_text" | translate | safe }} +
-
-
-
- {{ MultiCheckboxInput(form.defense_component, optional=False) }} - {{ "forms.portfolio.defense_component.help_text" | translate | safe }} +
+
+ {{ MultiCheckboxInput(form.defense_component, optional=False) }} + {{ "forms.portfolio.defense_component.help_text" | translate | safe }} +
-
-
- {{ - SaveButton( - text=('common.save' | translate), - form="portfolio-create", - element="input", - ) - }} -
- +
+ {% block next_button %} + {{ SaveButton(text=('common.save' | translate), form="portfolio-create", element="input") }} + {% endblock %} + + Cancel + + +
{% endblock %} From 812caf5d7d4fdc537b3e244cd5fea146feccb861 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 16 Dec 2019 11:02:18 -0500 Subject: [PATCH 2/7] Update schema and create/update Environments domain methods to enforce environment name uniqueness within an application context. --- ...dd_uniqueness_contraint_to_environment_.py | 28 +++++++++++++++++++ atst/domain/environments.py | 6 ++-- atst/models/environment.py | 8 +++++- tests/domain/test_environments.py | 23 ++++++++++++++- 4 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 alembic/versions/08f2a640e9c2_add_uniqueness_contraint_to_environment_.py diff --git a/alembic/versions/08f2a640e9c2_add_uniqueness_contraint_to_environment_.py b/alembic/versions/08f2a640e9c2_add_uniqueness_contraint_to_environment_.py new file mode 100644 index 00000000..56985f73 --- /dev/null +++ b/alembic/versions/08f2a640e9c2_add_uniqueness_contraint_to_environment_.py @@ -0,0 +1,28 @@ +"""add uniqueness contraint to environment within an application + +Revision ID: 08f2a640e9c2 +Revises: c487d91f1a26 +Create Date: 2019-12-16 10:43:12.331095 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '08f2a640e9c2' # pragma: allowlist secret +down_revision = 'c487d91f1a26' # pragma: allowlist secret +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_unique_constraint('environments_name_application_id_key', 'environments', ['name', 'application_id']) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint('environments_name_application_id_key', 'environments', type_='unique') + # ### end Alembic commands ### diff --git a/atst/domain/environments.py b/atst/domain/environments.py index a1056623..fd5f3eb0 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -12,6 +12,7 @@ from atst.models import ( CLIN, ) from atst.domain.environment_roles import EnvironmentRoles +from atst.utils import update_or_raise_already_exists_error from .exceptions import NotFoundError, DisabledError @@ -21,7 +22,7 @@ class Environments(object): def create(cls, user, application, name): environment = Environment(application=application, name=name, creator=user) db.session.add(environment) - db.session.commit() + update_or_raise_already_exists_error(message="environment") return environment @classmethod @@ -39,7 +40,8 @@ class Environments(object): if name is not None: environment.name = name db.session.add(environment) - db.session.commit() + update_or_raise_already_exists_error(message="environment") + return environment @classmethod def get(cls, environment_id): diff --git a/atst/models/environment.py b/atst/models/environment.py index 5fc642e5..115f3ed7 100644 --- a/atst/models/environment.py +++ b/atst/models/environment.py @@ -1,4 +1,4 @@ -from sqlalchemy import Column, ForeignKey, String, TIMESTAMP +from sqlalchemy import Column, ForeignKey, String, TIMESTAMP, UniqueConstraint from sqlalchemy.orm import relationship from sqlalchemy.dialects.postgresql import JSONB from enum import Enum @@ -38,6 +38,12 @@ class Environment( primaryjoin="and_(EnvironmentRole.environment_id == Environment.id, EnvironmentRole.deleted == False)", ) + __table_args__ = ( + UniqueConstraint( + "name", "application_id", name="environments_name_application_id_key" + ), + ) + class ProvisioningStatus(Enum): PENDING = "pending" COMPLETED = "completed" diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index 3aa5f547..298f2675 100644 --- a/tests/domain/test_environments.py +++ b/tests/domain/test_environments.py @@ -4,7 +4,7 @@ from uuid import uuid4 from atst.domain.environments import Environments from atst.domain.environment_roles import EnvironmentRoles -from atst.domain.exceptions import NotFoundError, DisabledError +from atst.domain.exceptions import AlreadyExistsError, DisabledError, NotFoundError from atst.models.environment_role import CSPRole, EnvironmentRole from tests.factories import ( @@ -100,6 +100,27 @@ def test_update_environment(): assert environment.name == "name 2" +def test_create_does_not_duplicate_names_within_application(): + application = ApplicationFactory.create() + name = "Your Environment" + user = application.portfolio.owner + + assert Environments.create(user, application, name) + with pytest.raises(AlreadyExistsError): + Environments.create(user, application, name) + + +def test_update_does_not_duplicate_names_within_application(): + application = ApplicationFactory.create() + name = "Your Environment" + environment = EnvironmentFactory.create(application=application, name=name) + dupe_env = EnvironmentFactory.create(application=application) + user = application.portfolio.owner + + with pytest.raises(AlreadyExistsError): + Environments.update(dupe_env, name) + + class EnvQueryTest: @property def NOW(self): From 63a5d9274b86a24ffd3e44b15ab7296385ae6207 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 16 Dec 2019 14:38:46 -0500 Subject: [PATCH 3/7] 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: From e82e0b482a5cd4c02924dacb127cce716d54d021 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 17 Dec 2019 15:49:48 -0500 Subject: [PATCH 4/7] remove unused imports --- .../08f2a640e9c2_add_uniqueness_contraint_to_environment_.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/alembic/versions/08f2a640e9c2_add_uniqueness_contraint_to_environment_.py b/alembic/versions/08f2a640e9c2_add_uniqueness_contraint_to_environment_.py index 56985f73..bfc5b894 100644 --- a/alembic/versions/08f2a640e9c2_add_uniqueness_contraint_to_environment_.py +++ b/alembic/versions/08f2a640e9c2_add_uniqueness_contraint_to_environment_.py @@ -6,8 +6,6 @@ Create Date: 2019-12-16 10:43:12.331095 """ from alembic import op -import sqlalchemy as sa -from sqlalchemy.dialects import postgresql # revision identifiers, used by Alembic. revision = '08f2a640e9c2' # pragma: allowlist secret From d300ccf31b74e8fd9cf8b304edaf4eb650957d47 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 18 Dec 2019 10:37:19 -0500 Subject: [PATCH 5/7] Catch error on update applications route when app name is duplicated within a portfolio --- .secrets.baseline | 4 +-- atst/routes/applications/new.py | 32 ++------------------ atst/routes/applications/settings.py | 34 +++++++++++++++++++--- tests/routes/applications/test_settings.py | 17 +++++++++++ tests/test_access.py | 12 ++++++-- 5 files changed, 61 insertions(+), 38 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 24361804..ffa18c6e 100644 --- a/.secrets.baseline +++ b/.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" } ] diff --git a/atst/routes/applications/new.py b/atst/routes/applications/new.py index e9238775..9d673b04 100644 --- a/atst/routes/applications/new.py +++ b/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//applications/new") @applications_bp.route("/applications//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( diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index 66131c15..fc95c615 100644 --- a/atst/routes/applications/settings.py +++ b/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//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(): - application_data = form.data - Applications.update(application, application_data) + updated_application = handle_update_application(form, application_id) + 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//delete", methods=["POST"]) diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py index 597664d1..e9117166 100644 --- a/tests/routes/applications/test_settings.py +++ b/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( diff --git a/tests/test_access.py b/tests/test_access.py index ad4bd5be..ccae69ba 100644 --- a/tests/test_access.py +++ b/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(ccpo, url, 200) - post_url_assert_status(rando, url, 404) + post_url_assert_status(dev, url, 302, data=_form_data()) + post_url_assert_status(ccpo, url, 302, data=_form_data()) + post_url_assert_status(rando, url, 404, data=_form_data()) # applications.update_environments From f543602076b6468eb7eddc8a2c833a5976bac0d6 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 18 Dec 2019 10:54:00 -0500 Subject: [PATCH 6/7] Update function name to better reflect what it does --- atst/domain/applications.py | 6 +++--- atst/domain/environments.py | 6 +++--- atst/domain/task_orders.py | 6 +++--- atst/utils/__init__.py | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/atst/domain/applications.py b/atst/domain/applications.py index c2321194..3dbb9953 100644 --- a/atst/domain/applications.py +++ b/atst/domain/applications.py @@ -11,7 +11,7 @@ from atst.models import ( ApplicationRoleStatus, EnvironmentRole, ) -from atst.utils import first_or_none, update_or_raise_already_exists_error +from atst.utils import first_or_none, commit_or_raise_already_exists_error class Applications(BaseDomainClass): @@ -28,7 +28,7 @@ class Applications(BaseDomainClass): if environment_names: Environments.create_many(user, application, environment_names) - update_or_raise_already_exists_error(message="application") + commit_or_raise_already_exists_error(message="application") return application @classmethod @@ -55,7 +55,7 @@ class Applications(BaseDomainClass): ) db.session.add(application) - update_or_raise_already_exists_error(message="application") + commit_or_raise_already_exists_error(message="application") return application @classmethod diff --git a/atst/domain/environments.py b/atst/domain/environments.py index fd5f3eb0..b8a59485 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -12,7 +12,7 @@ from atst.models import ( CLIN, ) from atst.domain.environment_roles import EnvironmentRoles -from atst.utils import update_or_raise_already_exists_error +from atst.utils import commit_or_raise_already_exists_error from .exceptions import NotFoundError, DisabledError @@ -22,7 +22,7 @@ class Environments(object): def create(cls, user, application, name): environment = Environment(application=application, name=name, creator=user) db.session.add(environment) - update_or_raise_already_exists_error(message="environment") + commit_or_raise_already_exists_error(message="environment") return environment @classmethod @@ -40,7 +40,7 @@ class Environments(object): if name is not None: environment.name = name db.session.add(environment) - update_or_raise_already_exists_error(message="environment") + commit_or_raise_already_exists_error(message="environment") return environment @classmethod diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index 8ad8b0f4..9ecf41e9 100644 --- a/atst/domain/task_orders.py +++ b/atst/domain/task_orders.py @@ -4,7 +4,7 @@ from atst.database import db from atst.models.clin import CLIN from atst.models.task_order import TaskOrder, SORT_ORDERING from . import BaseDomainClass -from atst.utils import update_or_raise_already_exists_error +from atst.utils import commit_or_raise_already_exists_error class TaskOrders(BaseDomainClass): @@ -15,7 +15,7 @@ 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) - update_or_raise_already_exists_error(message="task_order") + commit_or_raise_already_exists_error(message="task_order") TaskOrders.create_clins(task_order.id, clins) return task_order @@ -34,7 +34,7 @@ class TaskOrders(BaseDomainClass): task_order.number = number db.session.add(task_order) - update_or_raise_already_exists_error(message="task_order") + commit_or_raise_already_exists_error(message="task_order") return task_order @classmethod diff --git a/atst/utils/__init__.py b/atst/utils/__init__.py index 9772af67..d3f284cc 100644 --- a/atst/utils/__init__.py +++ b/atst/utils/__init__.py @@ -30,7 +30,7 @@ def pick(keys, dct): return {k: v for (k, v) in dct.items() if k in _keys} -def update_or_raise_already_exists_error(message): +def commit_or_raise_already_exists_error(message): try: db.session.commit() except IntegrityError: From 13e8cb8ee108c213423cb702e399e0c24687f9c5 Mon Sep 17 00:00:00 2001 From: graham-dds Date: Tue, 17 Dec 2019 16:43:35 -0500 Subject: [PATCH 7/7] Move form buttons to floating bottom action bar Form action buttons were previously in the sicky CTA bar. This commit moves them to a bottom action bar. --- styles/elements/_action_group.scss | 2 +- templates/home.html | 11 ++----- templates/task_orders/builder_base.html | 39 +++++++++++++------------ templates/task_orders/step_1.html | 1 - translations.yaml | 3 +- 5 files changed, 26 insertions(+), 30 deletions(-) diff --git a/styles/elements/_action_group.scss b/styles/elements/_action_group.scss index 1f79f7d2..73cad181 100644 --- a/styles/elements/_action_group.scss +++ b/styles/elements/_action_group.scss @@ -46,7 +46,7 @@ background: white; right: 0; padding-right: $gap * 4; - border-top: 1px solid $color-gray-light; + border-top: 1px solid $color-gray-lighter; width: 100%; z-index: 1; } diff --git a/templates/home.html b/templates/home.html index 4e459639..aadcd2a8 100644 --- a/templates/home.html +++ b/templates/home.html @@ -13,12 +13,6 @@ {% set sticky_header = "home.get_started" | translate %} {% endif %} - {% call StickyCTA(sticky_header) %} - - {{ "home.add_portfolio_button_text" | translate }} - - {% endcall %} -
{% include "fragments/flash.html" %} @@ -92,9 +86,10 @@
-
- JEDI heirarchy diagram + + {{ "home.add_portfolio_button_text" | translate }} +
diff --git a/templates/task_orders/builder_base.html b/templates/task_orders/builder_base.html index 362a1038..70bc20c9 100644 --- a/templates/task_orders/builder_base.html +++ b/templates/task_orders/builder_base.html @@ -8,8 +8,26 @@
{{ form.csrf_token }} - {% call StickyCTA(text=('task_orders.form.sticky_header_text' | translate({"step": step}) )) %} - + {{ StickyCTA( + text='task_orders.form.sticky_header_text' | translate, + context=('task_orders.form.sticky_header_context' | translate({"step": step}) )) }} + + {% call Modal(name='cancel', dismissable=True) %} +
+

Do you want to save this draft?

+
+ + +
+
+ {% endcall %} + + {% include "fragments/flash.html" %} + +
+ {% block to_builder_form_field %}{% endblock %} +
+ {% block next_button %} - {% endcall %} - - {% call Modal(name='cancel', dismissable=True) %} -
-

Do you want to save this draft?

-
- - -
-
- {% endcall %} - - {% include "fragments/flash.html" %} - -
- {% block to_builder_form_field %}{% endblock %} -
diff --git a/templates/task_orders/step_1.html b/templates/task_orders/step_1.html index 758099fa..6be3218c 100644 --- a/templates/task_orders/step_1.html +++ b/templates/task_orders/step_1.html @@ -1,7 +1,6 @@ {% extends "task_orders/builder_base.html" %} {% from 'components/icon.html' import Icon %} -{% from "components/sticky_cta.html" import StickyCTA %} {% from "task_orders/form_header.html" import TOFormStepHeader %} {% from 'components/upload_input.html' import UploadInput %} diff --git a/translations.yaml b/translations.yaml index 6a678f48..8fe26475 100644 --- a/translations.yaml +++ b/translations.yaml @@ -526,7 +526,8 @@ task_orders: description: Finally, plase confirm that your uploaded document representing the information you've entered contains the required signature from your Contracting Officer. You will be informed as soon as CCPO completes their review. alert_message: All task orders require a Contracting Officer signature. next_button: 'Confirm & Submit' - sticky_header_text: 'Add Task Order (step {step} of 5)' + sticky_header_text: 'Add Task Order' + sticky_header_context: 'Step {step} of 5' empty_state: header: Add approved task orders message: Upload your approved Task Order here. You are required to confirm you have the appropriate signature. You will have the ability to add additional approved Task Orders with more funding to this Portfolio in the future.