Merge pull request #841 from dod-ccpo/ppoc-role-id

Use portfolio_role.id for updating primary point of contact.

atst/forms/portfolio_member.py

@@ -80,7 +80,7 @@ class NewForm(BaseForm):
 
 
 class AssignPPOCForm(PermissionsForm):
-    user_id = SelectField(
+    role_id = SelectField(
         label=translate("forms.assign_ppoc.dod_id"),
         validators=[Required()],
         choices=[("", "- Select -")],

atst/routes/portfolios/admin.py

@@ -4,10 +4,8 @@ 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
@@ -73,9 +71,7 @@ def render_admin_page(portfolio, form=None):
     assign_ppoc_form = member_forms.AssignPPOCForm()
     for pf_role in portfolio.roles:
         if pf_role.user != portfolio.owner and pf_role.is_active:
-            assign_ppoc_form.user_id.choices += [
+            assign_ppoc_form.role_id.choices += [(pf_role.id, pf_role.full_name)]
-                (pf_role.user.id, pf_role.user.full_name)
-            ]
 
     current_member = first_or_none(
         lambda m: m.user_id == g.current_user.id, portfolio.members
@@ -135,18 +131,14 @@ def edit_members(portfolio_id):
 @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")
+    role_id = http_request.form.get("role_id")
 
     portfolio = Portfolios.get(g.current_user, portfolio_id)
-    new_ppoc = Users.get(user_id)
+    new_ppoc_role = PortfolioRoles.get_by_id(role_id)
 
-    if new_ppoc not in portfolio.users:
+    PortfolioRoles.make_ppoc(portfolio_role=new_ppoc_role)
-        raise NotFoundError("user not in portfolio")
 
-    portfolio_role = PortfolioRoles.get(portfolio_id=portfolio_id, user_id=user_id)
+    flash("primary_point_of_contact_changed", ppoc_name=new_ppoc_role.full_name)
-    PortfolioRoles.make_ppoc(portfolio_role=portfolio_role)
-
-    flash("primary_point_of_contact_changed", ppoc_name=new_ppoc.full_name)
 
     return redirect(
         url_for(

templates/fragments/admin/change_ppoc.html

@@ -22,7 +22,7 @@
     <div class='form-col form-col--half'>
       {{
         OptionsInput(
-          assign_ppoc_form.user_id
+          assign_ppoc_form.role_id
         )
       }}
     </div>

tests/routes/portfolios/test_admin.py

@@ -181,7 +181,7 @@ def test_update_portfolio_name(client, user_session):
 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},
+        data={"role_id": new_ppoc.id},
         follow_redirects=False,
     )
 
@@ -193,17 +193,9 @@ def updating_ppoc_successfully(client, old_ppoc, new_ppoc, portfolio):
         _anchor="primary-point-of-contact",
         _external=True,
     )
-    assert portfolio.owner.id == new_ppoc.id
+    assert portfolio.owner_role.id == new_ppoc.id
-    assert (
+    assert Permissions.EDIT_PORTFOLIO_POC in new_ppoc.permissions
-        Permissions.EDIT_PORTFOLIO_POC
+    assert Permissions.EDIT_PORTFOLIO_POC not in old_ppoc.permissions
-        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):
@@ -238,15 +230,14 @@ def test_update_ppoc_to_member_not_on_portfolio(client, user_session):
 
 def test_update_ppoc_when_ppoc(client, user_session):
     portfolio = PortfolioFactory.create()
-    original_ppoc = portfolio.owner
+    original_ppoc = portfolio.owner_role
-    new_ppoc = UserFactory.create()
+    new_ppoc = Portfolios.add_member(
-    Portfolios.add_member(
+        member=UserFactory.create(),
-        member=new_ppoc,
         portfolio=portfolio,
         permission_sets=[PermissionSets.VIEW_PORTFOLIO],
     )
 
-    user_session(original_ppoc)
+    user_session(original_ppoc.user)
 
     updating_ppoc_successfully(
         client=client, new_ppoc=new_ppoc, old_ppoc=original_ppoc, portfolio=portfolio
@@ -256,10 +247,9 @@ def test_update_ppoc_when_ppoc(client, user_session):
 def test_update_ppoc_when_cpo(client, user_session):
     ccpo = UserFactory.create_ccpo()
     portfolio = PortfolioFactory.create()
-    original_ppoc = portfolio.owner
+    original_ppoc = portfolio.owner_role
-    new_ppoc = UserFactory.create()
+    new_ppoc = Portfolios.add_member(
-    Portfolios.add_member(
+        member=UserFactory.create(),
-        member=new_ppoc,
         portfolio=portfolio,
         permission_sets=[PermissionSets.VIEW_PORTFOLIO],
     )