Use portfolio_role entity to display and update portfolio member info.

Previously, we were encoding the portfolio_role.user_id as part of the
form data for the portfolio admin page. This was convenient because it
allowed us to easily determine certain display attributes in the
template. Instead, we should rely on the PortfolioRole as the source of
truth for member information. This commit adds:

- Portfolio.owner_role to return the PortfolioRole of the owner
- explicitly passes the PortfolioRole IDs for the PPoC and current user
  to the template
- PortfolioRole.full_name for deriving the member name
This commit is contained in:
dandds 2019-05-16 08:04:48 -04:00
parent 802c62e3e9
commit c8682c0897
7 changed files with 41 additions and 27 deletions

View File

@ -9,8 +9,8 @@ from atst.utils.localization import translate
class PermissionsForm(BaseForm):
member = StringField()
user_id = HiddenField()
member_name = StringField()
member_id = HiddenField()
perms_app_mgmt = SelectField(
translate("forms.new_member.app_mgmt"),
choices=[

View File

@ -26,14 +26,18 @@ class Portfolio(Base, mixins.TimestampsMixin, mixins.AuditableMixin):
task_orders = relationship("TaskOrder")
@property
def owner(self):
def owner_role(self):
def _is_portfolio_owner(portfolio_role):
return PermissionSets.PORTFOLIO_POC in [
perms_set.name for perms_set in portfolio_role.permission_sets
]
owner = first_or_none(_is_portfolio_owner, self.roles)
return owner.user if owner else None
return first_or_none(_is_portfolio_owner, self.roles)
@property
def owner(self):
owner_role = self.owner_role
return owner_role.user if owner_role else None
@property
def users(self):

View File

@ -160,6 +160,10 @@ class PortfolioRole(
self.latest_invitation and self.latest_invitation.is_inactive
)
@property
def full_name(self):
return self.user.full_name
Index(
"portfolio_role_user_portfolio",

View File

@ -12,6 +12,7 @@ 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 import first_or_none
from atst.utils.flash import formatted_flash as flash
from atst.domain.exceptions import UnauthorizedError
@ -25,8 +26,8 @@ def permission_str(member, edit_perm_set, view_perm_set):
def serialize_member_form_data(member):
return {
"member": member.user.full_name,
"user_id": member.user_id,
"member_name": member.full_name,
"member_id": member.id,
"perms_app_mgmt": permission_str(
member,
PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT,
@ -53,7 +54,7 @@ def serialize_member_form_data(member):
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:
if member["member_id"] == portfolio.owner_role.id:
ppoc = member
members.remove(member)
members.insert(0, ppoc)
@ -76,6 +77,11 @@ def render_admin_page(portfolio, form=None):
(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
)
current_member_id = current_member.id if current_member else None
return render_template(
"portfolios/admin.html",
form=form,
@ -86,7 +92,8 @@ def render_admin_page(portfolio, form=None):
portfolio=portfolio,
audit_events=audit_events,
user=g.current_user,
members_data=members_data,
ppoc_id=members_data[0].get("member_id"),
current_member_id=current_member_id,
)
@ -105,12 +112,11 @@ def edit_members(portfolio_id):
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:
member_id = subform.member_id.data
member = PortfolioRoles.get_by_id(member_id)
if member is not portfolio.owner_role:
new_perm_set = subform.data["permission_sets"]
portfolio_role = PortfolioRoles.get(portfolio.id, user_id)
PortfolioRoles.update(portfolio_role, new_perm_set)
PortfolioRoles.update(member, new_perm_set)
flash("update_portfolio_members", portfolio=portfolio)

View File

@ -1,18 +1,18 @@
{% from "components/options_input.html" import OptionsInput %}
{% for subform in member_perms_form.members_permissions %}
{% set modal_id = "portfolio_id_{}_user_id_{}".format(portfolio.id, subform.user_id.data) %}
{% set ppoc = subform.user_id.data == portfolio.owner.id %}
{% set modal_id = "portfolio_id_{}_user_id_{}".format(portfolio.id, subform.member_id.data) %}
{% set ppoc = subform.member_id.data == ppoc_id %}
{% set archive_button_class = 'button-danger-outline' %}
<tr {% if ppoc %}class="members-table-ppoc"{% endif %}>
<td class='name'>{{ subform.member.data }}
<td class='name'>{{ subform.member_name.data }}
<div>
{% if ppoc %}
{% set archive_button_class = 'usa-button-disabled' %}
<span class='you'>PPoC</span>
{% endif %}
{% if subform.user_id.data == user.id %}
{% if subform.member_id.data == current_member_id %}
{% set archive_button_class = 'usa-button-disabled' %}
<span class='you'>(<span class='green'>you</span>)</span>
{% endif %}
@ -30,7 +30,7 @@
</a>
</td>
{% if not ppoc %}
{{ subform.user_id() }}
{{ subform.member_id() }}
{% endif %}
</tr>
{% endfor %}

View File

@ -1,14 +1,14 @@
{% for subform in member_perms_form.members_permissions %}
{% set ppoc = subform.user_id.data == portfolio.owner.id %}
{% set ppoc = subform.member_id.data == ppoc_id %}
{% set heading_perms = [subform.perms_app_mgmt, subform.perms_funding, subform.perms_reporting, subform.perms_portfolio_mgmt] %}
<tr>
<td class='name'>{{ subform.member.data }}
<td class='name'>{{ subform.member_name.data }}
<div>
{% if ppoc %}
<span class='you'>PPoC</span>
{% endif %}
{% if subform.user_id.data == user.id %}
{% if subform.member_id.data == current_member_id %}
<span class='you'>(<span class='green'>you</span>)</span>
{% endif %}
</div>

View File

@ -53,7 +53,7 @@ def test_update_member_permissions(client, user_session):
user_session(user)
form_data = {
"members_permissions-0-user_id": rando.id,
"members_permissions-0-member_id": rando_pf_role.id,
"members_permissions-0-perms_app_mgmt": "edit_portfolio_application_management",
"members_permissions-0-perms_funding": "view_portfolio_funding",
"members_permissions-0-perms_reporting": "view_portfolio_reports",
@ -90,7 +90,7 @@ def test_no_update_member_permissions_without_edit_access(client, user_session):
user_session(user)
form_data = {
"members_permissions-0-user_id": rando.id,
"members_permissions-0-member_id": rando_pf_role.id,
"members_permissions-0-perms_app_mgmt": "edit_portfolio_application_management",
"members_permissions-0-perms_funding": "view_portfolio_funding",
"members_permissions-0-perms_reporting": "view_portfolio_reports",
@ -114,14 +114,14 @@ def test_rerender_admin_page_if_member_perms_form_does_not_validate(
):
portfolio = PortfolioFactory.create()
user = UserFactory.create()
PortfolioRoleFactory.create(
role = PortfolioRoleFactory.create(
user=user,
portfolio=portfolio,
permission_sets=[PermissionSets.get(PermissionSets.EDIT_PORTFOLIO_ADMIN)],
)
user_session(user)
form_data = {
"members_permissions-0-user_id": user.id,
"members_permissions-0-member_id": role.id,
"members_permissions-0-perms_app_mgmt": "bad input",
"members_permissions-0-perms_funding": "view_portfolio_funding",
"members_permissions-0-perms_reporting": "view_portfolio_reports",
@ -149,7 +149,7 @@ def test_cannot_update_portfolio_ppoc_perms(client, user_session):
assert ppoc_pf_role.has_permission_set(PermissionSets.PORTFOLIO_POC)
member_perms_data = {
"members_permissions-0-user_id": ppoc.id,
"members_permissions-0-member_id": ppoc_pf_role.id,
"members_permissions-0-perms_app_mgmt": "view_portfolio_application_management",
"members_permissions-0-perms_funding": "view_portfolio_funding",
"members_permissions-0-perms_reporting": "view_portfolio_reports",