diff --git a/atst/routes/portfolios/admin.py b/atst/routes/portfolios/admin.py index fb90800c..3fc79d96 100644 --- a/atst/routes/portfolios/admin.py +++ b/atst/routes/portfolios/admin.py @@ -166,22 +166,23 @@ def edit(portfolio_id): @portfolios_bp.route( - "/portfolios//members//delete", methods=["POST"] + "/portfolios//members//delete", methods=["POST"] ) @user_can(Permissions.EDIT_PORTFOLIO_USERS, message="update portfolio members") -def remove_member(portfolio_id, user_id): - if str(g.current_user.id) == user_id: +def remove_member(portfolio_id, portfolio_role_id): + portfolio_role = PortfolioRoles.get_by_id(portfolio_role_id) + + if g.current_user.id == portfolio_role.user_id: raise UnauthorizedError( g.current_user, "you cant remove yourself from the portfolio" ) portfolio = Portfolios.get(user=g.current_user, portfolio_id=portfolio_id) - if user_id == str(portfolio.owner.id): + if portfolio_role.user_id == portfolio.owner.id: raise UnauthorizedError( g.current_user, "you can't delete the portfolios PPoC from the portfolio" ) - portfolio_role = PortfolioRoles.get(portfolio_id=portfolio_id, user_id=user_id) # TODO: should this cascade and disable any application and environment # roles they might have? PortfolioRoles.disable(portfolio_role=portfolio_role) diff --git a/templates/fragments/admin/members_edit.html b/templates/fragments/admin/members_edit.html index d190a90d..8fb62f35 100644 --- a/templates/fragments/admin/members_edit.html +++ b/templates/fragments/admin/members_edit.html @@ -1,3 +1,5 @@ +{% from "components/alert.html" import Alert %} +{% from "components/modal.html" import Modal %} {% from "components/options_input.html" import OptionsInput %} {% for subform in member_perms_form.members_permissions %} @@ -29,6 +31,7 @@ {{ "portfolios.members.archive_button" | translate }} + {% if not ppoc %} {{ subform.member_id() }} {% endif %} diff --git a/templates/fragments/admin/portfolio_members.html b/templates/fragments/admin/portfolio_members.html index 4dd098ec..7f167092 100644 --- a/templates/fragments/admin/portfolio_members.html +++ b/templates/fragments/admin/portfolio_members.html @@ -76,34 +76,32 @@ {% include "fragments/admin/add_new_portfolio_member.html" %} {% endif %} - {% if user_can(permissions.EDIT_PORTFOLIO_USERS) %} - {% for member in portfolio.members %} - {% set modal_id = "portfolio_id_{}_user_id_{}".format(portfolio.id, member.user_id) %} - {% call Modal(name=modal_id, dismissable=False) %} -

{{ "portfolios.admin.alert_header" | translate }}

- - {{ - Alert( - title="portfolios.admin.alert_title" | translate, - message="portfolios.admin.alert_message" | translate, - level="warning" - ) - }} - - diff --git a/tests/routes/portfolios/test_admin.py b/tests/routes/portfolios/test_admin.py index 370bf3db..569dc5cc 100644 --- a/tests/routes/portfolios/test_admin.py +++ b/tests/routes/portfolios/test_admin.py @@ -305,12 +305,16 @@ def test_remove_portfolio_member(client, user_session): portfolio = PortfolioFactory.create() user = UserFactory.create() - PortfolioRoleFactory.create(portfolio=portfolio, user=user) + member = PortfolioRoleFactory.create(portfolio=portfolio, user=user) user_session(portfolio.owner) response = client.post( - url_for("portfolios.remove_member", portfolio_id=portfolio.id, user_id=user.id), + url_for( + "portfolios.remove_member", + portfolio_id=portfolio.id, + portfolio_role_id=member.id, + ), follow_redirects=False, ) @@ -330,6 +334,9 @@ def test_remove_portfolio_member(client, user_session): def test_remove_portfolio_member_self(client, user_session): portfolio = PortfolioFactory.create() + portfolio_role = PortfolioRoles.get( + portfolio_id=portfolio.id, user_id=portfolio.owner.id + ) user_session(portfolio.owner) @@ -337,7 +344,7 @@ def test_remove_portfolio_member_self(client, user_session): url_for( "portfolios.remove_member", portfolio_id=portfolio.id, - user_id=portfolio.owner.id, + portfolio_role_id=portfolio_role.id, ), follow_redirects=False, ) @@ -358,6 +365,9 @@ def test_remove_portfolio_member_ppoc(client, user_session): user=user, permission_sets=[PermissionSets.get(PermissionSets.EDIT_PORTFOLIO_ADMIN)], ) + ppoc_port_role = PortfolioRoles.get( + portfolio_id=portfolio.id, user_id=portfolio.owner.id + ) user_session(user) @@ -365,7 +375,7 @@ def test_remove_portfolio_member_ppoc(client, user_session): url_for( "portfolios.remove_member", portfolio_id=portfolio.id, - user_id=portfolio.owner.id, + portfolio_role_id=ppoc_port_role.id, ), follow_redirects=False, )