Refactor application team routes

- pulling out helper functions for constructing forms
- return 400 for form validation errors
- "Yes" appears green on the read only version

atst/forms/team.py

@@ -26,7 +26,7 @@ class PermissionsForm(FlaskForm):
     )
     perms_del_env = SelectField(
         choices=[
-            ("View only", "No"),
+            (PermissionSets.VIEW_APPLICATION, "No"),
             (PermissionSets.DELETE_APPLICATION_ENVIRONMENTS, "Yes"),
         ]
     )
@@ -49,13 +49,6 @@ class MemberForm(FlaskForm):
     environment_roles = FieldList(FormField(EnvironmentForm))
     permission_sets = FormField(PermissionsForm)
 
-    @property
-    def data(self):
-        _data = super().data
-        _data.pop("csrf_token", None)
-
-        return _data
-
 
 class TeamForm(BaseForm):
     members = FieldList(FormField(MemberForm))

atst/routes/applications/team.py

@@ -5,7 +5,6 @@ from . import applications_bp
 from atst.domain.applications import Applications
 from atst.domain.application_roles import ApplicationRoles
 from atst.domain.authz.decorator import user_can_access_decorator as user_can
-from atst.domain.environments import Environments
 from atst.domain.environment_roles import EnvironmentRoles
 from atst.domain.exceptions import AlreadyExistsError
 from atst.domain.permission_sets import PermissionSets
@@ -14,52 +13,30 @@ from atst.forms.team import TeamForm
 from atst.models import Permissions
 from atst.services.invitation import Invitation as InvitationService
 from atst.utils.flash import formatted_flash as flash
-from atst.utils.localization import translate
 
 
-def permission_str(member, edit_perm_set):
+def get_form_permission_value(member, edit_perm_set):
     if member.has_permission_set(edit_perm_set):
-        return translate("portfolios.members.permissions.edit_access")
+        return edit_perm_set
     else:
-        return translate("portfolios.members.permissions.view_only")
+        return PermissionSets.VIEW_APPLICATION
 
 
-@applications_bp.route("/applications/<application_id>/team")
+def get_team_form(application):
-@user_can(Permissions.VIEW_APPLICATION, message="view portfolio applications")
-def team(application_id):
-    application = Applications.get(resource_id=application_id)
-
-    environment_users = {}
     team_data = []
     for member in application.members:
         user_id = member.user.id
         user_name = member.user.full_name
-        environment_users[user_id] = {
+        permission_sets = {
-            "permissions": {
+            "perms_team_mgmt": get_form_permission_value(
-                "team_management": permission_str(
                 member, PermissionSets.EDIT_APPLICATION_TEAM
             ),
-                "environment_management": permission_str(
+            "perms_env_mgmt": get_form_permission_value(
                 member, PermissionSets.EDIT_APPLICATION_ENVIRONMENTS
             ),
-                "delete_access": permission_str(
+            "perms_del_env": get_form_permission_value(
                 member, PermissionSets.DELETE_APPLICATION_ENVIRONMENTS
             ),
-            },
-            "environments": Environments.for_user(
-                user=member.user, application=application
-            ),
-        }
-        permission_sets = {
-            "perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM
-            if member.has_permission_set(PermissionSets.EDIT_APPLICATION_TEAM)
-            else "View only",
-            "perms_env_mgmt": PermissionSets.EDIT_APPLICATION_ENVIRONMENTS
-            if member.has_permission_set(PermissionSets.EDIT_APPLICATION_ENVIRONMENTS)
-            else "View only",
-            "perms_del_env": PermissionSets.DELETE_APPLICATION_ENVIRONMENTS
-            if member.has_permission_set(PermissionSets.DELETE_APPLICATION_ENVIRONMENTS)
-            else "View only",
         }
         roles = EnvironmentRoles.get_for_application_and_user(
             member.user.id, application.id
@@ -81,22 +58,37 @@ def team(application_id):
             }
         )
 
+    return TeamForm(data={"members": team_data})
+
+
+def get_new_member_form(application):
     env_roles = [
         {"environment_id": e.id, "environment_name": e.name}
         for e in application.environments
     ]
-    team_form = TeamForm(data={"members": team_data})
+
-    new_member_form = NewMemberForm(data={"environment_roles": env_roles})
+    return NewMemberForm(data={"environment_roles": env_roles})
+
+
+def render_team_page(application):
+    team_form = get_team_form(application)
+    new_member_form = get_new_member_form(application)
 
     return render_template(
         "portfolios/applications/team.html",
         application=application,
-        environment_users=environment_users,
         team_form=team_form,
         new_member_form=new_member_form,
     )
 
 
+@applications_bp.route("/applications/<application_id>/team")
+@user_can(Permissions.VIEW_APPLICATION, message="view portfolio applications")
+def team(application_id):
+    application = Applications.get(resource_id=application_id)
+    return render_team_page(application)
+
+
 @applications_bp.route("/application/<application_id>/team", methods=["POST"])
 @user_can(Permissions.EDIT_APPLICATION_MEMBER, message="update application member")
 def update_team(application_id):
@@ -107,7 +99,9 @@ def update_team(application_id):
         for member in form.members:
             app_role = ApplicationRoles.get(member.data["user_id"], application.id)
             new_perms = [
-                perm for perm in member.data["permission_sets"] if perm != "View only"
+                perm
+                for perm in member.data["permission_sets"]
+                if perm != PermissionSets.VIEW_APPLICATION
             ]
             ApplicationRoles.update_permission_sets(app_role, new_perms)
         flash("updated_application_members_permissions")
@@ -120,6 +114,8 @@ def update_team(application_id):
                 _anchor="application-members",
             )
         )
+    else:
+        return (render_team_page(application), 400)
 
 
 @applications_bp.route("/application/<application_id>/members/new", methods=["POST"])

templates/fragments/applications/read_only_team.html

@@ -1,26 +1,25 @@
-{% for member in application.members %}
+{% for member in team_form.members %}
-  {% set user = member.user %}
+  {% set user_permissions = [member.permission_sets.perms_team_mgmt, member.permission_sets.perms_env_mgmt, member.permission_sets.perms_del_env] %}
-  {% set user_info = environment_users[user.id] %}
-  {% set user_permissions = user_info["permissions"] %}
 
   {% macro PermissionField(value) %}
-    <div class="col col--grow user-permission{% if "Edit" in value %} green{% endif %}">{{ value }}</div>
+    <div class="col col--grow user-permission{% if "Edit" in value or "Yes" in value %} green{% endif %}">{{ value }}</div>
   {% endmacro %}
 
   <toggler inline-template>
     <li class="accordion-table__item">
       <div class="accordion-table__item-content row">
-        <div class="col col--grow">{{ user.full_name }}</div>
+        <div class="col col--grow">{{ member.user_name.data }}</div>
-        {{ PermissionField(user_permissions["delete_access"]) }}
+        {% for permission in user_permissions %}
-        {{ PermissionField(user_permissions["environment_management"]) }}
+          {% set perm = dict(permission.choices).get(permission.data) %}
-        {{ PermissionField(user_permissions["team_management"]) }}
+          {{ PermissionField(perm) }}
+        {% endfor %}
         <div class="col col--grow icon-link icon-link--large accordion-table__item__toggler">
           {% set open_html %}
-          {{ "portfolios.applications.team_settings.environments" | translate }} ({{ user_info['environments'] | length }}) {{ Icon('caret_down') }}
+          {{ "portfolios.applications.team_settings.environments" | translate }} ({{ member.environment_roles | length }}) {{ Icon('caret_down') }}
           {% endset %}
 
           {% set close_html %}
-            {{ "portfolios.applications.team_settings.environments" | translate }} ({{ user_info['environments'] | length }}) {{ Icon('caret_up') }}
+            {{ "portfolios.applications.team_settings.environments" | translate }} ({{ member.environment_roles | length }}) {{ Icon('caret_up') }}
           {% endset %}
 
           {{
@@ -34,9 +33,9 @@
       </div>
       {% call ToggleSection(section_name="environments") %}
         <ul>
-          {% for environment in user_info["environments"] %}
+          {% for environment in member.environment_roles %}
             <li class="accordion-table__item__expanded">
-              {{ environment.name }}
+              {{ environment.environment_name.data }}
             </li>
           {% endfor %}
         </ul>

tests/forms/test_team.py

@@ -9,7 +9,7 @@ def test_permissions_form_permission_sets():
     form_data = {
         "perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM,
         "perms_env_mgmt": PermissionSets.VIEW_APPLICATION,
-        "perms_del_env": "View only",
+        "perms_del_env": PermissionSets.VIEW_APPLICATION,
     }
     form = PermissionsForm(data=form_data)
 
@@ -17,7 +17,7 @@ def test_permissions_form_permission_sets():
     assert form.data == [
         PermissionSets.EDIT_APPLICATION_TEAM,
         PermissionSets.VIEW_APPLICATION,
-        "View only",
+        PermissionSets.VIEW_APPLICATION,
     ]
 
 
@@ -25,7 +25,7 @@ def test_permissions_form_invalid():
     form_data = {
         "perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM,
         "perms_env_mgmt": "not a real choice",
-        "perms_del_env": "View only",
+        "perms_del_env": PermissionSets.VIEW_APPLICATION,
     }
     form = PermissionsForm(data=form_data)
     assert not form.validate()

tests/routes/applications/test_team.py

@@ -1,3 +1,4 @@
+import pytest
 from flask import url_for
 
 from atst.domain.permission_sets import PermissionSets
@@ -62,6 +63,7 @@ def test_update_team_with_bad_permission_sets(client, user_session):
             "members-0-permission_sets-perms_env_mgmt": "some random string",
         },
     )
+    assert response.status_code == 400
     assert app_user.permission_sets == permission_sets