From 720c227c3268bbd39bc83e3ff8d62ef75e7b279d Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 13 Mar 2019 10:11:30 -0400 Subject: [PATCH] remove frontend role selection for edit member --- atst/domain/portfolio_roles.py | 7 +-- atst/domain/portfolios/portfolios.py | 5 +- atst/forms/edit_member.py | 18 ------- atst/forms/new_member.py | 63 ---------------------- atst/forms/portfolio_member.py | 71 +++++++++++++++++++++++++ atst/routes/portfolios/members.py | 26 +++------ templates/portfolios/members/edit.html | 25 ++++++++- tests/domain/test_portfolios.py | 4 +- tests/routes/portfolios/test_members.py | 26 +++++---- 9 files changed, 127 insertions(+), 118 deletions(-) delete mode 100644 atst/forms/edit_member.py create mode 100644 atst/forms/portfolio_member.py diff --git a/atst/domain/portfolio_roles.py b/atst/domain/portfolio_roles.py index db01f0e3..e7e58ed6 100644 --- a/atst/domain/portfolio_roles.py +++ b/atst/domain/portfolio_roles.py @@ -111,12 +111,13 @@ class PortfolioRoles(object): return [Roles.get(perms_set_name) for perms_set_name in perms_set_names] @classmethod - def update_role(cls, portfolio_role, role_name): - new_role = Roles.get(role_name) - portfolio_role.role = new_role + def update(cls, portfolio_role, set_names): + new_permission_sets = PortfolioRoles._permission_sets_for_names(set_names) + portfolio_role.permission_sets = new_permission_sets db.session.add(portfolio_role) db.session.commit() + return portfolio_role @classmethod diff --git a/atst/domain/portfolios/portfolios.py b/atst/domain/portfolios/portfolios.py index 748b386f..7bbea7be 100644 --- a/atst/domain/portfolios/portfolios.py +++ b/atst/domain/portfolios/portfolios.py @@ -114,12 +114,13 @@ class Portfolios(object): return portfolio_role @classmethod - def update_member(cls, user, portfolio, member, role_name): + def update_member(cls, user, portfolio, member, permission_sets): Authorization.check_portfolio_permission( user, portfolio, Permissions.EDIT_PORTFOLIO_USERS, "edit portfolio member" ) - return PortfolioRoles.update_role(member, role_name) + # need to update perms sets here + return PortfolioRoles.update(member, permission_sets) @classmethod def _create_portfolio_role( diff --git a/atst/forms/edit_member.py b/atst/forms/edit_member.py deleted file mode 100644 index 766ed65c..00000000 --- a/atst/forms/edit_member.py +++ /dev/null @@ -1,18 +0,0 @@ -from wtforms.validators import Required - -from .forms import BaseForm -from atst.forms.fields import SelectField -from atst.utils.localization import translate - -from .data import PORTFOLIO_ROLES - - -class EditMemberForm(BaseForm): - # This form also accepts a field for each environment in each application - # that the user is a member of - - portfolio_role = SelectField( - translate("forms.edit_member.portfolio_role_label"), - choices=PORTFOLIO_ROLES, - validators=[Required()], - ) diff --git a/atst/forms/new_member.py b/atst/forms/new_member.py index 8c683dc7..e69de29b 100644 --- a/atst/forms/new_member.py +++ b/atst/forms/new_member.py @@ -1,63 +0,0 @@ -from wtforms.fields import StringField -from wtforms.fields.html5 import EmailField -from wtforms.validators import Required, Email, Length - -from .forms import BaseForm -from atst.forms.validators import IsNumber -from atst.forms.fields import SelectField -from atst.utils.localization import translate - - -class NewMemberForm(BaseForm): - - first_name = StringField( - label=translate("forms.new_member.first_name_label"), validators=[Required()] - ) - last_name = StringField( - label=translate("forms.new_member.last_name_label"), validators=[Required()] - ) - email = EmailField( - translate("forms.new_member.email_label"), validators=[Required(), Email()] - ) - dod_id = StringField( - translate("forms.new_member.dod_id_label"), - validators=[Required(), Length(min=10), IsNumber()], - ) - perms_app_mgmt = SelectField( - None, - choices=[ - ("view_portfolio_application_management", "View Only"), - ("edit_portfolio_application_management", "Edit Access"), - ], - ) - perms_funding = SelectField( - None, - choices=[ - ("view_portfolio_funding", "View Only"), - ("edit_portfolio_funding", "Edit Access"), - ], - ) - perms_reporting = SelectField( - None, - choices=[ - ("view_portfolio_reports", "View Only"), - ("edit_portfolio_reports", "Edit Access"), - ], - ) - perms_portfolio_mgmt = SelectField( - None, - choices=[ - ("view_portfolio_admin", "View Only"), - ("edit_portfolio_admin", "Edit Access"), - ], - ) - - @property - def data(self): - _data = super().data - _data["permission_sets"] = [] - for field in _data: - if "perms" in field: - _data["permission_sets"].append(_data[field]) - - return _data diff --git a/atst/forms/portfolio_member.py b/atst/forms/portfolio_member.py new file mode 100644 index 00000000..f75c2869 --- /dev/null +++ b/atst/forms/portfolio_member.py @@ -0,0 +1,71 @@ +from wtforms.fields import StringField +from wtforms.fields.html5 import EmailField +from wtforms.validators import Required, Email, Length + +from .forms import BaseForm +from atst.forms.validators import IsNumber +from atst.forms.fields import SelectField +from atst.utils.localization import translate + + +class PermissionsForm(BaseForm): + perms_app_mgmt = SelectField( + None, + choices=[ + ("view_portfolio_application_management", "View Only"), + ("edit_portfolio_application_management", "Edit Access"), + ], + ) + perms_funding = SelectField( + None, + choices=[ + ("view_portfolio_funding", "View Only"), + ("edit_portfolio_funding", "Edit Access"), + ], + ) + perms_reporting = SelectField( + None, + choices=[ + ("view_portfolio_reports", "View Only"), + ("edit_portfolio_reports", "Edit Access"), + ], + ) + perms_portfolio_mgmt = SelectField( + None, + choices=[ + ("view_portfolio_admin", "View Only"), + ("edit_portfolio_admin", "Edit Access"), + ], + ) + + @property + def data(self): + _data = super().data + _data["permission_sets"] = [] + for field in _data: + if "perms" in field: + _data["permission_sets"].append(_data[field]) + + return _data + + +class EditForm(PermissionsForm): + # This form also accepts a field for each environment in each application + # that the user is a member of + pass + + +class NewForm(PermissionsForm): + first_name = StringField( + label=translate("forms.new_member.first_name_label"), validators=[Required()] + ) + last_name = StringField( + label=translate("forms.new_member.last_name_label"), validators=[Required()] + ) + email = EmailField( + translate("forms.new_member.email_label"), validators=[Required(), Email()] + ) + dod_id = StringField( + translate("forms.new_member.dod_id_label"), + validators=[Required(), Length(min=10), IsNumber()], + ) diff --git a/atst/routes/portfolios/members.py b/atst/routes/portfolios/members.py index 3fb86e65..30ba503b 100644 --- a/atst/routes/portfolios/members.py +++ b/atst/routes/portfolios/members.py @@ -10,8 +10,7 @@ from atst.domain.portfolio_roles import PortfolioRoles, MEMBER_STATUS_CHOICES from atst.domain.environments import Environments from atst.domain.environment_roles import EnvironmentRoles from atst.services.invitation import Invitation as InvitationService -from atst.forms.new_member import NewMemberForm -from atst.forms.edit_member import EditMemberForm +import atst.forms.portfolio_member as member_forms from atst.forms.data import ( ENVIRONMENT_ROLES, ENV_ROLE_MODAL_DESCRIPTION, @@ -70,7 +69,7 @@ def application_members(portfolio_id, application_id): @portfolios_bp.route("/portfolios//members/new") def new_member(portfolio_id): portfolio = Portfolios.get(g.current_user, portfolio_id) - form = NewMemberForm() + form = member_forms.NewForm() return render_template( "portfolios/members/new.html", portfolio=portfolio, form=form ) @@ -79,7 +78,7 @@ def new_member(portfolio_id): @portfolios_bp.route("/portfolios//members/new", methods=["POST"]) def create_member(portfolio_id): portfolio = Portfolios.get(g.current_user, portfolio_id) - form = NewMemberForm(http_request.form) + form = member_forms.NewForm(http_request.form) if form.validate(): try: @@ -115,7 +114,7 @@ def view_member(portfolio_id, member_id): ) member = PortfolioRoles.get(portfolio_id, member_id) applications = Applications.get_all(g.current_user, member, portfolio) - form = EditMemberForm(portfolio_role="admin") + form = member_forms.EditForm(portfolio_role="admin") editable = g.current_user == member.user can_revoke_access = Portfolios.can_revoke_access_for(portfolio, member) @@ -157,20 +156,11 @@ def update_member(portfolio_id, member_id): env_role = form_dict[entry] or None ids_and_roles.append({"id": env_id, "role": env_role}) - form = EditMemberForm(http_request.form) + form = member_forms.EditForm(http_request.form) if form.validate(): - new_role_name = None - if form.data["portfolio_role"] != member.role.name: - member = Portfolios.update_member( - g.current_user, portfolio, member, form.data["portfolio_role"] - ) - new_role_name = member.role_displayname - flash( - "portfolio_role_updated", - member_name=member.user_name, - updated_role=new_role_name, - ) - + member = Portfolios.update_member( + g.current_user, portfolio, member, form.data["permission_sets"] + ) updated_roles = Environments.update_environment_roles( g.current_user, portfolio, member, ids_and_roles ) diff --git a/templates/portfolios/members/edit.html b/templates/portfolios/members/edit.html index 2e23890c..6c31ae1c 100644 --- a/templates/portfolios/members/edit.html +++ b/templates/portfolios/members/edit.html @@ -21,7 +21,30 @@

