From a5194d6f70849ed26a065095a0af47d7aacfa2b8 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 7 Jan 2020 11:46:17 -0500 Subject: [PATCH 01/14] Remove PPoC files --- templates/portfolios/admin.html | 4 - .../portfolios/fragments/change_ppoc.html | 80 ------------------- .../fragments/primary_point_of_contact.html | 25 ------ 3 files changed, 109 deletions(-) delete mode 100644 templates/portfolios/fragments/change_ppoc.html delete mode 100644 templates/portfolios/fragments/primary_point_of_contact.html 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/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 %} -
-
From 5ba22d82e3bb97ab5a93d806d9676eb7880bdfa8 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 7 Jan 2020 12:00:34 -0500 Subject: [PATCH 02/14] Move toggle menu into its own macro. Use ToggleMenu macro in application team table. --- styles/atat.scss | 1 + styles/components/_portfolio_layout.scss | 49 ------------------- styles/components/_toggle_menu.scss | 49 +++++++++++++++++++ templates/applications/fragments/members.html | 42 ++++++---------- templates/components/toggle_menu.html | 17 +++++++ 5 files changed, 83 insertions(+), 75 deletions(-) create mode 100644 styles/components/_toggle_menu.scss create mode 100644 templates/components/toggle_menu.html 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..beb2f576 100644 --- a/styles/components/_portfolio_layout.scss +++ b/styles/components/_portfolio_layout.scss @@ -154,55 +154,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..a8ef0cbd --- /dev/null +++ b/styles/components/_toggle_menu.scss @@ -0,0 +1,49 @@ +.toggle-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; + white-space: nowrap; + + &:last-child { + border-bottom: 0; + } + + &:hover { + background-color: $color-aqua-lightest; + } + } + } +} diff --git a/templates/applications/fragments/members.html b/templates/applications/fragments/members.html index d6fb7290..662d1145 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, @@ -131,32 +132,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 %} From df285eaa7f225004587a68e54c47faf190d1e0f3 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 7 Jan 2020 12:06:22 -0500 Subject: [PATCH 03/14] Add toggle menu to portfolio managers table. Stub out menu items. --- styles/components/_portfolio_layout.scss | 4 ---- styles/components/_toggle_menu.scss | 4 ++++ templates/applications/fragments/members.html | 2 +- templates/portfolios/fragments/portfolio_members.html | 10 +++++++++- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/styles/components/_portfolio_layout.scss b/styles/components/_portfolio_layout.scss index beb2f576..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 { diff --git a/styles/components/_toggle_menu.scss b/styles/components/_toggle_menu.scss index a8ef0cbd..e05e867e 100644 --- a/styles/components/_toggle_menu.scss +++ b/styles/components/_toggle_menu.scss @@ -3,6 +3,10 @@ top: $gap; right: $gap * 2; + &__container { + position: relative; + } + .accordion-table__item__toggler { padding: $gap / 3; border: 1px solid $color-gray-lighter; diff --git a/templates/applications/fragments/members.html b/templates/applications/fragments/members.html index 662d1145..fcb7ccbc 100644 --- a/templates/applications/fragments/members.html +++ b/templates/applications/fragments/members.html @@ -120,7 +120,7 @@ {% endfor %} - + {% for env in member.environment_roles %}
diff --git a/templates/portfolios/fragments/portfolio_members.html b/templates/portfolios/fragments/portfolio_members.html index ab9413f2..67a365cd 100644 --- a/templates/portfolios/fragments/portfolio_members.html +++ b/templates/portfolios/fragments/portfolio_members.html @@ -5,6 +5,7 @@ {% 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 %}

Portfolio Managers

@@ -28,7 +29,7 @@ {% endif %} {{ Label(type=member.status, classes='label--below')}} - + {% for perm, value in member.permission_sets.items() -%}
{% if value -%} @@ -36,6 +37,13 @@ {%- endif %}
{%-endfor %} + {% if user_can(permissions.EDIT_PORTFOLIO_USERS) -%} + {% call ToggleMenu() %} + Edit Permissions + Resend Invite + Revoke Invite + {% endcall %} + {%- endif %} {%- endfor %} From 05e7dab6732f4b86312b95db4fce4470bc122530 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 7 Jan 2020 16:40:05 -0500 Subject: [PATCH 04/14] Delete ppoc related tests --- .secrets.baseline | 2 +- tests/routes/portfolios/test_admin.py | 32 --------------------------- 2 files changed, 1 insertion(+), 33 deletions(-) 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/tests/routes/portfolios/test_admin.py b/tests/routes/portfolios/test_admin.py index 65d0f5d1..d20843a3 100644 --- a/tests/routes/portfolios/test_admin.py +++ b/tests/routes/portfolios/test_admin.py @@ -11,29 +11,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 " Date: Tue, 7 Jan 2020 16:41:07 -0500 Subject: [PATCH 05/14] Add route to update portfolio manager perms, add modal form to update in the UI --- atst/routes/portfolios/admin.py | 36 +++++++++++-- atst/utils/flash.py | 12 +++++ .../fragments/portfolio_members.html | 30 +++++++++-- tests/routes/portfolios/test_admin.py | 51 +++++++++++++++++++ tests/routes/portfolios/test_invitations.py | 8 +-- tests/test_access.py | 18 +++++++ 6 files changed, 144 insertions(+), 11 deletions(-) diff --git a/atst/routes/portfolios/admin.py b/atst/routes/portfolios/admin.py index 699bdfab..921cd54a 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,6 +30,9 @@ 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 @@ -41,6 +41,7 @@ def filter_perm_sets_data(member): def filter_members_data(members_list, portfolio): members_data = [] for member in members_list: + permission_sets = filter_perm_sets_data(member) members_data.append( { "role_id": member.id, @@ -48,7 +49,7 @@ def filter_members_data(members_list, portfolio): "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 + "form": member_forms.PermissionsForm(permission_sets), } ) @@ -166,3 +167,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/utils/flash.py b/atst/utils/flash.py index da2c9253..3c9eacc1 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -153,6 +153,18 @@ MESSAGES = { "message": "flash.task_order.submitted.message", "category": "success", }, + "update_portfolio_member": { + "title_template": "Success!", + "message_template": """ + You have successfully updated access permissions for {{ member_name }}. + """, + "category": "success", + }, + "update_portfolio_member_error": { + "title_template": "Permissions for {{ member_name }} could not be updated", + "message_template": "An unexpected problem occurred with your request, please try again. If the problem persists, contact an administrator.", + "category": "error", + }, "updated_application_team_settings": { "title": "flash.success", "message": "flash.updated_application_team_settings", diff --git a/templates/portfolios/fragments/portfolio_members.html b/templates/portfolios/fragments/portfolio_members.html index 67a365cd..59bd5d8a 100644 --- a/templates/portfolios/fragments/portfolio_members.html +++ b/templates/portfolios/fragments/portfolio_members.html @@ -7,6 +7,29 @@ {% 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 -%} + {% set modal_name = "edit_member-{}".format(loop.index) %} + {% call Modal(modal_name, classes="form-content--app-mem") %} + + + + + {% endcall %} + {%- endfor %} +{%- endif %} +

Portfolio Managers

@@ -20,6 +43,7 @@ {% for member in members -%} + {% set perms_modal = "edit_member-{}".format(loop.index) %} {{ member.user_name }}{% if member.role_id == current_member_id %} (You){% endif %} @@ -39,7 +63,7 @@ {%-endfor %} {% if user_can(permissions.EDIT_PORTFOLIO_USERS) -%} {% call ToggleMenu() %} - Edit Permissions + Edit Permissions Resend Invite Revoke Invite {% endcall %} @@ -68,13 +92,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/tests/routes/portfolios/test_admin.py b/tests/routes/portfolios/test_admin.py index d20843a3..6adee9fc 100644 --- a/tests/routes/portfolios/test_admin.py +++ b/tests/routes/portfolios/test_admin.py @@ -222,3 +222,54 @@ def test_remove_portfolio_member_ppoc(client, user_session): PortfolioRoles.get(portfolio_id=portfolio.id, user_id=portfolio.owner.id).status == PortfolioRoleStatus.ACTIVE ) + + +def test_portfolios_update_member(client, user_session): + portfolio = PortfolioFactory.create() + portfolio_role = PortfolioRoleFactory.create( + portfolio=portfolio, + permission_sets=[PermissionSets.get(PermissionSets.EDIT_PORTFOLIO_ADMIN)], + ) + + form_data = { + "perms_app_mgmt": "y", + } + + user_session(portfolio.owner) + response = client.post( + url_for( + "portfolios.update_member", + portfolio_id=portfolio.id, + portfolio_role_id=portfolio_role.id, + ), + data=form_data, + follow_redirects=False, + ) + + assert response.status_code == 302 + assert portfolio_role.has_permission_set( + PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT + ) + assert not portfolio_role.has_permission_set(PermissionSets.EDIT_PORTFOLIO_ADMIN) + + +def test_can_not_update_ppoc_permissions(client, user_session): + portfolio = PortfolioFactory.create() + owner = portfolio.owner + + form_data = { + "perms_app_mgmt": "y", + } + + user_session(owner) + response = client.post( + url_for( + "portfolios.update_member", + portfolio_id=portfolio.id, + portfolio_role_id=portfolio.owner_role.id, + ), + data=form_data, + follow_redirects=False, + ) + + assert response.status_code == 400 diff --git a/tests/routes/portfolios/test_invitations.py b/tests/routes/portfolios/test_invitations.py index 44c72460..a5ebbf6a 100644 --- a/tests/routes/portfolios/test_invitations.py +++ b/tests/routes/portfolios/test_invitations.py @@ -269,10 +269,10 @@ def test_existing_member_invite_resent_to_email_submitted_in_form( _DEFAULT_PERMS_FORM_DATA = { - "permission_sets-perms_app_mgmt": False, - "permission_sets-perms_funding": False, - "permission_sets-perms_reporting": False, - "permission_sets-perms_portfolio_mgmt": False, + "permission_sets-perms_app_mgmt": "n", + "permission_sets-perms_funding": "n", + "permission_sets-perms_reporting": "n", + "permission_sets-perms_portfolio_mgmt": "n", } diff --git a/tests/test_access.py b/tests/test_access.py index 8b26010c..375c5e7e 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -373,6 +373,24 @@ def test_portfolios_edit_access(post_url_assert_status): post_url_assert_status(rando, url, 404) +# portfolios.update_member +def test_portfolios_update_member_access(post_url_assert_status): + ccpo = user_with(PermissionSets.EDIT_PORTFOLIO_ADMIN) + owner = user_with() + rando = user_with() + portfolio = PortfolioFactory.create(owner=owner) + portfolio_role = PortfolioRoleFactory.create(portfolio=portfolio) + + url = url_for( + "portfolios.update_member", + portfolio_id=portfolio.id, + portfolio_role_id=portfolio_role.id, + ) + post_url_assert_status(ccpo, url, 302) + post_url_assert_status(owner, url, 302) + post_url_assert_status(rando, url, 404) + + # applications.new def test_applications_new_access(get_url_assert_status): ccpo = user_with(PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT) From 4f345b462fe84b202541f1f768b7d896d632d46d Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 7 Jan 2020 16:54:50 -0500 Subject: [PATCH 06/14] Add resend invite form/modal, update routes and tests as necessary. --- atst/routes/portfolios/admin.py | 39 +++++++++++------ atst/routes/portfolios/invitations.py | 25 ++++++++--- atst/utils/flash.py | 5 +++ .../fragments/portfolio_members.html | 33 +++++++++++++- tests/routes/portfolios/test_invitations.py | 43 ++++--------------- 5 files changed, 90 insertions(+), 55 deletions(-) diff --git a/atst/routes/portfolios/admin.py b/atst/routes/portfolios/admin.py index 921cd54a..c98bd024 100644 --- a/atst/routes/portfolios/admin.py +++ b/atst/routes/portfolios/admin.py @@ -38,20 +38,35 @@ def filter_perm_sets_data(member): return perm_sets_data -def filter_members_data(members_list, portfolio): +def filter_members_data(members_list): members_data = [] for member in members_list: permission_sets = filter_perm_sets_data(member) - 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, - "form": member_forms.PermissionsForm(permission_sets), - } - ) + ppoc = 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: + 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() + ) + invite_token = ( + member.latest_invitation.token + if member.latest_invitation and member.latest_invitation.can_resend + else None + ) + member_data["update_invite_form"] = update_invite_form + member_data["invite_token"] = invite_token + + members_data.append(member_data) return sorted(members_data, key=lambda member: member["user_name"]) @@ -76,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, diff --git a/atst/routes/portfolios/invitations.py b/atst/routes/portfolios/invitations.py index 09a22d1f..54938380 100644 --- a/atst/routes/portfolios/invitations.py +++ b/atst/routes/portfolios/invitations.py @@ -54,13 +54,24 @@ 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 = "{} {}".format( + 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 3c9eacc1..000ff629 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_template": "Portfolio invitation error", + "message_template": "There was an error processing the invitation for {{ user_name }}.", + "category": "error", + }, "revoked_portfolio_access": { "title": "flash.portfolio_member.revoked.title", "message": "flash.portfolio_member.revoked.message", diff --git a/templates/portfolios/fragments/portfolio_members.html b/templates/portfolios/fragments/portfolio_members.html index 59bd5d8a..9ea7794f 100644 --- a/templates/portfolios/fragments/portfolio_members.html +++ b/templates/portfolios/fragments/portfolio_members.html @@ -9,6 +9,9 @@ {% if user_can(permissions.EDIT_PORTFOLIO_USERS) -%} {% for member in members -%} + {% 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") %} {% 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 %} {%- endfor %} {%- endif %} @@ -83,9 +105,11 @@ {% 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) %} @@ -110,6 +134,12 @@ {% if invite_pending or invite_expired -%} Resend Invite Revoke Invite + {% else %} + + Remove Manager + {%- endif %} {% endcall %} {%- endif %} From d550b4108ece69bc84228e148292c2bfac053e09 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 8 Jan 2020 15:55:10 -0500 Subject: [PATCH 10/14] Remove update ppoc route from the blueprint and skip related tests --- atst/routes/portfolios/admin.py | 3 ++- tests/routes/portfolios/test_admin.py | 9 ++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/atst/routes/portfolios/admin.py b/atst/routes/portfolios/admin.py index c98bd024..51c8ff50 100644 --- a/atst/routes/portfolios/admin.py +++ b/atst/routes/portfolios/admin.py @@ -109,7 +109,8 @@ def admin(portfolio_id): return render_admin_page(portfolio) -@portfolios_bp.route("/portfolios//update_ppoc", methods=["POST"]) +# 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): role_id = http_request.form.get("role_id") diff --git a/tests/routes/portfolios/test_admin.py b/tests/routes/portfolios/test_admin.py index 6adee9fc..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 @@ -24,6 +25,7 @@ def test_update_portfolio_name_and_description(client, user_session): assert portfolio.description == "a portfolio for things" +@pytest.mark.skip(reason="Out of scope for MVP") def updating_ppoc_successfully(client, old_ppoc, new_ppoc, portfolio): response = client.post( url_for("portfolios.update_ppoc", portfolio_id=portfolio.id, _external=True), @@ -44,6 +46,7 @@ def updating_ppoc_successfully(client, old_ppoc, new_ppoc, portfolio): assert Permissions.EDIT_PORTFOLIO_POC not in old_ppoc.permissions +@pytest.mark.skip(reason="Out of scope for MVP") def test_update_ppoc_no_user_id_specified(client, user_session): portfolio = PortfolioFactory.create() @@ -57,6 +60,7 @@ def test_update_ppoc_no_user_id_specified(client, user_session): assert response.status_code == 404 +@pytest.mark.skip(reason="Out of scope for MVP") def test_update_ppoc_to_member_not_on_portfolio(client, user_session): portfolio = PortfolioFactory.create() original_ppoc = portfolio.owner @@ -74,6 +78,7 @@ def test_update_ppoc_to_member_not_on_portfolio(client, user_session): assert portfolio.owner.id == original_ppoc.id +@pytest.mark.skip(reason="Out of scope for MVP") def test_update_ppoc_when_ppoc(client, user_session): portfolio = PortfolioFactory.create() original_ppoc = portfolio.owner_role @@ -90,7 +95,8 @@ def test_update_ppoc_when_ppoc(client, user_session): ) -def test_update_ppoc_when_cpo(client, user_session): +@pytest.mark.skip(reason="Out of scope for MVP") +def test_update_ppoc_when_ccpo(client, user_session): ccpo = UserFactory.create_ccpo() portfolio = PortfolioFactory.create() original_ppoc = portfolio.owner_role @@ -107,6 +113,7 @@ def test_update_ppoc_when_cpo(client, user_session): ) +@pytest.mark.skip(reason="Out of scope for MVP") def test_update_ppoc_when_not_ppoc(client, user_session): portfolio = PortfolioFactory.create() new_owner = UserFactory.create() From 5036504ae2e1dc8046220204ab2c5827f54d1855 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 14 Jan 2020 06:37:39 -0500 Subject: [PATCH 11/14] Skip coverage on unused route --- atst/routes/portfolios/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atst/routes/portfolios/admin.py b/atst/routes/portfolios/admin.py index 51c8ff50..dd386600 100644 --- a/atst/routes/portfolios/admin.py +++ b/atst/routes/portfolios/admin.py @@ -112,7 +112,7 @@ def admin(portfolio_id): # 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): +def update_ppoc(portfolio_id): # pragma: no cover role_id = http_request.form.get("role_id") portfolio = Portfolios.get(g.current_user, portfolio_id) From 0c733dd365d9d62d4109f81c36712addf933c5d4 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 15 Jan 2020 13:49:08 -0500 Subject: [PATCH 12/14] Update display of PPoC and remove option to edit PPoC perms --- atst/routes/portfolios/admin.py | 4 +- .../fragments/portfolio_members.html | 136 +++++++++--------- 2 files changed, 74 insertions(+), 66 deletions(-) diff --git a/atst/routes/portfolios/admin.py b/atst/routes/portfolios/admin.py index dd386600..ff9a4a11 100644 --- a/atst/routes/portfolios/admin.py +++ b/atst/routes/portfolios/admin.py @@ -42,7 +42,9 @@ def filter_members_data(members_list): members_data = [] for member in members_list: permission_sets = filter_perm_sets_data(member) - ppoc = PermissionSets.PORTFOLIO_POC in member.permission_sets + ppoc = ( + PermissionSets.get(PermissionSets.PORTFOLIO_POC) in member.permission_sets + ) member_data = { "role_id": member.id, "user_name": member.user_name, diff --git a/templates/portfolios/fragments/portfolio_members.html b/templates/portfolios/fragments/portfolio_members.html index 418fa4fc..0e171729 100644 --- a/templates/portfolios/fragments/portfolio_members.html +++ b/templates/portfolios/fragments/portfolio_members.html @@ -9,83 +9,85 @@ {% if user_can(permissions.EDIT_PORTFOLIO_USERS) -%} {% for member in members -%} - {% set invite_pending = member.status == 'invite_pending' %} - {% set invite_expired = member.status == 'invite_expired' %} + {% 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") %} - - - - - {% 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") %} + {% set modal_name = "edit_member-{}".format(loop.index) %} + {% call Modal(modal_name, classes="form-content--app-mem") %} - -
- {{ member.update_invite_form.csrf_token }} + + + {{ member.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", + name=modal_name, + form=member_form_fields.PermsFields(member.form, member_role_id=member.role_id), + submit_text="Save Changes", previous=False, - modal=resend_invite_modal + modal=modal_name, ) }}
{% 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}) }} -
- - + {% 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") %} + - - {% 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.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}) }} +
+ + +
- {{ "common.cancel" | translate }} -
- {% endcall %} + {% 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 %} @@ -130,7 +132,11 @@ {%-endfor %} {% if user_can(permissions.EDIT_PORTFOLIO_USERS) -%} {% call ToggleMenu() %} - Edit Permissions + + Edit Permissions + {% if invite_pending or invite_expired -%} Resend Invite Revoke Invite From d154b90c05e1924f81c2801849457f307cdef970 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 15 Jan 2020 13:49:33 -0500 Subject: [PATCH 13/14] Use translations in flash messages --- atst/utils/flash.py | 14 ++++++-------- translations.yaml | 9 +++++++++ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/atst/utils/flash.py b/atst/utils/flash.py index 000ff629..7a1cf4ce 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -129,8 +129,8 @@ MESSAGES = { "category": "success", }, "resend_portfolio_invitation_error": { - "title_template": "Portfolio invitation error", - "message_template": "There was an error processing the invitation for {{ user_name }}.", + "title": "flash.portfolio_invite.error.title", + "message": "flash.portfolio_invite.error.message", "category": "error", }, "revoked_portfolio_access": { @@ -159,15 +159,13 @@ MESSAGES = { "category": "success", }, "update_portfolio_member": { - "title_template": "Success!", - "message_template": """ - You have successfully updated access permissions for {{ member_name }}. - """, + "title": "flash.portfolio_member.update.title", + "message": "flash.portfolio_member.update.message", "category": "success", }, "update_portfolio_member_error": { - "title_template": "Permissions for {{ member_name }} could not be updated", - "message_template": "An unexpected problem occurred with your request, please try again. If the problem persists, contact an administrator.", + "title": "flash.portfolio_member.update_error.title", + "message": "flash.portfolio_member.update_error.message", "category": "error", }, "updated_application_team_settings": { diff --git a/translations.yaml b/translations.yaml index 99e2d3b0..f4a1a3fb 100644 --- a/translations.yaml +++ b/translations.yaml @@ -169,10 +169,19 @@ flash: revoked: title: Removed portfolio access message: Portfolio access successfully removed from {member_name}. + update: + title: Success! + message: You have successfully updated access permissions for {member_name}. + update_error: + title: Permissions for {member_name} could not be updated + message: An unexpected problem occurred with your request, please try again. If the problem persists, contact an administrator. portfolio_invite: resent: title: Invitation resent message: Successfully sent a new invitation to {user_name}. + error: + title: Portfolio invitation error + message: There was an error processing the invitation for {user_name}. session_expired: title: Session Expired message: Your session expired due to inactivity. Please log in again to continue. From 98065710014ba24617446134961f6aa52820bd84 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 16 Jan 2020 10:50:03 -0500 Subject: [PATCH 14/14] Refactoring: - Set dict values directly instead of creating a variable - Comment out unused route function entirely - Use f-strings for string interpolation - Move div inside if statement so empty divs are not printed --- atst/routes/portfolios/admin.py | 44 +++++++++---------- atst/routes/portfolios/invitations.py | 4 +- .../fragments/portfolio_members.html | 8 ++-- 3 files changed, 26 insertions(+), 30 deletions(-) diff --git a/atst/routes/portfolios/admin.py b/atst/routes/portfolios/admin.py index ff9a4a11..4318a47c 100644 --- a/atst/routes/portfolios/admin.py +++ b/atst/routes/portfolios/admin.py @@ -55,18 +55,16 @@ def filter_members_data(members_list): } if not ppoc: - update_invite_form = ( + 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() ) - invite_token = ( + member_data["invite_token"] = ( member.latest_invitation.token if member.latest_invitation and member.latest_invitation.can_resend else None ) - member_data["update_invite_form"] = update_invite_form - member_data["invite_token"] = invite_token members_data.append(member_data) @@ -113,25 +111,25 @@ def admin(portfolio_id): # 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", - ) - ) +# @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"]) diff --git a/atst/routes/portfolios/invitations.py b/atst/routes/portfolios/invitations.py index 54938380..7c48593f 100644 --- a/atst/routes/portfolios/invitations.py +++ b/atst/routes/portfolios/invitations.py @@ -67,9 +67,7 @@ def resend_invitation(portfolio_id, portfolio_token): ) flash("resend_portfolio_invitation", user_name=invite.user_name) else: - user_name = "{} {}".format( - form["user_data"]["first_name"].data, form["user_data"]["last_name"].data - ) + 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( diff --git a/templates/portfolios/fragments/portfolio_members.html b/templates/portfolios/fragments/portfolio_members.html index 0e171729..91e99d24 100644 --- a/templates/portfolios/fragments/portfolio_members.html +++ b/templates/portfolios/fragments/portfolio_members.html @@ -124,11 +124,11 @@ {% 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() %}