diff --git a/atst/domain/environments.py b/atst/domain/environments.py index 50484bad..2bb0e60f 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 == "no_access": + if new_role == None: updated = EnvironmentRoles.delete(user.id, environment.id) else: env_role = EnvironmentRoles.get(user.id, environment.id) @@ -113,6 +113,27 @@ class Environments(object): environment=environment, user=member, new_role=new_role ) + @classmethod + def get_members_by_role(cls, env, role): + env_roles = ( + db.session.query(EnvironmentRole) + .filter(EnvironmentRole.environment_id == env.id) + .filter(EnvironmentRole.role == role) + .all() + ) + + members_list = [] + for env_role in env_roles: + members_list.append( + { + "user_id": env_role.user_id, + "user_name": env_role.user.full_name, + "role": role, + } + ) + + return members_list + @classmethod def revoke_access(cls, environment, target_user): EnvironmentRoles.delete(environment.id, target_user.id) diff --git a/atst/forms/app_settings.py b/atst/forms/app_settings.py index c6320371..094c1fb6 100644 --- a/atst/forms/app_settings.py +++ b/atst/forms/app_settings.py @@ -1,21 +1,33 @@ from flask_wtf import FlaskForm -from wtforms.fields import FieldList, FormField, HiddenField, RadioField +from wtforms.fields import FieldList, FormField, HiddenField, RadioField, StringField from .forms import BaseForm from .data import ENV_ROLES -class EnvMemberRoleForm(FlaskForm): +class MemberForm(FlaskForm): user_id = HiddenField() + user_name = StringField() role = RadioField(choices=ENV_ROLES, default="no_access") @property def data(self): _data = super().data - _data.pop("csrf_token", None) + for field in _data: + if field == "role" and _data[field] == "no_access": + _data[field] = None return _data -class EnvironmentRolesForm(BaseForm): - team_roles = FieldList(FormField(EnvMemberRoleForm)) +class RoleForm(FlaskForm): + role = HiddenField() + members = FieldList(FormField(MemberForm)) + + +class EnvironmentRolesForm(FlaskForm): + team_roles = FieldList(FormField(RoleForm)) env_id = HiddenField() + + +class AppEnvRolesForm(BaseForm): + envs = FieldList(FormField(EnvironmentRolesForm)) diff --git a/atst/models/environment.py b/atst/models/environment.py index 410d9e91..1817517b 100644 --- a/atst/models/environment.py +++ b/atst/models/environment.py @@ -21,7 +21,7 @@ class Environment( @property def users(self): - return [r.user for r in self.roles] + return {r.user for r in self.roles} @property def num_users(self): diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index 283dfce4..f363487e 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -1,10 +1,9 @@ from flask import redirect, render_template, request as http_request, url_for from . import applications_bp -from atst.domain.environment_roles import EnvironmentRoles from atst.domain.environments import Environments from atst.domain.applications import Applications -from atst.forms.app_settings import EnvironmentRolesForm +from atst.forms.app_settings import AppEnvRolesForm 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 @@ -20,8 +19,8 @@ def get_environments_obj_for_app(application): "id": env.id, "name": env.name, "edit_form": EditEnvironmentForm(obj=env), - "members_form": EnvironmentRolesForm(data=data_for_env_members_form(env)), - "members": sort_env_users_by_role(env), + "member_count": len(env.users), + "members": [user.full_name for user in env.users], } environments_obj.append(env_data) @@ -29,33 +28,34 @@ def get_environments_obj_for_app(application): def sort_env_users_by_role(env): - users_dict = {"no_access": []} + users_list = [] + no_access_users = env.application.users - env.users + no_access_list = [ + {"user_id": user.id, "user_name": user.full_name, "role": "no_access"} + for user in no_access_users + ] + users_list.append({"role": "no_access", "members": no_access_list}) + for role in CSPRole: - users_dict[role.value] = [] + users_list.append( + { + "role": role.value, + "members": Environments.get_members_by_role(env, role.value), + } + ) - for user in env.application.users: - if user in env.users: - role = EnvironmentRoles.get(user.id, env.id) - users_dict[role.displayname].append( - {"name": user.full_name, "user_id": user.id} - ) - else: - users_dict["no_access"].append({"name": user.full_name, "user_id": user.id}) - - return users_dict + return users_list -def data_for_env_members_form(environment): - data = {"env_id": environment.id, "team_roles": []} - for user in environment.application.users: - env_role = EnvironmentRoles.get(user.id, environment.id) - - if env_role: - role = env_role.displayname - else: - role = "no_access" - - data["team_roles"].append({"user_id": user.id, "role": role}) +def data_for_app_env_roles_form(application): + data = {"envs": []} + for environment in application.environments: + data["envs"].append( + { + "env_id": environment.id, + "team_roles": sort_env_users_by_role(environment), + } + ) return data @@ -73,12 +73,15 @@ def check_users_are_in_application(user_ids, application): def settings(application_id): application = Applications.get(application_id) form = ApplicationForm(name=application.name, description=application.description) + environments_obj = get_environments_obj_for_app(application=application) + members_form = AppEnvRolesForm(data=data_for_app_env_roles_form(application)) return render_template( "portfolios/applications/settings.html", application=application, form=form, - environments_obj=get_environments_obj_for_app(application=application), + environments_obj=environments_obj, + members_form=members_form, ) @@ -146,13 +149,17 @@ def update(application_id): def update_env_roles(environment_id): environment = Environments.get(environment_id) application = environment.application - form = EnvironmentRolesForm(formdata=http_request.form) + form = AppEnvRolesForm(formdata=http_request.form) if form.validate(): - + env_data = [] try: - user_ids = [user["user_id"] for user in form.data["team_roles"]] - check_users_are_in_application(user_ids, application) + for env in form.envs.data: + if env["env_id"] == str(environment.id): + for role in env["team_roles"]: + user_ids = [user["user_id"] for user in role["members"]] + check_users_are_in_application(user_ids, application) + env_data = env_data + role["members"] except NotFoundError as err: app.logger.warning( "User {} requested environment role change for unauthorized user {} in application {}.".format( @@ -162,9 +169,9 @@ def update_env_roles(environment_id): ) raise (err) - env_data = form.data + Environments.update_env_roles_by_environment( - environment_id=environment_id, team_roles=env_data["team_roles"] + environment_id=environment_id, team_roles=env_data ) flash("application_environment_members_updated") diff --git a/styles/sections/_application_edit.scss b/styles/sections/_application_edit.scss index 846ae3b1..73a32c7e 100644 --- a/styles/sections/_application_edit.scss +++ b/styles/sections/_application_edit.scss @@ -133,7 +133,7 @@ .action-group-cancel__action { position: absolute; - right: ($search-button-width * 2) + ($gap * 2); + right: $search-button-width * 2 + $gap * 2; top: -($gap * 8); } } diff --git a/templates/fragments/applications/edit_environments.html b/templates/fragments/applications/edit_environments.html index 776731e3..2d64a0f2 100644 --- a/templates/fragments/applications/edit_environments.html +++ b/templates/fragments/applications/edit_environments.html @@ -6,7 +6,8 @@ {% from "components/text_input.html" import TextInput %} {% from "components/toggle_list.html" import ToggleButton, ToggleSection %} -{% macro RolePanel(users=[], role='no_access', members_form=[]) %} +{% macro RolePanel(role_form) %} + {% set role = role_form.role.data %} {% if role == 'no_access' %} {% set role = 'Unassigned (No Access)' %} {% set unassigned = True %} @@ -15,11 +16,11 @@

