From 926f89d97585f806006b9ca0d175f4316090904c Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 1 May 2019 13:33:04 -0400 Subject: [PATCH 01/14] Form for application team member table. Nested form for each member, with child forms for environment roles and permissions. --- atst/forms/team.py | 51 +++++++++++++++++++++++++++ tests/forms/__init__.py | 0 tests/forms/test_team.py | 26 ++++++++++++++ tests/routes/applications/__init__.py | 0 4 files changed, 77 insertions(+) create mode 100644 atst/forms/team.py create mode 100644 tests/forms/__init__.py create mode 100644 tests/forms/test_team.py create mode 100644 tests/routes/applications/__init__.py diff --git a/atst/forms/team.py b/atst/forms/team.py new file mode 100644 index 00000000..ccc49e35 --- /dev/null +++ b/atst/forms/team.py @@ -0,0 +1,51 @@ +from flask_wtf import FlaskForm +from wtforms.fields import FormField, FieldList, HiddenField + +from .application_member import EnvironmentForm +from .forms import BaseForm +from atst.forms.fields import SelectField +from atst.domain.permission_sets import PermissionSets +from atst.utils.localization import translate + + +class PermissionsForm(FlaskForm): + perms_env_mgmt = SelectField( + translate("portfolios.applications.members.new.manage_envs"), + choices=[ + (None, "View only"), + (PermissionSets.EDIT_APPLICATION_ENVIRONMENTS, "Edit access"), + ], + filters=[BaseForm.remove_empty_string], + ) + perms_team_mgmt = SelectField( + translate("portfolios.applications.members.new.manage_team"), + choices=[ + (None, "View only"), + (PermissionSets.EDIT_APPLICATION_TEAM, "Edit access"), + ], + filters=[BaseForm.remove_empty_string], + ) + perms_del_env = SelectField( + choices=[(None, "No"), (PermissionSets.DELETE_APPLICATION_ENVIRONMENTS, "Yes")], + filters=[BaseForm.remove_empty_string], + ) + + @property + def data(self): + _data = super().data + permission_sets = [] + for field in _data: + if _data[field] is not None: + permission_sets.append(_data[field]) + + return permission_sets + + +class MemberForm(FlaskForm): + user_id = HiddenField() + environment_roles = FieldList(FormField(EnvironmentForm)) + permission_sets = FormField(PermissionsForm) + + +class TeamForm(BaseForm): + members = FieldList(FormField(MemberForm)) diff --git a/tests/forms/__init__.py b/tests/forms/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/forms/test_team.py b/tests/forms/test_team.py new file mode 100644 index 00000000..671a44ee --- /dev/null +++ b/tests/forms/test_team.py @@ -0,0 +1,26 @@ +from wtforms.validators import ValidationError +import pytest + +from atst.domain.permission_sets import PermissionSets +from atst.forms.team import * + + +def test_permissions_form_permission_sets(): + form_data = { + "perms_env_mgmt": "", + "perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM, + "perms_del_env": "", + } + form = PermissionsForm(data=form_data) + assert form.validate() + assert form.data == [PermissionSets.EDIT_APPLICATION_TEAM] + + +def test_permissions_form_invalid(): + form_data = { + "perms_env_mgmt": "not a real choice", + "perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM, + "perms_del_env": "", + } + form = PermissionsForm(data=form_data) + assert not form.validate() diff --git a/tests/routes/applications/__init__.py b/tests/routes/applications/__init__.py new file mode 100644 index 00000000..e69de29b From 0da0f6a0ae39d8f80b33b1ed5c987140768766b0 Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 1 May 2019 15:30:42 -0400 Subject: [PATCH 02/14] WIP: use team form for application team page --- atst/domain/environment_roles.py | 14 ++++- atst/forms/team.py | 3 +- atst/models/application.py | 1 + atst/routes/applications/team.py | 43 +++++++++++++-- .../add_new_application_member.html | 20 +++---- .../applications/read_only_team.html | 46 ++++++++++++++++ templates/portfolios/applications/team.html | 52 +++---------------- tests/domain/test_environment_roles.py | 16 ++++++ 8 files changed, 134 insertions(+), 61 deletions(-) create mode 100644 templates/fragments/applications/read_only_team.html create mode 100644 tests/domain/test_environment_roles.py diff --git a/atst/domain/environment_roles.py b/atst/domain/environment_roles.py index 99728467..bb8fdb85 100644 --- a/atst/domain/environment_roles.py +++ b/atst/domain/environment_roles.py @@ -1,7 +1,7 @@ from flask import current_app as app from atst.database import db -from atst.models import EnvironmentRole +from atst.models import EnvironmentRole, Application, Environment class EnvironmentRoles(object): @@ -35,3 +35,15 @@ class EnvironmentRoles(object): return True else: return False + + @classmethod + def get_for_application_and_user(cls, user_id, application_id): + return ( + db.session.query(EnvironmentRole) + .join(Environment) + .join(Application, Environment.application_id == Application.id) + .filter(EnvironmentRole.user_id == user_id) + .filter(Application.id == application_id) + .filter(EnvironmentRole.environment_id == Environment.id) + .all() + ) diff --git a/atst/forms/team.py b/atst/forms/team.py index ccc49e35..f9661f72 100644 --- a/atst/forms/team.py +++ b/atst/forms/team.py @@ -1,5 +1,5 @@ from flask_wtf import FlaskForm -from wtforms.fields import FormField, FieldList, HiddenField +from wtforms.fields import FormField, FieldList, HiddenField, StringField from .application_member import EnvironmentForm from .forms import BaseForm @@ -43,6 +43,7 @@ class PermissionsForm(FlaskForm): class MemberForm(FlaskForm): user_id = HiddenField() + user_name = StringField() environment_roles = FieldList(FormField(EnvironmentForm)) permission_sets = FormField(PermissionsForm) diff --git a/atst/models/application.py b/atst/models/application.py index 9edb5166..ef783ff6 100644 --- a/atst/models/application.py +++ b/atst/models/application.py @@ -28,6 +28,7 @@ class Application( back_populates="application", primaryjoin="and_(Environment.application_id==Application.id, Environment.deleted==False)", ) + # TODO: filter condition on this relationship? roles = relationship("ApplicationRole") @property diff --git a/atst/routes/applications/team.py b/atst/routes/applications/team.py index 57994a3c..8bee43ec 100644 --- a/atst/routes/applications/team.py +++ b/atst/routes/applications/team.py @@ -2,12 +2,14 @@ from flask import render_template, request as http_request, g, url_for, redirect from . import applications_bp -from atst.domain.environments import Environments from atst.domain.applications import Applications +from atst.domain.environments import Environments +from atst.domain.environment_roles import EnvironmentRoles from atst.domain.authz.decorator import user_can_access_decorator as user_can from atst.domain.permission_sets import PermissionSets from atst.domain.exceptions import AlreadyExistsError from atst.forms.application_member import NewForm as NewMemberForm +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 @@ -27,8 +29,10 @@ 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] = { "permissions": { "delete_access": permission_str( @@ -45,18 +49,51 @@ def team(application_id): user=member.user, application=application ), } + permission_sets = { + "perms_env_mgmt": PermissionSets.EDIT_APPLICATION_ENVIRONMENTS + if member.has_permission_set(PermissionSets.EDIT_APPLICATION_ENVIRONMENTS) + else "", + "perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM + if member.has_permission_set(PermissionSets.EDIT_APPLICATION_TEAM) + else "", + "perms_del_env": PermissionSets.DELETE_APPLICATION_ENVIRONMENTS + if member.has_permission_set(PermissionSets.DELETE_APPLICATION_ENVIRONMENTS) + else "", + } + roles = EnvironmentRoles.get_for_application_and_user( + member.user.id, application.id + ) + environment_roles = [ + { + "environment_id": str(role.environment.id), + "environment_name": role.environment.name, + "role": role.role, + } + for role in roles + ] + team_data.append( + { + "user_id": str(user_id), + "user_name": user_name, + "permission_sets": permission_sets, + "environment_roles": environment_roles, + } + ) + + team_form = TeamForm(data={"members": team_data}) env_roles = [ {"environment_id": e.id, "environment_name": e.name} for e in application.environments ] - member_form = NewMemberForm(data={"environment_roles": env_roles}) + new_member_form = NewMemberForm(data={"environment_roles": env_roles}) return render_template( "portfolios/applications/team.html", application=application, environment_users=environment_users, - member_form=member_form, + team_form=team_form, + new_member_form=new_member_form, ) diff --git a/templates/fragments/applications/add_new_application_member.html b/templates/fragments/applications/add_new_application_member.html index 99f8f99a..3294b21c 100644 --- a/templates/fragments/applications/add_new_application_member.html +++ b/templates/fragments/applications/add_new_application_member.html @@ -9,23 +9,23 @@
- {{ TextInput(member_form.user_data.first_name, validation='requiredField') }} + {{ TextInput(new_member_form.user_data.first_name, validation='requiredField') }}
- {{ TextInput(member_form.user_data.last_name, validation='requiredField') }} + {{ TextInput(new_member_form.user_data.last_name, validation='requiredField') }}
- {{ TextInput(member_form.user_data.email, validation='email') }} + {{ TextInput(new_member_form.user_data.email, validation='email') }}
- {{ TextInput(member_form.user_data.phone_number, validation='usPhone', optional=True) }} + {{ TextInput(new_member_form.user_data.phone_number, validation='usPhone', optional=True) }}
- {{ TextInput(member_form.user_data.dod_id, validation='dodId') }} + {{ TextInput(new_member_form.user_data.dod_id, validation='dodId') }}
@@ -61,7 +61,7 @@
- {% for environment_data in member_form.environment_roles %} + {% for environment_data in new_member_form.environment_roles %} @@ -86,9 +86,9 @@ {% endfor %}

