fixed existing tests and lint errors

This commit is contained in:
David Kennedy 2025-02-01 08:10:10 -05:00
parent 041df217b5
commit 6671545c55
No known key found for this signature in database
GPG key ID: 6528A5386E66B96B
8 changed files with 57 additions and 43 deletions

View file

@ -28,7 +28,11 @@ from django.shortcuts import redirect
from django_fsm import get_available_FIELD_transitions, FSMField from django_fsm import get_available_FIELD_transitions, FSMField
from registrar.models import DomainInformation, Portfolio, UserPortfolioPermission, DomainInvitation from registrar.models import DomainInformation, Portfolio, UserPortfolioPermission, DomainInvitation
from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices
from registrar.utility.email_invitations import send_domain_invitation_email, send_portfolio_admin_addition_emails, send_portfolio_invitation_email from registrar.utility.email_invitations import (
send_domain_invitation_email,
send_portfolio_admin_addition_emails,
send_portfolio_invitation_email,
)
from registrar.views.utility.invitation_helper import ( from registrar.views.utility.invitation_helper import (
get_org_membership, get_org_membership,
get_requested_user, get_requested_user,

View file

@ -137,11 +137,9 @@ def validate_user_portfolio_permission(user_portfolio_permission):
"Based on current waffle flag settings, users cannot be assigned to multiple portfolios." "Based on current waffle flag settings, users cannot be assigned to multiple portfolios."
) )
existing_invitations = PortfolioInvitation.objects.filter( existing_invitations = PortfolioInvitation.objects.filter(email=user_portfolio_permission.user.email).exclude(
email=user_portfolio_permission.user.email Q(portfolio=user_portfolio_permission.portfolio)
).exclude( | Q(status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED)
Q(portfolio=user_portfolio_permission.portfolio) |
Q(status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED)
) )
if existing_invitations.exists(): if existing_invitations.exists():
raise ValidationError( raise ValidationError(
@ -199,11 +197,8 @@ def validate_portfolio_invitation(portfolio_invitation):
if not flag_is_active_for_user(user, "multiple_portfolios"): if not flag_is_active_for_user(user, "multiple_portfolios"):
existing_permissions = UserPortfolioPermission.objects.filter(user=user) existing_permissions = UserPortfolioPermission.objects.filter(user=user)
existing_invitations = PortfolioInvitation.objects.filter( existing_invitations = PortfolioInvitation.objects.filter(email=portfolio_invitation.email).exclude(
email=portfolio_invitation.email Q(id=portfolio_invitation.id) | Q(status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED)
).exclude(
Q(id=portfolio_invitation.id) |
Q(status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED)
) )
if existing_permissions.exists(): if existing_permissions.exists():

View file

@ -254,6 +254,7 @@ class TestDomainInvitationAdmin(TestCase):
email="test@example.com", email="test@example.com",
requestor=self.superuser, requestor=self.superuser,
portfolio=self.portfolio, portfolio=self.portfolio,
is_admin_invitation=False,
) )
# Assert success message # Assert success message
@ -504,6 +505,7 @@ class TestDomainInvitationAdmin(TestCase):
email="test@example.com", email="test@example.com",
requestor=self.superuser, requestor=self.superuser,
portfolio=self.portfolio, portfolio=self.portfolio,
is_admin_invitation=False,
) )
# Assert retrieve on domain invite only was called # Assert retrieve on domain invite only was called
@ -567,6 +569,7 @@ class TestDomainInvitationAdmin(TestCase):
email="test@example.com", email="test@example.com",
requestor=self.superuser, requestor=self.superuser,
portfolio=self.portfolio, portfolio=self.portfolio,
is_admin_invitation=False,
) )
# Assert retrieve on domain invite only was called # Assert retrieve on domain invite only was called
@ -693,6 +696,7 @@ class TestDomainInvitationAdmin(TestCase):
email="nonexistent@example.com", email="nonexistent@example.com",
requestor=self.superuser, requestor=self.superuser,
portfolio=self.portfolio, portfolio=self.portfolio,
is_admin_invitation=False,
) )
# Assert retrieve was not called # Assert retrieve was not called
@ -918,6 +922,7 @@ class TestDomainInvitationAdmin(TestCase):
email="nonexistent@example.com", email="nonexistent@example.com",
requestor=self.superuser, requestor=self.superuser,
portfolio=self.portfolio, portfolio=self.portfolio,
is_admin_invitation=False,
) )
# Assert retrieve on domain invite only was called # Assert retrieve on domain invite only was called
@ -979,6 +984,7 @@ class TestDomainInvitationAdmin(TestCase):
email="nonexistent@example.com", email="nonexistent@example.com",
requestor=self.superuser, requestor=self.superuser,
portfolio=self.portfolio, portfolio=self.portfolio,
is_admin_invitation=False,
) )
# Assert retrieve on domain invite only was called # Assert retrieve on domain invite only was called