{{ role }}

@@ -74,10 +71,6 @@
{% set open_members_button %} - {{ "common.members" | translate }} ({{ member_count }}) {{ Icon('caret_down') }} + {{ "common.members" | translate }} ({{ env['member_count'] }}) {{ Icon('caret_down') }} {% endset %} {% set close_members_button %} - {{ "common.members" | translate }} ({{ member_count }}) {{ Icon('caret_up') }} + {{ "common.members" | translate }} ({{ env['member_count'] }}) {{ Icon('caret_up') }} {% endset %} {{ @@ -128,13 +121,16 @@ {% call ToggleSection(section_name="members", classes="environment-roles") %}
Need to add someone new to the team? Jump to Team Settings
- {% 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']) }} + {% for env_form in members_form.envs %} + {{ env_form.env_id() }} + {% if env_form.env_id.data == env['id'] %} + {% for role_form in env_form.team_roles %} + {{ RolePanel(role_form) }} + {% endfor %} + {% endif %} {% endfor %} - {{ env['members_form'].env_id() }}
{{ SaveButton( @@ -199,7 +195,7 @@
{{ "portfolios.applications.add_environment" | translate }} - {{ Icon('plus-circle-solid') }} + {{ Icon('plus') }}
diff --git a/templates/fragments/applications/read_only_environments.html b/templates/fragments/applications/read_only_environments.html index 4a91fb3f..90181a2d 100644 --- a/templates/fragments/applications/read_only_environments.html +++ b/templates/fragments/applications/read_only_environments.html @@ -26,11 +26,11 @@ {% set open_members_button %} - {{ "common.members" | translate }} ({{ env['members'] | length }}) {{ Icon('caret_down') }} + {{ "common.members" | translate }} ({{ env['member_count'] }}) {{ Icon('caret_down') }} {% endset %} {% set close_members_button %} - {{ "common.members" | translate }} ({{ env['members'] | length }}) {{ Icon('caret_up') }} + {{ "common.members" | translate }} ({{ env['member_count'] }}) {{ Icon('caret_up') }} {% endset %} {{ @@ -47,7 +47,7 @@
    {% for member in env['members'] %}
  • -
    {{ member.name }}
    +
    {{ member }}
    • {% endfor %}
diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index 315397a4..8eb66620 100644 --- a/tests/domain/test_environments.py +++ b/tests/domain/test_environments.py @@ -39,9 +39,7 @@ 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, "no_access" - ) + assert Environments.update_env_role(env_role.environment, env_role.user, None) assert not EnvironmentRoles.get(env_role.user.id, env_role.environment.id) @@ -92,7 +90,7 @@ def test_update_env_roles_by_environment(): { "user_id": env_role_3.user.id, "name": env_role_3.user.full_name, - "role": "no_access", + "role": None, }, ] @@ -127,7 +125,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": "no_access"}, + {"id": testing.id, "role": None}, ] Environments.update_env_roles_by_member(user, env_roles) @@ -138,6 +136,56 @@ def test_update_env_roles_by_member(): assert not EnvironmentRoles.get(user.id, testing.id) +def test_get_members_by_role(db): + environment = EnvironmentFactory.create() + env_role_1 = EnvironmentRoleFactory.create( + environment=environment, role=CSPRole.BASIC_ACCESS.value + ) + env_role_2 = EnvironmentRoleFactory.create( + environment=environment, role=CSPRole.TECHNICAL_READ.value + ) + env_role_3 = EnvironmentRoleFactory.create( + environment=environment, role=CSPRole.TECHNICAL_READ.value + ) + rando_env = EnvironmentFactory.create() + rando_env_role = EnvironmentRoleFactory.create( + environment=rando_env, role=CSPRole.BASIC_ACCESS.value + ) + + basic_access_members = Environments.get_members_by_role( + environment, CSPRole.BASIC_ACCESS.value + ) + assert basic_access_members == [ + { + "user_id": env_role_1.user_id, + "user_name": env_role_1.user.full_name, + "role": CSPRole.BASIC_ACCESS.value, + } + ] + assert { + "user_id": rando_env_role.user_id, + "user_name": rando_env_role.user.full_name, + "role": CSPRole.BASIC_ACCESS.value, + } not in basic_access_members + assert Environments.get_members_by_role( + environment, CSPRole.TECHNICAL_READ.value + ) == [ + { + "user_id": env_role_2.user_id, + "user_name": env_role_2.user.full_name, + "role": CSPRole.TECHNICAL_READ.value, + }, + { + "user_id": env_role_3.user_id, + "user_name": env_role_3.user.full_name, + "role": CSPRole.TECHNICAL_READ.value, + }, + ] + assert ( + Environments.get_members_by_role(environment, CSPRole.BUSINESS_READ.value) == [] + ) + + def test_get_scoped_environments(db): developer = UserFactory.create() portfolio = PortfolioFactory.create( diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py index 15f8b3c7..9ef8b269 100644 --- a/tests/routes/applications/test_settings.py +++ b/tests/routes/applications/test_settings.py @@ -22,7 +22,7 @@ from atst.domain.exceptions import NotFoundError from atst.models.environment_role import CSPRole from atst.models.portfolio_role import Status as PortfolioRoleStatus from atst.forms.application import EditEnvironmentForm -from atst.forms.app_settings import EnvironmentRolesForm +from atst.forms.app_settings import AppEnvRolesForm from tests.utils import captured_templates @@ -116,23 +116,88 @@ def test_edit_application_environments_obj(app, client, user_session): assert response.status_code == 200 _, context = templates[0] + assert isinstance(context["members_form"], AppEnvRolesForm) env_obj = context["environments_obj"][0] assert env_obj["name"] == env.name assert env_obj["id"] == env.id assert isinstance(env_obj["edit_form"], EditEnvironmentForm) - assert isinstance(env_obj["members_form"], EnvironmentRolesForm) - assert env_obj["members"] == { - "no_access": [ - {"user_id": app_role.user_id, "name": app_role.user.full_name} - ], - CSPRole.BASIC_ACCESS.value: [ - {"user_id": env_role1.user_id, "name": env_role1.user.full_name} - ], - CSPRole.NETWORK_ADMIN.value: [ - {"user_id": env_role2.user_id, "name": env_role2.user.full_name} - ], - CSPRole.BUSINESS_READ.value: [], - CSPRole.TECHNICAL_READ.value: [], + assert ( + env_obj["members"].sort() + == [env_role1.user.full_name, env_role2.user.full_name].sort() + ) + + +def test_data_for_app_env_roles_form(app, client, user_session): + portfolio = PortfolioFactory.create() + application = Applications.create( + portfolio, + "Snazzy Application", + "A new application for me and my friends", + {"env"}, + ) + env = application.environments[0] + app_role = ApplicationRoleFactory.create(application=application) + env_role1 = EnvironmentRoleFactory.create( + environment=env, role=CSPRole.BASIC_ACCESS.value + ) + ApplicationRoleFactory.create(application=application, user=env_role1.user) + env_role2 = EnvironmentRoleFactory.create( + environment=env, role=CSPRole.NETWORK_ADMIN.value + ) + ApplicationRoleFactory.create(application=application, user=env_role2.user) + + user_session(portfolio.owner) + + with captured_templates(app) as templates: + response = app.test_client().get( + url_for("applications.settings", application_id=application.id) + ) + + assert response.status_code == 200 + _, context = templates[0] + + members_form = context["members_form"] + assert isinstance(members_form, AppEnvRolesForm) + assert members_form.data == { + "envs": [ + { + "env_id": env.id, + "team_roles": [ + { + "role": "no_access", + "members": [ + { + "user_id": app_role.user_id, + "user_name": app_role.user.full_name, + "role": None, + } + ], + }, + { + "role": CSPRole.BASIC_ACCESS.value, + "members": [ + { + "user_id": env_role1.user_id, + "user_name": env_role1.user.full_name, + "role": CSPRole.BASIC_ACCESS.value, + } + ], + }, + { + "role": CSPRole.NETWORK_ADMIN.value, + "members": [ + { + "user_id": env_role2.user_id, + "user_name": env_role2.user.full_name, + "role": CSPRole.NETWORK_ADMIN.value, + } + ], + }, + {"role": CSPRole.BUSINESS_READ.value, "members": []}, + {"role": CSPRole.TECHNICAL_READ.value, "members": []}, + ], + } + ] } @@ -234,15 +299,15 @@ def test_update_team_env_roles(client, user_session): app_role = ApplicationRoleFactory.create(application=application) form_data = { - "env_id": environment.id, - "team_roles-0-user_id": env_role_1.user.id, - "team_roles-0-role": CSPRole.NETWORK_ADMIN.value, - "team_roles-1-user_id": env_role_2.user.id, - "team_roles-1-role": CSPRole.BASIC_ACCESS.value, - "team_roles-2-user_id": env_role_3.user.id, - "team_roles-2-role": "no_access", - "team_roles-3-user_id": app_role.user.id, - "team_roles-3-role": CSPRole.TECHNICAL_READ.value, + "envs-0-env_id": environment.id, + "envs-0-team_roles-0-members-0-user_id": app_role.user.id, + "envs-0-team_roles-0-members-0-role": CSPRole.TECHNICAL_READ.value, + "envs-0-team_roles-1-members-0-user_id": env_role_1.user.id, + "envs-0-team_roles-1-members-0-role": CSPRole.NETWORK_ADMIN.value, + "envs-0-team_roles-1-members-1-user_id": env_role_2.user.id, + "envs-0-team_roles-1-members-1-role": CSPRole.BASIC_ACCESS.value, + "envs-0-team_roles-1-members-2-user_id": env_role_3.user.id, + "envs-0-team_roles-1-members-2-role": "no_access", } user_session(application.portfolio.owner)