Skip to content

Commit

Permalink
fixed a bug where non-portfolio domains were appearing in members jso…
Browse files Browse the repository at this point in the history
…n response
  • Loading branch information
dave-kennedy-ecs committed Oct 23, 2024
1 parent 07e4960 commit 740eba7
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 11 deletions.
39 changes: 33 additions & 6 deletions src/registrar/tests/test_views_members_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,29 +222,43 @@ def test_get_portfolio_members_json_with_domains(self):
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
# 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
Expand All @@ -259,28 +273,41 @@ def test_get_portfolio_invited_json_with_domains(self):
],
)

# 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
# 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."""
Expand Down
12 changes: 7 additions & 5 deletions src/registrar/views/portfolio_members_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ def initial_permissions_search(portfolio):
output_field=CharField(),
),
distinct=True,
filter=Q(user__permissions__domain__isnull=False), # filter out null values
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()),
)
Expand Down Expand Up @@ -121,10 +122,11 @@ class ArrayRemove(Func):

def initial_invitations_search(portfolio):
"""Perform initial invitations search and get related DomainInvitation data based on the email."""
# Get DomainInvitation query for matching email
domain_invitations = DomainInvitation.objects.filter(email=OuterRef("email")).annotate(
domain_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField())
)
# 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(
Expand Down

0 comments on commit 740eba7

Please sign in to comment.