diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 33765b178..0b96b4c48 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -190,11 +190,11 @@ class PortfolioInvitationAdminForm(UserChangeForm): model = models.PortfolioInvitation fields = "__all__" widgets = { - "portfolio_roles": FilteredSelectMultipleArrayWidget( - "portfolio_roles", is_stacked=False, choices=UserPortfolioRoleChoices.choices + "roles": FilteredSelectMultipleArrayWidget( + "roles", is_stacked=False, choices=UserPortfolioRoleChoices.choices ), - "portfolio_additional_permissions": FilteredSelectMultipleArrayWidget( - "portfolio_additional_permissions", + "additional_permissions": FilteredSelectMultipleArrayWidget( + "additional_permissions", is_stacked=False, choices=UserPortfolioPermissionChoices.choices, ), @@ -1409,8 +1409,8 @@ class PortfolioInvitationAdmin(ListHeaderAdmin): list_display = [ "email", "portfolio", - "portfolio_roles", - "portfolio_additional_permissions", + "roles", + "additional_permissions", "status", ] diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 8a07b3f27..8281aa50a 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1880,11 +1880,10 @@ class MembersTable extends LoadTableBase { * @param {*} sortBy - the sort column option * @param {*} order - the sort order {asc, desc} * @param {*} scroll - control for the scrollToElement functionality - * @param {*} status - control for the status filter * @param {*} searchTerm - the search term * @param {*} portfolio - the portfolio id */ - loadTable(page, sortBy = this.currentSortBy, order = this.currentOrder, scroll = this.scrollToTable, status = this.currentStatus, searchTerm =this.currentSearchTerm, portfolio = this.portfolioValue) { + loadTable(page, sortBy = this.currentSortBy, order = this.currentOrder, scroll = this.scrollToTable, searchTerm =this.currentSearchTerm, portfolio = this.portfolioValue) { // --------- SEARCH let searchParams = new URLSearchParams( @@ -1892,7 +1891,6 @@ class MembersTable extends LoadTableBase { "page": page, "sort_by": sortBy, "order": order, - "status": status, "search_term": searchTerm } ); @@ -1928,11 +1926,40 @@ class MembersTable extends LoadTableBase { const memberList = document.querySelector('.members__table tbody'); memberList.innerHTML = ''; + const invited = 'Invited'; + data.members.forEach(member => { - // const actionUrl = domain.action_url; const member_name = member.name; - const member_email = member.email; - const last_active = member.last_active; + const member_display = member.member_display; + const options = { year: 'numeric', month: 'short', day: 'numeric' }; + + // Handle last_active values + let last_active = member.last_active; + let last_active_formatted = ''; + let last_active_sort_value = ''; + + // Handle 'Invited' or null/empty values differently from valid dates + if (last_active && last_active !== invited) { + try { + // Try to parse the last_active as a valid date + last_active = new Date(last_active); + if (!isNaN(last_active)) { + last_active_formatted = last_active.toLocaleDateString('en-US', options); + last_active_sort_value = last_active.getTime(); // For sorting purposes + } else { + last_active_formatted='Invalid date' + } + } catch (e) { + console.error(`Error parsing date: ${last_active}. Error: ${e}`); + last_active_formatted='Invalid date' + } + } else { + // Handle 'Invited' or null + last_active = invited; + last_active_formatted = invited; + last_active_sort_value = invited; // Keep 'Invited' as a sortable string + } + const action_url = member.action_url; const action_label = member.action_label; const svg_icon = member.svg_icon; @@ -1945,10 +1972,10 @@ class MembersTable extends LoadTableBase { row.innerHTML = ` - ${member_email ? member_email : member_name} ${admin_tagHTML} + ${member_display} ${admin_tagHTML} - - ${last_active} + + ${last_active_formatted} diff --git a/src/registrar/assets/sass/_theme/_buttons.scss b/src/registrar/assets/sass/_theme/_buttons.scss index 2e4469e12..ff5ffb386 100644 --- a/src/registrar/assets/sass/_theme/_buttons.scss +++ b/src/registrar/assets/sass/_theme/_buttons.scss @@ -254,6 +254,7 @@ a .usa-icon, // Note: Can be simplified by adding text-secondary to delete anchors in tables button.text-secondary, button.text-secondary:hover, -.dotgov-table a.text-secondary { +a.text-secondary, +a.text-secondary:hover { color: $theme-color-error; } diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 4d1be6f31..ee923aac6 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -86,6 +86,26 @@ urlpatterns = [ views.PortfolioMembersView.as_view(), name="members", ), + path( + "member/", + views.PortfolioMemberView.as_view(), + name="member", + ), + path( + "member//permissions", + views.PortfolioMemberEditView.as_view(), + name="member-permissions", + ), + path( + "invitedmember/", + views.PortfolioInvitedMemberView.as_view(), + name="invitedmember", + ), + path( + "invitedmember//permissions", + views.PortfolioInvitedMemberEditView.as_view(), + name="invitedmember-permissions", + ), # path( # "no-organization-members/", # views.PortfolioNoMembersView.as_view(), diff --git a/src/registrar/fixtures/fixtures_user_portfolio_permissions.py b/src/registrar/fixtures/fixtures_user_portfolio_permissions.py index 3c64eb6b5..0ff740573 100644 --- a/src/registrar/fixtures/fixtures_user_portfolio_permissions.py +++ b/src/registrar/fixtures/fixtures_user_portfolio_permissions.py @@ -7,7 +7,7 @@ from registrar.fixtures.fixtures_users import UserFixture from registrar.models import User from registrar.models.portfolio import Portfolio from registrar.models.user_portfolio_permission import UserPortfolioPermission -from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices fake = Faker() logger = logging.getLogger(__name__) @@ -58,6 +58,7 @@ class UserPortfolioPermissionFixture: user=user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[UserPortfolioPermissionChoices.EDIT_MEMBERS], ) user_portfolio_permissions_to_create.append(user_portfolio_permission) else: diff --git a/src/registrar/forms/__init__.py b/src/registrar/forms/__init__.py index 033e955ed..121e2b3f7 100644 --- a/src/registrar/forms/__init__.py +++ b/src/registrar/forms/__init__.py @@ -13,4 +13,5 @@ from .domain import ( ) from .portfolio import ( PortfolioOrgAddressForm, + PortfolioMemberForm, ) diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index 999d76d51..7c8d2f171 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -4,7 +4,14 @@ import logging from django import forms from django.core.validators import RegexValidator -from ..models import DomainInformation, Portfolio, SeniorOfficial +from registrar.models import ( + PortfolioInvitation, + UserPortfolioPermission, + DomainInformation, + Portfolio, + SeniorOfficial, +) +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices logger = logging.getLogger(__name__) @@ -99,3 +106,57 @@ class PortfolioSeniorOfficialForm(forms.ModelForm): cleaned_data = super().clean() cleaned_data.pop("full_name", None) return cleaned_data + + +class PortfolioMemberForm(forms.ModelForm): + """ + Form for updating a portfolio member. + """ + + roles = forms.MultipleChoiceField( + choices=UserPortfolioRoleChoices.choices, + widget=forms.SelectMultiple(attrs={"class": "usa-select"}), + required=False, + label="Roles", + ) + + additional_permissions = forms.MultipleChoiceField( + choices=UserPortfolioPermissionChoices.choices, + widget=forms.SelectMultiple(attrs={"class": "usa-select"}), + required=False, + label="Additional Permissions", + ) + + class Meta: + model = UserPortfolioPermission + fields = [ + "roles", + "additional_permissions", + ] + + +class PortfolioInvitedMemberForm(forms.ModelForm): + """ + Form for updating a portfolio invited member. + """ + + roles = forms.MultipleChoiceField( + choices=UserPortfolioRoleChoices.choices, + widget=forms.SelectMultiple(attrs={"class": "usa-select"}), + required=False, + label="Roles", + ) + + additional_permissions = forms.MultipleChoiceField( + choices=UserPortfolioPermissionChoices.choices, + widget=forms.SelectMultiple(attrs={"class": "usa-select"}), + required=False, + label="Additional Permissions", + ) + + class Meta: + model = PortfolioInvitation + fields = [ + "roles", + "additional_permissions", + ] diff --git a/src/registrar/migrations/0134_rename_portfolio_additional_permissions_portfolioinvitation_additional_permissions_and_more.py b/src/registrar/migrations/0134_rename_portfolio_additional_permissions_portfolioinvitation_additional_permissions_and_more.py new file mode 100644 index 000000000..9a24438df --- /dev/null +++ b/src/registrar/migrations/0134_rename_portfolio_additional_permissions_portfolioinvitation_additional_permissions_and_more.py @@ -0,0 +1,23 @@ +# Generated by Django 4.2.10 on 2024-10-11 19:58 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0133_domainrequest_rejection_reason_email_and_more"), + ] + + operations = [ + migrations.RenameField( + model_name="portfolioinvitation", + old_name="portfolio_additional_permissions", + new_name="additional_permissions", + ), + migrations.RenameField( + model_name="portfolioinvitation", + old_name="portfolio_roles", + new_name="roles", + ), + ] diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py index 46d7bf124..b1f22ae83 100644 --- a/src/registrar/models/portfolio_invitation.py +++ b/src/registrar/models/portfolio_invitation.py @@ -4,6 +4,7 @@ import logging from django.contrib.auth import get_user_model from django.db import models from django_fsm import FSMField, transition +from registrar.models.domain_invitation import DomainInvitation from registrar.models.user_portfolio_permission import UserPortfolioPermission from .utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices # type: ignore from .utility.time_stamped_model import TimeStampedModel @@ -38,7 +39,7 @@ class PortfolioInvitation(TimeStampedModel): related_name="portfolios", ) - portfolio_roles = ArrayField( + roles = ArrayField( models.CharField( max_length=50, choices=UserPortfolioRoleChoices.choices, @@ -48,7 +49,7 @@ class PortfolioInvitation(TimeStampedModel): help_text="Select one or more roles.", ) - portfolio_additional_permissions = ArrayField( + additional_permissions = ArrayField( models.CharField( max_length=50, choices=UserPortfolioPermissionChoices.choices, @@ -67,6 +68,31 @@ class PortfolioInvitation(TimeStampedModel): def __str__(self): return f"Invitation for {self.email} on {self.portfolio} is {self.status}" + def get_managed_domains_count(self): + """Return the count of domain invitations managed by the invited user for this portfolio.""" + # Filter the UserDomainRole model to get domains where the user has a manager role + managed_domains = DomainInvitation.objects.filter( + email=self.email, domain__domain_info__portfolio=self.portfolio + ).count() + return managed_domains + + def get_portfolio_permissions(self): + """ + Retrieve the permissions for the user's portfolio roles from the invite. + This is similar logic to _get_portfolio_permissions in user_portfolio_permission + """ + # Use a set to avoid duplicate permissions + portfolio_permissions = set() + + if self.roles: + for role in self.roles: + portfolio_permissions.update(UserPortfolioPermission.PORTFOLIO_ROLE_PERMISSIONS.get(role, [])) + + if self.additional_permissions: + portfolio_permissions.update(self.additional_permissions) + + return list(portfolio_permissions) + @transition(field="status", source=PortfolioInvitationStatus.INVITED, target=PortfolioInvitationStatus.RETRIEVED) def retrieve(self): """When an invitation is retrieved, create the corresponding permission. @@ -88,8 +114,8 @@ class PortfolioInvitation(TimeStampedModel): user_portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( portfolio=self.portfolio, user=user ) - if self.portfolio_roles and len(self.portfolio_roles) > 0: - user_portfolio_permission.roles = self.portfolio_roles - if self.portfolio_additional_permissions and len(self.portfolio_additional_permissions) > 0: - user_portfolio_permission.additional_permissions = self.portfolio_additional_permissions + if self.roles and len(self.roles) > 0: + user_portfolio_permission.roles = self.roles + if self.additional_permissions and len(self.additional_permissions) > 0: + user_portfolio_permission.additional_permissions = self.additional_permissions user_portfolio_permission.save() diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 241afd328..c95a3f26b 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -1,5 +1,6 @@ from django.db import models from django.forms import ValidationError +from registrar.models.user_domain_role import UserDomainRole from registrar.utility.waffle import flag_is_active_for_user from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from .utility.time_stamped_model import TimeStampedModel @@ -79,6 +80,14 @@ class UserPortfolioPermission(TimeStampedModel): ) return readable_roles + def get_managed_domains_count(self): + """Return the count of domains managed by the user for this portfolio.""" + # Filter the UserDomainRole model to get domains where the user has a manager role + managed_domains = UserDomainRole.objects.filter( + user=self.user, role=UserDomainRole.Roles.MANAGER, domain__domain_info__portfolio=self.portfolio + ).count() + return managed_domains + def _get_portfolio_permissions(self): """ Retrieve the permissions for the user's portfolio roles. diff --git a/src/registrar/templates/includes/header_extended.html b/src/registrar/templates/includes/header_extended.html index 8f12706ab..23b7d1be3 100644 --- a/src/registrar/templates/includes/header_extended.html +++ b/src/registrar/templates/includes/header_extended.html @@ -93,12 +93,12 @@ {% endif %} - {% if has_organization_members_flag and has_view_members_portfolio_permission %} -
  • - - Members - -
  • + {% if has_organization_members_flag %} +
  • + + Members + +
  • {% endif %}
  • diff --git a/src/registrar/templates/includes/member_domain_management.html b/src/registrar/templates/includes/member_domain_management.html new file mode 100644 index 000000000..6bf3f1320 --- /dev/null +++ b/src/registrar/templates/includes/member_domain_management.html @@ -0,0 +1,6 @@ +

    Assigned domains

    +{% if domain_count > 0 %} +

    {{domain_count}}

    +{% else %} +

    This member does not manage any domains.{% if manage_button %} To assign this member a domain, click "Manage".{% endif %}

    +{% endif %} diff --git a/src/registrar/templates/includes/member_permissions.html b/src/registrar/templates/includes/member_permissions.html new file mode 100644 index 000000000..8cf75cfbf --- /dev/null +++ b/src/registrar/templates/includes/member_permissions.html @@ -0,0 +1,26 @@ +

    Member access

    +{% if permissions.roles and 'organization_admin' in permissions.roles %} +

    Admin access

    +{% elif permissions.roles and 'organization_member' in permissions.roles %} +

    Basic access

    +{% else %} +

    +{% endif %} + +

    Organization domain requests

    +{% if member_has_edit_request_portfolio_permission %} +

    View all requests plus create requests

    +{% elif member_has_view_all_requests_portfolio_permission %} +

    View all requests

    +{% else %} +

    No access

    +{% endif %} + +

    Organization members

    +{% if member_has_edit_members_portfolio_permission %} +

    View all members plus manage members

    +{% elif member_has_view_members_portfolio_permission %} +

    View all members

    +{% else %} +

    No access

    +{% endif %} \ No newline at end of file diff --git a/src/registrar/templates/includes/summary_item.html b/src/registrar/templates/includes/summary_item.html index d4c68395f..0600d7ea7 100644 --- a/src/registrar/templates/includes/summary_item.html +++ b/src/registrar/templates/includes/summary_item.html @@ -24,7 +24,11 @@ {% if sub_header_text %}

    {{ sub_header_text }}

    {% endif %} - {% if address %} + {% if permissions %} + {% include "includes/member_permissions.html" with permissions=value %} + {% elif domain_mgmt %} + {% include "includes/member_domain_management.html" with domain_count=value %} + {% elif address %} {% include "includes/organization_address.html" with organization=value %} {% elif contact %} {% if list %} @@ -122,9 +126,9 @@ class="usa-link usa-link--icon font-sans-sm line-height-sans-5" > - Edit {{ title }} + {% if manage_button %}Manage{% elif view_button %}View{% else %}Edit{% endif %} {{ title }} {% endif %} diff --git a/src/registrar/templates/portfolio_member.html b/src/registrar/templates/portfolio_member.html new file mode 100644 index 000000000..f2ee8f4c5 --- /dev/null +++ b/src/registrar/templates/portfolio_member.html @@ -0,0 +1,137 @@ +{% extends 'portfolio_base.html' %} +{% load static field_helpers%} + +{% block title %}Organization member {% endblock %} + +{% load static %} + +{% block portfolio_content %} +
    + + {% url 'members' as url %} + + + +

    Manage member

    + +
    +

    + {% if member %} + {{ member.email }} + {% elif portfolio_invitation %} + {{ portfolio_invitation.email }} + {% endif %} +

    + {% if has_edit_members_portfolio_permission %} + {% if member %} + + Remove member + + {% else %} + + Cancel invitation + + {% endif %} + +
    +
    + +
    + +
    + {% endif %} +
    + +
    + Last active: + {% if member and member.last_login %} + {{ member.last_login }} + {% elif portfolio_invitation %} + Invited + {% else %} + ⎯ + {% endif %} +
    + + Full name: + {% if member %} + {% if member.first_name or member.last_name %} + {{ member.get_formatted_name }} + {% else %} + ⎯ + {% endif %} + {% else %} + ⎯ + {% endif %} +
    + + Title or organization role: + {% if member and member.title %} + {{ member.title }} + {% else %} + ⎯ + {% endif %} +
    + + {% if portfolio_permission %} + {% include "includes/summary_item.html" with title='Member access and permissions' permissions=True value=portfolio_permission edit_link=edit_url editable=has_edit_members_portfolio_permission %} + {% elif portfolio_invitation %} + {% include "includes/summary_item.html" with title='Member access and permissions' permissions=True value=portfolio_invitation edit_link=edit_url editable=has_edit_members_portfolio_permission %} + {% endif %} + + {% comment %}view_button is passed below as true in all cases. This is because manage_button logic will trump view_button logic; ie. if manage_button is true, view_button will never be looked at{% endcomment %} + {% if portfolio_permission %} + {% include "includes/summary_item.html" with title='Domain management' domain_mgmt=True value=portfolio_permission.get_managed_domains_count edit_link='#' editable=True manage_button=has_edit_members_portfolio_permission view_button=True %} + {% elif portfolio_invitation %} + {% include "includes/summary_item.html" with title='Domain management' domain_mgmt=True value=portfolio_invitation.get_managed_domains_count edit_link='#' editable=True manage_button=has_edit_members_portfolio_permission view_button=True %} + {% else %} + {% include "includes/summary_item.html" with title='Domain management' domain_mgmt=True value=0 edit_link='#' editable=True manage_button=has_edit_members_portfolio_permission view_button=True %} + {% endif %} + +
    +{% endblock %} diff --git a/src/registrar/templates/portfolio_member_permissions.html b/src/registrar/templates/portfolio_member_permissions.html new file mode 100644 index 000000000..02d120360 --- /dev/null +++ b/src/registrar/templates/portfolio_member_permissions.html @@ -0,0 +1,42 @@ +{% extends 'portfolio_base.html' %} +{% load static field_helpers%} + +{% block title %}Organization member {% endblock %} + +{% load static %} + +{% block portfolio_content %} +
    +
    + + {% block messages %} + {% include "includes/form_messages.html" %} + {% endblock %} + +

    Manage member

    + +

    + {% if member %} + {{ member.email }} + {% elif invitation %} + {{ invitation.email }} + {% endif %} +

    + +
    + +
    + {% csrf_token %} + {% input_with_errors form.roles %} + {% input_with_errors form.additional_permissions %} + +
    + + + +
    +
    +{% endblock %} diff --git a/src/registrar/templates/portfolio_members.html b/src/registrar/templates/portfolio_members.html index 82e06c808..ffdb63099 100644 --- a/src/registrar/templates/portfolio_members.html +++ b/src/registrar/templates/portfolio_members.html @@ -18,6 +18,7 @@

    Members

    + {% if has_edit_members_portfolio_permission %}

    + {% endif %} {% include "includes/members_table.html" with portfolio=portfolio %} diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index a3f35ae8e..b29dccb08 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -246,9 +246,7 @@ def is_members_subpage(path): """Checks if the given page is a subpage of members. Takes a path name, like '/organization/'.""" # Since our pages aren't unified under a common path, we need this approach for now. - url_names = [ - "members", - ] + url_names = ["members", "member", "member-permissions", "invitedmember", "invitedmember-permissions"] return get_url_name(path) in url_names diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 33ae90da9..02902c1a9 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -152,12 +152,15 @@ class TestPortfolioInvitations(TestCase): self.invitation, _ = PortfolioInvitation.objects.get_or_create( email=self.email, portfolio=self.portfolio, - portfolio_roles=[self.portfolio_role_base, self.portfolio_role_admin], - portfolio_additional_permissions=[self.portfolio_permission_1, self.portfolio_permission_2], + roles=[self.portfolio_role_base, self.portfolio_role_admin], + additional_permissions=[self.portfolio_permission_1, self.portfolio_permission_2], ) def tearDown(self): super().tearDown() + DomainInvitation.objects.all().delete() + DomainInformation.objects.all().delete() + Domain.objects.all().delete() UserPortfolioPermission.objects.all().delete() Portfolio.objects.all().delete() PortfolioInvitation.objects.all().delete() @@ -209,8 +212,8 @@ class TestPortfolioInvitations(TestCase): PortfolioInvitation.objects.get_or_create( email=self.email, portfolio=portfolio2, - portfolio_roles=[self.portfolio_role_base, self.portfolio_role_admin], - portfolio_additional_permissions=[self.portfolio_permission_1, self.portfolio_permission_2], + roles=[self.portfolio_role_base, self.portfolio_role_admin], + additional_permissions=[self.portfolio_permission_1, self.portfolio_permission_2], ) with override_flag("multiple_portfolios", active=True): self.user.check_portfolio_invitations_on_login() @@ -233,8 +236,8 @@ class TestPortfolioInvitations(TestCase): PortfolioInvitation.objects.get_or_create( email=self.email, portfolio=portfolio2, - portfolio_roles=[self.portfolio_role_base, self.portfolio_role_admin], - portfolio_additional_permissions=[self.portfolio_permission_1, self.portfolio_permission_2], + roles=[self.portfolio_role_base, self.portfolio_role_admin], + additional_permissions=[self.portfolio_permission_1, self.portfolio_permission_2], ) self.user.check_portfolio_invitations_on_login() self.user.refresh_from_db() @@ -245,6 +248,52 @@ class TestPortfolioInvitations(TestCase): updated_invitation2, _ = PortfolioInvitation.objects.get_or_create(email=self.email, portfolio=portfolio2) self.assertEqual(updated_invitation2.status, PortfolioInvitation.PortfolioInvitationStatus.INVITED) + @less_console_noise_decorator + def test_get_managed_domains_count(self): + """Test that the correct number of domains, which are associated with the portfolio and + have invited the email of the portfolio invitation, are returned.""" + # Add three domains, one which is in the portfolio and email is invited to, + # one which is in the portfolio and email is not invited to, + # and one which is email is invited to and not in the portfolio. + # Arrange + # domain_in_portfolio should not be included in the count + domain_in_portfolio, _ = Domain.objects.get_or_create(name="domain_in_portfolio.gov", state=Domain.State.READY) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_in_portfolio, portfolio=self.portfolio) + # domain_in_portfolio_and_invited should be included in the count + domain_in_portfolio_and_invited, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_and_invited, portfolio=self.portfolio + ) + DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_and_invited) + # domain_invited should not be included in the count + domain_invited, _ = Domain.objects.get_or_create(name="domain_invited.gov", state=Domain.State.READY) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_invited) + DomainInvitation.objects.get_or_create(email=self.email, domain=domain_invited) + + # Assert + self.assertEqual(self.invitation.get_managed_domains_count(), 1) + + @less_console_noise_decorator + def test_get_portfolio_permissions(self): + """Test that get_portfolio_permissions returns the expected list of permissions, + based on the roles and permissions assigned to the invitation.""" + # Arrange + test_permission_list = set() + # add the arrays that are defined in UserPortfolioPermission for member and admin + test_permission_list.update( + UserPortfolioPermission.PORTFOLIO_ROLE_PERMISSIONS.get(UserPortfolioRoleChoices.ORGANIZATION_MEMBER, []) + ) + test_permission_list.update( + UserPortfolioPermission.PORTFOLIO_ROLE_PERMISSIONS.get(UserPortfolioRoleChoices.ORGANIZATION_ADMIN, []) + ) + # add the permissions that are added to the invitation as additional_permissions + test_permission_list.update([self.portfolio_permission_1, self.portfolio_permission_2]) + perm_list = list(test_permission_list) + # Verify + self.assertEquals(self.invitation.get_portfolio_permissions(), perm_list) + class TestUserPortfolioPermission(TestCase): @less_console_noise_decorator @@ -314,6 +363,40 @@ class TestUserPortfolioPermission(TestCase): ), ) + @less_console_noise_decorator + def test_get_managed_domains_count(self): + """Test that the correct number of managed domains associated with the portfolio + are returned.""" + # Add three domains, one which is in the portfolio and managed by the user, + # one which is in the portfolio and not managed by the user, + # and one which is managed by the user and not in the portfolio. + # Arrange + portfolio, _ = Portfolio.objects.get_or_create(creator=self.user, organization_name="Hotel California") + test_user = create_test_user() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( + portfolio=portfolio, user=test_user, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + # domain_in_portfolio should not be included in the count + domain_in_portfolio, _ = Domain.objects.get_or_create(name="domain_in_portfolio.gov", state=Domain.State.READY) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_in_portfolio, portfolio=portfolio) + # domain_in_portfolio_and_managed should be included in the count + domain_in_portfolio_and_managed, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_managed.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_and_managed, portfolio=portfolio + ) + UserDomainRole.objects.get_or_create( + user=test_user, domain=domain_in_portfolio_and_managed, role=UserDomainRole.Roles.MANAGER + ) + # domain_managed should not be included in the count + domain_managed, _ = Domain.objects.get_or_create(name="domain_managed.gov", state=Domain.State.READY) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_managed) + UserDomainRole.objects.get_or_create(user=test_user, domain=domain_managed, role=UserDomainRole.Roles.MANAGER) + + # Assert + self.assertEqual(portfolio_permission.get_managed_domains_count(), 1) + class TestUser(TestCase): """Test actions that occur on user login, diff --git a/src/registrar/tests/test_views_domains_json.py b/src/registrar/tests/test_views_domains_json.py index 07799104b..c4e5832c0 100644 --- a/src/registrar/tests/test_views_domains_json.py +++ b/src/registrar/tests/test_views_domains_json.py @@ -37,6 +37,7 @@ class GetDomainsJsonTest(TestWithUser, WebTest): UserDomainRole.objects.all().delete() UserPortfolioPermission.objects.all().delete() DomainInformation.objects.all().delete() + Domain.objects.all().delete() Portfolio.objects.all().delete() super().tearDown() diff --git a/src/registrar/tests/test_views_members_json.py b/src/registrar/tests/test_views_members_json.py index 75c3a3a66..9cd4e823c 100644 --- a/src/registrar/tests/test_views_members_json.py +++ b/src/registrar/tests/test_views_members_json.py @@ -1,6 +1,7 @@ from django.urls import reverse from registrar.models.portfolio import Portfolio +from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.user import User from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices @@ -38,6 +39,7 @@ class GetPortfolioMembersJsonTest(TestWithUser, WebTest): phone="8003114567", title="Admin", ) + cls.email5 = "fifth@example.com" # Create Portfolio cls.portfolio = Portfolio.objects.create(creator=cls.user, organization_name="Test Portfolio") @@ -67,6 +69,23 @@ class GetPortfolioMembersJsonTest(TestWithUser, WebTest): portfolio=cls.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], ) + PortfolioInvitation.objects.create( + email=cls.email5, + portfolio=cls.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + ) + + @classmethod + def tearDownClass(cls): + PortfolioInvitation.objects.all().delete() + UserPortfolioPermission.objects.all().delete() + Portfolio.objects.all().delete() + User.objects.all().delete() + super().tearDownClass() def setUp(self): super().setUp() @@ -83,14 +102,21 @@ class GetPortfolioMembersJsonTest(TestWithUser, WebTest): self.assertFalse(data["has_previous"]) self.assertFalse(data["has_next"]) self.assertEqual(data["num_pages"], 1) - self.assertEqual(data["total"], 4) - self.assertEqual(data["unfiltered_total"], 4) + self.assertEqual(data["total"], 5) + self.assertEqual(data["unfiltered_total"], 5) # Check the number of members - self.assertEqual(len(data["members"]), 4) + self.assertEqual(len(data["members"]), 5) # Check member fields - expected_emails = {self.user.email, self.user2.email, self.user3.email, self.user4.email} + expected_emails = { + self.user.email, + self.user2.email, + self.user3.email, + self.user4.email, + self.user4.email, + self.email5, + } actual_emails = {member["email"] for member in data["members"]} self.assertEqual(expected_emails, actual_emails) @@ -123,8 +149,8 @@ class GetPortfolioMembersJsonTest(TestWithUser, WebTest): self.assertTrue(data["has_next"]) self.assertFalse(data["has_previous"]) self.assertEqual(data["num_pages"], 2) - self.assertEqual(data["total"], 14) - self.assertEqual(data["unfiltered_total"], 14) + self.assertEqual(data["total"], 15) + self.assertEqual(data["unfiltered_total"], 15) # Check the number of members on page 1 self.assertEqual(len(data["members"]), 10) @@ -142,7 +168,7 @@ class GetPortfolioMembersJsonTest(TestWithUser, WebTest): self.assertEqual(data["num_pages"], 2) # Check the number of members on page 2 - self.assertEqual(len(data["members"]), 4) + self.assertEqual(len(data["members"]), 5) def test_search(self): """Test search functionality for portfolio members.""" diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index dfb0469d0..13173565c 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -10,6 +10,7 @@ from registrar.models import ( UserDomainRole, User, ) +from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.user_group import UserGroup from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices @@ -288,9 +289,9 @@ class TestPortfolio(WebTest): def test_accessible_pages_when_user_does_not_have_role(self): """Test that admin / memmber roles are associated with the right access""" self.app.set_user(self.user.username) - portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( - user=self.user, portfolio=self.portfolio, roles=portfolio_roles + user=self.user, portfolio=self.portfolio, roles=roles ) with override_flag("organization_feature", active=True): # This will redirect the user to the portfolio page. @@ -398,8 +399,8 @@ class TestPortfolio(WebTest): """When organization_feature flag is true and user has a portfolio, the portfolio should be set in session.""" self.client.force_login(self.user) - portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, roles=portfolio_roles) + roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, roles=roles) with override_flag("organization_feature", active=True): response = self.client.get(reverse("home")) # Ensure that middleware processes the session @@ -420,8 +421,8 @@ class TestPortfolio(WebTest): This test also satisfies the condition when multiple_portfolios flag is false and user has a portfolio, so won't add a redundant test for that.""" self.client.force_login(self.user) - portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, roles=portfolio_roles) + roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, roles=roles) response = self.client.get(reverse("home")) # Ensure that middleware processes the session session_middleware = SessionMiddleware(lambda request: None) @@ -457,8 +458,8 @@ class TestPortfolio(WebTest): """When multiple_portfolios flag is true and user has a portfolio, the portfolio should be set in session.""" self.client.force_login(self.user) - portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, roles=portfolio_roles) + roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, roles=roles) with override_flag("organization_feature", active=True), override_flag("multiple_portfolios", active=True): response = self.client.get(reverse("home")) # Ensure that middleware processes the session @@ -817,7 +818,6 @@ class TestPortfolio(WebTest): # Verify that view-only settings are sent in the dynamic HTML response = self.client.get(reverse("get_portfolio_members_json") + f"?portfolio={self.portfolio.pk}") - print(response.content) self.assertContains(response, '"action_label": "View"') self.assertContains(response, '"svg_icon": "visibility"') @@ -856,6 +856,230 @@ class TestPortfolio(WebTest): # TerminalHelper.colorful_logger(logger.info, TerminalColors.OKCYAN, f"{response.content}") self.assertContains(response, '"is_admin": true') + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + def test_cannot_view_member_page_when_flag_is_off(self): + """Test that user cannot access the member page when waffle flag is off""" + + # Verify that the user cannot access the member page + self.client.force_login(self.user) + response = self.client.get(reverse("member", kwargs={"pk": 1}), follow=True) + # Make sure the page is denied + self.assertEqual(response.status_code, 403) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_cannot_view_member_page_when_user_has_no_permission(self): + """Test that user cannot access the member page without proper permission""" + + # give user base permissions + UserPortfolioPermission.objects.get_or_create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + + # Verify that the user cannot access the member page + self.client.force_login(self.user) + response = self.client.get(reverse("member", kwargs={"pk": 1}), follow=True) + # Make sure the page is denied + self.assertEqual(response.status_code, 403) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_can_view_member_page_when_user_has_view_members(self): + """Test that user can access the member page with view_members permission""" + + # Arrange + # give user permissions to view members + permission_obj, _ = UserPortfolioPermission.objects.get_or_create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + ], + ) + + # Verify the page can be accessed + self.client.force_login(self.user) + response = self.client.get(reverse("member", kwargs={"pk": permission_obj.pk}), follow=True) + self.assertEqual(response.status_code, 200) + + # Assert text within the page is correct + self.assertContains(response, "First Last") + self.assertContains(response, self.user.email) + self.assertContains(response, "Basic access") + self.assertContains(response, "No access") + self.assertContains(response, "View all members") + self.assertContains(response, "This member does not manage any domains.") + + # Assert buttons and links within the page are correct + self.assertNotContains(response, "usa-button--more-actions") # test that 3 dot is not present + self.assertNotContains(response, "sprite.svg#edit") # test that Edit link is not present + self.assertNotContains(response, "sprite.svg#settings") # test that Manage link is not present + self.assertContains(response, "sprite.svg#visibility") # test that View link is present + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_can_view_member_page_when_user_has_edit_members(self): + """Test that user can access the member page with edit_members permission""" + + # Arrange + # give user permissions to view AND manage members + permission_obj, _ = UserPortfolioPermission.objects.get_or_create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + ) + + # Verify the page can be accessed + self.client.force_login(self.user) + response = self.client.get(reverse("member", kwargs={"pk": permission_obj.pk}), follow=True) + self.assertEqual(response.status_code, 200) + + # Assert text within the page is correct + self.assertContains(response, "First Last") + self.assertContains(response, self.user.email) + self.assertContains(response, "Admin access") + self.assertContains(response, "View all requests plus create requests") + self.assertContains(response, "View all members plus manage members") + self.assertContains( + response, 'This member does not manage any domains. To assign this member a domain, click "Manage"' + ) + + # Assert buttons and links within the page are correct + self.assertContains(response, "usa-button--more-actions") # test that 3 dot is present + self.assertContains(response, "sprite.svg#edit") # test that Edit link is present + self.assertContains(response, "sprite.svg#settings") # test that Manage link is present + self.assertNotContains(response, "sprite.svg#visibility") # test that View link is not present + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + def test_cannot_view_invitedmember_page_when_flag_is_off(self): + """Test that user cannot access the invitedmember page when waffle flag is off""" + + # Verify that the user cannot access the member page + self.client.force_login(self.user) + response = self.client.get(reverse("invitedmember", kwargs={"pk": 1}), follow=True) + # Make sure the page is denied + self.assertEqual(response.status_code, 403) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_cannot_view_invitedmember_page_when_user_has_no_permission(self): + """Test that user cannot access the invitedmember page without proper permission""" + + # give user base permissions + UserPortfolioPermission.objects.get_or_create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + + # Verify that the user cannot access the member page + self.client.force_login(self.user) + response = self.client.get(reverse("invitedmember", kwargs={"pk": 1}), follow=True) + # Make sure the page is denied + self.assertEqual(response.status_code, 403) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_can_view_invitedmember_page_when_user_has_view_members(self): + """Test that user can access the invitedmember page with view_members permission""" + + # Arrange + # give user permissions to view members + UserPortfolioPermission.objects.get_or_create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + ], + ) + portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create( + email="info@example.com", + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + ], + ) + + # Verify the page can be accessed + self.client.force_login(self.user) + response = self.client.get(reverse("invitedmember", kwargs={"pk": portfolio_invitation.pk}), follow=True) + self.assertEqual(response.status_code, 200) + + # Assert text within the page is correct + self.assertContains(response, "Invited") + self.assertContains(response, portfolio_invitation.email) + self.assertContains(response, "Basic access") + self.assertContains(response, "No access") + self.assertContains(response, "View all members") + self.assertContains(response, "This member does not manage any domains.") + + # Assert buttons and links within the page are correct + self.assertNotContains(response, "usa-button--more-actions") # test that 3 dot is not present + self.assertNotContains(response, "sprite.svg#edit") # test that Edit link is not present + self.assertNotContains(response, "sprite.svg#settings") # test that Manage link is not present + self.assertContains(response, "sprite.svg#visibility") # test that View link is present + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_can_view_invitedmember_page_when_user_has_edit_members(self): + """Test that user can access the invitedmember page with edit_members permission""" + + # Arrange + # give user permissions to view AND manage members + permission_obj, _ = UserPortfolioPermission.objects.get_or_create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + ) + portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create( + email="info@example.com", + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + ) + + # Verify the page can be accessed + self.client.force_login(self.user) + response = self.client.get(reverse("invitedmember", kwargs={"pk": portfolio_invitation.pk}), follow=True) + self.assertEqual(response.status_code, 200) + + # Assert text within the page is correct + self.assertContains(response, "Invited") + self.assertContains(response, portfolio_invitation.email) + self.assertContains(response, "Admin access") + self.assertContains(response, "View all requests plus create requests") + self.assertContains(response, "View all members plus manage members") + self.assertContains( + response, 'This member does not manage any domains. To assign this member a domain, click "Manage"' + ) + + # Assert buttons and links within the page are correct + self.assertContains(response, "usa-button--more-actions") # test that 3 dot is present + self.assertContains(response, "sprite.svg#edit") # test that Edit link is present + self.assertContains(response, "sprite.svg#settings") # test that Manage link is present + self.assertNotContains(response, "sprite.svg#visibility") # test that View link is not present + @less_console_noise_decorator @override_flag("organization_feature", active=True) def test_portfolio_domain_requests_page_when_user_has_no_permissions(self): @@ -1015,8 +1239,8 @@ class TestPortfolio(WebTest): def test_portfolio_cache_updates_when_modified(self): """Test that the portfolio in session updates when the portfolio is modified""" self.client.force_login(self.user) - portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, roles=portfolio_roles) + roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, roles=roles) with override_flag("organization_feature", active=True): # Initial request to set the portfolio in session @@ -1044,8 +1268,8 @@ class TestPortfolio(WebTest): def test_portfolio_cache_updates_when_flag_disabled_while_logged_in(self): """Test that the portfolio in session is set to None when the organization_feature flag is disabled""" self.client.force_login(self.user) - portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, roles=portfolio_roles) + roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, roles=roles) with override_flag("organization_feature", active=True): # Initial request to set the portfolio in session diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index 133e6750e..d2f2276cf 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -1,45 +1,41 @@ from django.http import JsonResponse from django.core.paginator import Paginator from django.contrib.auth.decorators import login_required -from django.db.models import Q +from django.db.models import Value, F, CharField, TextField, Q, Case, When +from django.db.models.functions import Concat, Coalesce +from django.urls import reverse +from django.db.models.functions import Cast from registrar.models.portfolio_invitation import PortfolioInvitation -from registrar.models.user import User from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices @login_required def get_portfolio_members_json(request): - """Given the current request, - get all members that are associated with the given portfolio""" + """Fetch members (permissions and invitations) for the given portfolio.""" + portfolio = request.GET.get("portfolio") - member_ids = get_member_ids_from_request(request, portfolio) - objects = User.objects.filter(id__in=member_ids) - admin_ids = UserPortfolioPermission.objects.filter( - portfolio=portfolio, - roles__overlap=[ - UserPortfolioRoleChoices.ORGANIZATION_ADMIN, - ], - ).values_list("user__id", flat=True) - portfolio_invitation_emails = PortfolioInvitation.objects.filter(portfolio=portfolio).values_list( - "email", flat=True - ) + # Two initial querysets which will be combined + permissions = initial_permissions_search(portfolio) + invitations = initial_invitations_search(portfolio) - unfiltered_total = objects.count() + # Get total across both querysets before applying filters + unfiltered_total = permissions.count() + invitations.count() - objects = apply_search(objects, request) - # objects = apply_status_filter(objects, request) + permissions = apply_search_term(permissions, request) + invitations = apply_search_term(invitations, request) + + # Union the two querysets + objects = permissions.union(invitations) objects = apply_sorting(objects, request) paginator = Paginator(objects, 10) page_number = request.GET.get("page", 1) page_obj = paginator.get_page(page_number) - members = [ - serialize_members(request, portfolio, member, request.user, admin_ids, portfolio_invitation_emails) - for member in page_obj.object_list - ] + + members = [serialize_members(request, portfolio, item, request.user) for item in page_obj.object_list] return JsonResponse( { @@ -54,71 +50,121 @@ def get_portfolio_members_json(request): ) -def get_member_ids_from_request(request, portfolio): - """Given the current request, - get all members that are associated with the given portfolio""" - member_ids = [] - if portfolio: - member_ids = UserPortfolioPermission.objects.filter(portfolio=portfolio).values_list("user__id", flat=True) - return member_ids +def initial_permissions_search(portfolio): + """Perform initial search for permissions before applying any filters.""" + permissions = UserPortfolioPermission.objects.filter(portfolio=portfolio) + permissions = ( + permissions.select_related("user") + .annotate( + first_name=F("user__first_name"), + last_name=F("user__last_name"), + email_display=F("user__email"), + last_active=Cast(F("user__last_login"), output_field=TextField()), # Cast last_login to text + additional_permissions_display=F("additional_permissions"), + member_display=Case( + # If email is present and not blank, use email + When(Q(user__email__isnull=False) & ~Q(user__email=""), then=F("user__email")), + # If first name or last name is present, use concatenation of first_name + " " + last_name + When( + Q(user__first_name__isnull=False) | Q(user__last_name__isnull=False), + then=Concat( + Coalesce(F("user__first_name"), Value("")), + Value(" "), + Coalesce(F("user__last_name"), Value("")), + ), + ), + # If neither, use an empty string + default=Value(""), + output_field=CharField(), + ), + source=Value("permission", output_field=CharField()), + ) + .values( + "id", + "first_name", + "last_name", + "email_display", + "last_active", + "roles", + "additional_permissions_display", + "member_display", + "source", + ) + ) + return permissions -def apply_search(queryset, request): - search_term = request.GET.get("search_term") +def initial_invitations_search(portfolio): + """Perform initial invitations search before applying any filters.""" + invitations = PortfolioInvitation.objects.filter(portfolio=portfolio) + invitations = invitations.annotate( + first_name=Value(None, output_field=CharField()), + last_name=Value(None, output_field=CharField()), + email_display=F("email"), + last_active=Value("Invited", output_field=TextField()), + additional_permissions_display=F("additional_permissions"), + member_display=F("email"), + source=Value("invitation", output_field=CharField()), + ).values( + "id", + "first_name", + "last_name", + "email_display", + "last_active", + "roles", + "additional_permissions_display", + "member_display", + "source", + ) + return invitations + +def apply_search_term(queryset, request): + """Apply search term to the queryset.""" + search_term = request.GET.get("search_term", "").lower() if search_term: queryset = queryset.filter( - Q(username__icontains=search_term) - | Q(first_name__icontains=search_term) + Q(first_name__icontains=search_term) | Q(last_name__icontains=search_term) - | Q(email__icontains=search_term) + | Q(email_display__icontains=search_term) ) return queryset def apply_sorting(queryset, request): + """Apply sorting to the queryset.""" sort_by = request.GET.get("sort_by", "id") # Default to 'id' order = request.GET.get("order", "asc") # Default to 'asc' - + # Adjust sort_by to match the annotated fields in the unioned queryset if sort_by == "member": - sort_by = ["email", "first_name", "middle_name", "last_name"] - else: - sort_by = [sort_by] - + sort_by = "member_display" if order == "desc": - sort_by = [f"-{field}" for field in sort_by] - - return queryset.order_by(*sort_by) + queryset = queryset.order_by(F(sort_by).desc()) + else: + queryset = queryset.order_by(sort_by) + return queryset -def serialize_members(request, portfolio, member, user, admin_ids, portfolio_invitation_emails): - # ------- VIEW ONLY - # If not view_only (the user has permissions to edit/manage users), show the gear icon with "Manage" link. - # If view_only (the user only has view user permissions), show the "View" link (no gear icon). - # We check on user_group_permision to account for the upcoming "Manage portfolio" button on admin. - user_can_edit_other_users = False - for user_group_permission in ["registrar.full_access_permission", "registrar.change_user"]: - if user.has_perm(user_group_permission): - user_can_edit_other_users = True - break +def serialize_members(request, portfolio, item, user): + # Check if the user can edit other users + user_can_edit_other_users = any( + user.has_perm(perm) for perm in ["registrar.full_access_permission", "registrar.change_user"] + ) view_only = not user.has_edit_members_portfolio_permission(portfolio) or not user_can_edit_other_users - # ------- USER STATUSES - is_invited = member.email in portfolio_invitation_emails - last_active = "Invited" if is_invited else "Unknown" - if member.last_login: - last_active = member.last_login.strftime("%b. %d, %Y") - is_admin = member.id in admin_ids + is_admin = UserPortfolioRoleChoices.ORGANIZATION_ADMIN in (item.get("roles") or []) + action_url = reverse("member" if item["source"] == "permission" else "invitedmember", kwargs={"pk": item["id"]}) - # ------- SERIALIZE + # Serialize member data member_json = { - "id": member.id, - "name": member.get_formatted_name(), - "email": member.email, + "id": item.get("id", ""), + "name": " ".join(filter(None, [item.get("first_name", ""), item.get("last_name", "")])), + "email": item.get("email_display", ""), + "member_display": item.get("member_display", ""), "is_admin": is_admin, - "last_active": last_active, - "action_url": "#", # reverse("members", kwargs={"pk": member.id}), # TODO: Future ticket? + "last_active": item.get("last_active", ""), + "action_url": action_url, "action_label": ("View" if view_only else "Manage"), "svg_icon": ("visibility" if view_only else "settings"), } diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 552fdb6ff..cc1a09b25 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -3,20 +3,30 @@ from django.http import Http404 from django.shortcuts import render from django.urls import reverse from django.contrib import messages -from registrar.forms.portfolio import PortfolioOrgAddressForm, PortfolioSeniorOfficialForm +from registrar.forms.portfolio import ( + PortfolioInvitedMemberForm, + PortfolioMemberForm, + PortfolioOrgAddressForm, + PortfolioSeniorOfficialForm, +) from registrar.models import Portfolio, User +from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.user_portfolio_permission import UserPortfolioPermission -from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from registrar.views.utility.permission_views import ( PortfolioDomainRequestsPermissionView, PortfolioDomainsPermissionView, PortfolioBasePermissionView, NoPortfolioDomainsPermissionView, + PortfolioInvitedMemberEditPermissionView, + PortfolioInvitedMemberPermissionView, + PortfolioMemberEditPermissionView, + PortfolioMemberPermissionView, PortfolioMembersPermissionView, ) from django.views.generic import View from django.views.generic.edit import FormMixin - +from django.shortcuts import get_object_or_404, redirect logger = logging.getLogger(__name__) @@ -51,6 +61,155 @@ class PortfolioMembersView(PortfolioMembersPermissionView, View): return render(request, "portfolio_members.html") +class PortfolioMemberView(PortfolioMemberPermissionView, View): + + template_name = "portfolio_member.html" + + def get(self, request, pk): + portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) + member = portfolio_permission.user + + # We have to explicitely name these with member_ otherwise we'll have conflicts with context preprocessors + member_has_view_all_requests_portfolio_permission = member.has_view_all_requests_portfolio_permission( + portfolio_permission.portfolio + ) + member_has_edit_request_portfolio_permission = member.has_edit_request_portfolio_permission( + portfolio_permission.portfolio + ) + member_has_view_members_portfolio_permission = member.has_view_members_portfolio_permission( + portfolio_permission.portfolio + ) + member_has_edit_members_portfolio_permission = member.has_edit_members_portfolio_permission( + portfolio_permission.portfolio + ) + + return render( + request, + self.template_name, + { + "edit_url": reverse("member-permissions", args=[pk]), + "portfolio_permission": portfolio_permission, + "member": member, + "member_has_view_all_requests_portfolio_permission": member_has_view_all_requests_portfolio_permission, + "member_has_edit_request_portfolio_permission": member_has_edit_request_portfolio_permission, + "member_has_view_members_portfolio_permission": member_has_view_members_portfolio_permission, + "member_has_edit_members_portfolio_permission": member_has_edit_members_portfolio_permission, + }, + ) + + +class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View): + + template_name = "portfolio_member_permissions.html" + form_class = PortfolioMemberForm + + def get(self, request, pk): + portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) + user = portfolio_permission.user + + form = self.form_class(instance=portfolio_permission) + + return render( + request, + self.template_name, + { + "form": form, + "member": user, + }, + ) + + def post(self, request, pk): + portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) + user = portfolio_permission.user + + form = self.form_class(request.POST, instance=portfolio_permission) + + if form.is_valid(): + form.save() + return redirect("member", pk=pk) + + return render( + request, + self.template_name, + { + "form": form, + "member": user, # Pass the user object again to the template + }, + ) + + +class PortfolioInvitedMemberView(PortfolioInvitedMemberPermissionView, View): + + template_name = "portfolio_member.html" + # form_class = PortfolioInvitedMemberForm + + def get(self, request, pk): + portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) + # form = self.form_class(instance=portfolio_invitation) + + # We have to explicitely name these with member_ otherwise we'll have conflicts with context preprocessors + member_has_view_all_requests_portfolio_permission = ( + UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS in portfolio_invitation.get_portfolio_permissions() + ) + member_has_edit_request_portfolio_permission = ( + UserPortfolioPermissionChoices.EDIT_REQUESTS in portfolio_invitation.get_portfolio_permissions() + ) + member_has_view_members_portfolio_permission = ( + UserPortfolioPermissionChoices.VIEW_MEMBERS in portfolio_invitation.get_portfolio_permissions() + ) + member_has_edit_members_portfolio_permission = ( + UserPortfolioPermissionChoices.EDIT_MEMBERS in portfolio_invitation.get_portfolio_permissions() + ) + + return render( + request, + self.template_name, + { + "edit_url": reverse("invitedmember-permissions", args=[pk]), + "portfolio_invitation": portfolio_invitation, + "member_has_view_all_requests_portfolio_permission": member_has_view_all_requests_portfolio_permission, + "member_has_edit_request_portfolio_permission": member_has_edit_request_portfolio_permission, + "member_has_view_members_portfolio_permission": member_has_view_members_portfolio_permission, + "member_has_edit_members_portfolio_permission": member_has_edit_members_portfolio_permission, + }, + ) + + +class PortfolioInvitedMemberEditView(PortfolioInvitedMemberEditPermissionView, View): + + template_name = "portfolio_member_permissions.html" + form_class = PortfolioInvitedMemberForm + + def get(self, request, pk): + portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) + form = self.form_class(instance=portfolio_invitation) + + return render( + request, + self.template_name, + { + "form": form, + "invitation": portfolio_invitation, + }, + ) + + def post(self, request, pk): + portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) + form = self.form_class(request.POST, instance=portfolio_invitation) + if form.is_valid(): + form.save() + return redirect("invitedmember", pk=pk) + + return render( + request, + self.template_name, + { + "form": form, + "invitation": portfolio_invitation, # Pass the user object again to the template + }, + ) + + class PortfolioNoDomainsView(NoPortfolioDomainsPermissionView, View): """Some users have access to the underlying portfolio, but not any domains. This is a custom view which explains that to the user - and denotes who to contact. diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index f400e7f4b..9cee2f61a 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -512,7 +512,81 @@ class PortfolioMembersPermission(PortfolioBasePermission): up from the portfolio's primary key in self.kwargs["pk"]""" portfolio = self.request.session.get("portfolio") - if not self.request.user.has_view_members_portfolio_permission(portfolio): + if not self.request.user.has_view_members_portfolio_permission( + portfolio + ) and not self.request.user.has_edit_members_portfolio_permission(portfolio): + return False + + return super().has_permission() + + +class PortfolioMemberPermission(PortfolioBasePermission): + """Permission mixin that allows access to portfolio member pages if user + has access, otherwise 403""" + + def has_permission(self): + """Check if this user has access to members for this portfolio. + + The user is in self.request.user and the portfolio can be looked + up from the portfolio's primary key in self.kwargs["pk"]""" + + portfolio = self.request.session.get("portfolio") + if not self.request.user.has_view_members_portfolio_permission( + portfolio + ) and not self.request.user.has_edit_members_portfolio_permission(portfolio): + return False + + return super().has_permission() + + +class PortfolioMemberEditPermission(PortfolioBasePermission): + """Permission mixin that allows access to portfolio member pages if user + has access to edit, otherwise 403""" + + def has_permission(self): + """Check if this user has access to members for this portfolio. + + The user is in self.request.user and the portfolio can be looked + up from the portfolio's primary key in self.kwargs["pk"]""" + + portfolio = self.request.session.get("portfolio") + if not self.request.user.has_edit_members_portfolio_permission(portfolio): + return False + + return super().has_permission() + + +class PortfolioInvitedMemberPermission(PortfolioBasePermission): + """Permission mixin that allows access to portfolio invited member pages if user + has access, otherwise 403""" + + def has_permission(self): + """Check if this user has access to members for this portfolio. + + The user is in self.request.user and the portfolio can be looked + up from the portfolio's primary key in self.kwargs["pk"]""" + + portfolio = self.request.session.get("portfolio") + if not self.request.user.has_view_members_portfolio_permission( + portfolio + ) and not self.request.user.has_edit_members_portfolio_permission(portfolio): + return False + + return super().has_permission() + + +class PortfolioInvitedMemberEditPermission(PortfolioBasePermission): + """Permission mixin that allows access to portfolio invited member pages if user + has access to edit, otherwise 403""" + + def has_permission(self): + """Check if this user has access to members for this portfolio. + + The user is in self.request.user and the portfolio can be looked + up from the portfolio's primary key in self.kwargs["pk"]""" + + portfolio = self.request.session.get("portfolio") + if not self.request.user.has_edit_members_portfolio_permission(portfolio): return False return super().has_permission() diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py index 414e58275..c1d25d691 100644 --- a/src/registrar/views/utility/permission_views.py +++ b/src/registrar/views/utility/permission_views.py @@ -15,10 +15,14 @@ from .mixins import ( DomainRequestWizardPermission, PortfolioDomainRequestsPermission, PortfolioDomainsPermission, + PortfolioInvitedMemberEditPermission, + PortfolioInvitedMemberPermission, + PortfolioMemberEditPermission, UserDeleteDomainRolePermission, UserProfilePermission, PortfolioBasePermission, PortfolioMembersPermission, + PortfolioMemberPermission, DomainRequestPortfolioViewonlyPermission, ) import logging @@ -253,7 +257,41 @@ class PortfolioDomainRequestsPermissionView(PortfolioDomainRequestsPermission, P class PortfolioMembersPermissionView(PortfolioMembersPermission, PortfolioBasePermissionView, abc.ABC): - """Abstract base view for portfolio domain request views that enforces permissions. + """Abstract base view for portfolio members views that enforces permissions. + + This abstract view cannot be instantiated. Actual views must specify + `template_name`. + """ + + +class PortfolioMemberPermissionView(PortfolioMemberPermission, PortfolioBasePermissionView, abc.ABC): + """Abstract base view for portfolio member views that enforces permissions. + + This abstract view cannot be instantiated. Actual views must specify + `template_name`. + """ + + +class PortfolioMemberEditPermissionView(PortfolioMemberEditPermission, PortfolioBasePermissionView, abc.ABC): + """Abstract base view for portfolio member edit views that enforces permissions. + + This abstract view cannot be instantiated. Actual views must specify + `template_name`. + """ + + +class PortfolioInvitedMemberPermissionView(PortfolioInvitedMemberPermission, PortfolioBasePermissionView, abc.ABC): + """Abstract base view for portfolio member views that enforces permissions. + + This abstract view cannot be instantiated. Actual views must specify + `template_name`. + """ + + +class PortfolioInvitedMemberEditPermissionView( + PortfolioInvitedMemberEditPermission, PortfolioBasePermissionView, abc.ABC +): + """Abstract base view for portfolio member edit views that enforces permissions. This abstract view cannot be instantiated. Actual views must specify `template_name`. diff --git a/src/zap.conf b/src/zap.conf index dd9ae1565..1f0548f2d 100644 --- a/src/zap.conf +++ b/src/zap.conf @@ -71,6 +71,7 @@ 10038 OUTOFSCOPE http://app:8080/domain_requests/ 10038 OUTOFSCOPE http://app:8080/domains/ 10038 OUTOFSCOPE http://app:8080/organization/ +10038 OUTOFSCOPE http://app:8080/permissions 10038 OUTOFSCOPE http://app:8080/suborganization/ 10038 OUTOFSCOPE http://app:8080/transfer/ # This URL always returns 404, so include it as well.