diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 8ecf36f52..31c75e05e 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -1329,6 +1329,14 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin):
get_roles.short_description = "Roles" # type: ignore
+ def delete_queryset(self, request, queryset):
+ """We override the delete method in the model.
+ When deleting in DJA, if you select multiple items in a table using checkboxes and apply a delete action
+ the model delete does not get called. This method gets called instead.
+ This override makes sure our code in the model gets executed in these situations."""
+ for obj in queryset:
+ obj.delete() # Calls the overridden delete method on each instance
+
class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin):
"""Custom user domain role admin class."""
@@ -1407,10 +1415,13 @@ class BaseInvitationAdmin(ListHeaderAdmin):
Normal flow on successful save_model on add is to redirect to changelist_view.
If there are errors, flow is modified to instead render change form.
"""
- # store current messages from request so that they are preserved throughout the method
+ # store current messages from request in storage so that they are preserved throughout the
+ # method, as some flows remove and replace all messages, and so we store here to retrieve
+ # later
storage = get_messages(request)
- # Check if there are any error or warning messages in the `messages` framework
- has_errors = any(message.level_tag in ["error", "warning"] for message in storage)
+ # Check if there are any error messages in the `messages` framework
+ # error messages stop the workflow; other message levels allow flow to continue as normal
+ has_errors = any(message.level_tag in ["error"] for message in storage)
if has_errors:
# Re-render the change form if there are errors or warnings
@@ -1552,13 +1563,14 @@ class DomainInvitationAdmin(BaseInvitationAdmin):
portfolio_invitation.save()
messages.success(request, f"{requested_email} has been invited to the organization: {domain_org}")
- send_domain_invitation_email(
+ if not send_domain_invitation_email(
email=requested_email,
requestor=requestor,
domains=domain,
is_member_of_different_org=member_of_a_different_org,
requested_user=requested_user,
- )
+ ):
+ messages.warning(request, "Could not send email confirmation to existing domain managers.")
if requested_user is not None:
# Domain Invitation creation for an existing User
obj.retrieve()
@@ -1657,6 +1669,14 @@ class PortfolioInvitationAdmin(BaseInvitationAdmin):
# Call the parent save method to save the object
super().save_model(request, obj, form, change)
+ def delete_queryset(self, request, queryset):
+ """We override the delete method in the model.
+ When deleting in DJA, if you select multiple items in a table using checkboxes and apply a delete action,
+ the model delete does not get called. This method gets called instead.
+ This override makes sure our code in the model gets executed in these situations."""
+ for obj in queryset:
+ obj.delete() # Calls the overridden delete method on each instance
+
class DomainInformationResource(resources.ModelResource):
"""defines how each field in the referenced model should be mapped to the corresponding fields in the
diff --git a/src/registrar/assets/src/sass/_theme/_tooltips.scss b/src/registrar/assets/src/sass/_theme/_tooltips.scss
index 22b5cf534..e1e31cbec 100644
--- a/src/registrar/assets/src/sass/_theme/_tooltips.scss
+++ b/src/registrar/assets/src/sass/_theme/_tooltips.scss
@@ -29,7 +29,7 @@
font-weight: 400 !important;
}
-.domains__table {
+.domains__table, .usa-table {
/*
Trick tooltips in the domains table to do 2 things...
1 - Shrink itself to a padded viewport window
diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py
index a58e3e2f9..58250e85c 100644
--- a/src/registrar/config/settings.py
+++ b/src/registrar/config/settings.py
@@ -25,6 +25,7 @@ from typing import Final
from botocore.config import Config
import json
import logging
+import traceback
from django.utils.log import ServerFormatter
# # # ###
@@ -471,7 +472,11 @@ class JsonFormatter(logging.Formatter):
"lineno": record.lineno,
"message": record.getMessage(),
}
- return json.dumps(log_record)
+ # Capture exception info if it exists
+ if record.exc_info:
+ log_record["exception"] = "".join(traceback.format_exception(*record.exc_info))
+
+ return json.dumps(log_record, ensure_ascii=False)
class JsonServerFormatter(ServerFormatter):
diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py
index 11c564c36..8feeb0794 100644
--- a/src/registrar/models/portfolio_invitation.py
+++ b/src/registrar/models/portfolio_invitation.py
@@ -8,6 +8,7 @@ from registrar.models import DomainInvitation, UserPortfolioPermission
from .utility.portfolio_helper import (
UserPortfolioPermissionChoices,
UserPortfolioRoleChoices,
+ cleanup_after_portfolio_member_deletion,
validate_portfolio_invitation,
) # type: ignore
from .utility.time_stamped_model import TimeStampedModel
@@ -115,3 +116,27 @@ class PortfolioInvitation(TimeStampedModel):
"""Extends clean method to perform additional validation, which can raise errors in django admin."""
super().clean()
validate_portfolio_invitation(self)
+
+ def delete(self, *args, **kwargs):
+
+ User = get_user_model()
+
+ email = self.email # Capture the email before the instance is deleted
+ portfolio = self.portfolio # Capture the portfolio before the instance is deleted
+
+ # Call the superclass delete method to actually delete the instance
+ super().delete(*args, **kwargs)
+
+ if self.status == self.PortfolioInvitationStatus.INVITED:
+
+ # Query the user by email
+ users = User.objects.filter(email=email)
+
+ if users.count() > 1:
+ # This should never happen, log an error if more than one object is returned
+ logger.error(f"Multiple users found with the same email: {email}")
+
+ # Retrieve the first user, or None if no users are found
+ user = users.first()
+
+ cleanup_after_portfolio_member_deletion(portfolio=portfolio, email=email, user=user)
diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py
index c4be90a9b..11d9c56e3 100644
--- a/src/registrar/models/user_portfolio_permission.py
+++ b/src/registrar/models/user_portfolio_permission.py
@@ -5,6 +5,7 @@ from registrar.models.utility.portfolio_helper import (
UserPortfolioRoleChoices,
DomainRequestPermissionDisplay,
MemberPermissionDisplay,
+ cleanup_after_portfolio_member_deletion,
validate_user_portfolio_permission,
)
from .utility.time_stamped_model import TimeStampedModel
@@ -188,3 +189,13 @@ class UserPortfolioPermission(TimeStampedModel):
"""Extends clean method to perform additional validation, which can raise errors in django admin."""
super().clean()
validate_user_portfolio_permission(self)
+
+ def delete(self, *args, **kwargs):
+
+ user = self.user # Capture the user before the instance is deleted
+ portfolio = self.portfolio # Capture the portfolio before the instance is deleted
+
+ # Call the superclass delete method to actually delete the instance
+ super().delete(*args, **kwargs)
+
+ cleanup_after_portfolio_member_deletion(portfolio=portfolio, email=user.email, user=user)
diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py
index b3bb07c3d..8c42b80c7 100644
--- a/src/registrar/models/utility/portfolio_helper.py
+++ b/src/registrar/models/utility/portfolio_helper.py
@@ -210,3 +210,32 @@ def validate_portfolio_invitation(portfolio_invitation):
"This user is already assigned to a portfolio invitation. "
"Based on current waffle flag settings, users cannot be assigned to multiple portfolios."
)
+
+
+def cleanup_after_portfolio_member_deletion(portfolio, email, user=None):
+ """
+ Cleans up after removing a portfolio member or a portfolio invitation.
+
+ Args:
+ portfolio: portfolio
+ user: passed when removing a portfolio member.
+ email: passed when removing a portfolio invitation, or passed as user.email
+ when removing a portfolio member.
+ """
+
+ DomainInvitation = apps.get_model("registrar.DomainInvitation")
+ UserDomainRole = apps.get_model("registrar.UserDomainRole")
+
+ # Fetch domain invitations matching the criteria
+ invitations = DomainInvitation.objects.filter(
+ email=email, domain__domain_info__portfolio=portfolio, status=DomainInvitation.DomainInvitationStatus.INVITED
+ )
+
+ # Call `cancel_invitation` on each invitation
+ for invitation in invitations:
+ invitation.cancel_invitation()
+ invitation.save()
+
+ if user:
+ # Remove user's domain roles for the current portfolio
+ UserDomainRole.objects.filter(user=user, domain__domain_info__portfolio=portfolio).delete()
diff --git a/src/registrar/templates/includes/header_extended.html b/src/registrar/templates/includes/header_extended.html
index 1e40a508d..83b71c3ab 100644
--- a/src/registrar/templates/includes/header_extended.html
+++ b/src/registrar/templates/includes/header_extended.html
@@ -92,11 +92,13 @@
{% endif %}
{% if has_organization_members_flag %}
+ {% if has_view_members_portfolio_permission %}
Members
+ {% endif %}
{% endif %}
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index 036e35a50..387319663 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -1427,7 +1427,7 @@ class TestPortfolioInvitationAdmin(TestCase):
@less_console_noise_decorator
@patch("registrar.admin.send_portfolio_invitation_email")
- @patch("django.contrib.messages.warning") # Mock the `messages.error` call
+ @patch("django.contrib.messages.error") # Mock the `messages.error` call
def test_save_exception_generic_error(self, mock_messages_error, mock_send_email):
"""Handle generic exceptions correctly during portfolio invitation."""
self.client.force_login(self.superuser)
diff --git a/src/registrar/tests/test_email_invitations.py b/src/registrar/tests/test_email_invitations.py
index b9fef1bf8..1377dec42 100644
--- a/src/registrar/tests/test_email_invitations.py
+++ b/src/registrar/tests/test_email_invitations.py
@@ -1,8 +1,11 @@
import unittest
from unittest.mock import patch, MagicMock
from datetime import date
+from registrar.models.domain import Domain
+from registrar.models.user import User
+from registrar.models.user_domain_role import UserDomainRole
from registrar.utility.email import EmailSendingError
-from registrar.utility.email_invitations import send_domain_invitation_email
+from registrar.utility.email_invitations import send_domain_invitation_email, send_emails_to_domain_managers
from api.tests.common import less_console_noise_decorator
@@ -290,16 +293,16 @@ class DomainInvitationEmail(unittest.TestCase):
email = "invitee@example.com"
is_member_of_different_org = False
- mock_send_domain_manager_emails.side_effect = EmailSendingError("Error sending email")
+ # Change the return value to False for mock_send_domain_manager_emails
+ mock_send_domain_manager_emails.return_value = False
- # Call and assert exception
- with self.assertRaises(EmailSendingError) as context:
- send_domain_invitation_email(
- email=email,
- requestor=mock_requestor,
- domains=mock_domain,
- is_member_of_different_org=is_member_of_different_org,
- )
+ # Call and assert that send_domain_invitation_email returns False
+ result = send_domain_invitation_email(
+ email=email,
+ requestor=mock_requestor,
+ domains=mock_domain,
+ is_member_of_different_org=is_member_of_different_org,
+ )
# Assertions
mock_normalize_domains.assert_called_once_with(mock_domain)
@@ -308,4 +311,161 @@ class DomainInvitationEmail(unittest.TestCase):
email, None, [mock_domain], mock_requestor, is_member_of_different_org
)
mock_send_invitation_email.assert_called_once_with(email, mock_requestor_email, [mock_domain], None)
- self.assertEqual(str(context.exception), "Error sending email")
+
+ # Assert that the result is False
+ self.assertFalse(result)
+
+ @less_console_noise_decorator
+ @patch("registrar.utility.email_invitations.send_templated_email")
+ @patch("registrar.models.UserDomainRole.objects.filter")
+ def test_send_emails_to_domain_managers_all_emails_sent_successfully(self, mock_filter, mock_send_templated_email):
+ """Test when all emails are sent successfully."""
+
+ # Setup mocks
+ mock_domain = MagicMock(spec=Domain)
+ mock_requestor_email = "requestor@example.com"
+ mock_email = "invitee@example.com"
+
+ # Create mock user and UserDomainRole
+ mock_user = MagicMock(spec=User)
+ mock_user.email = "manager@example.com"
+ mock_user_domain_role = MagicMock(spec=UserDomainRole, user=mock_user)
+
+ # Mock the filter method to return a list of mock UserDomainRole objects
+ mock_filter.return_value = [mock_user_domain_role]
+
+ # Mock successful email sending
+ mock_send_templated_email.return_value = None # No exception means success
+
+ # Call function
+ result = send_emails_to_domain_managers(mock_email, mock_requestor_email, mock_domain)
+
+ # Assertions
+ self.assertTrue(result) # All emails should be successfully sent
+ mock_send_templated_email.assert_called_once_with(
+ "emails/domain_manager_notification.txt",
+ "emails/domain_manager_notification_subject.txt",
+ to_address="manager@example.com",
+ context={
+ "domain": mock_domain,
+ "requestor_email": mock_requestor_email,
+ "invited_email_address": mock_email,
+ "domain_manager": mock_user,
+ "date": date.today(),
+ },
+ )
+
+ @less_console_noise_decorator
+ @patch("registrar.utility.email_invitations.send_templated_email")
+ @patch("registrar.models.UserDomainRole.objects.filter")
+ def test_send_emails_to_domain_managers_email_send_fails(self, mock_filter, mock_send_templated_email):
+ """Test when sending an email fails (raises EmailSendingError)."""
+
+ # Setup mocks
+ mock_domain = MagicMock(spec=Domain)
+ mock_requestor_email = "requestor@example.com"
+ mock_email = "invitee@example.com"
+
+ # Create mock user and UserDomainRole
+ mock_user = MagicMock(spec=User)
+ mock_user.email = "manager@example.com"
+ mock_user_domain_role = MagicMock(spec=UserDomainRole, user=mock_user)
+
+ # Mock the filter method to return a list of mock UserDomainRole objects
+ mock_filter.return_value = [mock_user_domain_role]
+
+ # Mock sending email to raise an EmailSendingError
+ mock_send_templated_email.side_effect = EmailSendingError("Email sending failed")
+
+ # Call function
+ result = send_emails_to_domain_managers(mock_email, mock_requestor_email, mock_domain)
+
+ # Assertions
+ self.assertFalse(result) # The result should be False as email sending failed
+ mock_send_templated_email.assert_called_once_with(
+ "emails/domain_manager_notification.txt",
+ "emails/domain_manager_notification_subject.txt",
+ to_address="manager@example.com",
+ context={
+ "domain": mock_domain,
+ "requestor_email": mock_requestor_email,
+ "invited_email_address": mock_email,
+ "domain_manager": mock_user,
+ "date": date.today(),
+ },
+ )
+
+ @less_console_noise_decorator
+ @patch("registrar.utility.email_invitations.send_templated_email")
+ @patch("registrar.models.UserDomainRole.objects.filter")
+ def test_send_emails_to_domain_managers_no_domain_managers(self, mock_filter, mock_send_templated_email):
+ """Test when there are no domain managers."""
+
+ # Setup mocks
+ mock_domain = MagicMock(spec=Domain)
+ mock_requestor_email = "requestor@example.com"
+ mock_email = "invitee@example.com"
+
+ # Mock no domain managers (empty UserDomainRole queryset)
+ mock_filter.return_value = []
+
+ # Call function
+ result = send_emails_to_domain_managers(mock_email, mock_requestor_email, mock_domain)
+
+ # Assertions
+ self.assertTrue(result) # No emails to send, so it should return True
+ mock_send_templated_email.assert_not_called() # No emails should be sent
+
+ @less_console_noise_decorator
+ @patch("registrar.utility.email_invitations.send_templated_email")
+ @patch("registrar.models.UserDomainRole.objects.filter")
+ def test_send_emails_to_domain_managers_some_emails_fail(self, mock_filter, mock_send_templated_email):
+ """Test when some emails fail to send."""
+
+ # Setup mocks
+ mock_domain = MagicMock(spec=Domain)
+ mock_requestor_email = "requestor@example.com"
+ mock_email = "invitee@example.com"
+
+ # Create mock users and UserDomainRoles
+ mock_user_1 = MagicMock(spec=User)
+ mock_user_1.email = "manager1@example.com"
+ mock_user_2 = MagicMock(spec=User)
+ mock_user_2.email = "manager2@example.com"
+
+ mock_user_domain_role_1 = MagicMock(spec=UserDomainRole, user=mock_user_1)
+ mock_user_domain_role_2 = MagicMock(spec=UserDomainRole, user=mock_user_2)
+ mock_filter.return_value = [mock_user_domain_role_1, mock_user_domain_role_2]
+
+ # Mock first email success and second email failure
+ mock_send_templated_email.side_effect = [None, EmailSendingError("Failed to send email")]
+
+ # Call function
+ result = send_emails_to_domain_managers(mock_email, mock_requestor_email, mock_domain)
+
+ # Assertions
+ self.assertFalse(result) # One email failed, so result should be False
+ mock_send_templated_email.assert_any_call(
+ "emails/domain_manager_notification.txt",
+ "emails/domain_manager_notification_subject.txt",
+ to_address="manager1@example.com",
+ context={
+ "domain": mock_domain,
+ "requestor_email": mock_requestor_email,
+ "invited_email_address": mock_email,
+ "domain_manager": mock_user_1,
+ "date": date.today(),
+ },
+ )
+ mock_send_templated_email.assert_any_call(
+ "emails/domain_manager_notification.txt",
+ "emails/domain_manager_notification_subject.txt",
+ to_address="manager2@example.com",
+ context={
+ "domain": mock_domain,
+ "requestor_email": mock_requestor_email,
+ "invited_email_address": mock_email,
+ "domain_manager": mock_user_2,
+ "date": date.today(),
+ },
+ )
diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py
index 0f55c8ebe..82fca3fc4 100644
--- a/src/registrar/tests/test_models.py
+++ b/src/registrar/tests/test_models.py
@@ -165,6 +165,7 @@ class TestPortfolioInvitations(TestCase):
DomainInformation.objects.all().delete()
Domain.objects.all().delete()
UserPortfolioPermission.objects.all().delete()
+ UserDomainRole.objects.all().delete()
Portfolio.objects.all().delete()
PortfolioInvitation.objects.all().delete()
User.objects.all().delete()
@@ -443,6 +444,294 @@ class TestPortfolioInvitations(TestCase):
pass
+ @less_console_noise_decorator
+ def test_delete_portfolio_invitation_deletes_portfolio_domain_invitations(self):
+ """Deleting a portfolio invitation causes domain invitations for the same email on the same
+ portfolio to be canceled."""
+
+ email_with_no_user = "email-with-no-user@email.gov"
+
+ domain_in_portfolio_1, _ = Domain.objects.get_or_create(
+ name="domain_in_portfolio_1.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(
+ creator=self.user, domain=domain_in_portfolio_1, portfolio=self.portfolio
+ )
+ invite_1, _ = DomainInvitation.objects.get_or_create(email=email_with_no_user, domain=domain_in_portfolio_1)
+
+ domain_in_portfolio_2, _ = Domain.objects.get_or_create(
+ name="domain_in_portfolio_and_invited_2.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(
+ creator=self.user, domain=domain_in_portfolio_2, portfolio=self.portfolio
+ )
+ invite_2, _ = DomainInvitation.objects.get_or_create(email=email_with_no_user, domain=domain_in_portfolio_2)
+
+ domain_not_in_portfolio, _ = Domain.objects.get_or_create(
+ name="domain_not_in_portfolio.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio)
+ invite_3, _ = DomainInvitation.objects.get_or_create(email=email_with_no_user, domain=domain_not_in_portfolio)
+
+ invitation_of_email_with_no_user, _ = PortfolioInvitation.objects.get_or_create(
+ email=email_with_no_user,
+ portfolio=self.portfolio,
+ roles=[self.portfolio_role_base, self.portfolio_role_admin],
+ additional_permissions=[self.portfolio_permission_1, self.portfolio_permission_2],
+ )
+
+ # The domain invitations start off as INVITED
+ self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED)
+ self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED)
+ self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED)
+
+ # Delete member (invite)
+ invitation_of_email_with_no_user.delete()
+
+ # Reload the objects from the database
+ invite_1 = DomainInvitation.objects.get(pk=invite_1.pk)
+ invite_2 = DomainInvitation.objects.get(pk=invite_2.pk)
+ invite_3 = DomainInvitation.objects.get(pk=invite_3.pk)
+
+ # The domain invitations to the portfolio domains have been canceled
+ self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.CANCELED)
+ self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.CANCELED)
+
+ # Invite 3 is unaffected
+ self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED)
+
+ @less_console_noise_decorator
+ def test_deleting_a_retrieved_invitation_has_no_side_effects(self):
+ """Deleting a retrieved portfolio invitation causes no side effects."""
+
+ domain_in_portfolio_1, _ = Domain.objects.get_or_create(
+ name="domain_in_portfolio_1.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(
+ creator=self.user, domain=domain_in_portfolio_1, portfolio=self.portfolio
+ )
+ invite_1, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_1)
+
+ domain_in_portfolio_2, _ = Domain.objects.get_or_create(
+ name="domain_in_portfolio_and_invited_2.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(
+ creator=self.user, domain=domain_in_portfolio_2, portfolio=self.portfolio
+ )
+ invite_2, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_2)
+
+ domain_in_portfolio_3, _ = Domain.objects.get_or_create(
+ name="domain_in_portfolio_3.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(
+ creator=self.user, domain=domain_in_portfolio_3, portfolio=self.portfolio
+ )
+ UserDomainRole.objects.get_or_create(
+ user=self.user, domain=domain_in_portfolio_3, role=UserDomainRole.Roles.MANAGER
+ )
+
+ domain_in_portfolio_4, _ = Domain.objects.get_or_create(
+ name="domain_in_portfolio_and_invited_4.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(
+ creator=self.user, domain=domain_in_portfolio_4, portfolio=self.portfolio
+ )
+ UserDomainRole.objects.get_or_create(
+ user=self.user, domain=domain_in_portfolio_4, role=UserDomainRole.Roles.MANAGER
+ )
+
+ domain_not_in_portfolio_1, _ = Domain.objects.get_or_create(
+ name="domain_not_in_portfolio.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_1)
+ invite_3, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_not_in_portfolio_1)
+
+ domain_not_in_portfolio_2, _ = Domain.objects.get_or_create(
+ name="domain_not_in_portfolio_2.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_2)
+ UserDomainRole.objects.get_or_create(
+ user=self.user, domain=domain_not_in_portfolio_2, role=UserDomainRole.Roles.MANAGER
+ )
+
+ # The domain invitations start off as INVITED
+ self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED)
+ self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED)
+ self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED)
+
+ # The user domain roles exist
+ self.assertTrue(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_in_portfolio_3,
+ ).exists()
+ )
+ self.assertTrue(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_in_portfolio_4,
+ ).exists()
+ )
+ self.assertTrue(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_not_in_portfolio_2,
+ ).exists()
+ )
+
+ # retrieve the invitation
+ self.invitation.retrieve()
+ self.invitation.save()
+
+ # Delete member (invite)
+ self.invitation.delete()
+
+ # Reload the objects from the database
+ invite_1 = DomainInvitation.objects.get(pk=invite_1.pk)
+ invite_2 = DomainInvitation.objects.get(pk=invite_2.pk)
+ invite_3 = DomainInvitation.objects.get(pk=invite_3.pk)
+
+ # Test that no side effects have been triggered
+ self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED)
+ self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED)
+ self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED)
+ self.assertTrue(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_in_portfolio_3,
+ ).exists()
+ )
+ self.assertTrue(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_in_portfolio_4,
+ ).exists()
+ )
+ self.assertTrue(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_not_in_portfolio_2,
+ ).exists()
+ )
+
+ @less_console_noise_decorator
+ def test_delete_portfolio_invitation_deletes_user_domain_roles(self):
+ """Deleting a portfolio invitation causes domain invitations for the same email on the same
+ portfolio to be canceled, also deletes any exiting user domain roles on the portfolio for the
+ user if the user exists."""
+
+ domain_in_portfolio_1, _ = Domain.objects.get_or_create(
+ name="domain_in_portfolio_1.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(
+ creator=self.user, domain=domain_in_portfolio_1, portfolio=self.portfolio
+ )
+ invite_1, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_1)
+
+ domain_in_portfolio_2, _ = Domain.objects.get_or_create(
+ name="domain_in_portfolio_and_invited_2.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(
+ creator=self.user, domain=domain_in_portfolio_2, portfolio=self.portfolio
+ )
+ invite_2, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_2)
+
+ domain_in_portfolio_3, _ = Domain.objects.get_or_create(
+ name="domain_in_portfolio_3.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(
+ creator=self.user, domain=domain_in_portfolio_3, portfolio=self.portfolio
+ )
+ UserDomainRole.objects.get_or_create(
+ user=self.user, domain=domain_in_portfolio_3, role=UserDomainRole.Roles.MANAGER
+ )
+
+ domain_in_portfolio_4, _ = Domain.objects.get_or_create(
+ name="domain_in_portfolio_and_invited_4.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(
+ creator=self.user, domain=domain_in_portfolio_4, portfolio=self.portfolio
+ )
+ UserDomainRole.objects.get_or_create(
+ user=self.user, domain=domain_in_portfolio_4, role=UserDomainRole.Roles.MANAGER
+ )
+
+ domain_not_in_portfolio_1, _ = Domain.objects.get_or_create(
+ name="domain_not_in_portfolio.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_1)
+ invite_3, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_not_in_portfolio_1)
+
+ domain_not_in_portfolio_2, _ = Domain.objects.get_or_create(
+ name="domain_not_in_portfolio_2.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_2)
+ UserDomainRole.objects.get_or_create(
+ user=self.user, domain=domain_not_in_portfolio_2, role=UserDomainRole.Roles.MANAGER
+ )
+
+ # The domain invitations start off as INVITED
+ self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED)
+ self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED)
+ self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED)
+
+ # The user domain roles exist
+ self.assertTrue(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_in_portfolio_3,
+ ).exists()
+ )
+ self.assertTrue(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_in_portfolio_4,
+ ).exists()
+ )
+ self.assertTrue(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_not_in_portfolio_2,
+ ).exists()
+ )
+
+ # Delete member (invite)
+ self.invitation.delete()
+
+ # Reload the objects from the database
+ invite_1 = DomainInvitation.objects.get(pk=invite_1.pk)
+ invite_2 = DomainInvitation.objects.get(pk=invite_2.pk)
+ invite_3 = DomainInvitation.objects.get(pk=invite_3.pk)
+
+ # The domain invitations to the portfolio domains have been canceled
+ self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.CANCELED)
+ self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.CANCELED)
+
+ # Invite 3 is unaffected
+ self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED)
+
+ # The user domain roles have been deleted for the domains in portfolio
+ self.assertFalse(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_in_portfolio_3,
+ ).exists()
+ )
+ self.assertFalse(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_in_portfolio_4,
+ ).exists()
+ )
+
+ # The user domain role on the domain not in portfolio still exists
+ self.assertTrue(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_not_in_portfolio_2,
+ ).exists()
+ )
+
class TestUserPortfolioPermission(TestCase):
@less_console_noise_decorator
@@ -458,6 +747,7 @@ class TestUserPortfolioPermission(TestCase):
Domain.objects.all().delete()
DomainInformation.objects.all().delete()
DomainRequest.objects.all().delete()
+ DomainInvitation.objects.all().delete()
UserPortfolioPermission.objects.all().delete()
Portfolio.objects.all().delete()
User.objects.all().delete()
@@ -751,6 +1041,129 @@ class TestUserPortfolioPermission(TestCase):
# Should return the forbidden permissions for member role
self.assertEqual(member_only_permissions, set(member_forbidden))
+ @less_console_noise_decorator
+ def test_delete_portfolio_permission_deletes_user_domain_roles(self):
+ """Deleting a user portfolio permission causes domain invitations for the same email on the same
+ portfolio to be canceled, also deletes any exiting user domain roles on the portfolio for the
+ user if the user exists."""
+
+ domain_in_portfolio_1, _ = Domain.objects.get_or_create(
+ name="domain_in_portfolio_1.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(
+ creator=self.user, domain=domain_in_portfolio_1, portfolio=self.portfolio
+ )
+ invite_1, _ = DomainInvitation.objects.get_or_create(email=self.user.email, domain=domain_in_portfolio_1)
+
+ domain_in_portfolio_2, _ = Domain.objects.get_or_create(
+ name="domain_in_portfolio_and_invited_2.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(
+ creator=self.user, domain=domain_in_portfolio_2, portfolio=self.portfolio
+ )
+ invite_2, _ = DomainInvitation.objects.get_or_create(email=self.user.email, domain=domain_in_portfolio_2)
+
+ domain_in_portfolio_3, _ = Domain.objects.get_or_create(
+ name="domain_in_portfolio_3.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(
+ creator=self.user, domain=domain_in_portfolio_3, portfolio=self.portfolio
+ )
+ UserDomainRole.objects.get_or_create(
+ user=self.user, domain=domain_in_portfolio_3, role=UserDomainRole.Roles.MANAGER
+ )
+
+ domain_in_portfolio_4, _ = Domain.objects.get_or_create(
+ name="domain_in_portfolio_and_invited_4.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(
+ creator=self.user, domain=domain_in_portfolio_4, portfolio=self.portfolio
+ )
+ UserDomainRole.objects.get_or_create(
+ user=self.user, domain=domain_in_portfolio_4, role=UserDomainRole.Roles.MANAGER
+ )
+
+ domain_not_in_portfolio_1, _ = Domain.objects.get_or_create(
+ name="domain_not_in_portfolio.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_1)
+ invite_3, _ = DomainInvitation.objects.get_or_create(email=self.user.email, domain=domain_not_in_portfolio_1)
+
+ domain_not_in_portfolio_2, _ = Domain.objects.get_or_create(
+ name="domain_not_in_portfolio_2.gov", state=Domain.State.READY
+ )
+ DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_2)
+ UserDomainRole.objects.get_or_create(
+ user=self.user, domain=domain_not_in_portfolio_2, role=UserDomainRole.Roles.MANAGER
+ )
+
+ # Create portfolio permission
+ portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(
+ portfolio=self.portfolio, user=self.user, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
+ )
+
+ # The domain invitations start off as INVITED
+ self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED)
+ self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED)
+ self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED)
+
+ # The user domain roles exist
+ self.assertTrue(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_in_portfolio_3,
+ ).exists()
+ )
+ self.assertTrue(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_in_portfolio_4,
+ ).exists()
+ )
+ self.assertTrue(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_not_in_portfolio_2,
+ ).exists()
+ )
+
+ # Delete member (user portfolio permission)
+ portfolio_permission.delete()
+
+ # Reload the objects from the database
+ invite_1 = DomainInvitation.objects.get(pk=invite_1.pk)
+ invite_2 = DomainInvitation.objects.get(pk=invite_2.pk)
+ invite_3 = DomainInvitation.objects.get(pk=invite_3.pk)
+
+ # The domain invitations to the portfolio domains have been canceled
+ self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.CANCELED)
+ self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.CANCELED)
+
+ # Invite 3 is unaffected
+ self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED)
+
+ # The user domain roles have been deleted for the domains in portfolio
+ self.assertFalse(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_in_portfolio_3,
+ ).exists()
+ )
+ self.assertFalse(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_in_portfolio_4,
+ ).exists()
+ )
+
+ # The user domain role on the domain not in portfolio still exists
+ self.assertTrue(
+ UserDomainRole.objects.filter(
+ user=self.user,
+ domain=domain_not_in_portfolio_2,
+ ).exists()
+ )
+
class TestUser(TestCase):
"""Test actions that occur on user login,
diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py
index 7d1e69783..02b61fda9 100644
--- a/src/registrar/tests/test_views_domain.py
+++ b/src/registrar/tests/test_views_domain.py
@@ -925,6 +925,40 @@ class TestDomainManagers(TestDomainOverview):
success_page = success_result.follow()
self.assertContains(success_page, "notauser@igorville.gov")
+ @override_flag("organization_feature", active=True)
+ @less_console_noise_decorator
+ @patch("registrar.views.domain.send_portfolio_invitation_email")
+ @patch("registrar.views.domain.send_domain_invitation_email")
+ def test_domain_user_add_form_fails_to_send_to_some_managers(
+ self, mock_send_domain_email, mock_send_portfolio_email
+ ):
+ """Adding an email not associated with a user works and sends portfolio invitation,
+ and when domain managers email(s) fail to send, assert proper warning displayed."""
+ add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id}))
+ session_id = self.app.cookies[settings.SESSION_COOKIE_NAME]
+
+ add_page.form["email"] = "notauser@igorville.gov"
+
+ self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
+
+ mock_send_domain_email.return_value = False
+
+ success_result = add_page.form.submit()
+
+ self.assertEqual(success_result.status_code, 302)
+ self.assertEqual(
+ success_result["Location"],
+ reverse("domain-users", kwargs={"pk": self.domain.id}),
+ )
+
+ # Verify that the invitation emails were sent
+ mock_send_portfolio_email.assert_called_once()
+ mock_send_domain_email.assert_called_once()
+
+ self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
+ success_page = success_result.follow()
+ self.assertContains(success_page, "Could not send email confirmation to existing domain managers.")
+
@boto3_mocking.patching
@override_flag("organization_feature", active=True)
@less_console_noise_decorator
diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py
index 3afbb2a7b..b50c9a36f 100644
--- a/src/registrar/tests/test_views_portfolio.py
+++ b/src/registrar/tests/test_views_portfolio.py
@@ -1097,8 +1097,10 @@ class TestPortfolio(WebTest):
@less_console_noise_decorator
@override_flag("organization_feature", active=True)
@override_flag("organization_requests", active=True)
+ @override_flag("organization_members", active=True)
def test_main_nav_when_user_has_no_permissions(self):
- """Test the nav contains a link to the no requests page"""
+ """Test the nav contains a link to the no requests page
+ Also test that members link not present"""
UserPortfolioPermission.objects.get_or_create(
user=self.user, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER]
)
@@ -1118,20 +1120,23 @@ class TestPortfolio(WebTest):
self.assertNotContains(portfolio_landing_page, "basic-nav-section-two")
# link to requests
self.assertNotContains(portfolio_landing_page, 'href="/requests/')
- # link to create
+ # link to create request
self.assertNotContains(portfolio_landing_page, 'href="/request/')
+ # link to members
+ self.assertNotContains(portfolio_landing_page, 'href="/members/')
@less_console_noise_decorator
@override_flag("organization_feature", active=True)
@override_flag("organization_requests", active=True)
+ @override_flag("organization_members", active=True)
def test_main_nav_when_user_has_all_permissions(self):
"""Test the nav contains a dropdown with a link to create and another link to view requests
- Also test for the existence of the Create a new request btn on the requests page"""
+ Also test for the existence of the Create a new request btn on the requests page
+ Also test for the existence of the members link"""
UserPortfolioPermission.objects.get_or_create(
user=self.user,
portfolio=self.portfolio,
roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
- additional_permissions=[UserPortfolioPermissionChoices.EDIT_REQUESTS],
)
self.client.force_login(self.user)
# create and submit a domain request
@@ -1151,6 +1156,8 @@ class TestPortfolio(WebTest):
self.assertContains(portfolio_landing_page, 'href="/requests/')
# link to create
self.assertContains(portfolio_landing_page, 'href="/request/')
+ # link to members
+ self.assertContains(portfolio_landing_page, 'href="/members/')
requests_page = self.client.get(reverse("domain-requests"))
@@ -1160,15 +1167,18 @@ class TestPortfolio(WebTest):
@less_console_noise_decorator
@override_flag("organization_feature", active=True)
@override_flag("organization_requests", active=True)
+ @override_flag("organization_members", active=True)
def test_main_nav_when_user_has_view_but_not_edit_permissions(self):
"""Test the nav contains a simple link to view requests
- Also test for the existence of the Create a new request btn on the requests page"""
+ Also test for the existence of the Create a new request btn on the requests page
+ Also test for the existence of members link"""
UserPortfolioPermission.objects.get_or_create(
user=self.user,
portfolio=self.portfolio,
additional_permissions=[
UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS,
+ UserPortfolioPermissionChoices.VIEW_MEMBERS,
],
)
self.client.force_login(self.user)
@@ -1189,6 +1199,8 @@ class TestPortfolio(WebTest):
self.assertContains(portfolio_landing_page, 'href="/requests/')
# link to create
self.assertNotContains(portfolio_landing_page, 'href="/request/')
+ # link to members
+ self.assertContains(portfolio_landing_page, 'href="/members/')
requests_page = self.client.get(reverse("domain-requests"))
@@ -3254,7 +3266,7 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest):
# assert that response is a redirect to reverse("members")
self.assertRedirects(response, reverse("members"))
# assert that messages contains message, "Could not send email invitation"
- mock_warning.assert_called_once_with(response.wsgi_request, "Could not send email invitation.")
+ mock_warning.assert_called_once_with(response.wsgi_request, "Could not send portfolio email invitation.")
# assert that portfolio invitation is not created
self.assertFalse(
PortfolioInvitation.objects.filter(email=self.new_member_email, portfolio=self.portfolio).exists(),
@@ -3339,7 +3351,7 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest):
# assert that response is a redirect to reverse("members")
self.assertRedirects(response, reverse("members"))
# assert that messages contains message, "Could not send email invitation"
- mock_warning.assert_called_once_with(response.wsgi_request, "Could not send email invitation.")
+ mock_warning.assert_called_once_with(response.wsgi_request, "Could not send portfolio email invitation.")
# assert that portfolio invitation is not created
self.assertFalse(
PortfolioInvitation.objects.filter(email=self.new_member_email, portfolio=self.portfolio).exists(),
diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py
index d589f814f..f9c3b89b2 100644
--- a/src/registrar/utility/email_invitations.py
+++ b/src/registrar/utility/email_invitations.py
@@ -27,6 +27,9 @@ def send_domain_invitation_email(
is_member_of_different_org (bool): if an email belongs to a different org
requested_user (User | None): The recipient if the email belongs to a user in the registrar
+ Returns:
+ Boolean indicating if all messages were sent successfully.
+
Raises:
MissingEmailError: If the requestor has no email associated with their account.
AlreadyDomainManagerError: If the email corresponds to an existing domain manager.
@@ -41,22 +44,28 @@ def send_domain_invitation_email(
send_invitation_email(email, requestor_email, domains, requested_user)
+ all_manager_emails_sent = True
# send emails to domain managers
for domain in domains:
- send_emails_to_domain_managers(
+ if not send_emails_to_domain_managers(
email=email,
requestor_email=requestor_email,
domain=domain,
requested_user=requested_user,
- )
+ ):
+ all_manager_emails_sent = False
+
+ return all_manager_emails_sent
def send_emails_to_domain_managers(email: str, requestor_email, domain: Domain, requested_user=None):
"""
Notifies all domain managers of the provided domain of a change
- Raises:
- EmailSendingError
+
+ Returns:
+ Boolean indicating if all messages were sent successfully.
"""
+ all_emails_sent = True
# Get each domain manager from list
user_domain_roles = UserDomainRole.objects.filter(domain=domain)
for user_domain_role in user_domain_roles:
@@ -75,10 +84,12 @@ def send_emails_to_domain_managers(email: str, requestor_email, domain: Domain,
"date": date.today(),
},
)
- except EmailSendingError as err:
- raise EmailSendingError(
- f"Could not send email manager notification to {user.email} for domain: {domain.name}"
- ) from err
+ except EmailSendingError:
+ logger.warning(
+ f"Could not send email manager notification to {user.email} for domain: {domain.name}", exc_info=True
+ )
+ all_emails_sent = False
+ return all_emails_sent
def normalize_domains(domains: Domain | list[Domain]) -> list[Domain]:
diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py
index 72e0e4b35..297cb689a 100644
--- a/src/registrar/views/domain.py
+++ b/src/registrar/views/domain.py
@@ -1255,24 +1255,26 @@ class DomainAddUserView(DomainFormBaseView):
def _handle_new_user_invitation(self, email, requestor, member_of_different_org):
"""Handle invitation for a new user who does not exist in the system."""
- send_domain_invitation_email(
+ if not send_domain_invitation_email(
email=email,
requestor=requestor,
domains=self.object,
is_member_of_different_org=member_of_different_org,
- )
+ ):
+ messages.warning(self.request, "Could not send email confirmation to existing domain managers.")
DomainInvitation.objects.get_or_create(email=email, domain=self.object)
messages.success(self.request, f"{email} has been invited to the domain: {self.object}")
def _handle_existing_user(self, email, requestor, requested_user, member_of_different_org):
"""Handle adding an existing user to the domain."""
- send_domain_invitation_email(
+ if not send_domain_invitation_email(
email=email,
requestor=requestor,
domains=self.object,
is_member_of_different_org=member_of_different_org,
requested_user=requested_user,
- )
+ ):
+ messages.warning(self.request, "Could not send email confirmation to existing domain managers.")
UserDomainRole.objects.create(
user=requested_user,
domain=self.object,
diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py
index f5db165d1..212ce089d 100644
--- a/src/registrar/views/portfolios.py
+++ b/src/registrar/views/portfolios.py
@@ -303,13 +303,14 @@ class PortfolioMemberDomainsEditView(PortfolioMemberDomainsEditPermissionView, V
# get added_domains from ids to pass to send email method and bulk create
added_domains = Domain.objects.filter(id__in=added_domain_ids)
member_of_a_different_org, _ = get_org_membership(portfolio, member.email, member)
- send_domain_invitation_email(
+ if not send_domain_invitation_email(
email=member.email,
requestor=requestor,
domains=added_domains,
is_member_of_different_org=member_of_a_different_org,
requested_user=member,
- )
+ ):
+ messages.warning(self.request, "Could not send email confirmation to existing domain managers.")
# Bulk create UserDomainRole instances for added domains
UserDomainRole.objects.bulk_create(
[
@@ -525,12 +526,13 @@ class PortfolioInvitedMemberDomainsEditView(PortfolioMemberDomainsEditPermission
# get added_domains from ids to pass to send email method and bulk create
added_domains = Domain.objects.filter(id__in=added_domain_ids)
member_of_a_different_org, _ = get_org_membership(portfolio, email, None)
- send_domain_invitation_email(
+ if not send_domain_invitation_email(
email=email,
requestor=requestor,
domains=added_domains,
is_member_of_different_org=member_of_a_different_org,
- )
+ ):
+ messages.warning(self.request, "Could not send email confirmation to existing domain managers.")
# Update existing invitations from CANCELED to INVITED
existing_invitations = DomainInvitation.objects.filter(domain__in=added_domains, email=email)
@@ -807,7 +809,7 @@ class PortfolioAddMemberView(PortfolioMembersPermissionView, FormMixin):
portfolio,
exc_info=True,
)
- messages.warning(self.request, "Could not send email invitation.")
+ messages.warning(self.request, "Could not send portfolio email invitation.")
elif isinstance(exception, MissingEmailError):
messages.error(self.request, str(exception))
logger.error(
@@ -816,4 +818,4 @@ class PortfolioAddMemberView(PortfolioMembersPermissionView, FormMixin):
)
else:
logger.warning("Could not send email invitation (Other Exception)", exc_info=True)
- messages.warning(self.request, "Could not send email invitation.")
+ messages.warning(self.request, "Could not send portfolio email invitation.")
diff --git a/src/registrar/views/report_views.py b/src/registrar/views/report_views.py
index 694d1e205..ee2c079f3 100644
--- a/src/registrar/views/report_views.py
+++ b/src/registrar/views/report_views.py
@@ -5,17 +5,20 @@ from django.views import View
from django.shortcuts import render
from django.contrib import admin
from django.db.models import Avg, F
+
+from registrar.views.utility.mixins import DomainAndRequestsReportsPermission, PortfolioReportsPermission
from .. import models
import datetime
from django.utils import timezone
-
+from django.contrib.admin.views.decorators import staff_member_required
+from django.utils.decorators import method_decorator
from registrar.utility import csv_export
-
import logging
logger = logging.getLogger(__name__)
+@method_decorator(staff_member_required, name="dispatch")
class AnalyticsView(View):
def get(self, request):
thirty_days_ago = datetime.datetime.today() - datetime.timedelta(days=30)
@@ -149,6 +152,7 @@ class AnalyticsView(View):
return render(request, "admin/analytics.html", context)
+@method_decorator(staff_member_required, name="dispatch")
class ExportDataType(View):
def get(self, request, *args, **kwargs):
# match the CSV example with all the fields
@@ -158,7 +162,7 @@ class ExportDataType(View):
return response
-class ExportDataTypeUser(View):
+class ExportDataTypeUser(DomainAndRequestsReportsPermission, View):
"""Returns a domain report for a given user on the request"""
def get(self, request, *args, **kwargs):
@@ -169,7 +173,7 @@ class ExportDataTypeUser(View):
return response
-class ExportMembersPortfolio(View):
+class ExportMembersPortfolio(PortfolioReportsPermission, View):
"""Returns a members report for a given portfolio"""
def get(self, request, *args, **kwargs):
@@ -197,7 +201,7 @@ class ExportMembersPortfolio(View):
return response
-class ExportDataTypeRequests(View):
+class ExportDataTypeRequests(DomainAndRequestsReportsPermission, View):
"""Returns a domain requests report for a given user on the request"""
def get(self, request, *args, **kwargs):
@@ -208,6 +212,7 @@ class ExportDataTypeRequests(View):
return response
+@method_decorator(staff_member_required, name="dispatch")
class ExportDataFull(View):
def get(self, request, *args, **kwargs):
# Smaller export based on 1
@@ -217,6 +222,7 @@ class ExportDataFull(View):
return response
+@method_decorator(staff_member_required, name="dispatch")
class ExportDataFederal(View):
def get(self, request, *args, **kwargs):
# Federal only
@@ -226,6 +232,7 @@ class ExportDataFederal(View):
return response
+@method_decorator(staff_member_required, name="dispatch")
class ExportDomainRequestDataFull(View):
"""Generates a downloaded report containing all Domain Requests (except started)"""
@@ -237,6 +244,7 @@ class ExportDomainRequestDataFull(View):
return response
+@method_decorator(staff_member_required, name="dispatch")
class ExportDataDomainsGrowth(View):
def get(self, request, *args, **kwargs):
start_date = request.GET.get("start_date", "")
@@ -249,6 +257,7 @@ class ExportDataDomainsGrowth(View):
return response
+@method_decorator(staff_member_required, name="dispatch")
class ExportDataRequestsGrowth(View):
def get(self, request, *args, **kwargs):
start_date = request.GET.get("start_date", "")
@@ -261,6 +270,7 @@ class ExportDataRequestsGrowth(View):
return response
+@method_decorator(staff_member_required, name="dispatch")
class ExportDataManagedDomains(View):
def get(self, request, *args, **kwargs):
start_date = request.GET.get("start_date", "")
@@ -272,6 +282,7 @@ class ExportDataManagedDomains(View):
return response
+@method_decorator(staff_member_required, name="dispatch")
class ExportDataUnmanagedDomains(View):
def get(self, request, *args, **kwargs):
start_date = request.GET.get("start_date", "")
diff --git a/src/registrar/views/utility/invitation_helper.py b/src/registrar/views/utility/invitation_helper.py
index 98c36b308..18c427940 100644
--- a/src/registrar/views/utility/invitation_helper.py
+++ b/src/registrar/views/utility/invitation_helper.py
@@ -3,7 +3,6 @@ from django.db import IntegrityError
from registrar.models import PortfolioInvitation, User, UserPortfolioPermission
from registrar.utility.email import EmailSendingError
import logging
-
from registrar.utility.errors import (
AlreadyDomainInvitedError,
AlreadyDomainManagerError,
@@ -61,20 +60,19 @@ def get_requested_user(email):
def handle_invitation_exceptions(request, exception, email):
"""Handle exceptions raised during the process."""
if isinstance(exception, EmailSendingError):
- logger.warning(str(exception), exc_info=True)
+ logger.warning(exception, exc_info=True)
messages.error(request, str(exception))
elif isinstance(exception, MissingEmailError):
messages.error(request, str(exception))
- logger.error(str(exception), exc_info=True)
+ logger.error(exception, exc_info=True)
elif isinstance(exception, OutsideOrgMemberError):
messages.error(request, str(exception))
- logger.warning(str(exception), exc_info=True)
elif isinstance(exception, AlreadyDomainManagerError):
- messages.warning(request, str(exception))
+ messages.error(request, str(exception))
elif isinstance(exception, AlreadyDomainInvitedError):
- messages.warning(request, str(exception))
+ messages.error(request, str(exception))
elif isinstance(exception, IntegrityError):
- messages.warning(request, f"{email} is already a manager for this domain")
+ messages.error(request, f"{email} is already a manager for this domain")
else:
logger.warning("Could not send email invitation (Other Exception)", exc_info=True)
- messages.warning(request, "Could not send email invitation.")
+ messages.error(request, "Could not send email invitation.")
diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py
index 08212088b..8a4666372 100644
--- a/src/registrar/views/utility/mixins.py
+++ b/src/registrar/views/utility/mixins.py
@@ -153,6 +153,48 @@ class PermissionsLoginMixin(PermissionRequiredMixin):
return super().handle_no_permission()
+class DomainAndRequestsReportsPermission(PermissionsLoginMixin):
+ """Permission mixin for domain and requests csv downloads"""
+
+ def has_permission(self):
+ """Check if this user has access to this domain.
+
+ The user is in self.request.user and the domain needs to be looked
+ up from the domain's primary key in self.kwargs["pk"]
+ """
+
+ if not self.request.user.is_authenticated:
+ return False
+
+ if self.request.user.is_restricted():
+ return False
+
+ return True
+
+
+class PortfolioReportsPermission(PermissionsLoginMixin):
+ """Permission mixin for portfolio csv downloads"""
+
+ def has_permission(self):
+ """Check if this user has access to this domain.
+
+ The user is in self.request.user and the domain needs to be looked
+ up from the domain's primary key in self.kwargs["pk"]
+ """
+
+ if not self.request.user.is_authenticated:
+ return False
+
+ if self.request.user.is_restricted():
+ return False
+
+ portfolio = self.request.session.get("portfolio")
+ if not self.request.user.has_view_members_portfolio_permission(portfolio):
+ return False
+
+ return self.request.user.is_org_user(self.request)
+
+
class DomainPermission(PermissionsLoginMixin):
"""Permission mixin that redirects to domain if user has access,
otherwise 403"""