diff --git a/atst/domain/portfolios/query.py b/atst/domain/portfolios/query.py index eb78c56d..e215b9d3 100644 --- a/atst/domain/portfolios/query.py +++ b/atst/domain/portfolios/query.py @@ -1,7 +1,13 @@ +from sqlalchemy import or_ from atst.database import db from atst.domain.common import Query from atst.models.portfolio import Portfolio from atst.models.portfolio_role import PortfolioRole, Status as PortfolioRoleStatus +from atst.models.application_role import ( + ApplicationRole, + Status as ApplicationRoleStatus, +) +from atst.models.application import Application class PortfoliosQuery(Query): @@ -11,9 +17,38 @@ class PortfoliosQuery(Query): def get_for_user(cls, user): return ( db.session.query(Portfolio) - .join(PortfolioRole) - .filter(PortfolioRole.user == user) - .filter(PortfolioRole.status == PortfolioRoleStatus.ACTIVE) + .filter( + or_( + Portfolio.id.in_( + db.session.query(Portfolio.id) + .join(Application) + .filter(Portfolio.id == Application.portfolio_id) + .filter( + Application.id.in_( + db.session.query(Application.id) + .join(ApplicationRole) + .filter( + ApplicationRole.application_id == Application.id + ) + .filter(ApplicationRole.user_id == user.id) + .filter( + ApplicationRole.status + == ApplicationRoleStatus.ACTIVE + ) + .subquery() + ) + ) + ), + Portfolio.id.in_( + db.session.query(Portfolio.id) + .join(PortfolioRole) + .filter(PortfolioRole.user == user) + .filter(PortfolioRole.status == PortfolioRoleStatus.ACTIVE) + .subquery() + ), + ) + ) + .order_by(Portfolio.name.asc()) .all() ) diff --git a/atst/routes/applications/index.py b/atst/routes/applications/index.py index 3aebb6b1..d4b2f276 100644 --- a/atst/routes/applications/index.py +++ b/atst/routes/applications/index.py @@ -15,6 +15,7 @@ def has_portfolio_applications(_user, portfolio=None, **_kwargs): return True +@applications_bp.route("/portfolios/") @applications_bp.route("/portfolios//applications") @user_can( Permissions.VIEW_APPLICATION, diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index 28bb079e..f0758b93 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -1,6 +1,6 @@ from datetime import date, timedelta -from flask import render_template, request as http_request, g, redirect, url_for +from flask import render_template, request as http_request, g from . import portfolios_bp from atst.domain.reports import Reports @@ -19,14 +19,6 @@ def portfolios(): return render_template("portfolios/blank_slate.html") -@portfolios_bp.route("/portfolios/") -@user_can(Permissions.VIEW_PORTFOLIO, message="view portfolio") -def show_portfolio(portfolio_id): - return redirect( - url_for("applications.portfolio_applications", portfolio_id=portfolio_id) - ) - - @portfolios_bp.route("/portfolios//reports") @user_can(Permissions.VIEW_PORTFOLIO_REPORTS, message="view portfolio reports") def reports(portfolio_id): diff --git a/atst/routes/portfolios/invitations.py b/atst/routes/portfolios/invitations.py index 4c3a53a8..dd1f1c5a 100644 --- a/atst/routes/portfolios/invitations.py +++ b/atst/routes/portfolios/invitations.py @@ -30,7 +30,7 @@ def accept_invitation(portfolio_token): ) return redirect( - url_for("portfolios.show_portfolio", portfolio_id=invite.portfolio.id) + url_for("applications.portfolio_applications", portfolio_id=invite.portfolio.id) ) diff --git a/atst/utils/flash.py b/atst/utils/flash.py index 0a96366a..001293af 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -145,7 +145,7 @@ MESSAGES = { "actions": """ {% from "components/icon.html" import Icon %}
- + {{ Icon('shield') }} {{ "flash.portfolio_home" | translate }} diff --git a/templates/navigation/global_sidenav.html b/templates/navigation/global_sidenav.html index 555b7fdd..e61524ee 100644 --- a/templates/navigation/global_sidenav.html +++ b/templates/navigation/global_sidenav.html @@ -21,7 +21,7 @@ {% if portfolios %} {% for other_portfolio in portfolios|sort(attribute='name') %} {{ SidenavItem(other_portfolio.name, - href=url_for("portfolios.show_portfolio", portfolio_id=other_portfolio.id), + href=url_for("applications.portfolio_applications", portfolio_id=other_portfolio.id), active=(other_portfolio.id | string) == request.view_args.get('portfolio_id') ) }} {% endfor %} diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index 6819b36b..bf6a415e 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -7,9 +7,12 @@ from atst.domain.portfolio_roles import PortfolioRoles from atst.domain.applications import Applications from atst.domain.environments import Environments from atst.domain.permission_sets import PermissionSets, PORTFOLIO_PERMISSION_SETS +from atst.models.application_role import Status as ApplicationRoleStatus from atst.models.portfolio_role import Status as PortfolioRoleStatus from tests.factories import ( + ApplicationFactory, + ApplicationRoleFactory, UserFactory, PortfolioRoleFactory, PortfolioFactory, @@ -164,6 +167,17 @@ def test_scoped_portfolio_returns_all_applications_for_portfolio_owner( assert len(scoped_portfolio.applications[0].environments) == 3 +def test_for_user_returns_portfolios_for_applications_user_invited_to(): + bob = UserFactory.create() + portfolio = PortfolioFactory.create() + application = ApplicationFactory.create(portfolio=portfolio) + ApplicationRoleFactory.create( + application=application, user=bob, status=ApplicationRoleStatus.ACTIVE + ) + + assert portfolio in Portfolios.for_user(user=bob) + + def test_for_user_returns_active_portfolios_for_user(portfolio, portfolio_owner): bob = UserFactory.create() PortfolioRoleFactory.create( diff --git a/tests/routes/portfolios/test_invitations.py b/tests/routes/portfolios/test_invitations.py index 5134957b..9dbf481f 100644 --- a/tests/routes/portfolios/test_invitations.py +++ b/tests/routes/portfolios/test_invitations.py @@ -34,7 +34,7 @@ def test_existing_member_accepts_valid_invite(client, user_session): # user is redirected to the portfolio view assert response.status_code == 302 assert ( - url_for("portfolios.show_portfolio", portfolio_id=invite.portfolio.id) + url_for("applications.portfolio_applications", portfolio_id=invite.portfolio.id) in response.headers["Location"] ) # the one-time use invite is no longer usable @@ -77,7 +77,7 @@ def test_new_member_accepts_valid_invite(monkeypatch, client, user_session): # user is redirected to the portfolio view assert response.status_code == 302 assert ( - url_for("portfolios.show_portfolio", portfolio_id=portfolio.id) + url_for("applications.portfolio_applications", portfolio_id=portfolio.id) in response.headers["Location"] ) # the user has access to the portfolio diff --git a/tests/test_access.py b/tests/test_access.py index 84f1d167..2d7874a4 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -466,19 +466,6 @@ def test_portfolios_revoke_invitation_access(post_url_assert_status): post_url_assert_status(user, url, status) -# portfolios.show_portfolio -def test_portfolios_show_portfolio_access(get_url_assert_status): - ccpo = user_with(PermissionSets.VIEW_PORTFOLIO) - owner = user_with() - rando = user_with() - portfolio = PortfolioFactory.create(owner=owner) - - url = url_for("portfolios.show_portfolio", portfolio_id=portfolio.id) - get_url_assert_status(ccpo, url, 302) - get_url_assert_status(owner, url, 302) - get_url_assert_status(rando, url, 404) - - # task_orders.so_review def test_task_orders_so_review_access(get_url_assert_status): ccpo = UserFactory.create_ccpo()