Update form to nest all envs in one form

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)

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))

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):

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] = []
-
-    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}
+        users_list.append(
+            {
+                "role": role.value,
+                "members": Environments.get_members_by_role(env, role.value),
+            }
         )
-        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"]]
+            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")

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);
     }
   }

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 @@
   <div class='environment-role'>
     <h4>{{ role }}</h4>
       <ul class='environment-role__users'>
-        {% for user in users %}
-          {% set section_name = "env_member_{}".format(user.user_id) %}
+        {% for member in role_form.members %}
+          {% set section_name = "env_member_{}".format(member.user_id) %}
 
           <li class="environment-role__user {{ 'unassigned' if unassigned }}">
-            {{ user.name }}
+            {{ member.user_name.data }}
             <span class="icon-link right">
               {% set edit_env_members_button %}
                 {{ Icon('edit', classes="icon--medium") }}
@@ -34,17 +35,13 @@
               }}
             </span>
             {% 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 %}
           </li>
         {% endfor %}
 
-        {% if users == [] %}
+        {% if role_form.members.data == [] %}
           <div class='environment-role__no-user'>Currently no members are in this role</div>
         {% endif %}
       </ul>
@@ -74,10 +71,6 @@
 
     <ul class="accordion-table__items">
       {% for env in environments_obj %}
-        {% set edit_form = env['edit_form'] %}
-        {% set member_count = env['members_form'].data['team_roles'] | length %}
-        {% set members_by_role = env['members'] %}
-        {% set unassigned = members_by_role['no_access'] %}
         {% set delete_environment_modal_id = "delete_modal_environment{}".format(env['id']) %}
 
         <toggler inline-template {% if edit_form.errors %}initial-selected-section="edit"{% endif %}>
@@ -108,11 +101,11 @@
               </div>
               <div class="col col--grow icon-link icon-link--large accordion-table__item__toggler">
                 {% 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") %}
                 <div class='app-team-settings-link'>Need to add someone new to the team? <a href='{{ url_for("applications.team", application_id=application.id) }}'>Jump to Team Settings</a></div>
                 <toggler inline-template>
-                  {% set members_form = env['members_form'] %}
                   <form action="{{ url_for('applications.update_env_roles', environment_id=env['id']) }}" method="post">
                     {{ 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() }}
                     <div class='action-group'>
                       {{
                         SaveButton(
@@ -199,7 +195,7 @@
   <div class="action-group">
     <a class='icon-link'>
       {{ "portfolios.applications.add_environment" | translate }}
-      {{ Icon('plus-circle-solid') }}
+      {{ Icon('plus') }}
     </a>
   </div>
 </div>

templates/fragments/applications/read_only_environments.html

@@ -26,11 +26,11 @@
 
               <span class="icon-link icon-link--large accordion-table__item__toggler">
                 {% 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 @@
               <ul>
                 {% for member in env['members'] %}
                 <li class="accordion-table__item__expanded">
-                  <div class="accordion-table__item__expanded_first">{{ member.name }}</div>
+                  <div class="accordion-table__item__expanded_first">{{ member }}</div>
                 </li>
                 {% endfor %}
               </ul>

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(

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}
+        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,
+                                }
                             ],
-            CSPRole.BASIC_ACCESS.value: [
-                {"user_id": env_role1.user_id, "name": env_role1.user.full_name}
+                        },
+                        {
+                            "role": CSPRole.BASIC_ACCESS.value,
+                            "members": [
+                                {
+                                    "user_id": env_role1.user_id,
+                                    "user_name": env_role1.user.full_name,
+                                    "role": CSPRole.BASIC_ACCESS.value,
+                                }
                             ],
-            CSPRole.NETWORK_ADMIN.value: [
-                {"user_id": env_role2.user_id, "name": env_role2.user.full_name}
+                        },
+                        {
+                            "role": CSPRole.NETWORK_ADMIN.value,
+                            "members": [
+                                {
+                                    "user_id": env_role2.user_id,
+                                    "user_name": env_role2.user.full_name,
+                                    "role": CSPRole.NETWORK_ADMIN.value,
+                                }
                             ],
-            CSPRole.BUSINESS_READ.value: [],
-            CSPRole.TECHNICAL_READ.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)