diff --git a/.secrets.baseline b/.secrets.baseline index bd9bdadc..5e2be19d 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -152,7 +152,7 @@ "hashed_secret": "e4f14805dfd1e6af030359090c535e149e6b4207", "is_secret": false, "is_verified": false, - "line_number": 665, + "line_number": 649, "type": "Hex High Entropy String" } ] diff --git a/atst/routes/portfolios/admin.py b/atst/routes/portfolios/admin.py index 699bdfab..4318a47c 100644 --- a/atst/routes/portfolios/admin.py +++ b/atst/routes/portfolios/admin.py @@ -19,9 +19,6 @@ from atst.domain.exceptions import UnauthorizedError def filter_perm_sets_data(member): perm_sets_data = { - "perms_portfolio_mgmt": bool( - member.has_permission_set(PermissionSets.EDIT_PORTFOLIO_ADMIN) - ), "perms_app_mgmt": bool( member.has_permission_set( PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT @@ -33,24 +30,43 @@ def filter_perm_sets_data(member): "perms_reporting": bool( member.has_permission_set(PermissionSets.EDIT_PORTFOLIO_REPORTS) ), + "perms_portfolio_mgmt": bool( + member.has_permission_set(PermissionSets.EDIT_PORTFOLIO_ADMIN) + ), } return perm_sets_data -def filter_members_data(members_list, portfolio): +def filter_members_data(members_list): members_data = [] for member in members_list: - members_data.append( - { - "role_id": member.id, - "user_name": member.user_name, - "permission_sets": filter_perm_sets_data(member), - "status": member.display_status, - "ppoc": PermissionSets.PORTFOLIO_POC in member.permission_sets, - # add in stuff here for forms - } + permission_sets = filter_perm_sets_data(member) + ppoc = ( + PermissionSets.get(PermissionSets.PORTFOLIO_POC) in member.permission_sets ) + member_data = { + "role_id": member.id, + "user_name": member.user_name, + "permission_sets": filter_perm_sets_data(member), + "status": member.display_status, + "ppoc": ppoc, + "form": member_forms.PermissionsForm(permission_sets), + } + + if not ppoc: + member_data["update_invite_form"] = ( + member_forms.NewForm(user_data=member.latest_invitation) + if member.latest_invitation and member.latest_invitation.can_resend + else member_forms.NewForm() + ) + member_data["invite_token"] = ( + member.latest_invitation.token + if member.latest_invitation and member.latest_invitation.can_resend + else None + ) + + members_data.append(member_data) return sorted(members_data, key=lambda member: member["user_name"]) @@ -75,7 +91,7 @@ def render_admin_page(portfolio, form=None): "portfolios/admin.html", form=form, portfolio_form=portfolio_form, - members=filter_members_data(member_list, portfolio), + members=filter_members_data(member_list), new_manager_form=member_forms.NewForm(), assign_ppoc_form=assign_ppoc_form, portfolio=portfolio, @@ -93,26 +109,27 @@ def admin(portfolio_id): return render_admin_page(portfolio) -@portfolios_bp.route("/portfolios//update_ppoc", methods=["POST"]) -@user_can(Permissions.EDIT_PORTFOLIO_POC, message="update portfolio ppoc") -def update_ppoc(portfolio_id): - role_id = http_request.form.get("role_id") - - portfolio = Portfolios.get(g.current_user, portfolio_id) - new_ppoc_role = PortfolioRoles.get_by_id(role_id) - - PortfolioRoles.make_ppoc(portfolio_role=new_ppoc_role) - - flash("primary_point_of_contact_changed", ppoc_name=new_ppoc_role.full_name) - - return redirect( - url_for( - "portfolios.admin", - portfolio_id=portfolio.id, - fragment="primary-point-of-contact", - _anchor="primary-point-of-contact", - ) - ) +# Updating PPoC is a post-MVP feature +# @portfolios_bp.route("/portfolios//update_ppoc", methods=["POST"]) +# @user_can(Permissions.EDIT_PORTFOLIO_POC, message="update portfolio ppoc") +# def update_ppoc(portfolio_id): # pragma: no cover +# role_id = http_request.form.get("role_id") +# +# portfolio = Portfolios.get(g.current_user, portfolio_id) +# new_ppoc_role = PortfolioRoles.get_by_id(role_id) +# +# PortfolioRoles.make_ppoc(portfolio_role=new_ppoc_role) +# +# flash("primary_point_of_contact_changed", ppoc_name=new_ppoc_role.full_name) +# +# return redirect( +# url_for( +# "portfolios.admin", +# portfolio_id=portfolio.id, +# fragment="primary-point-of-contact", +# _anchor="primary-point-of-contact", +# ) +# ) @portfolios_bp.route("/portfolios//edit", methods=["POST"]) @@ -166,3 +183,30 @@ def remove_member(portfolio_id, portfolio_role_id): fragment="portfolio-members", ) ) + + +@portfolios_bp.route( + "/portfolios//members/", methods=["POST"] +) +@user_can(Permissions.EDIT_PORTFOLIO_USERS, message="update portfolio members") +def update_member(portfolio_id, portfolio_role_id): + form_data = http_request.form + form = member_forms.PermissionsForm(formdata=form_data) + portfolio_role = PortfolioRoles.get_by_id(portfolio_role_id) + portfolio = Portfolios.get(user=g.current_user, portfolio_id=portfolio_id) + + if form.validate() and portfolio.owner_role != portfolio_role: + PortfolioRoles.update(portfolio_role, form.data["permission_sets"]) + flash("update_portfolio_member", member_name=portfolio_role.full_name) + + return redirect( + url_for( + "portfolios.admin", + portfolio_id=portfolio_id, + _anchor="portfolio-members", + fragment="portfolio-members", + ) + ) + else: + flash("update_portfolio_member_error", member_name=portfolio_role.full_name) + return (render_admin_page(portfolio), 400) diff --git a/atst/routes/portfolios/invitations.py b/atst/routes/portfolios/invitations.py index 09a22d1f..7c48593f 100644 --- a/atst/routes/portfolios/invitations.py +++ b/atst/routes/portfolios/invitations.py @@ -54,13 +54,22 @@ def revoke_invitation(portfolio_id, portfolio_token): ) @user_can(Permissions.EDIT_PORTFOLIO_USERS, message="resend invitation") def resend_invitation(portfolio_id, portfolio_token): - invite = PortfolioInvitations.resend(g.current_user, portfolio_token) - send_portfolio_invitation( - invitee_email=invite.email, - inviter_name=g.current_user.full_name, - token=invite.token, - ) - flash("resend_portfolio_invitation", user_name=invite.user_name) + form = member_forms.NewForm(http_request.form) + + if form.validate(): + invite = PortfolioInvitations.resend( + g.current_user, portfolio_token, form.data["user_data"] + ) + send_portfolio_invitation( + invitee_email=invite.email, + inviter_name=g.current_user.full_name, + token=invite.token, + ) + flash("resend_portfolio_invitation", user_name=invite.user_name) + else: + user_name = f"{form['user_data']['first_name'].data} {form['user_data']['last_name'].data}" + flash("resend_portfolio_invitation_error", user_name=user_name) + return redirect( url_for( "portfolios.admin", diff --git a/atst/utils/flash.py b/atst/utils/flash.py index da2c9253..7a1cf4ce 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -128,6 +128,11 @@ MESSAGES = { "message": "flash.portfolio_invite.resent.message", "category": "success", }, + "resend_portfolio_invitation_error": { + "title": "flash.portfolio_invite.error.title", + "message": "flash.portfolio_invite.error.message", + "category": "error", + }, "revoked_portfolio_access": { "title": "flash.portfolio_member.revoked.title", "message": "flash.portfolio_member.revoked.message", @@ -153,6 +158,16 @@ MESSAGES = { "message": "flash.task_order.submitted.message", "category": "success", }, + "update_portfolio_member": { + "title": "flash.portfolio_member.update.title", + "message": "flash.portfolio_member.update.message", + "category": "success", + }, + "update_portfolio_member_error": { + "title": "flash.portfolio_member.update_error.title", + "message": "flash.portfolio_member.update_error.message", + "category": "error", + }, "updated_application_team_settings": { "title": "flash.success", "message": "flash.updated_application_team_settings", diff --git a/styles/atat.scss b/styles/atat.scss index 0134dd89..72c7af40 100644 --- a/styles/atat.scss +++ b/styles/atat.scss @@ -39,6 +39,7 @@ @import "components/sticky_cta.scss"; @import "components/error_page.scss"; @import "components/member_form.scss"; +@import "components/toggle_menu.scss"; @import "sections/login"; @import "sections/home"; diff --git a/styles/components/_portfolio_layout.scss b/styles/components/_portfolio_layout.scss index fffc468f..81a15b69 100644 --- a/styles/components/_portfolio_layout.scss +++ b/styles/components/_portfolio_layout.scss @@ -130,10 +130,6 @@ &--th { width: 50%; } - - &--td { - position: relative; - } } .row { @@ -154,55 +150,6 @@ margin-right: $gap * 6; } } - - .app-member-menu { - position: absolute; - top: $gap; - right: $gap * 2; - - .accordion-table__item__toggler { - padding: $gap / 3; - border: 1px solid $color-gray-lighter; - border-radius: 3px; - cursor: pointer; - - &:hover, - &--active { - background-color: $color-aqua-lightest; - } - - .icon { - margin: $gap / 2; - } - } - - &__toggle { - position: absolute; - right: 0; - top: 30px; - background-color: $color-white; - border: 1px solid $color-gray-light; - z-index: 1; - margin-top: 0; - - a { - display: block; - padding: $gap; - border-bottom: 1px solid $color-gray-lighter; - text-decoration: none; - color: $color-black; - cursor: pointer; - - &:last-child { - border-bottom: 0; - } - - &:hover { - background-color: $color-aqua-lightest; - } - } - } - } } #add-new-env { diff --git a/styles/components/_toggle_menu.scss b/styles/components/_toggle_menu.scss new file mode 100644 index 00000000..b99bd75b --- /dev/null +++ b/styles/components/_toggle_menu.scss @@ -0,0 +1,58 @@ +.toggle-menu { + position: absolute; + top: $gap; + right: $gap * 2; + + &__container { + position: relative; + } + + .accordion-table__item__toggler { + padding: $gap / 3; + border: 1px solid $color-gray-lighter; + border-radius: 3px; + cursor: pointer; + + &:hover, + &--active { + background-color: $color-aqua-lightest; + } + + .icon { + margin: $gap / 2; + } + } + + &__toggle { + position: absolute; + right: 0; + top: 30px; + background-color: $color-white; + border: 1px solid $color-gray-light; + z-index: 1; + margin-top: 0; + + a { + display: block; + padding: $gap; + border-bottom: 1px solid $color-gray-lighter; + text-decoration: none; + color: $color-black; + cursor: pointer; + white-space: nowrap; + + &:last-child { + border-bottom: 0; + } + + &:hover { + background-color: $color-aqua-lightest; + } + + &.disabled { + color: $color-gray; + pointer-events: none; + } + } + } +} diff --git a/templates/applications/fragments/members.html b/templates/applications/fragments/members.html index d6fb7290..9bd80dd0 100644 --- a/templates/applications/fragments/members.html +++ b/templates/applications/fragments/members.html @@ -5,6 +5,7 @@ {% from "components/modal.html" import Modal %} {% from "components/multi_step_modal_form.html" import MultiStepModalForm %} {% from "components/save_button.html" import SaveButton %} +{% from "components/toggle_menu.html" import ToggleMenu %} {% macro MemberManagementTemplate( application, @@ -38,16 +39,17 @@ {% call Modal(modal_name, classes="form-content--app-mem") %}

{{ Icon('avatar') }} {{ "portfolios.applications.members.form.edit_access_header" | translate({ "user": member.user_name }) }}

-
{% endcall %} @@ -57,16 +59,17 @@ {% call Modal(resend_invite_modal, classes="form-content--app-mem") %}

{{ "portfolios.applications.members.new.verify" | translate }}

-
{{ member.update_invite_form.csrf_token }} - {{ member_fields.InfoFields(member.update_invite_form) }} -
- {{ SaveButton(text="Resend Invite")}} - {{ "common.cancel" | translate }} -
+ {{ member_form.SubmitStep( + name=resend_invite_modal, + form=member_fields.InfoFields(member.update_invite_form), + submit_text="Resend Invite", + previous=False, + modal=resend_invite_modal, + ) }}
{% endcall %} @@ -119,7 +122,7 @@ {% endfor %} - + {% for env in member.environment_roles %}
@@ -131,32 +134,21 @@
{% endfor %} {% if user_can(permissions.EDIT_APPLICATION_MEMBER) -%} - -
- - {{ Icon('ellipsis')}} - - - {{ Icon('ellipsis')}} - - -
- - {{ "portfolios.applications.members.menu.edit" | translate }} - - {% if invite_pending or invite_expired -%} - {% set revoke_invite_modal = "revoke_invite_{}".format(member.role_id) %} - {% set resend_invite_modal = "resend_invite-{}".format(member.role_id) %} - - {{ "portfolios.applications.members.menu.resend" | translate }} - - {% if user_can(permissions.DELETE_APPLICATION_MEMBER) -%} - {{ 'invites.revoke' | translate }} - {%- endif %} - {%- endif %} -
-
- + {% call ToggleMenu() %} + + {{ "portfolios.applications.members.menu.edit" | translate }} + + {% if invite_pending or invite_expired -%} + {% set revoke_invite_modal = "revoke_invite_{}".format(member.role_id) %} + {% set resend_invite_modal = "resend_invite-{}".format(member.role_id) %} + + {{ "portfolios.applications.members.menu.resend" | translate }} + + {% if user_can(permissions.DELETE_APPLICATION_MEMBER) -%} + {{ 'invites.revoke' | translate }} + {%- endif %} + {%- endif %} + {% endcall %} {%- endif %} diff --git a/templates/components/toggle_menu.html b/templates/components/toggle_menu.html new file mode 100644 index 00000000..b9cd0cfa --- /dev/null +++ b/templates/components/toggle_menu.html @@ -0,0 +1,17 @@ +{% from "components/icon.html" import Icon %} + +{% macro ToggleMenu() %} + +
+ + {{ Icon('ellipsis')}} + + + {{ Icon('ellipsis')}} + +
+ {{ caller() }} +
+
+ +{% endmacro %} diff --git a/templates/portfolios/admin.html b/templates/portfolios/admin.html index 3a924cc3..3c2a9b9c 100644 --- a/templates/portfolios/admin.html +++ b/templates/portfolios/admin.html @@ -59,10 +59,6 @@
- {% if user_can(permissions.VIEW_PORTFOLIO_POC) %} - {% include "portfolios/fragments/primary_point_of_contact.html" %} - {% endif %} - {% if user_can(permissions.VIEW_PORTFOLIO_USERS) %} {% include "portfolios/fragments/portfolio_members.html" %} {% endif %} diff --git a/templates/portfolios/fragments/change_ppoc.html b/templates/portfolios/fragments/change_ppoc.html deleted file mode 100644 index 478a55d4..00000000 --- a/templates/portfolios/fragments/change_ppoc.html +++ /dev/null @@ -1,80 +0,0 @@ -{% from "components/icon.html" import Icon %} -{% from "components/text_input.html" import TextInput %} -{% from "components/multi_step_modal_form.html" import MultiStepModalForm %} -{% from "components/alert.html" import Alert %} -{% from "components/options_input.html" import OptionsInput %} - -{% set step_one %} -
-

{{ "fragments.ppoc.update_ppoc_title" | translate }}

- - {{ - Alert( - level="warning", - title=("fragments.ppoc.alert.title" | translate), - message=("fragments.ppoc.alert.message" | translate), - ) - }} - -
-
- {{ - OptionsInput( - assign_ppoc_form.role_id, - optional=False - ) - }} -
-
-
-
-
- - - {{ "common.cancel" | translate }} - -
-{% endset %} - -{% set step_two %} -
-

{{ "fragments.ppoc.update_ppoc_confirmation_title" | translate }}

- - {{ - Alert( - level="info", - title=("fragments.ppoc.confirm_alert.title" | translate), - ) - }} - -
- - - {{ "common.cancel" | translate }} - -
-{% endset %} - -
- {% set disable_ppoc_button = 1 == portfolio.members |length %} - - {{ - MultiStepModalForm( - 'change-ppoc-form', - assign_ppoc_form, - form_action=url_for("portfolios.update_ppoc", portfolio_id=portfolio.id), - steps=[step_one, step_two], - ) - }} -
diff --git a/templates/portfolios/fragments/portfolio_members.html b/templates/portfolios/fragments/portfolio_members.html index ab9413f2..91e99d24 100644 --- a/templates/portfolios/fragments/portfolio_members.html +++ b/templates/portfolios/fragments/portfolio_members.html @@ -5,6 +5,92 @@ {% from "components/multi_step_modal_form.html" import MultiStepModalForm %} {% from 'components/save_button.html' import SaveButton %} {% import "portfolios/fragments/member_form_fields.html" as member_form_fields %} +{% from "components/toggle_menu.html" import ToggleMenu %} + +{% if user_can(permissions.EDIT_PORTFOLIO_USERS) -%} + {% for member in members -%} + {% if not member.ppoc -%} + {% set invite_pending = member.status == 'invite_pending' %} + {% set invite_expired = member.status == 'invite_expired' %} + + {% set modal_name = "edit_member-{}".format(loop.index) %} + {% call Modal(modal_name, classes="form-content--app-mem") %} +
+

{{ Icon('avatar') }} {{ "portfolios.applications.members.form.edit_access_header" | translate({ "user": member.user_name }) }}

+
+ + + + {% endcall %} + + {% if invite_pending or invite_expired -%} + {% set resend_invite_modal = "resend_invite-{}".format(member.role_id) %} + {% call Modal(resend_invite_modal, classes="form-content--app-mem") %} +
+

{{ "portfolios.applications.members.new.verify" | translate }}

+
+ +
+ {{ member.update_invite_form.csrf_token }} + {{ member_form.SubmitStep( + name=resend_invite_modal, + form=member_form_fields.InfoFields(member.update_invite_form.user_data), + submit_text="Resend Invite", + previous=False, + modal=resend_invite_modal + ) }} +
+ + {% endcall %} + + {% set revoke_invite_modal = "revoke_invite-{}".format(member.role_id) %} + {% call Modal(name=revoke_invite_modal) %} +
+ {{ member.form.csrf_token }} +

{{ "invites.revoke" | translate }}

+
+ {{ "invites.revoke_modal_text" | translate({"application": portfolio.name}) }} +
+ + +
+
+ {% endcall %} + {% else %} + {% set remove_manager_modal = "remove_manager-{}".format(member.role_id) %} + {% call Modal(name=remove_manager_modal, dismissable=False) %} +

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

+
+ {{ + Alert( + title="portfolios.admin.alert_title" | translate, + message="portfolios.admin.alert_message" | translate, + level="warning" + ) + }} +
+
+ {{ member.form.csrf_token }} + +
+ {{ "common.cancel" | translate }} +
+ {% endcall %} + {%- endif %} + {%- endif %} + {%- endfor %} +{%- endif %}

Portfolio Managers

@@ -19,6 +105,14 @@ {% for member in members -%} + {% set invite_pending = member.status == 'invite_pending' %} + {% set invite_expired = member.status == 'invite_expired' %} + {% set current_user = current_member_id == member.role_id %} + {% set perms_modal = "edit_member-{}".format(loop.index) %} + {% set resend_invite_modal = "resend_invite-{}".format(member.role_id) %} + {% set revoke_invite_modal = "revoke_invite-{}".format(member.role_id) %} + {% set remove_manager_modal = "remove_manager-{}".format(member.role_id) %} + {{ member.user_name }}{% if member.role_id == current_member_id %} (You){% endif %} @@ -28,14 +122,33 @@ {% endif %} {{ Label(type=member.status, classes='label--below')}} - + {% for perm, value in member.permission_sets.items() -%} -
- {% if value -%} + {% if value -%} +
{{ ("portfolios.admin.members.{}.{}".format(perm, value)) | translate }} - {%- endif %} -
+
+ {%- endif %} {%-endfor %} + {% if user_can(permissions.EDIT_PORTFOLIO_USERS) -%} + {% call ToggleMenu() %} + + Edit Permissions + + {% if invite_pending or invite_expired -%} + Resend Invite + Revoke Invite + {% else %} + + Remove Manager + + {%- endif %} + {% endcall %} + {%- endif %} {%- endfor %} @@ -60,13 +173,13 @@ form=member_form_fields.InfoFields(new_manager_form.user_data), next_button_text="Next: Permissions", previous=False, - modal=new_manager_modal_name, + modal=new_manager_modal, ), member_form.SubmitStep( name=new_manager_modal, form=member_form_fields.PermsFields(new_manager_form), submit_text="Add Mananger", - modal=new_manager_modal_name, + modal=new_manager_modal, ) ], ) }} diff --git a/templates/portfolios/fragments/primary_point_of_contact.html b/templates/portfolios/fragments/primary_point_of_contact.html deleted file mode 100644 index 3b1fcf2c..00000000 --- a/templates/portfolios/fragments/primary_point_of_contact.html +++ /dev/null @@ -1,25 +0,0 @@ -
-
- {% if g.matchesPath("primary-point-of-contact") %} - {% include "fragments/flash.html" %} - {% endif %} - -

