This member is assigned to ${num_domains} domains:
`; + domainsHTML += "Domains: Can view all organization domains. Can manage domains they are assigned to and edit information about the domain (including DNS settings).
"; + } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS)) { + permissionsHTML += "Domains: Can manage domains they are assigned to and edit information about the domain (including DNS settings).
"; + } + + // Check request-related permissions + if (member_permissions.includes(UserPortfolioPermissionChoices.EDIT_REQUESTS)) { + permissionsHTML += "Domain requests: Can view all organization domain requests. Can create domain requests and modify their own requests.
"; + } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS)) { + permissionsHTML += "Domain requests (view-only): Can view all organization domain requests. Can't create or modify any domain requests.
"; + } + + // Check member-related permissions + if (member_permissions.includes(UserPortfolioPermissionChoices.EDIT_MEMBERS)) { + permissionsHTML += "Members: Can manage members including inviting new members, removing current members, and assigning domains to members.
"; + } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_MEMBERS)) { + permissionsHTML += "Members (view-only): Can view all organizational members. Can't manage any members.
"; + } + + // If no specific permissions are assigned, display a message indicating no additional permissions + if (!permissionsHTML) { + permissionsHTML += "No additional permissions: There are no additional permissions for this member.
"; + } + + // Add a permissions header and wrap the entire output in a container + permissionsHTML = "| Member | -Last Active | +Member | +Last Active | Action | diff --git a/src/registrar/tests/test_views_members_json.py b/src/registrar/tests/test_views_members_json.py index 9cd4e823c..a97a517e2 100644 --- a/src/registrar/tests/test_views_members_json.py +++ b/src/registrar/tests/test_views_members_json.py @@ -1,21 +1,25 @@ from django.urls import reverse +from registrar.models.domain import Domain +from registrar.models.domain_information import DomainInformation +from registrar.models.domain_invitation import DomainInvitation from registrar.models.portfolio import Portfolio from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.user import User +from registrar.models.user_domain_role import UserDomainRole from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices -from .test_views import TestWithUser +from registrar.tests.common import MockEppLib, create_test_user from django_webtest import WebTest # type: ignore -class GetPortfolioMembersJsonTest(TestWithUser, WebTest): - @classmethod - def setUpClass(cls): - super().setUpClass() +class GetPortfolioMembersJsonTest(MockEppLib, WebTest): + def setUp(self): + super().setUp() + self.user = create_test_user() # Create additional users - cls.user2 = User.objects.create( + self.user2 = User.objects.create( username="test_user2", first_name="Second", last_name="User", @@ -23,7 +27,7 @@ def setUpClass(cls): phone="8003112345", title="Member", ) - cls.user3 = User.objects.create( + self.user3 = User.objects.create( username="test_user3", first_name="Third", last_name="User", @@ -31,7 +35,7 @@ def setUpClass(cls): phone="8003113456", title="Member", ) - cls.user4 = User.objects.create( + self.user4 = User.objects.create( username="test_user4", first_name="Fourth", last_name="User", @@ -39,15 +43,39 @@ def setUpClass(cls): phone="8003114567", title="Admin", ) - cls.email5 = "fifth@example.com" + self.user5 = User.objects.create( + username="test_user5", + first_name="Fifth", + last_name="User", + email="fifth@example.com", + phone="8003114568", + title="Admin", + ) + self.email6 = "fifth@example.com" # Create Portfolio - cls.portfolio = Portfolio.objects.create(creator=cls.user, organization_name="Test Portfolio") + self.portfolio = Portfolio.objects.create(creator=self.user, organization_name="Test Portfolio") # Assign permissions + + self.app.set_user(self.user.username) + + def tearDown(self): + UserDomainRole.objects.all().delete() + DomainInformation.objects.all().delete() + Domain.objects.all().delete() + PortfolioInvitation.objects.all().delete() + UserPortfolioPermission.objects.all().delete() + Portfolio.objects.all().delete() + User.objects.all().delete() + super().tearDown() + + def test_get_portfolio_members_json_authenticated(self): + """Test that portfolio members are returned properly for an authenticated user.""" + """Also tests that reposnse is 200 when no domains""" UserPortfolioPermission.objects.create( - user=cls.user, - portfolio=cls.portfolio, + user=self.user, + portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], additional_permissions=[ UserPortfolioPermissionChoices.VIEW_MEMBERS, @@ -55,44 +83,26 @@ def setUpClass(cls): ], ) UserPortfolioPermission.objects.create( - user=cls.user2, - portfolio=cls.portfolio, + user=self.user2, + portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], ) UserPortfolioPermission.objects.create( - user=cls.user3, - portfolio=cls.portfolio, + user=self.user3, + portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], ) UserPortfolioPermission.objects.create( - user=cls.user4, - portfolio=cls.portfolio, + user=self.user4, + portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], ) - PortfolioInvitation.objects.create( - email=cls.email5, - portfolio=cls.portfolio, + UserPortfolioPermission.objects.create( + user=self.user5, + portfolio=self.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() - self.app.set_user(self.user.username) - - def test_get_portfolio_members_json_authenticated(self): - """Test that portfolio members are returned properly for an authenticated user.""" response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id}) self.assertEqual(response.status_code, 200) data = response.json @@ -115,15 +125,228 @@ def test_get_portfolio_members_json_authenticated(self): self.user3.email, self.user4.email, self.user4.email, - self.email5, + self.user5.email, + } + actual_emails = {member["email"] for member in data["members"]} + self.assertEqual(expected_emails, actual_emails) + + expected_roles = { + UserPortfolioRoleChoices.ORGANIZATION_MEMBER, + UserPortfolioRoleChoices.ORGANIZATION_ADMIN, } + # Convert each member's roles list to a frozenset + actual_roles = {role for member in data["members"] for role in member["roles"]} + self.assertEqual(expected_roles, actual_roles) + + # Assert that the expected additional permissions are in the actual entire permissions list + expected_additional_permissions = { + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + } + # actual_permissions includes additional permissions as well as permissions from roles + actual_permissions = {permission for member in data["members"] for permission in member["permissions"]} + self.assertTrue(expected_additional_permissions.issubset(actual_permissions)) + + def test_get_portfolio_invited_json_authenticated(self): + """Test that portfolio invitees are returned properly for an authenticated user.""" + """Also tests that reposnse is 200 when no domains""" + PortfolioInvitation.objects.create( + email=self.email6, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + ) + + response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id}) + self.assertEqual(response.status_code, 200) + data = response.json + + # Check pagination info + self.assertEqual(data["page"], 1) + self.assertEqual(data["num_pages"], 1) + self.assertEqual(data["total"], 1) + self.assertEqual(data["unfiltered_total"], 1) + + # Check the number of members + self.assertEqual(len(data["members"]), 1) + + # Check member fields + expected_emails = {self.email6} actual_emails = {member["email"] for member in data["members"]} self.assertEqual(expected_emails, actual_emails) + expected_roles = { + UserPortfolioRoleChoices.ORGANIZATION_ADMIN, + } + # Convert each member's roles list to a frozenset + actual_roles = {role for member in data["members"] for role in member["roles"]} + self.assertEqual(expected_roles, actual_roles) + + expected_additional_permissions = { + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + } + actual_additional_permissions = { + permission for member in data["members"] for permission in member["permissions"] + } + self.assertTrue(expected_additional_permissions.issubset(actual_additional_permissions)) + + def test_get_portfolio_members_json_with_domains(self): + """Test that portfolio members are returned properly for an authenticated user and the response includes + the domains that the member manages..""" + UserPortfolioPermission.objects.create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + ) + UserPortfolioPermission.objects.create( + user=self.user2, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + UserPortfolioPermission.objects.create( + user=self.user3, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + UserPortfolioPermission.objects.create( + user=self.user4, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + ) + + # create domain for which user is manager and domain in portfolio + domain = Domain.objects.create( + name="somedomain1.com", + ) + DomainInformation.objects.create( + creator=self.user, + domain=domain, + portfolio=self.portfolio, + ) + UserDomainRole.objects.create( + user=self.user, + domain=domain, + role=UserDomainRole.Roles.MANAGER, + ) + + # create domain for which user is manager and domain not in portfolio + domain2 = Domain.objects.create( + name="somedomain2.com", + ) + DomainInformation.objects.create( + creator=self.user, + domain=domain2, + ) + UserDomainRole.objects.create( + user=self.user, + domain=domain2, + role=UserDomainRole.Roles.MANAGER, + ) + + response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id}) + self.assertEqual(response.status_code, 200) + data = response.json + + # Check if the domain appears in the response JSON and that domain2 does not + domain_names = [domain_name for member in data["members"] for domain_name in member.get("domain_names", [])] + self.assertIn("somedomain1.com", domain_names) + self.assertNotIn("somedomain2.com", domain_names) + + def test_get_portfolio_invited_json_with_domains(self): + """Test that portfolio invited members are returned properly for an authenticated user and the response includes + the domains that the member manages..""" + PortfolioInvitation.objects.create( + email=self.email6, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + ) + + # create a domain in the portfolio + domain = Domain.objects.create( + name="somedomain1.com", + ) + DomainInformation.objects.create( + creator=self.user, + domain=domain, + portfolio=self.portfolio, + ) + DomainInvitation.objects.create( + email=self.email6, + domain=domain, + ) + + # create a domain not in the portfolio + domain2 = Domain.objects.create( + name="somedomain2.com", + ) + DomainInformation.objects.create( + creator=self.user, + domain=domain2, + ) + DomainInvitation.objects.create( + email=self.email6, + domain=domain2, + ) + + response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id}) + self.assertEqual(response.status_code, 200) + data = response.json + + # Check if the domain appears in the response JSON and domain2 does not + domain_names = [domain_name for member in data["members"] for domain_name in member.get("domain_names", [])] + self.assertIn("somedomain1.com", domain_names) + self.assertNotIn("somedomain2.com", domain_names) + def test_pagination(self): """Test that pagination works properly when there are more members than page size.""" + UserPortfolioPermission.objects.create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + ) + UserPortfolioPermission.objects.create( + user=self.user2, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + UserPortfolioPermission.objects.create( + user=self.user3, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + UserPortfolioPermission.objects.create( + user=self.user4, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + ) + PortfolioInvitation.objects.create( + email=self.email6, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + ) + # Create additional members to exceed page size of 10 - for i in range(5, 15): + for i in range(6, 16): user, _ = User.objects.get_or_create( username=f"test_user{i}", first_name=f"User{i}", @@ -172,6 +395,40 @@ def test_pagination(self): def test_search(self): """Test search functionality for portfolio members.""" + UserPortfolioPermission.objects.create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + ) + UserPortfolioPermission.objects.create( + user=self.user2, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + UserPortfolioPermission.objects.create( + user=self.user3, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + UserPortfolioPermission.objects.create( + user=self.user4, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + ) + PortfolioInvitation.objects.create( + email=self.email6, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + ) + # Search by name response = self.app.get( reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id, "search_term": "Second"} diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index d2f2276cf..e78403659 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -1,14 +1,16 @@ from django.http import JsonResponse from django.core.paginator import Paginator from django.contrib.auth.decorators import login_required -from django.db.models import Value, F, CharField, TextField, Q, Case, When -from django.db.models.functions import Concat, Coalesce +from django.db.models import Value, F, CharField, TextField, Q, Case, When, OuterRef, Subquery +from django.db.models.expressions import Func +from django.db.models.functions import Cast, Coalesce, Concat +from django.contrib.postgres.aggregates import ArrayAgg from django.urls import reverse -from django.db.models.functions import Cast +from registrar.models.domain_invitation import DomainInvitation 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 @login_required @@ -40,6 +42,7 @@ def get_portfolio_members_json(request): return JsonResponse( { "members": members, + "UserPortfolioPermissionChoices": UserPortfolioPermissionChoices.to_dict(), "page": page_obj.number, "num_pages": paginator.num_pages, "has_previous": page_obj.has_previous(), @@ -59,8 +62,11 @@ def initial_permissions_search(portfolio): 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"), + last_active=Coalesce( + Cast(F("user__last_login"), output_field=TextField()), # Cast last_login to text + Value("Invalid date"), + output_field=TextField(), + ), 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")), @@ -77,6 +83,19 @@ def initial_permissions_search(portfolio): default=Value(""), output_field=CharField(), ), + domain_info=ArrayAgg( + # an array of domains, with id and name, colon separated + Concat( + F("user__permissions__domain_id"), + Value(":"), + F("user__permissions__domain__name"), + # specify the output_field to ensure union has same column types + output_field=CharField(), + ), + distinct=True, + filter=Q(user__permissions__domain__isnull=False) # filter out null values + & Q(user__permissions__domain__domain_info__portfolio=portfolio), # only include domains in portfolio + ), source=Value("permission", output_field=CharField()), ) .values( @@ -86,24 +105,43 @@ def initial_permissions_search(portfolio): "email_display", "last_active", "roles", - "additional_permissions_display", + "additional_permissions", "member_display", + "domain_info", "source", ) ) return permissions +# Custom Func to use array_remove to remove null values +class ArrayRemove(Func): + function = "array_remove" + template = "%(function)s(%(expressions)s, NULL)" + + def initial_invitations_search(portfolio): - """Perform initial invitations search before applying any filters.""" + """Perform initial invitations search and get related DomainInvitation data based on the email.""" + # Get DomainInvitation query for matching email and for the portfolio + domain_invitations = DomainInvitation.objects.filter( + email=OuterRef("email"), # Check if email matches the OuterRef("email") + domain__domain_info__portfolio=portfolio, # Check if the domain's portfolio matches the given portfolio + ).annotate(domain_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField())) + # PortfolioInvitation query 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"), + # Use ArrayRemove to return an empty list when no domain invitations are found + domain_info=ArrayRemove( + ArrayAgg( + Subquery(domain_invitations.values("domain_info")), + distinct=True, + ) + ), source=Value("invitation", output_field=CharField()), ).values( "id", @@ -112,8 +150,9 @@ def initial_invitations_search(portfolio): "email_display", "last_active", "roles", - "additional_permissions_display", + "additional_permissions", "member_display", + "domain_info", "source", ) return invitations @@ -159,11 +198,21 @@ def serialize_members(request, portfolio, item, user): # Serialize member data member_json = { "id": item.get("id", ""), + "source": item.get("source", ""), "name": " ".join(filter(None, [item.get("first_name", ""), item.get("last_name", "")])), "email": item.get("email_display", ""), "member_display": item.get("member_display", ""), + "roles": (item.get("roles") or []), + "permissions": UserPortfolioPermission.get_portfolio_permissions( + item.get("roles"), item.get("additional_permissions") + ), + # split domain_info array values into ids to form urls, and names + "domain_urls": [ + reverse("domain", kwargs={"pk": domain_info.split(":")[0]}) for domain_info in item.get("domain_info") + ], + "domain_names": [domain_info.split(":")[1] for domain_info in item.get("domain_info")], "is_admin": is_admin, - "last_active": item.get("last_active", ""), + "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"),
|---|