Use portfolio_role.id for updating primary point of contact.

Our forms should rely on role IDs for displaying user information on the
portfolio page. This way they are decoupled from user table data and can
eventually rely on invitation user data where an invitation has been
sent but a user does not exist yet.

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],
     )