View file

@ -58,7 +58,7 @@ class DomainInvitationEmail(unittest.TestCase):
# Assertions # Assertions
mock_normalize_domains.assert_called_once_with(mock_domain) mock_normalize_domains.assert_called_once_with(mock_domain)
mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain]) mock_get_requestor_email.assert_called_once_with(mock_requestor, domains=[mock_domain])
mock_validate_invitation.assert_called_once_with( mock_validate_invitation.assert_called_once_with(
email, None, [mock_domain], mock_requestor, is_member_of_different_org email, None, [mock_domain], mock_requestor, is_member_of_different_org
) )
@ -137,7 +137,7 @@ class DomainInvitationEmail(unittest.TestCase):
# Assertions # Assertions
mock_normalize_domains.assert_called_once_with([mock_domain1, mock_domain2]) mock_normalize_domains.assert_called_once_with([mock_domain1, mock_domain2])
mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain1, mock_domain2]) mock_get_requestor_email.assert_called_once_with(mock_requestor, domains=[mock_domain1, mock_domain2])
mock_validate_invitation.assert_called_once_with( mock_validate_invitation.assert_called_once_with(
email, None, [mock_domain1, mock_domain2], mock_requestor, is_member_of_different_org email, None, [mock_domain1, mock_domain2], mock_requestor, is_member_of_different_org
) )
@ -258,7 +258,7 @@ class DomainInvitationEmail(unittest.TestCase):
# Assertions # Assertions
mock_normalize_domains.assert_called_once_with(mock_domain) mock_normalize_domains.assert_called_once_with(mock_domain)
mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain]) mock_get_requestor_email.assert_called_once_with(mock_requestor, domains=[mock_domain])
mock_validate_invitation.assert_called_once_with( mock_validate_invitation.assert_called_once_with(
email, None, [mock_domain], mock_requestor, is_member_of_different_org email, None, [mock_domain], mock_requestor, is_member_of_different_org
) )
@ -306,7 +306,7 @@ class DomainInvitationEmail(unittest.TestCase):
# Assertions # Assertions
mock_normalize_domains.assert_called_once_with(mock_domain) mock_normalize_domains.assert_called_once_with(mock_domain)
mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain]) mock_get_requestor_email.assert_called_once_with(mock_requestor, domains=[mock_domain])
mock_validate_invitation.assert_called_once_with( mock_validate_invitation.assert_called_once_with(
email, None, [mock_domain], mock_requestor, is_member_of_different_org email, None, [mock_domain], mock_requestor, is_member_of_different_org
) )

View file

@ -810,7 +810,10 @@ class TestDomainManagers(TestDomainOverview):
# Verify that the invitation emails were sent # Verify that the invitation emails were sent
mock_send_portfolio_email.assert_called_once_with( mock_send_portfolio_email.assert_called_once_with(
email="mayor@igorville.gov", requestor=self.user, portfolio=self.portfolio email="mayor@igorville.gov",
requestor=self.user,
portfolio=self.portfolio,
is_admin_invitation=False,
) )
mock_send_domain_email.assert_called_once() mock_send_domain_email.assert_called_once()
call_args = mock_send_domain_email.call_args.kwargs call_args = mock_send_domain_email.call_args.kwargs
@ -864,7 +867,10 @@ class TestDomainManagers(TestDomainOverview):
# Verify that the invitation emails were sent # Verify that the invitation emails were sent
mock_send_portfolio_email.assert_called_once_with( mock_send_portfolio_email.assert_called_once_with(
email="notauser@igorville.gov", requestor=self.user, portfolio=self.portfolio email="notauser@igorville.gov",
requestor=self.user,
portfolio=self.portfolio,
is_admin_invitation=False,
) )
mock_send_domain_email.assert_called_once() mock_send_domain_email.assert_called_once()
call_args = mock_send_domain_email.call_args.kwargs call_args = mock_send_domain_email.call_args.kwargs
@ -999,7 +1005,10 @@ class TestDomainManagers(TestDomainOverview):
# Verify that the invitation emails were sent # Verify that the invitation emails were sent
mock_send_portfolio_email.assert_called_once_with( mock_send_portfolio_email.assert_called_once_with(
email="mayor@igorville.gov", requestor=self.user, portfolio=self.portfolio email="mayor@igorville.gov",
requestor=self.user,
portfolio=self.portfolio,
is_admin_invitation=False,
) )
mock_send_domain_email.assert_not_called() mock_send_domain_email.assert_not_called()

