From 5fce7c7b4e888615a86a9f83eb5a9dadb5d405c3 Mon Sep 17 00:00:00 2001
From: matthewswspence <Matthewswspence@gmail.com>
Date: Tue, 14 Jan 2025 14:33:30 -0600
Subject: [PATCH 01/20] initial fix

---
 src/registrar/tests/test_admin.py    |  13 +-
 src/registrar/views/transfer_user.py | 275 ++++++++++++++++++++-------
 2 files changed, 215 insertions(+), 73 deletions(-)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index 3195f8237..e38c47f74 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -2734,7 +2734,7 @@ class TestTransferUser(WebTest):
 
     @less_console_noise_decorator
     def test_transfer_user_transfers_user_portfolio_roles_no_error_when_duplicates(self):
-        """Assert that duplicate portfolio user roles do not throw errorsd"""
+        """Assert that duplicate portfolio user roles do not throw errors"""
         portfolio1 = Portfolio.objects.create(organization_name="Hotel California", creator=self.user2)
         UserPortfolioPermission.objects.create(
             user=self.user1, portfolio=portfolio1, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
@@ -2759,7 +2759,7 @@ class TestTransferUser(WebTest):
 
             messages.error.assert_not_called()
 
-    @less_console_noise_decorator
+    # @less_console_noise_decorator
     def test_transfer_user_transfers_domain_request_creator_and_investigator(self):
         """Assert that domain request fields get transferred"""
         domain_request = completed_domain_request(user=self.user2, name="wasteland.gov", investigator=self.user2)
@@ -2776,6 +2776,7 @@ class TestTransferUser(WebTest):
         self.assertEquals(domain_request.creator, self.user1)
         self.assertEquals(domain_request.investigator, self.user1)
 
+
     @less_console_noise_decorator
     def test_transfer_user_transfers_domain_information_creator(self):
         """Assert that domain fields get transferred"""
@@ -2866,7 +2867,7 @@ class TestTransferUser(WebTest):
         with self.assertRaises(User.DoesNotExist):
             self.user2.refresh_from_db()
 
-    @less_console_noise_decorator
+    # @less_console_noise_decorator
     def test_transfer_user_throws_transfer_and_delete_success_messages(self):
         """Test that success messages for data transfer and user deletion are displayed."""
         # Ensure the setup for VerifiedByStaff
@@ -2882,6 +2883,10 @@ class TestTransferUser(WebTest):
             submit_form["selected_user"] = self.user2.pk
             after_submit = submit_form.submit().follow()
 
+            print("mock_success_message.call_args_list:")
+            for call in mock_success_message.call_args_list:
+                print(call)
+
             self.assertContains(after_submit, "<h1>Change user</h1>")
 
             mock_success_message.assert_any_call(
@@ -2898,7 +2903,7 @@ class TestTransferUser(WebTest):
     def test_transfer_user_throws_error_message(self):
         """Test that an error message is thrown if the transfer fails."""
         with patch(
-            "registrar.views.TransferUserView.transfer_user_fields_and_log", side_effect=Exception("Simulated Error")
+            "registrar.views.TransferUserView.transfer_related_fields_and_log", side_effect=Exception("Simulated Error")
         ):
             with patch("django.contrib.messages.error") as mock_error:
                 # Access the transfer user page
diff --git a/src/registrar/views/transfer_user.py b/src/registrar/views/transfer_user.py
index 69a0f4997..fa4c59a3f 100644
--- a/src/registrar/views/transfer_user.py
+++ b/src/registrar/views/transfer_user.py
@@ -1,7 +1,10 @@
 import logging
+from django.db import transaction
+from django.db.models import Manager,ForeignKey, OneToOneField, ManyToManyField, ManyToOneRel
 
 from django.shortcuts import render, get_object_or_404, redirect
 from django.views import View
+from registrar import models
 from registrar.models.domain import Domain
 from registrar.models.domain_information import DomainInformation
 from registrar.models.domain_request import DomainRequest
@@ -21,22 +24,8 @@ logger = logging.getLogger(__name__)
 class TransferUserView(View):
     """Transfer user methods that set up the transfer_user template and handle the forms on it."""
 
-    JOINS = [
-        (DomainRequest, "creator"),
-        (DomainInformation, "creator"),
-        (Portfolio, "creator"),
-        (DomainRequest, "investigator"),
-        (UserDomainRole, "user"),
-        (VerifiedByStaff, "requestor"),
-        (UserPortfolioPermission, "user"),
-    ]
-
-    # Future-proofing in case joined fields get added on the user model side
-    # This was tested in the first portfolio model iteration and works
-    USER_FIELDS: List[Any] = []
-
     def get(self, request, user_id):
-        """current_user referes to the 'source' user where the button that redirects to this view was clicked.
+        """current_user refers to the 'source' user where the button that redirects to this view was clicked.
         other_users exclude current_user and populate a dropdown, selected_user is the selection in the dropdown.
 
         This also querries the relevant domains and domain requests, and the admin context needed for the sidenav."""
@@ -71,86 +60,234 @@ class TransferUserView(View):
 
     def post(self, request, user_id):
         """This handles the transfer from selected_user to current_user then deletes selected_user.
-
-        NOTE: We have a ticket to refactor this into a more solid lookup for related fields in #2645"""
-
+        """
         current_user = get_object_or_404(User, pk=user_id)
         selected_user_id = request.POST.get("selected_user")
         selected_user = get_object_or_404(User, pk=selected_user_id)
 
         try:
-            change_logs = []
+            # Make this atomic so that we don't get any partial transfers
+            with transaction.atomic():
+                change_logs = []
 
-            # Transfer specific fields
-            self.transfer_user_fields_and_log(selected_user, current_user, change_logs)
+                self._delete_duplicate_user_domain_roles_and_log(selected_user, current_user, change_logs)
 
-            # Perform the updates and log the changes
-            for model_class, field_name in self.JOINS:
-                self.update_joins_and_log(model_class, field_name, selected_user, current_user, change_logs)
+                self._delete_duplicate_user_portfolio_permissions_and_log(selected_user, current_user, change_logs)
+                # Dynamically handle related fields 
+                self.transfer_related_fields_and_log(selected_user, current_user, change_logs)
 
-            # Success message if any related objects were updated
-            if change_logs:
-                success_message = f"Data transferred successfully for the following objects: {change_logs}"
-                messages.success(request, success_message)
+                # Success message if any related objects were updated
+                logger.debug(f"change_logs: {change_logs}")
+                if change_logs:
+                    logger.debug(f"change_logs: {change_logs}")
+                    success_message = f"Data transferred successfully for the following objects: {change_logs}"
+                    messages.success(request, success_message)
 
-            selected_user.delete()
-            messages.success(request, f"Deleted {selected_user} {selected_user.username}")
+                logger.debug("Deleting old user")
+                selected_user.delete()
+                messages.success(request, f"Deleted {selected_user} {selected_user.username}")
 
         except Exception as e:
             messages.error(request, f"An error occurred during the transfer: {e}")
+            logger.error(f"An error occurred during the transfer: {e}", exc_info=True)
 
         return redirect("admin:registrar_user_change", object_id=user_id)
-
-    @classmethod
-    def update_joins_and_log(cls, model_class, field_name, selected_user, current_user, change_logs):
+    
+    def _delete_duplicate_user_portfolio_permissions_and_log(self, selected_user, current_user, change_logs):
         """
-        Helper function to update the user join fields for a given model and log the changes.
+        Check and remove duplicate UserPortfolioPermission objects from the selected_user based on portfolios associated with the current_user.
+        """
+        try:
+            # Fetch portfolios associated with the current_user
+            current_user_portfolios = UserPortfolioPermission.objects.filter(user=current_user).values_list('portfolio_id', flat=True)
+
+            # Identify duplicates in selected_user for these portfolios
+            duplicates = (
+                UserPortfolioPermission.objects
+                .filter(user=selected_user, portfolio_id__in=current_user_portfolios)
+            )
+
+            duplicate_count = duplicates.count()
+
+            if duplicate_count > 0:
+                # Log the specific duplicates before deletion for better traceability
+                duplicate_permissions = list(duplicates)
+                logger.debug(f"Duplicate permissions to be removed: {duplicate_permissions}")
+
+                duplicates.delete()
+                logger.info(f"Removed {duplicate_count} duplicate UserPortfolioPermission(s) from user_id {selected_user.id} for portfolios already associated with user_id {current_user.id}")
+                change_logs.append(f"Removed {duplicate_count} duplicate UserPortfolioPermission(s) from user_id {selected_user.id} for portfolios already associated with user_id {current_user.id}")
+        
+        except Exception as e:
+            logger.error(f"Failed to check and remove duplicate UserPortfolioPermissions: {e}", exc_info=True)
+            raise
+
+    def _delete_duplicate_user_domain_roles_and_log(self, selected_user, current_user, change_logs):
+        """
+        Check and remove duplicate UserDomainRole objects from the selected_user based on domains associated with the current_user.
+        Retain one instance per domain to maintain data integrity.
         """
 
-        filter_kwargs = {field_name: selected_user}
-        updated_objects = model_class.objects.filter(**filter_kwargs)
+        try:
+            # Fetch domains associated with the current_user
+            current_user_domains = UserDomainRole.objects.filter(user=current_user).values_list('domain_id', flat=True)
 
-        for obj in updated_objects:
-            # Check for duplicate UserDomainRole before updating
-            if model_class == UserDomainRole:
-                if model_class.objects.filter(user=current_user, domain=obj.domain).exists():
-                    continue  # Skip the update to avoid a duplicate
+            # Identify duplicates in selected_user for these domains
+            duplicates = (
+                UserDomainRole.objects
+                .filter(user=selected_user, domain_id__in=current_user_domains)
+            )
 
-            if model_class == UserPortfolioPermission:
-                if model_class.objects.filter(user=current_user, portfolio=obj.portfolio).exists():
-                    continue  # Skip the update to avoid a duplicate
+            duplicate_count = duplicates.count()
 
-            # Update the field on the object and save it
+            if duplicate_count > 0:
+                duplicates.delete()
+                logger.info(
+                    f"Removed {duplicate_count} duplicate UserDomainRole(s) from user_id {selected_user.id} "
+                    f"for domains already associated with user_id {current_user.id}"
+                )
+                change_logs.append(
+                    f"Removed {duplicate_count} duplicate UserDomainRole(s) from user_id {selected_user.id} "
+                    f"for domains already associated with user_id {current_user.id}"
+                )
+        
+        except Exception as e:
+            logger.error(f"Failed to check and remove duplicate UserDomainRoles: {e}", exc_info=True)
+            raise
+
+    def transfer_related_fields_and_log(self, selected_user, current_user, change_logs):
+        """
+        Dynamically find all related fields to the User model and transfer them from selected_user to current_user.
+        Handles ForeignKey, OneToOneField, ManyToManyField, and ManyToOneRel relationships.
+        """
+        user_model = User
+
+        # Handle forward relationships
+        for related_field in user_model._meta.get_fields():
+            if related_field.is_relation and related_field.related_model:
+                if isinstance(related_field, ForeignKey):
+                    self._handle_foreign_key(related_field, selected_user, current_user, change_logs)
+                elif isinstance(related_field, OneToOneField):
+                    self._handle_one_to_one(related_field, selected_user, current_user, change_logs)
+                elif isinstance(related_field, ManyToManyField):
+                    self._handle_many_to_many(related_field, selected_user, current_user, change_logs)
+                elif isinstance(related_field, ManyToOneRel):
+                    self._handle_many_to_one_rel(related_field, selected_user, current_user, change_logs)
+
+        # # Handle reverse relationships
+        for related_object in user_model._meta.related_objects:
+            if isinstance(related_object, ManyToOneRel):
+                self._handle_many_to_one_rel(related_object, selected_user, current_user, change_logs)
+            elif isinstance(related_object.field, OneToOneField):
+                self._handle_one_to_one_reverse(related_object, selected_user, current_user, change_logs)
+            elif isinstance(related_object.field, ForeignKey):
+                self._handle_foreign_key_reverse(related_object, selected_user, current_user, change_logs)
+            elif isinstance(related_object.field, ManyToManyField):
+                self._handle_many_to_many_reverse(related_object, selected_user, current_user, change_logs)
+
+    def _handle_foreign_key(self, related_field: ForeignKey, selected_user, current_user, change_logs):
+        related_name = related_field.get_accessor_name()
+        # for foreign key relationships, getattr returns a manager
+        related_manager = getattr(selected_user, related_name, None)
+
+        if related_manager:
+            # get all the related objects
+            related_queryset = related_manager.all()
+            for obj in related_queryset:
+                # set the foreign key to the current user
+                setattr(obj, related_field.field.name, current_user)
+                obj.save()
+                log_entry = f'Transferred {related_field.field.name} from {selected_user} to {current_user}'
+                logger.info(log_entry)
+                change_logs.append(log_entry)
+    
+    def _handle_one_to_one(self, related_field: OneToOneField, selected_user, current_user, change_logs):
+        related_name = related_field.get_accessor_name()
+        # for one to one relationships, getattr returns the related object
+        related_object = getattr(selected_user, related_name, None)
+
+        if related_object:
+            # set the one to one field to the current user
+            setattr(related_object, related_field.field.name, current_user)
+            related_object.save()
+            log_entry = f'Transferred {related_field.field.name} from {selected_user} to {current_user}'
+            logger.info(log_entry)
+            change_logs.append(log_entry)
+
+    def _handle_many_to_many(self, related_field: ManyToManyField, selected_user, current_user, change_logs):
+        # for many to many relationships, getattr returns a manager
+        related_manager = getattr(selected_user, related_field.name, None)
+        if related_manager:
+            # get all the related objects
+            related_queryset = related_manager.all()
+            # add the related objects to the current user
+            getattr(current_user, related_field.name).add(*related_queryset)
+            log_entry = f'Transferred {related_field.name} from {selected_user} to {current_user}'
+            logger.info(log_entry)
+            change_logs.append(log_entry)
+
+    def _handle_many_to_one_rel(self, related_object: ManyToOneRel, selected_user: User, current_user: User, change_logs: List[str]):
+        """
+        Handles ManyToOneRel relationships, where multiple objects relate to the User via a ForeignKey.
+        """
+        related_model = related_object.related_model
+        related_name = related_object.field.name
+
+        # for many to one relationships, we need a queryset
+        related_queryset = related_model.objects.filter(**{related_name: selected_user})
+        for obj in related_queryset:
+            setattr(obj, related_name, current_user)
+            obj.save()
+            log_entry = f'Transferred {related_name} from {selected_user} to {current_user}'
+            logger.info(log_entry)
+            change_logs.append(log_entry)
+
+    def _handle_one_to_one_reverse(self, related_object: OneToOneField, selected_user: User, current_user: User, change_logs: List[str]):
+        """
+        Handles reverse OneToOneField relationships.
+        """
+        related_model = related_object.related_model
+        field_name = related_object.field.name
+
+        try:
+            related_instance = related_model.objects.filter(**{field_name: selected_user}).first()
+            setattr(related_instance, field_name, current_user)
+            related_instance.save()
+            log_entry = f'Transferred {field_name} from {selected_user} to {current_user}'
+            logger.info(log_entry)
+            change_logs.append(log_entry)
+        except related_model.DoesNotExist:
+            logger.warning(f"No related instance found for reverse OneToOneField {field_name} for {selected_user}")
+    
+    def _handle_foreign_key_reverse(self, related_object: ForeignKey, selected_user: User, current_user: User, change_logs: List[str]):
+        """
+        Handles reverse ForeignKey relationships.
+        """
+        related_model = related_object.related_model
+        field_name = related_object.field.name
+
+        related_queryset = related_model.objects.filter(**{field_name: selected_user})
+        for obj in related_queryset:
             setattr(obj, field_name, current_user)
             obj.save()
+            log_entry = f'Transferred {field_name} from {selected_user} to {current_user}'
+            logger.info(log_entry)
+            change_logs.append(log_entry)
 
-            # Log the change
-            cls.log_change(obj, field_name, selected_user, current_user, change_logs)
-
-    @classmethod
-    def transfer_user_fields_and_log(cls, selected_user, current_user, change_logs):
+    def _handle_many_to_many_reverse(self, related_object: ManyToManyField, selected_user: User, current_user: User, change_logs: List[str]):
         """
-        Transfers portfolio fields from the selected_user to the current_user.
-        Logs the changes for each transferred field.
+        Handles reverse ManyToManyField relationships.
         """
-        for field in cls.USER_FIELDS:
-            field_value = getattr(selected_user, field, None)
+        related_model = related_object.related_model
+        field_name = related_object.field.name
 
-            if field_value:
-                setattr(current_user, field, field_value)
-                cls.log_change(current_user, field, field_value, field_value, change_logs)
-
-        current_user.save()
-
-    @classmethod
-    def log_change(cls, obj, field_name, field_value, new_value, change_logs):
-        """Logs the change for a specific field on an object"""
-        log_entry = f'Changed {field_name} from "{field_value}" to "{new_value}" on {obj}'
-
-        logger.info(log_entry)
-
-        # Collect the related object for the success message
-        change_logs.append(log_entry)
+        related_manager = related_model.objects.filter(**{field_name: selected_user})
+        if related_manager:
+            related_qs = related_manager.all()
+            getattr(current_user, field_name).add(*related_qs)
+            log_entry = f'Transferred {field_name} from {selected_user} to {current_user}'
+            logger.info(log_entry)
+            change_logs.append(log_entry)
 
     @classmethod
     def get_domains(cls, user):

From bc9bc8fbaee837b9e23ac822cb55d590eb3ac95b Mon Sep 17 00:00:00 2001
From: matthewswspence <Matthewswspence@gmail.com>
Date: Tue, 14 Jan 2025 14:58:30 -0600
Subject: [PATCH 02/20] refactor transfer user function

---
 src/registrar/tests/test_admin.py    |   9 +-
 src/registrar/views/transfer_user.py | 139 ++++++++++++---------------
 2 files changed, 65 insertions(+), 83 deletions(-)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index e38c47f74..0cc75d6e2 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -2759,7 +2759,7 @@ class TestTransferUser(WebTest):
 
             messages.error.assert_not_called()
 
-    # @less_console_noise_decorator
+    @less_console_noise_decorator
     def test_transfer_user_transfers_domain_request_creator_and_investigator(self):
         """Assert that domain request fields get transferred"""
         domain_request = completed_domain_request(user=self.user2, name="wasteland.gov", investigator=self.user2)
@@ -2776,7 +2776,6 @@ class TestTransferUser(WebTest):
         self.assertEquals(domain_request.creator, self.user1)
         self.assertEquals(domain_request.investigator, self.user1)
 
-
     @less_console_noise_decorator
     def test_transfer_user_transfers_domain_information_creator(self):
         """Assert that domain fields get transferred"""
@@ -2867,7 +2866,7 @@ class TestTransferUser(WebTest):
         with self.assertRaises(User.DoesNotExist):
             self.user2.refresh_from_db()
 
-    # @less_console_noise_decorator
+    @less_console_noise_decorator
     def test_transfer_user_throws_transfer_and_delete_success_messages(self):
         """Test that success messages for data transfer and user deletion are displayed."""
         # Ensure the setup for VerifiedByStaff
@@ -2892,8 +2891,8 @@ class TestTransferUser(WebTest):
             mock_success_message.assert_any_call(
                 ANY,
                 (
-                    "Data transferred successfully for the following objects: ['Changed requestor "
-                    + 'from "Furiosa Jabassa " to "Max Rokatanski " on immortan.joe@citadel.com\']'
+                    "Data transferred successfully for the following objects: ['Transferred requestor "
+                    + "from Furiosa Jabassa  to Max Rokatanski ']"
                 ),
             )
 
diff --git a/src/registrar/views/transfer_user.py b/src/registrar/views/transfer_user.py
index fa4c59a3f..e10a5f3fc 100644
--- a/src/registrar/views/transfer_user.py
+++ b/src/registrar/views/transfer_user.py
@@ -1,6 +1,6 @@
 import logging
 from django.db import transaction
-from django.db.models import Manager,ForeignKey, OneToOneField, ManyToManyField, ManyToOneRel
+from django.db.models import Manager, ForeignKey, OneToOneField, ManyToManyField, ManyToOneRel
 
 from django.shortcuts import render, get_object_or_404, redirect
 from django.views import View
@@ -59,8 +59,7 @@ class TransferUserView(View):
         return render(request, "admin/transfer_user.html", context)
 
     def post(self, request, user_id):
-        """This handles the transfer from selected_user to current_user then deletes selected_user.
-        """
+        """This handles the transfer from selected_user to current_user then deletes selected_user."""
         current_user = get_object_or_404(User, pk=user_id)
         selected_user_id = request.POST.get("selected_user")
         selected_user = get_object_or_404(User, pk=selected_user_id)
@@ -73,7 +72,7 @@ class TransferUserView(View):
                 self._delete_duplicate_user_domain_roles_and_log(selected_user, current_user, change_logs)
 
                 self._delete_duplicate_user_portfolio_permissions_and_log(selected_user, current_user, change_logs)
-                # Dynamically handle related fields 
+                # Dynamically handle related fields
                 self.transfer_related_fields_and_log(selected_user, current_user, change_logs)
 
                 # Success message if any related objects were updated
@@ -92,19 +91,20 @@ class TransferUserView(View):
             logger.error(f"An error occurred during the transfer: {e}", exc_info=True)
 
         return redirect("admin:registrar_user_change", object_id=user_id)
-    
+
     def _delete_duplicate_user_portfolio_permissions_and_log(self, selected_user, current_user, change_logs):
         """
         Check and remove duplicate UserPortfolioPermission objects from the selected_user based on portfolios associated with the current_user.
         """
         try:
             # Fetch portfolios associated with the current_user
-            current_user_portfolios = UserPortfolioPermission.objects.filter(user=current_user).values_list('portfolio_id', flat=True)
+            current_user_portfolios = UserPortfolioPermission.objects.filter(user=current_user).values_list(
+                "portfolio_id", flat=True
+            )
 
             # Identify duplicates in selected_user for these portfolios
-            duplicates = (
-                UserPortfolioPermission.objects
-                .filter(user=selected_user, portfolio_id__in=current_user_portfolios)
+            duplicates = UserPortfolioPermission.objects.filter(
+                user=selected_user, portfolio_id__in=current_user_portfolios
             )
 
             duplicate_count = duplicates.count()
@@ -115,9 +115,13 @@ class TransferUserView(View):
                 logger.debug(f"Duplicate permissions to be removed: {duplicate_permissions}")
 
                 duplicates.delete()
-                logger.info(f"Removed {duplicate_count} duplicate UserPortfolioPermission(s) from user_id {selected_user.id} for portfolios already associated with user_id {current_user.id}")
-                change_logs.append(f"Removed {duplicate_count} duplicate UserPortfolioPermission(s) from user_id {selected_user.id} for portfolios already associated with user_id {current_user.id}")
-        
+                logger.info(
+                    f"Removed {duplicate_count} duplicate UserPortfolioPermission(s) from user_id {selected_user.id} for portfolios already associated with user_id {current_user.id}"
+                )
+                change_logs.append(
+                    f"Removed {duplicate_count} duplicate UserPortfolioPermission(s) from user_id {selected_user.id} for portfolios already associated with user_id {current_user.id}"
+                )
+
         except Exception as e:
             logger.error(f"Failed to check and remove duplicate UserPortfolioPermissions: {e}", exc_info=True)
             raise
@@ -130,13 +134,10 @@ class TransferUserView(View):
 
         try:
             # Fetch domains associated with the current_user
-            current_user_domains = UserDomainRole.objects.filter(user=current_user).values_list('domain_id', flat=True)
+            current_user_domains = UserDomainRole.objects.filter(user=current_user).values_list("domain_id", flat=True)
 
             # Identify duplicates in selected_user for these domains
-            duplicates = (
-                UserDomainRole.objects
-                .filter(user=selected_user, domain_id__in=current_user_domains)
-            )
+            duplicates = UserDomainRole.objects.filter(user=selected_user, domain_id__in=current_user_domains)
 
             duplicate_count = duplicates.count()
 
@@ -150,7 +151,7 @@ class TransferUserView(View):
                     f"Removed {duplicate_count} duplicate UserDomainRole(s) from user_id {selected_user.id} "
                     f"for domains already associated with user_id {current_user.id}"
                 )
-        
+
         except Exception as e:
             logger.error(f"Failed to check and remove duplicate UserDomainRoles: {e}", exc_info=True)
             raise
@@ -187,65 +188,48 @@ class TransferUserView(View):
 
     def _handle_foreign_key(self, related_field: ForeignKey, selected_user, current_user, change_logs):
         related_name = related_field.get_accessor_name()
-        # for foreign key relationships, getattr returns a manager
         related_manager = getattr(selected_user, related_name, None)
 
-        if related_manager:
-            # get all the related objects
+        if related_manager.count() > 0:
             related_queryset = related_manager.all()
             for obj in related_queryset:
-                # set the foreign key to the current user
                 setattr(obj, related_field.field.name, current_user)
                 obj.save()
-                log_entry = f'Transferred {related_field.field.name} from {selected_user} to {current_user}'
-                logger.info(log_entry)
-                change_logs.append(log_entry)
-    
+                self.log_change(selected_user, current_user, related_field.field.name, change_logs)
+
     def _handle_one_to_one(self, related_field: OneToOneField, selected_user, current_user, change_logs):
         related_name = related_field.get_accessor_name()
-        # for one to one relationships, getattr returns the related object
         related_object = getattr(selected_user, related_name, None)
 
         if related_object:
-            # set the one to one field to the current user
             setattr(related_object, related_field.field.name, current_user)
             related_object.save()
-            log_entry = f'Transferred {related_field.field.name} from {selected_user} to {current_user}'
-            logger.info(log_entry)
-            change_logs.append(log_entry)
+            self.log_change(selected_user, current_user, related_field.field.name, change_logs)
 
     def _handle_many_to_many(self, related_field: ManyToManyField, selected_user, current_user, change_logs):
-        # for many to many relationships, getattr returns a manager
         related_manager = getattr(selected_user, related_field.name, None)
-        if related_manager:
-            # get all the related objects
+        if related_manager.count() > 0:
             related_queryset = related_manager.all()
-            # add the related objects to the current user
             getattr(current_user, related_field.name).add(*related_queryset)
-            log_entry = f'Transferred {related_field.name} from {selected_user} to {current_user}'
-            logger.info(log_entry)
-            change_logs.append(log_entry)
+            self.log_change(selected_user, current_user, related_field.name, change_logs)
 
-    def _handle_many_to_one_rel(self, related_object: ManyToOneRel, selected_user: User, current_user: User, change_logs: List[str]):
-        """
-        Handles ManyToOneRel relationships, where multiple objects relate to the User via a ForeignKey.
-        """
+    def _handle_many_to_one_rel(
+        self, related_object: ManyToOneRel, selected_user: User, current_user: User, change_logs: List[str]
+    ):
         related_model = related_object.related_model
         related_name = related_object.field.name
 
-        # for many to one relationships, we need a queryset
         related_queryset = related_model.objects.filter(**{related_name: selected_user})
-        for obj in related_queryset:
-            setattr(obj, related_name, current_user)
-            obj.save()
-            log_entry = f'Transferred {related_name} from {selected_user} to {current_user}'
-            logger.info(log_entry)
-            change_logs.append(log_entry)
 
-    def _handle_one_to_one_reverse(self, related_object: OneToOneField, selected_user: User, current_user: User, change_logs: List[str]):
-        """
-        Handles reverse OneToOneField relationships.
-        """
+        if related_queryset.count() > 0:
+            for obj in related_queryset:
+                setattr(obj, related_name, current_user)
+                obj.save()
+                self.log_change(selected_user, current_user, related_name, change_logs)
+
+    def _handle_one_to_one_reverse(
+        self, related_object: OneToOneField, selected_user: User, current_user: User, change_logs: List[str]
+    ):
         related_model = related_object.related_model
         field_name = related_object.field.name
 
@@ -253,41 +237,40 @@ class TransferUserView(View):
             related_instance = related_model.objects.filter(**{field_name: selected_user}).first()
             setattr(related_instance, field_name, current_user)
             related_instance.save()
-            log_entry = f'Transferred {field_name} from {selected_user} to {current_user}'
-            logger.info(log_entry)
-            change_logs.append(log_entry)
+            self.log_change(selected_user, current_user, field_name, change_logs)
         except related_model.DoesNotExist:
             logger.warning(f"No related instance found for reverse OneToOneField {field_name} for {selected_user}")
-    
-    def _handle_foreign_key_reverse(self, related_object: ForeignKey, selected_user: User, current_user: User, change_logs: List[str]):
-        """
-        Handles reverse ForeignKey relationships.
-        """
+
+    def _handle_foreign_key_reverse(
+        self, related_object: ForeignKey, selected_user: User, current_user: User, change_logs: List[str]
+    ):
         related_model = related_object.related_model
         field_name = related_object.field.name
 
         related_queryset = related_model.objects.filter(**{field_name: selected_user})
-        for obj in related_queryset:
-            setattr(obj, field_name, current_user)
-            obj.save()
-            log_entry = f'Transferred {field_name} from {selected_user} to {current_user}'
-            logger.info(log_entry)
-            change_logs.append(log_entry)
 
-    def _handle_many_to_many_reverse(self, related_object: ManyToManyField, selected_user: User, current_user: User, change_logs: List[str]):
-        """
-        Handles reverse ManyToManyField relationships.
-        """
+        if related_queryset.count() > 0:
+            for obj in related_queryset:
+                setattr(obj, field_name, current_user)
+                obj.save()
+                self.log_change(selected_user, current_user, field_name, change_logs)
+
+    def _handle_many_to_many_reverse(
+        self, related_object: ManyToManyField, selected_user: User, current_user: User, change_logs: List[str]
+    ):
         related_model = related_object.related_model
         field_name = related_object.field.name
 
-        related_manager = related_model.objects.filter(**{field_name: selected_user})
-        if related_manager:
-            related_qs = related_manager.all()
-            getattr(current_user, field_name).add(*related_qs)
-            log_entry = f'Transferred {field_name} from {selected_user} to {current_user}'
-            logger.info(log_entry)
-            change_logs.append(log_entry)
+        related_queryset = related_model.objects.filter(**{field_name: selected_user})
+        if related_queryset.count() > 0:
+            getattr(current_user, field_name).add(*related_queryset)
+            self.log_change(selected_user, current_user, field_name, change_logs)
+
+    @classmethod
+    def log_change(cls, selected_user, current_user, field_name, change_logs):
+        log_entry = f"Transferred {field_name} from {selected_user} to {current_user}"
+        logger.info(log_entry)
+        change_logs.append(log_entry)
 
     @classmethod
     def get_domains(cls, user):

From 4c737bab25ecbc1db06b20aed6e0ba0b9d091f96 Mon Sep 17 00:00:00 2001
From: matthewswspence <Matthewswspence@gmail.com>
Date: Tue, 14 Jan 2025 16:07:12 -0600
Subject: [PATCH 03/20] clean up for PR

---
 src/registrar/tests/test_admin.py    | 4 ----
 src/registrar/views/transfer_user.py | 9 ++-------
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index 0cc75d6e2..8baf5e42d 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -2882,10 +2882,6 @@ class TestTransferUser(WebTest):
             submit_form["selected_user"] = self.user2.pk
             after_submit = submit_form.submit().follow()
 
-            print("mock_success_message.call_args_list:")
-            for call in mock_success_message.call_args_list:
-                print(call)
-
             self.assertContains(after_submit, "<h1>Change user</h1>")
 
             mock_success_message.assert_any_call(
diff --git a/src/registrar/views/transfer_user.py b/src/registrar/views/transfer_user.py
index e10a5f3fc..fdef91b43 100644
--- a/src/registrar/views/transfer_user.py
+++ b/src/registrar/views/transfer_user.py
@@ -1,22 +1,18 @@
 import logging
 from django.db import transaction
-from django.db.models import Manager, ForeignKey, OneToOneField, ManyToManyField, ManyToOneRel
+from django.db.models import ForeignKey, OneToOneField, ManyToManyField, ManyToOneRel
 
 from django.shortcuts import render, get_object_or_404, redirect
 from django.views import View
-from registrar import models
 from registrar.models.domain import Domain
-from registrar.models.domain_information import DomainInformation
 from registrar.models.domain_request import DomainRequest
-from registrar.models.portfolio import Portfolio
 from registrar.models.user import User
 from django.contrib.admin import site
 from django.contrib import messages
 
 from registrar.models.user_domain_role import UserDomainRole
 from registrar.models.user_portfolio_permission import UserPortfolioPermission
-from registrar.models.verified_by_staff import VerifiedByStaff
-from typing import Any, List
+from typing import List
 
 logger = logging.getLogger(__name__)
 
@@ -85,7 +81,6 @@ class TransferUserView(View):
                 logger.debug("Deleting old user")
                 selected_user.delete()
                 messages.success(request, f"Deleted {selected_user} {selected_user.username}")
-
         except Exception as e:
             messages.error(request, f"An error occurred during the transfer: {e}")
             logger.error(f"An error occurred during the transfer: {e}", exc_info=True)

From c0f5dca8c1d7b514209556f2f85ed06acb0583d5 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 15 Jan 2025 14:50:59 -0500
Subject: [PATCH 04/20] add email to domain managers on domain invitation

---
 .../emails/domain_manager_notification.txt    | 43 +++++++++++++++++++
 .../domain_manager_notification_subject.txt   |  1 +
 src/registrar/utility/email_invitations.py    | 37 ++++++++++++++++
 3 files changed, 81 insertions(+)
 create mode 100644 src/registrar/templates/emails/domain_manager_notification.txt
 create mode 100644 src/registrar/templates/emails/domain_manager_notification_subject.txt

diff --git a/src/registrar/templates/emails/domain_manager_notification.txt b/src/registrar/templates/emails/domain_manager_notification.txt
new file mode 100644
index 000000000..aa8c6bf34
--- /dev/null
+++ b/src/registrar/templates/emails/domain_manager_notification.txt
@@ -0,0 +1,43 @@
+{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #}
+Hi,{% if domain_manager and domain_manager.first_name %} {{ domain_manager.first_name }}.{% endif %}
+
+A domain manager was invited to {{ domain.name }}.
+DOMAIN: {{ domain.name }}
+INVITED BY: {{ requestor_email }}
+INVITED ON: {{date}}
+MANAGER INVITED: {{ invited_email_address }}
+
+
+----------------------------------------------------------------
+
+
+NEXT STEPS
+
+The person who received the invitation will become a domain manager once they log in to the
+.gov registrar. They'll need to access the registrar using a Login.gov account that's
+associated with the invited email address.
+
+If you need to cancel this invitation or remove the domain manager (because they've already
+logged in), you can do that by going to this domain in the .gov registrar <https://manage.get.gov/>.
+
+
+WHY DID YOU RECEIVE THIS EMAIL? 
+
+You’re listed as a domain manager for {{ domain.name }}, so you’ll receive a notification whenever
+someone is invited to manage that domain.
+
+If you have questions or concerns, reach out to the person who sent the invitation or reply to this email.
+
+
+THANK YOU
+.Gov helps the public identify official, trusted information. Thank you for using a .gov domain.
+
+----------------------------------------------------------------
+
+The .gov team
+Contact us: <https://get.gov/contact/>
+Learn about .gov <https://get.gov>
+
+The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency
+(CISA) <https://cisa.gov/>
+{% endautoescape %}
diff --git a/src/registrar/templates/emails/domain_manager_notification_subject.txt b/src/registrar/templates/emails/domain_manager_notification_subject.txt
new file mode 100644
index 000000000..0e9918de0
--- /dev/null
+++ b/src/registrar/templates/emails/domain_manager_notification_subject.txt
@@ -0,0 +1 @@
+A domain manager was invited to {{ domain.name }}
\ No newline at end of file
diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py
index 48c796340..fda901fba 100644
--- a/src/registrar/utility/email_invitations.py
+++ b/src/registrar/utility/email_invitations.py
@@ -1,6 +1,8 @@
+from datetime import date
 from django.conf import settings
 from registrar.models import DomainInvitation
 from registrar.models.domain import Domain
+from registrar.models.user_domain_role import UserDomainRole
 from registrar.utility.errors import (
     AlreadyDomainInvitedError,
     AlreadyDomainManagerError,
@@ -41,6 +43,39 @@ def send_domain_invitation_email(
 
     send_invitation_email(email, requestor_email, domains, requested_user)
 
+    # send emails to domain managers
+    for domain in domains:
+        send_emails_to_domain_managers(
+            email=email,
+            requestor_email=requestor_email,
+            domain=domain,
+            requested_user=requested_user,
+        )
+
+
+def send_emails_to_domain_managers(email: str, requestor_email, domain: Domain, requested_user=None):
+    # Get each domain manager from list
+    user_domain_roles = UserDomainRole.objects.filter(domain=domain)
+    for user_domain_role in user_domain_roles:
+        # Send email to each domain manager
+        user = user_domain_role.user
+        try:
+            send_templated_email(
+                "emails/domain_manager_notification.txt",
+                "emails/domain_manager_notification_subject.txt",
+                to_address=user.email,
+                context={
+                    "domain": domain,
+                    "requestor_email": requestor_email,
+                    "invited_email_address": email,
+                    "domain_manager": user,
+                    "date": date.today(),
+                },
+            )
+        except EmailSendingError as err:
+            domain_names = ", ".join([domain.name for domain in domains])
+            raise EmailSendingError(f"Could not send email invitation to {email} for domains: {domain_names}") from err
+
 
 def normalize_domains(domains):
     """Ensures domains is always a list."""
@@ -69,6 +104,8 @@ def validate_invitation(email, domains, requestor, is_member_of_different_org):
     for domain in domains:
         validate_existing_invitation(email, domain)
 
+        # NOTE: should we also be validating against existing user_domain_roles
+
 
 def check_outside_org_membership(email, requestor, is_member_of_different_org):
     """Raise an error if the email belongs to a different organization."""

From ca136c650c710a14a17b9f3fd8501fee48a36b23 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 15 Jan 2025 14:56:04 -0500
Subject: [PATCH 05/20] updated error message when EmailSendingError

---
 src/registrar/utility/email_invitations.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py
index fda901fba..ba660499e 100644
--- a/src/registrar/utility/email_invitations.py
+++ b/src/registrar/utility/email_invitations.py
@@ -73,8 +73,7 @@ def send_emails_to_domain_managers(email: str, requestor_email, domain: Domain,
                 },
             )
         except EmailSendingError as err:
-            domain_names = ", ".join([domain.name for domain in domains])
-            raise EmailSendingError(f"Could not send email invitation to {email} for domains: {domain_names}") from err
+            raise EmailSendingError(f"Could not send email manager notification to {user.email} for domains: {domain.name}") from err
 
 
 def normalize_domains(domains):

From 287638786dbdc4a47c405cfb3e10dc0c7a8115cb Mon Sep 17 00:00:00 2001
From: Rachid Mrad <rachid.mrad@ecstech.com>
Date: Wed, 15 Jan 2025 15:03:24 -0500
Subject: [PATCH 06/20] informational alerts on change forms

---
 src/registrar/admin.py                             |  4 +++-
 .../admin/domain_invitation_change_form.html       | 14 ++++++++++++++
 .../django/admin/user_domain_role_change_form.html | 14 ++++++++++++++
 3 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100644 src/registrar/templates/django/admin/domain_invitation_change_form.html
 create mode 100644 src/registrar/templates/django/admin/user_domain_role_change_form.html

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 1b6e2de6a..e89147b11 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -1367,6 +1367,8 @@ class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin):
 
     autocomplete_fields = ["user", "domain"]
 
+    change_form_template = "django/admin/user_domain_role_change_form.html"
+
     # Fixes a bug where non-superusers are redirected to the main page
     def delete_view(self, request, object_id, extra_context=None):
         """Custom delete_view implementation that specifies redirect behaviour"""
@@ -1500,7 +1502,7 @@ class DomainInvitationAdmin(BaseInvitationAdmin):
 
     autocomplete_fields = ["domain"]
 
-    change_form_template = "django/admin/email_clipboard_change_form.html"
+    change_form_template = "django/admin/domain_invitation_change_form.html"
 
     # Select domain invitations to change -> Domain invitations
     def changelist_view(self, request, extra_context=None):
diff --git a/src/registrar/templates/django/admin/domain_invitation_change_form.html b/src/registrar/templates/django/admin/domain_invitation_change_form.html
new file mode 100644
index 000000000..6ce6ed0d1
--- /dev/null
+++ b/src/registrar/templates/django/admin/domain_invitation_change_form.html
@@ -0,0 +1,14 @@
+{% extends 'django/admin/email_clipboard_change_form.html' %}
+{% load custom_filters %}
+{% load i18n static %}
+
+{% block content_subtitle %}
+    <div class="usa-alert usa-alert--info usa-alert--slim">
+        <div class="usa-alert__body margin-left-1 maxw-none">
+            <p class="usa-alert__text maxw-none">
+                If you add someone to a domain here, it will trigger emails to the invitee and all managers of the domain when you click "save." If you don't want to trigger those emails, use the <a class="usa-link" href="{% url 'admin:registrar_userdomainrole_changelist' %}">User domain roles permissions table</a> instead.
+            </p>
+        </div>
+    </div>
+    {{ block.super }}
+{% endblock %}
diff --git a/src/registrar/templates/django/admin/user_domain_role_change_form.html b/src/registrar/templates/django/admin/user_domain_role_change_form.html
new file mode 100644
index 000000000..200734ec1
--- /dev/null
+++ b/src/registrar/templates/django/admin/user_domain_role_change_form.html
@@ -0,0 +1,14 @@
+{% extends 'django/admin/email_clipboard_change_form.html' %}
+{% load custom_filters %}
+{% load i18n static %}
+
+{% block content_subtitle %}
+    <div class="usa-alert usa-alert--info usa-alert--slim">
+        <div class="usa-alert__body margin-left-1 maxw-none">
+            <p class="usa-alert__text maxw-none">
+                If you add someone to a domain here, it won't trigger any emails. To trigger emails, use the <a class="usa-link" href="{% url 'admin:registrar_domaininvitation_changelist' %}">User Domain Role invitations table</a> instead.
+            </p>
+        </div>
+    </div>
+    {{ block.super }}
+{% endblock %}

From cb8494017acbc69c843f19edb727a0392338b3a3 Mon Sep 17 00:00:00 2001
From: Rachid Mrad <rachid.mrad@ecstech.com>
Date: Wed, 15 Jan 2025 15:28:15 -0500
Subject: [PATCH 07/20] template tests

---
 .../admin/user_domain_role_change_form.html   |  2 +-
 src/registrar/tests/test_admin.py             | 54 +++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/src/registrar/templates/django/admin/user_domain_role_change_form.html b/src/registrar/templates/django/admin/user_domain_role_change_form.html
index 200734ec1..d8c298bc1 100644
--- a/src/registrar/templates/django/admin/user_domain_role_change_form.html
+++ b/src/registrar/templates/django/admin/user_domain_role_change_form.html
@@ -6,7 +6,7 @@
     <div class="usa-alert usa-alert--info usa-alert--slim">
         <div class="usa-alert__body margin-left-1 maxw-none">
             <p class="usa-alert__text maxw-none">
-                If you add someone to a domain here, it won't trigger any emails. To trigger emails, use the <a class="usa-link" href="{% url 'admin:registrar_domaininvitation_changelist' %}">User Domain Role invitations table</a> instead.
+                If you add someone to a domain here, it will not trigger any emails. To trigger emails, use the <a class="usa-link" href="{% url 'admin:registrar_domaininvitation_changelist' %}">User Domain Role invitations table</a> instead.
             </p>
         </div>
     </div>
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index 2a7a52a13..673057e20 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -165,6 +165,33 @@ class TestDomainInvitationAdmin(TestCase):
             response, "Domain invitations contain all individuals who have been invited to manage a .gov domain."
         )
         self.assertContains(response, "Show more")
+    
+    @less_console_noise_decorator
+    def test_has_change_form_description(self):
+        """Tests if this model has a model description on the change form view"""
+        self.client.force_login(self.superuser)
+
+        domain, _ = Domain.objects.get_or_create(
+            name="systemofadown.com"
+        )
+
+        domain_invitation, _ = DomainInvitation.objects.get_or_create(
+            email="toxicity@systemofadown.com", domain=domain
+        )
+
+        response = self.client.get(
+            "/admin/registrar/domaininvitation/{}/change/".format(domain_invitation.pk),
+            follow=True,
+        )
+
+        # Make sure that the page is loaded correctly
+        self.assertEqual(response.status_code, 200)
+
+        # Test for a description snippet
+        self.assertContains(
+            response,
+            "If you add someone to a domain here, it will trigger emails to the invitee and all managers of the domain when you click",
+        )
 
     @less_console_noise_decorator
     def test_get_filters(self):
@@ -1956,6 +1983,33 @@ class TestUserDomainRoleAdmin(TestCase):
             response, "This table represents the managers who are assigned to each domain in the registrar"
         )
         self.assertContains(response, "Show more")
+    
+    @less_console_noise_decorator
+    def test_has_change_form_description(self):
+        """Tests if this model has a model description on the change form view"""
+        self.client.force_login(self.superuser)
+
+        domain, _ = Domain.objects.get_or_create(
+            name="systemofadown.com"
+        )
+
+        user_domain_role, _ = UserDomainRole.objects.get_or_create(
+            user=self.superuser, domain=domain, role=[UserDomainRole.Roles.MANAGER]
+        )
+
+        response = self.client.get(
+            "/admin/registrar/userdomainrole/{}/change/".format(user_domain_role.pk),
+            follow=True,
+        )
+
+        # Make sure that the page is loaded correctly
+        self.assertEqual(response.status_code, 200)
+
+        # Test for a description snippet
+        self.assertContains(
+            response,
+            "If you add someone to a domain here, it will not trigger any emails.",
+        )
 
     def test_domain_sortable(self):
         """Tests if the UserDomainrole sorts by domain correctly"""

From 4a0dc40cee4e26d4f948afb4b76105a43ad35cc5 Mon Sep 17 00:00:00 2001
From: Rachid Mrad <rachid.mrad@ecstech.com>
Date: Wed, 15 Jan 2025 16:00:03 -0500
Subject: [PATCH 08/20] fix mock_send_domain_email unit tests

---
 src/registrar/tests/test_views_domain.py | 43 +++++++++++++-----------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py
index f13490312..a9de8d6e7 100644
--- a/src/registrar/tests/test_views_domain.py
+++ b/src/registrar/tests/test_views_domain.py
@@ -750,11 +750,12 @@ class TestDomainManagers(TestDomainOverview):
         response = self.client.get(reverse("domain-users-add", kwargs={"pk": self.domain.id}))
         self.assertContains(response, "Add a domain manager")
 
-    @boto3_mocking.patching
     @less_console_noise_decorator
-    def test_domain_user_add_form(self):
+    @patch("registrar.views.domain.send_domain_invitation_email")
+    def test_domain_user_add_form(self, mock_send_domain_email):
         """Adding an existing user works."""
         get_user_model().objects.get_or_create(email="mayor@igorville.gov")
+        user = User.objects.filter(email="mayor@igorville.gov").first()
         add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id}))
         session_id = self.app.cookies[settings.SESSION_COOKIE_NAME]
 
@@ -762,10 +763,11 @@ class TestDomainManagers(TestDomainOverview):
 
         self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
 
-        mock_client = MockSESClient()
-        with boto3_mocking.clients.handler_for("sesv2", mock_client):
-            with less_console_noise():
-                success_result = add_page.form.submit()
+        success_result = add_page.form.submit()
+
+        mock_send_domain_email.assert_called_once_with(
+            email="mayor@igorville.gov", requestor=self.user, domains=self.domain, is_member_of_different_org=None, requested_user=user
+        )
 
         self.assertEqual(success_result.status_code, 302)
         self.assertEqual(
@@ -974,13 +976,13 @@ class TestDomainManagers(TestDomainOverview):
         success_page = success_result.follow()
         self.assertContains(success_page, "Failed to send email.")
 
-    @boto3_mocking.patching
     @less_console_noise_decorator
-    def test_domain_invitation_created(self):
+    @patch("registrar.views.domain.send_domain_invitation_email")
+    def test_domain_invitation_created(self, mock_send_domain_email):
         """Add user on a nonexistent email creates an invitation.
 
         Adding a non-existent user sends an email as a side-effect, so mock
-        out the boto3 SES email sending here.
+        out send_domain_invitation_email here.
         """
         # make sure there is no user with this email
         email_address = "mayor@igorville.gov"
@@ -993,10 +995,11 @@ class TestDomainManagers(TestDomainOverview):
         add_page.form["email"] = email_address
         self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
 
-        mock_client = MockSESClient()
-        with boto3_mocking.clients.handler_for("sesv2", mock_client):
-            with less_console_noise():
-                success_result = add_page.form.submit()
+        success_result = add_page.form.submit()
+
+        mock_send_domain_email.assert_called_once_with(
+            email="mayor@igorville.gov", requestor=self.user, domains=self.domain, is_member_of_different_org=None
+        )
 
         self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
         success_page = success_result.follow()
@@ -1005,13 +1008,13 @@ class TestDomainManagers(TestDomainOverview):
         self.assertContains(success_page, "Cancel")  # link to cancel invitation
         self.assertTrue(DomainInvitation.objects.filter(email=email_address).exists())
 
-    @boto3_mocking.patching
     @less_console_noise_decorator
-    def test_domain_invitation_created_for_caps_email(self):
+    @patch("registrar.views.domain.send_domain_invitation_email")
+    def test_domain_invitation_created_for_caps_email(self, mock_send_domain_email):
         """Add user on a nonexistent email with CAPS creates an invitation to lowercase email.
 
         Adding a non-existent user sends an email as a side-effect, so mock
-        out the boto3 SES email sending here.
+        out send_domain_invitation_email here.
         """
         # make sure there is no user with this email
         email_address = "mayor@igorville.gov"
@@ -1025,9 +1028,11 @@ class TestDomainManagers(TestDomainOverview):
         add_page.form["email"] = caps_email_address
         self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
 
-        mock_client = MockSESClient()
-        with boto3_mocking.clients.handler_for("sesv2", mock_client):
-            success_result = add_page.form.submit()
+        success_result = add_page.form.submit()
+
+        mock_send_domain_email.assert_called_once_with(
+            email="mayor@igorville.gov", requestor=self.user, domains=self.domain, is_member_of_different_org=None
+        )
 
         self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
         success_page = success_result.follow()

From c10a7738ca236a0c9572cde5a1cda8c6f8679359 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 15 Jan 2025 16:27:08 -0500
Subject: [PATCH 09/20] tests and lint

---
 src/registrar/tests/test_email_invitations.py | 303 ++++++++++++++++++
 src/registrar/utility/email_invitations.py    |   8 +-
 2 files changed, 307 insertions(+), 4 deletions(-)
 create mode 100644 src/registrar/tests/test_email_invitations.py

diff --git a/src/registrar/tests/test_email_invitations.py b/src/registrar/tests/test_email_invitations.py
new file mode 100644
index 000000000..6a0423f4d
--- /dev/null
+++ b/src/registrar/tests/test_email_invitations.py
@@ -0,0 +1,303 @@
+import unittest
+from unittest.mock import patch, MagicMock
+from datetime import date
+from registrar.utility.email import EmailSendingError
+from registrar.utility.email_invitations import send_domain_invitation_email
+
+
+class DomainInvitationEmail(unittest.TestCase):
+
+    @patch("registrar.utility.email_invitations.send_templated_email")
+    @patch("registrar.utility.email_invitations.UserDomainRole.objects.filter")
+    @patch("registrar.utility.email_invitations.validate_invitation")
+    @patch("registrar.utility.email_invitations.get_requestor_email")
+    @patch("registrar.utility.email_invitations.send_invitation_email")
+    @patch("registrar.utility.email_invitations.normalize_domains")
+    def test_send_domain_invitation_email(
+        self,
+        mock_normalize_domains,
+        mock_send_invitation_email,
+        mock_get_requestor_email,
+        mock_validate_invitation,
+        mock_user_domain_role_filter,
+        mock_send_templated_email,
+    ):
+        """Test sending domain invitation email for one domain.
+        Should also send emails to manager of that domain.
+        """
+        # Setup
+        mock_domain = MagicMock(name="domain1")
+        mock_domain.name = "example.com"
+        mock_normalize_domains.return_value = [mock_domain]
+
+        mock_requestor = MagicMock()
+        mock_requestor_email = "requestor@example.com"
+        mock_get_requestor_email.return_value = mock_requestor_email
+
+        mock_user1 = MagicMock()
+        mock_user1.email = "manager1@example.com"
+
+        mock_user_domain_role_filter.return_value = [MagicMock(user=mock_user1)]
+
+        email = "invitee@example.com"
+        is_member_of_different_org = False
+
+        # Call the function
+        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)
+        mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain])
+        mock_validate_invitation.assert_called_once_with(
+            email, [mock_domain], mock_requestor, is_member_of_different_org
+        )
+        mock_send_invitation_email.assert_called_once_with(email, mock_requestor_email, [mock_domain], None)
+        mock_user_domain_role_filter.assert_called_once_with(domain=mock_domain)
+        mock_send_templated_email.assert_called_once_with(
+            "emails/domain_manager_notification.txt",
+            "emails/domain_manager_notification_subject.txt",
+            to_address=mock_user1.email,
+            context={
+                "domain": mock_domain,
+                "requestor_email": mock_requestor_email,
+                "invited_email_address": email,
+                "domain_manager": mock_user1,
+                "date": date.today(),
+            },
+        )
+
+    @patch("registrar.utility.email_invitations.send_templated_email")
+    @patch("registrar.utility.email_invitations.UserDomainRole.objects.filter")
+    @patch("registrar.utility.email_invitations.validate_invitation")
+    @patch("registrar.utility.email_invitations.get_requestor_email")
+    @patch("registrar.utility.email_invitations.send_invitation_email")
+    @patch("registrar.utility.email_invitations.normalize_domains")
+    def test_send_domain_invitation_email_multiple_domains(
+        self,
+        mock_normalize_domains,
+        mock_send_invitation_email,
+        mock_get_requestor_email,
+        mock_validate_invitation,
+        mock_user_domain_role_filter,
+        mock_send_templated_email,
+    ):
+        """Test sending domain invitation email for multiple domains.
+        Should also send emails to managers of each domain.
+        """
+        # Setup
+        # Create multiple mock domains
+        mock_domain1 = MagicMock(name="domain1")
+        mock_domain1.name = "example.com"
+        mock_domain2 = MagicMock(name="domain2")
+        mock_domain2.name = "example.org"
+
+        mock_normalize_domains.return_value = [mock_domain1, mock_domain2]
+
+        mock_requestor = MagicMock()
+        mock_requestor_email = "requestor@example.com"
+        mock_get_requestor_email.return_value = mock_requestor_email
+
+        mock_user1 = MagicMock()
+        mock_user1.email = "manager1@example.com"
+        mock_user2 = MagicMock()
+        mock_user2.email = "manager2@example.com"
+
+        # Configure domain roles for each domain
+        def filter_side_effect(domain):
+            if domain == mock_domain1:
+                return [MagicMock(user=mock_user1)]
+            elif domain == mock_domain2:
+                return [MagicMock(user=mock_user2)]
+            return []
+
+        mock_user_domain_role_filter.side_effect = filter_side_effect
+
+        email = "invitee@example.com"
+        is_member_of_different_org = False
+
+        # Call the function
+        send_domain_invitation_email(
+            email=email,
+            requestor=mock_requestor,
+            domains=[mock_domain1, mock_domain2],
+            is_member_of_different_org=is_member_of_different_org,
+        )
+
+        # Assertions
+        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_validate_invitation.assert_called_once_with(
+            email, [mock_domain1, mock_domain2], mock_requestor, is_member_of_different_org
+        )
+        mock_send_invitation_email.assert_called_once_with(
+            email, mock_requestor_email, [mock_domain1, mock_domain2], None
+        )
+
+        # Check that domain manager emails were sent for both domains
+        mock_user_domain_role_filter.assert_any_call(domain=mock_domain1)
+        mock_user_domain_role_filter.assert_any_call(domain=mock_domain2)
+
+        mock_send_templated_email.assert_any_call(
+            "emails/domain_manager_notification.txt",
+            "emails/domain_manager_notification_subject.txt",
+            to_address=mock_user1.email,
+            context={
+                "domain": mock_domain1,
+                "requestor_email": mock_requestor_email,
+                "invited_email_address": email,
+                "domain_manager": mock_user1,
+                "date": date.today(),
+            },
+        )
+        mock_send_templated_email.assert_any_call(
+            "emails/domain_manager_notification.txt",
+            "emails/domain_manager_notification_subject.txt",
+            to_address=mock_user2.email,
+            context={
+                "domain": mock_domain2,
+                "requestor_email": mock_requestor_email,
+                "invited_email_address": email,
+                "domain_manager": mock_user2,
+                "date": date.today(),
+            },
+        )
+
+        # Verify the total number of calls to send_templated_email
+        self.assertEqual(mock_send_templated_email.call_count, 2)
+
+    @patch("registrar.utility.email_invitations.validate_invitation")
+    def test_send_domain_invitation_email_raises_invite_validation_exception(self, mock_validate_invitation):
+        """Test sending domain invitation email for one domain and assert exception
+        when invite validation fails.
+        """
+        # Setup
+        mock_validate_invitation.side_effect = ValueError("Validation failed")
+        email = "invitee@example.com"
+        requestor = MagicMock()
+        domain = MagicMock()
+
+        # Call and assert exception
+        with self.assertRaises(ValueError) as context:
+            send_domain_invitation_email(email, requestor, domain, is_member_of_different_org=False)
+
+        self.assertEqual(str(context.exception), "Validation failed")
+        mock_validate_invitation.assert_called_once()
+
+    @patch("registrar.utility.email_invitations.get_requestor_email")
+    def test_send_domain_invitation_email_raises_get_requestor_email_exception(self, mock_get_requestor_email):
+        """Test sending domain invitation email for one domain and assert exception
+        when get_requestor_email fails.
+        """
+        # Setup
+        mock_get_requestor_email.side_effect = ValueError("Validation failed")
+        email = "invitee@example.com"
+        requestor = MagicMock()
+        domain = MagicMock()
+
+        # Call and assert exception
+        with self.assertRaises(ValueError) as context:
+            send_domain_invitation_email(email, requestor, domain, is_member_of_different_org=False)
+
+        self.assertEqual(str(context.exception), "Validation failed")
+        mock_get_requestor_email.assert_called_once()
+
+    @patch("registrar.utility.email_invitations.validate_invitation")
+    @patch("registrar.utility.email_invitations.get_requestor_email")
+    @patch("registrar.utility.email_invitations.send_invitation_email")
+    @patch("registrar.utility.email_invitations.normalize_domains")
+    def test_send_domain_invitation_email_raises_sending_email_exception(
+        self,
+        mock_normalize_domains,
+        mock_send_invitation_email,
+        mock_get_requestor_email,
+        mock_validate_invitation,
+    ):
+        """Test sending domain invitation email for one domain and assert exception
+        when send_invitation_email fails.
+        """
+        # Setup
+        mock_domain = MagicMock(name="domain1")
+        mock_domain.name = "example.com"
+        mock_normalize_domains.return_value = [mock_domain]
+
+        mock_requestor = MagicMock()
+        mock_requestor_email = "requestor@example.com"
+        mock_get_requestor_email.return_value = mock_requestor_email
+
+        mock_user1 = MagicMock()
+        mock_user1.email = "manager1@example.com"
+
+        email = "invitee@example.com"
+        is_member_of_different_org = False
+
+        mock_send_invitation_email.side_effect = EmailSendingError("Error sending email")
+
+        # 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,
+            )
+
+        # Assertions
+        mock_normalize_domains.assert_called_once_with(mock_domain)
+        mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain])
+        mock_validate_invitation.assert_called_once_with(
+            email, [mock_domain], mock_requestor, is_member_of_different_org
+        )
+        self.assertEqual(str(context.exception), "Error sending email")
+
+    @patch("registrar.utility.email_invitations.send_emails_to_domain_managers")
+    @patch("registrar.utility.email_invitations.validate_invitation")
+    @patch("registrar.utility.email_invitations.get_requestor_email")
+    @patch("registrar.utility.email_invitations.send_invitation_email")
+    @patch("registrar.utility.email_invitations.normalize_domains")
+    def test_send_domain_invitation_email_manager_emails_send_mail_exception(
+        self,
+        mock_normalize_domains,
+        mock_send_invitation_email,
+        mock_get_requestor_email,
+        mock_validate_invitation,
+        mock_send_domain_manager_emails,
+    ):
+        """Test sending domain invitation email for one domain and assert exception
+        when send_emails_to_domain_managers fails.
+        """
+        # Setup
+        mock_domain = MagicMock(name="domain1")
+        mock_domain.name = "example.com"
+        mock_normalize_domains.return_value = [mock_domain]
+
+        mock_requestor = MagicMock()
+        mock_requestor_email = "requestor@example.com"
+        mock_get_requestor_email.return_value = mock_requestor_email
+
+        email = "invitee@example.com"
+        is_member_of_different_org = False
+
+        mock_send_domain_manager_emails.side_effect = EmailSendingError("Error sending email")
+
+        # 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,
+            )
+
+        # Assertions
+        mock_normalize_domains.assert_called_once_with(mock_domain)
+        mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain])
+        mock_validate_invitation.assert_called_once_with(
+            email, [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")
diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py
index ba660499e..c2bf22c30 100644
--- a/src/registrar/utility/email_invitations.py
+++ b/src/registrar/utility/email_invitations.py
@@ -1,8 +1,6 @@
 from datetime import date
 from django.conf import settings
-from registrar.models import DomainInvitation
-from registrar.models.domain import Domain
-from registrar.models.user_domain_role import UserDomainRole
+from registrar.models import Domain, DomainInvitation, UserDomainRole
 from registrar.utility.errors import (
     AlreadyDomainInvitedError,
     AlreadyDomainManagerError,
@@ -73,7 +71,9 @@ def send_emails_to_domain_managers(email: str, requestor_email, domain: Domain,
                 },
             )
         except EmailSendingError as err:
-            raise EmailSendingError(f"Could not send email manager notification to {user.email} for domains: {domain.name}") from err
+            raise EmailSendingError(
+                f"Could not send email manager notification to {user.email} for domains: {domain.name}"
+            ) from err
 
 
 def normalize_domains(domains):

From 13c9e1c1135e3bb0b1fa936614721c214a14df69 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 15 Jan 2025 16:35:04 -0500
Subject: [PATCH 10/20] lint

---
 src/registrar/tests/test_admin.py             | 18 ++++++------------
 src/registrar/tests/test_email_invitations.py |  8 ++++++++
 src/registrar/tests/test_views_domain.py      |  8 ++++++--
 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index 673057e20..1decf02dd 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -165,19 +165,15 @@ class TestDomainInvitationAdmin(TestCase):
             response, "Domain invitations contain all individuals who have been invited to manage a .gov domain."
         )
         self.assertContains(response, "Show more")
-    
+
     @less_console_noise_decorator
     def test_has_change_form_description(self):
         """Tests if this model has a model description on the change form view"""
         self.client.force_login(self.superuser)
 
-        domain, _ = Domain.objects.get_or_create(
-            name="systemofadown.com"
-        )
+        domain, _ = Domain.objects.get_or_create(name="systemofadown.com")
 
-        domain_invitation, _ = DomainInvitation.objects.get_or_create(
-            email="toxicity@systemofadown.com", domain=domain
-        )
+        domain_invitation, _ = DomainInvitation.objects.get_or_create(email="toxicity@systemofadown.com", domain=domain)
 
         response = self.client.get(
             "/admin/registrar/domaininvitation/{}/change/".format(domain_invitation.pk),
@@ -190,7 +186,7 @@ class TestDomainInvitationAdmin(TestCase):
         # Test for a description snippet
         self.assertContains(
             response,
-            "If you add someone to a domain here, it will trigger emails to the invitee and all managers of the domain when you click",
+            "If you add someone to a domain here, it will trigger emails to the invitee and all managers of the domain",
         )
 
     @less_console_noise_decorator
@@ -1983,15 +1979,13 @@ class TestUserDomainRoleAdmin(TestCase):
             response, "This table represents the managers who are assigned to each domain in the registrar"
         )
         self.assertContains(response, "Show more")
-    
+
     @less_console_noise_decorator
     def test_has_change_form_description(self):
         """Tests if this model has a model description on the change form view"""
         self.client.force_login(self.superuser)
 
-        domain, _ = Domain.objects.get_or_create(
-            name="systemofadown.com"
-        )
+        domain, _ = Domain.objects.get_or_create(name="systemofadown.com")
 
         user_domain_role, _ = UserDomainRole.objects.get_or_create(
             user=self.superuser, domain=domain, role=[UserDomainRole.Roles.MANAGER]
diff --git a/src/registrar/tests/test_email_invitations.py b/src/registrar/tests/test_email_invitations.py
index 6a0423f4d..87384d3be 100644
--- a/src/registrar/tests/test_email_invitations.py
+++ b/src/registrar/tests/test_email_invitations.py
@@ -4,9 +4,12 @@ from datetime import date
 from registrar.utility.email import EmailSendingError
 from registrar.utility.email_invitations import send_domain_invitation_email
 
+from api.tests.common import less_console_noise_decorator
+
 
 class DomainInvitationEmail(unittest.TestCase):
 
+    @less_console_noise_decorator
     @patch("registrar.utility.email_invitations.send_templated_email")
     @patch("registrar.utility.email_invitations.UserDomainRole.objects.filter")
     @patch("registrar.utility.email_invitations.validate_invitation")
@@ -71,6 +74,7 @@ class DomainInvitationEmail(unittest.TestCase):
             },
         )
 
+    @less_console_noise_decorator
     @patch("registrar.utility.email_invitations.send_templated_email")
     @patch("registrar.utility.email_invitations.UserDomainRole.objects.filter")
     @patch("registrar.utility.email_invitations.validate_invitation")
@@ -170,6 +174,7 @@ class DomainInvitationEmail(unittest.TestCase):
         # Verify the total number of calls to send_templated_email
         self.assertEqual(mock_send_templated_email.call_count, 2)
 
+    @less_console_noise_decorator
     @patch("registrar.utility.email_invitations.validate_invitation")
     def test_send_domain_invitation_email_raises_invite_validation_exception(self, mock_validate_invitation):
         """Test sending domain invitation email for one domain and assert exception
@@ -188,6 +193,7 @@ class DomainInvitationEmail(unittest.TestCase):
         self.assertEqual(str(context.exception), "Validation failed")
         mock_validate_invitation.assert_called_once()
 
+    @less_console_noise_decorator
     @patch("registrar.utility.email_invitations.get_requestor_email")
     def test_send_domain_invitation_email_raises_get_requestor_email_exception(self, mock_get_requestor_email):
         """Test sending domain invitation email for one domain and assert exception
@@ -206,6 +212,7 @@ class DomainInvitationEmail(unittest.TestCase):
         self.assertEqual(str(context.exception), "Validation failed")
         mock_get_requestor_email.assert_called_once()
 
+    @less_console_noise_decorator
     @patch("registrar.utility.email_invitations.validate_invitation")
     @patch("registrar.utility.email_invitations.get_requestor_email")
     @patch("registrar.utility.email_invitations.send_invitation_email")
@@ -254,6 +261,7 @@ class DomainInvitationEmail(unittest.TestCase):
         )
         self.assertEqual(str(context.exception), "Error sending email")
 
+    @less_console_noise_decorator
     @patch("registrar.utility.email_invitations.send_emails_to_domain_managers")
     @patch("registrar.utility.email_invitations.validate_invitation")
     @patch("registrar.utility.email_invitations.get_requestor_email")
diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py
index a9de8d6e7..45758e502 100644
--- a/src/registrar/tests/test_views_domain.py
+++ b/src/registrar/tests/test_views_domain.py
@@ -9,7 +9,7 @@ from registrar.utility.email import EmailSendingError
 from waffle.testutils import override_flag
 from api.tests.common import less_console_noise_decorator
 from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices
-from .common import MockEppLib, MockSESClient, create_user  # type: ignore
+from .common import MockEppLib, create_user  # type: ignore
 from django_webtest import WebTest  # type: ignore
 import boto3_mocking  # type: ignore
 
@@ -766,7 +766,11 @@ class TestDomainManagers(TestDomainOverview):
         success_result = add_page.form.submit()
 
         mock_send_domain_email.assert_called_once_with(
-            email="mayor@igorville.gov", requestor=self.user, domains=self.domain, is_member_of_different_org=None, requested_user=user
+            email="mayor@igorville.gov",
+            requestor=self.user,
+            domains=self.domain,
+            is_member_of_different_org=None,
+            requested_user=user,
         )
 
         self.assertEqual(success_result.status_code, 302)

From 3420cc3329cf884ed3b31a9cb0adc53625a24cee Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 15 Jan 2025 16:41:49 -0500
Subject: [PATCH 11/20] more linter

---
 src/registrar/utility/email_invitations.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py
index c2bf22c30..3653d4290 100644
--- a/src/registrar/utility/email_invitations.py
+++ b/src/registrar/utility/email_invitations.py
@@ -76,7 +76,7 @@ def send_emails_to_domain_managers(email: str, requestor_email, domain: Domain,
             ) from err
 
 
-def normalize_domains(domains):
+def normalize_domains(domains: Domain | list[Domain]) -> list[Domain]:
     """Ensures domains is always a list."""
     return [domains] if isinstance(domains, Domain) else domains
 

From 4aa80abc8ce17aa1ba3c1d762fa8a959c1d4a727 Mon Sep 17 00:00:00 2001
From: matthewswspence <Matthewswspence@gmail.com>
Date: Wed, 15 Jan 2025 20:42:28 -0600
Subject: [PATCH 12/20] refactor to simplify and make duplication logic generic

---
 src/registrar/views/transfer_user.py | 227 +++++++++------------------
 1 file changed, 71 insertions(+), 156 deletions(-)

diff --git a/src/registrar/views/transfer_user.py b/src/registrar/views/transfer_user.py
index fdef91b43..37b6a3cc0 100644
--- a/src/registrar/views/transfer_user.py
+++ b/src/registrar/views/transfer_user.py
@@ -1,6 +1,6 @@
 import logging
-from django.db import transaction
-from django.db.models import ForeignKey, OneToOneField, ManyToManyField, ManyToOneRel
+from django.db import transaction, IntegrityError
+from django.db.models import ForeignKey, OneToOneField, ManyToManyField, ManyToOneRel, ManyToManyRel, OneToOneRel, Model, UniqueConstraint
 
 from django.shortcuts import render, get_object_or_404, redirect
 from django.views import View
@@ -12,7 +12,8 @@ from django.contrib import messages
 
 from registrar.models.user_domain_role import UserDomainRole
 from registrar.models.user_portfolio_permission import UserPortfolioPermission
-from typing import List
+
+from psycopg2 import errorcodes
 
 logger = logging.getLogger(__name__)
 
@@ -65,9 +66,6 @@ class TransferUserView(View):
             with transaction.atomic():
                 change_logs = []
 
-                self._delete_duplicate_user_domain_roles_and_log(selected_user, current_user, change_logs)
-
-                self._delete_duplicate_user_portfolio_permissions_and_log(selected_user, current_user, change_logs)
                 # Dynamically handle related fields
                 self.transfer_related_fields_and_log(selected_user, current_user, change_logs)
 
@@ -87,70 +85,6 @@ class TransferUserView(View):
 
         return redirect("admin:registrar_user_change", object_id=user_id)
 
-    def _delete_duplicate_user_portfolio_permissions_and_log(self, selected_user, current_user, change_logs):
-        """
-        Check and remove duplicate UserPortfolioPermission objects from the selected_user based on portfolios associated with the current_user.
-        """
-        try:
-            # Fetch portfolios associated with the current_user
-            current_user_portfolios = UserPortfolioPermission.objects.filter(user=current_user).values_list(
-                "portfolio_id", flat=True
-            )
-
-            # Identify duplicates in selected_user for these portfolios
-            duplicates = UserPortfolioPermission.objects.filter(
-                user=selected_user, portfolio_id__in=current_user_portfolios
-            )
-
-            duplicate_count = duplicates.count()
-
-            if duplicate_count > 0:
-                # Log the specific duplicates before deletion for better traceability
-                duplicate_permissions = list(duplicates)
-                logger.debug(f"Duplicate permissions to be removed: {duplicate_permissions}")
-
-                duplicates.delete()
-                logger.info(
-                    f"Removed {duplicate_count} duplicate UserPortfolioPermission(s) from user_id {selected_user.id} for portfolios already associated with user_id {current_user.id}"
-                )
-                change_logs.append(
-                    f"Removed {duplicate_count} duplicate UserPortfolioPermission(s) from user_id {selected_user.id} for portfolios already associated with user_id {current_user.id}"
-                )
-
-        except Exception as e:
-            logger.error(f"Failed to check and remove duplicate UserPortfolioPermissions: {e}", exc_info=True)
-            raise
-
-    def _delete_duplicate_user_domain_roles_and_log(self, selected_user, current_user, change_logs):
-        """
-        Check and remove duplicate UserDomainRole objects from the selected_user based on domains associated with the current_user.
-        Retain one instance per domain to maintain data integrity.
-        """
-
-        try:
-            # Fetch domains associated with the current_user
-            current_user_domains = UserDomainRole.objects.filter(user=current_user).values_list("domain_id", flat=True)
-
-            # Identify duplicates in selected_user for these domains
-            duplicates = UserDomainRole.objects.filter(user=selected_user, domain_id__in=current_user_domains)
-
-            duplicate_count = duplicates.count()
-
-            if duplicate_count > 0:
-                duplicates.delete()
-                logger.info(
-                    f"Removed {duplicate_count} duplicate UserDomainRole(s) from user_id {selected_user.id} "
-                    f"for domains already associated with user_id {current_user.id}"
-                )
-                change_logs.append(
-                    f"Removed {duplicate_count} duplicate UserDomainRole(s) from user_id {selected_user.id} "
-                    f"for domains already associated with user_id {current_user.id}"
-                )
-
-        except Exception as e:
-            logger.error(f"Failed to check and remove duplicate UserDomainRoles: {e}", exc_info=True)
-            raise
-
     def transfer_related_fields_and_log(self, selected_user, current_user, change_logs):
         """
         Dynamically find all related fields to the User model and transfer them from selected_user to current_user.
@@ -158,108 +92,89 @@ class TransferUserView(View):
         """
         user_model = User
 
-        # Handle forward relationships
         for related_field in user_model._meta.get_fields():
-            if related_field.is_relation and related_field.related_model:
-                if isinstance(related_field, ForeignKey):
-                    self._handle_foreign_key(related_field, selected_user, current_user, change_logs)
-                elif isinstance(related_field, OneToOneField):
+            if related_field.is_relation:
+                # Field objects represent forward relationships
+                if isinstance(related_field, OneToOneField):
                     self._handle_one_to_one(related_field, selected_user, current_user, change_logs)
                 elif isinstance(related_field, ManyToManyField):
                     self._handle_many_to_many(related_field, selected_user, current_user, change_logs)
+                elif isinstance(related_field, ForeignKey):
+                    self._handle_foreign_key(related_field, selected_user, current_user, change_logs)
+                # Relationship objects represent reverse relationships
                 elif isinstance(related_field, ManyToOneRel):
-                    self._handle_many_to_one_rel(related_field, selected_user, current_user, change_logs)
+                    # ManyToOneRel is a reverse ForeignKey
+                    self._handle_foreign_key_reverse(related_field, selected_user, current_user, change_logs)
+                elif isinstance(related_field, OneToOneRel):
+                    self._handle_one_to_one_reverse(related_field, selected_user, current_user, change_logs)
+                elif isinstance(related_field, ManyToManyRel):
+                    self._handle_many_to_many_reverse(related_field, selected_user, current_user, change_logs)
+                else:
+                    logger.error(f"Unknown relationship type for field {related_field}")
+                    raise ValueError(f"Unknown relationship type for field {related_field}")
 
-        # # Handle reverse relationships
-        for related_object in user_model._meta.related_objects:
-            if isinstance(related_object, ManyToOneRel):
-                self._handle_many_to_one_rel(related_object, selected_user, current_user, change_logs)
-            elif isinstance(related_object.field, OneToOneField):
-                self._handle_one_to_one_reverse(related_object, selected_user, current_user, change_logs)
-            elif isinstance(related_object.field, ForeignKey):
-                self._handle_foreign_key_reverse(related_object, selected_user, current_user, change_logs)
-            elif isinstance(related_object.field, ManyToManyField):
-                self._handle_many_to_many_reverse(related_object, selected_user, current_user, change_logs)
-
-    def _handle_foreign_key(self, related_field: ForeignKey, selected_user, current_user, change_logs):
-        related_name = related_field.get_accessor_name()
-        related_manager = getattr(selected_user, related_name, None)
-
-        if related_manager.count() > 0:
-            related_queryset = related_manager.all()
-            for obj in related_queryset:
-                setattr(obj, related_field.field.name, current_user)
-                obj.save()
-                self.log_change(selected_user, current_user, related_field.field.name, change_logs)
-
-    def _handle_one_to_one(self, related_field: OneToOneField, selected_user, current_user, change_logs):
-        related_name = related_field.get_accessor_name()
-        related_object = getattr(selected_user, related_name, None)
-
-        if related_object:
-            setattr(related_object, related_field.field.name, current_user)
-            related_object.save()
+    def _handle_foreign_key_reverse(self, related_field: ManyToOneRel, selected_user, current_user, change_logs):
+        # Handle reverse ForeignKey relationships
+        related_manager = getattr(selected_user, related_field.get_accessor_name(), None)
+        if related_manager and related_manager.exists():
+            for related_object in related_manager.all():
+                # use an atomic transaction to set a save point in case of a unique constraint violation
+                with transaction.atomic():
+                    try:
+                        setattr(related_object, related_field.field.name, current_user)
+                        related_object.save()
+                    except IntegrityError as e:
+                        if e.__cause__.pgcode == errorcodes.UNIQUE_VIOLATION:
+                            # roll back to the savepoint, effectively ignoring this transaction
+                            continue
+                        else:
+                            raise e
             self.log_change(selected_user, current_user, related_field.field.name, change_logs)
 
-    def _handle_many_to_many(self, related_field: ManyToManyField, selected_user, current_user, change_logs):
-        related_manager = getattr(selected_user, related_field.name, None)
-        if related_manager.count() > 0:
-            related_queryset = related_manager.all()
-            getattr(current_user, related_field.name).add(*related_queryset)
+    def _handle_foreign_key(self, related_field: ForeignKey, selected_user, current_user, change_logs):
+        # Handle ForeignKey relationships
+        related_object = getattr(selected_user, related_field.name, None)
+        if related_object:
+            setattr(current_user, related_field.name, related_object)
+            current_user.save()
             self.log_change(selected_user, current_user, related_field.name, change_logs)
 
-    def _handle_many_to_one_rel(
-        self, related_object: ManyToOneRel, selected_user: User, current_user: User, change_logs: List[str]
-    ):
-        related_model = related_object.related_model
-        related_name = related_object.field.name
+    def _handle_one_to_one(self, related_field: OneToOneField, selected_user, current_user, change_logs):
+        # Handle OneToOne relationship
+        related_object = getattr(selected_user, related_field.name, None)
+        if related_object:
+            setattr(current_user, related_field.name, related_object)
+            current_user.save()
+            self.log_change(selected_user, current_user, related_field.name, change_logs)
 
-        related_queryset = related_model.objects.filter(**{related_name: selected_user})
+    def _handle_many_to_many(self, related_field: ManyToManyField, selected_user, current_user, change_logs):
+        # Handle ManyToMany relationship
+        related_name = related_field.remote_field.name
+        related_manager = getattr(selected_user, related_name, None)
+        if related_manager and related_manager.exists():
+            for instance in related_manager.all():
+                getattr(instance, related_name).remove(selected_user)
+                getattr(instance, related_name).add(current_user)
+            self.log_change(selected_user, current_user, related_name, change_logs)
 
-        if related_queryset.count() > 0:
-            for obj in related_queryset:
-                setattr(obj, related_name, current_user)
-                obj.save()
-                self.log_change(selected_user, current_user, related_name, change_logs)
+    def _handle_many_to_many_reverse(self, related_field: ManyToManyRel, selected_user, current_user, change_logs):
+        # Handle reverse relationship
+        related_name = related_field.field.name
+        related_manager = getattr(selected_user, related_name, None)
+        if related_manager and related_manager.exists():
+            for instance in related_manager.all():
+                getattr(instance, related_name).remove(selected_user)
+                getattr(instance, related_name).add(current_user)
+            self.log_change(selected_user, current_user, related_name, change_logs)
 
-    def _handle_one_to_one_reverse(
-        self, related_object: OneToOneField, selected_user: User, current_user: User, change_logs: List[str]
-    ):
-        related_model = related_object.related_model
-        field_name = related_object.field.name
-
-        try:
-            related_instance = related_model.objects.filter(**{field_name: selected_user}).first()
+    def _handle_one_to_one_reverse(self, related_field: OneToOneRel, selected_user, current_user, change_logs):
+        # Handle reverse relationship
+        field_name = related_field.get_accessor_name()
+        related_instance = getattr(selected_user, field_name, None)
+        if related_instance:
             setattr(related_instance, field_name, current_user)
             related_instance.save()
             self.log_change(selected_user, current_user, field_name, change_logs)
-        except related_model.DoesNotExist:
-            logger.warning(f"No related instance found for reverse OneToOneField {field_name} for {selected_user}")
-
-    def _handle_foreign_key_reverse(
-        self, related_object: ForeignKey, selected_user: User, current_user: User, change_logs: List[str]
-    ):
-        related_model = related_object.related_model
-        field_name = related_object.field.name
-
-        related_queryset = related_model.objects.filter(**{field_name: selected_user})
-
-        if related_queryset.count() > 0:
-            for obj in related_queryset:
-                setattr(obj, field_name, current_user)
-                obj.save()
-                self.log_change(selected_user, current_user, field_name, change_logs)
-
-    def _handle_many_to_many_reverse(
-        self, related_object: ManyToManyField, selected_user: User, current_user: User, change_logs: List[str]
-    ):
-        related_model = related_object.related_model
-        field_name = related_object.field.name
-
-        related_queryset = related_model.objects.filter(**{field_name: selected_user})
-        if related_queryset.count() > 0:
-            getattr(current_user, field_name).add(*related_queryset)
-            self.log_change(selected_user, current_user, field_name, change_logs)
 
     @classmethod
     def log_change(cls, selected_user, current_user, field_name, change_logs):

From 7b00c19c0e162f2a61189898bea3871e33c95a62 Mon Sep 17 00:00:00 2001
From: matthewswspence <Matthewswspence@gmail.com>
Date: Wed, 15 Jan 2025 21:23:30 -0600
Subject: [PATCH 13/20] refactor even more transfer user logic

---
 src/registrar/utility/db_helpers.py  | 19 +++++++++++++
 src/registrar/views/transfer_user.py | 41 ++++++++++++++--------------
 2 files changed, 39 insertions(+), 21 deletions(-)
 create mode 100644 src/registrar/utility/db_helpers.py

diff --git a/src/registrar/utility/db_helpers.py b/src/registrar/utility/db_helpers.py
new file mode 100644
index 000000000..cb23d4f3e
--- /dev/null
+++ b/src/registrar/utility/db_helpers.py
@@ -0,0 +1,19 @@
+from contextlib import contextmanager
+from django.db import transaction, IntegrityError
+from psycopg2 import errorcodes
+
+@contextmanager
+def ignore_unique_violation():
+    """
+    Execute within an atomic transaction so that if a unique constraint violation occurs,
+    the individual transaction is rolled back without invalidating any larger transaction.
+    """
+    with transaction.atomic():
+        try:
+            yield
+        except IntegrityError as e:
+            if e.__cause__.pgcode == errorcodes.UNIQUE_VIOLATION:
+                # roll back to the savepoint, effectively ignoring this transaction
+                pass
+            else:
+                raise e
diff --git a/src/registrar/views/transfer_user.py b/src/registrar/views/transfer_user.py
index 37b6a3cc0..9dd959d69 100644
--- a/src/registrar/views/transfer_user.py
+++ b/src/registrar/views/transfer_user.py
@@ -15,6 +15,8 @@ from registrar.models.user_portfolio_permission import UserPortfolioPermission
 
 from psycopg2 import errorcodes
 
+from registrar.utility.db_helpers import ignore_unique_violation
+
 logger = logging.getLogger(__name__)
 
 
@@ -118,33 +120,27 @@ class TransferUserView(View):
         related_manager = getattr(selected_user, related_field.get_accessor_name(), None)
         if related_manager and related_manager.exists():
             for related_object in related_manager.all():
-                # use an atomic transaction to set a save point in case of a unique constraint violation
-                with transaction.atomic():
-                    try:
-                        setattr(related_object, related_field.field.name, current_user)
-                        related_object.save()
-                    except IntegrityError as e:
-                        if e.__cause__.pgcode == errorcodes.UNIQUE_VIOLATION:
-                            # roll back to the savepoint, effectively ignoring this transaction
-                            continue
-                        else:
-                            raise e
+                with ignore_unique_violation():
+                    setattr(related_object, related_field.field.name, current_user)
+                    related_object.save()
             self.log_change(selected_user, current_user, related_field.field.name, change_logs)
 
     def _handle_foreign_key(self, related_field: ForeignKey, selected_user, current_user, change_logs):
         # Handle ForeignKey relationships
         related_object = getattr(selected_user, related_field.name, None)
         if related_object:
-            setattr(current_user, related_field.name, related_object)
-            current_user.save()
+            with ignore_unique_violation():
+                setattr(current_user, related_field.name, related_object)
+                current_user.save()
             self.log_change(selected_user, current_user, related_field.name, change_logs)
 
     def _handle_one_to_one(self, related_field: OneToOneField, selected_user, current_user, change_logs):
         # Handle OneToOne relationship
         related_object = getattr(selected_user, related_field.name, None)
         if related_object:
-            setattr(current_user, related_field.name, related_object)
-            current_user.save()
+            with ignore_unique_violation():
+                setattr(current_user, related_field.name, related_object)
+                current_user.save()
             self.log_change(selected_user, current_user, related_field.name, change_logs)
 
     def _handle_many_to_many(self, related_field: ManyToManyField, selected_user, current_user, change_logs):
@@ -153,8 +149,9 @@ class TransferUserView(View):
         related_manager = getattr(selected_user, related_name, None)
         if related_manager and related_manager.exists():
             for instance in related_manager.all():
-                getattr(instance, related_name).remove(selected_user)
-                getattr(instance, related_name).add(current_user)
+                with ignore_unique_violation():
+                    getattr(instance, related_name).remove(selected_user)
+                    getattr(instance, related_name).add(current_user)
             self.log_change(selected_user, current_user, related_name, change_logs)
 
     def _handle_many_to_many_reverse(self, related_field: ManyToManyRel, selected_user, current_user, change_logs):
@@ -163,8 +160,9 @@ class TransferUserView(View):
         related_manager = getattr(selected_user, related_name, None)
         if related_manager and related_manager.exists():
             for instance in related_manager.all():
-                getattr(instance, related_name).remove(selected_user)
-                getattr(instance, related_name).add(current_user)
+                with ignore_unique_violation():
+                    getattr(instance, related_name).remove(selected_user)
+                    getattr(instance, related_name).add(current_user)
             self.log_change(selected_user, current_user, related_name, change_logs)
 
     def _handle_one_to_one_reverse(self, related_field: OneToOneRel, selected_user, current_user, change_logs):
@@ -172,8 +170,9 @@ class TransferUserView(View):
         field_name = related_field.get_accessor_name()
         related_instance = getattr(selected_user, field_name, None)
         if related_instance:
-            setattr(related_instance, field_name, current_user)
-            related_instance.save()
+            with ignore_unique_violation():
+                setattr(related_instance, field_name, current_user)
+                related_instance.save()
             self.log_change(selected_user, current_user, field_name, change_logs)
 
     @classmethod

From 931aff915fa0f8406fa6532575946c8fe4d8a396 Mon Sep 17 00:00:00 2001
From: matthewswspence <Matthewswspence@gmail.com>
Date: Wed, 15 Jan 2025 21:27:07 -0600
Subject: [PATCH 14/20] linters and remove debug logs

---
 src/registrar/utility/db_helpers.py  | 1 +
 src/registrar/views/transfer_user.py | 9 ++-------
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/src/registrar/utility/db_helpers.py b/src/registrar/utility/db_helpers.py
index cb23d4f3e..5b7e0392c 100644
--- a/src/registrar/utility/db_helpers.py
+++ b/src/registrar/utility/db_helpers.py
@@ -2,6 +2,7 @@ from contextlib import contextmanager
 from django.db import transaction, IntegrityError
 from psycopg2 import errorcodes
 
+
 @contextmanager
 def ignore_unique_violation():
     """
diff --git a/src/registrar/views/transfer_user.py b/src/registrar/views/transfer_user.py
index 9dd959d69..31000257d 100644
--- a/src/registrar/views/transfer_user.py
+++ b/src/registrar/views/transfer_user.py
@@ -1,6 +1,6 @@
 import logging
-from django.db import transaction, IntegrityError
-from django.db.models import ForeignKey, OneToOneField, ManyToManyField, ManyToOneRel, ManyToManyRel, OneToOneRel, Model, UniqueConstraint
+from django.db import transaction
+from django.db.models import ForeignKey, OneToOneField, ManyToManyField, ManyToOneRel, ManyToManyRel, OneToOneRel
 
 from django.shortcuts import render, get_object_or_404, redirect
 from django.views import View
@@ -13,8 +13,6 @@ from django.contrib import messages
 from registrar.models.user_domain_role import UserDomainRole
 from registrar.models.user_portfolio_permission import UserPortfolioPermission
 
-from psycopg2 import errorcodes
-
 from registrar.utility.db_helpers import ignore_unique_violation
 
 logger = logging.getLogger(__name__)
@@ -72,13 +70,10 @@ class TransferUserView(View):
                 self.transfer_related_fields_and_log(selected_user, current_user, change_logs)
 
                 # Success message if any related objects were updated
-                logger.debug(f"change_logs: {change_logs}")
                 if change_logs:
-                    logger.debug(f"change_logs: {change_logs}")
                     success_message = f"Data transferred successfully for the following objects: {change_logs}"
                     messages.success(request, success_message)
 
-                logger.debug("Deleting old user")
                 selected_user.delete()
                 messages.success(request, f"Deleted {selected_user} {selected_user.username}")
         except Exception as e:

From 7e832c4a2bd01dc764c87ecb0143df446e2c1217 Mon Sep 17 00:00:00 2001
From: matthewswspence <Matthewswspence@gmail.com>
Date: Thu, 16 Jan 2025 09:34:53 -0600
Subject: [PATCH 15/20] remove unnecessary use of contextmanager

---
 src/registrar/views/transfer_user.py | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/registrar/views/transfer_user.py b/src/registrar/views/transfer_user.py
index 31000257d..fa66185ca 100644
--- a/src/registrar/views/transfer_user.py
+++ b/src/registrar/views/transfer_user.py
@@ -124,9 +124,8 @@ class TransferUserView(View):
         # Handle ForeignKey relationships
         related_object = getattr(selected_user, related_field.name, None)
         if related_object:
-            with ignore_unique_violation():
-                setattr(current_user, related_field.name, related_object)
-                current_user.save()
+            setattr(current_user, related_field.name, related_object)
+            current_user.save()
             self.log_change(selected_user, current_user, related_field.name, change_logs)
 
     def _handle_one_to_one(self, related_field: OneToOneField, selected_user, current_user, change_logs):
@@ -165,9 +164,8 @@ class TransferUserView(View):
         field_name = related_field.get_accessor_name()
         related_instance = getattr(selected_user, field_name, None)
         if related_instance:
-            with ignore_unique_violation():
-                setattr(related_instance, field_name, current_user)
-                related_instance.save()
+            setattr(related_instance, field_name, current_user)
+            related_instance.save()
             self.log_change(selected_user, current_user, field_name, change_logs)
 
     @classmethod

From 016c0851419abeacefaa7c726581beab0a6ca668 Mon Sep 17 00:00:00 2001
From: matthewswspence <Matthewswspence@gmail.com>
Date: Fri, 17 Jan 2025 15:33:41 -0600
Subject: [PATCH 16/20] Change log_change

---
 src/registrar/views/transfer_user.py | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/src/registrar/views/transfer_user.py b/src/registrar/views/transfer_user.py
index fa66185ca..ea9da33c3 100644
--- a/src/registrar/views/transfer_user.py
+++ b/src/registrar/views/transfer_user.py
@@ -118,7 +118,8 @@ class TransferUserView(View):
                 with ignore_unique_violation():
                     setattr(related_object, related_field.field.name, current_user)
                     related_object.save()
-            self.log_change(selected_user, current_user, related_field.field.name, change_logs)
+            obj_type = related_field.related_model.__name__
+            self.log_change(obj_type, selected_user, current_user, related_field.field.name, change_logs)
 
     def _handle_foreign_key(self, related_field: ForeignKey, selected_user, current_user, change_logs):
         # Handle ForeignKey relationships
@@ -126,7 +127,7 @@ class TransferUserView(View):
         if related_object:
             setattr(current_user, related_field.name, related_object)
             current_user.save()
-            self.log_change(selected_user, current_user, related_field.name, change_logs)
+            self.log_change(related_object, selected_user, current_user, related_field.name, change_logs)
 
     def _handle_one_to_one(self, related_field: OneToOneField, selected_user, current_user, change_logs):
         # Handle OneToOne relationship
@@ -135,7 +136,7 @@ class TransferUserView(View):
             with ignore_unique_violation():
                 setattr(current_user, related_field.name, related_object)
                 current_user.save()
-            self.log_change(selected_user, current_user, related_field.name, change_logs)
+            self.log_change(related_object.__name__, selected_user, current_user, related_field.name, change_logs)
 
     def _handle_many_to_many(self, related_field: ManyToManyField, selected_user, current_user, change_logs):
         # Handle ManyToMany relationship
@@ -146,7 +147,8 @@ class TransferUserView(View):
                 with ignore_unique_violation():
                     getattr(instance, related_name).remove(selected_user)
                     getattr(instance, related_name).add(current_user)
-            self.log_change(selected_user, current_user, related_name, change_logs)
+            obj_type = related_field.related_model.__name__
+            self.log_change(obj_type, selected_user, current_user, related_name, change_logs)
 
     def _handle_many_to_many_reverse(self, related_field: ManyToManyRel, selected_user, current_user, change_logs):
         # Handle reverse relationship
@@ -157,7 +159,8 @@ class TransferUserView(View):
                 with ignore_unique_violation():
                     getattr(instance, related_name).remove(selected_user)
                     getattr(instance, related_name).add(current_user)
-            self.log_change(selected_user, current_user, related_name, change_logs)
+            obj_type = related_field.related_model.__name__
+            self.log_change(obj_type, selected_user, current_user, related_name, change_logs)
 
     def _handle_one_to_one_reverse(self, related_field: OneToOneRel, selected_user, current_user, change_logs):
         # Handle reverse relationship
@@ -166,11 +169,11 @@ class TransferUserView(View):
         if related_instance:
             setattr(related_instance, field_name, current_user)
             related_instance.save()
-            self.log_change(selected_user, current_user, field_name, change_logs)
+            self.log_change(related_instance.__name__, selected_user, current_user, field_name, change_logs)
 
     @classmethod
-    def log_change(cls, selected_user, current_user, field_name, change_logs):
-        log_entry = f"Transferred {field_name} from {selected_user} to {current_user}"
+    def log_change(cls, obj, selected_user, current_user, field_name, change_logs):
+        log_entry = f"Changed {field_name} from {selected_user} to {current_user} on {obj}"
         logger.info(log_entry)
         change_logs.append(log_entry)
 

From eca4cd003aaa47855d580b8577cf41a21ddff2b2 Mon Sep 17 00:00:00 2001
From: matthewswspence <Matthewswspence@gmail.com>
Date: Fri, 17 Jan 2025 17:03:13 -0600
Subject: [PATCH 17/20] move logs into loops for some helper functions

---
 src/registrar/views/transfer_user.py | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/registrar/views/transfer_user.py b/src/registrar/views/transfer_user.py
index ea9da33c3..d030717b4 100644
--- a/src/registrar/views/transfer_user.py
+++ b/src/registrar/views/transfer_user.py
@@ -118,8 +118,7 @@ class TransferUserView(View):
                 with ignore_unique_violation():
                     setattr(related_object, related_field.field.name, current_user)
                     related_object.save()
-            obj_type = related_field.related_model.__name__
-            self.log_change(obj_type, selected_user, current_user, related_field.field.name, change_logs)
+                self.log_change(related_object.__name__, selected_user, current_user, related_field.field.name, change_logs)
 
     def _handle_foreign_key(self, related_field: ForeignKey, selected_user, current_user, change_logs):
         # Handle ForeignKey relationships
@@ -147,8 +146,7 @@ class TransferUserView(View):
                 with ignore_unique_violation():
                     getattr(instance, related_name).remove(selected_user)
                     getattr(instance, related_name).add(current_user)
-            obj_type = related_field.related_model.__name__
-            self.log_change(obj_type, selected_user, current_user, related_name, change_logs)
+                self.log_change(instance.__name__, selected_user, current_user, related_name, change_logs)
 
     def _handle_many_to_many_reverse(self, related_field: ManyToManyRel, selected_user, current_user, change_logs):
         # Handle reverse relationship
@@ -159,8 +157,7 @@ class TransferUserView(View):
                 with ignore_unique_violation():
                     getattr(instance, related_name).remove(selected_user)
                     getattr(instance, related_name).add(current_user)
-            obj_type = related_field.related_model.__name__
-            self.log_change(obj_type, selected_user, current_user, related_name, change_logs)
+                self.log_change(instance.__name__, selected_user, current_user, related_name, change_logs)
 
     def _handle_one_to_one_reverse(self, related_field: OneToOneRel, selected_user, current_user, change_logs):
         # Handle reverse relationship

From 9d8755f28052ce31045980f8323260f5db26982a Mon Sep 17 00:00:00 2001
From: matthewswspence <Matthewswspence@gmail.com>
Date: Fri, 17 Jan 2025 17:16:24 -0600
Subject: [PATCH 18/20] log objects instead of names

---
 src/registrar/views/transfer_user.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/registrar/views/transfer_user.py b/src/registrar/views/transfer_user.py
index d030717b4..f574b76d9 100644
--- a/src/registrar/views/transfer_user.py
+++ b/src/registrar/views/transfer_user.py
@@ -118,7 +118,7 @@ class TransferUserView(View):
                 with ignore_unique_violation():
                     setattr(related_object, related_field.field.name, current_user)
                     related_object.save()
-                self.log_change(related_object.__name__, selected_user, current_user, related_field.field.name, change_logs)
+                self.log_change(related_object, selected_user, current_user, related_field.field.name, change_logs)
 
     def _handle_foreign_key(self, related_field: ForeignKey, selected_user, current_user, change_logs):
         # Handle ForeignKey relationships
@@ -135,7 +135,7 @@ class TransferUserView(View):
             with ignore_unique_violation():
                 setattr(current_user, related_field.name, related_object)
                 current_user.save()
-            self.log_change(related_object.__name__, selected_user, current_user, related_field.name, change_logs)
+            self.log_change(related_object, selected_user, current_user, related_field.name, change_logs)
 
     def _handle_many_to_many(self, related_field: ManyToManyField, selected_user, current_user, change_logs):
         # Handle ManyToMany relationship
@@ -146,7 +146,7 @@ class TransferUserView(View):
                 with ignore_unique_violation():
                     getattr(instance, related_name).remove(selected_user)
                     getattr(instance, related_name).add(current_user)
-                self.log_change(instance.__name__, selected_user, current_user, related_name, change_logs)
+                self.log_change(instance, selected_user, current_user, related_name, change_logs)
 
     def _handle_many_to_many_reverse(self, related_field: ManyToManyRel, selected_user, current_user, change_logs):
         # Handle reverse relationship
@@ -157,7 +157,7 @@ class TransferUserView(View):
                 with ignore_unique_violation():
                     getattr(instance, related_name).remove(selected_user)
                     getattr(instance, related_name).add(current_user)
-                self.log_change(instance.__name__, selected_user, current_user, related_name, change_logs)
+                self.log_change(instance, selected_user, current_user, related_name, change_logs)
 
     def _handle_one_to_one_reverse(self, related_field: OneToOneRel, selected_user, current_user, change_logs):
         # Handle reverse relationship
@@ -166,7 +166,7 @@ class TransferUserView(View):
         if related_instance:
             setattr(related_instance, field_name, current_user)
             related_instance.save()
-            self.log_change(related_instance.__name__, selected_user, current_user, field_name, change_logs)
+            self.log_change(related_instance, selected_user, current_user, field_name, change_logs)
 
     @classmethod
     def log_change(cls, obj, selected_user, current_user, field_name, change_logs):

From 6905531061e48eb3f24b70f0d8c64982e9b3b34a Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 20 Jan 2025 07:34:00 -0500
Subject: [PATCH 19/20] fixing err message, and updating comment

---
 src/registrar/utility/email_invitations.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py
index 3653d4290..25e9db0f3 100644
--- a/src/registrar/utility/email_invitations.py
+++ b/src/registrar/utility/email_invitations.py
@@ -52,6 +52,11 @@ def send_domain_invitation_email(
 
 
 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
+    """
     # Get each domain manager from list
     user_domain_roles = UserDomainRole.objects.filter(domain=domain)
     for user_domain_role in user_domain_roles:
@@ -72,7 +77,7 @@ def send_emails_to_domain_managers(email: str, requestor_email, domain: Domain,
             )
         except EmailSendingError as err:
             raise EmailSendingError(
-                f"Could not send email manager notification to {user.email} for domains: {domain.name}"
+                f"Could not send email manager notification to {user.email} for domain: {domain.name}"
             ) from err
 
 

From 9b6f637270edcf96f74aa00e125cc7e4b0473c2e Mon Sep 17 00:00:00 2001
From: matthewswspence <Matthewswspence@gmail.com>
Date: Tue, 21 Jan 2025 10:45:19 -0600
Subject: [PATCH 20/20] fix unit test

---
 src/registrar/tests/test_admin.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index 8baf5e42d..d58ee59a2 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -2866,7 +2866,7 @@ class TestTransferUser(WebTest):
         with self.assertRaises(User.DoesNotExist):
             self.user2.refresh_from_db()
 
-    @less_console_noise_decorator
+    # @less_console_noise_decorator
     def test_transfer_user_throws_transfer_and_delete_success_messages(self):
         """Test that success messages for data transfer and user deletion are displayed."""
         # Ensure the setup for VerifiedByStaff
@@ -2884,11 +2884,13 @@ class TestTransferUser(WebTest):
 
             self.assertContains(after_submit, "<h1>Change user</h1>")
 
+            print(mock_success_message.call_args_list)
+
             mock_success_message.assert_any_call(
                 ANY,
                 (
-                    "Data transferred successfully for the following objects: ['Transferred requestor "
-                    + "from Furiosa Jabassa  to Max Rokatanski ']"
+                    "Data transferred successfully for the following objects: ['Changed requestor "
+                    + "from Furiosa Jabassa  to Max Rokatanski  on immortan.joe@citadel.com']"
                 ),
             )