rename and rearrange some portfolio route functions

atst/routes/__init__.py

@@ -71,9 +71,7 @@ def home():
         ]
 
         if is_portfolio_owner:
-            return redirect(
+            return redirect(url_for("portfolios.reports", portfolio_id=portfolio_id))
-                url_for("portfolios.portfolio_reports", portfolio_id=portfolio_id)
-            )
         else:
             return redirect(
                 url_for(

atst/routes/portfolios/__init__.py

@@ -6,10 +6,7 @@ portfolios_bp = Blueprint("portfolios", __name__)
 from . import index
 from . import members
 from . import invitations
-from atst.domain.exceptions import UnauthorizedError
+from . import admin
-from atst.domain.portfolios import Portfolios
-from atst.domain.authz import Authorization
-from atst.models.permissions import Permissions
 from atst.utils.context_processors import portfolio as portfolio_context_processor
 
 

atst/routes/portfolios/admin.py

@@ -0,0 +1,192 @@
+from flask import render_template, request as http_request, g, redirect, url_for
+
+from . import portfolios_bp
+from atst.domain.portfolios import Portfolios
+from atst.domain.portfolio_roles import PortfolioRoles
+from atst.domain.permission_sets import PermissionSets
+from atst.domain.users import Users
+from atst.domain.audit_log import AuditLog
+from atst.domain.common import Paginator
+from atst.domain.exceptions import NotFoundError
+from atst.forms.portfolio import PortfolioForm
+import atst.forms.portfolio_member as member_forms
+from atst.models.permissions import Permissions
+from atst.domain.authz.decorator import user_can_access_decorator as user_can
+from atst.utils.flash import formatted_flash as flash
+from atst.domain.exceptions import UnauthorizedError
+
+
+def permission_str(member, edit_perm_set, view_perm_set):
+    if member.has_permission_set(edit_perm_set):
+        return edit_perm_set
+    else:
+        return view_perm_set
+
+
+def serialize_member_form_data(member):
+    return {
+        "member": member.user.full_name,
+        "user_id": member.user_id,
+        "perms_app_mgmt": permission_str(
+            member,
+            PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT,
+            PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT,
+        ),
+        "perms_funding": permission_str(
+            member,
+            PermissionSets.EDIT_PORTFOLIO_FUNDING,
+            PermissionSets.VIEW_PORTFOLIO_FUNDING,
+        ),
+        "perms_reporting": permission_str(
+            member,
+            PermissionSets.EDIT_PORTFOLIO_REPORTS,
+            PermissionSets.VIEW_PORTFOLIO_REPORTS,
+        ),
+        "perms_portfolio_mgmt": permission_str(
+            member,
+            PermissionSets.EDIT_PORTFOLIO_ADMIN,
+            PermissionSets.VIEW_PORTFOLIO_ADMIN,
+        ),
+    }
+
+
+def get_members_data(portfolio):
+    members = [serialize_member_form_data(member) for member in portfolio.members]
+    for member in members:
+        if member["user_id"] == portfolio.owner.id:
+            ppoc = member
+            members.remove(member)
+    members.insert(0, ppoc)
+    return members
+
+
+def render_admin_page(portfolio, form=None):
+    pagination_opts = Paginator.get_pagination_opts(http_request)
+    audit_events = AuditLog.get_portfolio_events(portfolio, pagination_opts)
+    members_data = get_members_data(portfolio)
+    portfolio_form = PortfolioForm(data={"name": portfolio.name})
+    member_perms_form = member_forms.MembersPermissionsForm(
+        data={"members_permissions": members_data}
+    )
+
+    assign_ppoc_form = member_forms.AssignPPOCForm()
+    assign_ppoc_form.user_id.choices += [
+        (user.id, user.full_name) for user in portfolio.users if user != portfolio.owner
+    ]
+
+    return render_template(
+        "portfolios/admin.html",
+        form=form,
+        portfolio_form=portfolio_form,
+        member_perms_form=member_perms_form,
+        member_form=member_forms.NewForm(),
+        assign_ppoc_form=assign_ppoc_form,
+        portfolio=portfolio,
+        audit_events=audit_events,
+        user=g.current_user,
+        members_data=members_data,
+    )
+
+
+@portfolios_bp.route("/portfolios/<portfolio_id>/admin")
+@user_can(Permissions.VIEW_PORTFOLIO_ADMIN, message="view portfolio admin page")
+def admin(portfolio_id):
+    portfolio = Portfolios.get_for_update(portfolio_id)
+    return render_admin_page(portfolio)
+
+
+@portfolios_bp.route("/portfolios/<portfolio_id>/admin", methods=["POST"])
+@user_can(Permissions.EDIT_PORTFOLIO_USERS, message="view portfolio admin page")
+def edit_members(portfolio_id):
+    portfolio = Portfolios.get_for_update(portfolio_id)
+    member_perms_form = member_forms.MembersPermissionsForm(http_request.form)
+
+    if member_perms_form.validate():
+        for subform in member_perms_form.members_permissions:
+            user_id = subform.user_id.data
+            member = Users.get(user_id=user_id)
+            if member is not portfolio.owner:
+                new_perm_set = subform.data["permission_sets"]
+                portfolio_role = PortfolioRoles.get(portfolio.id, user_id)
+                PortfolioRoles.update(portfolio_role, new_perm_set)
+
+        flash("update_portfolio_members", portfolio=portfolio)
+
+        return redirect(
+            url_for(
+                "portfolios.admin",
+                portfolio_id=portfolio_id,
+                fragment="portfolio-members",
+                _anchor="portfolio-members",
+            )
+        )
+    else:
+        return render_admin_page(portfolio)
+
+
+@portfolios_bp.route("/portfolios/<portfolio_id>/update_ppoc", methods=["POST"])
+@user_can(Permissions.EDIT_PORTFOLIO_POC, message="update portfolio ppoc")
+def update_ppoc(portfolio_id):
+    user_id = http_request.form.get("user_id")
+
+    portfolio = Portfolios.get(g.current_user, portfolio_id)
+    new_ppoc = Users.get(user_id)
+
+    if new_ppoc not in portfolio.users:
+        raise NotFoundError("user not in portfolio")
+
+    portfolio_role = PortfolioRoles.get(portfolio_id=portfolio_id, user_id=user_id)
+    PortfolioRoles.make_ppoc(portfolio_role=portfolio_role)
+
+    flash("primary_point_of_contact_changed", ppoc_name=new_ppoc.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/<portfolio_id>/edit", methods=["POST"])
+@user_can(Permissions.EDIT_PORTFOLIO_NAME, message="edit portfolio")
+def edit(portfolio_id):
+    portfolio = Portfolios.get_for_update(portfolio_id)
+    form = PortfolioForm(http_request.form)
+    if form.validate():
+        Portfolios.update(portfolio, form.data)
+        return redirect(
+            url_for("applications.portfolio_applications", portfolio_id=portfolio.id)
+        )
+    else:
+        # rerender portfolio admin page
+        return render_admin_page(portfolio, form)
+
+
+@portfolios_bp.route(
+    "/portfolios/<portfolio_id>/members/<user_id>/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:
+        raise UnauthorizedError(
+            g.current_user, "you cant remove yourself 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)
+
+    flash("portfolio_member_removed", member_name=portfolio_role.user.full_name)
+
+    return redirect(
+        url_for(
+            "portfolios.admin",
+            portfolio_id=portfolio_id,
+            _anchor="portfolio-members",
+            fragment="portfolio-members",
+        )
+    )

atst/routes/portfolios/index.py

@@ -5,18 +5,8 @@ from flask import render_template, request as http_request, g, redirect, url_for
 from . import portfolios_bp
 from atst.domain.reports import Reports
 from atst.domain.portfolios import Portfolios
-from atst.domain.portfolio_roles import PortfolioRoles
-from atst.domain.permission_sets import PermissionSets
-from atst.domain.users import Users
-from atst.domain.audit_log import AuditLog
-from atst.domain.common import Paginator
-from atst.domain.exceptions import NotFoundError
-from atst.forms.portfolio import PortfolioForm
-import atst.forms.portfolio_member as member_forms
 from atst.models.permissions import Permissions
 from atst.domain.authz.decorator import user_can_access_decorator as user_can
-from atst.utils.flash import formatted_flash as flash
-from atst.domain.exceptions import UnauthorizedError
 
 
 @portfolios_bp.route("/portfolios")
@@ -29,155 +19,6 @@ def portfolios():
         return render_template("portfolios/blank_slate.html")
 
 
-def permission_str(member, edit_perm_set, view_perm_set):
-    if member.has_permission_set(edit_perm_set):
-        return edit_perm_set
-    else:
-        return view_perm_set
-
-
-def serialize_member_form_data(member):
-    return {
-        "member": member.user.full_name,
-        "user_id": member.user_id,
-        "perms_app_mgmt": permission_str(
-            member,
-            PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT,
-            PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT,
-        ),
-        "perms_funding": permission_str(
-            member,
-            PermissionSets.EDIT_PORTFOLIO_FUNDING,
-            PermissionSets.VIEW_PORTFOLIO_FUNDING,
-        ),
-        "perms_reporting": permission_str(
-            member,
-            PermissionSets.EDIT_PORTFOLIO_REPORTS,
-            PermissionSets.VIEW_PORTFOLIO_REPORTS,
-        ),
-        "perms_portfolio_mgmt": permission_str(
-            member,
-            PermissionSets.EDIT_PORTFOLIO_ADMIN,
-            PermissionSets.VIEW_PORTFOLIO_ADMIN,
-        ),
-    }
-
-
-def get_members_data(portfolio):
-    members = [serialize_member_form_data(member) for member in portfolio.members]
-    for member in members:
-        if member["user_id"] == portfolio.owner.id:
-            ppoc = member
-            members.remove(member)
-    members.insert(0, ppoc)
-    return members
-
-
-def render_admin_page(portfolio, form=None):
-    pagination_opts = Paginator.get_pagination_opts(http_request)
-    audit_events = AuditLog.get_portfolio_events(portfolio, pagination_opts)
-    members_data = get_members_data(portfolio)
-    portfolio_form = PortfolioForm(data={"name": portfolio.name})
-    member_perms_form = member_forms.MembersPermissionsForm(
-        data={"members_permissions": members_data}
-    )
-
-    assign_ppoc_form = member_forms.AssignPPOCForm()
-    assign_ppoc_form.user_id.choices += [
-        (user.id, user.full_name) for user in portfolio.users if user != portfolio.owner
-    ]
-
-    return render_template(
-        "portfolios/admin.html",
-        form=form,
-        portfolio_form=portfolio_form,
-        member_perms_form=member_perms_form,
-        member_form=member_forms.NewForm(),
-        assign_ppoc_form=assign_ppoc_form,
-        portfolio=portfolio,
-        audit_events=audit_events,
-        user=g.current_user,
-        members_data=members_data,
-    )
-
-
-@portfolios_bp.route("/portfolios/<portfolio_id>/admin")
-@user_can(Permissions.VIEW_PORTFOLIO_ADMIN, message="view portfolio admin page")
-def portfolio_admin(portfolio_id):
-    portfolio = Portfolios.get_for_update(portfolio_id)
-    return render_admin_page(portfolio)
-
-
-@portfolios_bp.route("/portfolios/<portfolio_id>/admin", methods=["POST"])
-@user_can(Permissions.EDIT_PORTFOLIO_USERS, message="view portfolio admin page")
-def edit_portfolio_members(portfolio_id):
-    portfolio = Portfolios.get_for_update(portfolio_id)
-    member_perms_form = member_forms.MembersPermissionsForm(http_request.form)
-
-    if member_perms_form.validate():
-        for subform in member_perms_form.members_permissions:
-            user_id = subform.user_id.data
-            member = Users.get(user_id=user_id)
-            if member is not portfolio.owner:
-                new_perm_set = subform.data["permission_sets"]
-                portfolio_role = PortfolioRoles.get(portfolio.id, user_id)
-                PortfolioRoles.update(portfolio_role, new_perm_set)
-
-        flash("update_portfolio_members", portfolio=portfolio)
-
-        return redirect(
-            url_for(
-                "portfolios.portfolio_admin",
-                portfolio_id=portfolio_id,
-                fragment="portfolio-members",
-                _anchor="portfolio-members",
-            )
-        )
-    else:
-        return render_admin_page(portfolio)
-
-
-@portfolios_bp.route("/portfolios/<portfolio_id>/update_ppoc", methods=["POST"])
-@user_can(Permissions.EDIT_PORTFOLIO_POC, message="update portfolio ppoc")
-def update_ppoc(portfolio_id):
-    user_id = http_request.form.get("user_id")
-
-    portfolio = Portfolios.get(g.current_user, portfolio_id)
-    new_ppoc = Users.get(user_id)
-
-    if new_ppoc not in portfolio.users:
-        raise NotFoundError("user not in portfolio")
-
-    portfolio_role = PortfolioRoles.get(portfolio_id=portfolio_id, user_id=user_id)
-    PortfolioRoles.make_ppoc(portfolio_role=portfolio_role)
-
-    flash("primary_point_of_contact_changed", ppoc_name=new_ppoc.full_name)
-
-    return redirect(
-        url_for(
-            "portfolios.portfolio_admin",
-            portfolio_id=portfolio.id,
-            fragment="primary-point-of-contact",
-            _anchor="primary-point-of-contact",
-        )
-    )
-
-
-@portfolios_bp.route("/portfolios/<portfolio_id>/edit", methods=["POST"])
-@user_can(Permissions.EDIT_PORTFOLIO_NAME, message="edit portfolio")
-def edit_portfolio(portfolio_id):
-    portfolio = Portfolios.get_for_update(portfolio_id)
-    form = PortfolioForm(http_request.form)
-    if form.validate():
-        Portfolios.update(portfolio, form.data)
-        return redirect(
-            url_for("applications.portfolio_applications", portfolio_id=portfolio.id)
-        )
-    else:
-        # rerender portfolio admin page
-        return render_admin_page(portfolio, form)
-
-
 @portfolios_bp.route("/portfolios/<portfolio_id>")
 @user_can(Permissions.VIEW_PORTFOLIO, message="view portfolio")
 def show_portfolio(portfolio_id):
@@ -188,7 +29,7 @@ def show_portfolio(portfolio_id):
 
 @portfolios_bp.route("/portfolios/<portfolio_id>/reports")
 @user_can(Permissions.VIEW_PORTFOLIO_REPORTS, message="view portfolio reports")
-def portfolio_reports(portfolio_id):
+def reports(portfolio_id):
     portfolio = Portfolios.get(g.current_user, portfolio_id)
     today = date.today()
     month = http_request.args.get("month", today.month)
@@ -220,30 +61,3 @@ def portfolio_reports(portfolio_id):
         expiration_date=expiration_date,
         remaining_days=remaining_days,
     )
-
-
-@portfolios_bp.route(
-    "/portfolios/<portfolio_id>/members/<user_id>/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:
-        raise UnauthorizedError(
-            g.current_user, "you cant remove yourself 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)
-
-    flash("portfolio_member_removed", member_name=portfolio_role.user.full_name)
-
-    return redirect(
-        url_for(
-            "portfolios.portfolio_admin",
-            portfolio_id=portfolio_id,
-            _anchor="portfolio-members",
-            fragment="portfolio-members",
-        )
-    )

atst/routes/portfolios/invitations.py

@@ -41,7 +41,7 @@ def revoke_invitation(portfolio_id, token):
 
     return redirect(
         url_for(
-            "portfolios.portfolio_admin",
+            "portfolios.admin",
             portfolio_id=portfolio_id,
             _anchor="portfolio-members",
             fragment="portfolio-members",
@@ -59,7 +59,7 @@ def resend_invitation(portfolio_id, token):
     flash("resend_portfolio_invitation", user_name=invite.user_name)
     return redirect(
         url_for(
-            "portfolios.portfolio_admin",
+            "portfolios.admin",
             portfolio_id=portfolio_id,
             fragment="portfolio-members",
             _anchor="portfolio-members",

atst/routes/portfolios/members.py

@@ -52,7 +52,7 @@ def create_member(portfolio_id):
 
     return redirect(
         url_for(
-            "portfolios.portfolio_admin",
+            "portfolios.admin",
             portfolio_id=portfolio_id,
             fragment="portfolio-members",
             _anchor="portfolio-members",

templates/fragments/admin/portfolio_members.html

@@ -11,7 +11,7 @@
     {% if g.matchesPath("portfolio-members") %}
       {% include "fragments/flash.html" %}
     {% endif %}
-      <form method='POST' id="member-perms" action='{{ url_for("portfolios.edit_portfolio_members", portfolio_id=portfolio.id) }}' autocomplete="off" enctype="multipart/form-data">
+      <form method='POST' id="member-perms" action='{{ url_for("portfolios.edit_members", portfolio_id=portfolio.id) }}' autocomplete="off" enctype="multipart/form-data">
       {{ member_perms_form.csrf_token }}
 
       <div class='application-list-item'>

templates/portfolios/admin.html

@@ -17,7 +17,7 @@
 
         {% if user_can(permissions.VIEW_PORTFOLIO_NAME) %}
         <base-form inline-template>
-          <form method="POST" action="{{ url_for('portfolios.edit_portfolio', portfolio_id=portfolio.id) }}" autocomplete="false">
+          <form method="POST" action="{{ url_for('portfolios.edit', portfolio_id=portfolio.id) }}" autocomplete="false">
             {{ portfolio_form.csrf_token }}
             <div class='form-row'>
               <div class='form-col form-col--half'>
@@ -54,7 +54,7 @@
 
     {% if user_can(permissions.VIEW_PORTFOLIO_ACTIVITY_LOG) %}
       {% include "fragments/audit_events_log.html" %}
-      {{ Pagination(audit_events, 'portfolios.portfolio_admin', portfolio_id=portfolio.id) }}
+      {{ Pagination(audit_events, 'portfolios.admin', portfolio_id=portfolio.id) }}
     {% endif %}
   </div>
 {% endblock %}

templates/portfolios/header.html

@@ -60,8 +60,8 @@
       {{ Link(
         icon='chart-pie',
         text='navigation.portfolio_navigation.breadcrumbs.reports' | translate,
-        url=url_for("portfolios.portfolio_reports", portfolio_id=portfolio.id),
+        url=url_for("portfolios.reports", portfolio_id=portfolio.id),
-        active=request.url_rule.endpoint == "portfolios.portfolio_reports",
+        active=request.url_rule.endpoint == "portfolios.reports",
       ) }}
     {% endif %}
     {{ Link(
@@ -74,8 +74,8 @@
       {{ Link(
         icon='cog',
         text='navigation.portfolio_navigation.breadcrumbs.admin' | translate,
-        url=url_for("portfolios.portfolio_admin", portfolio_id=portfolio.id),
+        url=url_for("portfolios.admin", portfolio_id=portfolio.id),
-        active=request.url_rule.endpoint == "portfolios.portfolio_admin",
+        active=request.url_rule.endpoint == "portfolios.admin",
       ) }}
     {% endif %}
   </div>

templates/portfolios/reports/index.html

@@ -130,7 +130,7 @@
   {% set current_month_index = current_month.strftime('%m/%Y') %}
   {% set prev_month_index = prev_month.strftime('%m/%Y') %}
   {% set two_months_ago_index = two_months_ago.strftime('%m/%Y') %}
-  {% set reports_url = url_for("portfolios.portfolio_reports", portfolio_id=portfolio.id) %}
+  {% set reports_url = url_for("portfolios.reports", portfolio_id=portfolio.id) %}
 
   {% if not portfolio.applications %}
 
@@ -357,7 +357,7 @@
               {% if month.month == current_month.month and month.year == current_month.year %}
                 selected='selected'
               {% endif %}
-              value='{{ url_for("portfolios.portfolio_reports",
+              value='{{ url_for("portfolios.reports",
                                 portfolio_id=portfolio.id,
                                 month=month.month,
                                 year=month.year) }}'

tests/routes/applications/test_index.py

@@ -23,7 +23,7 @@ def test_user_with_permission_has_budget_report_link(client, user_session):
         url_for("applications.portfolio_applications", portfolio_id=portfolio.id)
     )
     assert (
-        url_for("portfolios.portfolio_reports", portfolio_id=portfolio.id)
+        url_for("portfolios.reports", portfolio_id=portfolio.id)
         in response.data.decode()
     )
 
@@ -39,7 +39,7 @@ def test_user_without_permission_has_no_budget_report_link(client, user_session)
         url_for("applications.portfolio_applications", portfolio_id=portfolio.id)
     )
     assert (
-        url_for("portfolios.portfolio_reports", portfolio_id=portfolio.id)
+        url_for("portfolios.reports", portfolio_id=portfolio.id)
         not in response.data.decode()
     )
 

tests/routes/portfolios/test_admin.py

@@ -2,6 +2,10 @@ from flask import url_for
 
 from atst.domain.permission_sets import PermissionSets
 from atst.domain.portfolio_roles import PortfolioRoles
+from atst.domain.portfolios import Portfolios
+from atst.models.permissions import Permissions
+from atst.models.portfolio_role import Status as PortfolioRoleStatus
+from atst.utils.localization import translate
 
 from tests.factories import PortfolioFactory, PortfolioRoleFactory, UserFactory
 
@@ -16,7 +20,7 @@ def test_member_table_access(client, user_session):
         permission_sets=[PermissionSets.get(PermissionSets.VIEW_PORTFOLIO_ADMIN)],
     )
 
-    url = url_for("portfolios.portfolio_admin", portfolio_id=portfolio.id)
+    url = url_for("portfolios.admin", portfolio_id=portfolio.id)
 
     # editable
     user_session(admin)
@@ -57,7 +61,7 @@ def test_update_member_permissions(client, user_session):
     }
 
     response = client.post(
-        url_for("portfolios.edit_portfolio_members", portfolio_id=portfolio.id),
+        url_for("portfolios.edit_members", portfolio_id=portfolio.id),
         data=form_data,
         follow_redirects=True,
     )
@@ -94,7 +98,7 @@ def test_no_update_member_permissions_without_edit_access(client, user_session):
     }
 
     response = client.post(
-        url_for("portfolios.edit_portfolio_members", portfolio_id=portfolio.id),
+        url_for("portfolios.edit_members", portfolio_id=portfolio.id),
         data=form_data,
         follow_redirects=True,
     )
@@ -125,7 +129,7 @@ def test_rerender_admin_page_if_member_perms_form_does_not_validate(
     }
 
     response = client.post(
-        url_for("portfolios.edit_portfolio_members", portfolio_id=portfolio.id),
+        url_for("portfolios.edit_members", portfolio_id=portfolio.id),
         data=form_data,
         follow_redirects=True,
     )
@@ -153,10 +157,203 @@ def test_cannot_update_portfolio_ppoc_perms(client, user_session):
     }
 
     response = client.post(
-        url_for("portfolios.edit_portfolio_members", portfolio_id=portfolio.id),
+        url_for("portfolios.edit_members", portfolio_id=portfolio.id),
         data=member_perms_data,
         follow_redirects=True,
     )
 
     assert response.status_code == 404
     assert ppoc_pf_role.has_permission_set(PermissionSets.PORTFOLIO_POC)
+
+
+def test_update_portfolio_name(client, user_session):
+    portfolio = PortfolioFactory.create()
+    user_session(portfolio.owner)
+    response = client.post(
+        url_for("portfolios.edit", portfolio_id=portfolio.id),
+        data={"name": "a cool new name"},
+        follow_redirects=True,
+    )
+    assert response.status_code == 200
+    assert portfolio.name == "a cool new name"
+
+
+def updating_ppoc_successfully(client, old_ppoc, new_ppoc, portfolio):
+    response = client.post(
+        url_for("portfolios.update_ppoc", portfolio_id=portfolio.id, _external=True),
+        data={"user_id": new_ppoc.id},
+        follow_redirects=False,
+    )
+
+    assert response.status_code == 302
+    assert response.headers["Location"] == url_for(
+        "portfolios.admin",
+        portfolio_id=portfolio.id,
+        fragment="primary-point-of-contact",
+        _anchor="primary-point-of-contact",
+        _external=True,
+    )
+    assert portfolio.owner.id == new_ppoc.id
+    assert (
+        Permissions.EDIT_PORTFOLIO_POC
+        in PortfolioRoles.get(
+            portfolio_id=portfolio.id, user_id=new_ppoc.id
+        ).permissions
+    )
+    assert (
+        Permissions.EDIT_PORTFOLIO_POC
+        not in PortfolioRoles.get(portfolio.id, old_ppoc.id).permissions
+    )
+
+
+def test_update_ppoc_no_user_id_specified(client, user_session):
+    portfolio = PortfolioFactory.create()
+
+    user_session(portfolio.owner)
+
+    response = client.post(
+        url_for("portfolios.update_ppoc", portfolio_id=portfolio.id, _external=True),
+        follow_redirects=False,
+    )
+
+    assert response.status_code == 404
+
+
+def test_update_ppoc_to_member_not_on_portfolio(client, user_session):
+    portfolio = PortfolioFactory.create()
+    original_ppoc = portfolio.owner
+    non_portfolio_member = UserFactory.create()
+
+    user_session(original_ppoc)
+
+    response = client.post(
+        url_for("portfolios.update_ppoc", portfolio_id=portfolio.id, _external=True),
+        data={"user_id": non_portfolio_member.id},
+        follow_redirects=False,
+    )
+
+    assert response.status_code == 404
+    assert portfolio.owner.id == original_ppoc.id
+
+
+def test_update_ppoc_when_ppoc(client, user_session):
+    portfolio = PortfolioFactory.create()
+    original_ppoc = portfolio.owner
+    new_ppoc = UserFactory.create()
+    Portfolios.add_member(
+        member=new_ppoc,
+        portfolio=portfolio,
+        permission_sets=[PermissionSets.VIEW_PORTFOLIO],
+    )
+
+    user_session(original_ppoc)
+
+    updating_ppoc_successfully(
+        client=client, new_ppoc=new_ppoc, old_ppoc=original_ppoc, portfolio=portfolio
+    )
+
+
+def test_update_ppoc_when_cpo(client, user_session):
+    ccpo = UserFactory.create_ccpo()
+    portfolio = PortfolioFactory.create()
+    original_ppoc = portfolio.owner
+    new_ppoc = UserFactory.create()
+    Portfolios.add_member(
+        member=new_ppoc,
+        portfolio=portfolio,
+        permission_sets=[PermissionSets.VIEW_PORTFOLIO],
+    )
+
+    user_session(ccpo)
+
+    updating_ppoc_successfully(
+        client=client, new_ppoc=new_ppoc, old_ppoc=original_ppoc, portfolio=portfolio
+    )
+
+
+def test_update_ppoc_when_not_ppoc(client, user_session):
+    portfolio = PortfolioFactory.create()
+    new_owner = UserFactory.create()
+
+    user_session(new_owner)
+
+    response = client.post(
+        url_for("portfolios.update_ppoc", portfolio_id=portfolio.id, _external=True),
+        data={"dod_id": new_owner.dod_id},
+        follow_redirects=False,
+    )
+
+    assert response.status_code == 404
+
+
+def test_portfolio_admin_screen_when_ppoc(client, user_session):
+    portfolio = PortfolioFactory.create()
+    user_session(portfolio.owner)
+    response = client.get(url_for("portfolios.admin", portfolio_id=portfolio.id))
+    assert response.status_code == 200
+    assert portfolio.name in response.data.decode()
+    assert translate("fragments.ppoc.update_btn").encode("utf8") in response.data
+
+
+def test_portfolio_admin_screen_when_not_ppoc(client, user_session):
+    portfolio = PortfolioFactory.create()
+    user = UserFactory.create()
+    permission_sets = PermissionSets.get_many(
+        [PermissionSets.EDIT_PORTFOLIO_ADMIN, PermissionSets.VIEW_PORTFOLIO_ADMIN]
+    )
+    PortfolioRoleFactory.create(
+        portfolio=portfolio, user=user, permission_sets=permission_sets
+    )
+    user_session(user)
+    response = client.get(url_for("portfolios.admin", portfolio_id=portfolio.id))
+    assert response.status_code == 200
+    assert portfolio.name in response.data.decode()
+    assert translate("fragments.ppoc.update_btn").encode("utf8") not in response.data
+
+
+def test_remove_portfolio_member(client, user_session):
+    portfolio = PortfolioFactory.create()
+
+    user = UserFactory.create()
+    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),
+        follow_redirects=False,
+    )
+
+    assert response.status_code == 302
+    assert response.headers["Location"] == url_for(
+        "portfolios.admin",
+        portfolio_id=portfolio.id,
+        _anchor="portfolio-members",
+        fragment="portfolio-members",
+        _external=True,
+    )
+    assert (
+        PortfolioRoles.get(portfolio_id=portfolio.id, user_id=user.id).status
+        == PortfolioRoleStatus.DISABLED
+    )
+
+
+def test_remove_portfolio_member_self(client, user_session):
+    portfolio = PortfolioFactory.create()
+
+    user_session(portfolio.owner)
+
+    response = client.post(
+        url_for(
+            "portfolios.remove_member",
+            portfolio_id=portfolio.id,
+            user_id=portfolio.owner.id,
+        ),
+        follow_redirects=False,
+    )
+
+    assert response.status_code == 404
+    assert (
+        PortfolioRoles.get(portfolio_id=portfolio.id, user_id=portfolio.owner.id).status
+        == PortfolioRoleStatus.ACTIVE
+    )

tests/routes/portfolios/test_index.py

@@ -0,0 +1,65 @@
+from flask import url_for
+
+from tests.factories import (
+    random_future_date,
+    random_past_date,
+    PortfolioFactory,
+    TaskOrderFactory,
+    UserFactory,
+)
+from atst.utils.localization import translate
+
+
+def test_portfolio_index_with_existing_portfolios(client, user_session):
+    portfolio = PortfolioFactory.create()
+    user_session(portfolio.owner)
+
+    response = client.get(url_for("portfolios.portfolios"))
+
+    assert response.status_code == 200
+    assert portfolio.name.encode("utf8") in response.data
+    assert (
+        translate("portfolios.index.empty.start_button").encode("utf8")
+        not in response.data
+    )
+
+
+def test_portfolio_index_without_existing_portfolios(client, user_session):
+    user = UserFactory.create()
+    user_session(user)
+
+    response = client.get(url_for("portfolios.portfolios"))
+
+    assert response.status_code == 200
+    assert (
+        translate("portfolios.index.empty.start_button").encode("utf8") in response.data
+    )
+
+
+def test_portfolio_reports(client, user_session):
+    portfolio = PortfolioFactory.create(
+        applications=[
+            {"name": "application1", "environments": [{"name": "application1 prod"}]}
+        ]
+    )
+    task_order = TaskOrderFactory.create(
+        number="42",
+        start_date=random_past_date(),
+        end_date=random_future_date(),
+        portfolio=portfolio,
+    )
+    user_session(portfolio.owner)
+    response = client.get(url_for("portfolios.reports", portfolio_id=portfolio.id))
+    assert response.status_code == 200
+    assert portfolio.name in response.data.decode()
+    expiration_date = task_order.end_date.strftime("%Y-%m-%d")
+    assert expiration_date in response.data.decode()
+
+
+def test_portfolio_reports_with_mock_portfolio(client, user_session):
+    portfolio = PortfolioFactory.create(name="A-Wing")
+    user_session(portfolio.owner)
+    response = client.get(url_for("portfolios.reports", portfolio_id=portfolio.id))
+    assert response.status_code == 200
+    assert portfolio.name in response.data.decode()
+    assert "$251,626.00 Total spend to date" in response.data.decode()

tests/routes/portfolios/test_members.py

@@ -15,9 +15,7 @@ _DEFAULT_PERMS_FORM_DATA = {
 def test_user_with_permission_has_add_member_link(client, user_session):
     portfolio = PortfolioFactory.create()
     user_session(portfolio.owner)
-    response = client.get(
+    response = client.get(url_for("portfolios.admin", portfolio_id=portfolio.id))
-        url_for("portfolios.portfolio_admin", portfolio_id=portfolio.id)
-    )
     assert response.status_code == 200
     assert (
         url_for("portfolios.create_member", portfolio_id=portfolio.id).encode()

tests/routes/portfolios/test_portfolios_index.py

@@ -1,275 +0,0 @@
-import pytest
-
-from flask import url_for
-from atst.domain.permission_sets import PermissionSets
-from atst.domain.portfolios import Portfolios
-from atst.models.permissions import Permissions
-from atst.domain.portfolio_roles import PortfolioRoles
-from atst.models.portfolio_role import Status as PortfolioRoleStatus
-from atst.domain.exceptions import UnauthorizedError
-
-from tests.factories import (
-    random_future_date,
-    random_past_date,
-    PortfolioFactory,
-    PortfolioRoleFactory,
-    TaskOrderFactory,
-    UserFactory,
-)
-from atst.utils.localization import translate
-
-
-def test_update_portfolio_name(client, user_session):
-    portfolio = PortfolioFactory.create()
-    user_session(portfolio.owner)
-    response = client.post(
-        url_for("portfolios.edit_portfolio", portfolio_id=portfolio.id),
-        data={"name": "a cool new name"},
-        follow_redirects=True,
-    )
-    assert response.status_code == 200
-    assert portfolio.name == "a cool new name"
-
-
-def updating_ppoc_successfully(client, old_ppoc, new_ppoc, portfolio):
-    response = client.post(
-        url_for("portfolios.update_ppoc", portfolio_id=portfolio.id, _external=True),
-        data={"user_id": new_ppoc.id},
-        follow_redirects=False,
-    )
-
-    assert response.status_code == 302
-    assert response.headers["Location"] == url_for(
-        "portfolios.portfolio_admin",
-        portfolio_id=portfolio.id,
-        fragment="primary-point-of-contact",
-        _anchor="primary-point-of-contact",
-        _external=True,
-    )
-    assert portfolio.owner.id == new_ppoc.id
-    assert (
-        Permissions.EDIT_PORTFOLIO_POC
-        in PortfolioRoles.get(
-            portfolio_id=portfolio.id, user_id=new_ppoc.id
-        ).permissions
-    )
-    assert (
-        Permissions.EDIT_PORTFOLIO_POC
-        not in PortfolioRoles.get(portfolio.id, old_ppoc.id).permissions
-    )
-
-
-def test_update_ppoc_no_user_id_specified(client, user_session):
-    portfolio = PortfolioFactory.create()
-
-    user_session(portfolio.owner)
-
-    response = client.post(
-        url_for("portfolios.update_ppoc", portfolio_id=portfolio.id, _external=True),
-        follow_redirects=False,
-    )
-
-    assert response.status_code == 404
-
-
-def test_update_ppoc_to_member_not_on_portfolio(client, user_session):
-    portfolio = PortfolioFactory.create()
-    original_ppoc = portfolio.owner
-    non_portfolio_member = UserFactory.create()
-
-    user_session(original_ppoc)
-
-    response = client.post(
-        url_for("portfolios.update_ppoc", portfolio_id=portfolio.id, _external=True),
-        data={"user_id": non_portfolio_member.id},
-        follow_redirects=False,
-    )
-
-    assert response.status_code == 404
-    assert portfolio.owner.id == original_ppoc.id
-
-
-def test_update_ppoc_when_ppoc(client, user_session):
-    portfolio = PortfolioFactory.create()
-    original_ppoc = portfolio.owner
-    new_ppoc = UserFactory.create()
-    Portfolios.add_member(
-        member=new_ppoc,
-        portfolio=portfolio,
-        permission_sets=[PermissionSets.VIEW_PORTFOLIO],
-    )
-
-    user_session(original_ppoc)
-
-    updating_ppoc_successfully(
-        client=client, new_ppoc=new_ppoc, old_ppoc=original_ppoc, portfolio=portfolio
-    )
-
-
-def test_update_ppoc_when_cpo(client, user_session):
-    ccpo = UserFactory.create_ccpo()
-    portfolio = PortfolioFactory.create()
-    original_ppoc = portfolio.owner
-    new_ppoc = UserFactory.create()
-    Portfolios.add_member(
-        member=new_ppoc,
-        portfolio=portfolio,
-        permission_sets=[PermissionSets.VIEW_PORTFOLIO],
-    )
-
-    user_session(ccpo)
-
-    updating_ppoc_successfully(
-        client=client, new_ppoc=new_ppoc, old_ppoc=original_ppoc, portfolio=portfolio
-    )
-
-
-def test_update_ppoc_when_not_ppoc(client, user_session):
-    portfolio = PortfolioFactory.create()
-    new_owner = UserFactory.create()
-
-    user_session(new_owner)
-
-    response = client.post(
-        url_for("portfolios.update_ppoc", portfolio_id=portfolio.id, _external=True),
-        data={"dod_id": new_owner.dod_id},
-        follow_redirects=False,
-    )
-
-    assert response.status_code == 404
-
-
-def test_portfolio_index_with_existing_portfolios(client, user_session):
-    portfolio = PortfolioFactory.create()
-    user_session(portfolio.owner)
-
-    response = client.get(url_for("portfolios.portfolios"))
-
-    assert response.status_code == 200
-    assert portfolio.name.encode("utf8") in response.data
-    assert (
-        translate("portfolios.index.empty.start_button").encode("utf8")
-        not in response.data
-    )
-
-
-def test_portfolio_index_without_existing_portfolios(client, user_session):
-    user = UserFactory.create()
-    user_session(user)
-
-    response = client.get(url_for("portfolios.portfolios"))
-
-    assert response.status_code == 200
-    assert (
-        translate("portfolios.index.empty.start_button").encode("utf8") in response.data
-    )
-
-
-def test_portfolio_admin_screen_when_ppoc(client, user_session):
-    portfolio = PortfolioFactory.create()
-    user_session(portfolio.owner)
-    response = client.get(
-        url_for("portfolios.portfolio_admin", portfolio_id=portfolio.id)
-    )
-    assert response.status_code == 200
-    assert portfolio.name in response.data.decode()
-    assert translate("fragments.ppoc.update_btn").encode("utf8") in response.data
-
-
-def test_portfolio_admin_screen_when_not_ppoc(client, user_session):
-    portfolio = PortfolioFactory.create()
-    user = UserFactory.create()
-    permission_sets = PermissionSets.get_many(
-        [PermissionSets.EDIT_PORTFOLIO_ADMIN, PermissionSets.VIEW_PORTFOLIO_ADMIN]
-    )
-    PortfolioRoleFactory.create(
-        portfolio=portfolio, user=user, permission_sets=permission_sets
-    )
-    user_session(user)
-    response = client.get(
-        url_for("portfolios.portfolio_admin", portfolio_id=portfolio.id)
-    )
-    assert response.status_code == 200
-    assert portfolio.name in response.data.decode()
-    assert translate("fragments.ppoc.update_btn").encode("utf8") not in response.data
-
-
-def test_remove_portfolio_member(client, user_session):
-    portfolio = PortfolioFactory.create()
-
-    user = UserFactory.create()
-    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),
-        follow_redirects=False,
-    )
-
-    assert response.status_code == 302
-    assert response.headers["Location"] == url_for(
-        "portfolios.portfolio_admin",
-        portfolio_id=portfolio.id,
-        _anchor="portfolio-members",
-        fragment="portfolio-members",
-        _external=True,
-    )
-    assert (
-        PortfolioRoles.get(portfolio_id=portfolio.id, user_id=user.id).status
-        == PortfolioRoleStatus.DISABLED
-    )
-
-
-def test_remove_portfolio_member_self(client, user_session):
-    portfolio = PortfolioFactory.create()
-
-    user_session(portfolio.owner)
-
-    response = client.post(
-        url_for(
-            "portfolios.remove_member",
-            portfolio_id=portfolio.id,
-            user_id=portfolio.owner.id,
-        ),
-        follow_redirects=False,
-    )
-
-    assert response.status_code == 404
-    assert (
-        PortfolioRoles.get(portfolio_id=portfolio.id, user_id=portfolio.owner.id).status
-        == PortfolioRoleStatus.ACTIVE
-    )
-
-
-def test_portfolio_reports(client, user_session):
-    portfolio = PortfolioFactory.create(
-        applications=[
-            {"name": "application1", "environments": [{"name": "application1 prod"}]}
-        ]
-    )
-    task_order = TaskOrderFactory.create(
-        number="42",
-        start_date=random_past_date(),
-        end_date=random_future_date(),
-        portfolio=portfolio,
-    )
-    user_session(portfolio.owner)
-    response = client.get(
-        url_for("portfolios.portfolio_reports", portfolio_id=portfolio.id)
-    )
-    assert response.status_code == 200
-    assert portfolio.name in response.data.decode()
-    expiration_date = task_order.end_date.strftime("%Y-%m-%d")
-    assert expiration_date in response.data.decode()
-
-
-def test_portfolio_reports_with_mock_portfolio(client, user_session):
-    portfolio = PortfolioFactory.create(name="A-Wing")
-    user_session(portfolio.owner)
-    response = client.get(
-        url_for("portfolios.portfolio_reports", portfolio_id=portfolio.id)
-    )
-    assert response.status_code == 200
-    assert portfolio.name in response.data.decode()
-    assert "$251,626.00 Total spend to date" in response.data.decode()

tests/test_access.py

@@ -232,14 +232,14 @@ def test_application_settings_access(get_url_assert_status):
     get_url_assert_status(rando, url, 404)
 
 
-# portfolios.edit_portfolio
+# portfolios.edit
-def test_portfolios_edit_portfolio_access(post_url_assert_status):
+def test_portfolios_edit_access(post_url_assert_status):
     ccpo = user_with(PermissionSets.EDIT_PORTFOLIO_ADMIN)
     owner = user_with()
     rando = user_with()
     portfolio = PortfolioFactory.create(owner=owner)
 
-    url = url_for("portfolios.edit_portfolio", portfolio_id=portfolio.id)
+    url = url_for("portfolios.edit", portfolio_id=portfolio.id)
     post_url_assert_status(ccpo, url, 200)
     post_url_assert_status(owner, url, 200)
     post_url_assert_status(rando, url, 404)
@@ -296,14 +296,14 @@ def test_applications_new_access(get_url_assert_status):
     get_url_assert_status(rando, url, 404)
 
 
-# portfolios.portfolio_admin
+# portfolios.admin
-def test_portfolios_portfolio_admin_access(get_url_assert_status):
+def test_portfolios_admin_access(get_url_assert_status):
     ccpo = user_with(PermissionSets.VIEW_PORTFOLIO_ADMIN)
     owner = user_with()
     rando = user_with()
     portfolio = PortfolioFactory.create(owner=owner)
 
-    url = url_for("portfolios.portfolio_admin", portfolio_id=portfolio.id)
+    url = url_for("portfolios.admin", portfolio_id=portfolio.id)
     get_url_assert_status(ccpo, url, 200)
     get_url_assert_status(owner, url, 200)
     get_url_assert_status(rando, url, 404)
@@ -335,14 +335,14 @@ def test_task_orders_portfolio_funding_access(get_url_assert_status):
     get_url_assert_status(rando, url, 404)
 
 
-# portfolios.portfolio_reports
+# portfolios.reports
 def test_portfolios_portfolio_reports_access(get_url_assert_status):
     ccpo = user_with(PermissionSets.VIEW_PORTFOLIO_REPORTS)
     owner = user_with()
     rando = user_with()
     portfolio = PortfolioFactory.create(owner=owner)
 
-    url = url_for("portfolios.portfolio_reports", portfolio_id=portfolio.id)
+    url = url_for("portfolios.reports", portfolio_id=portfolio.id)
     get_url_assert_status(ccpo, url, 200)
     get_url_assert_status(owner, url, 200)
     get_url_assert_status(rando, url, 404)