View file

@ -3290,7 +3290,7 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest):
# Assert # Assert
# assert that the send_portfolio_invitation_email called # assert that the send_portfolio_invitation_email called
mock_send_email.assert_called_once_with( mock_send_email.assert_called_once_with(
email=self.new_member_email, requestor=self.user, portfolio=self.portfolio email=self.new_member_email, requestor=self.user, portfolio=self.portfolio, is_admin_invitation=False
) )
# assert that response is a redirect to reverse("members") # assert that response is a redirect to reverse("members")
self.assertRedirects(response, reverse("members")) self.assertRedirects(response, reverse("members"))
@ -3334,7 +3334,10 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest):
# Assert # Assert
# assert that the send_portfolio_invitation_email called # assert that the send_portfolio_invitation_email called
mock_send_email.assert_called_once_with( mock_send_email.assert_called_once_with(
email=self.new_member_email, requestor=self.user, portfolio=self.portfolio email=self.new_member_email,
requestor=self.user,
portfolio=self.portfolio,
is_admin_invitation=False,
) )
# assert that response is a redirect to reverse("members") # assert that response is a redirect to reverse("members")
self.assertRedirects(response, reverse("members")) self.assertRedirects(response, reverse("members"))

View file

@ -270,7 +270,9 @@ def _send_portfolio_admin_addition_emails_to_portfolio_admins(email: str, reques
) )
except EmailSendingError: except EmailSendingError:
logger.warning( logger.warning(
f"Could not send email organization admin notification to {user.email} for portfolio: {portfolio.name}", "Could not send email organization admin notification to %s " "for portfolio: %s",
user.email,
portfolio.organization_name,
exc_info=True, exc_info=True,
) )
all_emails_sent = False all_emails_sent = False
@ -321,7 +323,9 @@ def _send_portfolio_admin_removal_emails_to_portfolio_admins(email: str, request
) )
except EmailSendingError: except EmailSendingError:
logger.warning( logger.warning(
f"Could not send email organization admin notification to {user.email} for portfolio: {portfolio.name}", "Could not send email organization admin notification to %s " "for portfolio: %s",
user.email,
portfolio.organization_name,
exc_info=True, exc_info=True,
) )
all_emails_sent = False all_emails_sent = False

View file

