From 644acc68fadc4bdfc2f903a2af8977dabf905964 Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 7 May 2019 11:47:27 -0400 Subject: [PATCH] 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 | 9 +- atst/routes/applications/team.py | 90 +++++++++---------- .../applications/read_only_team.html | 25 +++--- tests/forms/test_team.py | 6 +- tests/routes/applications/test_team.py | 2 + 5 files changed, 61 insertions(+), 71 deletions(-) diff --git a/atst/forms/team.py b/atst/forms/team.py index 5b12a7cd..51e9c883 100644 --- a/atst/forms/team.py +++ b/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)) diff --git a/atst/routes/applications/team.py b/atst/routes/applications/team.py index cfec59a9..3b823f7a 100644 --- a/atst/routes/applications/team.py +++ b/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//team") -@user_can(Permissions.VIEW_APPLICATION, message="view portfolio applications") -def team(application_id): - application = Applications.get(resource_id=application_id) - - environment_users = {} +def get_team_form(application): team_data = [] for member in application.members: user_id = member.user.id user_name = member.user.full_name - environment_users[user_id] = { - "permissions": { - "team_management": permission_str( - member, PermissionSets.EDIT_APPLICATION_TEAM - ), - "environment_management": permission_str( - member, PermissionSets.EDIT_APPLICATION_ENVIRONMENTS - ), - "delete_access": permission_str( - 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", + "perms_team_mgmt": get_form_permission_value( + member, PermissionSets.EDIT_APPLICATION_TEAM + ), + "perms_env_mgmt": get_form_permission_value( + member, PermissionSets.EDIT_APPLICATION_ENVIRONMENTS + ), + "perms_del_env": get_form_permission_value( + member, PermissionSets.DELETE_APPLICATION_ENVIRONMENTS + ), } 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//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//team", methods=["POST"]) @user_can(Permissions.EDIT_APPLICATION_MEMBER, message="update application member") def update_team(application_id): @@ -107,19 +99,23 @@ 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") - return redirect( - url_for( - "applications.team", - application_id=application_id, - fragment="application-members", - _anchor="application-members", + return redirect( + url_for( + "applications.team", + application_id=application_id, + fragment="application-members", + _anchor="application-members", + ) ) - ) + else: + return (render_team_page(application), 400) @applications_bp.route("/application//members/new", methods=["POST"]) diff --git a/templates/fragments/applications/read_only_team.html b/templates/fragments/applications/read_only_team.html index 2adf1694..cb0e3be1 100644 --- a/templates/fragments/applications/read_only_team.html +++ b/templates/fragments/applications/read_only_team.html @@ -1,26 +1,25 @@ -{% for member in application.members %} - {% set user = member.user %} - {% set user_info = environment_users[user.id] %} - {% set user_permissions = user_info["permissions"] %} +{% for member in team_form.members %} + {% set user_permissions = [member.permission_sets.perms_team_mgmt, member.permission_sets.perms_env_mgmt, member.permission_sets.perms_del_env] %} {% macro PermissionField(value) %} -
{{ value }}
+
{{ value }}
{% endmacro %}
  • -
    {{ user.full_name }}
    - {{ PermissionField(user_permissions["delete_access"]) }} - {{ PermissionField(user_permissions["environment_management"]) }} - {{ PermissionField(user_permissions["team_management"]) }} +
    {{ member.user_name.data }}
    + {% for permission in user_permissions %} + {% set perm = dict(permission.choices).get(permission.data) %} + {{ PermissionField(perm) }} + {% endfor %}
    {% 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 @@
    {% call ToggleSection(section_name="environments") %}
      - {% for environment in user_info["environments"] %} + {% for environment in member.environment_roles %}
    • - {{ environment.name }} + {{ environment.environment_name.data }}
      • {% endfor %}
    diff --git a/tests/forms/test_team.py b/tests/forms/test_team.py index 895c5e6a..c5990698 100644 --- a/tests/forms/test_team.py +++ b/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() diff --git a/tests/routes/applications/test_team.py b/tests/routes/applications/test_team.py index e5c0d54f..108c4f25 100644 --- a/tests/routes/applications/test_team.py +++ b/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