{{ "portfolios.applications.members.new.manage_perms" | translate({"application_name": application.name}) }}

- {{ CheckboxInput(member_form.permission_sets.perms_team_mgmt, classes="input__inline-fields") }} - {% call CheckboxInput(member_form.permission_sets.perms_env_mgmt, classes="input__inline-fields") %} - {% set field=member_form.permission_sets.perms_del_env %} + {{ CheckboxInput(new_member_form.permission_sets.perms_team_mgmt, classes="input__inline-fields") }} + {% call CheckboxInput(new_member_form.permission_sets.perms_env_mgmt, classes="input__inline-fields") %} + {% set field=new_member_form.permission_sets.perms_del_env %} {{ value }} + {% endmacro %} + + +
  • +
    +
    {{ user.full_name }}
    + {{ PermissionField(user_permissions["delete_access"]) }} + {{ PermissionField(user_permissions["environment_management"]) }} + {{ PermissionField(user_permissions["team_management"]) }} +
    + {% set open_html %} + {{ "portfolios.applications.team_settings.environments" | translate }} ({{ user_info['environments'] | length }}) {{ Icon('caret_down') }} + {% endset %} + + {% set close_html %} + {{ "portfolios.applications.team_settings.environments" | translate }} ({{ user_info['environments'] | length }}) {{ Icon('caret_up') }} + {% endset %} + + {{ + ToggleButton( + open_html=open_html, + close_html=close_html, + section_name="environments" + ) + }} +
    +
    + {% call ToggleSection(section_name="environments") %} +
      + {% for environment in user_info["environments"] %} +
    • + {{ environment.name }} +
      • + {% endfor %} +
    + {% endcall %} +
    • +     +{% endfor %} diff --git a/templates/portfolios/applications/team.html b/templates/portfolios/applications/team.html index e02a5459..c5e94b0a 100644 --- a/templates/portfolios/applications/team.html +++ b/templates/portfolios/applications/team.html @@ -61,52 +61,12 @@
      - {% for member in application.members %} - {% set user = member.user %} - {% set user_info = environment_users[user.id] %} - {% set user_permissions = user_info["permissions"] %} - - {% macro PermissionField(value) %} -
      {{ value }}
      - {% endmacro %} - - -
    • -
      -
      {{ user.full_name }}
      - {{ PermissionField(user_permissions["delete_access"]) }} - {{ PermissionField(user_permissions["environment_management"]) }} - {{ PermissionField(user_permissions["team_management"]) }} -
      - {% set open_html %} - {{ "portfolios.applications.team_settings.environments" | translate }} ({{ user_info['environments'] | length }}) {{ Icon('caret_down') }} - {% endset %} - - {% set close_html %} - {{ "portfolios.applications.team_settings.environments" | translate }} ({{ user_info['environments'] | length }}) {{ Icon('caret_up') }} - {% endset %} - - {{ - ToggleButton( - open_html=open_html, - close_html=close_html, - section_name="environments" - ) - }} -
      -
      - {% call ToggleSection(section_name="environments") %} -
        - {% for environment in user_info["environments"] %} -
      • - {{ environment.name }} -
        • - {% endfor %} -
      - {% endcall %} -
      • -       - {% endfor %} + {% if user_can(permissions.EDIT_APPLICATION_MEMBER) %} + {# edit version goes here #} + {% include "fragments/applications/read_only_team.html" %} + {% elif user_can(permissions.VIEW_APPLICATION_MEMBER) %} + {% include "fragments/applications/read_only_team.html" %} + {% endif %}
    diff --git a/tests/domain/test_environment_roles.py b/tests/domain/test_environment_roles.py new file mode 100644 index 00000000..de42d70e --- /dev/null +++ b/tests/domain/test_environment_roles.py @@ -0,0 +1,16 @@ +from atst.domain.environment_roles import EnvironmentRoles + +from tests.factories import * + + +def test_get_for_application_and_user(): + user = UserFactory.create() + application = ApplicationFactory.create() + env1 = EnvironmentFactory.create(application=application) + EnvironmentFactory.create(application=application) + EnvironmentRoleFactory.create(user=user, environment=env1) + + roles = EnvironmentRoles.get_for_application_and_user(user.id, application.id) + assert len(roles) == 1 + assert roles[0].environment == env1 + assert roles[0].user == user From c1ae5129bc3f015989c2ddd65cda95035a3afb24 Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 1 May 2019 16:20:53 -0400 Subject: [PATCH 03/14] Add edit template with dropdown selectors --- js/components/toggler.js | 2 + styles/components/_accordion_table.scss | 9 ++++ .../fragments/applications/edit_team.html | 43 ++++++++++++++++ .../applications/read_only_team.html | 50 +++++++++---------- templates/portfolios/applications/team.html | 3 +- 5 files changed, 80 insertions(+), 27 deletions(-) create mode 100644 templates/fragments/applications/edit_team.html diff --git a/js/components/toggler.js b/js/components/toggler.js index 39eca87f..583dec53 100644 --- a/js/components/toggler.js +++ b/js/components/toggler.js @@ -1,5 +1,6 @@ import FormMixin from '../mixins/form' import textinput from './text_input' +import optionsinput from './options_input' export default { name: 'toggler', @@ -8,6 +9,7 @@ export default { components: { textinput, + optionsinput, }, data: function() { diff --git a/styles/components/_accordion_table.scss b/styles/components/_accordion_table.scss index e9060bc5..28cd08de 100644 --- a/styles/components/_accordion_table.scss +++ b/styles/components/_accordion_table.scss @@ -39,6 +39,15 @@ .accordion-table__item-content { padding: ($gap * 2); + + .usa-input { + margin: 0; + } + + select { + border: none; + font-weight: $font-normal; + } } .accordion-table__items { diff --git a/templates/fragments/applications/edit_team.html b/templates/fragments/applications/edit_team.html new file mode 100644 index 00000000..d034ec6a --- /dev/null +++ b/templates/fragments/applications/edit_team.html @@ -0,0 +1,43 @@ +{% from "components/options_input.html" import OptionsInput %} + +{% for member_form in team_form.members %} + {% set environment_roles_form = member_form.environment_roles %} + {% set permissions_form = member_form.permission_sets %} + + +
  • +
    +
    {{ member_form.user_name.data }}
    +
    {{ OptionsInput(permissions_form.perms_team_mgmt, label=False) }}
    +
    {{ OptionsInput(permissions_form.perms_env_mgmt, label=False) }}
    +
    {{ OptionsInput(permissions_form.perms_del_env, label=False) }}
    +
    + {% set open_html %} + {{ "portfolios.applications.team_settings.environments" | translate }} ({{ environment_roles_form | length }}) {{ Icon('caret_down') }} + {% endset %} + + {% set close_html %} + {{ "portfolios.applications.team_settings.environments" | translate }} ({{ environment_roles_form | length }}) {{ Icon('caret_up') }} + {% endset %} + + {{ + ToggleButton( + open_html=open_html, + close_html=close_html, + section_name="environments" + ) + }} +
    +
    + {% call ToggleSection(section_name="environments") %} +
      + {% for environment_form in environment_roles_form %} +
    • + {{ environment_form.environment_name.data }} +
      • + {% endfor %} +
    + {% endcall %} +
    • +     +{% endfor %} diff --git a/templates/fragments/applications/read_only_team.html b/templates/fragments/applications/read_only_team.html index 41a793c4..2adf1694 100644 --- a/templates/fragments/applications/read_only_team.html +++ b/templates/fragments/applications/read_only_team.html @@ -3,34 +3,34 @@ {% set user_info = environment_users[user.id] %} {% set user_permissions = user_info["permissions"] %} - {% macro PermissionField(value) %} -
    {{ value }}
    - {% endmacro %} + {% macro PermissionField(value) %} +
    {{ value }}
    + {% endmacro %} - -
  • -
    -
    {{ user.full_name }}
    - {{ PermissionField(user_permissions["delete_access"]) }} - {{ PermissionField(user_permissions["environment_management"]) }} - {{ PermissionField(user_permissions["team_management"]) }} -
    - {% set open_html %} - {{ "portfolios.applications.team_settings.environments" | translate }} ({{ user_info['environments'] | length }}) {{ Icon('caret_down') }} - {% endset %} + +
  • +
    +
    {{ user.full_name }}
    + {{ PermissionField(user_permissions["delete_access"]) }} + {{ PermissionField(user_permissions["environment_management"]) }} + {{ PermissionField(user_permissions["team_management"]) }} +
    + {% set open_html %} + {{ "portfolios.applications.team_settings.environments" | translate }} ({{ user_info['environments'] | length }}) {{ Icon('caret_down') }} + {% endset %} - {% set close_html %} - {{ "portfolios.applications.team_settings.environments" | translate }} ({{ user_info['environments'] | length }}) {{ Icon('caret_up') }} - {% endset %} + {% set close_html %} + {{ "portfolios.applications.team_settings.environments" | translate }} ({{ user_info['environments'] | length }}) {{ Icon('caret_up') }} + {% endset %} - {{ - ToggleButton( - open_html=open_html, - close_html=close_html, - section_name="environments" - ) - }} -
    + {{ + ToggleButton( + open_html=open_html, + close_html=close_html, + section_name="environments" + ) + }} +
    {% call ToggleSection(section_name="environments") %}
      diff --git a/templates/portfolios/applications/team.html b/templates/portfolios/applications/team.html index c5e94b0a..9449ff6d 100644 --- a/templates/portfolios/applications/team.html +++ b/templates/portfolios/applications/team.html @@ -62,8 +62,7 @@
        {% if user_can(permissions.EDIT_APPLICATION_MEMBER) %} - {# edit version goes here #} - {% include "fragments/applications/read_only_team.html" %} + {% include "fragments/applications/edit_team.html" %} {% elif user_can(permissions.VIEW_APPLICATION_MEMBER) %} {% include "fragments/applications/read_only_team.html" %} {% endif %} From 56920888372e2c2bba27b886812542ab498c7cdf Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 1 May 2019 16:37:37 -0400 Subject: [PATCH 04/14] Add Save Button to edit version of team table -adds base form component -Save button does not function properly yet -alphabetizes imports on base-form --- js/components/forms/base_form.js | 24 +++--- templates/portfolios/applications/team.html | 94 +++++++++++---------- 2 files changed, 63 insertions(+), 55 deletions(-) diff --git a/js/components/forms/base_form.js b/js/components/forms/base_form.js index 535a098d..a7259eae 100644 --- a/js/components/forms/base_form.js +++ b/js/components/forms/base_form.js @@ -1,26 +1,28 @@ import ally from 'ally.js' -import FormMixin from '../../mixins/form' -import textinput from '../text_input' -import optionsinput from '../options_input' -import DateSelector from '../date_selector' -import MultiStepModalForm from './multi_step_modal_form' -import multicheckboxinput from '../multi_checkbox_input' import checkboxinput from '../checkbox_input' +import DateSelector from '../date_selector' +import FormMixin from '../../mixins/form' import levelofwarrant from '../levelofwarrant' import Modal from '../../mixins/modal' +import multicheckboxinput from '../multi_checkbox_input' +import MultiStepModalForm from './multi_step_modal_form' +import optionsinput from '../options_input' +import textinput from '../text_input' +import toggler from '../toggler' export default { name: 'base-form', components: { - textinput, - optionsinput, - DateSelector, - MultiStepModalForm, - multicheckboxinput, checkboxinput, + DateSelector, levelofwarrant, Modal, + multicheckboxinput, + MultiStepModalForm, + optionsinput, + textinput, + toggler, }, mixins: [FormMixin], } diff --git a/templates/portfolios/applications/team.html b/templates/portfolios/applications/team.html index 9449ff6d..992ef782 100644 --- a/templates/portfolios/applications/team.html +++ b/templates/portfolios/applications/team.html @@ -2,6 +2,7 @@ {% from "components/empty_state.html" import EmptyState %} {% from "components/icon.html" import Icon %} +{% from 'components/save_button.html' import SaveButton %} {% from "components/toggle_list.html" import ToggleButton, ToggleSection %} {% set secondary_breadcrumb = 'portfolios.applications.team_settings.title' | translate({ "application_name": application.name }) %} @@ -24,60 +25,65 @@
        -
        - {% if g.matchesPath("application-members") %} - {% include "fragments/flash.html" %} - {% endif %} -
        -
        -
        -
        - {{ "portfolios.applications.team_settings.section.title" | translate({ "application_name": application.name }) }} + +
        + {% if g.matchesPath("application-members") %} + {% include "fragments/flash.html" %} + {% endif %} +
        +
        +
        +
        + {{ "portfolios.applications.team_settings.section.title" | translate({ "application_name": application.name }) }} +
        + + {{ Icon('info') }} + {{ "portfolios.admin.settings_info" | translate }} + +
        +
        + +
        +
        +
        + {{ "common.name" | translate }} +
        +
        + {{ "portfolios.applications.team_settings.section.table.delete_access" | translate }} +
        +
        + {{ "portfolios.applications.team_settings.section.table.environment_management" | translate }} +
        +
        + {{ "portfolios.applications.team_settings.section.table.team_management" | translate }} +
        +
        +  
        - - {{ Icon('info') }} - {{ "portfolios.admin.settings_info" | translate }} - +
          + {% if user_can(permissions.EDIT_APPLICATION_MEMBER) %} + {% include "fragments/applications/edit_team.html" %} + {% elif user_can(permissions.VIEW_APPLICATION_MEMBER) %} + {% include "fragments/applications/read_only_team.html" %} + {% endif %} +
        -
        -
        -
        -
        - {{ "common.name" | translate }} -
        -
        - {{ "portfolios.applications.team_settings.section.table.delete_access" | translate }} -
        -
        - {{ "portfolios.applications.team_settings.section.table.environment_management" | translate }} -
        -
        - {{ "portfolios.applications.team_settings.section.table.team_management" | translate }} -
        -
        -   -
        -
        -
          - {% if user_can(permissions.EDIT_APPLICATION_MEMBER) %} - {% include "fragments/applications/edit_team.html" %} - {% elif user_can(permissions.VIEW_APPLICATION_MEMBER) %} - {% include "fragments/applications/read_only_team.html" %} - {% endif %} -
        -
        - -
        +
        + {% if user_can(permissions.EDIT_APPLICATION_MEMBER) %} + {{ SaveButton(text=('common.save' | translate), element="input", form="member-perms") }} + {% endif %} + {% if user_can(permissions.CREATE_APPLICATION_MEMBER) %} {% include "fragments/applications/add_new_application_member.html" %} {% endif %}
        - -
        + +
        +
        {% endif %} {% endblock %} From 416ea1de82bd5f2069b7e9d9add345359ccb074a Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 1 May 2019 17:22:32 -0400 Subject: [PATCH 05/14] Watch for changes on the options input, which is a grandchild of base-form --- js/components/options_input.js | 8 ++++++++ js/components/toggler.js | 2 +- js/mixins/form.js | 3 +-- styles/elements/_tables.scss | 1 + templates/components/options_input.html | 3 ++- templates/fragments/applications/edit_team.html | 6 +++--- templates/portfolios/applications/team.html | 4 ++-- 7 files changed, 18 insertions(+), 9 deletions(-) diff --git a/js/components/options_input.js b/js/components/options_input.js index fa4d8a57..2970b135 100644 --- a/js/components/options_input.js +++ b/js/components/options_input.js @@ -1,8 +1,11 @@ import { emitEvent } from '../lib/emitters' +import FormMixin from '../mixins/form' export default { name: 'optionsinput', + mixins: [FormMixin], + props: { name: String, initialErrors: { @@ -10,6 +13,10 @@ export default { default: () => [], }, initialValue: String, + watch: { + type: Boolean, + default: false, + }, }, data: function() { @@ -27,6 +34,7 @@ export default { emitEvent('field-change', this, { value: e.target.value, name: this.name, + watch: this.watch, }) this.showError = false this.showValid = true diff --git a/js/components/toggler.js b/js/components/toggler.js index 583dec53..f1c34c96 100644 --- a/js/components/toggler.js +++ b/js/components/toggler.js @@ -1,6 +1,6 @@ import FormMixin from '../mixins/form' -import textinput from './text_input' import optionsinput from './options_input' +import textinput from './text_input' export default { name: 'toggler', diff --git a/js/mixins/form.js b/js/mixins/form.js index 497497b1..f8e97f4d 100644 --- a/js/mixins/form.js +++ b/js/mixins/form.js @@ -16,8 +16,7 @@ export default { const { name, valid, parent_uid } = event if (typeof this[name] !== undefined) { this.fields[name] = valid - - if (parent_uid === this._uid) { + if (event['parent_uid'] === this._uid || event['watch']) { this.changed = true } } diff --git a/styles/elements/_tables.scss b/styles/elements/_tables.scss index 5ad64929..b18b1884 100644 --- a/styles/elements/_tables.scss +++ b/styles/elements/_tables.scss @@ -126,6 +126,7 @@ table { @include h4; font-size: $lead-font-size; + justify-content: space-between; flex: 2; } } diff --git a/templates/components/options_input.html b/templates/components/options_input.html index ff8af362..81a1e50c 100644 --- a/templates/components/options_input.html +++ b/templates/components/options_input.html @@ -1,13 +1,14 @@ {% from "components/icon.html" import Icon %} {% from "components/tooltip.html" import Tooltip %} -{% macro OptionsInput(field, tooltip, inline=False, label=True, disabled=False) -%} +{% macro OptionsInput(field, tooltip, inline=False, label=True, disabled=False, watch=False) -%}
        diff --git a/templates/fragments/applications/edit_team.html b/templates/fragments/applications/edit_team.html index d034ec6a..094ccf40 100644 --- a/templates/fragments/applications/edit_team.html +++ b/templates/fragments/applications/edit_team.html @@ -8,9 +8,9 @@
      • {{ member_form.user_name.data }}
        -
        {{ OptionsInput(permissions_form.perms_team_mgmt, label=False) }}
        -
        {{ OptionsInput(permissions_form.perms_env_mgmt, label=False) }}
        -
        {{ OptionsInput(permissions_form.perms_del_env, label=False) }}
        +
        {{ OptionsInput(permissions_form.perms_team_mgmt, label=False, watch=True) }}
        +
        {{ OptionsInput(permissions_form.perms_env_mgmt, label=False, watch=True) }}
        +
        {{ OptionsInput(permissions_form.perms_del_env, label=False, watch=True) }}
        {% set open_html %} {{ "portfolios.applications.team_settings.environments" | translate }} ({{ environment_roles_form | length }}) {{ Icon('caret_down') }} diff --git a/templates/portfolios/applications/team.html b/templates/portfolios/applications/team.html index 992ef782..f6df4f25 100644 --- a/templates/portfolios/applications/team.html +++ b/templates/portfolios/applications/team.html @@ -32,11 +32,11 @@ {% endif %}
        -
        +
        {{ "portfolios.applications.team_settings.section.title" | translate({ "application_name": application.name }) }}
        - + {{ Icon('info') }} {{ "portfolios.admin.settings_info" | translate }} From de41afa935e4d876d32ad9fd2d83e04ce2f5d680 Mon Sep 17 00:00:00 2001 From: Montana Date: Thu, 2 May 2019 13:51:41 -0400 Subject: [PATCH 06/14] Add form to template - do not use None as a dropdown value because it was causing issues - cleans up some comments --- atst/forms/team.py | 17 +++-- atst/routes/applications/team.py | 1 + .../fragments/applications/edit_team.html | 70 ++++++++++--------- templates/portfolios/applications/team.html | 2 +- 4 files changed, 50 insertions(+), 40 deletions(-) diff --git a/atst/forms/team.py b/atst/forms/team.py index f9661f72..fbbdffe5 100644 --- a/atst/forms/team.py +++ b/atst/forms/team.py @@ -12,27 +12,25 @@ class PermissionsForm(FlaskForm): perms_env_mgmt = SelectField( translate("portfolios.applications.members.new.manage_envs"), choices=[ - (None, "View only"), + ("", "View only"), (PermissionSets.EDIT_APPLICATION_ENVIRONMENTS, "Edit access"), ], - filters=[BaseForm.remove_empty_string], ) perms_team_mgmt = SelectField( translate("portfolios.applications.members.new.manage_team"), choices=[ - (None, "View only"), + ("", "View only"), (PermissionSets.EDIT_APPLICATION_TEAM, "Edit access"), ], - filters=[BaseForm.remove_empty_string], ) perms_del_env = SelectField( - choices=[(None, "No"), (PermissionSets.DELETE_APPLICATION_ENVIRONMENTS, "Yes")], - filters=[BaseForm.remove_empty_string], + choices=[("", "No"), (PermissionSets.DELETE_APPLICATION_ENVIRONMENTS, "Yes")] ) @property def data(self): _data = super().data + _data.pop("csrf_token", None) permission_sets = [] for field in _data: if _data[field] is not None: @@ -47,6 +45,13 @@ 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 8bee43ec..22b5b78e 100644 --- a/atst/routes/applications/team.py +++ b/atst/routes/applications/team.py @@ -32,6 +32,7 @@ def team(application_id): team_data = [] for member in application.members: user_id = member.user.id + # TODO: if no members, we get a server error user_name = member.user.full_name environment_users[user_id] = { "permissions": { diff --git a/templates/fragments/applications/edit_team.html b/templates/fragments/applications/edit_team.html index 094ccf40..a88361d6 100644 --- a/templates/fragments/applications/edit_team.html +++ b/templates/fragments/applications/edit_team.html @@ -4,40 +4,44 @@ {% set environment_roles_form = member_form.environment_roles %} {% set permissions_form = member_form.permission_sets %} - -
      • -
        -
        {{ member_form.user_name.data }}
        -
        {{ OptionsInput(permissions_form.perms_team_mgmt, label=False, watch=True) }}
        -
        {{ OptionsInput(permissions_form.perms_env_mgmt, label=False, watch=True) }}
        -
        {{ OptionsInput(permissions_form.perms_del_env, label=False, watch=True) }}
        -
        - {% set open_html %} - {{ "portfolios.applications.team_settings.environments" | translate }} ({{ environment_roles_form | length }}) {{ Icon('caret_down') }} - {% endset %} +
        + {{ team_form.csrf_token }} + +
      • +
        +
        {{ member_form.user_name.data }}
        +
        {{ OptionsInput(permissions_form.perms_team_mgmt, label=False, watch=True) }}
        +
        {{ OptionsInput(permissions_form.perms_env_mgmt, label=False, watch=True) }}
        +
        {{ OptionsInput(permissions_form.perms_del_env, label=False, watch=True) }}
        +
        + {% set open_html %} + {{ "portfolios.applications.team_settings.environments" | translate }} ({{ environment_roles_form | length }}) {{ Icon('caret_down') }} + {% endset %} - {% set close_html %} - {{ "portfolios.applications.team_settings.environments" | translate }} ({{ environment_roles_form | length }}) {{ Icon('caret_up') }} - {% endset %} + {% set close_html %} + {{ "portfolios.applications.team_settings.environments" | translate }} ({{ environment_roles_form | length }}) {{ Icon('caret_up') }} + {% endset %} - {{ - ToggleButton( - open_html=open_html, - close_html=close_html, - section_name="environments" - ) - }} + {{ + ToggleButton( + open_html=open_html, + close_html=close_html, + section_name="environments" + ) + }} +
        -
        • - {% call ToggleSection(section_name="environments") %} -
          - {% for environment_form in environment_roles_form %} -
        • - {{ environment_form.environment_name.data }} -
          • - {% endfor %} -
        - {% endcall %} -
        • -         + {% call ToggleSection(section_name="environments") %} +
          + {% for environment_form in environment_roles_form %} +
        • + {{ environment_form.environment_name.data }} +
          • + {% endfor %} +
        + {% endcall %} + {{ member_form.user_id() }} +
        • + + {% endfor %} diff --git a/templates/portfolios/applications/team.html b/templates/portfolios/applications/team.html index f6df4f25..b80b070f 100644 --- a/templates/portfolios/applications/team.html +++ b/templates/portfolios/applications/team.html @@ -73,7 +73,7 @@
        {% if user_can(permissions.EDIT_APPLICATION_MEMBER) %} - {{ SaveButton(text=('common.save' | translate), element="input", form="member-perms") }} + {{ SaveButton(text=('common.save' | translate), element="input", form="team") }} {% endif %} {% if user_can(permissions.CREATE_APPLICATION_MEMBER) %} From 9f266ea4e4defa5b37c341819fe5d6d2472df774 Mon Sep 17 00:00:00 2001 From: Montana Date: Thu, 2 May 2019 15:09:12 -0400 Subject: [PATCH 07/14] Update user permission sets - adds two domain functions for application roles --- atst/domain/application_roles.py | 21 +++++++++++++++++++++ atst/routes/applications/team.py | 24 ++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/atst/domain/application_roles.py b/atst/domain/application_roles.py index af000706..ae157ecf 100644 --- a/atst/domain/application_roles.py +++ b/atst/domain/application_roles.py @@ -28,3 +28,24 @@ class ApplicationRoles(object): db.session.add(role) db.session.commit() + + @classmethod + def get(cls, user_id, application_id): + existing_app_role = ( + db.session.query(ApplicationRole) + .filter_by(user_id=user_id, application_id=application_id) + .one_or_none() + ) + + return existing_app_role + + @classmethod + def update_permission_sets(cls, application_role, new_perm_sets_names): + application_role.permission_sets = ApplicationRoles._permission_sets_for_names( + new_perm_sets_names + ) + + db.session.add(application_role) + db.session.commit() + + return application_role diff --git a/atst/routes/applications/team.py b/atst/routes/applications/team.py index 22b5b78e..ea548d07 100644 --- a/atst/routes/applications/team.py +++ b/atst/routes/applications/team.py @@ -3,6 +3,7 @@ from flask import render_template, request as http_request, g, url_for, redirect from . import applications_bp from atst.domain.applications import Applications +from atst.domain.application_roles import ApplicationRoles from atst.domain.environments import Environments from atst.domain.environment_roles import EnvironmentRoles from atst.domain.authz.decorator import user_can_access_decorator as user_can @@ -98,6 +99,29 @@ def team(application_id): ) +@applications_bp.route("/application//team", methods=["POST"]) +@user_can(Permissions.EDIT_APPLICATION_MEMBER, message="update application member") +def update_team(application_id): + application = Applications.get(application_id) + form = TeamForm(http_request.form) + + if form.validate(): + # TODO check that all users coming through are app members + 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 != ""] + ApplicationRoles.update_permission_sets(app_role, new_perms) + + return redirect( + url_for( + "applications.team", + application_id=application_id, + fragment="application-members", + _anchor="application-members", + ) + ) + + @applications_bp.route("/application//members/new", methods=["POST"]) @user_can( Permissions.CREATE_APPLICATION_MEMBER, message="create new application member" From 1759732d38bcc865a2096fa025d6120cacecbc50 Mon Sep 17 00:00:00 2001 From: Montana Date: Thu, 2 May 2019 15:25:29 -0400 Subject: [PATCH 08/14] Add success banner - The banner does not display which members were updated --- atst/routes/applications/team.py | 1 + atst/utils/flash.py | 7 +++++++ translations.yaml | 1 + 3 files changed, 9 insertions(+) diff --git a/atst/routes/applications/team.py b/atst/routes/applications/team.py index ea548d07..227aa644 100644 --- a/atst/routes/applications/team.py +++ b/atst/routes/applications/team.py @@ -111,6 +111,7 @@ def update_team(application_id): app_role = ApplicationRoles.get(member.data["user_id"], application.id) new_perms = [perm for perm in member.data["permission_sets"] if perm != ""] ApplicationRoles.update_permission_sets(app_role, new_perms) + flash("updated_application_members_permissions") return redirect( url_for( diff --git a/atst/utils/flash.py b/atst/utils/flash.py index 9fff1be2..53190fa1 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -168,6 +168,13 @@ MESSAGES = { """, "category": "success", }, + "updated_application_members_permissions": { + "title_template": translate("flash.success"), + "message_template": """ +

        {{ "flash.updated_application_members_permissions" | translate }}

        + """, + "category": "success", + }, } diff --git a/translations.yaml b/translations.yaml index d6f065dc..66ac5b15 100644 --- a/translations.yaml +++ b/translations.yaml @@ -72,6 +72,7 @@ flash: portfolio_home: Go to my portfolio home page success: Success! new_application_member: 'You have successfully invited {user_name} to the team.' + updated_application_members_permissions: 'You have successfully updated member permissions.' footer: about_link_text: Joint Enterprise Defense Infrastructure browser_support: JEDI Cloud supported on these web browsers From 526bcb16610302bfb5a520804b48b447eb544f05 Mon Sep 17 00:00:00 2001 From: Montana Date: Thu, 2 May 2019 16:15:01 -0400 Subject: [PATCH 09/14] Update tests --- atst/forms/team.py | 21 ++++++++------- atst/routes/applications/team.py | 8 +++--- .../fragments/applications/edit_team.html | 2 +- tests/domain/test_application_roles.py | 27 +++++++++++++++++++ tests/forms/test_team.py | 15 +++++++---- tests/routes/applications/test_team.py | 1 - 6 files changed, 53 insertions(+), 21 deletions(-) diff --git a/atst/forms/team.py b/atst/forms/team.py index fbbdffe5..513955f1 100644 --- a/atst/forms/team.py +++ b/atst/forms/team.py @@ -9,22 +9,25 @@ from atst.utils.localization import translate class PermissionsForm(FlaskForm): - perms_env_mgmt = SelectField( - translate("portfolios.applications.members.new.manage_envs"), - choices=[ - ("", "View only"), - (PermissionSets.EDIT_APPLICATION_ENVIRONMENTS, "Edit access"), - ], - ) perms_team_mgmt = SelectField( translate("portfolios.applications.members.new.manage_team"), choices=[ - ("", "View only"), + (PermissionSets.VIEW_APPLICATION, "View only"), (PermissionSets.EDIT_APPLICATION_TEAM, "Edit access"), ], ) + perms_env_mgmt = SelectField( + translate("portfolios.applications.members.new.manage_envs"), + choices=[ + (PermissionSets.VIEW_APPLICATION, "View only"), + (PermissionSets.EDIT_APPLICATION_ENVIRONMENTS, "Edit access"), + ], + ) perms_del_env = SelectField( - choices=[("", "No"), (PermissionSets.DELETE_APPLICATION_ENVIRONMENTS, "Yes")] + choices=[ + ("View only", "No"), + (PermissionSets.DELETE_APPLICATION_ENVIRONMENTS, "Yes"), + ] ) @property diff --git a/atst/routes/applications/team.py b/atst/routes/applications/team.py index 227aa644..dde891a9 100644 --- a/atst/routes/applications/team.py +++ b/atst/routes/applications/team.py @@ -4,11 +4,11 @@ from flask import render_template, request as http_request, g, url_for, redirect 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.authz.decorator import user_can_access_decorator as user_can -from atst.domain.permission_sets import PermissionSets from atst.domain.exceptions import AlreadyExistsError +from atst.domain.permission_sets import PermissionSets from atst.forms.application_member import NewForm as NewMemberForm from atst.forms.team import TeamForm from atst.models import Permissions @@ -33,7 +33,6 @@ def team(application_id): team_data = [] for member in application.members: user_id = member.user.id - # TODO: if no members, we get a server error user_name = member.user.full_name environment_users[user_id] = { "permissions": { @@ -82,12 +81,11 @@ def team(application_id): } ) - team_form = TeamForm(data={"members": team_data}) - 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 render_template( diff --git a/templates/fragments/applications/edit_team.html b/templates/fragments/applications/edit_team.html index a88361d6..f33cfe9c 100644 --- a/templates/fragments/applications/edit_team.html +++ b/templates/fragments/applications/edit_team.html @@ -4,7 +4,7 @@ {% set environment_roles_form = member_form.environment_roles %} {% set permissions_form = member_form.permission_sets %} -
        + {{ team_form.csrf_token }}
      • diff --git a/tests/domain/test_application_roles.py b/tests/domain/test_application_roles.py index f430c8cb..b77546af 100644 --- a/tests/domain/test_application_roles.py +++ b/tests/domain/test_application_roles.py @@ -33,3 +33,30 @@ def test_enabled_application_role(): ApplicationRoles.enable(app_role) assert app_role.status == ApplicationRoleStatus.ACTIVE + + +def test_get(): + user = UserFactory.create() + application = ApplicationFactory.create() + app_role = ApplicationRoleFactory.create(user=user, application=application) + + assert ApplicationRoles.get(user.id, application.id) + assert app_role.application == application + assert app_role.user == user + + +def test_update_permission_sets(): + user = UserFactory.create() + application = ApplicationFactory.create() + app_role = ApplicationRoleFactory.create(user=user, application=application) + + view_app = [PermissionSets.get(PermissionSets.VIEW_APPLICATION)] + new_perms_names = [ + PermissionSets.EDIT_APPLICATION_TEAM, + PermissionSets.DELETE_APPLICATION_ENVIRONMENTS, + ] + new_perms = PermissionSets.get_many(new_perms_names) + # view application permission is included by default + assert app_role.permission_sets == view_app + assert ApplicationRoles.update_permission_sets(app_role, new_perms_names) + assert set(app_role.permission_sets) == set(new_perms + view_app) diff --git a/tests/forms/test_team.py b/tests/forms/test_team.py index 671a44ee..895c5e6a 100644 --- a/tests/forms/test_team.py +++ b/tests/forms/test_team.py @@ -7,20 +7,25 @@ from atst.forms.team import * def test_permissions_form_permission_sets(): form_data = { - "perms_env_mgmt": "", "perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM, - "perms_del_env": "", + "perms_env_mgmt": PermissionSets.VIEW_APPLICATION, + "perms_del_env": "View only", } form = PermissionsForm(data=form_data) + assert form.validate() - assert form.data == [PermissionSets.EDIT_APPLICATION_TEAM] + assert form.data == [ + PermissionSets.EDIT_APPLICATION_TEAM, + PermissionSets.VIEW_APPLICATION, + "View only", + ] def test_permissions_form_invalid(): form_data = { - "perms_env_mgmt": "not a real choice", "perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM, - "perms_del_env": "", + "perms_env_mgmt": "not a real choice", + "perms_del_env": "View only", } 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 1c3e0a17..af776822 100644 --- a/tests/routes/applications/test_team.py +++ b/tests/routes/applications/test_team.py @@ -10,7 +10,6 @@ def test_application_team(client, user_session): user_session(portfolio.owner) response = client.get(url_for("applications.team", application_id=application.id)) - assert response.status_code == 200 From 1a2051b2bfb6dcf0aad005de9c8ac4be4e7831f6 Mon Sep 17 00:00:00 2001 From: Montana Date: Mon, 6 May 2019 15:14:42 -0400 Subject: [PATCH 10/14] Don't store empty strings as perm values --- atst/routes/applications/team.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/atst/routes/applications/team.py b/atst/routes/applications/team.py index dde891a9..99f4a27e 100644 --- a/atst/routes/applications/team.py +++ b/atst/routes/applications/team.py @@ -51,15 +51,15 @@ def team(application_id): ), } permission_sets = { - "perms_env_mgmt": PermissionSets.EDIT_APPLICATION_ENVIRONMENTS - if member.has_permission_set(PermissionSets.EDIT_APPLICATION_ENVIRONMENTS) - else "", - "perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM - if member.has_permission_set(PermissionSets.EDIT_APPLICATION_TEAM) - else "", "perms_del_env": PermissionSets.DELETE_APPLICATION_ENVIRONMENTS if member.has_permission_set(PermissionSets.DELETE_APPLICATION_ENVIRONMENTS) - else "", + else translate("portfolios.members.permissions.view_only"), + "perms_env_mgmt": PermissionSets.EDIT_APPLICATION_ENVIRONMENTS + if member.has_permission_set(PermissionSets.EDIT_APPLICATION_ENVIRONMENTS) + else translate("portfolios.members.permissions.view_only"), + "perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM + if member.has_permission_set(PermissionSets.EDIT_APPLICATION_TEAM) + else translate("portfolios.members.permissions.view_only"), } roles = EnvironmentRoles.get_for_application_and_user( member.user.id, application.id @@ -107,7 +107,9 @@ def update_team(application_id): # TODO check that all users coming through are app members 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 != ""] + new_perms = [ + perm for perm in member.data["permission_sets"] if perm != "View only" + ] ApplicationRoles.update_permission_sets(app_role, new_perms) flash("updated_application_members_permissions") From 2a9de4f3b9f089056c02bb8e2fed3838f9bd94c4 Mon Sep 17 00:00:00 2001 From: Montana Date: Mon, 6 May 2019 15:29:07 -0400 Subject: [PATCH 11/14] Match the orders of the permissions everywhere --- atst/routes/applications/team.py | 22 ++++++++++----------- templates/portfolios/applications/team.html | 4 ++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/atst/routes/applications/team.py b/atst/routes/applications/team.py index 99f4a27e..056b5d38 100644 --- a/atst/routes/applications/team.py +++ b/atst/routes/applications/team.py @@ -36,14 +36,14 @@ def team(application_id): user_name = member.user.full_name environment_users[user_id] = { "permissions": { - "delete_access": permission_str( - member, PermissionSets.DELETE_APPLICATION_ENVIRONMENTS + "team_management": permission_str( + member, PermissionSets.EDIT_APPLICATION_TEAM ), "environment_management": permission_str( member, PermissionSets.EDIT_APPLICATION_ENVIRONMENTS ), - "team_management": permission_str( - member, PermissionSets.EDIT_APPLICATION_TEAM + "delete_access": permission_str( + member, PermissionSets.DELETE_APPLICATION_ENVIRONMENTS ), }, "environments": Environments.for_user( @@ -51,15 +51,15 @@ def team(application_id): ), } permission_sets = { - "perms_del_env": PermissionSets.DELETE_APPLICATION_ENVIRONMENTS - if member.has_permission_set(PermissionSets.DELETE_APPLICATION_ENVIRONMENTS) - else translate("portfolios.members.permissions.view_only"), - "perms_env_mgmt": PermissionSets.EDIT_APPLICATION_ENVIRONMENTS - if member.has_permission_set(PermissionSets.EDIT_APPLICATION_ENVIRONMENTS) - else translate("portfolios.members.permissions.view_only"), "perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM if member.has_permission_set(PermissionSets.EDIT_APPLICATION_TEAM) - else translate("portfolios.members.permissions.view_only"), + 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 diff --git a/templates/portfolios/applications/team.html b/templates/portfolios/applications/team.html index b80b070f..ed79d259 100644 --- a/templates/portfolios/applications/team.html +++ b/templates/portfolios/applications/team.html @@ -49,13 +49,13 @@ {{ "common.name" | translate }}
        • - {{ "portfolios.applications.team_settings.section.table.delete_access" | translate }} + {{ "portfolios.applications.team_settings.section.table.team_management" | translate }}
        {{ "portfolios.applications.team_settings.section.table.environment_management" | translate }}
        - {{ "portfolios.applications.team_settings.section.table.team_management" | translate }} + {{ "portfolios.applications.team_settings.section.table.delete_access" | translate }}
          From 09c0bed47d9f62b37743748561b76fc5b82b17e5 Mon Sep 17 00:00:00 2001 From: Montana Date: Mon, 6 May 2019 17:09:51 -0400 Subject: [PATCH 12/14] Move edit form outside of the for loop --- templates/fragments/applications/edit_team.html | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/templates/fragments/applications/edit_team.html b/templates/fragments/applications/edit_team.html index f33cfe9c..2283c70e 100644 --- a/templates/fragments/applications/edit_team.html +++ b/templates/fragments/applications/edit_team.html @@ -1,11 +1,12 @@ {% from "components/options_input.html" import OptionsInput %} -{% for member_form in team_form.members %} - {% set environment_roles_form = member_form.environment_roles %} - {% set permissions_form = member_form.permission_sets %} + + {{ team_form.csrf_token }} + + {% for member_form in team_form.members %} + {% set environment_roles_form = member_form.environment_roles %} + {% set permissions_form = member_form.permission_sets %} - - {{ team_form.csrf_token }}
      • @@ -43,5 +44,5 @@ {{ member_form.user_id() }}
        • - -{% endfor %} + {% endfor %} + From d5307b440f824df3e236f97d77394579d2cdb74e Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 7 May 2019 10:23:58 -0400 Subject: [PATCH 13/14] Make user_id required and add post tests - raise an exception in ApplicationRoles.get() - permission_sets is not changed if bad data is sent --- atst/domain/application_roles.py | 20 ++++--- atst/forms/team.py | 3 +- atst/routes/applications/team.py | 1 - tests/domain/test_application_roles.py | 11 ++++ tests/routes/applications/test_team.py | 77 +++++++++++++++++++++++++- 5 files changed, 102 insertions(+), 10 deletions(-) diff --git a/atst/domain/application_roles.py b/atst/domain/application_roles.py index ae157ecf..e1956aba 100644 --- a/atst/domain/application_roles.py +++ b/atst/domain/application_roles.py @@ -1,6 +1,9 @@ +from sqlalchemy.orm.exc import NoResultFound + from atst.database import db from atst.models import ApplicationRole, ApplicationRoleStatus -from atst.domain.permission_sets import PermissionSets +from .permission_sets import PermissionSets +from .exceptions import NotFoundError class ApplicationRoles(object): @@ -31,13 +34,16 @@ class ApplicationRoles(object): @classmethod def get(cls, user_id, application_id): - existing_app_role = ( - db.session.query(ApplicationRole) - .filter_by(user_id=user_id, application_id=application_id) - .one_or_none() - ) + try: + app_role = ( + db.session.query(ApplicationRole) + .filter_by(user_id=user_id, application_id=application_id) + .one() + ) + except NoResultFound: + raise NotFoundError("application_role") - return existing_app_role + return app_role @classmethod def update_permission_sets(cls, application_role, new_perm_sets_names): diff --git a/atst/forms/team.py b/atst/forms/team.py index 513955f1..5b12a7cd 100644 --- a/atst/forms/team.py +++ b/atst/forms/team.py @@ -1,5 +1,6 @@ from flask_wtf import FlaskForm from wtforms.fields import FormField, FieldList, HiddenField, StringField +from wtforms.validators import Required from .application_member import EnvironmentForm from .forms import BaseForm @@ -43,7 +44,7 @@ class PermissionsForm(FlaskForm): class MemberForm(FlaskForm): - user_id = HiddenField() + user_id = HiddenField(validators=[Required()]) user_name = StringField() environment_roles = FieldList(FormField(EnvironmentForm)) permission_sets = FormField(PermissionsForm) diff --git a/atst/routes/applications/team.py b/atst/routes/applications/team.py index 056b5d38..cfec59a9 100644 --- a/atst/routes/applications/team.py +++ b/atst/routes/applications/team.py @@ -104,7 +104,6 @@ def update_team(application_id): form = TeamForm(http_request.form) if form.validate(): - # TODO check that all users coming through are app members for member in form.members: app_role = ApplicationRoles.get(member.data["user_id"], application.id) new_perms = [ diff --git a/tests/domain/test_application_roles.py b/tests/domain/test_application_roles.py index b77546af..b26dd13e 100644 --- a/tests/domain/test_application_roles.py +++ b/tests/domain/test_application_roles.py @@ -1,4 +1,7 @@ +import pytest + from atst.domain.application_roles import ApplicationRoles +from atst.domain.exceptions import NotFoundError from atst.domain.permission_sets import PermissionSets from atst.models import ApplicationRoleStatus @@ -45,6 +48,14 @@ def test_get(): assert app_role.user == user +def test_get_handles_invalid_id(): + user = UserFactory.create() + application = ApplicationFactory.create() + + with pytest.raises(NotFoundError): + ApplicationRoles.get(user.id, application.id) + + def test_update_permission_sets(): user = UserFactory.create() application = ApplicationFactory.create() diff --git a/tests/routes/applications/test_team.py b/tests/routes/applications/test_team.py index af776822..e5c0d54f 100644 --- a/tests/routes/applications/test_team.py +++ b/tests/routes/applications/test_team.py @@ -1,6 +1,8 @@ from flask import url_for -from tests.factories import PortfolioFactory, ApplicationFactory, UserFactory +from atst.domain.permission_sets import PermissionSets + +from tests.factories import * def test_application_team(client, user_session): @@ -13,6 +15,79 @@ def test_application_team(client, user_session): assert response.status_code == 200 +def test_update_team(client, user_session): + application = ApplicationFactory.create() + owner = application.portfolio.owner + app_role = ApplicationRoleFactory.create( + application=application, permission_sets=[] + ) + app_user = app_role.user + user_session(owner) + response = client.post( + url_for("applications.update_team", application_id=application.id), + data={ + "members-0-user_id": app_user.id, + "members-0-permission_sets-perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM, + "members-0-permission_sets-perms_env_mgmt": PermissionSets.EDIT_APPLICATION_ENVIRONMENTS, + "members-0-permission_sets-perms_del_env": PermissionSets.DELETE_APPLICATION_ENVIRONMENTS, + }, + ) + + assert response.status_code == 302 + actual_perms_names = [perm.name for perm in app_role.permission_sets] + expected_perms_names = [ + PermissionSets.VIEW_APPLICATION, + PermissionSets.EDIT_APPLICATION_TEAM, + PermissionSets.EDIT_APPLICATION_ENVIRONMENTS, + PermissionSets.DELETE_APPLICATION_ENVIRONMENTS, + ] + assert expected_perms_names == actual_perms_names + + +def test_update_team_with_bad_permission_sets(client, user_session): + application = ApplicationFactory.create() + owner = application.portfolio.owner + app_role = ApplicationRoleFactory.create( + application=application, permission_sets=[] + ) + app_user = app_role.user + permission_sets = app_user.permission_sets + + user_session(owner) + response = client.post( + url_for("applications.update_team", application_id=application.id), + data={ + "members-0-user_id": app_user.id, + "members-0-permission_sets-perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM, + "members-0-permission_sets-perms_env_mgmt": "some random string", + }, + ) + assert app_user.permission_sets == permission_sets + + +def test_update_team_with_non_app_user(client, user_session): + application = ApplicationFactory.create() + owner = application.portfolio.owner + app_role = ApplicationRoleFactory.create( + application=application, permission_sets=[] + ) + non_app_user = UserFactory.create() + app_user = app_role.user + + user_session(owner) + response = client.post( + url_for("applications.update_team", application_id=application.id), + data={ + "members-0-user_id": non_app_user.id, + "members-0-permission_sets-perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM, + "members-0-permission_sets-perms_env_mgmt": PermissionSets.EDIT_APPLICATION_ENVIRONMENTS, + "members-0-permission_sets-perms_del_env": PermissionSets.DELETE_APPLICATION_ENVIRONMENTS, + }, + ) + + assert response.status_code == 404 + + def test_create_member(client, user_session): user = UserFactory.create() application = ApplicationFactory.create( From 644acc68fadc4bdfc2f903a2af8977dabf905964 Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 7 May 2019 11:47:27 -0400 Subject: [PATCH 14/14] 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