{{ "fragments.ppoc.title" | translate }}

-

{{ "fragments.ppoc.subtitle" | translate }}

- -

- - {{ portfolio.owner.first_name }} - {{ portfolio.owner.last_name }} - -
- {{ portfolio.owner.email }} -
- {{ portfolio.owner.phone_number | usPhone }} -

- - {% if user_can(permissions.EDIT_PORTFOLIO_POC) %} - {% include "portfolios/fragments/change_ppoc.html" %} - {% endif %} -
-
diff --git a/tests/routes/portfolios/test_admin.py b/tests/routes/portfolios/test_admin.py index 65d0f5d1..fee71c7b 100644 --- a/tests/routes/portfolios/test_admin.py +++ b/tests/routes/portfolios/test_admin.py @@ -1,3 +1,4 @@ +import pytest from flask import url_for from unittest.mock import MagicMock @@ -11,29 +12,6 @@ from atst.utils.localization import translate from tests.factories import PortfolioFactory, PortfolioRoleFactory, UserFactory -def test_member_table_access(client, user_session): - admin = UserFactory.create() - portfolio = PortfolioFactory.create(owner=admin) - rando = UserFactory.create() - PortfolioRoleFactory.create( - user=rando, - portfolio=portfolio, - permission_sets=[PermissionSets.get(PermissionSets.VIEW_PORTFOLIO_ADMIN)], - ) - - url = url_for("portfolios.admin", portfolio_id=portfolio.id) - - # editable - user_session(admin) - edit_resp = client.get(url) - assert "