@ -155,11 +155,9 @@ class PortfolioMemberDeleteView(PortfolioMemberPermission, View):
if not send_portfolio_admin_removal_emails( if not send_portfolio_admin_removal_emails(
email=portfolio_member_permission.user.email, email=portfolio_member_permission.user.email,
requestor=request.user, requestor=request.user,
portfolio=portfolio_member_permission.portfolio portfolio=portfolio_member_permission.portfolio,
): ):
messages.warning( messages.warning(self.request, "Could not send email notification to existing organization admins.")
self.request, "Could not send email notification to existing organization admins."
)
except Exception as e: except Exception as e:
self._handle_exceptions(e) self._handle_exceptions(e)
@ -179,7 +177,7 @@ class PortfolioMemberDeleteView(PortfolioMemberPermission, View):
if isinstance(exception, MissingEmailError): if isinstance(exception, MissingEmailError):
messages.warning(self.request, "Could not send email notification to existing organization admins.") messages.warning(self.request, "Could not send email notification to existing organization admins.")
logger.warning( logger.warning(
f"Could not send email notification to existing organization admins.", "Could not send email notification to existing organization admins.",
exc_info=True, exc_info=True,
) )
else: else:
@ -218,7 +216,7 @@ class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View):
if not send_portfolio_admin_addition_emails( if not send_portfolio_admin_addition_emails(
email=portfolio_permission.user.email, email=portfolio_permission.user.email,
requestor=request.user, requestor=request.user,
portfolio=portfolio_permission.portfolio portfolio=portfolio_permission.portfolio,
): ):
messages.warning( messages.warning(
self.request, "Could not send email notification to existing organization admins." self.request, "Could not send email notification to existing organization admins."
@ -227,13 +225,13 @@ class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View):
if not send_portfolio_admin_removal_emails( if not send_portfolio_admin_removal_emails(
email=portfolio_permission.user.email, email=portfolio_permission.user.email,
requestor=request.user, requestor=request.user,
portfolio=portfolio_permission.portfolio portfolio=portfolio_permission.portfolio,
): ):
messages.warning( messages.warning(
self.request, "Could not send email notification to existing organization admins." self.request, "Could not send email notification to existing organization admins."
) )
# Check if user is removing their own admin or edit role # Check if user is removing their own admin or edit role
removing_admin_role_on_self = (request.user == user) removing_admin_role_on_self = request.user == user
except Exception as e: except Exception as e:
self._handle_exceptions(e) self._handle_exceptions(e)
form.save() form.save()
@ -254,7 +252,7 @@ class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View):
if isinstance(exception, MissingEmailError): if isinstance(exception, MissingEmailError):
messages.warning(self.request, "Could not send email notification to existing organization admins.") messages.warning(self.request, "Could not send email notification to existing organization admins.")
logger.warning( logger.warning(
f"Could not send email notification to existing organization admins.", "Could not send email notification to existing organization admins.",
exc_info=True, exc_info=True,
) )
else: else:
@ -262,7 +260,6 @@ class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View):
messages.warning(self.request, "Could not send email notification to existing organization admins.") messages.warning(self.request, "Could not send email notification to existing organization admins.")
class PortfolioMemberDomainsView(PortfolioMemberDomainsPermissionView, View): class PortfolioMemberDomainsView(PortfolioMemberDomainsPermissionView, View):
template_name = "portfolio_member_domains.html" template_name = "portfolio_member_domains.html"
@ -447,13 +444,9 @@ class PortfolioInvitedMemberDeleteView(PortfolioMemberPermission, View):
try: try:
# attempt to send notification emails of the removal to portfolio admins # attempt to send notification emails of the removal to portfolio admins
if not send_portfolio_admin_removal_emails( if not send_portfolio_admin_removal_emails(
email=portfolio_invitation.email, email=portfolio_invitation.email, requestor=request.user, portfolio=portfolio_invitation.portfolio
requestor=request.user,
portfolio=portfolio_invitation.portfolio
): ):
messages.warning( messages.warning(self.request, "Could not send email notification to existing organization admins.")
self.request, "Could not send email notification to existing organization admins."
)
except Exception as e: except Exception as e:
self._handle_exceptions(e) self._handle_exceptions(e)
@ -472,7 +465,7 @@ class PortfolioInvitedMemberDeleteView(PortfolioMemberPermission, View):
if isinstance(exception, MissingEmailError): if isinstance(exception, MissingEmailError):
messages.warning(self.request, "Could not send email notification to existing organization admins.") messages.warning(self.request, "Could not send email notification to existing organization admins.")
logger.warning( logger.warning(
f"Could not send email notification to existing organization admins.", "Could not send email notification to existing organization admins.",
exc_info=True, exc_info=True,
) )
else: else:
@ -516,7 +509,7 @@ class PortfolioInvitedMemberEditView(PortfolioMemberEditPermissionView, View):
if not send_portfolio_admin_removal_emails( if not send_portfolio_admin_removal_emails(
email=portfolio_invitation.email, email=portfolio_invitation.email,
requestor=request.user, requestor=request.user,
portfolio=portfolio_invitation.portfolio portfolio=portfolio_invitation.portfolio,
): ):
messages.warning( messages.warning(
self.request, "Could not send email notification to existing organization admins." self.request, "Could not send email notification to existing organization admins."
@ -541,7 +534,7 @@ class PortfolioInvitedMemberEditView(PortfolioMemberEditPermissionView, View):
if isinstance(exception, MissingEmailError): if isinstance(exception, MissingEmailError):
messages.warning(self.request, "Could not send email notification to existing organization admins.") messages.warning(self.request, "Could not send email notification to existing organization admins.")
logger.warning( logger.warning(
f"Could not send email notification to existing organization admins.", "Could not send email notification to existing organization admins.",
exc_info=True, exc_info=True,
) )
else: else: