From 060c6834bff249d590b517f18e70b67d47d4507d Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 1 May 2019 13:52:49 -0400 Subject: [PATCH] Add save button and temp member role toggle to env member form --- atst/domain/environments.py | 2 +- atst/forms/app_settings.py | 11 +++- atst/forms/data.py | 4 +- atst/routes/applications/settings.py | 35 ++++++++---- atst/utils/flash.py | 5 ++ js/components/toggler.js | 1 + styles/sections/_application_edit.scss | 48 +++++++++++++++- templates/components/toggle_list.html | 4 +- .../applications/edit_environments.html | 55 +++++++++++++++++-- tests/domain/test_environments.py | 8 ++- tests/routes/applications/test_settings.py | 12 ++-- tests/test_access.py | 2 +- 12 files changed, 148 insertions(+), 39 deletions(-) diff --git a/atst/domain/environments.py b/atst/domain/environments.py index 9c8e4d52..50484bad 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -73,7 +73,7 @@ class Environments(object): def update_env_role(cls, environment, user, new_role): updated = False - if new_role is None: + if new_role == "no_access": updated = EnvironmentRoles.delete(user.id, environment.id) else: env_role = EnvironmentRoles.get(user.id, environment.id) diff --git a/atst/forms/app_settings.py b/atst/forms/app_settings.py index 44bcbc8e..c6320371 100644 --- a/atst/forms/app_settings.py +++ b/atst/forms/app_settings.py @@ -1,14 +1,19 @@ from flask_wtf import FlaskForm -from wtforms.fields import StringField, HiddenField, RadioField, FieldList, FormField +from wtforms.fields import FieldList, FormField, HiddenField, RadioField from .forms import BaseForm from .data import ENV_ROLES class EnvMemberRoleForm(FlaskForm): - name = StringField() user_id = HiddenField() - role = RadioField(choices=ENV_ROLES, coerce=BaseForm.remove_empty_string) + role = RadioField(choices=ENV_ROLES, default="no_access") + + @property + def data(self): + _data = super().data + _data.pop("csrf_token", None) + return _data class EnvironmentRolesForm(BaseForm): diff --git a/atst/forms/data.py b/atst/forms/data.py index 2e0b689d..2ddc3124 100644 --- a/atst/forms/data.py +++ b/atst/forms/data.py @@ -217,4 +217,6 @@ REQUIRED_DISTRIBUTIONS = [ ("other", "Other as necessary"), ] -ENV_ROLES = [(role.value, role.value) for role in CSPRole] + [(None, "No access")] +ENV_ROLES = [(role.value, role.value) for role in CSPRole] + [ + ("no_access", "No access") +] diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index a87415a2..283dfce4 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -9,7 +9,6 @@ from atst.forms.application import ApplicationForm, EditEnvironmentForm from atst.domain.authz.decorator import user_can_access_decorator as user_can from atst.models.environment_role import CSPRole from atst.domain.exceptions import NotFoundError -from atst.models.environment_role import CSPRole from atst.models.permissions import Permissions from atst.utils.flash import formatted_flash as flash @@ -48,11 +47,15 @@ def sort_env_users_by_role(env): def data_for_env_members_form(environment): data = {"env_id": environment.id, "team_roles": []} - for user in environment.users: + for user in environment.application.users: env_role = EnvironmentRoles.get(user.id, environment.id) - data["team_roles"].append( - {"name": user.full_name, "user_id": user.id, "role": env_role.displayname} - ) + + if env_role: + role = env_role.displayname + else: + role = "no_access" + + data["team_roles"].append({"user_id": user.id, "role": role}) return data @@ -68,10 +71,8 @@ def check_users_are_in_application(user_ids, application): @applications_bp.route("/applications//settings") @user_can(Permissions.VIEW_APPLICATION, message="view application edit form") def settings(application_id): - # refactor like portfolio admin render function application = Applications.get(application_id) form = ApplicationForm(name=application.name, description=application.description) - csp_roles = [role.value for role in CSPRole] return render_template( "portfolios/applications/settings.html", @@ -145,12 +146,12 @@ def update(application_id): def update_env_roles(environment_id): environment = Environments.get(environment_id) application = environment.application - env_roles_form = EnvironmentRolesForm(http_request.form) + form = EnvironmentRolesForm(formdata=http_request.form) - if env_roles_form.validate(): + if form.validate(): try: - user_ids = [user["user_id"] for user in env_roles_form.data["team_roles"]] + user_ids = [user["user_id"] for user in form.data["team_roles"]] check_users_are_in_application(user_ids, application) except NotFoundError as err: app.logger.warning( @@ -161,11 +162,21 @@ def update_env_roles(environment_id): ) raise (err) - env_data = env_roles_form.data + env_data = form.data Environments.update_env_roles_by_environment( environment_id=environment_id, team_roles=env_data["team_roles"] ) - return redirect(url_for("applications.settings", application_id=application.id)) + + flash("application_environment_members_updated") + + return redirect( + url_for( + "applications.settings", + application_id=application.id, + fragment="application-environments", + _anchor="application-environments", + ) + ) else: # TODO: Create a better pattern to handle when a form doesn't validate # if a user is submitting the data via the web page then they diff --git a/atst/utils/flash.py b/atst/utils/flash.py index f2b07ed6..c411e22e 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -7,6 +7,11 @@ MESSAGES = { "message_template": 'The environment "{{ environment_name }}" has been deleted', "category": "success", }, + "application_environment_members_updated": { + "title_template": "Application environment members updated", + "message_template": "Application environment members have been updated", + "category": "success", + }, "application_environments_updated": { "title_template": "Application environments updated", "message_template": "Application environments have been updated", diff --git a/js/components/toggler.js b/js/components/toggler.js index f1c34c96..2f1c1f0b 100644 --- a/js/components/toggler.js +++ b/js/components/toggler.js @@ -8,6 +8,7 @@ export default { mixins: [FormMixin], components: { + optionsinput, textinput, optionsinput, }, diff --git a/styles/sections/_application_edit.scss b/styles/sections/_application_edit.scss index a24d9373..846ae3b1 100644 --- a/styles/sections/_application_edit.scss +++ b/styles/sections/_application_edit.scss @@ -25,11 +25,14 @@ .app-team-settings-link { font-size: $small-font-size; font-weight: $font-normal; - padding-left: $gap * 2; +} + +.environment-roles { + padding: 0 ($gap * 3) ($gap * 3); } .environment-role { - padding: $gap * 3; + padding: ($gap * 2) 0; h4 { margin-bottom: $gap / 4; @@ -50,10 +53,35 @@ margin: $gap; white-space: nowrap; width: 20rem; + position: relative; &.unassigned { border: solid 1px $color-gray-light; } + + .icon-link { + padding: 0; + } + + .environment-role__user-field { + position: absolute; + background-color: $color-white; + margin-top: $gap * 2; + padding: $gap; + left: -0.1rem; + border: solid 1px $color-gray-light; + width: 20rem; + z-index: 3; + + .usa-input { + margin: 0; + + li { + background-color: white; + border: none; + } + } + } } .environment-role__no-user { @@ -94,3 +122,19 @@ font-weight: $font-normal; color: $color-gray-medium; } + +.application-list-item { + .usa-button-primary { + width: $search-button-width * 2; + } + + .action-group-cancel { + position: relative; + + .action-group-cancel__action { + position: absolute; + right: ($search-button-width * 2) + ($gap * 2); + top: -($gap * 8); + } + } +} diff --git a/templates/components/toggle_list.html b/templates/components/toggle_list.html index 08526c5a..64f85b66 100644 --- a/templates/components/toggle_list.html +++ b/templates/components/toggle_list.html @@ -9,8 +9,8 @@ {% endmacro %} -{% macro ToggleSection(section_name) %} -
+{% macro ToggleSection(section_name, classes) %} +
{{ caller() }}
{% endmacro %} diff --git a/templates/fragments/applications/edit_environments.html b/templates/fragments/applications/edit_environments.html index 05b2ff1d..776731e3 100644 --- a/templates/fragments/applications/edit_environments.html +++ b/templates/fragments/applications/edit_environments.html @@ -1,11 +1,12 @@ {% from "components/delete_confirmation.html" import DeleteConfirmation %} {% from "components/icon.html" import Icon %} {% from "components/modal.html" import Modal %} +{% from "components/options_input.html" import OptionsInput %} {% from "components/save_button.html" import SaveButton %} {% from "components/text_input.html" import TextInput %} {% from "components/toggle_list.html" import ToggleButton, ToggleSection %} -{% macro RolePanel(users=[], role='no_access') %} +{% macro RolePanel(users=[], role='no_access', members_form=[]) %} {% if role == 'no_access' %} {% set role = 'Unassigned (No Access)' %} {% set unassigned = True %} @@ -15,8 +16,31 @@

{{ role }}

    {% for user in users %} + {% set section_name = "env_member_{}".format(user.user_id) %} +
  • - {{ user.name }}{{ Icon('edit', classes="icon--medium right") }} + {{ user.name }} + + {% set edit_env_members_button %} + {{ Icon('edit', classes="icon--medium") }} + {% endset %} + + {{ + ToggleButton( + open_html=edit_env_members_button, + close_html=edit_env_members_button, + section_name=section_name + ) + }} + + {% call ToggleSection(section_name=section_name, classes="environment-role__user-field") %} + {% for member in members_form %} + {% if member.user_id.data == user.user_id %} + {{ OptionsInput(member.role, label=False) }} + {{ member.user_id() }} + {% endif %} + {% endfor %} + {% endcall %}
    • {% endfor %} @@ -101,11 +125,30 @@
- {% call ToggleSection(section_name="members") %} + {% call ToggleSection(section_name="members", classes="environment-roles") %}
Need to add someone new to the team? Jump to Team Settings
- {% for role, members in members_by_role.items() %} - {{ RolePanel(users=members, role=role) }} - {% endfor %} + + {% set members_form = env['members_form'] %} +
+ {{ members_form.csrf_token }} + {% for role, members in members_by_role.items() %} + {{ RolePanel(users=members, role=role, members_form=env['members_form']['team_roles']) }} + {% endfor %} + {{ env['members_form'].env_id() }} +
+ {{ + SaveButton( + text=("portfolios.applications.update_button_text" | translate) + ) + }} +
+
+ +
+ + {{ "common.cancel" | translate }} + +
{% endcall %} {% call ToggleSection(section_name="edit") %} diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index 1db1b437..315397a4 100644 --- a/tests/domain/test_environments.py +++ b/tests/domain/test_environments.py @@ -39,7 +39,9 @@ def test_update_env_role_no_access(): user=env_role.user, application=env_role.environment.application ) - assert Environments.update_env_role(env_role.environment, env_role.user, None) + assert Environments.update_env_role( + env_role.environment, env_role.user, "no_access" + ) assert not EnvironmentRoles.get(env_role.user.id, env_role.environment.id) @@ -90,7 +92,7 @@ def test_update_env_roles_by_environment(): { "user_id": env_role_3.user.id, "name": env_role_3.user.full_name, - "role": None, + "role": "no_access", }, ] @@ -125,7 +127,7 @@ def test_update_env_roles_by_member(): {"id": dev.id, "role": CSPRole.NETWORK_ADMIN.value}, {"id": staging.id, "role": CSPRole.BUSINESS_READ.value}, {"id": prod.id, "role": CSPRole.TECHNICAL_READ.value}, - {"id": testing.id, "role": None}, + {"id": testing.id, "role": "no_access"}, ] Environments.update_env_roles_by_member(user, env_roles) diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py index d335e4bb..15f8b3c7 100644 --- a/tests/routes/applications/test_settings.py +++ b/tests/routes/applications/test_settings.py @@ -123,13 +123,13 @@ def test_edit_application_environments_obj(app, client, user_session): assert isinstance(env_obj["members_form"], EnvironmentRolesForm) assert env_obj["members"] == { "no_access": [ - {"name": app_role.user.full_name, "user_id": app_role.user_id} + {"user_id": app_role.user_id, "name": app_role.user.full_name} ], CSPRole.BASIC_ACCESS.value: [ - {"name": env_role1.user.full_name, "user_id": env_role1.user_id} + {"user_id": env_role1.user_id, "name": env_role1.user.full_name} ], CSPRole.NETWORK_ADMIN.value: [ - {"name": env_role2.user.full_name, "user_id": env_role2.user_id} + {"user_id": env_role2.user_id, "name": env_role2.user.full_name} ], CSPRole.BUSINESS_READ.value: [], CSPRole.TECHNICAL_READ.value: [], @@ -236,16 +236,12 @@ def test_update_team_env_roles(client, user_session): form_data = { "env_id": environment.id, "team_roles-0-user_id": env_role_1.user.id, - "team_roles-0-name": env_role_1.user.full_name, "team_roles-0-role": CSPRole.NETWORK_ADMIN.value, "team_roles-1-user_id": env_role_2.user.id, - "team_roles-1-name": env_role_2.user.full_name, "team_roles-1-role": CSPRole.BASIC_ACCESS.value, "team_roles-2-user_id": env_role_3.user.id, - "team_roles-2-name": env_role_3.user.full_name, - "team_roles-2-role": "", + "team_roles-2-role": "no_access", "team_roles-3-user_id": app_role.user.id, - "team_roles-3-name": app_role.user.full_name, "team_roles-3-role": CSPRole.TECHNICAL_READ.value, } diff --git a/tests/test_access.py b/tests/test_access.py index 43d4a5f4..84f1d167 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -16,7 +16,7 @@ from tests.factories import ( ApplicationRoleFactory, EnvironmentFactory, EnvironmentRoleFactory, - InvitationFactory, + PortfolioInvitationFactory, PortfolioFactory, PortfolioRoleFactory, TaskOrderFactory,