{{ member.user.full_name }}

- {{ Selector(form.portfolio_role) }} + + + + + + + + + + + + + + + +
{{ "portfolios.members.permissions.app_mgmt" | translate }}{{ "portfolios.members.permissions.funding" | translate }}{{ "portfolios.members.permissions.reporting" | translate }}{{ "portfolios.members.permissions.portfolio_mgmt" | translate }}
+ {{ form.perms_app_mgmt() }} + + {{ form.perms_funding() }} + + {{ form.perms_reporting() }} + + {{ form.perms_portfolio_mgmt() }} +
diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index 34b926ea..54b0f740 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -114,10 +114,10 @@ def test_update_portfolio_role_role(portfolio, portfolio_owner): } PortfolioRoleFactory._meta.sqlalchemy_session_persistence = "flush" member = PortfolioRoleFactory.create(portfolio=portfolio) - role_name = "admin" + permission_sets = ["edit_portfolio_funding"] updated_member = Portfolios.update_member( - portfolio_owner, portfolio, member, role_name + portfolio_owner, portfolio, member, permission_sets=permission_sets ) assert updated_member.portfolio == portfolio diff --git a/tests/routes/portfolios/test_members.py b/tests/routes/portfolios/test_members.py index 012b53e7..af63da9a 100644 --- a/tests/routes/portfolios/test_members.py +++ b/tests/routes/portfolios/test_members.py @@ -12,10 +12,18 @@ from atst.domain.portfolio_roles import PortfolioRoles from atst.domain.applications import Applications from atst.domain.environments import Environments from atst.domain.environment_roles import EnvironmentRoles +from atst.domain.roles import Roles from atst.queue import queue from atst.models.portfolio_role import Status as PortfolioRoleStatus from atst.models.invitation import Status as InvitationStatus +_DEFAULT_PERMS_FORM_DATA = { + "perms_app_mgmt": "view_portfolio_application_management", + "perms_funding": "view_portfolio_funding", + "perms_reporting": "view_portfolio_reports", + "perms_portfolio_mgmt": "view_portfolio_admin", +} + def create_portfolio_and_invite_user( ws_role="developer", @@ -90,10 +98,7 @@ def test_create_member(client, user_session): "last_name": "Zuckerman", "email": "some_pig@zuckermans.com", "portfolio_role": "developer", - "perms_app_mgmt": "view_portfolio_application_management", - "perms_funding": "view_portfolio_funding", - "perms_reporting": "view_portfolio_reports", - "perms_portfolio_mgmt": "view_portfolio_admin", + **_DEFAULT_PERMS_FORM_DATA, }, follow_redirects=True, ) @@ -121,7 +126,6 @@ def test_view_member_shows_role(client, user_session): assert "initial-choice='developer'".encode() in response.data -@pytest.mark.skip(reason="need to re-implement for permission set changes") def test_update_member_portfolio_role(client, user_session): portfolio = PortfolioFactory.create() user = UserFactory.create() @@ -131,20 +135,20 @@ def test_update_member_portfolio_role(client, user_session): url_for( "portfolios.update_member", portfolio_id=portfolio.id, member_id=user.id ), - data={"portfolio_role": "security_auditor"}, + data={**_DEFAULT_PERMS_FORM_DATA, "perms_funding": "edit_portfolio_funding"}, follow_redirects=True, ) assert response.status_code == 200 - assert b"role updated successfully" in response.data - assert member.role_name == "security_auditor" + edit_funding = Roles.get("edit_portfolio_funding") + assert edit_funding in member.permission_sets -@pytest.mark.skip(reason="update member permission sets not implemented") def test_update_member_portfolio_role_with_no_data(client, user_session): portfolio = PortfolioFactory.create() user = UserFactory.create() member = PortfolioRoles.add(user, portfolio.id) user_session(portfolio.owner) + original_perms_len = len(member.permission_sets) response = client.post( url_for( "portfolios.update_member", portfolio_id=portfolio.id, member_id=user.id @@ -153,10 +157,9 @@ def test_update_member_portfolio_role_with_no_data(client, user_session): follow_redirects=True, ) assert response.status_code == 200 - assert member.role_name == "developer" + assert len(member.permission_sets) == original_perms_len -@pytest.mark.skip(reason="update member permission sets not implemented") def test_update_member_environment_role(client, user_session): portfolio = PortfolioFactory.create() user = UserFactory.create() @@ -180,6 +183,7 @@ def test_update_member_environment_role(client, user_session): data={ "env_" + str(env1_id): "security_auditor", "env_" + str(env2_id): "devops", + **_DEFAULT_PERMS_FORM_DATA, }, follow_redirects=True, )